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

[Bug] "offline_access" "openid" scope causes token cache misses #1548

Closed
4 tasks
stackunderflows opened this issue Dec 18, 2019 · 7 comments
Closed
4 tasks
Assignees
Labels
Milestone

Comments

@stackunderflows
Copy link

Which Version of MSAL are you using ?
Microsoft Identity 4.7.1

Platform
Xamarin.Forms on iOS and Android

What authentication flow has the issue?

  • Desktop / Mobile
    • Interactive
    • Integrated Windows Auth
    • Username Password
    • Device code flow (browserless)

Other? - AcquireTokenSilent

Is this a new or existing app?
This is a new app.

Repro
Login to AD B2C with email credentials. Create a loop that calls AcquireTokensSilent repeatedly. Notice that a new access token is returned every time, even though it is not close to expiring.

        public async Task<UserContext> AcquireAccessToken()
        {
            IEnumerable<IAccount> accounts = await PCA.GetAccountsAsync();

            AuthenticationResult authResult = await PCA.AcquireTokenSilent(_configuration.Scopes, GetAccountByPolicy(accounts, _configuration.PolicySignUpIn))
               .WithB2CAuthority(_configuration.AuthoritySignInUp)
               .ExecuteAsync();

            var newContext = UpdateUserInfo(authResult);

            AccessToken = newContext.AccessToken;
            Debug.WriteLine($"Access token is {newContext.AccessToken}");

            return newContext;
        }

Expected behavior
I expected that the access token would be pulled from cache and not refreshed.

Actual behavior
Access token is refreshed every time, even when it doesn't need to be.

@jmprieur jmprieur changed the title [Bug] [Bug] Access tokens re-requested from B2C whereas they should be taken from the cache Dec 19, 2019
@bgavrilMS
Copy link
Member

  1. What scopes are you using?
  2. Does GetAccountByPolicy(accounts, _configuration.PolicySignUpIn) return anything?

@stackunderflows
Copy link
Author

  1. public string[] Scopes => new string[] { $"https://{Tenant}/mobileapi/user_impersonation", "offline_access" };
  2. GetAccountByPolicy seems like it returns the account correctly. The fields are populate except for username, which says "Missing from Token response".

@bgavrilMS bgavrilMS self-assigned this Jan 6, 2020
@bgavrilMS
Copy link
Member

Ok, for point 2 this looks ok, it is expected that some Identity Providers do not return a username by default.

For point 1, I think the issue is similar to #1547

Can you try to remove offline_access and see if works ? MSAL requests that scope anyway, but worth a try.

@bgavrilMS
Copy link
Member

bgavrilMS commented Jan 6, 2020

Yeah, I think this is the problem. MSAL requests offline_access but ESTS when it responds does not add offline_access to the response (mind you, it does give you a refresh token). This causes a problem in the token cache.

@jmprieur @henrik-me, @shoatman - I think we can make a small fix for this in MSAL, i.e. if the developer asks for offline_access, do not use this scope to look in the cache. Alternatively, we can ban people from explicitly asking for offline_access, but that is more intrusive.

@stackunderflows
Copy link
Author

I can confirm, removing offline_access caused it to use the cached token. I wasn't sure if I needed that, so I'll take it off. Thanks for the help!

@henrik-me
Copy link
Contributor

As per internal discussion seems like we have to improve the cache lookup filtering out OIDC scopes? I don't think we should ask people to do anything we should be able to filter correctly and do appropriate cache lookup.

@jmprieur jmprieur added bug and removed Investigate labels Jan 7, 2020
@jmprieur jmprieur added this to the 4.8 milestone Jan 7, 2020
@bgavrilMS
Copy link
Member

Yes, I'll propose a PR shortly.

@bgavrilMS bgavrilMS added the Fixed label Jan 7, 2020
@bgavrilMS bgavrilMS changed the title [Bug] Access tokens re-requested from B2C whereas they should be taken from the cache [Bug] "offline_access" scope causes token cache misses Jan 7, 2020
@bgavrilMS bgavrilMS changed the title [Bug] "offline_access" scope causes token cache misses [Bug] "offline_access" "openid" scope causes token cache misses Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants