Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#162727467 Enable social login #18

Merged
merged 5 commits into from
Jan 22, 2019

Conversation

abejide001
Copy link
Contributor

What does this PR do?

Implement Social(twitter, facebook and google) authentication

Description of Task to be completed?

  • Write logic for login via twitter using passport twitter
  • Write logic for login via facebook using passport facebook
  • Write logic for login via google using passport google
  • Write test for social controller
  • Modify password and userType to allowNull

How should this be manually tested?

  • Run npm i
  • Please contact the developer for application secrets
  • On the browser, enter the following URL
    -for facebook localhost:9000/api/v1/auth/facebook
    -for twitter localhost:9000/api/v1/auth/twitter
    -for google localhost:9000/api/v1/auth/google
    The URL return the following data
    message and token

Any background context you want to provide?

Twitter takes time to supply the consumer secret(about 5hours), and allowNull was set to true for password and userType, because the response coming in does not contain password, and the userType can be edited after the logs in and edit profile

What are the relevant pivotal tracker stories?

162727467

Screenshots (if appropriate)

screen shot 2019-01-14 at 11 53 46 pm

screen shot 2019-01-15 at 7 52 25 am

#### Questions: N/A

Implement social controller
Add services folder, and implement strategy for twitter, facebook, and google
Modify migrations, and allowNull for password and signUpType
Write test for Social Controller
[Delivers #162727467]
Implement social controller
Add services folder, and implement strategy for twitter, facebook, and google
Modify migrations, and allowNull for password and signUpType
Write test for Social Controller
Fix merge conflicts
[Delivers 162727467]
@@ -0,0 +1,92 @@
import 'babel-polyfill';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,32 @@
/* eslint-disable no-underscore-dangle */
import passport from 'passport';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,25 @@
import express from 'express';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

@@ -0,0 +1,174 @@
/* eslint-disable no-underscore-dangle */
/* eslint-disable camelcase */
import models from '../db/models';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

passport.authenticate('twitter', { scope: ['include_email =true'] }));

socialRouter.get('/twitter/callback',
passport.authenticate('twitter'), SocialController.getToken);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline before ')' function-paren-newline

socialRouter.get('/twitter',
passport.authenticate('twitter', { scope: ['include_email =true'] }));

socialRouter.get('/twitter/callback',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline after '(' function-paren-newline

passport.authenticate('google'), SocialController.getToken);

socialRouter.get('/twitter',
passport.authenticate('twitter', { scope: ['include_email =true'] }));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline before ')' function-paren-newline

socialRouter.get('/google/callback',
passport.authenticate('google'), SocialController.getToken);

socialRouter.get('/twitter',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline after '(' function-paren-newline

passport.authenticate('google', { scope: ['profile', 'email'] }));

socialRouter.get('/google/callback',
passport.authenticate('google'), SocialController.getToken);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline before ')' function-paren-newline


socialRouter.get('/facebook/callback',
passport.authenticate('facebook'), SocialController.getToken);
socialRouter.get('/google',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline after '(' function-paren-newline

passport.authenticate('facebook', { scope: ['email'] }));

socialRouter.get('/facebook/callback',
passport.authenticate('facebook'), SocialController.getToken);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline before ')' function-paren-newline

socialRouter.get('/facebook',
passport.authenticate('facebook', { scope: ['email'] }));

socialRouter.get('/facebook/callback',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline after '(' function-paren-newline

const socialRouter = express.Router();

socialRouter.get('/facebook',
passport.authenticate('facebook', { scope: ['email'] }));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline before ')' function-paren-newline


const socialRouter = express.Router();

socialRouter.get('/facebook',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected a newline after '(' function-paren-newline

Copy link

@andela-dbamidele andela-dbamidele left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good 👍

});
}
userDetails.newUser = isCreated;
return done(null, userDetails);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using a callback? It's required by passport?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is

const {
displayName, emails,
} = profile;
const { bio } = profile || null;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will crash the app when/if profile is not available and it resolves to null; You might want to do something like

const bio = profile.bio || null;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, on it.

displayName, emails,
} = profile;
const { bio } = profile || null;
const { imgURL } = profile || null;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

const {
displayName, emails,
} = profile;
const { bio } = profile || null;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, on it.

displayName, emails,
} = profile;
const { bio } = profile || null;
const { imgURL } = profile || null;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, on it.

Copy link
Contributor

@julietezekwe julietezekwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@wombolo wombolo self-requested a review January 21, 2019 12:14
Copy link
Contributor

@wombolo wombolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andela-dbamidele andela-dbamidele merged commit 9887ced into staging Jan 22, 2019
@andela-dbamidele andela-dbamidele deleted the feature/162727467/enable-social-login branch January 22, 2019 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants