Skip to content
This repository has been archived by the owner on Dec 3, 2021. It is now read-only.

Replace the usage of sub by preferred_username as login string #72

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

juanvaccarezza
Copy link

Use the preferred_username in claim as login name if available, if not
use sub.
Use the given_name in claim as name if available, if not use login.

This was tested using keycloak.
The rationale of this change is that it is easier to manage groups and permissions using a login name rather than the sub (which is a horrible string).

Sub is still being used to identify the provided accounts.

Use the preferred_username in claim as login name if available, if not
use sub.
Use the given_name in claim as name if available, if not use login.
@juanvaccarezza
Copy link
Author

Following information about the openid connect standadard.

https://openid.net/specs/openid-connect-core-1_0.html

The following sections are pertinenet to the standard way of using claims.

  1. ID Token

sub
REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII characters in length. The sub value is a case sensitive string.

5.1. Standard Claims

sub
Subject - Identifier for the End-User at the Issuer.

preferred_username
Shorthand name by which the End-User wishes to be referred to at the RP, such as janedoe or j.doe. This value MAY be any valid JSON string including special characters such as @, /, or whitespace. The RP MUST NOT rely upon this value being unique, as discussed in Section 5.7.

Copy link
Contributor

@mguimard mguimard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the PR. This seems great, however, the preferred_username in claims can possibly be already in use in the ACCOUNT table, this will lead to failure when trying to create the account.
The given_name part is OK.

@juanvaccarezza
Copy link
Author

Hi,
I see your point.
The problem is that there won't throw any exeption and successfully create the account. And then there will be 2 users with the same loginname

I think is better to have the login name extracted from the preffered_username in claims, since it the one the user needs to loging right?

What do you think is a better approach to create a constraint on the Account bean, or maybe add this logic to oAuthManager.findAvailableLogin?

I prefer the first approach. And just throw an error claiming that the user name has been taken.

If you gave me some directions I can provide this changes along with some tests maybe?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants