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

Update logic for matching requested claims for AT #2401

Merged
merged 16 commits into from
May 28, 2024

Conversation

sansrivastav
Copy link
Contributor

@sansrivastav sansrivastav commented May 5, 2024

Current logic of matching credentials only compares requested claims when we specify requested claims.
So, if there are 2 accesss tokens in storage:
AT1 - with no requested claims specified
AT2 - with a specific requested claims RC2

And then we query the storage without specifying any claims, current logic returns both AT1 and AT2.

The proposed change overrides getCredentialsFilteredBy() adding an extra boolean mustMatchExactClaims. When this parameter is set, we match claims explicitly even when no claims are provided. In the case above, only AT1 will be returned.
We will call this new overloaded method from MSAL CPP so that the behavior of MSAL CPP is same across all platforms including Android.

@sansrivastav sansrivastav marked this pull request as ready for review May 22, 2024 01:30
@sansrivastav sansrivastav requested a review from a team as a code owner May 22, 2024 01:30
@sansrivastav sansrivastav changed the title Handle empty scopes Update logic for matching requested claims for AT May 22, 2024
Copy link
Collaborator

@nickbopp nickbopp left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@mohitc1 mohitc1 left a comment

Choose a reason for hiding this comment

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

Left a suggestion.

@sansrivastav sansrivastav merged commit d696fd7 into dev May 28, 2024
18 checks passed
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.

None yet

4 participants