Oauth unlink social accounts#253
Merged
AlexCuse merged 26 commits intoApr 13, 2024
Merged
Conversation
kion
approved these changes
Mar 28, 2024
AlexCuse
reviewed
Mar 29, 2024
AlexCuse
reviewed
Mar 30, 2024
diegosperes
force-pushed
the
oauth-unlink-social-accounts
branch
2 times, most recently
from
April 1, 2024 22:05
7f6cee1 to
ab77e1c
Compare
Merged
AlexCuse
self-requested a review
April 3, 2024 01:31
cainlevy
requested changes
Apr 4, 2024
cainlevy
left a comment
cainlevy
left a comment
Member
There was a problem hiding this comment.
Thanks for working on this! I'm sorry for the delay in reviewing.
AlexCuse
reviewed
Apr 5, 2024
…ic GET oauth/info endpoint, and private DELETE /account/{id}/oauth endpoint
…count from an user that does not exists.
diegosperes
force-pushed
the
oauth-unlink-social-accounts
branch
from
April 8, 2024 20:22
ea18267 to
bbce75d
Compare
Contributor
Author
AlexCuse
requested changes
Apr 10, 2024
AlexCuse
left a comment
AlexCuse
left a comment
Contributor
There was a problem hiding this comment.
This is looking really good think this is my last round of comments.
| @@ -0,0 +1,62 @@ | |||
| package services_test | |||
Contributor
|
@cainlevy do you think we need to worry about revoking the stored access token here or is it sufficient to "forget"? Looking at apple's implementation it would be fairly simple, though it does require an additional URL to be defined https://developer.apple.com/documentation/sign_in_with_apple/revoke_tokens/ A quick search on google and microsoft seems to indicate the tokens expire after an hour anyway so may be nothing to worry about there. |
AlexCuse
reviewed
Apr 11, 2024
diegosperes
force-pushed
the
oauth-unlink-social-accounts
branch
from
April 11, 2024 21:26
3c081af to
e20e128
Compare
diegosperes
force-pushed
the
oauth-unlink-social-accounts
branch
from
April 11, 2024 21:28
e20e128 to
5277772
Compare
cainlevy
approved these changes
Apr 12, 2024
cainlevy
left a comment
cainlevy
left a comment
Member
There was a problem hiding this comment.
thank you! it's looking good.
Member
Personally I would expect it's enough to forget them. Revoking them sounds very thorough but also would require a higher level of integration with each identity provider. |
AlexCuse
approved these changes
Apr 13, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Where the discussion started. Authn issue.
Summary
This PR is adding the ability of share oauth account information and delete them.
Changes
emailcolumn tooauth_accountstable in order to facilitate users identify their accounts.GET /account/{id}to returnoauth_providerscontaining OAuth information.DELETE /oauth/{provider}allowing user unlink OAuth accounts.GET /oauth/inforeturning oauth information for linked accounts.DELETE /account/{id}/oauthallowing unlink OAuth accounts for the given user.There's a particular use case that requires special attention. When a user registers via the OAuth flow, a random password is generated. To unlink the social account, Authn verifies whether the user needs to set a new password. As a side effect, a new error, NEW_PASSWORD_REQUIRED, has been introduced.