AzureAD / microsoft-authentication-library-for-js

Microsoft Authentication Library (MSAL) for JS
http://aka.ms/aadv2
MIT License
3.64k stars 2.65k forks source link

Library triggers authorization requests to get id tokens on every route change #2238

Closed mderriey closed 3 years ago

mderriey commented 4 years ago

Library

Framework

Angular 9

Description

The framework generates too many id token requests when switching routes. We have a relatively basic setup, with different routes that use MsalGuard to ensure that users are signed in, and every route change triggers an authorization request to AAD in an iframe to get an id token.

I followed the call stack while debugging and it goes like:

  1. MsalGuard calls acquireTokenSilent using the AAD client id as the one and only scope; see code here
  2. MsalService delegates acquireTokenSilent to UserAgentApplication; see code here
  3. UserAgentApplication calls RequestUtils.validateRequest which translates the unique scope that matches the configured client id into the openid and profile scope; see UserAgentApplication call and RequestUtils.validateRequest implementation
  4. Further down in UserAgentApplication.acquireTokenSilent, we try to get a cached token; see code here
  5. The getCachedToken method internally calls getAllAccessTokens
  6. getAllAccessTokens looks in the keys of the configured storage and tries to match three pieces of information:

    • the client id of the application;
    • the home account identifier; and
    • the hardcoded value "scopes";

    In our case, this method only returns one token, which doesn't have the scopes we're looking for, which as a reminder are openid and profile — more on that below.

  7. Back in getCachedToken, since we don't find a token with matching scopes, we return null
  8. Ultimately, up the stack, we decide to call renewIdToken which initiates an authorization request via an iframe.

Our config is as follows:

// MSAL_CONFIG
return {
  auth: {
    clientId: '<aad-client-id>',
    redirectUri: '<redirect-uri>',
    authority: 'https://login.microsoftonline.com/<tenant-id>',
    validateAuthority: true,
  },
};

// MSAL_ANGULAR_CONFIG
return {
  protectedResourceMap: ['/api', ['<aad-client-id>/.default']]
};

At runtime, from a clean tab, the following happens:

Looking at the cache, I have two entries.

I assume one represents the id token that was acquired during the login flow. It looks like this:

// Key
{
  "authority": "https://login.microsoftonline.com/<aad-tenant-id>/",
  "clientId": "<aad-client-id>",
  "homeAccountIdentifier": "<account-identifier>"
}

// Value
{
  "accessToken": "<token>", // this is the same value as idToken
  "idToken": "<token>", // this is the same value as accessToken
  "expiresIn": "1599551521",
  "homeAccountIdentifier": "<base-64-string>"
}

The other entry in the cache, which I think represents the access token, looks like below:

// Key
{
  "authority": "https://login.microsoftonline.com/<aad-tenant-id>/",
  "clientId": "<aad-client-id>",
  "homeAccountIdentifier": "<account-identifier>",
  "scopes": "<aad-client-id>/.default"
}

// Value
{
  "accessToken": "<token1>", // this is NOT the same value as idToken
  "idToken": "<token2>", // this is NOT the same value as accessToken
  "expiresIn": "1599551486",
  "homeAccountIdentifier": "<base-64-string>"
}

I'd say the MsalService/UserAgentApplication is supposed to get the id token from the cache, but because its key doesn't have the "scopes" value in it, it's a cache miss, which means we get a new one every time.

Error Message

Security

Regression

MSAL Configuration

Provided above.

Reproduction steps

// Provide relevant code snippets here.
// For Azure B2C issues, please include your policies.

Expected behavior

Browsers/Environment

mderriey commented 4 years ago

I reverted back to msal@1.3.2 and @azure/msal-angular@1.0.0, and can confirm that no additional requests are made when navigating between pages.

There are still two entries in the cache.

Entry number 1:

// Key
{
  "authority": "https://login.microsoftonline.com/<aad-tenant-id>/",
  "clientId": "<aad-client-id>",
  "scopes": "<aad-client-id>",
  "homeAccountIdentifier": "<account-identifier>"
}

// Value
{
  "accessToken": "<token>", // same as idToken
  "idToken": "<token>", // same as accessToken
  "expiresIn": "1599554514",
  "homeAccountIdentifier": "<base-64-string>"
}

Entry 2

// Key
{
  "authority": "https://login.microsoftonline.com/<aad-tenant-id>/",
  "clientId": "<aad-client-id>",
  "scopes": "<aad-client-id>/.default",
  "homeAccountIdentifier": "<account-identifier>"
}

// Value
{
  "accessToken": "<token1>", // NOT the same as idToken
  "idToken": "<token2>", // NOT the same as accessToken
  "expiresIn": "1599554611",
  "homeAccountIdentifier": "<base-64-string>"
}
JayCeeJr commented 4 years ago

Reverting to msal@1.3.2 and @azure/msal-angular@1.0.0 worked for me as well.

