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

#167574878 Facebook Social Login #17

Merged
merged 1 commit into from
Aug 5, 2019

Conversation

Adekoreday
Copy link
Contributor

@Adekoreday Adekoreday commented Jul 31, 2019

Enable Social Login using facebook

What does this PR do?

  • This PR enables social authentication using facebook

Description of Task proposed in this pull request?

  • enables social login using facebook
  • ensure optimum test coverage

How should this be manually tested (Quality Assurance)?

npm install
npm run start: dev (for development server)

open your browser and navigate to 
http://localhost:5000/api/v1/auth/facebook
A dialogue prompting a user to authenticate their account via facebook
On authentication, the user would be created and their details returned. via a browser window
as shown below

What are the relevant pivotal tracker stories?

167574878

Any background context you want to add (Operations Impact)?

Ensure the localhost port = 5000, as this is the only port recognized by facebook in our configurations
Ensure the REDIRECT_URL= to the exact address in the .env.sample file
add the following in the .env file
FACEBOOK_APP_ID=
FACEBOOK_APP_SECRET=
REDIRECT_URL=http://localhost:5000/api/v1/auth/facebook/callback

What I have learned working on this feature:

I learned how to enable facebook login using passport.js
I learned how to stub functions using sinon.js

Screenshots:

Screenshot 2019-08-03 at 4 11 26 AM

@@ -0,0 +1,28 @@
export default {
Copy link

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,43 @@
import chai from 'chai';
Copy link

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,3 @@
import socialLogin from './socialLogin.test';
Copy link

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,8 @@
import express from 'express';
Copy link

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,17 @@
import express from 'express';
Copy link

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,4 @@
export default (error, req, res, next) => {
Copy link

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,16 @@
import passport from 'passport';
Copy link

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,16 @@
import passport from 'passport';
Copy link

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,31 @@
/* eslint-disable require-jsdoc */
import Debug from 'debug';
Copy link

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,12 @@
export default (sequelize, DataTypes) => {
Copy link

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'

@Adekoreday Adekoreday added the pr:wip PR is still a work in progress label Jul 31, 2019
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch 3 times, most recently from 072b15f to 070a327 Compare August 1, 2019 17:18
server/index.js Outdated
resave: true,
saveUninitialized: true
})
);
Copy link

Choose a reason for hiding this comment

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

Unexpected newline before ')' function-paren-newline

server/index.js Outdated
@@ -22,6 +21,13 @@ app.use(logger('dev'));
app.use(json());
app.use(urlencoded({ extended: false }));
app.use('/api-docs', swaggerUi.serve, swaggerUi.setup(swaggerDoc));
app.use(
Copy link

Choose a reason for hiding this comment

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

Unexpected newline after '(' function-paren-newline

cb(null, profile);
}
)
);
Copy link

Choose a reason for hiding this comment

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

Unexpected newline before ')' function-paren-newline

import { Strategy as FacebookStrategy } from 'passport-facebook';

export default () => {
passport.use(
Copy link

Choose a reason for hiding this comment

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

Unexpected newline after '(' function-paren-newline

@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from 070a327 to f7f1600 Compare August 1, 2019 19:20
serialize = sinon.stub(passport, 'serializeUser').yields({}, () => {});
deserialize = sinon.stub(passport, 'deserializeUser').yields({}, () => {});
sinons = sinon.stub(User, 'findOne').returns(false);
passportUse = sinon.stub(passport, 'use').callsFake(({}) => {});
Copy link

Choose a reason for hiding this comment

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

Unexpected empty object pattern no-empty-pattern

* @param {function} done express next function
* @returns {JSON} JSON object with details of new user
*/
const callback = (accessToken, refreshToken, profile, done) => done(null, profile);
Copy link

Choose a reason for hiding this comment

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

Line 11 exceeds the maximum line length of 80 max-len

@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from f7f1600 to 08c31bd Compare August 1, 2019 19:32

const { expect } = chai;
chai.use(chaiHttp);
let serialize, deserialize, sinons, passportUse;
Copy link

Choose a reason for hiding this comment

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

'passportUse' is defined but never used. Allowed unused vars must match /should|expect/ no-unused-vars

@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch 2 times, most recently from 24133b0 to 38d5436 Compare August 1, 2019 19:35
@Adekoreday Adekoreday added pr:review This is PR is currently under review by team and removed pr:wip PR is still a work in progress labels Aug 1, 2019
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from 38d5436 to 6cd00cd Compare August 1, 2019 19:55
@Adekoreday Adekoreday added pr:wip PR is still a work in progress and removed pr:review This is PR is currently under review by team labels Aug 2, 2019
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from 6cd00cd to e5f011d Compare August 2, 2019 14:00
userName: givenName
}
});
console.log('my users', users);
Copy link

