-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Refresh Authorization Code on When.All (causing multiple refresh authentications) #2014
Comments
Hi Leonard, I've provided three slightly different implementation examples here how to solve refresh token problem with Polly. Similar question as your has been asked at end of last year: how to make sure that only a single refresh request is sent out. Here I have provided a sample for that (basically a IMHO my suggested solution is a bit more convenient than passing around the |
@peter-csala thank you for the solution provided! I'm guessing in my user case I will still need the i.e.: HttpClient httpClient = (HttpClient)context["httpClient"];
if (await localStorageService.GetSecurityTokenAsync().ConfigureAwait(false) is string securityToken
&& !string.IsNullOrEmpty(securityToken))
{
if (httpClient.DefaultRequestHeaders.Contains(Constants.AuthenticationTokenHeaderKey))
{
_ = httpClient.DefaultRequestHeaders.Remove(Constants.AuthenticationTokenHeaderKey);
}
httpClient.DefaultRequestHeaders.Add(Constants.AuthenticationTokenHeaderKey, securityToken);
} On next retry the |
The presented code itself yes it updates the related header in a way that the next attempt could read the refreshed token. But because it is just a code fragment that's why I can not say that it solves your original question/problem. Your header update logic is not atomic so, multiple threads can update the header simultaneously if it is not treated as a critical section (by protecting with a lock). As with my suggested sample make sure only one thread can perform the token refresh at the same time. |
@peter-csala thank you for your quick response 👍 The related code is the following: public static AsyncRetryPolicy<HttpResponseMessage> AuthEnsuringPolicy = Policy<HttpResponseMessage>
.HandleResult(r => r.StatusCode == HttpStatusCode.Unauthorized)
.RetryAsync(1, onRetryAsync: async (ex, i, context) => _ = await App.ContainerProvider.Resolve<IAuthTokenService>().RefreshTokenAsync((HttpClient)context["httpClient"]).ConfigureAwait(true)); The public class AuthTokenService : IAuthTokenService
{
private static readonly SemaphoreSlim _semaphoreSlim = new(1);
private DateTime? _lastRefreshed;
public async Task<bool> RefreshTokenAsync(HttpClient httpClient)
{
await _semaphoreSlim.WaitAsync().ConfigureAwait(false);
try
{
//Use any arbitrary logic to detect simultaneous calls
if (_lastRefreshed.HasValue && _lastRefreshed - DateTime.UtcNow < TimeSpan.FromSeconds(3))
{
Console.WriteLine("No refreshment happened");
return false;
}
ILocalStorageService localStorageService = App.ContainerProvider.Resolve<ILocalStorageService>();
if (localStorageService.GetCustomer() is Customer customer
&& await customer.GetPasswordAsync().ConfigureAwait(true) is string password
&& await App.ContainerProvider.Resolve<IUserManagementService>().RefreshAuthorizationTokenAsync(customer.Mobile, password).ConfigureAwait(true) is not null
&& await localStorageService.GetSecurityTokenAsync().ConfigureAwait(false) is string securityToken
&& !string.IsNullOrEmpty(securityToken))
{
if (httpClient.DefaultRequestHeaders.Contains(Constants.AuthenticationTokenHeaderKey))
{
_ = httpClient.DefaultRequestHeaders.Remove(Constants.AuthenticationTokenHeaderKey);
}
httpClient.DefaultRequestHeaders.Add(Constants.AuthenticationTokenHeaderKey, securityToken);
Debug.WriteLine($"Refreshment happened {DateTime.UtcNow}");
_lastRefreshed = DateTime.UtcNow;
}
}
finally
{
_ = _semaphoreSlim.Release();
}
return false;
}
} The requestMessage.SetPolicyExecutionContext(new Context
{
{ "retrycount", 0 },
{ "httpClient", httpClient }
});
HttpResponseMessage httpResponseMessage = await httpClient.SendAsync(requestMessage).ConfigureAwait(false);
using Stream stream = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
using StreamReader steamReader = new(stream);
using JsonTextReader jsonTextReader = new(steamReader);
if (httpResponseMessage.IsSuccessStatusCode)
{
return new MemoryStream(buffer: stream.ToByteArray());
} Just a side question as well, on the if (_lastRefreshed.HasValue && _lastRefreshed - DateTime.UtcNow < TimeSpan.FromSeconds(3))
{
Console.WriteLine("No refreshment happened");
return false;
} |
@LeoJHarris According to my understanding there both of these means "exclusive lock" private static readonly SemaphoreSlim _semaphoreSlim = new(1); and private static readonly SemaphoreSlim _semaphoreSlim = new(1, 1); When you acquire a token the semaphore is decrementing its counter. So, if we would call the Here is a dotnet fiddle to play with it: https://dotnetfiddle.net/o6vEk2 |
@peter-csala Hi have found the lock to be working correctly, no issues there, however the following seems to be happening when attempting to refresh the token and make subsequent calls again on retry: First time: [0:] Did Login 3/13/2024 10:44:28 PM
[0:] Refreshment happened 3/13/2024 10:44:28 PM
[0:] No refreshment happened 3/13/2024 10:44:28 PM
[0:] No refreshment happened 3/13/2024 10:44:28 PM
[0:] No refreshment happened 3/13/2024 10:44:28 PM
[0:] No refreshment happened 3/13/2024 10:44:28 PM
[0:] No refreshment happened 3/13/2024 10:44:28 PM
[0:] No refreshment happened 3/13/2024 10:44:28 PM Later on this is called again shortly after on retry: [0:] Did Login 3/13/2024 10:44:40 PM
[0:] Refreshment happened 3/13/2024 10:44:40 PM
[0:] No refreshment happened 3/13/2024 10:44:40 PM
... I have updated the public static AsyncRetryPolicy<HttpResponseMessage> AuthEnsuringPolicy = Policy
.Handle<HttpRequestException>()
.OrResult<HttpResponseMessage>(resp => resp.StatusCode == HttpStatusCode.Unauthorized)
.WaitAndRetryAsync(3,
retryAttempt => TimeSpan.FromSeconds(15),
onRetry: async (resp, timeSpan, context) => _ = await App.ContainerProvider.Resolve<IAuthTokenService>().RefreshTokenAsync((HttpClient)context["httpClient"]).ConfigureAwait(true));
} I could be wrong but from my testing it would seem that when setting |
Sorry for the late response I was away from keyboard for several days. I've tried to reproduce your problem with and without Polly. Without Pollyprivate const string ClientName = "TestClient";
private const string HeaderKey = "AuthToken";
public static async Task Main()
{
var collection = new ServiceCollection();
collection.AddHttpClient(ClientName, (sp, client) =>
{
client.DefaultRequestHeaders.Add(HeaderKey, GetToken());
});
var provider = collection.BuildServiceProvider();
//Simulate retry
for(int i = 0; i < 5; i++)
{
Test(provider);
await Task.Delay(Random.Shared.Next(1000));
}
}
public static void Test(IServiceProvider sp)
{
var factory = sp.GetRequiredService<IHttpClientFactory>();
var client = factory.CreateClient(ClientName);
var token = client.DefaultRequestHeaders.GetValues(HeaderKey).First();
Console.WriteLine($"Token: {token}");
//Simulate token refresh
client.DefaultRequestHeaders.Remove(HeaderKey);
client.DefaultRequestHeaders.Add(HeaderKey, GetToken());
}
private static string GetToken() => DateTime.UtcNow.TimeOfDay.ToString(); And it correctly updates the header. See the related dotnet fiddle. With Pollyprivate const string ClientName = "TestClient";
private const string HeaderKey = "AuthToken";
private const string ContextKey = "HttpClient";
public static async Task Main()
{
var collection = new ServiceCollection();
collection.AddHttpClient(ClientName, (sp, client) =>
{
client.DefaultRequestHeaders.Add(HeaderKey, GetToken());
});
var provider = collection.BuildServiceProvider();
var context = new Polly.Context();
var factory = provider.GetRequiredService<IHttpClientFactory>();
var client = factory.CreateClient(ClientName);
context[ContextKey] = client;
try
{
await GetRetry().ExecuteAsync(ctx => Test(), context);
}
catch(Exception)
{
Console.WriteLine("Final retry failed as well");
}
}
public static Task Test() => Task.FromException(new Exception("Damn"));
private static string GetToken() => DateTime.UtcNow.TimeOfDay.ToString();
public static IAsyncPolicy GetRetry() =>
Policy
.Handle<Exception>()
.WaitAndRetryAsync(4,
retryAttempt => TimeSpan.FromMilliseconds(Random.Shared.Next(1000)),
onRetry: (ex, ts, ctx) => {
var client = ctx[ContextKey] as HttpClient;
var token = client.DefaultRequestHeaders.GetValues(HeaderKey).First();
Console.WriteLine($"Token: {token}");
client.DefaultRequestHeaders.Remove(HeaderKey);
client.DefaultRequestHeaders.Add(HeaderKey, GetToken());
}); And it also correctly updates the header. See the related dotnet fiddle. So, there must be something else which is not shared in this thread. |
@peter-csala sorry for my own delay in responding, many tasks I am across and put this issue back a bit but I will check this out shortly and get back to you if I can find further information on this. Will be in touch shortly. |
@peter-csala again thank you for your continued assistance on this issue 🙏 hopefully the following can provide a more complete picture of what my code is doing but somethings have been deliberately omitted for brevity, I don't think there is anything outside these code blocks further I can add that would impact the ability to refresh the token for future calls: App.xaml.cs Sets up the IAsyncPolicy<HttpResponseMessage> wrapOfRetryAndFallback =
Policy.WrapAsync(Policy.HandleResult<HttpResponseMessage>(r =>
!r.IsSuccessStatusCode).FallbackAsync(fallbackActionAsync, onFallBackAsync), PollyPolicies.AuthEnsuringPolicy);
_ = containerRegistry.RegisterSingleton<IHttpClientFactory>(() =>
new ServiceCollection().AddHttpClient(Constants.HttpClientWithRetry).AddPolicyHandler(wrapOfRetryAndFallback).Services.BuildServiceProvider().GetService<IHttpClientFactory>()); ApiService.cs Following is where the initial request is called from such as async Task<IdentityToken?> IApiService.LoginCustomerAsync(Login login)
{
StringContent stringContent = new(login.ToJson(), Encoding.UTF8, "application/json");
MemoryStream? result =
await executeRestRequestAsync(new Uri(string.Format("{0}/Access/Login?...."))), HttpMethod.Post, stringContent, requiresAuthenticationTokenHeader: false)
.ConfigureAwait(false);
if (result is not null)
{
DataContractJsonSerializer js = new(typeof(IdentityToken));
return (IdentityToken?)js.ReadObject(result);
}
throw new Exception("HTTP returned null");
} Method that handles each request, this is where I retrieve the Security Token if it exists will be used as the private async Task<MemoryStream?> executeRestRequestAsync(Uri uri, HttpMethod httpMethod, StringContent? stringContent = null, bool requiresAuthenticationTokenHeader = true)
{
HttpClient httpClient = _httpClientFactory.CreateClient(Constants.HttpClientWithRetry);
// Only add security key if empty and not null (will be null on first login customer
// required on that request)
if (await _localStorageService.GetSecurityTokenAsync().ConfigureAwait(false) is string securityToken && !string.IsNullOrEmpty(securityToken) && requiresAuthenticationTokenHeader)
{
if (httpClient.DefaultRequestHeaders.Contains(Constants.AuthenticationTokenHeaderKey))
{
_ = httpClient.DefaultRequestHeaders.Remove(Constants.AuthenticationTokenHeaderKey);
}
httpClient.DefaultRequestHeaders.Add(Constants.AuthenticationTokenHeaderKey, securityToken);
}
HttpRequestMessage? requestMessage = null;
// Check the HTTP method enum to determine the HTTP method to call
switch (httpMethod)
{
case HttpMethod.Get:
requestMessage = new(System.Net.Http.HttpMethod.Get, uri);
break;
case HttpMethod.Post:
requestMessage = new(System.Net.Http.HttpMethod.Post, uri)
{
Content = stringContent
};
break;
case HttpMethod.Put:
requestMessage = new(System.Net.Http.HttpMethod.Put, uri)
{
Content = stringContent
};
break;
case HttpMethod.Delete:
requestMessage = new(System.Net.Http.HttpMethod.Delete, uri);
break;
}
requestMessage.SetPolicyExecutionContext(new Context
{
{ "retrycount", 2 },
{ "httpClient", httpClient }
});
HttpResponseMessage httpResponseMessage = await httpClient.SendAsync(requestMessage).ConfigureAwait(true);
using Stream stream = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
using StreamReader steamReader = new(stream);
using JsonTextReader jsonTextReader = new(steamReader);
if (httpResponseMessage.IsSuccessStatusCode)
{
return new MemoryStream(buffer: stream.ToByteArray());
}
... PollyPolicies.cs The retry policy will take the public static AsyncRetryPolicy<HttpResponseMessage> AuthEnsuringPolicy = Policy
.Handle<HttpRequestException>()
.OrResult<HttpResponseMessage>(resp => resp.StatusCode == HttpStatusCode.Unauthorized && App.ContainerProvider.Resolve<ILocalStorageService>().GetStaySignedIn())
.WaitAndRetryAsync(3,
retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)),
onRetry: async (resp, timeSpan, context) =>
{
if (context["httpClient"] is HttpClient httpClient
&& App.ContainerProvider.Resolve<ILocalStorageService>().GetCustomer() is Customer customer && !customer.IsLoggedIn
&& customer.StaySignedIn)
{
_ = await App.ContainerProvider.Resolve<IAuthTokenService>().RefreshTokenAsync(httpClient).ConfigureAwait(true);
}
}); AuthTokenService.cs This appears to be working fine, the public class AuthTokenService : IAuthTokenService
{
private static readonly SemaphoreSlim _semaphoreSlim = new(1);
private static DateTime? _lastRefreshed;
public async Task<bool> RefreshTokenAsync(HttpClient httpClient)
{
await _semaphoreSlim.WaitAsync().ConfigureAwait(true);
try
{
if (!_lastRefreshed.HasValue)
{
await UpdateAuthenticationTokenHeaderAsync(httpClient).ConfigureAwait(true);
}
else if ((_lastRefreshed.Value - DateTime.UtcNow).Duration() < TimeSpan.FromSeconds(5).Duration())
{
Debug.WriteLine($"No refreshment happened {DateTime.UtcNow}");
return false;
}
else
{
await UpdateAuthenticationTokenHeaderAsync(httpClient).ConfigureAwait(true);
}
}
finally
{
_ = _semaphoreSlim.Release();
}
return false;
}
private static async Task UpdateAuthenticationTokenHeaderAsync(HttpClient httpClient)
{
ILocalStorageService localStorageService = App.ContainerProvider.Resolve<ILocalStorageService>();
if (localStorageService.GetCustomer() is Customer customer
&& await customer.GetPasswordAsync().ConfigureAwait(true) is string password
&& await App.ContainerProvider.Resolve<IUserManagementService>().RefreshAuthorizationTokenAsync(customer.Mobile, password).ConfigureAwait(true) is not null)
{
if (httpClient.DefaultRequestHeaders.Contains(Constants.AuthenticationTokenHeaderKey))
{
_ = httpClient.DefaultRequestHeaders.Remove(Constants.AuthenticationTokenHeaderKey);
}
if (await localStorageService.GetSecurityTokenAsync().ConfigureAwait(false) is string securityToken
&& !string.IsNullOrEmpty(securityToken))
{
httpClient.DefaultRequestHeaders.Add(Constants.AuthenticationTokenHeaderKey, securityToken);
}
Debug.WriteLine($"Refreshment happened {DateTime.UtcNow}");
_lastRefreshed = DateTime.UtcNow;
}
}
} UserManagementService.cs The end routine lays within the async Task<long?> IUserManagementService.RefreshAuthorizationTokenAsync(string mobileNumber, string password)
{
IdentityToken? identityToken = await Policy<IdentityToken?>.Handle<Exception>().FallbackAsync(async (outcome, context, ct) =>
{
...
}
return outcome.Result;
}, (ct, cx) => Task.CompletedTask).ExecuteAsync(async () => await _apiService.LoginCustomerAsync(new Login
{
Password = password,
Username = mobileNumber
}).ConfigureAwait(true)).ConfigureAwait(true);
if (identityToken is not null)
{
bool isCustomer = false;
string? identityId = string.Empty;
if (!identityToken.Claims.Any())
{
...
}
else
{
foreach (IdentityTokenClaim c in identityToken.Claims)
{
if (string.Compare(c.ClaimType, Constants.RoleClaim) == 0)
{
isCustomer = string.Compare(c.ClaimValue, "Customer", true) == 0;
}
if (string.Compare(c.ClaimType, Constants.NameIdentifierClaim) == 0)
{
identityId = c.ClaimValue;
}
}
if (isCustomer && identityId is not null && long.TryParse(identityId, out long customerId))
{
await _localStorageService.SetAuthTokenAsync(identityToken.SecurityToken).ConfigureAwait(true);
_localStorageService.SetSecurityTokenExpiry(identityToken.Expires);
return customerId;
}
else
{
...
}
}
}
return null;
} Let me know if you can see something that might be causing issues but from the |
@LeoJHarris Could you please create a sample github repo with the above code fragments? Then I could play with it on my machine to better understand the data and control flow. UPDATE 1 I tried to extract relevant code fragments from your post. Mainly decorating the public static async Task Main()
{
var collection = new ServiceCollection();
collection.AddHttpClient(ClientName, (sp, client) =>
{
client.DefaultRequestHeaders.Add(HeaderKey, GetToken());
})
.AddPolicyHandler(GetRetry());
var provider = collection.BuildServiceProvider();
var context = new Polly.Context();
var factory = provider.GetRequiredService<IHttpClientFactory>();
var client = factory.CreateClient(ClientName);
context[ContextKey] = client;
HttpRequestMessage message = new HttpRequestMessage(HttpMethod.Get, "http://httpstat.us/500");
message.SetPolicyExecutionContext(context);
try
{
var response = await client.SendAsync(message);
response.EnsureSuccessStatusCode(); //throws HttpRequestException
}
catch(Exception)
{
Console.WriteLine("Final retry failed as well");
}
} private static string GetToken() => DateTime.UtcNow.TimeOfDay.ToString();
private static IAsyncPolicy<HttpResponseMessage> GetRetry() =>
Policy<HttpResponseMessage>
.Handle<HttpRequestException>()
.OrResult(res => !res.IsSuccessStatusCode)
.WaitAndRetryAsync(4,
retryAttempt => TimeSpan.FromMilliseconds(Random.Shared.Next(1000)),
onRetry: (ex, ts, ctx) => {
var client = ctx[ContextKey] as HttpClient;
var token = client.DefaultRequestHeaders.GetValues(HeaderKey).First();
Console.WriteLine($"Token: {token}");
client.DefaultRequestHeaders.Remove(HeaderKey);
client.DefaultRequestHeaders.Add(HeaderKey, GetToken());
}); It still works like a charm. |
I will try get a sample application shortly. Thank you! |
@peter-csala this might take some time to get you a sample application, but Ill endeavor to get this ASAP. Keen to get this sorted. |
Sure thing, no problem. Take your time. |
This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made. |
This issue was closed because it has been inactive for 14 days since being marked as stale. |
What are you wanting to achieve?
I am trying to refresh an expired authentication code whilst also ensuring that the refreshing authentication code block is not called repeated times for every fail.
In our application we use
Task.WhenAll
that will fire multiple requests, if all requests should fail due toUnauthorized
then theAuthEnsuringPolicy
executes which is currently being called multiple times which is not ideal.What code or approach do you have so far?
I followed this example to use the
Context
and it works fine when making single requests. The issues starts happening for multiple requests such as usingTask.WhenAll
for sending multiple http requests, the Polly Policy is called for every fail resulting in many attempts to refresh code, it would be better if we can cancel other attempts to refresh code on first entering the code block and then retry all requests again once a successful refresh was achieved on the first attempt. Can this be done?So far my code is:
Also we setup like this:
Usage:
Additional context
Initially I thought we could use the CancellationTokenSource somewhere but not to sure.
The text was updated successfully, but these errors were encountered: