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

feat: add support for groups when roles are missing #55

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

vknabel
Copy link
Contributor

@vknabel vknabel commented Feb 7, 2025

Description

In practice not all OIDC providers expose roles. Some expose groups instead. We still prefer roles when possible, but fall back to groups. Alternatively a new type of user extractor would be required to be introduced, effectively requiring to add a new config field for iam tenant configs and still adding the same config field to the generic claims.

This should be not break any clients.

@vknabel vknabel requested a review from majst01 February 7, 2025 12:51
@vknabel vknabel requested a review from a team as a code owner February 7, 2025 12:51
@vknabel vknabel force-pushed the add-support-for-groups-when-roles-missing branch from 14ab38f to 94c9992 Compare February 7, 2025 12:52
In practice not all OIDC providers expose roles. Some expose groups instead. We still prefer roles when possible, but fall back to groups. Alternatively a new type of user extractor would be required to be instroduced, effectively requiring to add a new config field for iam tenant configs.
@vknabel vknabel force-pushed the add-support-for-groups-when-roles-missing branch from 94c9992 to 121228c Compare February 7, 2025 12:57
@Gerrit91
Copy link
Contributor

This would only be breaking if there were tokens that contain both claims at the time being and then the new roles claim would overwrite the groups claim. But as far as I know there are no such tokens around.

@vknabel
Copy link
Contributor Author

vknabel commented Feb 10, 2025

It could only break the current behavior, if roles would be empty in the token, and it would include groups with a group name in the correct format. Highly unlikely and would point to an error in the configuration of the OIDC provider itself.

@vknabel vknabel merged commit 17e8ad3 into master Feb 10, 2025
1 check passed
@vknabel vknabel deleted the add-support-for-groups-when-roles-missing branch February 10, 2025 12:14
@Gerrit91
Copy link
Contributor

/cc @michaelottofits

@michaelottofits
Copy link

The authorization information in the token claims can be configured with the OIDC provider. It does not matter whether the claim is named roles or groups or whatever. I only see these requirements for OIDC providers that may not have such a standard configuration. I wouldn't change anything here

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

Successfully merging this pull request may close these issues.

4 participants