MSAL fails to create separate hidden iframes for each token request based on target authority, resulting in returning the wrong token or error to registered callbacks #1225
Comments
keystroke
added
the
bug
|
As another data point, I added the "forceRefresh" request option to the acquire token silent request calls, and it did not help the issue: |
|
@sameerag tagging you here (hope its okay) to avoid reading above if this explanation is enough for you ;) It seems that MSAL is creating iframes based only on target scope of token which needs to be acquired, instead of including target authority as well. In above issue description, I show how acquiring multiple tokens concurrently does not work correctly, and will return duplicate data to the various callbacks in the promise. It seems (going from logs below) that the iframes created are identified only by target scope. Since the token requests I'm making for each directory are of the same scope, only one frame is created, though all the callbacks are registered, and thus they all get invoked with the result of that single frame. This also explains why switching to serial token acquisition works, as the frame is re-used each time with the correct token request properties. I think that the iframes created by MSAL must be addressed by both target scopes and target authority (and perhaps you might identify other things to consider for multi-tenant scenarios). |
keystroke
changed the title
|
I found a way to work around the issue for now... though it is obviously quite hacky. In my previously-shared example, you can modify the token scopes to request 'openid' a duplicate number of times. This effectively "tricks" MSAL into creating unique hidden iframes for each target directory, as it uses the duplicated scopes in the identifier: In below image, you can see we get unique resulting iframes:
It works only because AAD knows how to handle the duplicate scopes, and doesn't treat it as an error. Note this isn't really a practical workaround to use generically, as you'd have to build a "queue" to wrap MSAL token methods and grab "openid scope strings" out of a queue to use with each token request, waiting until one becomes available. That might sound crazy to you, but you already have to do this if you use MSAL to acquire multiple tokens, some of which may require popups, as while a popup is running, any silent token requests can interfere with it, so when user code wants to initiate a popup request, you must drain all active silent token requests, enqueueing others if they come-in, and then proceed with the popup flow, after which you dequeue any other pending popup requests, followed by dequeueing any pending silent token requests. So this workaround is just an extra step to queue silent token requests based on the availability of these "openid scope strings", just as is done for popups, but you can have multiple active at once, instead of only a single popup. |
|
@keystroke very interesting find and I think we have an issue to fix on the first look. Can I reach out once we figure out the best way to fix this? |
|
@sameerag thanks for quick response! I will happily test any new drops if you can provide them in a single js file which I can load with a script tag. |
|
@keystroke can you pull the changes from #1225? I can share the compiled js offline if you are ready to test it |
|
@sameerag Thanks! I can try this out today or tomorrow if you can provide the compiled JS (if you want to send via email or teams instead of posting here that is also fine). |
Library
msal@1.2.0or@azure/msal@1.x.x@azure/msal-angular@0.x.x@azure/msal-angular@1.x.x@azure/msal-angularjs@1.x.xFramework
None.
Description
[UPDATE]
See my latest comment; I've renamed the issue according to what I believe is the root cause. It seems that MSAL is creating hidden iframes identified only by target scopes in the token request, but failing to include target authority. This results in multiple concurrent token requests with distinct authorities but identical scopes to be joined together and all of them receive the same result from that single hidden iframe!
I have also found a hacky workaround, the mechanism for which might be usable in more scoped scenarios if people hit this issue, though creating a generic version of the workaround would be difficult (easier to just fix in MSAL directly to scope target hidden iframes by authority and scopes).
[/UPDATE]
When requesting tokens for multiple different authorities at the same time, the same response (error or result) will be return to all the .then or .catch handlers.
Details described in this post are primarily for above issue, but I notice many issues trying to sign-in a user and make API calls to multiple different directories, all concurrently. For example, only one popup may be active at a time. This is perhaps fine, but while a popup is active, any silent token requests will terminate the popup... I would expect that silent token requests should not affect a running popup request, and they should be isolated somehow.
In general, please advise on how to setup using MSAL if I want to sign a user into their home directory, and then proceed to acquire many different tokens concurrently, including handling popups for each necessary consent (e.g. in below issue example you will see 9 directories I am attempting to authenticate with, and some of them require consent; I have built a wrapper to allow the user to proceed with granting consent via popups one at a time, and once consent is granted, I acquire other tokens in each directory and make other API calls, and all of this means I have build an insanely complicated wrapper around MSAL that ensures token requests are processed serially as necessary which is obviously terrible). For example do I need to be creating separate MSAL client objects for each target directory? What about the caching behavior? etc.
Security
Is this issue security related?
I would say yes, because this error isn't obvious at first and when testing my web app I was taking actions against the wrong tenant! Especially since with the new graph APIs, the target tenant is only in the token, not in the URI like it used to be!
Regression
Don't recall. I've been waiting for MSAL to mature fully to do my basic operations:
In one way or another, I haven't been able to achieve this with MSAL due to issues like this. I understand there were recent changes to cache behavior. PLEASE ADVISE how I need to set this up to accomplish this. Do I need separate instances of MSAL clients?
Configuration
Please provide your MSAL configuration options.
Reproduction steps
Above, you can see there are two methods provided for the "discovery" process:
When I use the first option ("beginTenantDiscoveryConcurrent"), I get the same token response for all handlers that complete within a threshold (below I am tracing the target tenant for which I initiated the token request, followed by the resulting "tid" claim in the token that MSAL sent to my .then callback):
Above was for a case where the token for the first tenant was successful, and sent to all my .then handlers. As you can see and further investigate for yourself, it is the exact same token sent to all of them.
Below shows a case where one token call completed successfully and received a distinct response, but all the others received the incorrect error message (notice the error messages all have the same identifiers and time stamps on them):
And for a final example, below shows when I use the second option ("beginTenantDiscoverySerial"), I get the expected output, with distinct error or success responses:
Expected behavior
I expect that the acquire token silent calls for each directory should have a distinct response or failure; if user doesn't exist in target tenant, or target tenant doesn't exist, or consent is required for the target tenant, I expect the corresponding error to be raised. If the token acquisition is successful, I expect to receive the access token for the target directory tenant with the target claims.
Browsers
Saw error in both IE11 and Chrome (latest version).
The text was updated successfully, but these errors were encountered: