[release/2.1] Fix windows auth when we receive a 401 after auth completes (#32243)

PR #31589 introduced a regression in the case where we receive another 401 after Windows auth completes (this is caused by the user being successfully authenticated, but not authorized to access the specified resource). We were incorrectly draining the response in this case. As a result, the user would receive a disposed response and eventually throw an ObjectDisposedException.

The fix is to adjust the logic here where we only drain if we know we are going to send another request for the rest of the Windows auth (Negotiate/NTLM) challenge-response handshake.

Add new unit test for Windows auth scenario.

Author: davidsh

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs

@@ -56,6 +56,7 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
                     challenge.AuthenticationType == AuthenticationType.Ntlm)
                 {
                     bool isNewConnection = false;
+                    bool needDrain = true;
                     try
                     {
                         if (response.Headers.ConnectionClose.GetValueOrDefault())
@@ -70,10 +71,7 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
                             connectionPool.IncrementConnectionCount();
                             connection.Acquire();
                             isNewConnection = true;
-                        }
-                        else
-                        {
-                            await connection.DrainResponseAsync(response).ConfigureAwait(false);
+                            needDrain = false;
                         }
 
                         string challengeData = challenge.ChallengeData;
@@ -92,6 +90,11 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
                                     break;
                                 }
 
+                                if (needDrain)
+                                {
+                                    await connection.DrainResponseAsync(response).ConfigureAwait(false);
+                                }
+
                                 SetRequestAuthenticationHeaderValue(request, new AuthenticationHeaderValue(challenge.SchemeName, challengeResponse), isProxyAuth);
 
                                 response = await InnerSendAsync(request, isProxyAuth, connectionPool, connection, cancellationToken).ConfigureAwait(false);
@@ -100,7 +103,7 @@ private static async Task<HttpResponseMessage> SendWithNtAuthAsync(HttpRequestMe
                                     break;
                                 }
 
-                                await connection.DrainResponseAsync(response).ConfigureAwait(false);
+                                needDrain = true;
                             }
                         }
                         finally

src/System.Net.Http/tests/FunctionalTests/NtAuthTests.cs

@@ -0,0 +1,133 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Threading.Tasks;
+
+using Xunit;
+using Xunit.Abstractions;
+
+namespace System.Net.Http.Functional.Tests
+{
+    public class NtAuthServer : IDisposable
+    {
+        // When tests projects are run in parallel, overlapping port ranges can cause a race condition
+        // when looking for free ports during dynamic port allocation.
+        private const int PortMin = 5001;
+        private const int PortMax = 8000;
+
+        private HttpListener _listener;
+        private Task _serverTask;
+
+        public NtAuthServer(AuthenticationSchemes authSchemes)
+        {
+            // Create server.
+            CreateServer(authSchemes);
+
+            // Start listening for requests.
+            _serverTask = Task.Run(async () =>
+            {
+                try
+                {
+                    while (true)
+                    {
+                        HttpListenerContext context = await _listener.GetContextAsync();
+
+                        bool noAccess = (context.Request.RawUrl == NoAccessUrl);
+                        if (noAccess)
+                        {
+                            context.Response.AddHeader("Www-Authenticate", "Negotiate");
+                        }
+
+                        context.Response.StatusCode = noAccess ? 401 : 200;
+                        context.Response.ContentLength64 = 0;
+                        context.Response.OutputStream.Close();
+                    }
+                }
+                catch (HttpListenerException)
+                {
+                    // Ignore.
+                }
+            });
+        }
+
+        public void Dispose()
+        {
+            _listener.Stop();
+            _serverTask.Wait();
+        }
+
+        public string BaseUrl { get; private set; }
+        public string NoAccessUrl => "/noaccess";
+
+        private void CreateServer(AuthenticationSchemes authSchemes)
+        {
+            for (int port = PortMin; port < PortMax; port++)
+            {
+                string url = $"http://localhost:{port}/";
+
+                _listener = new HttpListener();
+                _listener.Prefixes.Add(url);
+                _listener.AuthenticationSchemes = authSchemes;
+
+                try
+                {
+                    _listener.Start();
+                    BaseUrl = url;
+                    return;
+                }
+                catch (HttpListenerException)
+                {
+                }
+            }
+
+            throw new Exception("Failed to locate a free port.");
+        }
+    }
+
+    public class NtAuthServers : IDisposable
+    {
+        public readonly NtAuthServer NtlmServer = new NtAuthServer(AuthenticationSchemes.Ntlm);
+        public readonly NtAuthServer NegotiateServer = new NtAuthServer(AuthenticationSchemes.Negotiate);
+
+        public void Dispose()
+        {
+            NtlmServer.Dispose();
+            NegotiateServer.Dispose();
+        }
+    }
+
+    public class NtAuthTests : IClassFixture<NtAuthServers>
+    {
+        private readonly NtAuthServers _servers;
+        private readonly ITestOutputHelper _output;
+
+        public NtAuthTests(NtAuthServers servers, ITestOutputHelper output)
+        {
+            _servers = servers;
+            _output = output;
+        }
+
+        [OuterLoop]
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))]    // HttpListener doesn't support nt auth on non-Windows platforms
+        [InlineData(true, HttpStatusCode.OK)]
+        [InlineData(true, HttpStatusCode.Unauthorized)]
+        [InlineData(false, HttpStatusCode.OK)]
+        [InlineData(false, HttpStatusCode.Unauthorized)]
+        public async Task GetAsync_NtAuthServer_ExpectedStatusCode(bool ntlm, HttpStatusCode expectedStatusCode)
+        {
+            NtAuthServer server = ntlm ? _servers.NtlmServer : _servers.NegotiateServer;
+
+            var handler = new HttpClientHandler();
+            handler.UseDefaultCredentials = true;
+            using (var client = new HttpClient(handler))
+            {
+                client.BaseAddress = new Uri(server.BaseUrl);
+                string path = expectedStatusCode == HttpStatusCode.Unauthorized ? server.NoAccessUrl : "";
+                HttpResponseMessage response = await client.GetAsync(path);
+
+                Assert.Equal(expectedStatusCode, response.StatusCode);
+            }
+        }
+    }
+}

src/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

@@ -132,6 +132,7 @@
     <Compile Include="HttpClientHandlerTest.Decompression.cs" />
     <Compile Include="HttpClientTest.netcoreapp.cs" />
     <Compile Include="HttpMethodTest.netcoreapp.cs" />
+    <Compile Include="NtAuthTests.cs" />
     <Compile Include="ReadOnlyMemoryContentTest.cs" />
     <Compile Include="SocketsHttpHandlerTest.cs" />
   </ItemGroup>