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 in-progress temporary cache in v1 #4014

Merged
merged 5 commits into from Sep 7, 2021

Conversation

tnorling
Copy link
Collaborator

@tnorling tnorling commented Aug 31, 2021

Today in both msal.js v1 and v2 the in-progress temporary cache entry is bound to the clientId which invoked the interaction. This means that if your page is running multiple instances of msal interactive requests will not be blocked by ongoing interactive requests by another instance. This is particularly problematic when using redirects as only 1 redirect can happen at a given time.

This PR, along with #4015, makes several changes to make the multi-instance interaction scenario more robust:

  • Moves temporary cache to sessionStorage for parity with v2 (for now only the interaction in progress cache item is moved)
  • Removes clientId from the interaction in progress cache key
  • Replaces the interaction in progress cache value with the clientId that invoked interaction
  • Preflight validation checks if any clientId has interaction in progress
  • Redirect response handling checks if this clientId has interaction in progress

The cache key/value will be standard (msal.interaction.status) for both v1 and v2 to help with migration efforts.

@github-actions github-actions bot added the msal@1.x Related to msal@1.x (implicit flow) label Aug 31, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #4014 (8f7dfa1) into dev (98533d9) will increase coverage by 0.04%.
The diff coverage is 84.61%.

Flag Coverage Δ *Carryforward flag
msal-angular 95.85% <ø> (ø) Carriedforward from 98533d9
msal-browser 88.00% <ø> (ø) Carriedforward from 98533d9
msal-common 85.02% <ø> (ø) Carriedforward from 98533d9
msal-core 76.90% <84.61%> (+0.23%) ⬆️
msal-node 81.57% <ø> (ø) Carriedforward from 98533d9
msal-react 94.11% <ø> (ø) Carriedforward from 98533d9

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
lib/msal-core/src/UserAgentApplication.ts 69.44% <63.63%> (+0.16%) ⬆️
lib/msal-core/src/cache/AuthCache.ts 81.41% <100.00%> (+2.41%) ⬆️
lib/msal-core/src/utils/Constants.ts 90.69% <100.00%> (+0.22%) ⬆️

@tnorling tnorling marked this pull request as ready for review September 2, 2021 20:42
Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

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

lgtm

@tnorling tnorling merged commit 48b4e0e into dev Sep 7, 2021
@tnorling tnorling deleted the msal-core-interaction-in-progress branch September 7, 2021 17:55
@ghost
Copy link

ghost commented Sep 8, 2021

🎉msal@v1.4.13 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Sep 8, 2021

🎉@azure/msal-browser@v2.17.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
msal@1.x Related to msal@1.x (implicit flow)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants