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] Windows Broker and Embedded Web View do check cancellation tokens #3225

Closed
kyle-rader opened this issue Mar 11, 2022 · 3 comments · Fixed by #3450
Closed

[Bug] Windows Broker and Embedded Web View do check cancellation tokens #3225

kyle-rader opened this issue Mar 11, 2022 · 3 comments · Fixed by #3450

Comments

@kyle-rader
Copy link

Version: MSAL.NET @ 4.42
Platform: Windows 11, net5.0-windows10.0.17763.0
Auth flow: Broker and Interactive Web auth (embedded web view only)

Repro
@bgavrilMS - we have an easy repro internally and a current bugfix in the auth CLI.

private class MsalHttpClientFactoryAdaptor : IMsalHttpClientFactory
        {
            private HttpClient instance;

            public MsalHttpClientFactoryAdaptor(HttpClient instance)
            {
                if (instance == null)
                {
                    throw new ArgumentNullException(nameof(instance));
                }

                this.instance = instance;
            }

            public HttpClient GetHttpClient()
            {
                // MSAL calls this method each time it wants to use an HTTP client.
                // We ensure we only create a single instance to avoid socket exhaustion.
                return this.instance;
            }
        }
//---

ver pca = PublicClientApplicationBuilder
    .Create(this.clientId.ToString())
    .WithAuthority(this.Authority)
    .WithBroker();

var tokenSource = new CancellationStokenSource();
var timeout = TimeSpan.FromSeconds(15);

tokenSource.CancelAfter(timeout);

await pca.AcquireTokenInteractive(scopes)
    .ExecuteAsync(tokenSource.Token);

// Expect OperationCancelledException to be thrown - but the Web or broker UI hangs forever.

Expected behavior
An OperationCancelledException should be thrown by calling token.ThrowIfCancellationRequested().

Possible solution
Internally we have a workaround to run await a timer task that checks if the task is done every 100ms and after the timeout, returns a Task.FromResult<T>(null).

Additional context / logs / screenshots / links to code
We have debugged the un-cancellable call to this Task that runs with a custom Task Scheduler

@MichelZ
Copy link
Contributor

MichelZ commented Jul 1, 2022

Is it as simple as using the cancellationToken in Wait?
e.g.

Task.Factory.StartNew(
        sendAuthorizeRequest,
        cancellationToken,
        TaskCreationOptions.None,
        staTaskScheduler).Wait(cancellationToken);

@MichelZ
Copy link
Contributor

MichelZ commented Jul 1, 2022

I took a different approach than just killing the thread. The Form gets auto-closed when the Cancellation Token is set, and an OperationCancelledException gets thrown

@pmaytak
Copy link
Contributor

pmaytak commented Aug 4, 2022

Released in 4.46.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment