Refactor the token verification in CookieAuthenticationOptionsExtensions to use refresh_token to re-acquire a token when it expires, and use IAbpDistributedLock.#25011
Conversation
…ons to use refresh_token to re-acquire a token when it expires, and use IAbpDistributedLock.
|
|
There was a problem hiding this comment.
Pull request overview
This PR refactors ABP’s cookie authentication token validation to refresh an expiring access_token using the refresh_token, coordinating refresh attempts via IAbpDistributedLock, and wires up the required distributed locking abstractions dependency in Volo.Abp.AspNetCore.
Changes:
- Add refresh-token based access token renewal during
CookieAuthenticationOptionsprincipal validation. - Use
IAbpDistributedLockto prevent concurrent refresh attempts. - Add
Volo.Abp.DistributedLocking.Abstractionsreference and module dependency to the ASP.NET Core package/module.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| framework/src/Volo.Abp.AspNetCore/Volo/Abp/AspNetCore/AbpAspNetCoreModule.cs | Adds module dependency on AbpDistributedLockingAbstractionsModule so locking services are available. |
| framework/src/Volo.Abp.AspNetCore/Volo.Abp.AspNetCore.csproj | Adds project reference to Volo.Abp.DistributedLocking.Abstractions. |
| framework/src/Volo.Abp.AspNetCore/Microsoft/Extensions/DependencyInjection/CookieAuthenticationOptionsExtensions.cs | Implements refresh-token flow and distributed-lock coordination during cookie principal validation. |
| 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; |
There was a problem hiding this comment.
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.
| 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); |
There was a problem hiding this comment.
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).
| | ||
| if (tokenEndpoint.IsNullOrWhiteSpace()) | ||
| { | ||
| logger.LogWarning("No token endpoint configured. Skipping token refresh."); |
There was a problem hiding this comment.
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.
| logger.LogWarning("No token endpoint configured. Skipping token refresh."); | |
| logger.LogWarning("No token endpoint configured. Skipping token refresh and signing out."); |
Description
Resolves #xxxx (write the related issue number if available)
TODO: Describe what this PR has changed, add screenshot or animated GIF if available, write if it is a breaking change, and how to fix the breaking changes for existing applications if so.
Checklist
How to test it?
Please describe how this can be tested by the test engineers if it is not already explicit - or remove this section if no need to description.
Refactor the token verification in CookieAuthenticationOptionsExtensions to use refresh_token to re-acquire a token when it expires, and use IAbpDistributedLock.
PR Classification
新增特性：为 ABP 框架的 Cookie 认证中间件引入 access_token 自动刷新机制，提升用户体验和安全性。
PR Summary
本次提交实现了 access_token 即将过期时自动使用 refresh_token 刷新令牌的功能，并通过分布式锁防止并发刷新问题，同时补充了相关依赖。