From 2beedc5bdb6cb7d5301f3280c85c9a33d48979db Mon Sep 17 00:00:00 2001 From: David Shulman Date: Fri, 2 Sep 2016 12:35:12 -0700 Subject: [PATCH] Fix WinHttpHandler for Basic auth with default credentials We were getting an error "The parameter is incorrect" from WinHTTP when we tried to invoke default credential handling even for cases where the server doesn't use Negotiate or NTLM. Negotiate and NTLM are the only schemes that can use default credentials since they need to be transmitted via a challenge-response mechanism. The fix here is to only set WinHTTP default credentials handling for Negotiate or NTLM schemes. Otherwise, it should behave as-if there were no credentials set. Fixes #11266 --- .../src/System/Net/Http/WinHttpAuthHelper.cs | 33 +++++++++++++------ .../FunctionalTests/HttpClientHandlerTest.cs | 16 +++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs index 538e3b9dde8e..0bd1516299b9 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs @@ -103,14 +103,15 @@ public void CheckResponseForAuthentication( serverAuthScheme = ChooseAuthScheme(supportedSchemes); if (serverAuthScheme != 0) { - SetWinHttpCredential( + if (SetWinHttpCredential( state.RequestHandle, state.ServerCredentials, uri, serverAuthScheme, - authTarget); - - state.RetryRequest = true; + authTarget)) + { + state.RetryRequest = true; + } } break; @@ -295,7 +296,7 @@ public void ChangeDefaultCredentialsPolicy( } } - private void SetWinHttpCredential( + private bool SetWinHttpCredential( SafeWinHttpHandle requestHandle, ICredentials credentials, Uri uri, @@ -314,15 +315,25 @@ private void SetWinHttpCredential( if (networkCredential == null) { - return; + return false; } if (networkCredential == CredentialCache.DefaultNetworkCredentials) { - // Allow WinHTTP to transmit the default credentials. - ChangeDefaultCredentialsPolicy(requestHandle, authTarget, allowDefaultCredentials:true); - userName = null; - password = null; + // Only Negotiate and NTLM can use default credentials. Otherwise, + // behave as-if there were no credentials. + if (authScheme == Interop.WinHttp.WINHTTP_AUTH_SCHEME_NEGOTIATE || + authScheme == Interop.WinHttp.WINHTTP_AUTH_SCHEME_NTLM) + { + // Allow WinHTTP to transmit the default credentials. + ChangeDefaultCredentialsPolicy(requestHandle, authTarget, allowDefaultCredentials:true); + userName = null; + password = null; + } + else + { + return false; + } } else { @@ -352,6 +363,8 @@ private void SetWinHttpCredential( { WinHttpException.ThrowExceptionUsingLastError(); } + + return true; } private static uint ChooseAuthScheme(uint supportedSchemes) diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs index aa088756b41f..4164382dde17 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs @@ -296,6 +296,22 @@ public async Task GetAsync_DefaultAutomaticDecompression_HeadersRemoved(Uri serv } } + [Fact] + public async Task GetAsync_ServerNeedsBasicAuthAndSetDefaultCredentials_StatusCodeUnauthorized() + { + var handler = new HttpClientHandler(); + handler.Credentials = CredentialCache.DefaultCredentials; + using (var client = new HttpClient(handler)) + { + Uri uri = HttpTestServers.BasicAuthUriForCreds(secure:false, userName:Username, password:Password); + using (HttpResponseMessage response = await client.GetAsync(uri)) + { + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + } + } + + [OuterLoop] // TODO: Issue #11345 [Fact] public async Task GetAsync_ServerNeedsAuthAndSetCredential_StatusCodeOK() {