Choose a reason for hiding this comment

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

Unexpected console statement no-console

import {
generateToken,
serverResponse,
serverError,
Copy link

Choose a reason for hiding this comment

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

'serverError' is defined but never used. Allowed unused vars must match /should|expect/ no-unused-vars

import models from '../database/models';
import {
generateToken,
serverResponse,
Copy link

Choose a reason for hiding this comment

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

'serverResponse' is defined but never used. Allowed unused vars must match /should|expect/ no-unused-vars

@Adekoreday Adekoreday added pr:review This is PR is currently under review by team and removed pr:wip PR is still a work in progress labels Aug 2, 2019
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch 2 times, most recently from 1842077 to 98a95ee Compare August 2, 2019 17:21
@Adekoreday Adekoreday added pr:wip PR is still a work in progress and removed pr:review This is PR is currently under review by team labels Aug 2, 2019
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from 98a95ee to e722d24 Compare August 2, 2019 18:22
* @param {function} done express next function
* @returns {JSON} JSON object with details of new user
*/
const callback = (accessToken, refreshToken, profile, done) => done(null, profile);
Copy link

Choose a reason for hiding this comment

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

Line 9 exceeds the maximum line length of 80 max-len

@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from e722d24 to 985de51 Compare August 2, 2019 18:31
@Adekoreday Adekoreday removed the pr:wip PR is still a work in progress label Aug 2, 2019
@Adekoreday Adekoreday closed this Aug 4, 2019
@Adekoreday Adekoreday reopened this Aug 4, 2019
@andela andela deleted a comment from Adekoreday Aug 4, 2019
henryade
henryade previously approved these changes Aug 5, 2019
JuwonAbiola
JuwonAbiola previously approved these changes Aug 5, 2019
chialuka
chialuka previously approved these changes Aug 5, 2019
@Adekoreday Adekoreday added pr:ready This PR is ready to be merged and removed pr:review This is PR is currently under review by team labels Aug 5, 2019
@Adekoreday Adekoreday dismissed stale reviews from chialuka, JuwonAbiola, and henryade via dcf9ec5 August 5, 2019 12:09
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from ff2b2b7 to dcf9ec5 Compare August 5, 2019 12:09
* @returns {Object} facebookUserData with details of new user
*/
const facebookData = (request) => {
const { email } = request.user._json;
Copy link

Choose a reason for hiding this comment

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

Unexpected dangling '_' in '_json' no-underscore-dangle

@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch 5 times, most recently from b054fa8 to 7a0c25a Compare August 5, 2019 13:05
henryade
henryade previously approved these changes Aug 5, 2019
chialuka
chialuka previously approved these changes Aug 5, 2019
JuwonAbiola
JuwonAbiola previously approved these changes Aug 5, 2019
userAgent,
ipAddress: ip
};
const Data = await createSocialUsers(data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Data -> user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted thanks

@Adekoreday Adekoreday dismissed stale reviews from JuwonAbiola, chialuka, and henryade via ef869be August 5, 2019 13:44
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from 7a0c25a to ef869be Compare August 5, 2019 13:44
#167573212 Implement Login Feature
@Adekoreday Adekoreday force-pushed the ft-social-auth-facebook-167574878 branch from ef869be to 6c19aed Compare August 5, 2019 13:51
Copy link
Contributor

@dinobi dinobi left a comment

Choose a reason for hiding this comment

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

@Adekoreday please be aware that this code will be refactored entirely when implementing Google and Twitter, because the logic will remain the same asides how we retrieve user data from the payload returned by passport callback. Please always think in the big picture when implementing solutions.

@dinobi dinobi merged commit 3411542 into develop Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready This PR is ready to be merged type:feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants