-
Notifications
You must be signed in to change notification settings - Fork 319
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
Silently migrate from OpenID 2.0 to OAuth2 authentication for Google. #564
Conversation
5989ab3
to
3af7fad
Compare
Test driving shortly. I assume that pro is offline until this is deployed so give me a few moments to check out the PR and see if it's all peachy with another account. I separated my pro account to only be GH so I should be able to see migration as well as new account creation. |
Whoops I spaced on new account creation. Give me some time to make sure that code won't explode. |
K... appears that the stored |
…oogle using OAuth.
Okay, I've tested signing up under the new OAuth code and changing your default to the Google strategy. I already migrated my OpenID accounts on development, so I can't retest my migration. |
Hmmm... running a local pro on the latest HEAD of this PR here and Did you blank out the OAuth key? that might make it go away on local pro... I noticed it's restarted on nodejitsu pro. We should probably fix that btw... if an OAuth API key/value is present in the DB but the package isn't don't load that passport strategy. Anywhoo I'm +1 as I can't foresee any other issues at this time. |
I just put the api creds in and now it appears on production locally. |
Hmmm just got a 404 on account creation on local pro. |
Because I set the callback up for production.
|
durr it's late... hehe... I forgot to change that redirect to localhost |
New account creation corroborated. See pro Only thing I forgot to remind you of is the possible expiration of that API key... in my tests it seemed like it expired my key after a period... e.g. I think the refreshToken wasn't handled in my twiddlings. |
Right before deploy we might want to bump the project version number too to |
Silently migrate from OpenID to OAuth2 authentication for Google. Merge... concensus... hopefully the API key doesn't expire but nodejitsu pro is currently stopped as I expected... so getting this rolling so it can be up again. Dev may exhibit a similar behavior at some point since the id is the same as pro but different key.
See also: * OpenUserJS#564 (comment)
* Update dispersed dep for #564 * Move David to tail end of md as not always accurate
* This ends our migration of OpenID 2.0 to OAuth 2.0 which google ended quite a while ago * This also uses OAuth 2.0 exclusively ... same token returned on tested existing and new account Closes OpenUserJS#889
* Too many nested if/else's makes it highly unreadable between OAuth and OpenID i.e. easier commit history searching. Been thinking about doing this since removal commit... doing it. * Probable location to add `aToken` to which is actually `aAccessToken` in dep README's. Possible location could be `aReq.session.accessToken` Applies to OpenUserJS#1705 Post OpenUserJS#889 OpenUserJS#564
* Too many nested if/else's makes it highly unreadable between OAuth and OpenID i.e. easier commit history searching. Been thinking about doing this since removal commit... doing it. * Probable location to add `aToken` to which is actually `aAccessToken` in dep README's. Possible location could be `aReq.session.accessToken` Applies to #1705 Post #889 #564 Auto-merge
Closes #484.