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

Silently migrate from OpenID 2.0 to OAuth2 authentication for Google. #564

Merged
merged 2 commits into from
Feb 5, 2015

Conversation

sizzlemctwizzle
Copy link
Member

Closes #484.

@sizzlemctwizzle sizzlemctwizzle added PR READY This is used to indicate that a pull request (PR) is ready for evaluation. expedite Immediate and on the front burner. labels Feb 5, 2015
@Martii
Copy link
Member

Martii commented Feb 5, 2015

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.

@sizzlemctwizzle
Copy link
Member Author

Whoops I spaced on new account creation. Give me some time to make sure that code won't explode.

@sizzlemctwizzle sizzlemctwizzle removed the PR READY This is used to indicate that a pull request (PR) is ready for evaluation. label Feb 5, 2015
@Martii
Copy link
Member

Martii commented Feb 5, 2015

K... appears that the stored auths item is the same value on dev for my google account... didn't expect that but it did reprompt me to access my profile instead of the usual deprecation warning I was getting and jerone used to create the issue.

@sizzlemctwizzle sizzlemctwizzle added the PR READY This is used to indicate that a pull request (PR) is ready for evaluation. label Feb 5, 2015
@sizzlemctwizzle
Copy link
Member Author

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.

@Martii
Copy link
Member

Martii commented Feb 5, 2015

changing your default to the Google strategy

Marti is my GH account on dev and Martii is my google account on dev... so hopefully we don't have a conflict now with that (assuming I understand you correctly here). I didn't write down my GH auth value on dev. :\

@Martii
Copy link
Member

Martii commented Feb 5, 2015

Hmmm... running a local pro on the latest HEAD of this PR here and Google doesn't appear in the drop down. If this goes to nodejitsu pro it may also not show up for new account creation... lemme recheck my checkout... confirmed Google is absent in local pro... so until deployed I can't corroborate new account creation.


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.

@sizzlemctwizzle
Copy link
Member Author

I just put the api creds in and now it appears on production locally.

@Martii
Copy link
Member

Martii commented Feb 5, 2015

Hmmm just got a 404 on account creation on local pro.

@sizzlemctwizzle
Copy link
Member Author

Because I set the callback up for production.
On Feb 5, 2015 3:00 AM, "Marti Martz" [email protected] wrote:

Hmmm just got a 404 on account creation on local pro.


Reply to this email directly or view it on GitHub
#564 (comment)
.

@Martii
Copy link
Member

Martii commented Feb 5, 2015

durr it's late... hehe... I forgot to change that redirect to localhost

@Martii
Copy link
Member

Martii commented Feb 5, 2015

New account creation corroborated. See pro @Martii ... want to give this a shot on nodejitsu pro.... merging soon?. I can roll back the snapshot back tomorrow if it fails at any point with API key blank out and you are unavailable.


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.

@Martii
Copy link
Member

Martii commented Feb 5, 2015

Right before deploy we might want to bump the project version number too to 0.1.7+0000000 so we know that a rollback could have consequences with this strategy (if not handled with care) after merged/deployed/lived.

@Martii Martii added CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. migration Use this to indicate that it may apply to an existing or announced migration. security Usually relates to something critical. labels Feb 5, 2015
Martii added a commit that referenced this pull request Feb 5, 2015
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.
@Martii Martii merged commit f0e47cb into master Feb 5, 2015
Martii pushed a commit to Martii/OpenUserJS.org that referenced this pull request Feb 5, 2015
@Martii Martii mentioned this pull request Feb 5, 2015
@Martii Martii removed expedite Immediate and on the front burner. PR READY This is used to indicate that a pull request (PR) is ready for evaluation. labels Feb 5, 2015
Martii added a commit that referenced this pull request Feb 5, 2015
* Update dispersed dep for #564
* Move David to tail end of md as not always accurate
@Martii Martii deleted the issue-484 branch October 3, 2015 20:27
@Martii Martii changed the title Silently migrate from OpenID to OAuth2 authentication for Google. Silently migrate from OpenID 2.0 to OAuth2 authentication for Google. Mar 12, 2019
Martii added a commit to Martii/OpenUserJS.org that referenced this pull request Mar 12, 2019
* 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
Martii added a commit to Martii/OpenUserJS.org that referenced this pull request Feb 6, 2020
* 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
Martii added a commit that referenced this pull request Feb 6, 2020
* 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
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CODE Some other Code related issue and it should clearly describe what it is affecting in a comment. migration Use this to indicate that it may apply to an existing or announced migration. security Usually relates to something critical.
Development

Successfully merging this pull request may close these issues.

Google OpenID 2.0 Deprecation
2 participants