Skip to content
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

Fix for claims and client capabilities JSON merge bug in net6 #4452

Merged
merged 10 commits into from
Dec 6, 2023

Conversation

gladjohn
Copy link
Contributor

@gladjohn gladjohn commented Dec 4, 2023

Fixes : #4447

Identified a bug in MSAL's JSON merging logic affecting applications targeting .NET 6 and above. JSON objects with properties of the same name but different structures are not merged correctly, leading to data loss

// JSON 1
"access_token": {
    "xms_cc": {
        "values": ["cp1", "cp2"]
    }
}

// JSON 2
"access_token": {
    "nbf": {
        "essential": true,
        "value": "1701540593"
    }
}

problem : After merging, xms_cc is lost, retaining only the nbf property.

fix : This fix ensures that JSON merging is performed correctly, avoiding data loss when properties have the same name but different structures. The implementation considers various value kinds (objects, arrays) and recursively merges nested structures. It addresses the identified issue and provides a reliable merging mechanism for JSON objects.

Testing : Unit tests and manual verification that we pass capabilities now in net6.

Other notes : Unit tests also has coverage for net48 that uses newtonsoft code path, and we do not have this issue in that code path

@gladjohn gladjohn marked this pull request as ready for review December 4, 2023 22:58
@gladjohn gladjohn changed the title [draft] fix for json merge fix for json merge bug in net6 Dec 5, 2023
Copy link
Member

@bgavrilMS bgavrilMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve, but I agree with Peter's comment

@pmaytak pmaytak changed the title fix for json merge bug in net6 fix for claims and client capabilities JSON merge bug in net6 Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSI CAE Claims handling - claims merging issue
4 participants