-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Protected resource map fix by sequencing matching protected resources keys #7274
Open
abhixkj
wants to merge
17
commits into
AzureAD:dev
Choose a base branch
from
abhixkj:7173-protectedResourceMap-fix-abhixkj
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
11a0e9d
Revert 7137
abhixkj 8a6664f
Reverted msal.interceptor.config.ts
abhixkj 9771e10
Adding Indexing to matched Scopes
abhixkj cc95d0d
remove relativeResourcesWithIndex
abhixkj a95566f
Fixed breaking changes from PR #7137
abhixkj 99dcf05
Change files
abhixkj cd9a570
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj 4b6404a
Refactor Code Add comments
abhixkj fdec331
Refactor II
abhixkj 85c4979
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj 8913415
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj 55e3875
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj e679534
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj aa3d9bd
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj 2699ea1
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj a044640
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj c159da7
Merge branch 'dev' into 7173-protectedResourceMap-fix-abhixkj
abhixkj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
7 changes: 7 additions & 0 deletions
7
change/@azure-msal-angular-e152bfdb-3816-46c9-8731-0c5646710895.json
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "minor", | ||
"comment": "Fixed breaking changes from PR #7137", | ||
"packageName": "@azure/msal-angular", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
Why
Math.max
here instead of directly usingindexn
? What's the scenario whereindexn
is < 0?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 also don't quite understand what's happening here w.r.t. index. Why are you looking for the index of the key, then later sorting by index? I see comments around what is happening but it would be helpful to also include comments about why
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.
In issue #7111 , the user had reported a sample request like
https://MY_API_SITE_1/api/sites?$filter=siteUrl eq 'https://MY_API_SITE_2'
which was failing, so this logic matches the pattern for key in the URL and tries to pick the first match, I am just storing the index as 0 if no match found. All this logic to handle the request mentioned in #7111. It was difficult to explain in comments as these requests with kind of sub URL is rare. Hope you could help me with the comments.
You may check the test cases added by jo-arroyo for #7111 fix. this logic targets that fix in a way such that it does not break existing matching logic with path-scopes matching.