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
Corrects OIDC group parsing in case userinfo_endpoint is not OIDC compliant #19772
base: main
Are you sure you want to change the base?
Conversation
…pliant Signed-off-by: Vitor <vitor.barth@gmail.com>
@@ -403,6 +403,10 @@ func groupsFromClaims(gp claimsProvider, k string) ([]string, bool) { | |||
} | |||
res = append(res, s) | |||
} | |||
// if remote.Groups is not a []string, but local.Groups is, this will create a fallback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, given remote/local are not declared in this func, this chunk should also be moved out...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, given remote/local are not declared in this func, this chunk should also be moved out...
It makes sense, but IMO a comment in this location is necessary to explain this conditional.
What are your thoughts on changing the wording to something like
in case all elements of the groupClaim are invalid, this creates a fallback to the next groupClaim
?
This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days. |
This PR was closed because it has been stalled for 30 days with no activity. If this PR is still relevant, please re-open a new PR against main. |
@vbob @reasonerjt any progress on this PR? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #19772 +/- ##
===========================================
- Coverage 67.54% 46.10% -21.45%
===========================================
Files 991 246 -745
Lines 109167 13565 -95602
Branches 2719 2781 +62
===========================================
- Hits 73742 6254 -67488
+ Misses 31461 6993 -24468
+ Partials 3964 318 -3646
Flags with carried forward coverage won't be shown. Click here to find out more. |
Comprehensive Summary of your change
Issue being fixed
Fixes #19756
Please indicate you've done the following: