HttpClientHandler with PreAuthenticate enabled can't refresh expired password

[zivkan]

Description

I have a client app that talks to remote HTTP servers via HttpClient, and authentication handled via HttpClientHandler.Credentials, also using HttpClientHandler.PreAuthenticate = true to avoid needlessly hammering the web server with unauthenticated HTTP requests that need to get HTTP 401 responses (fewer requests also reduces latency, from the client point of view).

Say the customer logs into the website and changes their password, or we're sending OAuth2 tokens over Basic auth, rather than Bearer. It appears that HttpClientHandler won't refresh the credential in its credential cache, despite getting a fresh credential from HttpClientHandler.Credentials.

FWIW, it also looks like there's a potential perf improvement by avoiding getting the credential twice. In the sample output below, look for the "Credential Plugin: returning credential" lines.

Reproduction Steps

Here's a sample app that demonstrates:

using System.Net;
using System.Text;

HttpListener listener = new HttpListener();
string prefix = "http://localhost:1234/";
listener.Prefixes.Add(prefix);
listener.Start();

var credentialPlugin = new CredentialPlugin();

Task httpHandlerTask = HandleHttpRequests(listener);

using HttpClientHandler httpClientHandler = new()
{
    PreAuthenticate = true,
    Credentials = credentialPlugin
};
using HttpClient httpClient = new(httpClientHandler);

await SendRequest(httpClient, prefix);
await SendRequest(httpClient, prefix + "api1/ABC");
await SendRequest(httpClient, prefix + "api2/123");

credentialPlugin.RefreshToken();

await SendRequest(httpClient, prefix + "api1/DEF");
// Try another time, just in case the cred cache is updated after the first failed attempt
await SendRequest(httpClient, prefix + "api1/DEF");

await SendRequest(httpClient, prefix + "api2/456");

listener.Stop();
await httpHandlerTask;

async Task SendRequest(HttpClient client, string url)
{
    using var response = await httpClient.GetAsync(url);
    Console.WriteLine($"Client: response code {response.StatusCode} from {url}");
}

async Task HandleHttpRequests(HttpListener listener)
{
    while (true)
    {
        try
        {
            var request = await listener.GetContextAsync();
            var authenticationHeader = request.Request.Headers.Get("Authorization");
            if (string.IsNullOrEmpty(authenticationHeader))
            {
                Console.WriteLine("Server: unauthenticated request to " + request.Request.RawUrl);
                request.Response.AddHeader("WWW-Authenticate", "basic");
                request.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
            }
            else
            {
                var auth = request.Request.Headers["Authorization"];
                var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(auth.Replace("Basic", "", StringComparison.OrdinalIgnoreCase)));
                if (decoded == $"{credentialPlugin.UserName}:{credentialPlugin.Password}")
                {
                    Console.WriteLine($"Server: authenticated request ({decoded}) to {request.Request.RawUrl}");
                    request.Response.StatusCode = (int)HttpStatusCode.OK;
                }
                else
                {
                    Console.WriteLine($"Server: wrong username/password ({decoded}) request to " + request.Request.RawUrl);
                    request.Response.AddHeader("WWW-Authenticate", "basic");
                    request.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
                }
            }
            request.Response.Close();
        }
        catch
        {
            // this is how http listener "gracefully" stops?
            return;
        }
    }
}

class CredentialPlugin : ICredentials
{
    public CredentialPlugin()
    {
        UserName = "username";
        counter = 0;
        Password = "password";
    }

    private int counter;
    public string UserName {get; private set;}
    public string Password { get;private set;}

    // pretend this comes from an OAuth2 service
    public void RefreshToken()
    {
        counter++;
        Password = "password" + counter;
        Console.WriteLine($"Credential Plugin: Changed to '{UserName}:{Password}'");
    }

    NetworkCredential? ICredentials.GetCredential(Uri uri, string authType)
    {
        Console.WriteLine($"Credential Plugin: returning credential '{UserName}:{Password}'");
        return new NetworkCredential(UserName, Password);
    }
}

Expected behavior

After the "credential plugin" changes the password, I would expect HttpClient to:

