Corrects OIDC group parsing in case userinfo_endpoint is not OIDC compliant #19772
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.
IMO, given remote/local are not declared in this func, this chunk should also be moved out...
There was a problem hiding this comment.
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?
Vad1mo
added
the
release-note/enhancement
|
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: