I am trying to use the OpenID Connect authentication middleware provided by the Katana project.
There is an error in the implementation that causes a dead end in these conditions:
- Running at the host where the request has a thread-affinity (e.g., IIS).
- The OpenID Connect metadata document was not restored or the cached copy has expired.
- The application calls
SignOut for the authentication method. - An action occurs in the application that causes a write to the response stream.
The deadlock is due to the way the middleware authenticates the callback from the host signaling the headers. The root of the problem in this method:
private static void OnSendingHeaderCallback(object state) { AuthenticationHandler handler = (AuthenticationHandler)state; handler.ApplyResponseAsync().Wait(); }
From Microsoft.Owin.Security.Infrastructure.AuthenticationHandler
Calling Task.Wait() is only safe when the returned Task already completed, which was not done with the OpenID Connect middleware.
The binder uses an instance of Microsoft.IdentityModel.Protocols.ConfigurationManager<T> to manage a cached copy of its configuration. This is an asynchronous implementation using SemaphoreSlim as an asynchronous lock and repository of an HTTP document to get the configuration. I suspect this is a trigger for the Wait() deadlock call.
This is a method that I suspect is the reason:
public async Task<T> GetConfigurationAsync(CancellationToken cancel) { DateTimeOffset now = DateTimeOffset.UtcNow; if (_currentConfiguration != null && _syncAfter > now) { return _currentConfiguration; } await _refreshLock.WaitAsync(cancel); try { Exception retrieveEx = null; if (_syncAfter <= now) { try { // Don't use the individual CT here, this is a shared operation that shouldn't be affected by an individual cancellation. // The transport should have it own timeouts, etc.. _currentConfiguration = await _configRetriever.GetConfigurationAsync(_metadataAddress, _docRetriever, CancellationToken.None); Contract.Assert(_currentConfiguration != null); _lastRefresh = now; _syncAfter = DateTimeUtil.Add(now.UtcDateTime, _automaticRefreshInterval); } catch (Exception ex) { retrieveEx = ex; _syncAfter = DateTimeUtil.Add(now.UtcDateTime, _automaticRefreshInterval < _refreshInterval ? _automaticRefreshInterval : _refreshInterval); } } if (_currentConfiguration == null) { throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, ErrorMessages.IDX10803, _metadataAddress ?? "null"), retrieveEx); } // Stale metadata is better than no metadata return _currentConfiguration; } finally { _refreshLock.Release(); } }
I tried adding .ConfigureAwait(false) to all the expected operations in an attempt to marshal the continuations in the thread pool, not the ASP.NET worker thread, but I had no success in preventing the deadlock.
Is there a deeper problem that I can solve? I am not opposed to replacing components - I have already created my own experimental implementations of IConfiguratioManager<T> . Is there a simple fix that can be applied to prevent a dead end?