feat(oidc): optimize the userinfo endpoint #7706
Merged
Conversation
This file contains 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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This reverts commit 5f0262f.
livio-a
reviewed
Apr 9, 2024
livio-a
approved these changes
Apr 9, 2024
muhlemmer
added a commit
that referenced
this pull request
Apr 13, 2024
When tokens were obtained using the client credentials grant, with audience and role scopes, userinfo would not return the role claims. This had multiple causes: 1. There is no auth request flow, so for legacy userinfo project data was never attached to the token 2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience. This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc. The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions. Fixes #6662
13 tasks
muhlemmer
added a commit
that referenced
this pull request
Apr 13, 2024
with audience and role scopes, userinfo would not return the role claims. For userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. Therefore we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience. This PR fixes it, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc. The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions. Fixes #6662
muhlemmer
added a commit
that referenced
this pull request
Apr 14, 2024
When tokens were obtained using the client credentials grant, with audience and role scopes, userinfo would not return the role claims. This had multiple causes: 1. There is no auth request flow, so for legacy userinfo project data was never attached to the token 2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience. This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc. The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions. Fixes #6662
|
🎉 This PR is included in version 2.50.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
livio-a
added a commit
that referenced
this pull request
Apr 16, 2024
* fix(oidc): roles in userinfo for client credentials token When tokens were obtained using the client credentials grant, with audience and role scopes, userinfo would not return the role claims. This had multiple causes: 1. There is no auth request flow, so for legacy userinfo project data was never attached to the token 2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience. This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc. The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions. Fixes #6662 * chore(deps): update all go deps (#7764) This change updates all go modules, including oidc, a major version of go-jose and the go 1.22 release. * Revert "chore(deps): update all go deps" (#7772) Revert "chore(deps): update all go deps (#7764)" This reverts commit 6893e7d. --------- Co-authored-by: Livio Spring <livio.a@gmail.com>
livio-a
pushed a commit
that referenced
this pull request
Apr 16, 2024
* fix(oidc): roles in userinfo for client credentials token When tokens were obtained using the client credentials grant, with audience and role scopes, userinfo would not return the role claims. This had multiple causes: 1. There is no auth request flow, so for legacy userinfo project data was never attached to the token 2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience. This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc. The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions. Fixes #6662 * chore(deps): update all go deps (#7764) This change updates all go modules, including oidc, a major version of go-jose and the go 1.22 release. * Revert "chore(deps): update all go deps" (#7772) Revert "chore(deps): update all go deps (#7764)" This reverts commit 6893e7d. --------- Co-authored-by: Livio Spring <livio.a@gmail.com> (cherry picked from commit 9ccbbe0)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Optimize the userinfo endpoint by reducing the amount of database queries.
Mostly reuses optimizations introduced by #6909. The legacy and trigger feature flags are also shared.
During implementation I found that the optimized introspection (and the new userinfo code from this PR) would always return the project roles, even if
project_role_assertionwas set to false. This is now fixed, however it seems that with v2 tokens with legacy userinfo & introspection never returned project roles if this setting was true. This bug was likely introduced with the v2 token implementation here:zitadel/internal/api/oidc/auth_request.go
Lines 260 to 263 in d39b32a
This issue and how to handle should be discussed in review.
Closes #6910
Definition of Ready