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.

Sign up for GitHub

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

Jump to bottom

2130019. Fix cache credential look-up. #5484

Merged
merged 3 commits into from Dec 20, 2022
Merged

2130019. Fix cache credential look-up. #5484

merged 3 commits into from Dec 20, 2022

Conversation

konstantin-msft
Copy link
Collaborator

Changes:

  • Fix cache credential look-up.
  • Add unit tests.

@github-actions github-actions bot added the msal-common Related to msal-common package label Dec 14, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2022 •
edited

Codecov Report

Merging #5484 (5261c3a) into dev (cbf8b83) will increase coverage by 0.47%.
The diff coverage is 100.00%.

Flag Coverage Δ
msal-angular 96.50% <ø> (?)
msal-browser 86.50% <ø> (+0.01%) ⬆️
msal-common 85.28% <100.00%> (?)
msal-node 83.37% <ø> (?)
msal-node-extensions 76.03% <ø> (ø)
msal-react 94.68% <ø> (?)
node-token-validation 88.88% <ø> (?)
Impacted Files Coverage Δ
...msal-common/src/cache/entities/CredentialEntity.ts 95.45% <100.00%> (ø)
lib/msal-node/src/cache/TokenCache.ts 75.90% <0.00%> (ø)
...de/src/cache/distributed/DistributedCachePlugin.ts 100.00% <0.00%> (ø)
...sal-common/src/cache/entities/AppMetadataEntity.ts 90.00% <0.00%> (ø)
lib/msal-react/src/hooks/useMsal.ts 100.00% <0.00%> (ø)
lib/msal-common/src/authority/ProtocolMode.ts 100.00% <0.00%> (ø)
lib/msal-angular/src/msal.module.ts 100.00% <0.00%> (ø)
...src/telemetry/performance/StubPerformanceClient.ts 100.00% <0.00%> (ø)
lib/msal-react/src/utils/utilities.ts 84.00% <0.00%> (ø)
lib/msal-common/src/account/CcsCredential.ts 100.00% <0.00%> (ø)
... and 115 more

@konstantin-msft konstantin-msft marked this pull request as ready for review December 14, 2022 18:04
derisen
derisen approved these changes Dec 19, 2022
View reviewed changes
Copy link
Contributor

@derisen derisen left a comment

Choose a reason for hiding this comment

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

LGTM! I think this fixes #5313, so you might wanna link that issue

@konstantin-msft konstantin-msft linked an issue Dec 20, 2022 that may be closed by this pull request
Azure AD B2C user flow name can break token cache #5313
Closed
@konstantin-msft konstantin-msft merged commit 1510e6a into dev Dec 20, 2022
@konstantin-msft konstantin-msft deleted the 2130019_fix_cache_cred_lookup branch December 20, 2022 20:00
@ghost
Copy link

ghost commented Jan 9, 2023

🎉@azure/msal-common@v9.0.2 has been released which incorporates this pull request.:tada:

We recommend upgrading to the latest version of @azure/msal-browser or @azure/msal-node to take advantage of this change.

Handy links:

adamblok
adamblok reviewed Jan 12, 2023
View reviewed changes
lib/msal-common/src/cache/entities/CredentialEntity.ts
return CredentialType.ACCESS_TOKEN_WITH_AUTH_SCHEME;
const separator = Separators.CACHE_KEY_SEPARATOR;
// Match host names like "login.microsoftonline.com", "https://accounts.google.com:4000", etc.
const domainRe = "(https?:\\/\\/)?([\\w-]+\\.)*([\\w-]{1,63})(\\.(\\w{2,3}))(\\:[0-9]{4,5})?";
Copy link

Choose a reason for hiding this comment

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

This pattern produces error in Safari browsers, see: #5561

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamblok adamblok adamblok left review comments

@tnorling tnorling tnorling approved these changes

@derisen derisen derisen approved these changes

@sameerag sameerag Awaiting requested review from sameerag sameerag is a code owner

@hectormmg hectormmg Awaiting requested review from hectormmg hectormmg is a code owner

@jo-arroyo jo-arroyo Awaiting requested review from jo-arroyo jo-arroyo is a code owner

@peterzenz peterzenz Awaiting requested review from peterzenz peterzenz is a code owner

@bgavrilMS bgavrilMS Awaiting requested review from bgavrilMS

@Robbie-Microsoft Robbie-Microsoft Awaiting requested review from Robbie-Microsoft Robbie-Microsoft is a code owner

Assignees
No one assigned
Labels
msal-common Related to msal-common package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Azure AD B2C user flow name can break token cache
5 participants
@konstantin-msft @codecov-commenter @adamblok @tnorling @derisen