Skip to content
This repository has been archived by the owner on Jul 31, 2019. It is now read-only.

[WIP] Implement Social Sign-In #2435

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

[WIP] Implement Social Sign-In #2435

wants to merge 14 commits into from

Conversation

ryanwarsaw
Copy link
Contributor

@ryanwarsaw ryanwarsaw commented Aug 15, 2017

Warning: This is still is progress, don't merge!

@ryanwarsaw
Copy link
Contributor Author

Update

So this code actually compiles and runs now, but I want to do some better error handling before asking for a review, and considering pushing this to production. This does contain some breaking changes, the most prominent one being the changes to the .env file.

The passport-webmaker strategy also reformats the response from the id.webmaker.org server, so there is potential for breakage there, but I've gone through the code pretty in-depth to try and find as many if not all of the places where this could cause problems.

If you try to run this locally, make sure you re-populate your id.webmaker.org database with new dummy data, where you point the callback URL to /login/webmaker/callback rather than it's default which is /callback (since this is a non-existent route now).

@ryanwarsaw
Copy link
Contributor Author

Hey @gideonthomas I think this is ready for review, I've taken care of all the items I've outlined above, let me know if you've got any suggested changes, or need help getting this running.

This isn't a very easy PR to review, it'll probably take a few hours just a heads up.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
1 participant