Skip to content

Refactor the token verification in CookieAuthenticationOptionsExtensions to use refresh_token to re-acquire a token when it expires, and use IAbpDistributedLock. #25011

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.

Sign up for GitHub

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

Open
lanpin wants to merge 1 commit into
base: dev
Choose a base branch
from
+105 −3
Open

Refactor the token verification in CookieAuthenticationOptionsExtensions to use refresh_token to re-acquire a token when it expires, and use IAbpDistributedLock. #25011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions ...NetCore/Microsoft/Extensions/DependencyInjection/CookieAuthenticationOptionsExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Volo.Abp.DistributedLocking;
using Volo.Abp.Threading;

namespace Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -35,6 +36,104 @@ public static CookieAuthenticationOptions CheckTokenExpiration(this CookieAuthen
if (!tokenExpiresAt.IsNullOrWhiteSpace() && DateTimeOffset.TryParseExact(tokenExpiresAt, "o", CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var expiresAt) &&
expiresAt <= DateTimeOffset.UtcNow.Add(advance.Value))
{
var refreshToken = principalContext.Properties.GetTokenValue("refresh_token");
if (refreshToken.IsNullOrWhiteSpace())
{
await SignOutAndInvokePreviousHandlerAsync(principalContext, previousHandler);
return;
}

logger.LogInformation("The access_token expires within {AdvanceSeconds}s but a refresh_token is available; attempting to refresh.", advance.Value.TotalSeconds);

var openIdConnectOptions = await GetOpenIdConnectOptions(principalContext, oidcAuthenticationScheme);

var tokenEndpoint = openIdConnectOptions.Configuration?.TokenEndpoint;
if (tokenEndpoint.IsNullOrWhiteSpace() && !openIdConnectOptions.Authority.IsNullOrWhiteSpace())
{
tokenEndpoint = openIdConnectOptions.Authority.EnsureEndsWith('/') + "connect/token";
}

if (tokenEndpoint.IsNullOrWhiteSpace())
{
logger.LogWarning("No token endpoint configured. Skipping token refresh.");
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log message says “Skipping token refresh.” but the code immediately signs the user out. This is misleading when diagnosing production issues; consider updating the message to reflect the actual behavior (no endpoint configured => sign-out) or change the behavior to truly skip refresh without signing out when the token is not yet expired.

Suggested change
logger.LogWarning("No token endpoint configured. Skipping token refresh.");
logger.LogWarning("No token endpoint configured. Skipping token refresh and signing out.");

Copilot uses AI. Check for mistakes.
await SignOutAndInvokePreviousHandlerAsync(principalContext, previousHandler);
return;
}

var clientId = principalContext.Properties.GetString("client_id");
var clientSecret = principalContext.Properties.GetString("client_secret");

var refreshRequest = new RefreshTokenRequest
{
Address = tokenEndpoint,
ClientId = clientId ?? openIdConnectOptions.ClientId!,
ClientSecret = clientSecret ?? openIdConnectOptions.ClientSecret,
RefreshToken = refreshToken
};

var cancellationTokenProvider = principalContext.HttpContext.RequestServices.GetRequiredService<ICancellationTokenProvider>();

const int RefreshTokenLockTimeoutSeconds = 3;
const string RefreshTokenLockKeyFormat = "refresh_token_lock_{0}";

var userKey =
principalContext.Principal?.FindFirst("sub")?.Value
?? principalContext.Principal?.FindFirst(System.Security.Claims.ClaimTypes.NameIdentifier)?.Value
?? "unknown";

var lockKey = string.Format(CultureInfo.InvariantCulture, RefreshTokenLockKeyFormat, userKey);
var lockTimeout = TimeSpan.FromSeconds(RefreshTokenLockTimeoutSeconds);
Comment on lines +76 to +85
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock key is derived from the user’s "sub" / NameIdentifier claim, which causes all concurrent sessions for the same user (multiple browsers/devices) to contend on the same lock even though they have different refresh_tokens. This can introduce unnecessary blocking and makes the lock ineffective at its intended granularity (refresh token/session). Consider keying the lock off a stable per-session value (e.g., hash of the refresh_token or a session id claim) and avoid using a raw identifier directly in the lock name (PII leakage into the lock store).

Copilot uses AI. Check for mistakes.

var abpDistributedLock = principalContext.HttpContext.RequestServices.GetRequiredService<IAbpDistributedLock>();

await using (var handle = await abpDistributedLock.TryAcquireAsync(lockKey, lockTimeout, cancellationTokenProvider.Token))
{
if (handle != null)
{
var response = await openIdConnectOptions.Backchannel.RequestRefreshTokenAsync(refreshRequest, cancellationTokenProvider.Token);

if (response.IsError)
{
logger.LogError("Token refresh failed: {Error}", response.Error);
await SignOutAndInvokePreviousHandlerAsync(principalContext, previousHandler);
return;
}

if (response.ExpiresIn <= 0)
{
logger.LogWarning("The token endpoint response does not contain a valid expires_in value. Skipping token refresh.");
await SignOutAndInvokePreviousHandlerAsync(principalContext, previousHandler);
return;
}

if (response.AccessToken.IsNullOrWhiteSpace())
{
logger.LogWarning("The token endpoint response does not contain a new access_token. Skipping token refresh.");
await SignOutAndInvokePreviousHandlerAsync(principalContext, previousHandler);
return;
}

if (response.RefreshToken.IsNullOrWhiteSpace())
{
logger.LogInformation("The token endpoint response does not contain a new refresh_token. The old refresh_token will continue to be used until it expires.");
}

logger.LogInformation("Token refreshed successfully. Updating cookie with new tokens.");
var newTokens = new[]
{
new AuthenticationToken { Name = "access_token", Value = response.AccessToken },
new AuthenticationToken { Name = "refresh_token", Value = response.RefreshToken ?? refreshToken },
new AuthenticationToken { Name = "expires_at", Value = DateTimeOffset.UtcNow.AddSeconds(response.ExpiresIn).ToString("o", CultureInfo.InvariantCulture) }
};

principalContext.Properties.StoreTokens(newTokens);
principalContext.ShouldRenew = true;

await InvokePreviousHandlerAsync(principalContext, previousHandler);
return;
}
}

logger.LogInformation("The access_token expires within {AdvanceSeconds}s; signing out.", advance.Value.TotalSeconds);
await SignOutAndInvokePreviousHandlerAsync(principalContext, previousHandler);
return;
Comment on lines +89 to 139
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the distributed lock can’t be acquired (handle == null), the code falls through and signs the user out. In concurrent-request scenarios, one request will refresh the token while other requests fail to acquire the lock and will incorrectly log the user out. Instead, when the lock isn’t acquired, avoid signing out; either invoke the previous handler and let the next request use the renewed cookie, or only sign out when the token is already expired (expiresAt <= UtcNow) and refresh can’t be performed.

Copilot uses AI. Check for mistakes.
Expand Down
3 changes: 2 additions & 1 deletion framework/src/Volo.Abp.AspNetCore/Volo.Abp.AspNetCore.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk.Web">
<Project Sdk="Microsoft.NET.Sdk.Web">

<Import Project="..\..\..\configureawait.props" />
<Import Project="..\..\..\common.props" />
Expand All @@ -23,6 +23,7 @@
<ProjectReference Include="..\Volo.Abp.AspNetCore.Abstractions\Volo.Abp.AspNetCore.Abstractions.csproj" />
<ProjectReference Include="..\Volo.Abp.Auditing\Volo.Abp.Auditing.csproj" />
<ProjectReference Include="..\Volo.Abp.Authorization\Volo.Abp.Authorization.csproj" />
<ProjectReference Include="..\Volo.Abp.DistributedLocking.Abstractions\Volo.Abp.DistributedLocking.Abstractions.csproj" />
<ProjectReference Include="..\Volo.Abp.ExceptionHandling\Volo.Abp.ExceptionHandling.csproj" />
<ProjectReference Include="..\Volo.Abp.Http\Volo.Abp.Http.csproj" />
<ProjectReference Include="..\Volo.Abp.Security\Volo.Abp.Security.csproj" />
Expand Down
6 changes: 4 additions & 2 deletions framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/AbpAspNetCoreModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting.StaticWebAssets;
using Microsoft.AspNetCore.RequestLocalization;
Expand All @@ -18,6 +18,7 @@
using Volo.Abp.Uow;
using Volo.Abp.Validation;
using Volo.Abp.VirtualFileSystem;
using Volo.Abp.DistributedLocking;

namespace Volo.Abp.AspNetCore;

Expand All @@ -30,7 +31,8 @@ namespace Volo.Abp.AspNetCore;
typeof(AbpAuthorizationModule),
typeof(AbpValidationModule),
typeof(AbpExceptionHandlingModule),
typeof(AbpAspNetCoreAbstractionsModule)
typeof(AbpAspNetCoreAbstractionsModule),
typeof(AbpDistributedLockingAbstractionsModule)
)]
public class AbpAspNetCoreModule : AbpModule
{
Expand Down
Loading