tnorling commented 4 years ago

This is a known issue with msal@1.4.0 and will be addressed by #2206

LastTribunal commented 4 years ago

Downgrading from 1.4 is not an option for me since I use this library to access both AAD and ADFS.. But I wired up my own token refresh, and it works well so far (built off of @scambier in https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/759)

async function getTokenInternal() {
    var token
    if (client.getAccount()) {
        var cachedToken = await getCachedToken(300)
        if (cachedToken != null) token = cachedToken
        else
            await client.acquireTokenSilent(request)
            .then(response => {
                token = response.idToken.rawIdToken
            })
            .catch(err => {
                console.log(err);
                logout()
            });
    } else {
        await login().then(response => {
            token = response
        })
    }
    return token
}

async function getCachedToken(seconds) {
    const timestamp = Math.floor((new Date()).getTime() / 1000)
    let token
    try {
        for (const key of Object.keys(sessionStorage)) {
            if (key.includes('"authority":')) {
                const val = await JSON.parse(sessionStorage.getItem(key))
                if (val.expiresIn > (timestamp + seconds) && val.idToken === val.accessToken)
                    token = val.idToken
            }
        }
    } catch (err) {
        console.dir(err)
    }
    return token
}
mderriey commented 4 years ago

Hey folks, thanks for pushing msal@1.4.1.

I tested it and I unfortunately observe the same behaviour as @1.4.0, i.e. every route change triggers an id token refresh request to AAD.

I suspect in my case, it might be because we're using a multitenant application:

The call stack is:

See also some logs — I was surprised to see the logs mentioned 1.4.0 but I triple checked I had 1.4.1 installed.

```text Thu, 01 Oct 2020 04:59:20 GMT:1.4.0-Verbose location change event from old url to new url Thu, 01 Oct 2020 04:59:20 GMT:1.4.0-Verbose AcquireTokenSilent has been called Thu, 01 Oct 2020 04:59:20 GMT:1.4.0-Verbose Telemetry Event started: 44b5c944-13d8-4b9c-9206-1375539bd16c_6f2f2b9e-01cd-4929-b6b3-bb897b517510-msal.api_event Thu, 01 Oct 2020 04:59:22 GMT:1.4.0-Verbose Account set from MSAL Cache Thu, 01 Oct 2020 04:59:22 GMT:1.4.0-Verbose Response type: id_token Thu, 01 Oct 2020 04:59:22 GMT:1.4.0-Verbose Finished building server authentication request Thu, 01 Oct 2020 04:59:22 GMT:1.4.0-Verbose Query parameters populated from existing SSO or account Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose GetCachedToken has been called Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose No tokens found Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose Getting all cached access tokens Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose No authority passed, filtering tokens by scope Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose One matching token found, setting authorityInstance Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose Evaluating access token found Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose Access token expiration is within offset, using access token found in cache Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose No token found in cache lookup Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose Cached metadata found for authority Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose OpenID Connect scopes only, renewing idToken Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Info RenewIdToken has been called Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose RenewIdToken expected state: Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose Silent login is true, set silentAuthenticationState Thu, 01 Oct 2020 04:59:23 GMT:1.4.0-Verbose monitorWindowForIframe polling started Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose monitorIframeForHash found url in hash Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose HandleAuthenticationResponse has been called Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose GetResponseState has been called Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Hash contains state. Creating stateInfo object Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose State matches cached state, setting requestType to login Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Obtained state from response Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Info ProcessCallBack has been called. Processing callback from redirect response Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose SaveTokenFromHash has been called Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Info State status: true; Request type: LOGIN Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Server returns success Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Info State is right Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Fragment has session state, caching Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Info Fragment has idToken Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose PopulateAuthority has been called Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Fragment has clientInfo Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Account object created from response Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose IdToken has nonce Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Nonce matches, saving idToken to cache Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose SaveAccessToken has been called Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Response parameters does not contain scope, OIDC scopes set as scope Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Saving token to cache Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose New expiration set Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Status set to complete, temporary cache cleared Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Response tokenType set to id_token Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Calling callback provided to processCallback Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Successfully acquired token Thu, 01 Oct 2020 04:59:25 GMT:1.4.0-Verbose Telemetry Event stopped: 44b5c944-13d8-4b9c-9206-1375539bd16c_6f2f2b9e-01cd-4929-b6b3-bb897b517510-msal.api_event ```

Because we're using @azure/msal-angular, we have no possibility to pass the correct authority into the acquireTokenSilent call.

Happy to open a different issue if you deem it's a different one.

tnorling commented 4 years ago

@mderriey Thanks for the detailed explanation, this behavior with common authorities was fixed a while back for access token lookup but it seems this was not carried over to the new id_token lookup logic. We will make sure this gets addressed in 1.4.2, apologies for the inconvenience.

github-actions[bot] commented 4 years ago

This issue has not seen activity in 14 days. It will be closed in 7 days if it remains stale.