1. Send a request to the server with the old credentials (which it does)
2. When the request fails with 401, get new credentials (which it does)
3. Send a new request to the server with the new credentials (it does not currently)
4. Update the credential cache, so subsequent requests use the new credentials, not the old credentials (it does not currently)

Basically, when it gets an HTTP 401 response, I'd like it to clear the credentials from the pre-auth cache, and then mimic the behaviour when the first request didn't have auth in the first place.

Here's pseudocode (that looks awfully like valid C# code) that explains my expected behaviour. Basically, try once (preauth if possible), and if the response is a 401, try a second time if credentials can be obtained.

async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request)
{
  HttpResponseMessage response;
   if (_options.PreAuthenticate && TryGetCachedCredential(request.Uri, out var credential)
   {
     response = await SendWithAuthAsync(request, credential);
   }
   else
  {
    response = await SendVerbatimAsync(request);
  }

  if (response.Status == 401 && _options.Credentials != null)
  {
    credential = _options.Credentials.GetCredentials(request.Uri, response.Headers.WwwAuthorization.Scheme);
    if (credential != null)
    {
      response = await SendWithAuthAsync(request, credential);
      if (response.Status != 401)
      {
        UpdatePreauthCache(request.Uri, credential);
      }
    }
  }

  return response;
}

I'm sure there's a lot more complexity than what I assume, but I hope it explains what I thought HttpClient was going to do, and what I'd like it to do.

Actual behavior

Here's the output of my sample app. You can see it only tries the first password over and over, and despite asking the (simulated) "credential plugin" for a fresh token (twice per HTTP request), it keeps trying the old

Server: unauthenticated request to /
Credential Plugin: returning credential 'username:password'
Credential Plugin: returning credential 'username:password'
Server: authenticated request (username:password) to /
Client: response code OK from http://localhost:1234/
Server: authenticated request (username:password) to /api1/ABC
Client: response code OK from http://localhost:1234/api1/ABC
Server: authenticated request (username:password) to /api2/123
Client: response code OK from http://localhost:1234/api2/123
Credential Plugin: Changed to 'username:password1'
Server: wrong username/password (username:password) request to /api1/DEF
Credential Plugin: returning credential 'username:password1'
Credential Plugin: returning credential 'username:password1'
Client: response code Unauthorized from http://localhost:1234/api1/DEF
Server: wrong username/password (username:password) request to /api1/DEF
Credential Plugin: returning credential 'username:password1'
Credential Plugin: returning credential 'username:password1'
Client: response code Unauthorized from http://localhost:1234/api1/DEF
Server: wrong username/password (username:password) request to /api2/456
Credential Plugin: returning credential 'username:password1'
Credential Plugin: returning credential 'username:password1'
Client: response code Unauthorized from http://localhost:1234/api2/456

Regression?

No. Same behaviour on .NET Framework, and with WinHttpHandler.

Known Workarounds

1. Don't use PreAuthenticate

The problem with this is that it doubles the number of HTTP requests the client sends to the server (and increases server load) because time the app asks HttpClient to send a request, it's first sent unauthenticated, and then HttpClient will silently try again with credentials. Depending on the customer's network, the latency in doubling the number of requests can also have a noticeable perf impact, depending on the scenario.

2. Don't use HttpClientHandler.Credentials for HTTP Basic auth

In an app that supports NTLM, Kerberos/Negotiate, and Basic auth, the app has to increase complexity, duplicating some code that HttpClient already has to handle auth depending on the scheme requested by the 401 response's WWW-Authenticate header.

Configuration

No response

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

I have a client app that talks to remote HTTP servers via HttpClient, and authentication handled via HttpClientHandler.Credentials, also using HttpClientHandler.PreAuthenticate = true to avoid needlessly hammering the web server with unauthenticated HTTP requests that need to get HTTP 401 responses (fewer requests also reduces latency, from the client point of view).

Say the customer logs into the website and changes their password, or we're sending OAuth2 tokens over Basic auth, rather than Bearer. It appears that HttpClientHandler won't refresh the credential in its credential cache, despite getting a fresh credential from HttpClientHandler.Credentials.

FWIW, it also looks like there's a potential perf improvement by avoiding getting the credential twice. In the sample output below, look for the "Credential Plugin: returning credential" lines.

Reproduction Steps

Here's a sample app that demonstrates:

using System.Net;
using System.Text;

HttpListener listener = new HttpListener();
string prefix = "http://localhost:1234/";
listener.Prefixes.Add(prefix);
listener.Start();

var credentialPlugin = new CredentialPlugin();

Task httpHandlerTask = HandleHttpRequests(listener);

using HttpClientHandler httpClientHandler = new()
{
    PreAuthenticate = true,
    Credentials = credentialPlugin
};
using HttpClient httpClient = new(httpClientHandler);

await SendRequest(httpClient, prefix);
await SendRequest(httpClient, prefix + "api1/ABC");
await SendRequest(httpClient, prefix + "api2/123");

credentialPlugin.RefreshToken();

await SendRequest(httpClient, prefix + "api1/DEF");
// Try another time, just in case the cred cache is updated after the first failed attempt
await SendRequest(httpClient, prefix + "api1/DEF");

await SendRequest(httpClient, prefix + "api2/456");

listener.Stop();
await httpHandlerTask;

async Task SendRequest(HttpClient client, string url)
{
    using var response = await httpClient.GetAsync(url);
    Console.WriteLine($"Client: response code {response.StatusCode} from {url}");
}

async Task HandleHttpRequests(HttpListener listener)
{
    while (true)
    {
        try
        {
            var request = await listener.GetContextAsync();
            var authenticationHeader = request.Request.Headers.Get("Authorization");
            if (string.IsNullOrEmpty(authenticationHeader))
            {
                Console.WriteLine("Server: unauthenticated request to " + request.Request.RawUrl);
                request.Response.AddHeader("WWW-Authenticate", "basic");
                request.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
            }
            else
            {
                var auth = request.Request.Headers["Authorization"];
                var decoded = Encoding.UTF8.GetString(Convert.FromBase64String(auth.Replace("Basic", "", StringComparison.OrdinalIgnoreCase)));
                if (decoded == $"{credentialPlugin.UserName}:{credentialPlugin.Password}")
                {
                    Console.WriteLine($"Server: authenticated request ({decoded}) to {request.Request.RawUrl}");
                    request.Response.StatusCode = (int)HttpStatusCode.OK;
                }
                else
                {
                    Console.WriteLine($"Server: wrong username/password ({decoded}) request to " + request.Request.RawUrl);
                    request.Response.AddHeader("WWW-Authenticate", "basic");
                    request.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
                }
            }
            request.Response.Close();
        }
        catch
        {
            // this is how http listener "gracefully" stops?
            return;
        }
    }
}

class CredentialPlugin : ICredentials
{
    public CredentialPlugin()
    {
        UserName = "username";
        counter = 0;
        Password = "password";
    }

    private int counter;
    public string UserName {get; private set;}
    public string Password { get;private set;}

    // pretend this comes from an OAuth2 service
    public void RefreshToken()
    {
        counter++;
        Password = "password" + counter;
        Console.WriteLine($"Credential Plugin: Changed to '{UserName}:{Password}'");
    }

    NetworkCredential? ICredentials.GetCredential(Uri uri, string authType)
    {
        Console.WriteLine($"Credential Plugin: returning credential '{UserName}:{Password}'");
        return new NetworkCredential(UserName, Password);
    }
}

Expected behavior

After the "credential plugin" changes the password, I would expect HttpClient to:

1. Send a request to the server with the old credentials (which it does)
2. When the request fails with 401, get new credentials (which it does)
3. Send a new request to the server with the new credentials (it does not currently)
4. Update the credential cache, so subsequent requests use the new credentials, not the old credentials (it does not currently)

Basically, when it gets an HTTP 401 response, I'd like it to clear the credentials from the pre-auth cache, and then mimic the behaviour when the first request didn't have auth in the first place.

Here's pseudocode (that looks awfully like valid C# code) that explains my expected behaviour. Basically, try once (preauth if possible), and if the response is a 401, try a second time if credentials can be obtained.

async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request)
{
  HttpResponseMessage response;
   if (_options.PreAuthenticate && TryGetCachedCredential(request.Uri, out var credential)
   {
     response = await SendWithAuthAsync(request, credential);
   }
   else
  {
    response = await SendVerbatimAsync(request);
  }

  if (response.Status == 401 && _options.Credentials != null)
  {
    credential = _options.Credentials.GetCredentials(request.Uri, response.Headers.WwwAuthorization.Scheme);
    if (credential != null)
    {
      response = await SendWithAuthAsync(request, credential);
      if (response.Status != 401)
      {
        UpdatePreauthCache(request.Uri, credential);
      }
    }
  }

  return response;
}

I'm sure there's a lot more complexity than what I assume, but I hope it explains what I thought HttpClient was going to do, and what I'd like it to do.

Actual behavior

Here's the output of my sample app. You can see it only tries the first password over and over, and despite asking the (simulated) "credential plugin" for a fresh token (twice per HTTP request), it keeps trying the old

Server: unauthenticated request to /
Credential Plugin: returning credential 'username:password'
Credential Plugin: returning credential 'username:password'
Server: authenticated request (username:password) to /
Client: response code OK from http://localhost:1234/
Server: authenticated request (username:password) to /api1/ABC
Client: response code OK from http://localhost:1234/api1/ABC
Server: authenticated request (username:password) to /api2/123
Client: response code OK from http://localhost:1234/api2/123
Credential Plugin: Changed to 'username:password1'
Server: wrong username/password (username:password) request to /api1/DEF
Credential Plugin: returning credential 'username:password1'
Credential Plugin: returning credential 'username:password1'
Client: response code Unauthorized from http://localhost:1234/api1/DEF
Server: wrong username/password (username:password) request to /api1/DEF
Credential Plugin: returning credential 'username:password1'
Credential Plugin: returning credential 'username:password1'
Client: response code Unauthorized from http://localhost:1234/api1/DEF
Server: wrong username/password (username:password) request to /api2/456
Credential Plugin: returning credential 'username:password1'
Credential Plugin: returning credential 'username:password1'
Client: response code Unauthorized from http://localhost:1234/api2/456

Regression?

No. Same behaviour on .NET Framework, and with WinHttpHandler.

Known Workarounds

1. Don't use PreAuthenticate

The problem with this is that it doubles the number of HTTP requests the client sends to the server (and increases server load) because time the app asks HttpClient to send a request, it's first sent unauthenticated, and then HttpClient will silently try again with credentials. Depending on the customer's network, the latency in doubling the number of requests can also have a noticeable perf impact, depending on the scenario.

2. Don't use HttpClientHandler.Credentials for HTTP Basic auth

In an app that supports NTLM, Kerberos/Negotiate, and Basic auth, the app has to increase complexity, duplicating some code that HttpClient already has to handle auth depending on the scheme requested by the 401 response's WWW-Authenticate header.

Configuration

No response

Other information

No response

Author:	zivkan
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	-

[zivkan]

The behaviour is related to:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.cs

Lines 268 to 274 in f9cbb40

	if (performedBasicPreauth)
	{
	if (NetEventSource.Log.IsEnabled())
	{
	NetEventSource.AuthenticationError(authUri, $"Pre-authentication with {(isProxyAuth ? "proxy" : "server")} failed.");
	}
	break;

There are no comments to explain why it works in this way. A few lines earlier, on line 242, it obtains new credentials, but I don't understand why it bails out when performedBasicPreauth is true. It's not a safe assumption that the creds it obtained on line 242 are the same creds that were used from the PreAuth cache when making the request on line 240.

[zivkan]

From the docs: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclienthandler.preauthenticate?view=net-7.0

If the client request to a specific Uri is not successfully authenticated, the request uses standard authentication procedures.

Granted, this sentence/paragraph is after another paragraph that starts "After a client request to a specific Uri is successfully authenticated".

But English is a bit ambiguous, so I'm not sure the documentation is sufficiently clear to set expectations the behaviour is a bug (as I originally claimed), or not. The docs don't explicitly state what should be expected if the credentials change after HttpClientHandler caches credentials.

I did notice that HttpClientHandler.Credentials setter throws if called after at least 1 HTTP request is sent. I haven't yet tried to use the BCL's CredentialCache allows changing credentials after a request. But CredentialCache isn't used in my app (NuGet). Long before I joined the team, someone else implemented a custom class that implements ICredentials. The docs for ICredentials.GetCredential also doesn't discuss whether it's valid to return a different value for the same URL, in a different point in time.

[rzikm]

I have had a look at this, and the fix does not seem to be a simple oneliner as I originally thought.

The underlying HTTP stack has a separate CredentialCache to keep track of which creds on which URI have succeeded and reuses them from there. But, unfortunately, in order to remove the credentials from the cache you have to guess the exact uri used to insert it in.

I see few options:

• Add a new method to CredentialCache (which requires going through API review since the type is in System.Net.Primitives assembly, not System.Net.Http)
• Move the type to System.Net.Http assembly so we can add internal method for that
• Ditch the type altogether, and implement another cache from scratch, while it may seem like duplicating work, it also presents opportunity for extending the cache to Digest and other schemes (e.g. Bearer requested in another issue) in the future, as modifying the cache to store more data like scheme and other attributes needed is simple on internal types.

@dotnet/ncl, what do you think?

[rzikm]

Another, more drastic option would be not having the cache at all and always flowing the creds if PreAuthenticate is true. This would be a breaking change and would create a difference between .NET Framework and .NET Core (which we appraently had in the past but fixed in dotnet/corefx#28047). It seems to me that the preauthentication cache is there because of this section in the Basic auth RFC

Given the absolute URI ([RFC3986], Section 4.3) of an authenticated
request, the authentication scope of that request is obtained by
removing all characters after the last slash ("/") character of the
path component ("hier_part"; see [RFC3986], Section 3). A client
SHOULD assume that resources identified by URIs with a prefix-match
of the authentication scope are also within the protection space
specified by the realm value of that authenticated request.

A client MAY preemptively send the corresponding Authorization header
field with requests for resources in that space without receipt of
another challenge from the server. Similarly, when a client sends a
request to a proxy, it MAY reuse a user-id and password in the Proxy-
Authorization header field without receiving another challenge from
the proxy server.

Neither the Basic auth RFC nor general HTTP Auth RFC prohibit clients sending auth headers without prior challenge from the server (the wording in general HTTP auth RFC actually suggests that servers should be prepared to receive unprompted Auth headers).

[karelz]

I don't think we should go back to always PreAuthenticate behavior.
If we have a copy of CredentialCache, won't that cause problems wherever the original type is used?

[ManickaP]

Just as a technical possibility this could be solved by subclassing NetworkingCredential for the purposes of PreAuthCredentials cache and remembering in it the key under which it was inserted. Whether this a good idea, that's a different question 😄

[rzikm]

If we have a copy of CredentialCache, won't that cause problems wherever the original type is used?

not sure I understand, the instance I am talking about is one internally created by the HTTP connection pool, and is solely under our control.

Just as a technical possibility this could be solved by subclassing NetworkingCredential for the purposes of PreAuthCredentials cache and remembering in it the key under which it was inserted. Whether this a good idea, that's a different question 😄

right, it might work but it really feels ugly

[wfurt]

I would probably support the idea of subclass (or private clone) if this is something the implementation keeps internally. One could implement ICredentials object and manage the credentials themselves but it would still not work if we clone it and use it under the cover.

We could also add public surface to CredentialCache if that looks reasonable and do it only if the provided credentials is instance of it.

[rzikm]

We discussed this in the team and will go with something resembling the third option

Ditch the type altogether, and implement another cache from scratch, while it may seem like duplicating work, it also presents opportunity for extending the cache to Digest and other schemes (e.g. Bearer requested in #91867) in the future, as modifying the cache to store more data like scheme and other attributes needed is simple on internal types.

Attempt will be to reuse as much as possible from the CredentialCache type to avoid reinventing the wheel.