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

MsalProvider not handling token expiration properly #189

Open
matheus-inacio opened this issue May 28, 2022 · 3 comments
Open

MsalProvider not handling token expiration properly #189

matheus-inacio opened this issue May 28, 2022 · 3 comments
Labels
Area: Providers bug 🐛 Something isn't working

Comments

@matheus-inacio
Copy link

Describe the bug

My app periodically check for files in OneDrive and i've notices that if the app is kept opened long enough, the token will expire and the user will be prompt to select an account again.

I've manage to work around this issue by creating my own MsalProvider and changing the GetTokenWithScopesAsync as follows:

protected async Task<string> GetTokenWithScopesAsync(string[] scopes, bool silentOnly = false)
{
    await SemaphoreSlim.WaitAsync();

    try
    {
        AuthenticationResult authResult = null;
        try
        {
            var account = Account ?? (await Client.GetAccountsAsync()).FirstOrDefault();
            if (account != null)
            {
                authResult = await Client.AcquireTokenSilent(scopes, account).ExecuteAsync();
            }
        }
        catch (MsalUiRequiredException)
        {
           // CODE ADDED TO WORK AROUND THE EXPIRATION TOKEN ISSUE
            if (Account != null)
            {
                Account = null;
                SemaphoreSlim.Release();
                return await GetTokenWithScopesAsync(scopes, silentOnly);
            }
        }
        catch
        {
            // Unexpected exception
            // TODO: Send exception to a logger.
        }

        if (authResult == null && !silentOnly)
        {
            try
            {
                var paramBuilder = Client.AcquireTokenInteractive(scopes);

                if (Account != null)
                {
                    paramBuilder = paramBuilder.WithAccount(Account);
                }
                paramBuilder = paramBuilder.WithPrompt(Microsoft.Identity.Client.Prompt.NoPrompt);

                authResult = await paramBuilder.ExecuteAsync();
            }
            catch
            {
                // Unexpected exception
                // TODO: Send exception to a logger.
            }
        }

        Account = authResult?.Account;

        return authResult?.AccessToken;
    }
    finally
    {
        SemaphoreSlim.Release();
    }
}

I'm not submitting this as a PR cause i think that this is not the best way to handle this issue.

Steps to Reproduce

Steps to reproduce the behavior:

  1. Open an app and make a request so that a token is generated
  2. Keep the app open until the token expires
  3. Try to make another request
  4. The app will prompt you to select an account again.

Expected behavior

I believe that once the user has authenticated the account selection window should not be prompt again.

Environment

Package Version(s): 7.1.1

Windows 11 Build Number:
- [x] Windows 11 (22000)

App min and target version:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [x] May 2019 Update (18362)
- [x] Windows 11 (Build 22000)

Device form factor:
- [x] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [ ] 2017 (version: )
- [ ] 2019 (version: ) 
- [ ] 2019 Preview (version: )
- [x] 2022

@matheus-inacio matheus-inacio added the bug 🐛 Something isn't working label May 28, 2022
@ghost ghost added the needs triage 🔍 label May 28, 2022
@ghost
Copy link

ghost commented May 28, 2022

Hello matheus-inacio, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker
Copy link
Member

michael-hawker commented Jun 14, 2022

I'm wondering if silentOnly is set to true here, so we're not getting the prompt to do the interactive re-login maybe?

if (authResult == null && !silentOnly)

In the docs about providers here it does show falling back to interactive if the exception is hit.

So, maybe we just need to remove the check for !silentOnly and always do that if we don't have an account after trying to acquire silently.

Does that sound right @shweaver-MSFT?

@shweaver-MSFT
Copy link
Member

shweaver-MSFT commented Jun 14, 2022

I'm not sure. GetTokenWithScopesAsync is called during AuthenticateRequestAsync, but it does not specify silentOnly, so I'm expecting the prompt to show under most/all circumstances. The only time the function is called with the silent flag is during TrySilentSignInAsync.

Thinking of one scenario, I imagine that when the token is expired and using the LoginButton, the sign in will fail when the token is expired. LoginButton will attempt to silently login on load, but otherwise the button does not attempt any interactive login without being invoked by the user first.

In this situation, perhaps calling SignInAsync or GetTokenAsync might resolve the issue because it would prompt an interactive login.

But why is MSAL not auto-refreshing the token? Why do we need to interactively login again? That I cannot answer. A question for the MSAL team perhaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Providers bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants