-
Notifications
You must be signed in to change notification settings - Fork 2k
claims: add JSON serialization for interface arrays #26958
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
base: main
Are you sure you want to change the base?
claims: add JSON serialization for interface arrays #26958
Conversation
|
Hi @othman-essabir and thanks for raising this PR. We are currently in a feature freeze due to the upcoming 1.11.0 release, so won't be reviewing this immediately, however, I have placed it onto our backlog ready for review for the 1.11.x milestone. |
tgross
left a comment
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.
I did a little digging and apparently this pattern is more common than I'd have thought. Shows up in JWTs issued by GCP too. A few top-level comments:
- Let's add a changelog describing this user-facing change (use
make clto generate the changelog file) - Let's add some test cases to
lib/auth/claims_test.gocovering this - It looks like most of this code was borrowed from Consul and is being incubated as a library to share. Let's make a PR to Consul as well (ref
oidcjwt.go#L195) as well to get their take on this.
|
Hi @tgross Thanks for taking a look ! I’ll add the changelog, the test cases in lib/auth/claims_test.go, and open a PR on Consul for their feedback as well. |
b56a616 to
b43b1e9
Compare
|
Hello, |
b43b1e9 to
9c5d79d
Compare
Implement handling of []interface{} types by serializing them to
JSON string format. This allows arrays like ["role1", "role2"] to
be converted to string representations for further processing.
9c5d79d to
c1342b6
Compare
tgross
left a comment
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.
LGTM! We'll get this merged after the 1.11.0 GA goes out.
The current implementation lacks support for parsing and handling array structures in OIDC claims, which severely limits SSO integration with OIDC providers like Keycloak. Previously, array values in OIDC claims were raising "converting claim" errors. The parsing failed to properly map user roles from array-based claims.
Description
[]interface{}that marshals arrays to JSON string format.Testing & Reproduction Steps
Before fix
Trying to map the
rolesclaim results in a 500 error:http: request failed: method=POST path=/v1/acl/oidc/complete-auth \ error="error converting claim 'roles' to string from unknown type []interface {}" code=500After fix
Example of the processed claim:
internal_claims = { "roles": "[\"role1\",\"role2\",\"roleN\"]" }Changes to Security Controls
Yes, this PR includes changes to security controls:
Access Controls: Enables proper role mapping from OIDC providers, ensuring users receive correct authorization levels