Skip to content

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

Closed
@habuma

Description

@habuma

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    in: oauth2An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose)

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions