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.

Sign up for GitHub

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

Jump to bottom

#164198183 Social authentication #24

Merged
merged 4 commits into from
Mar 12, 2019
Merged

#164198183 Social authentication #24

merged 4 commits into from
Mar 12, 2019

Conversation

Andhrah
Copy link

@Andhrah Andhrah commented Mar 11, 2019

What does this PR do?

It enables social authentication using facebook, twitter and google

Description of Task to be completed?

  • write unit test
  • write integration test
  • install passport
  • install passport strategy for facebook, twitter and google
  • configure passport and passport strategies
  • resolve conflicts

How should this be manually tested?

  • Follow the steps below:
    • Run the following commands in your terminal
      git clone https://github.com/andela/apollo-ah-backend.git
      cd apollo-ah-backend
  • Run npm install to install all dependencies.
  • Create your own .env file using the .env.example
  • Register or create your app on twitter developer, faceback developer and google developer site
  • Paste the following code in the javascript file created
  • Visit api/v1/auth/facebook to authenticate with facebook
  • api/v1/auth/twitter to authenticate with twitter
  • api/v1/auth/google to authenticate with google

Any background context you want to provide?

N/A

What are the relevant pivotal tracker stories?

#164198183

Screenshots (if appropriate)

facebook

google

twitter

Questions:

feat(social-authentication): enable social authentication
517551d 
- write unit test
- write integration test
- install passport
- install passport strategy for facebook, twitter and google
- configure passport and passport strategies

[Finish #164198183]
houndci-bot
houndci-bot reviewed Mar 11, 2019
View reviewed changes
test/unit/middlewares/socialAuth.test.js Show resolved Hide resolved
test/unit/middlewares/socialAuth.test.js Show resolved Hide resolved
test/unit/middlewares/socialAuth.test.js Show resolved Hide resolved
test/unit/middlewares/socialAuth.test.js Show resolved Hide resolved
test/unit/middlewares/socialAuth.test.js Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
@Andhrah Andhrah added WIP This pull request is a work in progress under-review This pull request is undergoing review labels Mar 11, 2019
@Andhrah Andhrah force-pushed the feature/164198183/social-authentication branch from 4a47178 to a66c80d Compare March 11, 2019 01:31
houndci-bot
houndci-bot reviewed Mar 11, 2019
View reviewed changes
test/integrations/routes/auth.test.js Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Outdated Show resolved Hide resolved
test/integrations/routes/auth.test.js Show resolved Hide resolved
test/integrations/routes/auth.test.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
@Andhrah Andhrah added ready This pull request is ready for review and removed WIP This pull request is a work in progress labels Mar 11, 2019
aanchirinah
aanchirinah approved these changes Mar 11, 2019
View reviewed changes
Copy link
Contributor

@aanchirinah aanchirinah left a comment

Choose a reason for hiding this comment

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

LGTM

@aanchirinah
Copy link
Contributor

@Andraquin , You have failing tests and builds

@Andhrah Andhrah added WIP This pull request is a work in progress and removed ready This pull request is ready for review labels Mar 11, 2019
houndci-bot
houndci-bot reviewed Mar 11, 2019
View reviewed changes
routes/auth.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
routes/auth.js Show resolved Hide resolved
controllers/auth.js Outdated Show resolved Hide resolved
controllers/auth.js Outdated Show resolved Hide resolved
controllers/auth.js Outdated Show resolved Hide resolved
auth/passport.js Outdated Show resolved Hide resolved
auth/passport.js Outdated Show resolved Hide resolved
@jomadoye
Copy link

jomadoye commented Mar 12, 2019 •
edited
Loading

Hi @Andraquin If there's a Passport strategy for medium.com please add it.

cc @aanchirinah

@Andhrah Andhrah force-pushed the feature/164198183/social-authentication branch from 357526a to 4decc7e Compare March 12, 2019 08:33
houndci-bot
houndci-bot reviewed Mar 12, 2019
View reviewed changes
auth/passport.js Outdated Show resolved Hide resolved
auth/passport.js Outdated Show resolved Hide resolved
auth/passport.js Outdated Show resolved Hide resolved
auth/passport.js Outdated Show resolved Hide resolved
auth/passport.js Outdated Show resolved Hide resolved
auth/passport.js Show resolved Hide resolved
@Andhrah Andhrah force-pushed the feature/164198183/social-authentication branch from 4decc7e to 9e87d00 Compare March 12, 2019 11:12
davdwhyte87
davdwhyte87 reviewed Mar 12, 2019
View reviewed changes
Copy link
Contributor

@davdwhyte87 davdwhyte87 left a comment

Choose a reason for hiding this comment

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

Nice work. Check, the eslinting, and your jsdoc in the controller, auth.js, then you could rename your controller to authController.js

(social authentication): document api
b1655d9 
- rename controllers/auth.js to controllers.authController.js
- document api endpoint using swagger

[Finish #164198183]
@Andhrah
Copy link
Author

Andhrah commented Mar 12, 2019

Nice work. Check, the eslinting, and your jsdoc in the controller, auth.js, then you could rename your controller to authController.js

@davdwhyte87 Thank you, I have attended to the above mentioned

@Andhrah
Copy link
Author

Andhrah commented Mar 12, 2019

Hi @Andraquin If there's a Passport strategy for medium.com please add it.

cc @aanchirinah

Hi, @jomadoye there is no passport strategy for medium.com
cc @aanchirinah

@davdwhyte87 davdwhyte87 self-requested a review March 12, 2019 12:49
davdwhyte87
davdwhyte87 approved these changes Mar 12, 2019
View reviewed changes
Copy link
Contributor

@davdwhyte87 davdwhyte87 left a comment

Choose a reason for hiding this comment

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

LGTM

@Andhrah
Copy link
Author

Andhrah commented Mar 12, 2019

@Andraquin , You have failing tests and builds

@aanchirinah, I have it fixed now

@Andhrah Andhrah added the ready This pull request is ready for review label Mar 12, 2019
@Andhrah Andhrah removed WIP This pull request is a work in progress under-review This pull request is undergoing review labels Mar 12, 2019
@aanchirinah aanchirinah merged commit 8a7eaf7 into staging Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@aanchirinah aanchirinah aanchirinah approved these changes

@davdwhyte87 davdwhyte87 davdwhyte87 approved these changes

Assignees
No one assigned
Labels
ready This pull request is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Andhrah @aanchirinah @jomadoye @houndci-bot @davdwhyte87