feat: Implement refresh token functionality and enhance authentication#51
Conversation
…n responses - Added RefreshAccessTokenCommand and RefreshAccessTokenCommandHandler to handle token refresh logic. - Introduced RefreshToken entity and repository for managing refresh tokens in the database. - Updated LoginCommand and LoginExternalCommand to include RememberMe flag for token persistence. - Enhanced LoginResponse and LoginExternalResponse to include access and refresh token expiration times. - Updated AuthController to include a new endpoint for refreshing access tokens. - Refactored ITokenService to return access token with expiration time. - Implemented validation for refresh token commands.
… revocation logic - Introduced SecurityException to handle security violations during token refresh. - Updated RefreshAccessTokenCommandHandler to revoke entire token chains upon detection of compromised tokens. - Enhanced IRefreshTokenRepository methods to include reason for token revocation. - Added RevokedReason property to RefreshToken entity for tracking revocation reasons. - Implemented migration for the new RefreshTokens table structure including RevokedReason. - Updated GlobalExceptionMiddleware to handle SecurityException and return appropriate responses.
WalkthroughImplements a comprehensive refresh token feature with token rotation, revocation, and security violation handling. Introduces RefreshToken domain entity, repository, and command handler; updates token service to return expiration dates; modifies login handlers to persist refresh tokens; removes token generation from registration; and adds endpoint for refreshing access tokens with chain revocation on compromise. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Handler as RefreshAccessTokenCommandHandler
participant TokenSvc as ITokenService
participant TokenRepo as IRefreshTokenRepository
participant UnitOfWork
Client->>Controller: POST /refresh-access-token (RefreshToken)
Controller->>Handler: Handle(command, cancellationToken)
Handler->>TokenRepo: GetByTokenAsync(token)
TokenRepo-->>Handler: refreshToken (or null)
alt Token not found or invalid
Handler-->>Controller: UnauthorizedException
else Token expired
Handler-->>Controller: UnauthorizedException
else Token revoked (compromised)
Handler->>Handler: RevokeEntireTokenChainAsync(token)
Handler->>TokenRepo: RevokeAsync (all chain tokens)
Handler->>TokenRepo: RevokeAllForUserAsync (all user tokens)
Handler->>UnitOfWork: SaveChangesAsync
Handler-->>Controller: SecurityException
else Token valid
Handler->>TokenSvc: GenerateAccessToken(user)
TokenSvc-->>Handler: (accessToken, expiresAt)
Handler->>TokenSvc: GenerateRefreshToken(user)
TokenSvc-->>Handler: newRefreshToken
Handler->>TokenRepo: RevokeAsync(oldToken, "TokenRotation")
Handler->>TokenRepo: AddAsync(newRefreshToken)
Handler->>UnitOfWork: SaveChangesAsync
Handler-->>Controller: Result.Success(RefreshAccessTokenResponse)
Controller-->>Client: 200 OK with new tokens
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring careful attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (17)
src/Application/Features/Auth/LoginExternal/LoginExternalResponse.cs (1)
20-24: Consider addingRefreshTokenExpiresAtfor external login consistency
LoginExternalResponseexposesRefreshTokenbut onlyAccessTokenExpiresAt.LoginResponsenow includes bothAccessTokenExpiresAtandRefreshTokenExpiresAt; mirroring that here would simplify client handling and keep the auth surface consistent across login flows.src/Application/Interfaces/Services/Auth/ITokenService.cs (1)
10-23: NewGenerateAccessTokensignature looks good; consider a small result DTO if this growsReturning
(string accessToken, DateTimeOffset accessTokenExpiresAt)matches the updatedTokenServiceimplementation and makes expiration explicit. If you later add more metadata (e.g., scopes, token type), a smallAccessTokenResultDTO could improve readability and versioning, but the tuple is perfectly fine for the current needs. Based on relevant code snippets.src/Domain/Constants/TokenConstants.cs (1)
1-54: Good centralization of token expirations and revocation reasonsThe
TokenConstantsclass nicely removes magicTimeSpans and reason strings from the domain logic. If these reasons are ever compared programmatically or localized, consider using short reason codes or an enum plus a separate mapping to display strings; otherwise this shape is fine.src/Application/Features/Auth/Register/RegisterResponse.cs (1)
33-41: Roles and avatar fields fit new registration flow; consider read‑only collectionThe shift from tokens to
RolesandAvatarUrlmatches the tokenless registration behavior and looks correct.If you don’t need to mutate
Rolesafter creation, consider exposingIReadOnlyList<string>orstring[]instead ofList<string>to avoid accidental modification of the response object post‑construction. This is optional but can make the DTO contract clearer.src/Application/Features/Auth/LoginExternal/LoginExternalCommand.cs (1)
43-46: DefaultingRememberMetotruechanges external login persistence behaviorWith
public bool RememberMe { get; init; } = true;, any client that omits this field will now get “remember me” semantics (longer‑lived refresh tokens) by default. That’s a meaningful behavioral and security change.If the intent is to make persistence an explicit opt‑in (and to align with the regular login flow, if that defaults to non‑persistent), consider defaulting to
false:- public bool RememberMe { get; init; } = true; + public bool RememberMe { get; init; } = false;Please confirm that the current default aligns with your product/security requirements.
src/Application/Common/Exceptions/SecurityException.cs (1)
1-16: CustomSecurityExceptionimplementation is straightforward and appropriateThe sealed
SecurityExceptiontype with standard message/inner‑exception constructors is correct and integrates cleanly with the middleware mapping.Note: its name overlaps with
System.Security.SecurityException, so if you later need both types in the same file you may want to use fully qualified names or a using alias, but that’s not a blocker here.src/Web.Api/Controllers/V1/AuthController.cs (2)
20-47: Align XML docs with current token/registration behaviorThe updated login XML comments describing access token (15 minutes) and refresh token (1/30 days) are helpful, but they now couple documentation to configuration. If these values are driven by configuration or
TokenConstants, make sure they stay in sync over time.Also, the registration endpoint docs still say “Registration result with user info and tokens”, but
RegisterResponseno longer includes tokens. It would be clearer to update the text, for example:- /// <returns>Registration result with user info and tokens</returns> + /// <returns>Registration result with user information</returns>This keeps the public contract documentation aligned with the current behavior.
120-142: Refresh endpoint behavior and response envelope consistencyThe new
refresh-access-tokenendpoint is wired correctly to MediatR and returns a successApiResponse<RefreshAccessTokenResponse>whenresult.IsSuccessis true. A couple of behavioral points to consider:
Envelope consistency
Other auth endpoints (login,login-external,register) return bare response types on 200 (e.g.,LoginResponse), whereas this action returns anApiResponse<RefreshAccessTokenResponse>. If clients consume these endpoints together, you may want a consistent envelope strategy (all wrapped or all bare) to avoid special‑casing this route.Error status codes
All non‑successful MediatR results here are mapped to400 Bad Request:return BadRequest(ApiResponse<object>.CreateFailure(result.Error!.Description));If the command handler differentiates error types (e.g., unauthorized vs validation vs conflict), you might want to mirror the pattern used in
Register/VerifyEmailand map them to more specific HTTP status codes, or ensure that truly unauthorized cases throwSecurityException/UnauthorizedExceptionso they go through the global middleware instead of this 400 path.Neither of these is a correctness bug, but aligning them can simplify client integration and make error semantics clearer.
src/Application/Features/Auth/Register/RegisterCommandHandler.cs (1)
17-21: Registration handler now correctly focuses on account creation onlyRemoving token generation from registration and returning a
RegisterResponsewithUserIdand basic profile/role data makes this handler single‑purpose and consistent with the new login/refresh flows. Just ensure API clients are updated to call the login endpoint after successful registration instead of expecting tokens here.Also applies to: 76-84
src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandHandler.cs (3)
18-28: Constructor and dependency usage look good, but unit-of-work use is asymmetricThe injected dependencies are validated correctly and stored as readonly fields. However,
IUnitOfWorkis only used in the revoked‑token branch, not on the successful refresh path. If your architectural pattern expects handlers to commit viaIUnitOfWork.SaveChangesAsync, consider either:
- consistently committing after updating/adding refresh tokens in the success path, or
- relying entirely on a pipeline/outer unit-of-work and removing the explicit
SaveChangesAsyncin the revoked branch.Right now the revoked path is committed explicitly while the normal path is not, which makes persistence semantics harder to reason about.
69-89: Refresh rotation flow is correct; consider explicit persistence and collision handling strategyThe rotation logic (generate new access + refresh, revoke old token with
TokenRotation, create newRefreshTokenentity with the same persistence policy) is aligned with best practices. Two follow‑ups worth considering:
- Persistence: similar to the earlier comment, confirm where
SaveChangesis invoked for this success path so revocation and the new token insert are guaranteed to be committed.- Uniqueness: the DB enforces a unique index on
Token. IfGenerateRefreshTokenever collides,AddAsyncwill fail. If this is a concern for you, you might want a retry loop onGenerateRefreshTokenwhen a unique‑constraint violation occurs.These are more about robustness than correctness of the current logic.
100-145: RevokeEntireTokenChainAsync is safe; minor redundancy with RevokeAllForUserAsyncThe chain traversal via
ReplacedByTokenplus theprocessedTokensguard handles potential cycles and correctly collects tokens to revoke. CallingRevokeAllForUserAsyncafterwards provides a strong security guarantee.This does mean that tokens in
tokensToRevokemay be affected twice, depending on repository implementation. IfRevokeAllForUserAsynconly targets still‑active tokens, this is fine; otherwise, you might be doing redundant work or overriding the more specificTokenChainAssassinationreason withSecurityBreachAllTokensRevoked. Not a blocker, but worth checking the repository behavior.src/Application/Features/Auth/Login/LoginCommandHandler.cs (1)
60-72: Refresh token persistence flow looks correct; align transaction strategy with refresh handlerThe sequence
- generate access token + expiry,
- generate refresh token value,
- create
RefreshTokenentity withisPersistent = request.RememberMe,AddAsyncviaIRefreshTokenRepository,is logically sound and matches the rotation semantics used in the refresh handler.
Just make sure your transaction / unit‑of‑work story is consistent with
RefreshAccessTokenCommandHandlerso that both login and refresh paths persist refresh tokens in the same way (either via a pipeline or explicitSaveChanges).src/Application/Interfaces/Repositories/IRefreshTokenRepository.cs (1)
8-44: Repository surface is appropriate; clarify “active” and cleanup semantics in implementationThe interface covers the needed operations for login/refresh flows and admin/cleanup tasks. When implementing:
- Define “active” in
GetActiveTokensByUserIdAsyncconsistently withRefreshToken.IsActive(not expired, not revoked, and possibly not soft‑deleted).- Ensure
CleanExpiredTokensAsyncclearly documents whether it physically deletes rows or just marks them as revoked/deleted, so callers know what to expect.No changes needed here, just implementation‑level clarity.
src/Infrastructure/Repositories/RefreshTokenRepository.cs (2)
32-37: Active token filter is strict on expiry and may have redundant conditionsThe active-token query:
.Where(rt => rt.UserId == userId && rt.RevokedAt == null && rt.ExpiresAt > DateTimeOffset.UtcNow && rt.RevokedReason == null)
RevokedReason == nullis likely redundant ifRevokealways sets bothRevokedAtandRevokedReason.- Using
>againstDateTimeOffset.UtcNowmeans a token withExpiresAt == nowis already treated as inactive; confirm that this matches how you validate tokens elsewhere.If these aren’t intentional invariants, you could simplify the predicate or align it with your validation logic.
64-76:CleanExpiredTokensAsyncsemantics and performanceTwo points to consider:
- The filter removes all revoked tokens, even if they have not yet expired:
.Where(rt => rt.ExpiresAt < DateTimeOffset.UtcNow || rt.RevokedAt != null)If you rely on revoked tokens for reuse detection or audit, you may want to either:
- Restrict cleanup to
ExpiresAt < now, or- Keep revoked-but-unexpired tokens for some retention window.
- For large token tables, materializing
expiredTokensinto memory and then callingRemoveRangecan be inefficient. EF Core 8 supportsExecuteDeleteAsync, which can do a set‑based delete in the database without loading entities.These aren’t correctness bugs but could be tightened for clearer intent and better scalability.
src/Infrastructure/Services/TokenService.cs (1)
27-37: Access‑token tuple API looks good; consider minor time/config polishThe new signature and expiry calculation align with the interface and refresh‑token flow. Two small improvements you might consider:
- Use
DateTimeOffset.UtcNowfor the expiry you return, to avoid implicitDateTime→DateTimeOffsetconversion.- Use
int.TryParse(with a sane default) forExpirationHoursto better handle misconfigured values.These are not blockers, just polish.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/Application/Common/Exceptions/SecurityException.cs(1 hunks)src/Application/Features/Auth/Login/LoginCommand.cs(2 hunks)src/Application/Features/Auth/Login/LoginCommandHandler.cs(4 hunks)src/Application/Features/Auth/LoginExternal/LoginExternalCommand.cs(1 hunks)src/Application/Features/Auth/LoginExternal/LoginExternalCommandHandler.cs(6 hunks)src/Application/Features/Auth/LoginExternal/LoginExternalResponse.cs(1 hunks)src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommand.cs(1 hunks)src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandHandler.cs(1 hunks)src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandValidator.cs(1 hunks)src/Application/Features/Auth/Register/RegisterCommandHandler.cs(2 hunks)src/Application/Features/Auth/Register/RegisterResponse.cs(1 hunks)src/Application/Interfaces/Repositories/IRefreshTokenRepository.cs(1 hunks)src/Application/Interfaces/Services/Auth/ITokenService.cs(1 hunks)src/Domain/Constants/TokenConstants.cs(1 hunks)src/Domain/Entities/RefreshToken.cs(1 hunks)src/Infrastructure/Data/Configurations/RefreshTokenConfiguration.cs(1 hunks)src/Infrastructure/Data/Contexts/DataContext.cs(1 hunks)src/Infrastructure/Data/Migrations/20251121160401_CreateRefreshTokenTable.Designer.cs(1 hunks)src/Infrastructure/Data/Migrations/20251121160401_CreateRefreshTokenTable.cs(1 hunks)src/Infrastructure/Data/Migrations/DataContextModelSnapshot.cs(2 hunks)src/Infrastructure/Extensions/ServiceCollectionExtensions.cs(1 hunks)src/Infrastructure/Repositories/RefreshTokenRepository.cs(1 hunks)src/Infrastructure/Services/TokenService.cs(2 hunks)src/Web.Api/Controllers/V1/AuthController.cs(3 hunks)src/Web.Api/Middleware/GlobalExceptionMiddleware.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (17)
src/Infrastructure/Extensions/ServiceCollectionExtensions.cs (1)
src/Infrastructure/Repositories/RefreshTokenRepository.cs (2)
RefreshTokenRepository(11-77)RefreshTokenRepository(15-18)
src/Infrastructure/Data/Contexts/DataContext.cs (2)
src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)
src/Application/Interfaces/Repositories/IRefreshTokenRepository.cs (3)
src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)src/Infrastructure/Repositories/RefreshTokenRepository.cs (1)
Update(39-42)
src/Domain/Constants/TokenConstants.cs (1)
src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)
src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandHandler.cs (5)
src/Application/Interfaces/Repositories/IRefreshTokenRepository.cs (6)
Task(13-13)Task(18-18)Task(23-23)Task(33-33)Task(38-38)Task(43-43)src/Domain/Constants/TokenConstants.cs (3)
RefreshToken(11-22)TokenConstants(6-54)RevocationReasons(27-53)src/Domain/Entities/RefreshToken.cs (3)
RefreshToken(8-82)RefreshToken(62-71)Revoke(76-81)src/Application/Common/Exceptions/SecurityException.cs (3)
SecurityException(7-16)SecurityException(9-11)SecurityException(13-15)src/Application/Interfaces/Services/Auth/ITokenService.cs (1)
GenerateRefreshToken(18-18)
src/Domain/Entities/RefreshToken.cs (1)
src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)
src/Web.Api/Middleware/GlobalExceptionMiddleware.cs (1)
src/Application/Common/Exceptions/SecurityException.cs (3)
SecurityException(7-16)SecurityException(9-11)SecurityException(13-15)
src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandValidator.cs (2)
src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)
src/Infrastructure/Data/Configurations/RefreshTokenConfiguration.cs (3)
src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)src/Infrastructure/Data/Contexts/Schemas.cs (1)
Schemas(3-7)
src/Application/Interfaces/Services/Auth/ITokenService.cs (1)
src/Infrastructure/Services/TokenService.cs (2)
accessToken(30-37)GenerateAccessToken(93-131)
src/Application/Features/Auth/LoginExternal/LoginExternalCommandHandler.cs (5)
src/Application/Interfaces/Repositories/IRefreshTokenRepository.cs (6)
Task(13-13)Task(18-18)Task(23-23)Task(33-33)Task(38-38)Task(43-43)src/Infrastructure/Repositories/RefreshTokenRepository.cs (4)
Task(20-23)Task(25-30)Task(32-37)Task(44-52)src/Application/Interfaces/Services/Auth/ITokenService.cs (3)
Task(23-23)accessToken(13-13)GenerateRefreshToken(18-18)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)src/Domain/Entities/User.cs (1)
User(8-42)
src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommand.cs (2)
src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)
src/Application/Features/Auth/Login/LoginCommandHandler.cs (4)
src/Domain/Constants/TokenConstants.cs (2)
TokenConstants(6-54)RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)src/Application/Interfaces/Services/Auth/ITokenService.cs (2)
accessToken(13-13)GenerateRefreshToken(18-18)src/Infrastructure/Services/TokenService.cs (3)
accessToken(30-37)GenerateAccessToken(93-131)GenerateRefreshToken(42-48)
src/Infrastructure/Data/Migrations/20251121160401_CreateRefreshTokenTable.Designer.cs (1)
src/Infrastructure/Data/Migrations/20251121160401_CreateRefreshTokenTable.cs (1)
CreateRefreshTokenTable(9-77)
src/Application/Features/Auth/Register/RegisterResponse.cs (1)
src/Web.Api/Services/CurrentUserService.cs (1)
List(58-90)
src/Web.Api/Controllers/V1/AuthController.cs (4)
src/Application/Interfaces/Services/Auth/ITokenService.cs (1)
Task(23-23)src/Infrastructure/Services/TokenService.cs (1)
Task(50-53)src/Domain/Constants/TokenConstants.cs (1)
RefreshToken(11-22)src/Domain/Entities/RefreshToken.cs (2)
RefreshToken(8-82)RefreshToken(62-71)
src/Infrastructure/Services/TokenService.cs (1)
src/Application/Interfaces/Services/Auth/ITokenService.cs (1)
accessToken(13-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (17)
src/Application/Features/Auth/Login/LoginCommand.cs (1)
33-51: Clear separation of access/refresh expirations inLoginResponseExposing both
AccessTokenExpiresAt(short‑lived) andRefreshTokenExpiresAt(RememberMe‑dependent) makes the contract explicit and should simplify client-side token management and UX around session duration.src/Infrastructure/Extensions/ServiceCollectionExtensions.cs (1)
82-95: Refresh token repository registration is correctly wired
IRefreshTokenRepositorymapped toRefreshTokenRepositoryas scoped aligns with theDataContextlifetime and should integrate cleanly with the new refresh-token workflows.src/Infrastructure/Data/Contexts/DataContext.cs (1)
29-35: ExposeRefreshTokensDbSet is correct and consistentAdding
DbSet<RefreshToken> RefreshTokens => Set<RefreshToken>();aligns with the existing pattern for domain entities and is required for EF Core configuration/migrations to work. No issues from a correctness or design standpoint.src/Web.Api/Middleware/GlobalExceptionMiddleware.cs (1)
102-110: SecurityException mapping to 401 with structured error looks goodHandling
SecurityExceptionas 401 Unauthorized with a typed error payload (Type = "SecurityError",Detailsfrom the exception) is consistent with the existing pattern for custom exceptions and will make client handling straightforward.src/Infrastructure/Data/Configurations/RefreshTokenConfiguration.cs (1)
13-46: RefreshToken EF configuration is well‑structuredTable mapping, key, property constraints, indexes, and the soft‑delete query filter all look appropriate for the refresh‑token use case (lookup by token, user‑scoped queries, and cleanup by expiry). The relationship to
Userwith cascade delete plus the filter on!rt.User.IsDeletedis consistent with a soft‑delete model.No issues from an EF Core or schema‑design standpoint.
src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommand.cs (1)
11-17: Well-structured command/response for refresh flowCommand and response contracts look solid: required init-only properties, optional rotated
RefreshToken, explicitAccessTokenExpiresAt/RefreshTokenExpiresAt, andUserInfopayload all align well with a secure refresh-token flow.Also applies to: 22-47
src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandValidator.cs (1)
10-17: Validator covers essential constraints on refresh tokenUsing
NotEmpty()plus a minimum length is an appropriate, lightweight guard for the refresh token input here; this should integrate cleanly with the CQRS pipeline.src/Application/Features/Auth/LoginExternal/LoginExternalCommandHandler.cs (1)
22-28: Good refactor: shared token generation + remember‑me supportConsolidating token generation into
GenerateTokenResponseand threadingrememberMethrough all three login paths keeps the external login flow consistent and makes the new refresh‑token behavior easy to maintain. The newIRefreshTokenRepositorydependency and use of the domainRefreshTokenentity also fit well with the existing UoW/repository pattern.Also applies to: 80-81, 113-114, 174-185
src/Infrastructure/Data/Migrations/20251121160401_CreateRefreshTokenTable.cs (1)
14-42: RefreshTokens table schema and indexes look appropriateThe table definition aligns with the
RefreshTokendomain model, and the chosen indexes (byExpiresAt, uniqueToken,UserId, and(UserId, RevokedAt)) should support common queries (lookup by token, active tokens by user, cleanup) efficiently.Also applies to: 44-68, 73-76
src/Infrastructure/Data/Migrations/DataContextModelSnapshot.cs (1)
150-205: Model snapshot aligns with RefreshToken schema and indexingThe RefreshToken model in the snapshot (including
IsPersistent, constrainedRevokedReason/Tokenlengths, and the added indexes onExpiresAt,Token(unique), and(UserId, RevokedAt)) is consistent with the new migration and supports the intended refresh‑token workflows.src/Application/Features/Auth/RefreshAccessToken/RefreshAccessTokenCommandHandler.cs (1)
30-54: Token validation logic is sound; just ensure the repository eagerly loads the User navigationThe sequence of checks (invalid → revoked → expired → missing user) is reasonable and uses appropriate exception types. One thing to verify:
storedRefreshToken.Useris assumed to be populated. EnsureIRefreshTokenRepository.GetByTokenAsyncincludes the relatedUser(and itsRoles) to avoid null navigation access at this point.src/Application/Features/Auth/Login/LoginCommandHandler.cs (2)
40-44: Refresh token expiry calculation is consistent and clearUsing
TokenConstants.RefreshToken.NormalExpirationvsRememberMeExpirationbased onrequest.RememberMeis straightforward and aligns with the domain constants. This keeps the expiry policy centralized and easy to adjust.
73-80: Updated response surface is coherent with new token modelReturning
AccessToken,RefreshToken, and both expiration timestamps alongsideUsermakes the client contract explicit and aligns with the new token lifecycle. No issues here.src/Domain/Entities/RefreshToken.cs (1)
8-81: Entity design matches the intended refresh‑token lifecycleThe properties and helpers (
Create,Revoke,IsExpired,IsActive,IsPersistent,ReplacedByToken) provide exactly what the application layer needs for rotation and chain revocation. UsingreplacedByToken ?? TokeninRevokealso keeps the chain well‑defined even when there is no successor token.No issues from a domain‑model standpoint.
src/Infrastructure/Data/Migrations/20251121160401_CreateRefreshTokenTable.Designer.cs (1)
153-207: RefreshTokens mapping and indexes align with the domain modelThe EF model for
Domain.Entities.RefreshToken(columns plus indexes onToken,ExpiresAt,UserId, and(UserId, RevokedAt)) matches the domain entity and the Up migration. This should support efficient lookup by token and user for the flows introduced in this PR.src/Infrastructure/Repositories/RefreshTokenRepository.cs (1)
20-23: Repository write methods rely on outer SaveChanges – verify call sites
AddAsync,Update,RevokeAsync,RevokeAllForUserAsync, andCleanExpiredTokensAsyncall mutate the DbContext but never callSaveChanges/SaveChangesAsync. That’s fine with a unit-of-work pattern, but if any caller (especially background jobs runningCleanExpiredTokensAsync) forgets to commit, changes will be silently discarded. Please double‑check that every code path using these methods always persists the context afterward.Also applies to: 39-42, 44-62, 64-76
src/Infrastructure/Services/TokenService.cs (1)
50-53:GetClaimsFromTokenAsyncis incomplete but not a current blocker for auth flowsVerification shows that while
GetClaimsFromTokenAsyncthrowsNotImplementedException, it is not currently called by any active code path. The refresh-token flow inRefreshAccessTokenCommandHandlerand login flows inLoginCommandHandleronly callGenerateAccessToken()andGenerateRefreshToken(). User claims are loaded from stored refresh-token entities, not from JWT claims extraction. Since no code currently depends on this method, it does not block existing auth workflows.However, implementing the method is still sensible for interface completeness. The
ValidateTokenmethod exists and returns the requiredClaimsPrincipal?, supporting the suggested implementation approach.
…me-and-refresh-token feat: Implement refresh token functionality and enhance authentication
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.