From 8579504a6c6c1e817856b400db0316f5211668cc Mon Sep 17 00:00:00 2001 From: Thomas Luizon Rodrigues Gregorio Date: Sun, 12 Jul 2026 07:25:17 -0300 Subject: [PATCH 1/4] fix(api): harden auth/session surface (#243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four targeted hardenings on the refresh/session flow, none regressing the #329 double-rotation guard (its concurrency tests still pass): - Account-wide revocation: add IAuthSessionService.RevokeAllSessionsAsync, which revokes every active session for a user (for password/security events), wired to a new authenticated POST /api/auth/logout-all endpoint. Single-session logout is unchanged. - Pre-rotation validation: UserSession.Rotate now returns a Result and guards that the session is still usable (not revoked/expired) and the new hash is non-empty before mutating, so a rotation can never land on an inactive session. This complements the xmin concurrency token from #329 (DB-level loser rejection) with an explicit in-memory invariant. - Refresh-token format validation: RefreshSessionCommandValidator (and the logout validator, which takes the same token) now enforce the exact server-issued shape — 128 uppercase-hex chars — via a shared RefreshTokenRules, rejecting malformed tokens with a 400 before any lookup. - Refresh rate limit: the refresh endpoints move to a dedicated `refresh` policy partitioned by a SHA-256 hash of the refresh token (a stable per-session identity) instead of the caller IP, so a stolen/targeted token cannot be replayed faster by rotating source IPs. Falls back to IP when no token is present. The key is hashed so logs never carry the raw secret. Intelligent tests for each: domain Rotate guards, RevokeAllSessions scoping (only the target user's active sessions), validator format/length/case edges, and refresh-token partition-key resolution + filter wiring. The new endpoint is mapped in the agent catalog (AuthManage) and openapi.json regenerated. orbit-api-only, additive/append-only contract; no client change required. Refs thomasluizon/orbit-ui-mobile#243 Co-Authored-By: Claude Opus 4.8 --- src/Orbit.Api/Controllers/AuthController.cs | 27 ++++- .../DistributedRateLimitAttribute.cs | 52 +++++++++ src/Orbit.Api/openapi.json | 32 ++++++ .../Auth/Commands/LogoutAllSessionsCommand.cs | 16 +++ .../LogoutAllSessionsCommandValidator.cs | 13 +++ .../LogoutSessionCommandValidator.cs | 3 +- .../RefreshSessionCommandValidator.cs | 3 +- .../Auth/Validators/RefreshTokenRules.cs | 18 ++++ src/Orbit.Domain/Common/DomainErrors.cs | 1 + src/Orbit.Domain/Entities/UserSession.cs | 9 +- .../Interfaces/IAuthSessionService.cs | 1 + .../AgentCatalogService.Capabilities.cs | 1 + .../Services/AuthSessionService.cs | 26 ++++- .../Services/DistributedRateLimitService.cs | 1 + .../LogoutAllSessionsCommandHandlerTests.cs | 44 ++++++++ .../LogoutAllSessionsCommandValidatorTests.cs | 26 +++++ .../LogoutSessionCommandValidatorTests.cs | 18 +++- .../RefreshSessionCommandValidatorTests.cs | 36 ++++++- .../Entities/UserSessionTests.cs | 72 +++++++++++++ .../DistributedRateLimitFilterTests.cs | 28 ++++- .../DistributedRateLimitPartitionKeyTests.cs | 70 ++++++++++++ .../AuthSessionServiceRevokeAllTests.cs | 102 ++++++++++++++++++ 22 files changed, 581 insertions(+), 18 deletions(-) create mode 100644 src/Orbit.Application/Auth/Commands/LogoutAllSessionsCommand.cs create mode 100644 src/Orbit.Application/Auth/Validators/LogoutAllSessionsCommandValidator.cs create mode 100644 src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs create mode 100644 tests/Orbit.Application.Tests/Commands/Auth/LogoutAllSessionsCommandHandlerTests.cs create mode 100644 tests/Orbit.Application.Tests/Validators/LogoutAllSessionsCommandValidatorTests.cs create mode 100644 tests/Orbit.Domain.Tests/Entities/UserSessionTests.cs create mode 100644 tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs diff --git a/src/Orbit.Api/Controllers/AuthController.cs b/src/Orbit.Api/Controllers/AuthController.cs index d28547e5..8e723919 100644 --- a/src/Orbit.Api/Controllers/AuthController.cs +++ b/src/Orbit.Api/Controllers/AuthController.cs @@ -204,7 +204,7 @@ await RecordDirectAuthAuditAsync( } [HttpPost("refresh")] - [DistributedRateLimit("auth")] + [DistributedRateLimit("refresh")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] public async Task Refresh( @@ -225,7 +225,7 @@ public async Task Refresh( } [HttpPost("operations/refresh")] - [DistributedRateLimit("auth")] + [DistributedRateLimit("refresh")] [AllowAnonymous] public async Task RefreshOperation( [FromBody] RefreshSessionOperationRequest request, @@ -306,6 +306,26 @@ await RecordDirectAuthAuditAsync( policyReason: result.Error)); } + [Authorize] + [HttpPost("logout-all")] + [DistributedRateLimit("auth")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + public async Task LogoutAll(CancellationToken cancellationToken) + { + var command = new LogoutAllSessionsCommand(HttpContext.GetUserId()); + var result = await mediator.Send(command, cancellationToken); + + if (result.IsSuccess) + { + LogAllSessionsRevoked(logger, HttpContext.GetUserId(), HttpContext.GetRequestId()); + return Ok(new { message = "Logged out of all sessions" }); + } + + LogSessionRevocationFailed(logger, result.Error, HttpContext.GetRequestId()); + return result.ToErrorResult(); + } + [Authorize] [HttpPost("request-deletion")] [DistributedRateLimit("auth")] @@ -392,6 +412,9 @@ public async Task ConfirmDeletion( [LoggerMessage(EventId = 14, Level = LogLevel.Warning, Message = "Session revocation failed: {Error}. RequestId={RequestId}")] private static partial void LogSessionRevocationFailed(ILogger logger, string? error, string requestId); + [LoggerMessage(EventId = 15, Level = LogLevel.Information, Message = "All sessions revoked for {UserId}. RequestId={RequestId}")] + private static partial void LogAllSessionsRevoked(ILogger logger, Guid userId, string requestId); + private static AgentExecuteOperationResponse BuildOperationResponse( string operationId, AgentOperationStatus status, diff --git a/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs b/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs index 5ecef635..b6080b96 100644 --- a/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs +++ b/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs @@ -1,4 +1,6 @@ using System.Reflection; +using System.Security.Cryptography; +using System.Text; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Orbit.Api.Extensions; @@ -102,6 +104,9 @@ private static string ResolvePartitionKey(string policyName, HttpContext context if (TryResolveEmailPartitionKey(policyName, actionArguments, out var emailPartitionKey)) return emailPartitionKey; + if (TryResolveRefreshTokenPartitionKey(policyName, actionArguments, out var refreshPartitionKey)) + return refreshPartitionKey; + return $"ip:{context.GetClientIpAddress() ?? "unknown"}"; } @@ -147,6 +152,53 @@ public static bool TryResolveEmailPartitionKey( return false; } + private static readonly HashSet RefreshTokenPartitionedPolicies = + new(StringComparer.OrdinalIgnoreCase) { "refresh" }; + + /// + /// For unauthenticated requests under the refresh policy, partitions by a SHA-256 hash of the + /// request's refresh token instead of the caller IP. A refresh token uniquely identifies one user + /// session, so hashing it yields a stable per-session bucket that a stolen or targeted token cannot + /// escape by rotating source IPs — closing the cross-IP brute-force/replay gap that IP partitioning + /// leaves open. The token is hashed so the partition key (which is logged) never carries the raw + /// secret. Returns false when the policy isn't refresh partitioned or no non-blank refresh token is + /// present, so the caller falls back to IP partitioning. + /// + public static bool TryResolveRefreshTokenPartitionKey( + string policyName, + IEnumerable actionArguments, + out string partitionKey) + { + partitionKey = string.Empty; + + if (!RefreshTokenPartitionedPolicies.Contains(policyName)) + return false; + + foreach (var argument in actionArguments) + { + if (argument is null) + continue; + + var tokenProperty = argument.GetType().GetProperty( + "RefreshToken", + BindingFlags.Public | BindingFlags.Instance); + + if (tokenProperty?.PropertyType != typeof(string)) + continue; + + if (tokenProperty.GetValue(argument) is not string rawToken || string.IsNullOrWhiteSpace(rawToken)) + continue; + + partitionKey = $"{policyName.ToLowerInvariant()}:token:{HashRefreshToken(rawToken)}"; + return true; + } + + return false; + } + + private static string HashRefreshToken(string refreshToken) => + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(refreshToken))); + [LoggerMessage( EventId = 1, Level = LogLevel.Warning, diff --git a/src/Orbit.Api/openapi.json b/src/Orbit.Api/openapi.json index 926afd88..353857c2 100644 --- a/src/Orbit.Api/openapi.json +++ b/src/Orbit.Api/openapi.json @@ -1652,6 +1652,38 @@ } } }, + "/api/Auth/logout-all": { + "post": { + "tags": [ + "Auth" + ], + "responses": { + "200": { + "description": "OK" + }, + "401": { + "description": "Unauthorized", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/Auth/request-deletion": { "post": { "tags": [ diff --git a/src/Orbit.Application/Auth/Commands/LogoutAllSessionsCommand.cs b/src/Orbit.Application/Auth/Commands/LogoutAllSessionsCommand.cs new file mode 100644 index 00000000..8c9fa828 --- /dev/null +++ b/src/Orbit.Application/Auth/Commands/LogoutAllSessionsCommand.cs @@ -0,0 +1,16 @@ +using MediatR; +using Orbit.Domain.Common; +using Orbit.Domain.Interfaces; + +namespace Orbit.Application.Auth.Commands; + +public record LogoutAllSessionsCommand(Guid UserId) : IRequest; + +public class LogoutAllSessionsCommandHandler(IAuthSessionService authSessionService) + : IRequestHandler +{ + public Task Handle(LogoutAllSessionsCommand request, CancellationToken cancellationToken) + { + return authSessionService.RevokeAllSessionsAsync(request.UserId, cancellationToken); + } +} diff --git a/src/Orbit.Application/Auth/Validators/LogoutAllSessionsCommandValidator.cs b/src/Orbit.Application/Auth/Validators/LogoutAllSessionsCommandValidator.cs new file mode 100644 index 00000000..849e6002 --- /dev/null +++ b/src/Orbit.Application/Auth/Validators/LogoutAllSessionsCommandValidator.cs @@ -0,0 +1,13 @@ +using FluentValidation; +using Orbit.Application.Auth.Commands; + +namespace Orbit.Application.Auth.Validators; + +public class LogoutAllSessionsCommandValidator : AbstractValidator +{ + public LogoutAllSessionsCommandValidator() + { + RuleFor(x => x.UserId) + .NotEmpty(); + } +} diff --git a/src/Orbit.Application/Auth/Validators/LogoutSessionCommandValidator.cs b/src/Orbit.Application/Auth/Validators/LogoutSessionCommandValidator.cs index df2a3621..02ccffba 100644 --- a/src/Orbit.Application/Auth/Validators/LogoutSessionCommandValidator.cs +++ b/src/Orbit.Application/Auth/Validators/LogoutSessionCommandValidator.cs @@ -7,7 +7,6 @@ public class LogoutSessionCommandValidator : AbstractValidator x.RefreshToken) - .NotEmpty(); + RefreshTokenRules.AddRefreshTokenRules(RuleFor(x => x.RefreshToken)); } } diff --git a/src/Orbit.Application/Auth/Validators/RefreshSessionCommandValidator.cs b/src/Orbit.Application/Auth/Validators/RefreshSessionCommandValidator.cs index b8056f7d..403c49e4 100644 --- a/src/Orbit.Application/Auth/Validators/RefreshSessionCommandValidator.cs +++ b/src/Orbit.Application/Auth/Validators/RefreshSessionCommandValidator.cs @@ -7,7 +7,6 @@ public class RefreshSessionCommandValidator : AbstractValidator x.RefreshToken) - .NotEmpty(); + RefreshTokenRules.AddRefreshTokenRules(RuleFor(x => x.RefreshToken)); } } diff --git a/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs b/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs new file mode 100644 index 00000000..b011b9cc --- /dev/null +++ b/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs @@ -0,0 +1,18 @@ +using FluentValidation; + +namespace Orbit.Application.Auth.Validators; + +public static class RefreshTokenRules +{ + public const int TokenLength = 128; + public const string TokenPattern = "^[0-9A-F]+$"; + + public static void AddRefreshTokenRules(IRuleBuilder rule) + { + rule + .NotEmpty() + .Length(TokenLength) + .Matches(TokenPattern) + .WithMessage("Refresh token format is invalid."); + } +} diff --git a/src/Orbit.Domain/Common/DomainErrors.cs b/src/Orbit.Domain/Common/DomainErrors.cs index 2e38e335..3a2f0be7 100644 --- a/src/Orbit.Domain/Common/DomainErrors.cs +++ b/src/Orbit.Domain/Common/DomainErrors.cs @@ -8,6 +8,7 @@ public static class DomainErrors { public static readonly AppError UserIdRequired = new("USER_ID_REQUIRED", "User ID is required."); public static readonly AppError TokenHashRequired = new("TOKEN_HASH_REQUIRED", "Token hash is required."); + public static readonly AppError SessionNotActive = new("SESSION_NOT_ACTIVE", "Session is no longer active."); public static readonly AppError NameRequired = new("NAME_REQUIRED", "Name is required"); public static readonly AppError InvalidHandle = new("INVALID_HANDLE", "Handle must be 3-20 characters using only letters, numbers, or underscores."); diff --git a/src/Orbit.Domain/Entities/UserSession.cs b/src/Orbit.Domain/Entities/UserSession.cs index f361fb63..a8d5a09b 100644 --- a/src/Orbit.Domain/Entities/UserSession.cs +++ b/src/Orbit.Domain/Entities/UserSession.cs @@ -35,11 +35,18 @@ public bool CanUse(DateTime nowUtc) => RevokedAtUtc is null && (!ExpiresAtUtc.HasValue || ExpiresAtUtc.Value > nowUtc); - public void Rotate(string newTokenHash, DateTime? newExpiresAtUtc, DateTime usedAtUtc) + public Result Rotate(string newTokenHash, DateTime? newExpiresAtUtc, DateTime usedAtUtc) { + if (string.IsNullOrWhiteSpace(newTokenHash)) + return Result.Failure(DomainErrors.TokenHashRequired); + + if (!CanUse(usedAtUtc)) + return Result.Failure(DomainErrors.SessionNotActive); + TokenHash = newTokenHash; ExpiresAtUtc = newExpiresAtUtc; LastUsedAtUtc = usedAtUtc; + return Result.Success(); } public void Revoke(DateTime revokedAtUtc) diff --git a/src/Orbit.Domain/Interfaces/IAuthSessionService.cs b/src/Orbit.Domain/Interfaces/IAuthSessionService.cs index 8f923ab4..673a9f75 100644 --- a/src/Orbit.Domain/Interfaces/IAuthSessionService.cs +++ b/src/Orbit.Domain/Interfaces/IAuthSessionService.cs @@ -8,4 +8,5 @@ public interface IAuthSessionService Task> CreateSessionAsync(Guid userId, string email, CancellationToken cancellationToken = default); Task> RefreshSessionAsync(string refreshToken, CancellationToken cancellationToken = default); Task RevokeSessionAsync(string refreshToken, CancellationToken cancellationToken = default); + Task RevokeAllSessionsAsync(Guid userId, CancellationToken cancellationToken = default); } diff --git a/src/Orbit.Infrastructure/Services/AgentCatalogService.Capabilities.cs b/src/Orbit.Infrastructure/Services/AgentCatalogService.Capabilities.cs index 8e6fba1e..4ca0a6b6 100644 --- a/src/Orbit.Infrastructure/Services/AgentCatalogService.Capabilities.cs +++ b/src/Orbit.Infrastructure/Services/AgentCatalogService.Capabilities.cs @@ -1004,6 +1004,7 @@ private static AgentCapability[] AccountAndAuthCapabilities() "AuthController.RefreshOperation", "AuthController.Logout", "AuthController.LogoutOperation", + "AuthController.LogoutAll", "OAuthController.GetMetadata", "OAuthController.Register", "OAuthController.GetProtectedResourceMetadata", diff --git a/src/Orbit.Infrastructure/Services/AuthSessionService.cs b/src/Orbit.Infrastructure/Services/AuthSessionService.cs index 2700f8ac..7560bf34 100644 --- a/src/Orbit.Infrastructure/Services/AuthSessionService.cs +++ b/src/Orbit.Infrastructure/Services/AuthSessionService.cs @@ -62,11 +62,14 @@ public async Task> RefreshSessionAsync(string refreshToken return Result.Failure(ErrorMessages.InvalidSession); var newRefreshToken = GenerateRefreshToken(); - session.Rotate( + var rotateResult = session.Rotate( HashToken(newRefreshToken), GetRefreshExpiry(nowUtc), nowUtc); + if (rotateResult.IsFailure) + return Result.Failure(ErrorMessages.InvalidSession); + try { await unitOfWork.SaveChangesAsync(cancellationToken); @@ -98,6 +101,27 @@ public async Task RevokeSessionAsync(string refreshToken, CancellationTo return Result.Success(); } + public async Task RevokeAllSessionsAsync(Guid userId, CancellationToken cancellationToken = default) + { + if (userId == Guid.Empty) + return Result.Failure(DomainErrors.UserIdRequired); + + var nowUtc = DateTime.UtcNow; + var activeSessions = await userSessionRepository.FindTrackedAsync( + s => s.UserId == userId && s.RevokedAtUtc == null, + cancellationToken); + + if (activeSessions.Count == 0) + return Result.Success(); + + foreach (var session in activeSessions) + session.Revoke(nowUtc); + + await unitOfWork.SaveChangesAsync(cancellationToken); + + return Result.Success(); + } + private static string GenerateRefreshToken() { return Convert.ToHexString(RandomNumberGenerator.GetBytes(64)); diff --git a/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs b/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs index 15a383c3..a8f4952f 100644 --- a/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs +++ b/src/Orbit.Infrastructure/Services/DistributedRateLimitService.cs @@ -14,6 +14,7 @@ public class DistributedRateLimitService(OrbitDbContext dbContext, TimeProvider new Dictionary(StringComparer.OrdinalIgnoreCase) { ["auth"] = new(TimeSpan.FromMinutes(1), PermitLimit: 10, SegmentCount: 1), + ["refresh"] = new(TimeSpan.FromMinutes(1), PermitLimit: 10, SegmentCount: 1), ["waitlist"] = new(TimeSpan.FromMinutes(10), PermitLimit: 5, SegmentCount: 1), ["chat"] = new(TimeSpan.FromMinutes(1), PermitLimit: 20, SegmentCount: 4), ["ai-resolve"] = new(TimeSpan.FromMinutes(1), PermitLimit: 30, SegmentCount: 4), diff --git a/tests/Orbit.Application.Tests/Commands/Auth/LogoutAllSessionsCommandHandlerTests.cs b/tests/Orbit.Application.Tests/Commands/Auth/LogoutAllSessionsCommandHandlerTests.cs new file mode 100644 index 00000000..d3b9bd15 --- /dev/null +++ b/tests/Orbit.Application.Tests/Commands/Auth/LogoutAllSessionsCommandHandlerTests.cs @@ -0,0 +1,44 @@ +using FluentAssertions; +using NSubstitute; +using Orbit.Application.Auth.Commands; +using Orbit.Domain.Common; +using Orbit.Domain.Interfaces; + +namespace Orbit.Application.Tests.Commands.Auth; + +public class LogoutAllSessionsCommandHandlerTests +{ + private readonly IAuthSessionService _authSessionService = Substitute.For(); + private readonly LogoutAllSessionsCommandHandler _handler; + + public LogoutAllSessionsCommandHandlerTests() + { + _handler = new LogoutAllSessionsCommandHandler(_authSessionService); + } + + [Fact] + public async Task Handle_RevokesEverySessionForTheUser() + { + var userId = Guid.NewGuid(); + _authSessionService.RevokeAllSessionsAsync(userId, Arg.Any()) + .Returns(Result.Success()); + + var result = await _handler.Handle(new LogoutAllSessionsCommand(userId), CancellationToken.None); + + result.IsSuccess.Should().BeTrue(); + await _authSessionService.Received(1).RevokeAllSessionsAsync(userId, Arg.Any()); + } + + [Fact] + public async Task Handle_PropagatesFailure() + { + var userId = Guid.NewGuid(); + _authSessionService.RevokeAllSessionsAsync(userId, Arg.Any()) + .Returns(Result.Failure("User ID is required.", "USER_ID_REQUIRED")); + + var result = await _handler.Handle(new LogoutAllSessionsCommand(userId), CancellationToken.None); + + result.IsFailure.Should().BeTrue(); + result.ErrorCode.Should().Be("USER_ID_REQUIRED"); + } +} diff --git a/tests/Orbit.Application.Tests/Validators/LogoutAllSessionsCommandValidatorTests.cs b/tests/Orbit.Application.Tests/Validators/LogoutAllSessionsCommandValidatorTests.cs new file mode 100644 index 00000000..1af46b7f --- /dev/null +++ b/tests/Orbit.Application.Tests/Validators/LogoutAllSessionsCommandValidatorTests.cs @@ -0,0 +1,26 @@ +using FluentAssertions; +using Orbit.Application.Auth.Commands; +using Orbit.Application.Auth.Validators; + +namespace Orbit.Application.Tests.Validators; + +public class LogoutAllSessionsCommandValidatorTests +{ + private readonly LogoutAllSessionsCommandValidator _validator = new(); + + [Fact] + public void Validate_WithUserId_Passes() + { + var result = _validator.Validate(new LogoutAllSessionsCommand(Guid.NewGuid())); + result.IsValid.Should().BeTrue(); + } + + [Fact] + public void Validate_EmptyUserId_Fails() + { + var result = _validator.Validate(new LogoutAllSessionsCommand(Guid.Empty)); + result.IsValid.Should().BeFalse(); + result.Errors.Should().ContainSingle() + .Which.PropertyName.Should().Be("UserId"); + } +} diff --git a/tests/Orbit.Application.Tests/Validators/LogoutSessionCommandValidatorTests.cs b/tests/Orbit.Application.Tests/Validators/LogoutSessionCommandValidatorTests.cs index 561ec2dc..5b360f7b 100644 --- a/tests/Orbit.Application.Tests/Validators/LogoutSessionCommandValidatorTests.cs +++ b/tests/Orbit.Application.Tests/Validators/LogoutSessionCommandValidatorTests.cs @@ -1,3 +1,4 @@ +using System.Security.Cryptography; using FluentAssertions; using Orbit.Application.Auth.Commands; using Orbit.Application.Auth.Validators; @@ -8,10 +9,12 @@ public class LogoutSessionCommandValidatorTests { private readonly LogoutSessionCommandValidator _validator = new(); + private static string ValidToken() => Convert.ToHexString(RandomNumberGenerator.GetBytes(64)); + [Fact] - public void Validate_ValidRefreshToken_Passes() + public void Validate_ServerIssuedToken_Passes() { - var result = _validator.Validate(new LogoutSessionCommand("valid-token")); + var result = _validator.Validate(new LogoutSessionCommand(ValidToken())); result.IsValid.Should().BeTrue(); } @@ -23,7 +26,14 @@ public void Validate_EmptyRefreshToken_Fails(string? token) { var result = _validator.Validate(new LogoutSessionCommand(token!)); result.IsValid.Should().BeFalse(); - result.Errors.Should().ContainSingle() - .Which.PropertyName.Should().Be("RefreshToken"); + result.Errors.Should().OnlyContain(failure => failure.PropertyName == "RefreshToken"); + } + + [Fact] + public void Validate_MalformedToken_Fails() + { + var nonHex = new string('Z', RefreshTokenRules.TokenLength); + var result = _validator.Validate(new LogoutSessionCommand(nonHex)); + result.IsValid.Should().BeFalse(); } } diff --git a/tests/Orbit.Application.Tests/Validators/RefreshSessionCommandValidatorTests.cs b/tests/Orbit.Application.Tests/Validators/RefreshSessionCommandValidatorTests.cs index 8b38d1df..e614db5f 100644 --- a/tests/Orbit.Application.Tests/Validators/RefreshSessionCommandValidatorTests.cs +++ b/tests/Orbit.Application.Tests/Validators/RefreshSessionCommandValidatorTests.cs @@ -1,3 +1,4 @@ +using System.Security.Cryptography; using FluentAssertions; using Orbit.Application.Auth.Commands; using Orbit.Application.Auth.Validators; @@ -8,10 +9,12 @@ public class RefreshSessionCommandValidatorTests { private readonly RefreshSessionCommandValidator _validator = new(); + private static string ValidToken() => Convert.ToHexString(RandomNumberGenerator.GetBytes(64)); + [Fact] - public void Validate_ValidRefreshToken_Passes() + public void Validate_ServerIssuedToken_Passes() { - var result = _validator.Validate(new RefreshSessionCommand("valid-token")); + var result = _validator.Validate(new RefreshSessionCommand(ValidToken())); result.IsValid.Should().BeTrue(); } @@ -23,7 +26,32 @@ public void Validate_EmptyRefreshToken_Fails(string? token) { var result = _validator.Validate(new RefreshSessionCommand(token!)); result.IsValid.Should().BeFalse(); - result.Errors.Should().ContainSingle() - .Which.PropertyName.Should().Be("RefreshToken"); + result.Errors.Should().OnlyContain(failure => failure.PropertyName == "RefreshToken"); + } + + [Fact] + public void Validate_WrongLengthToken_Fails() + { + var tooShort = new string('A', RefreshTokenRules.TokenLength - 1); + var tooLong = new string('A', RefreshTokenRules.TokenLength + 1); + + _validator.Validate(new RefreshSessionCommand(tooShort)).IsValid.Should().BeFalse(); + _validator.Validate(new RefreshSessionCommand(tooLong)).IsValid.Should().BeFalse(); + } + + [Fact] + public void Validate_NonHexToken_Fails() + { + var nonHex = new string('Z', RefreshTokenRules.TokenLength); + var result = _validator.Validate(new RefreshSessionCommand(nonHex)); + result.IsValid.Should().BeFalse(); + } + + [Fact] + public void Validate_LowercaseHexToken_Fails() + { + var lowercase = ValidToken().ToLowerInvariant(); + var result = _validator.Validate(new RefreshSessionCommand(lowercase)); + result.IsValid.Should().BeFalse(); } } diff --git a/tests/Orbit.Domain.Tests/Entities/UserSessionTests.cs b/tests/Orbit.Domain.Tests/Entities/UserSessionTests.cs new file mode 100644 index 00000000..154b7b18 --- /dev/null +++ b/tests/Orbit.Domain.Tests/Entities/UserSessionTests.cs @@ -0,0 +1,72 @@ +using FluentAssertions; +using Orbit.Domain.Entities; + +namespace Orbit.Domain.Tests.Entities; + +public class UserSessionTests +{ + [Fact] + public void Rotate_ActiveSession_UpdatesTokenExpiryAndLastUsed() + { + var session = UserSession.Create(Guid.NewGuid(), "old-hash", DateTime.UtcNow.AddDays(1)).Value; + var rotatedAt = DateTime.UtcNow.AddMinutes(5); + var newExpiry = DateTime.UtcNow.AddDays(30); + + var result = session.Rotate("new-hash", newExpiry, rotatedAt); + + result.IsSuccess.Should().BeTrue(); + session.TokenHash.Should().Be("new-hash"); + session.ExpiresAtUtc.Should().Be(newExpiry); + session.LastUsedAtUtc.Should().Be(rotatedAt); + } + + [Fact] + public void Rotate_NonExpiringSession_Succeeds() + { + var session = UserSession.Create(Guid.NewGuid(), "old-hash", null).Value; + + var result = session.Rotate("new-hash", null, DateTime.UtcNow); + + result.IsSuccess.Should().BeTrue(); + session.TokenHash.Should().Be("new-hash"); + } + + [Fact] + public void Rotate_RevokedSession_FailsAndLeavesTokenUntouched() + { + var session = UserSession.Create(Guid.NewGuid(), "old-hash", DateTime.UtcNow.AddDays(1)).Value; + session.Revoke(DateTime.UtcNow); + + var result = session.Rotate("new-hash", DateTime.UtcNow.AddDays(30), DateTime.UtcNow); + + result.IsFailure.Should().BeTrue(); + result.ErrorCode.Should().Be("SESSION_NOT_ACTIVE"); + session.TokenHash.Should().Be("old-hash"); + } + + [Fact] + public void Rotate_ExpiredSession_Fails() + { + var session = UserSession.Create(Guid.NewGuid(), "old-hash", DateTime.UtcNow.AddDays(-1)).Value; + + var result = session.Rotate("new-hash", DateTime.UtcNow.AddDays(30), DateTime.UtcNow); + + result.IsFailure.Should().BeTrue(); + result.ErrorCode.Should().Be("SESSION_NOT_ACTIVE"); + session.TokenHash.Should().Be("old-hash"); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void Rotate_BlankNewTokenHash_Fails(string newTokenHash) + { + var session = UserSession.Create(Guid.NewGuid(), "old-hash", DateTime.UtcNow.AddDays(1)).Value; + + var result = session.Rotate(newTokenHash, DateTime.UtcNow.AddDays(30), DateTime.UtcNow); + + result.IsFailure.Should().BeTrue(); + result.ErrorCode.Should().Be("TOKEN_HASH_REQUIRED"); + session.TokenHash.Should().Be("old-hash"); + } +} diff --git a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs index ccab30b5..1b30cd6e 100644 --- a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs +++ b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Logging; using NSubstitute; using NSubstitute.ExceptionExtensions; +using Orbit.Api.Controllers; using Orbit.Api.RateLimiting; using Orbit.Domain.Interfaces; using Orbit.Domain.Models; @@ -78,7 +79,26 @@ await filter.OnActionExecutionAsync(context, () => context.Result.Should().NotBeNull(); } - private static (ActionExecutingContext Context, HttpContext HttpContext) CreateExecutingContext() + [Fact] + public async Task RefreshPolicy_PartitionsUnauthenticatedRequestByRefreshTokenNotIp() + { + const string refreshToken = + "AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555"; + string? capturedPartitionKey = null; + _service.TryAcquireAsync("refresh", Arg.Do(key => capturedPartitionKey = key), Arg.Any()) + .Returns(new DistributedRateLimitDecision(true, 1, 10, DateTime.UtcNow.AddMinutes(1))); + + var filter = new DistributedRateLimitFilter("refresh", _service, _logger); + var (context, _) = CreateExecutingContext(new AuthController.RefreshSessionRequest(refreshToken)); + + await filter.OnActionExecutionAsync(context, () => Task.FromResult(CreateExecutedContext(context))); + + capturedPartitionKey.Should().StartWith("refresh:token:"); + capturedPartitionKey.Should().NotContain(refreshToken); + } + + private static (ActionExecutingContext Context, HttpContext HttpContext) CreateExecutingContext( + params object?[] actionArguments) { var httpContext = new DefaultHttpContext(); httpContext.Request.Method = "POST"; @@ -89,10 +109,14 @@ private static (ActionExecutingContext Context, HttpContext HttpContext) CreateE new RouteData(), new ControllerActionDescriptor()); + var arguments = new Dictionary(); + for (var index = 0; index < actionArguments.Length; index++) + arguments[$"arg{index}"] = actionArguments[index]; + var executingContext = new ActionExecutingContext( actionContext, [], - new Dictionary(), + arguments, controller: new object()); return (executingContext, httpContext); diff --git a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs index 80065c1d..24d38a84 100644 --- a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs +++ b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs @@ -74,6 +74,66 @@ [new AuthController.SendCodeRequest("a@x.com")], partitionKey.Should().BeEmpty(); } + private const string RefreshTokenA = + "AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555"; + + private const string RefreshTokenB = + "1111AAAA2222BBBB3333CCCC4444DDDD5555EEEE6666FFFF1111AAAA2222BBBB3333CCCC4444DDDD5555EEEE6666FFFF1111AAAA2222BBBB3333CCCC4444DDDD5555EEEE"; + + [Fact] + public void TryResolveRefreshTokenPartitionKey_Refresh_HashesTokenUnderTokenPrefixWithoutLeakingSecret() + { + var resolved = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); + + resolved.Resolved.Should().BeTrue(); + resolved.PartitionKey.Should().StartWith("refresh:token:"); + resolved.PartitionKey.Should().NotContain(RefreshTokenA); + } + + [Fact] + public void TryResolveRefreshTokenPartitionKey_Refresh_SameTokenMapsToSameKeyAcrossRequests() + { + var first = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); + var second = ResolveRefreshFor("refresh", new AuthController.RefreshSessionOperationRequest(RefreshTokenA)); + + first.PartitionKey.Should().Be(second.PartitionKey); + } + + [Fact] + public void TryResolveRefreshTokenPartitionKey_Refresh_DifferentTokensMapToDifferentKeys() + { + var first = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); + var second = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenB)); + + first.PartitionKey.Should().NotBe(second.PartitionKey); + } + + [Fact] + public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenTokenIsBlank() + { + var resolved = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(" ")); + + resolved.Resolved.Should().BeFalse(); + resolved.PartitionKey.Should().BeEmpty(); + } + + [Fact] + public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenNoTokenArgument() + { + var resolved = ResolveRefreshFor("refresh", new AuthController.SendCodeRequest("a@x.com")); + + resolved.Resolved.Should().BeFalse(); + } + + [Fact] + public void TryResolveRefreshTokenPartitionKey_DoesNotApplyToUnrelatedPolicy() + { + var resolved = ResolveRefreshFor("auth", new AuthController.RefreshSessionRequest(RefreshTokenA)); + + resolved.Resolved.Should().BeFalse(); + resolved.PartitionKey.Should().BeEmpty(); + } + private static (bool Resolved, string PartitionKey) ResolveFor(string policyName, object request) { var resolved = DistributedRateLimitFilter.TryResolveEmailPartitionKey( @@ -83,4 +143,14 @@ private static (bool Resolved, string PartitionKey) ResolveFor(string policyName return (resolved, partitionKey); } + + private static (bool Resolved, string PartitionKey) ResolveRefreshFor(string policyName, object request) + { + var resolved = DistributedRateLimitFilter.TryResolveRefreshTokenPartitionKey( + policyName, + [request], + out var partitionKey); + + return (resolved, partitionKey); + } } diff --git a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs new file mode 100644 index 00000000..fdc45392 --- /dev/null +++ b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs @@ -0,0 +1,102 @@ +using System.Security.Cryptography; +using System.Text; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; +using NSubstitute; +using Orbit.Domain.Entities; +using Orbit.Domain.Interfaces; +using Orbit.Infrastructure.Configuration; +using Orbit.Infrastructure.Persistence; +using Orbit.Infrastructure.Services; + +namespace Orbit.Infrastructure.Tests.Services; + +public class AuthSessionServiceRevokeAllTests +{ + [Fact] + public async Task RevokeAllSessionsAsync_RevokesOnlyActiveSessionsOfTargetUser() + { + var dbName = NewDbName(); + var target = Guid.NewGuid(); + var other = Guid.NewGuid(); + var alreadyRevokedAt = DateTime.UtcNow.AddDays(-3); + + var targetActiveA = UserSession.Create(target, Hash("target-a"), DateTime.UtcNow.AddDays(90)).Value; + var targetActiveB = UserSession.Create(target, Hash("target-b"), null).Value; + var targetRevoked = UserSession.Create(target, Hash("target-revoked"), DateTime.UtcNow.AddDays(90)).Value; + targetRevoked.Revoke(alreadyRevokedAt); + var otherActive = UserSession.Create(other, Hash("other-a"), DateTime.UtcNow.AddDays(90)).Value; + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.AddRange(targetActiveA, targetActiveB, targetRevoked, otherActive); + await seed.SaveChangesAsync(); + } + + await using (var context = CreateContext(dbName)) + { + var result = await CreateService(context).RevokeAllSessionsAsync(target, CancellationToken.None); + result.IsSuccess.Should().BeTrue(); + } + + await using var verify = CreateContext(dbName); + var sessions = await verify.UserSessions.ToListAsync(); + + sessions.Single(s => s.Id == targetActiveA.Id).RevokedAtUtc.Should().NotBeNull(); + sessions.Single(s => s.Id == targetActiveB.Id).RevokedAtUtc.Should().NotBeNull(); + sessions.Single(s => s.Id == targetRevoked.Id).RevokedAtUtc.Should().BeCloseTo(alreadyRevokedAt, TimeSpan.FromSeconds(1)); + sessions.Single(s => s.Id == otherActive.Id).RevokedAtUtc.Should().BeNull(); + } + + [Fact] + public async Task RevokeAllSessionsAsync_NoActiveSessions_Succeeds() + { + var dbName = NewDbName(); + await using var context = CreateContext(dbName); + + var result = await CreateService(context).RevokeAllSessionsAsync(Guid.NewGuid(), CancellationToken.None); + + result.IsSuccess.Should().BeTrue(); + } + + [Fact] + public async Task RevokeAllSessionsAsync_EmptyUserId_Fails() + { + var dbName = NewDbName(); + await using var context = CreateContext(dbName); + + var result = await CreateService(context).RevokeAllSessionsAsync(Guid.Empty, CancellationToken.None); + + result.IsFailure.Should().BeTrue(); + result.ErrorCode.Should().Be("USER_ID_REQUIRED"); + } + + private static AuthSessionService CreateService(OrbitDbContext context) + { + var tokenService = Substitute.For(); + tokenService.GenerateToken(Arg.Any(), Arg.Any()).Returns("access-token"); + + return new AuthSessionService( + new GenericRepository(context), + new GenericRepository(context), + tokenService, + new UnitOfWork(context), + Options.Create(new JwtSettings + { + SecretKey = "test-secret-key-that-is-at-least-32-bytes-long-for-hmac", + Issuer = "test-issuer", + Audience = "test-audience", + ExpiryMinutes = 0, + RefreshExpiryDays = 90 + })); + } + + private static OrbitDbContext CreateContext(string dbName) => + new(new DbContextOptionsBuilder().UseInMemoryDatabase(dbName).Options); + + private static string NewDbName() => $"AuthSessionRevokeAll_{Guid.NewGuid()}"; + + private static string Hash(string token) => + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(token))); +} From 5658611b3c81aafaa52dff371b1c534678cc07bd Mon Sep 17 00:00:00 2001 From: Thomas Luizon Rodrigues Gregorio Date: Sun, 12 Jul 2026 07:46:14 -0300 Subject: [PATCH 2/4] =?UTF-8?q?fix(api):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?inline=20refresh-token=20format=20gate=20+=20revoke-all=20concu?= =?UTF-8?q?rrency=20(#243)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rate limit: TryResolveRefreshTokenPartitionKey now accepts a token only if it matches the exact server-issued shape (RefreshTokenRules.IsWellFormed, the single source of format truth); a malformed body (the trivial vary-per-request bypass) falls back to IP partitioning so the request is still throttled per IP and unvalidated input no longer reaches the rate-limit DB round-trip. - RevokeAllSessionsAsync now catches DbUpdateConcurrencyException (DiscardChanges + INVALID_SESSION), matching RefreshSessionAsync, so a concurrent refresh during logout-all fails gracefully instead of 500. - Tests: malformed/lowercase/wrong-length tokens fall back to IP; revoke-all concurrency conflict returns a controlled failure without throwing. Refs thomasluizon/orbit-ui-mobile#243 Co-Authored-By: Claude Opus 4.8 --- .../DistributedRateLimitAttribute.cs | 10 +++-- .../Auth/Validators/RefreshTokenRules.cs | 8 ++-- .../Services/AuthSessionService.cs | 10 ++++- .../DistributedRateLimitFilterTests.cs | 4 +- .../DistributedRateLimitPartitionKeyTests.cs | 18 +++++++-- .../AuthSessionServiceRevokeAllTests.cs | 40 ++++++++++++++++++- 6 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs b/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs index b6080b96..e49a1210 100644 --- a/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs +++ b/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs @@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Orbit.Api.Extensions; +using Orbit.Application.Auth.Validators; using Orbit.Domain.Interfaces; using Orbit.Domain.Models; @@ -161,8 +162,11 @@ public static bool TryResolveEmailPartitionKey( /// session, so hashing it yields a stable per-session bucket that a stolen or targeted token cannot /// escape by rotating source IPs — closing the cross-IP brute-force/replay gap that IP partitioning /// leaves open. The token is hashed so the partition key (which is logged) never carries the raw - /// secret. Returns false when the policy isn't refresh partitioned or no non-blank refresh token is - /// present, so the caller falls back to IP partitioning. + /// secret. The token is accepted only if it matches the exact server-issued shape + /// (); a malformed token (the trivial "vary the body to + /// mint a fresh bucket" bypass) resolves to false so the caller falls back to IP partitioning and the + /// request is still throttled per source IP. Also returns false when the policy isn't refresh + /// partitioned or no refresh token is present. /// public static bool TryResolveRefreshTokenPartitionKey( string policyName, @@ -186,7 +190,7 @@ public static bool TryResolveRefreshTokenPartitionKey( if (tokenProperty?.PropertyType != typeof(string)) continue; - if (tokenProperty.GetValue(argument) is not string rawToken || string.IsNullOrWhiteSpace(rawToken)) + if (tokenProperty.GetValue(argument) is not string rawToken || !RefreshTokenRules.IsWellFormed(rawToken)) continue; partitionKey = $"{policyName.ToLowerInvariant()}:token:{HashRefreshToken(rawToken)}"; diff --git a/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs b/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs index b011b9cc..20c627c1 100644 --- a/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs +++ b/src/Orbit.Application/Auth/Validators/RefreshTokenRules.cs @@ -5,14 +5,16 @@ namespace Orbit.Application.Auth.Validators; public static class RefreshTokenRules { public const int TokenLength = 128; - public const string TokenPattern = "^[0-9A-F]+$"; + + public static bool IsWellFormed(string? token) => + token is { Length: TokenLength } + && token.All(static character => character is (>= '0' and <= '9') or (>= 'A' and <= 'F')); public static void AddRefreshTokenRules(IRuleBuilder rule) { rule .NotEmpty() - .Length(TokenLength) - .Matches(TokenPattern) + .Must(token => IsWellFormed(token)) .WithMessage("Refresh token format is invalid."); } } diff --git a/src/Orbit.Infrastructure/Services/AuthSessionService.cs b/src/Orbit.Infrastructure/Services/AuthSessionService.cs index 7560bf34..fc78d846 100644 --- a/src/Orbit.Infrastructure/Services/AuthSessionService.cs +++ b/src/Orbit.Infrastructure/Services/AuthSessionService.cs @@ -117,7 +117,15 @@ public async Task RevokeAllSessionsAsync(Guid userId, CancellationToken foreach (var session in activeSessions) session.Revoke(nowUtc); - await unitOfWork.SaveChangesAsync(cancellationToken); + try + { + await unitOfWork.SaveChangesAsync(cancellationToken); + } + catch (DbUpdateConcurrencyException) + { + unitOfWork.DiscardChanges(); + return Result.Failure(ErrorMessages.InvalidSession); + } return Result.Success(); } diff --git a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs index 1b30cd6e..6e5cf777 100644 --- a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs +++ b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs @@ -9,6 +9,7 @@ using NSubstitute.ExceptionExtensions; using Orbit.Api.Controllers; using Orbit.Api.RateLimiting; +using Orbit.Application.Auth.Validators; using Orbit.Domain.Interfaces; using Orbit.Domain.Models; @@ -82,8 +83,7 @@ await filter.OnActionExecutionAsync(context, () => [Fact] public async Task RefreshPolicy_PartitionsUnauthenticatedRequestByRefreshTokenNotIp() { - const string refreshToken = - "AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555"; + var refreshToken = new string('A', RefreshTokenRules.TokenLength); string? capturedPartitionKey = null; _service.TryAcquireAsync("refresh", Arg.Do(key => capturedPartitionKey = key), Arg.Any()) .Returns(new DistributedRateLimitDecision(true, 1, 10, DateTime.UtcNow.AddMinutes(1))); diff --git a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs index 24d38a84..49b0eaae 100644 --- a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs +++ b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs @@ -1,6 +1,7 @@ using FluentAssertions; using Orbit.Api.Controllers; using Orbit.Api.RateLimiting; +using Orbit.Application.Auth.Validators; namespace Orbit.Infrastructure.Tests.RateLimiting; @@ -74,11 +75,9 @@ [new AuthController.SendCodeRequest("a@x.com")], partitionKey.Should().BeEmpty(); } - private const string RefreshTokenA = - "AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555FFFF6666AAAA1111BBBB2222CCCC3333DDDD4444EEEE5555"; + private static readonly string RefreshTokenA = new('A', RefreshTokenRules.TokenLength); - private const string RefreshTokenB = - "1111AAAA2222BBBB3333CCCC4444DDDD5555EEEE6666FFFF1111AAAA2222BBBB3333CCCC4444DDDD5555EEEE6666FFFF1111AAAA2222BBBB3333CCCC4444DDDD5555EEEE"; + private static readonly string RefreshTokenB = new('B', RefreshTokenRules.TokenLength); [Fact] public void TryResolveRefreshTokenPartitionKey_Refresh_HashesTokenUnderTokenPrefixWithoutLeakingSecret() @@ -117,6 +116,17 @@ public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenTokenIsBlank resolved.PartitionKey.Should().BeEmpty(); } + [Theory] + [InlineData("short")] + [InlineData("g-not-hex-but-right-length-padding-000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")] + public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenTokenIsMalformed(string malformedToken) + { + var lowercase = new string('a', RefreshTokenRules.TokenLength); + + ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(malformedToken)).Resolved.Should().BeFalse(); + ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(lowercase)).Resolved.Should().BeFalse(); + } + [Fact] public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenNoTokenArgument() { diff --git a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs index fdc45392..524b7514 100644 --- a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs +++ b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs @@ -2,6 +2,7 @@ using System.Text; using FluentAssertions; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Options; using NSubstitute; using Orbit.Domain.Entities; @@ -60,6 +61,27 @@ public async Task RevokeAllSessionsAsync_NoActiveSessions_Succeeds() result.IsSuccess.Should().BeTrue(); } + [Fact] + public async Task RevokeAllSessionsAsync_ConcurrencyConflict_ReturnsInvalidSessionWithoutThrowing() + { + var dbName = NewDbName(); + var userId = Guid.NewGuid(); + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.Add(UserSession.Create(userId, Hash("active"), DateTime.UtcNow.AddDays(90)).Value); + await seed.SaveChangesAsync(); + } + + await using var context = CreateContext(dbName, new ConflictAlwaysInterceptor()); + + var act = async () => await CreateService(context).RevokeAllSessionsAsync(userId, CancellationToken.None); + + var result = await act.Should().NotThrowAsync(); + result.Which.IsFailure.Should().BeTrue(); + result.Which.ErrorCode.Should().Be("INVALID_SESSION"); + } + [Fact] public async Task RevokeAllSessionsAsync_EmptyUserId_Fails() { @@ -92,11 +114,25 @@ private static AuthSessionService CreateService(OrbitDbContext context) })); } - private static OrbitDbContext CreateContext(string dbName) => - new(new DbContextOptionsBuilder().UseInMemoryDatabase(dbName).Options); + private static OrbitDbContext CreateContext(string dbName, ISaveChangesInterceptor? interceptor = null) + { + var builder = new DbContextOptionsBuilder().UseInMemoryDatabase(dbName); + if (interceptor is not null) + builder.AddInterceptors(interceptor); + return new OrbitDbContext(builder.Options); + } private static string NewDbName() => $"AuthSessionRevokeAll_{Guid.NewGuid()}"; private static string Hash(string token) => Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(token))); + + private sealed class ConflictAlwaysInterceptor : SaveChangesInterceptor + { + public override ValueTask> SavingChangesAsync( + DbContextEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + throw new DbUpdateConcurrencyException("simulated stale token"); + } + } } From 89d2dab38f613be2e0e8df9f47c51fa3ce1c374d Mon Sep 17 00:00:00 2001 From: Thomas Luizon Rodrigues Gregorio Date: Sun, 12 Jul 2026 11:40:04 -0300 Subject: [PATCH 3/4] fix(api): gate refresh rate-limit partition on a real session lookup (#243) The refresh policy partitioned by SHA256(RefreshToken) whenever the token merely matched RefreshTokenRules.IsWellFormed (128 uppercase-hex). Minting a fresh well-formed token per request is as cheap as the server's own token generation, so a single IP could mint unlimited unthrottled partitions, defeating the 10/min cap and forcing a Serializable rate-limit transaction per request (DoS on the auth surface). Format alone is now insufficient: the per-session (per-token) partition is granted only after IAuthSessionService.HasSessionForTokenAsync confirms the token maps to a real stored session (AnyAsync over the unique TokenHash index). A forged or malformed token an attacker can mint for free never earns a private bucket and falls back to per-IP throttling, while a genuine token keeps its per-session bucket that survives source-IP rotation. Split the pure partition helper into TryExtractRefreshToken + BuildRefreshTokenPartitionKey; add filter, partition-key, and service-level tests including the well-formed-but-forged token -> IP fallback path. Co-Authored-By: Claude Opus 4.8 --- .../DistributedRateLimitAttribute.cs | 55 ++++++++---- .../Interfaces/IAuthSessionService.cs | 8 ++ .../Services/AuthSessionService.cs | 6 ++ .../DistributedRateLimitFilterTests.cs | 31 +++++-- .../DistributedRateLimitPartitionKeyTests.cs | 67 ++++++++------- ...thSessionServiceHasSessionForTokenTests.cs | 84 +++++++++++++++++++ .../FeatureFlagAndAgentSupportTests.cs | 5 +- 7 files changed, 203 insertions(+), 53 deletions(-) create mode 100644 tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs diff --git a/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs b/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs index e49a1210..9d057f83 100644 --- a/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs +++ b/src/Orbit.Api/RateLimiting/DistributedRateLimitAttribute.cs @@ -19,19 +19,25 @@ public IFilterMetadata CreateInstance(IServiceProvider serviceProvider) => new DistributedRateLimitFilter( policyName, serviceProvider.GetRequiredService(), + serviceProvider.GetRequiredService(), serviceProvider.GetRequiredService>()); } public sealed partial class DistributedRateLimitFilter( string policyName, IDistributedRateLimitService distributedRateLimitService, + IAuthSessionService authSessionService, ILogger logger) : IAsyncActionFilter { private const string FailOpenPolicy = "support"; public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) { - var partitionKey = ResolvePartitionKey(policyName, context.HttpContext, context.ActionArguments.Values); + var partitionKey = await ResolvePartitionKeyAsync( + policyName, + context.HttpContext, + context.ActionArguments.Values, + context.HttpContext.RequestAborted); DistributedRateLimitDecision decision; try @@ -97,7 +103,11 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE await next(); } - private static string ResolvePartitionKey(string policyName, HttpContext context, IEnumerable actionArguments) + private async Task ResolvePartitionKeyAsync( + string policyName, + HttpContext context, + IEnumerable actionArguments, + CancellationToken cancellationToken) { if (context.User.Identity?.IsAuthenticated == true) return $"user:{context.GetUserId()}"; @@ -105,8 +115,11 @@ private static string ResolvePartitionKey(string policyName, HttpContext context if (TryResolveEmailPartitionKey(policyName, actionArguments, out var emailPartitionKey)) return emailPartitionKey; - if (TryResolveRefreshTokenPartitionKey(policyName, actionArguments, out var refreshPartitionKey)) - return refreshPartitionKey; + if (TryExtractRefreshToken(policyName, actionArguments, out var refreshToken) + && await authSessionService.HasSessionForTokenAsync(refreshToken, cancellationToken)) + { + return BuildRefreshTokenPartitionKey(policyName, refreshToken); + } return $"ip:{context.GetClientIpAddress() ?? "unknown"}"; } @@ -157,23 +170,20 @@ public static bool TryResolveEmailPartitionKey( new(StringComparer.OrdinalIgnoreCase) { "refresh" }; /// - /// For unauthenticated requests under the refresh policy, partitions by a SHA-256 hash of the - /// request's refresh token instead of the caller IP. A refresh token uniquely identifies one user - /// session, so hashing it yields a stable per-session bucket that a stolen or targeted token cannot - /// escape by rotating source IPs — closing the cross-IP brute-force/replay gap that IP partitioning - /// leaves open. The token is hashed so the partition key (which is logged) never carries the raw - /// secret. The token is accepted only if it matches the exact server-issued shape - /// (); a malformed token (the trivial "vary the body to - /// mint a fresh bucket" bypass) resolves to false so the caller falls back to IP partitioning and the - /// request is still throttled per source IP. Also returns false when the policy isn't refresh - /// partitioned or no refresh token is present. + /// For unauthenticated requests under the refresh policy, extracts the request's refresh token + /// when it matches the exact server-issued shape (). Format + /// alone is not enough to earn a per-session bucket: the caller additionally confirms the token maps to + /// a real stored session before partitioning by it, so a malformed OR well-formed-but-forged token — + /// the "vary the body to mint a fresh, never-throttled bucket" bypass, which is as cheap for an attacker + /// as minting a real token — never yields a private bucket and instead falls back to per-IP throttling. + /// Returns false when the policy isn't refresh partitioned or no well-formed refresh token is present. /// - public static bool TryResolveRefreshTokenPartitionKey( + public static bool TryExtractRefreshToken( string policyName, IEnumerable actionArguments, - out string partitionKey) + out string refreshToken) { - partitionKey = string.Empty; + refreshToken = string.Empty; if (!RefreshTokenPartitionedPolicies.Contains(policyName)) return false; @@ -193,13 +203,22 @@ public static bool TryResolveRefreshTokenPartitionKey( if (tokenProperty.GetValue(argument) is not string rawToken || !RefreshTokenRules.IsWellFormed(rawToken)) continue; - partitionKey = $"{policyName.ToLowerInvariant()}:token:{HashRefreshToken(rawToken)}"; + refreshToken = rawToken; return true; } return false; } + /// + /// Builds the per-session rate-limit partition key for a refresh token already confirmed to map to a + /// real stored session. The token is SHA-256 hashed so the partition key (which is logged) never carries + /// the raw secret; the same token always maps to the same bucket, so a stolen or targeted token cannot + /// escape throttling by rotating source IPs. + /// + public static string BuildRefreshTokenPartitionKey(string policyName, string refreshToken) => + $"{policyName.ToLowerInvariant()}:token:{HashRefreshToken(refreshToken)}"; + private static string HashRefreshToken(string refreshToken) => Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(refreshToken))); diff --git a/src/Orbit.Domain/Interfaces/IAuthSessionService.cs b/src/Orbit.Domain/Interfaces/IAuthSessionService.cs index 673a9f75..dbd6bf99 100644 --- a/src/Orbit.Domain/Interfaces/IAuthSessionService.cs +++ b/src/Orbit.Domain/Interfaces/IAuthSessionService.cs @@ -9,4 +9,12 @@ public interface IAuthSessionService Task> RefreshSessionAsync(string refreshToken, CancellationToken cancellationToken = default); Task RevokeSessionAsync(string refreshToken, CancellationToken cancellationToken = default); Task RevokeAllSessionsAsync(Guid userId, CancellationToken cancellationToken = default); + + /// + /// Returns whether a stored session exists for the given refresh token. The refresh rate limiter uses + /// this so only a token that maps to a real, server-issued session earns a per-session partition; a + /// forged or malformed token an attacker can mint for free never yields a private bucket and is instead + /// throttled per source IP, closing the token-varying rate-limit bypass. + /// + Task HasSessionForTokenAsync(string refreshToken, CancellationToken cancellationToken = default); } diff --git a/src/Orbit.Infrastructure/Services/AuthSessionService.cs b/src/Orbit.Infrastructure/Services/AuthSessionService.cs index fc78d846..be74b53c 100644 --- a/src/Orbit.Infrastructure/Services/AuthSessionService.cs +++ b/src/Orbit.Infrastructure/Services/AuthSessionService.cs @@ -130,6 +130,12 @@ public async Task RevokeAllSessionsAsync(Guid userId, CancellationToken return Result.Success(); } + public async Task HasSessionForTokenAsync(string refreshToken, CancellationToken cancellationToken = default) + { + var tokenHash = HashToken(refreshToken); + return await userSessionRepository.AnyAsync(session => session.TokenHash == tokenHash, cancellationToken); + } + private static string GenerateRefreshToken() { return Convert.ToHexString(RandomNumberGenerator.GetBytes(64)); diff --git a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs index 6e5cf777..bfb1f778 100644 --- a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs +++ b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitFilterTests.cs @@ -18,6 +18,7 @@ namespace Orbit.Infrastructure.Tests.RateLimiting; public class DistributedRateLimitFilterTests { private readonly IDistributedRateLimitService _service = Substitute.For(); + private readonly IAuthSessionService _authSessionService = Substitute.For(); private readonly ILogger _logger = Substitute.For>(); [Fact] @@ -26,7 +27,7 @@ public async Task SupportPolicy_FailsOpen_WhenRateLimitStoreUnavailable() _service.TryAcquireAsync("support", Arg.Any(), Arg.Any()) .ThrowsAsync(new InvalidOperationException("rate-limit store down")); - var filter = new DistributedRateLimitFilter("support", _service, _logger); + var filter = new DistributedRateLimitFilter("support", _service, _authSessionService, _logger); var (context, _) = CreateExecutingContext(); var nextCalled = false; @@ -46,7 +47,7 @@ public async Task NonSupportPolicy_FailsClosed_WhenRateLimitStoreUnavailable() _service.TryAcquireAsync("chat", Arg.Any(), Arg.Any()) .ThrowsAsync(new InvalidOperationException("rate-limit store down")); - var filter = new DistributedRateLimitFilter("chat", _service, _logger); + var filter = new DistributedRateLimitFilter("chat", _service, _authSessionService, _logger); var (context, _) = CreateExecutingContext(); var nextCalled = false; @@ -66,7 +67,7 @@ public async Task SupportPolicy_StillThrottles_WhenStoreReportsLimitExceeded() _service.TryAcquireAsync("support", Arg.Any(), Arg.Any()) .Returns(new DistributedRateLimitDecision(false, 3, 3, DateTime.UtcNow.AddHours(1))); - var filter = new DistributedRateLimitFilter("support", _service, _logger); + var filter = new DistributedRateLimitFilter("support", _service, _authSessionService, _logger); var (context, _) = CreateExecutingContext(); var nextCalled = false; @@ -81,14 +82,15 @@ await filter.OnActionExecutionAsync(context, () => } [Fact] - public async Task RefreshPolicy_PartitionsUnauthenticatedRequestByRefreshTokenNotIp() + public async Task RefreshPolicy_PartitionsUnauthenticatedRequestByRefreshToken_WhenTokenMapsToRealSession() { var refreshToken = new string('A', RefreshTokenRules.TokenLength); + _authSessionService.HasSessionForTokenAsync(refreshToken, Arg.Any()).Returns(true); string? capturedPartitionKey = null; _service.TryAcquireAsync("refresh", Arg.Do(key => capturedPartitionKey = key), Arg.Any()) .Returns(new DistributedRateLimitDecision(true, 1, 10, DateTime.UtcNow.AddMinutes(1))); - var filter = new DistributedRateLimitFilter("refresh", _service, _logger); + var filter = new DistributedRateLimitFilter("refresh", _service, _authSessionService, _logger); var (context, _) = CreateExecutingContext(new AuthController.RefreshSessionRequest(refreshToken)); await filter.OnActionExecutionAsync(context, () => Task.FromResult(CreateExecutedContext(context))); @@ -97,6 +99,25 @@ public async Task RefreshPolicy_PartitionsUnauthenticatedRequestByRefreshTokenNo capturedPartitionKey.Should().NotContain(refreshToken); } + [Fact] + public async Task RefreshPolicy_FallsBackToIpPartition_WhenWellFormedTokenHasNoRealSession() + { + var forgedToken = new string('A', RefreshTokenRules.TokenLength); + _authSessionService.HasSessionForTokenAsync(forgedToken, Arg.Any()).Returns(false); + string? capturedPartitionKey = null; + _service.TryAcquireAsync("refresh", Arg.Do(key => capturedPartitionKey = key), Arg.Any()) + .Returns(new DistributedRateLimitDecision(true, 1, 10, DateTime.UtcNow.AddMinutes(1))); + + var filter = new DistributedRateLimitFilter("refresh", _service, _authSessionService, _logger); + var (context, httpContext) = CreateExecutingContext(new AuthController.RefreshSessionRequest(forgedToken)); + httpContext.Connection.RemoteIpAddress = System.Net.IPAddress.Parse("203.0.113.7"); + + await filter.OnActionExecutionAsync(context, () => Task.FromResult(CreateExecutedContext(context))); + + capturedPartitionKey.Should().StartWith("ip:"); + capturedPartitionKey.Should().NotStartWith("refresh:token:"); + } + private static (ActionExecutingContext Context, HttpContext HttpContext) CreateExecutingContext( params object?[] actionArguments) { diff --git a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs index 49b0eaae..5d28d787 100644 --- a/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs +++ b/tests/Orbit.Infrastructure.Tests/RateLimiting/DistributedRateLimitPartitionKeyTests.cs @@ -80,68 +80,79 @@ [new AuthController.SendCodeRequest("a@x.com")], private static readonly string RefreshTokenB = new('B', RefreshTokenRules.TokenLength); [Fact] - public void TryResolveRefreshTokenPartitionKey_Refresh_HashesTokenUnderTokenPrefixWithoutLeakingSecret() + public void BuildRefreshTokenPartitionKey_HashesTokenUnderTokenPrefixWithoutLeakingSecret() { - var resolved = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); + var partitionKey = DistributedRateLimitFilter.BuildRefreshTokenPartitionKey("refresh", RefreshTokenA); - resolved.Resolved.Should().BeTrue(); - resolved.PartitionKey.Should().StartWith("refresh:token:"); - resolved.PartitionKey.Should().NotContain(RefreshTokenA); + partitionKey.Should().StartWith("refresh:token:"); + partitionKey.Should().NotContain(RefreshTokenA); } [Fact] - public void TryResolveRefreshTokenPartitionKey_Refresh_SameTokenMapsToSameKeyAcrossRequests() + public void BuildRefreshTokenPartitionKey_SameTokenMapsToSameKey() { - var first = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); - var second = ResolveRefreshFor("refresh", new AuthController.RefreshSessionOperationRequest(RefreshTokenA)); + var first = DistributedRateLimitFilter.BuildRefreshTokenPartitionKey("refresh", RefreshTokenA); + var second = DistributedRateLimitFilter.BuildRefreshTokenPartitionKey("refresh", RefreshTokenA); - first.PartitionKey.Should().Be(second.PartitionKey); + first.Should().Be(second); } [Fact] - public void TryResolveRefreshTokenPartitionKey_Refresh_DifferentTokensMapToDifferentKeys() + public void BuildRefreshTokenPartitionKey_DifferentTokensMapToDifferentKeys() { - var first = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); - var second = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenB)); + var first = DistributedRateLimitFilter.BuildRefreshTokenPartitionKey("refresh", RefreshTokenA); + var second = DistributedRateLimitFilter.BuildRefreshTokenPartitionKey("refresh", RefreshTokenB); - first.PartitionKey.Should().NotBe(second.PartitionKey); + first.Should().NotBe(second); } [Fact] - public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenTokenIsBlank() + public void TryExtractRefreshToken_Refresh_ReturnsWellFormedTokenAcrossRequestShapes() { - var resolved = ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(" ")); + var fromPlain = ExtractRefreshFor("refresh", new AuthController.RefreshSessionRequest(RefreshTokenA)); + var fromOperation = ExtractRefreshFor("refresh", new AuthController.RefreshSessionOperationRequest(RefreshTokenA)); + + fromPlain.Resolved.Should().BeTrue(); + fromPlain.RefreshToken.Should().Be(RefreshTokenA); + fromOperation.Resolved.Should().BeTrue(); + fromOperation.RefreshToken.Should().Be(RefreshTokenA); + } + + [Fact] + public void TryExtractRefreshToken_Refresh_FallsBackWhenTokenIsBlank() + { + var resolved = ExtractRefreshFor("refresh", new AuthController.RefreshSessionRequest(" ")); resolved.Resolved.Should().BeFalse(); - resolved.PartitionKey.Should().BeEmpty(); + resolved.RefreshToken.Should().BeEmpty(); } [Theory] [InlineData("short")] [InlineData("g-not-hex-but-right-length-padding-000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")] - public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenTokenIsMalformed(string malformedToken) + public void TryExtractRefreshToken_Refresh_FallsBackWhenTokenIsMalformed(string malformedToken) { var lowercase = new string('a', RefreshTokenRules.TokenLength); - ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(malformedToken)).Resolved.Should().BeFalse(); - ResolveRefreshFor("refresh", new AuthController.RefreshSessionRequest(lowercase)).Resolved.Should().BeFalse(); + ExtractRefreshFor("refresh", new AuthController.RefreshSessionRequest(malformedToken)).Resolved.Should().BeFalse(); + ExtractRefreshFor("refresh", new AuthController.RefreshSessionRequest(lowercase)).Resolved.Should().BeFalse(); } [Fact] - public void TryResolveRefreshTokenPartitionKey_Refresh_FallsBackWhenNoTokenArgument() + public void TryExtractRefreshToken_Refresh_FallsBackWhenNoTokenArgument() { - var resolved = ResolveRefreshFor("refresh", new AuthController.SendCodeRequest("a@x.com")); + var resolved = ExtractRefreshFor("refresh", new AuthController.SendCodeRequest("a@x.com")); resolved.Resolved.Should().BeFalse(); } [Fact] - public void TryResolveRefreshTokenPartitionKey_DoesNotApplyToUnrelatedPolicy() + public void TryExtractRefreshToken_DoesNotApplyToUnrelatedPolicy() { - var resolved = ResolveRefreshFor("auth", new AuthController.RefreshSessionRequest(RefreshTokenA)); + var resolved = ExtractRefreshFor("auth", new AuthController.RefreshSessionRequest(RefreshTokenA)); resolved.Resolved.Should().BeFalse(); - resolved.PartitionKey.Should().BeEmpty(); + resolved.RefreshToken.Should().BeEmpty(); } private static (bool Resolved, string PartitionKey) ResolveFor(string policyName, object request) @@ -154,13 +165,13 @@ private static (bool Resolved, string PartitionKey) ResolveFor(string policyName return (resolved, partitionKey); } - private static (bool Resolved, string PartitionKey) ResolveRefreshFor(string policyName, object request) + private static (bool Resolved, string RefreshToken) ExtractRefreshFor(string policyName, object request) { - var resolved = DistributedRateLimitFilter.TryResolveRefreshTokenPartitionKey( + var resolved = DistributedRateLimitFilter.TryExtractRefreshToken( policyName, [request], - out var partitionKey); + out var refreshToken); - return (resolved, partitionKey); + return (resolved, refreshToken); } } diff --git a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs new file mode 100644 index 00000000..8fada31e --- /dev/null +++ b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs @@ -0,0 +1,84 @@ +using System.Security.Cryptography; +using System.Text; +using FluentAssertions; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; +using NSubstitute; +using Orbit.Domain.Entities; +using Orbit.Domain.Interfaces; +using Orbit.Infrastructure.Configuration; +using Orbit.Infrastructure.Persistence; +using Orbit.Infrastructure.Services; + +namespace Orbit.Infrastructure.Tests.Services; + +public class AuthSessionServiceHasSessionForTokenTests +{ + [Fact] + public async Task HasSessionForTokenAsync_ReturnsTrue_WhenAStoredSessionMatchesTheToken() + { + var dbName = NewDbName(); + const string refreshToken = "a-real-server-issued-token"; + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.Add(UserSession.Create(Guid.NewGuid(), Hash(refreshToken), DateTime.UtcNow.AddDays(90)).Value); + await seed.SaveChangesAsync(); + } + + await using var context = CreateContext(dbName); + + var exists = await CreateService(context).HasSessionForTokenAsync(refreshToken, CancellationToken.None); + + exists.Should().BeTrue(); + } + + [Fact] + public async Task HasSessionForTokenAsync_ReturnsFalse_WhenNoStoredSessionMatchesTheToken() + { + var dbName = NewDbName(); + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.Add(UserSession.Create(Guid.NewGuid(), Hash("a-different-token"), DateTime.UtcNow.AddDays(90)).Value); + await seed.SaveChangesAsync(); + } + + await using var context = CreateContext(dbName); + + var exists = await CreateService(context).HasSessionForTokenAsync("a-forged-token-no-session-has", CancellationToken.None); + + exists.Should().BeFalse(); + } + + private static AuthSessionService CreateService(OrbitDbContext context) + { + var tokenService = Substitute.For(); + tokenService.GenerateToken(Arg.Any(), Arg.Any()).Returns("access-token"); + + return new AuthSessionService( + new GenericRepository(context), + new GenericRepository(context), + tokenService, + new UnitOfWork(context), + Options.Create(new JwtSettings + { + SecretKey = "test-secret-key-that-is-at-least-32-bytes-long-for-hmac", + Issuer = "test-issuer", + Audience = "test-audience", + ExpiryMinutes = 0, + RefreshExpiryDays = 90 + })); + } + + private static OrbitDbContext CreateContext(string dbName) + { + var builder = new DbContextOptionsBuilder().UseInMemoryDatabase(dbName); + return new OrbitDbContext(builder.Options); + } + + private static string NewDbName() => $"AuthSessionHasToken_{Guid.NewGuid()}"; + + private static string Hash(string token) => + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(token))); +} diff --git a/tests/Orbit.Infrastructure.Tests/Services/FeatureFlagAndAgentSupportTests.cs b/tests/Orbit.Infrastructure.Tests/Services/FeatureFlagAndAgentSupportTests.cs index bb0c8ec8..5ab015d0 100644 --- a/tests/Orbit.Infrastructure.Tests/Services/FeatureFlagAndAgentSupportTests.cs +++ b/tests/Orbit.Infrastructure.Tests/Services/FeatureFlagAndAgentSupportTests.cs @@ -134,6 +134,7 @@ public void DistributedRateLimitAttribute_CreatesFilterWithResolvedService() var logger = Substitute.For>(); var serviceProvider = Substitute.For(); serviceProvider.GetService(typeof(IDistributedRateLimitService)).Returns(rateLimitService); + serviceProvider.GetService(typeof(IAuthSessionService)).Returns(Substitute.For()); serviceProvider.GetService(typeof(ILogger)).Returns(logger); var attribute = new DistributedRateLimitAttribute("chat"); @@ -150,7 +151,7 @@ public async Task DistributedRateLimitFilter_UsesAuthenticatedUserPartitionAndCa var logger = Substitute.For>(); rateLimitService.TryAcquireAsync("chat", $"user:{userId}", Arg.Any()) .Returns(new DistributedRateLimitDecision(true, 20, 1, DateTime.UtcNow.AddSeconds(30))); - var filter = new DistributedRateLimitFilter("chat", rateLimitService, logger); + var filter = new DistributedRateLimitFilter("chat", rateLimitService, Substitute.For(), logger); var context = CreateActionExecutingContext(new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity( @@ -178,7 +179,7 @@ public async Task DistributedRateLimitFilter_ReturnsTooManyRequestsForAnonymousI var retryAt = DateTime.UtcNow.AddSeconds(15); rateLimitService.TryAcquireAsync("auth", "ip:203.0.113.10", Arg.Any()) .Returns(new DistributedRateLimitDecision(false, 5, 5, retryAt)); - var filter = new DistributedRateLimitFilter("auth", rateLimitService, logger); + var filter = new DistributedRateLimitFilter("auth", rateLimitService, Substitute.For(), logger); var httpContext = new DefaultHttpContext(); httpContext.Connection.RemoteIpAddress = IPAddress.Parse("203.0.113.10"); httpContext.TraceIdentifier = "req_rate_limited"; From 9f1fa62a2275703f2b346c742a8b68aa84b0ff3b Mon Sep 17 00:00:00 2001 From: Thomas Luizon Rodrigues Gregorio Date: Sun, 12 Jul 2026 12:07:23 -0300 Subject: [PATCH 4/4] fix(api): make logout-all concurrency-resilient + scope refresh partition to usable sessions (#243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RevokeAllSessionsAsync batched every active session into one SaveChanges: a single concurrent write to any one of them (e.g. another device auto-refreshing at the same instant) threw DbUpdateConcurrencyException, discarded the whole batch, and returned failure — so NONE of the user's sessions were revoked. That silently defeats the account-wide "log out everywhere" security control and let an attacker holding one stolen token keep looping refresh to block revocation. Replace the discard-and-fail catch with the codebase's concurrency-retry idiom: on conflict, ResetTracking and reload current state, then re-revoke and retry (bounded, mirroring DistributedRateLimitService). A single conflicting row no longer defeats the batch; genuine repeated conflicts still surface a controlled failure without throwing. Also scope HasSessionForTokenAsync to currently-usable sessions (RevokedAtUtc == null and not expired, mirroring UserSession.CanUse), so a revoked or expired token no longer earns a stable private rate-limit bucket and falls back to per-IP throttling. Tests: transient-conflict retry revokes every active session; revoked and expired tokens are excluded from the per-token partition. Co-Authored-By: Claude Opus 4.8 --- .../Services/AuthSessionService.cs | 47 ++++++++++++------- ...thSessionServiceHasSessionForTokenTests.cs | 41 ++++++++++++++++ .../AuthSessionServiceRevokeAllTests.cs | 42 +++++++++++++++++ 3 files changed, 112 insertions(+), 18 deletions(-) diff --git a/src/Orbit.Infrastructure/Services/AuthSessionService.cs b/src/Orbit.Infrastructure/Services/AuthSessionService.cs index be74b53c..4ce8fe5c 100644 --- a/src/Orbit.Infrastructure/Services/AuthSessionService.cs +++ b/src/Orbit.Infrastructure/Services/AuthSessionService.cs @@ -18,6 +18,8 @@ public class AuthSessionService( IUnitOfWork unitOfWork, IOptions jwtSettings) : IAuthSessionService { + private const int MaxRevokeAllAttempts = 3; + private readonly JwtSettings _jwtSettings = jwtSettings.Value; private DateTime? GetRefreshExpiry(DateTime nowUtc) => @@ -107,33 +109,42 @@ public async Task RevokeAllSessionsAsync(Guid userId, CancellationToken return Result.Failure(DomainErrors.UserIdRequired); var nowUtc = DateTime.UtcNow; - var activeSessions = await userSessionRepository.FindTrackedAsync( - s => s.UserId == userId && s.RevokedAtUtc == null, - cancellationToken); - - if (activeSessions.Count == 0) - return Result.Success(); - - foreach (var session in activeSessions) - session.Revoke(nowUtc); - try - { - await unitOfWork.SaveChangesAsync(cancellationToken); - } - catch (DbUpdateConcurrencyException) + for (var attempt = 0; attempt < MaxRevokeAllAttempts; attempt++) { - unitOfWork.DiscardChanges(); - return Result.Failure(ErrorMessages.InvalidSession); + var activeSessions = await userSessionRepository.FindTrackedAsync( + s => s.UserId == userId && s.RevokedAtUtc == null, + cancellationToken); + + if (activeSessions.Count == 0) + return Result.Success(); + + foreach (var session in activeSessions) + session.Revoke(nowUtc); + + try + { + await unitOfWork.SaveChangesAsync(cancellationToken); + return Result.Success(); + } + catch (DbUpdateConcurrencyException) + { + unitOfWork.ResetTracking(); + } } - return Result.Success(); + return Result.Failure(ErrorMessages.InvalidSession); } public async Task HasSessionForTokenAsync(string refreshToken, CancellationToken cancellationToken = default) { + var nowUtc = DateTime.UtcNow; var tokenHash = HashToken(refreshToken); - return await userSessionRepository.AnyAsync(session => session.TokenHash == tokenHash, cancellationToken); + return await userSessionRepository.AnyAsync( + session => session.TokenHash == tokenHash + && session.RevokedAtUtc == null + && (session.ExpiresAtUtc == null || session.ExpiresAtUtc > nowUtc), + cancellationToken); } private static string GenerateRefreshToken() diff --git a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs index 8fada31e..b1527bdf 100644 --- a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs +++ b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceHasSessionForTokenTests.cs @@ -51,6 +51,47 @@ public async Task HasSessionForTokenAsync_ReturnsFalse_WhenNoStoredSessionMatche exists.Should().BeFalse(); } + [Fact] + public async Task HasSessionForTokenAsync_ReturnsFalse_WhenTheMatchingSessionIsRevoked() + { + var dbName = NewDbName(); + const string refreshToken = "a-revoked-token"; + var session = UserSession.Create(Guid.NewGuid(), Hash(refreshToken), DateTime.UtcNow.AddDays(90)).Value; + session.Revoke(DateTime.UtcNow); + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.Add(session); + await seed.SaveChangesAsync(); + } + + await using var context = CreateContext(dbName); + + var exists = await CreateService(context).HasSessionForTokenAsync(refreshToken, CancellationToken.None); + + exists.Should().BeFalse(); + } + + [Fact] + public async Task HasSessionForTokenAsync_ReturnsFalse_WhenTheMatchingSessionIsExpired() + { + var dbName = NewDbName(); + const string refreshToken = "an-expired-token"; + var session = UserSession.Create(Guid.NewGuid(), Hash(refreshToken), DateTime.UtcNow.AddDays(-1)).Value; + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.Add(session); + await seed.SaveChangesAsync(); + } + + await using var context = CreateContext(dbName); + + var exists = await CreateService(context).HasSessionForTokenAsync(refreshToken, CancellationToken.None); + + exists.Should().BeFalse(); + } + private static AuthSessionService CreateService(OrbitDbContext context) { var tokenService = Substitute.For(); diff --git a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs index 524b7514..c05bf79c 100644 --- a/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs +++ b/tests/Orbit.Infrastructure.Tests/Services/AuthSessionServiceRevokeAllTests.cs @@ -61,6 +61,33 @@ public async Task RevokeAllSessionsAsync_NoActiveSessions_Succeeds() result.IsSuccess.Should().BeTrue(); } + [Fact] + public async Task RevokeAllSessionsAsync_TransientConcurrencyConflict_RetriesAndRevokesEveryActiveSession() + { + var dbName = NewDbName(); + var userId = Guid.NewGuid(); + var sessionA = UserSession.Create(userId, Hash("active-a"), DateTime.UtcNow.AddDays(90)).Value; + var sessionB = UserSession.Create(userId, Hash("active-b"), DateTime.UtcNow.AddDays(90)).Value; + + await using (var seed = CreateContext(dbName)) + { + seed.UserSessions.AddRange(sessionA, sessionB); + await seed.SaveChangesAsync(); + } + + await using (var context = CreateContext(dbName, new ConflictOnceInterceptor())) + { + var result = await CreateService(context).RevokeAllSessionsAsync(userId, CancellationToken.None); + result.IsSuccess.Should().BeTrue(); + } + + await using var verify = CreateContext(dbName); + var sessions = await verify.UserSessions.Where(s => s.UserId == userId).ToListAsync(); + + sessions.Should().HaveCount(2); + sessions.Should().OnlyContain(s => s.RevokedAtUtc != null); + } + [Fact] public async Task RevokeAllSessionsAsync_ConcurrencyConflict_ReturnsInvalidSessionWithoutThrowing() { @@ -135,4 +162,19 @@ public override ValueTask> SavingChangesAsync( throw new DbUpdateConcurrencyException("simulated stale token"); } } + + private sealed class ConflictOnceInterceptor : SaveChangesInterceptor + { + private bool _hasThrown; + + public override ValueTask> SavingChangesAsync( + DbContextEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + if (_hasThrown) + return base.SavingChangesAsync(eventData, result, cancellationToken); + + _hasThrown = true; + throw new DbUpdateConcurrencyException("simulated transient stale token"); + } + } }