modify the OpenIDConnectAuthenticator.buildClaimMappings method to also handle the ArrayList type as an array#203
Open
rgl wants to merge 1 commit into
Open
Conversation
…so handle the ArrayList type as an array
|
|
This file contains hidden or 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
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.
Proposed changes in this pull request
When we configure Azure Entra ID as an OIDC IdP, wso2am does not correctly map the OIDC ID Token
rolesclaim to the user profileRolesproperty.For example, when wso2am receives this ID Token:
{ "aud": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeee0000", "iss": "https://login.microsoftonline.com/aaaaaaaa-bbbb-cccc-dddd-eeeeeeee0001/v2.0", "iat": 1, "nbf": 1, "exp": 1, "email": "john.doe@example.com", "name": "John Doe", "oid": "x", "preferred_username": "john.doe@example.com", "rh": "x", "roles": [ "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", // test-a "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb", // test-b "cccccccc-cccc-cccc-cccc-cccccccccccc" // test-c ], "sid": "x", "sub": "x", "tid": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeee0001", "uti": "x", "ver": "2.0" }And wso2am is configured to map the
cccccccc-cccc-cccc-cccc-ccccccccccccrole claim totest-c, the associatedRolesuser profile property, will end-up with that array being converted to a single string as (notice the[stray char):Instead of being:
This happens because there is a type confusion in the OpenIDConnectAuthenticator.buildClaimMappings function.
When that function is called to handle the
rolesID Token property, itsMap.Entry<String, Object> entryparameter receivesArrayListvalue object instead of aJSONArray, as such, the value is converted to a string usingArrayList.toString, and that will convert the array to the[aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa, bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb, cccccccc-cccc-cccc-cccc-cccccccccccc]string. But, later code expects that a multi-value claim to be serialized asaaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa,bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb,cccccccc-cccc-cccc-cccc-cccccccccccc(a comma separated string), so the finalrolesthat ends-up in the wso2am user profile is broken, and by extension, the claims mapping is also broken, as its executes without actually doing any mapping.When should this PR be merged
Not sure whether the
JSONArraytype should be handled at all. Maybe it should for backwards compatibility?Also, not sure whether we should check for the Iterable interface type instead.
So, please advise, and I will change this PR.
Follow up actions
[List any possible follow-up actions here; for instance, testing data
migrations, software that we need to install on staging and production
environments.]
Checklist (for reviewing)
General
Functionality
Code
Tests
Security
Documentation