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

CommonOAuth2Provider should use unique "id" field instead of non-unique "name" field. #4763

Closed
habuma opened this issue Oct 31, 2017 · 1 comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Milestone

Comments

@habuma
Copy link
Member

habuma commented Oct 31, 2017

Summary

CommonOAuth2Provider sets up a Facebook or GitHub client registration to use the "name" field as the user's name, which ultimately translates into the principal's name. But there's no guarantee of uniqueness on the "name" field. (It's easy to imagine there being hundreds/thousands of users whose name is "Jim Smith" on Facebook.)

If "Jim Smith" authenticates via Facebook, and then another "Jim Smith" also authenticates, the registry will contain the OAuth2AuthorizedClient for the 2nd one. When the application attempts to retrieve the OAuth2AuthorizedClient to get an access token and act on behalf of the first "Jim Smith", it will be the 2nd "Jim Smith" whose access token is retrieved. "Jim Smith" #1 will be reading and/or writing to "Jim Smith" #2's Facebook account.

Or consider the scenario where "Jim Smith" authenticates to an application via Facebook, performs some work resulting in data that is linked to his principal. Later, he changes his name on Facebook to "James Smith" and then authenticates into the application again. His data is missing, because it was linked to "Jim Smith", not "James Smith". (Actually, the data is not gone...just orphaned.)

Actual Behavior

I've not actually observed this behavior, but it is clear from looking at how OAuth2AuthorizedClientService uses the principal name as a key to store and retrieve OAuth2AuthorizedClient, that the described scenario could easily play out.

Expected Behavior

Each user would be uniquely identified by a unique identifier from Facebook and/or Google, not the non-unique "name" field.

Configuration

n/a

Version

Spring Security 5.0.0.RC1

Sample

n/a

@jgrandja jgrandja added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Nov 1, 2017
@jgrandja jgrandja added this to the 5.0.0 milestone Nov 1, 2017
@jgrandja
Copy link
Contributor

jgrandja commented Nov 1, 2017

Resolved via #4764

@jgrandja jgrandja closed this as completed Nov 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants