From d5d1741167e4aa1fedc6a3c5aa90b8d29463708d Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 12 Oct 2026 21:23:16 -0500 Subject: [PATCH 1/9] Synchronize account and user token lifetimes --- src/Exceptionless.Core/Models/OAuthToken.cs | 1 + src/Exceptionless.Core/Models/Token.cs | 1 + src/Exceptionless.Core/Models/User.cs | 1 + .../Services/OAuthService.cs | 26 +++++-- .../Api/Handlers/AuthHandler.cs | 25 ++++++- .../Api/Handlers/OAuthHandler.cs | 2 +- .../Api/Handlers/TokenHandler.cs | 11 ++- .../Security/ApiKeyAuthenticationHandler.cs | 9 ++- .../Exceptionless.Tests/Api/Data/openapi.json | 6 ++ .../Api/Endpoints/AuthEndpointTests.cs | 74 +++++++++++++++++++ .../Api/Endpoints/OAuthEndpointTests.cs | 35 +++++++++ .../Api/Handlers/AuthHandlerTests.cs | 34 ++++++++- .../Models/OAuthTokenSerializerTests.cs | 14 +++- .../Serializer/Models/TokenSerializerTests.cs | 4 + .../Serializer/Models/UserSerializerTests.cs | 4 + 15 files changed, 226 insertions(+), 21 deletions(-) diff --git a/src/Exceptionless.Core/Models/OAuthToken.cs b/src/Exceptionless.Core/Models/OAuthToken.cs index 96807c44f6..32b31c86f1 100644 --- a/src/Exceptionless.Core/Models/OAuthToken.cs +++ b/src/Exceptionless.Core/Models/OAuthToken.cs @@ -13,6 +13,7 @@ public class OAuthToken : IIdentity, IHaveDates, IValidatableObject [Required] [ObjectId] public string UserId { get; set; } = null!; + public string? AuthenticationVersion { get; set; } [Required] [MaxLength(2048)] diff --git a/src/Exceptionless.Core/Models/Token.cs b/src/Exceptionless.Core/Models/Token.cs index d2592769f2..d5ec4ea4e4 100644 --- a/src/Exceptionless.Core/Models/Token.cs +++ b/src/Exceptionless.Core/Models/Token.cs @@ -35,6 +35,7 @@ public class Token : IOwnedByOrganizationAndProjectWithIdentity, IHaveDates, IVa [ObjectId] public string? DefaultProjectId { get; set; } public string? Refresh { get; set; } + public string? AuthenticationVersion { get; set; } public TokenType Type { get; set; } public HashSet Scopes { get; set; } = new(); public DateTime? ExpiresUtc { get; set; } diff --git a/src/Exceptionless.Core/Models/User.cs b/src/Exceptionless.Core/Models/User.cs index 0a8f2c9143..7786c21019 100644 --- a/src/Exceptionless.Core/Models/User.cs +++ b/src/Exceptionless.Core/Models/User.cs @@ -20,6 +20,7 @@ public record User : IIdentity, IHaveDates, IValidatableObject public ISet OrganizationIds { get; init; } = new HashSet(); public string? Password { get; set; } + public string? AuthenticationVersion { get; set; } public string? Salt { get; set; } public string? PasswordResetToken { get; set; } public DateTime PasswordResetTokenExpiration { get; set; } diff --git a/src/Exceptionless.Core/Services/OAuthService.cs b/src/Exceptionless.Core/Services/OAuthService.cs index 138d6c2de0..4f0ecd9b12 100644 --- a/src/Exceptionless.Core/Services/OAuthService.cs +++ b/src/Exceptionless.Core/Services/OAuthService.cs @@ -384,6 +384,13 @@ public async Task ValidateAuthorizationRequestAsync(OAuth } public async Task CreateAuthorizationCodeAsync(OAuthAuthorizeRequest request, string userId, IReadOnlyCollection organizationIds) + { + var user = await userRepository.GetByIdAsync(userId) + ?? throw new InvalidOperationException("The user does not exist."); + return await CreateAuthorizationCodeAsync(request, user, organizationIds); + } + + public async Task CreateAuthorizationCodeAsync(OAuthAuthorizeRequest request, User user, IReadOnlyCollection organizationIds) { await oauthApplicationRepository.AddOrganizationIdsAsync(request.ClientId, organizationIds, o => o.ImmediateConsistency().Notifications(false)); @@ -392,7 +399,8 @@ public async Task CreateAuthorizationCodeAsync(OAuthAuthorizeRequest req { ClientId = request.ClientId, RedirectUri = request.RedirectUri, - UserId = userId, + UserId = user.Id, + AuthenticationVersion = user.AuthenticationVersion, CodeChallenge = request.CodeChallenge, Resource = request.Resource ?? throw new InvalidOperationException("OAuth resource must be validated before creating an authorization code."), Scopes = NormalizeScopes(request.Scope), @@ -432,7 +440,11 @@ public async Task ExchangeAuthorizationCodeAsync(OAuthTok if (!ValidateCodeVerifier(code.CodeChallenge, request.CodeVerifier)) return OAuthTokenIssueResult.Invalid("invalid_grant", "Invalid PKCE verifier."); - return OAuthTokenIssueResult.Success(await CreateTokenAsync(code.UserId, code.ClientId, code.Resource, code.Scopes, code.OrganizationIds)); + var user = await userRepository.GetByIdAsync(code.UserId); + if (user is not { IsActive: true } || !String.Equals(user.AuthenticationVersion, code.AuthenticationVersion, StringComparison.Ordinal)) + return OAuthTokenIssueResult.Invalid("invalid_grant", "Authorization code is invalid or expired."); + + return OAuthTokenIssueResult.Success(await CreateTokenAsync(code.UserId, code.ClientId, code.Resource, code.Scopes, code.OrganizationIds, authenticationVersion: code.AuthenticationVersion)); } public async Task RefreshAsync(OAuthTokenRequest request) @@ -473,8 +485,8 @@ public async Task RefreshAsync(OAuthTokenRequest request) return OAuthTokenIssueResult.Invalid("invalid_grant", "Refresh token is invalid."); } - var user = await userRepository.GetByIdAsync(token.UserId, o => o.ImmediateConsistency()); - if (user is null || !user.IsActive) + var user = await userRepository.GetByIdAsync(token.UserId); + if (user is null || !user.IsActive || !String.Equals(user.AuthenticationVersion, token.AuthenticationVersion, StringComparison.Ordinal)) { await RevokeOAuthGrantFamilyAsync(token); return OAuthTokenIssueResult.Invalid("invalid_grant", "Refresh token is invalid."); @@ -490,7 +502,7 @@ public async Task RefreshAsync(OAuthTokenRequest request) if (!token.IsDisabled) await SpendRefreshTokenAsync(token, utcNow); - return OAuthTokenIssueResult.Success(await CreateTokenAsync(token.UserId, token.ClientId, token.Resource, scopeValidation.Scopes, activeOrganizationIds, token.GrantId)); + return OAuthTokenIssueResult.Success(await CreateTokenAsync(token.UserId, token.ClientId, token.Resource, scopeValidation.Scopes, activeOrganizationIds, token.GrantId, token.AuthenticationVersion)); } public async Task RevokeAsync(string? tokenValue, string? clientId) @@ -590,7 +602,7 @@ private Task DisableTokenAsync(OAuthToken token) return oauthTokenRepository.SaveAsync(token, o => o.ImmediateConsistency()); } - private async Task CreateTokenAsync(string userId, string clientId, string resource, IReadOnlyCollection scopes, IReadOnlyCollection organizationIds, string? grantId = null) + private async Task CreateTokenAsync(string userId, string clientId, string resource, IReadOnlyCollection scopes, IReadOnlyCollection organizationIds, string? grantId = null, string? authenticationVersion = null) { var utcNow = timeProvider.GetUtcNow().UtcDateTime; var accessToken = CreateOAuthToken(); @@ -599,6 +611,7 @@ private async Task CreateTokenAsync(string userId, string cl { Id = ObjectId.GenerateNewId().ToString(), UserId = userId, + AuthenticationVersion = authenticationVersion, ClientId = clientId, GrantId = String.IsNullOrWhiteSpace(grantId) ? StringExtensions.GetNewToken() : grantId, Resource = resource, @@ -863,6 +876,7 @@ public record OAuthClientRegistrationResponse public record OAuthAuthorizationCode { + public string? AuthenticationVersion { get; init; } public required string ClientId { get; init; } public required string RedirectUri { get; init; } public required string UserId { get; init; } diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index b02c4325c2..4265f06900 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -311,7 +311,10 @@ public async Task> Handle(RemoveExternalLogin message) try { if (user.RemoveOAuthAccount(message.ProviderName, message.ProviderUserId.Value)) + { + user.AuthenticationVersion = Core.Extensions.StringExtensions.GetNewToken(); await userRepository.SaveAsync(user, o => o.Cache()); + } } catch (Exception ex) { @@ -654,6 +657,7 @@ private async Task ChangePasswordAsync(User user, string password, string tag, H user.Salt = Core.Extensions.StringExtensions.GetNewToken(); user.Password = password.ToSaltedHash(user.Salt); + user.AuthenticationVersion = Core.Extensions.StringExtensions.GetNewToken(); user.ResetPasswordResetToken(); try @@ -671,16 +675,26 @@ private async Task ChangePasswordAsync(User user, string password, string tag, H private async Task ResetUserTokensAsync(User user, string tag, HttpContext httpContext) { using var _ = logger.BeginScope(new ExceptionlessState().Tag(tag).Identity(user.EmailAddress).SetHttpContext(httpContext)); + long total = 0; + try + { + total += await tokenRepository.RemoveAllByUserIdAsync(user.Id); + } + catch (Exception ex) + { + logger.LogError(ex, "Error removing user tokens for {UserId}: {Message}", user.Id, ex.Message); + } + try { - long total = await tokenRepository.RemoveAllByUserIdAsync(user.Id); total += await oauthTokenRepository.RemoveAllByUserIdAsync(user.Id); - logger.RemovedUserTokens(total, user.EmailAddress); } catch (Exception ex) { - logger.LogCritical(ex, "Error removing user tokens for {EmailAddress}: {Message}", user.EmailAddress, ex.Message); + logger.LogError(ex, "Error removing OAuth tokens for {UserId}: {Message}", user.Id, ex.Message); } + + logger.RemovedUserTokens(total, user.EmailAddress); } private async Task GetOrCreateAuthenticationTokenAsync(User user) @@ -688,7 +702,9 @@ private async Task GetOrCreateAuthenticationTokenAsync(User user) var userTokens = await tokenRepository.GetByTypeAndUserIdAsync(TokenType.Authentication, user.Id); var utcNow = timeProvider.GetUtcNow().UtcDateTime; - var validAccessToken = userTokens.Documents.FirstOrDefault(token => !token.ExpiresUtc.HasValue || token.ExpiresUtc > utcNow); + var validAccessToken = userTokens.Documents.FirstOrDefault(token => !token.IsDisabled && !token.IsSuspended + && String.Equals(token.AuthenticationVersion, user.AuthenticationVersion, StringComparison.Ordinal) + && (!token.ExpiresUtc.HasValue || token.ExpiresUtc > utcNow)); if (validAccessToken is not null) return validAccessToken.Id; @@ -696,6 +712,7 @@ private async Task GetOrCreateAuthenticationTokenAsync(User user) { Id = Core.Extensions.StringExtensions.GetNewToken(), UserId = user.Id, + AuthenticationVersion = user.AuthenticationVersion, CreatedUtc = utcNow, UpdatedUtc = utcNow, ExpiresUtc = utcNow.AddMonths(3), diff --git a/src/Exceptionless.Web/Api/Handlers/OAuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/OAuthHandler.cs index b615d9960c..68b7a215b2 100644 --- a/src/Exceptionless.Web/Api/Handlers/OAuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/OAuthHandler.cs @@ -194,7 +194,7 @@ private async Task CompleteAuthorizationAsync(OAuthAuthorizeRequest req if (!organizationValidation.IsValid) return OAuthError("invalid_request", organizationValidation.ErrorDescription); - string code = await oauthService.CreateAuthorizationCodeAsync(request, HttpContext.Request.GetUser().Id, organizationValidation.OrganizationIds); + string code = await oauthService.CreateAuthorizationCodeAsync(request, HttpContext.Request.GetUser(), organizationValidation.OrganizationIds); string redirectUri = BuildRedirectUri(request.RedirectUri, code, request.State, GetOrigin()); return HttpResults.Ok(new OAuthAuthorizeResponse { RedirectUri = redirectUri }); } diff --git a/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs b/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs index aef15d8f04..6511f11258 100644 --- a/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs @@ -17,6 +17,7 @@ namespace Exceptionless.Web.Api.Handlers; public class TokenHandler( ITokenRepository repository, + IUserRepository userRepository, IProjectRepository projectRepository, ApiMapper mapper, IAppQueryValidator validator, @@ -266,12 +267,18 @@ private async Task> CreateTokenImplAsync(NewToken value) return null; } - private Task AddModelAsync(Token value) + private async Task AddModelAsync(Token value) { value.Id = StringExtensions.GetNewToken(); value.CreatedUtc = value.UpdatedUtc = timeProvider.GetUtcNow().UtcDateTime; value.Type = TokenType.Access; value.CreatedBy = GetCurrentUserId(); + if (!String.IsNullOrEmpty(value.UserId)) + { + var user = String.Equals(value.UserId, GetCurrentUserId(), StringComparison.Ordinal) ? HttpContext.Request.GetUser() + : await userRepository.GetByIdAsync(value.UserId); + value.AuthenticationVersion = user?.AuthenticationVersion; + } if (value.Scopes.Contains(AuthorizationRoles.GlobalAdmin)) value.Scopes.Add(AuthorizationRoles.User); @@ -279,7 +286,7 @@ private Task AddModelAsync(Token value) if (value.Scopes.Contains(AuthorizationRoles.User)) value.Scopes.Add(AuthorizationRoles.Client); - return repository.AddAsync(value, o => o.Cache()); + return await repository.AddAsync(value, o => o.Cache()); } private async Task CanDeleteAsync(Token value) diff --git a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs index 6dea7c7a46..edecc8755c 100644 --- a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs +++ b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs @@ -129,13 +129,16 @@ protected override async Task HandleAuthenticateAsync() if (!String.IsNullOrEmpty(tokenRecord.UserId)) { - var user = await _userRepository.GetByIdAsync(tokenRecord.UserId, o => o.Cache()); + var user = await _userRepository.GetByIdAsync(tokenRecord.UserId); if (user is null) { Logger.LogInformation("Could not find user {UserId} for token on {Path}", tokenRecord.UserId, Request.Path); return AuthenticateResult.Fail("Token is not valid"); } + if (!user.IsActive || !String.Equals(user.AuthenticationVersion, tokenRecord.AuthenticationVersion, StringComparison.Ordinal)) + return AuthenticateResult.Fail("Token is not valid"); + return AuthenticateResult.Success(CreateUserAuthenticationTicket(user, tokenRecord)); } @@ -239,14 +242,14 @@ private async Task AuthenticateOAuthBearerAsync(string token if (!await _oauthService.IsAccessTokenClientValidAsync(tokenRecord.ClientId)) return AuthenticateResult.Fail("OAuth client is not valid"); - var user = await _userRepository.GetByIdAsync(tokenRecord.UserId, o => o.Cache()); + var user = await _userRepository.GetByIdAsync(tokenRecord.UserId); if (user is null) { Logger.LogInformation("Could not find user {UserId} for OAuth token on {Path}", tokenRecord.UserId, Request.Path); return AuthenticateResult.Fail("Token is not valid"); } - if (!user.IsActive) + if (!user.IsActive || !String.Equals(user.AuthenticationVersion, tokenRecord.AuthenticationVersion, StringComparison.Ordinal)) { await DisableOAuthTokenAsync(tokenRecord); return AuthenticateResult.Fail("User is not valid"); diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index 273d98100f..432e5f5321 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -14130,6 +14130,12 @@ "string" ] }, + "authentication_version": { + "type": [ + "null", + "string" + ] + }, "salt": { "type": [ "null", diff --git a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs index 2bedd9a8bf..58d47d4f2a 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs @@ -1038,6 +1038,47 @@ public async Task BasicAuthentication_MalformedHeader_ReturnsUnauthorized(string Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); } + [Fact] + public async Task AuthenticateAsync_InactiveUserToken_ReturnsUnauthorized() + { + // Arrange + const string email = "inactive-token-user@exceptionless.test"; + const string password = "Password1$"; + const string salt = "1234567890123456"; + + var user = new User + { + EmailAddress = email, + Password = password.ToSaltedHash(salt), + Salt = salt, + FullName = "Inactive Token User", + Roles = AuthorizationRoles.AllScopes + }; + user.MarkEmailAddressVerified(); + await _userRepository.AddAsync(user); + + var login = await SendRequestAsAsync(r => r + .Post() + .AppendPath("auth/login") + .Content(new Login { Email = email, Password = password }) + .StatusCodeShouldBeOk()); + Assert.NotNull(login); + Assert.False(String.IsNullOrEmpty(login.Token)); + + var storedUser = await _userRepository.GetByEmailAddressAsync(email); + Assert.NotNull(storedUser); + await _userRepository.SaveAsync(storedUser with { IsActive = false }, o => o.ImmediateConsistency()); + + // Act + var response = await SendRequestAsync(r => r + .BearerToken(login.Token) + .AppendPath("users/me") + .StatusCodeShouldBeUnauthorized()); + + // Assert + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + [Fact] public async Task BasicAuthentication_PasswordContainingColons_ReturnsCurrentUser() { @@ -1286,6 +1327,39 @@ public async Task PasswordLogin_ThrottlingExpires_LogsInWithoutPasswordResetOrRe Assert.Equal(HttpStatusCode.OK, allowed.StatusCode); } + [Fact] + public async Task ChangePassword_StoredOldTokenSurvivesCleanup_CannotAuthenticate() + { + // Arrange + const string newPassword = "ChangedPassword2$"; + var login = await SendRequestAsAsync(r => r.Post().AppendPath("auth/login") + .Content(new Login { Email = SampleDataService.TEST_USER_EMAIL, Password = SampleDataService.TEST_USER_PASSWORD }) + .StatusCodeShouldBeOk()); + Assert.NotNull(login); + var oldToken = await _tokenRepository.GetByIdAsync(login.Token, o => o.Cache(false)); + Assert.NotNull(oldToken); + await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeOk()); + + // Act + var changed = await SendRequestAsAsync(r => r.Post().AppendPath("auth/change-password") + .BearerToken(login.Token) + .Content(new ChangePasswordModel { CurrentPassword = SampleDataService.TEST_USER_PASSWORD, Password = newPassword }) + .StatusCodeShouldBeOk()); + // Retain a record as a failed cleanup would, after warming the authentication cache. + await _tokenRepository.AddAsync(oldToken, o => o.ImmediateConsistency()); + + // Assert + Assert.NotNull(changed); + Assert.NotEqual(login.Token, changed.Token); + await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); + await SendRequestAsync(r => r.BearerToken(changed.Token).AppendPath("users/me").StatusCodeShouldBeOk()); + await SendRequestAsync(r => r.BasicAuthorization("client", login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(user); + Assert.False(String.IsNullOrEmpty(user.AuthenticationVersion)); + Assert.Equal(user.AuthenticationVersion, (await _tokenRepository.GetByIdAsync(changed.Token))!.AuthenticationVersion); + } + [Fact] public async Task CanChangePasswordAsync() { diff --git a/tests/Exceptionless.Tests/Api/Endpoints/OAuthEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/OAuthEndpointTests.cs index 86dec3f35f..064f099964 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/OAuthEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/OAuthEndpointTests.cs @@ -1835,6 +1835,41 @@ public async Task TokenAsync_ConcurrentAuthorizationCodeUse_OnlyOneSucceeds() Assert.Equal("invalid_grant", error.Error); } + [Fact] + public async Task TokenAsync_AccountGenerationChanged_RejectsExistingCodesAndRefreshTokens() + { + // Arrange + var issued = await IssueTokenAsync(resource: RestApiResource, scope: $"{AuthorizationRoles.ProjectsRead} {AuthorizationRoles.OfflineAccess}"); + string code = await CreateAuthorizationCodeAsync(PkceVerifier); + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(user); + user.AuthenticationVersion = StringExtensions.GetNewToken(); + await _userRepository.SaveAsync(user, o => o.ImmediateConsistency()); + using var client = CreateHttpClient(); + using var request = new HttpRequestMessage(HttpMethod.Get, "projects"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", issued.AccessToken); + using var exchange = CreateTokenExchangeContent(code, PkceVerifier); + using var refresh = new FormUrlEncodedContent(new Dictionary + { + ["grant_type"] = OAuthGrantTypes.RefreshToken, + ["client_id"] = ClientId, + ["refresh_token"] = issued.RefreshToken! + }); + + // Act + using var accessResponse = await client.SendAsync(request, TestCancellationToken); + using var codeResponse = await client.PostAsync("oauth/token", exchange, TestCancellationToken); + using var refreshResponse = await client.PostAsync("oauth/token", refresh, TestCancellationToken); + var stored = await GetStoredOAuthTokenAsync(issued.AccessToken); + + // Assert + Assert.Equal(HttpStatusCode.Unauthorized, accessResponse.StatusCode); + Assert.Equal(HttpStatusCode.BadRequest, codeResponse.StatusCode); + Assert.Equal(HttpStatusCode.BadRequest, refreshResponse.StatusCode); + Assert.NotNull(stored); + Assert.True(stored.IsDisabled); + } + private async Task RemoveTestUserFromOrganizationAsync(string organizationId) { var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index a70baa3de7..ae65279821 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -1,7 +1,9 @@ +using System.Reflection; using Exceptionless.Core; using Exceptionless.Core.Authentication; using Exceptionless.Core.Configuration; using Exceptionless.Core.Mail; +using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; using Exceptionless.Core.Services; using Exceptionless.Web.Api.Handlers; @@ -49,7 +51,33 @@ public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exce new OperationCanceledException("Repository operation was canceled.") }; - private AuthHandler CreateHandler(IUserRepository userRepository) + [Fact] + public async Task ResetUserTokensAsync_OneStoreThrows_StillAttemptsBothStores() + { + // Arrange + var tokens = new Mock(); + var oauthTokens = new Mock(); + tokens.Setup(repository => repository.RemoveAllByUserIdAsync("test-user", null)) + .Throws(new InvalidOperationException("Token store unavailable.")); + oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync("test-user", null)) + .ReturnsAsync(0L); + var handler = CreateHandler(Mock.Of(), tokens.Object, oauthTokens.Object); + var method = typeof(AuthHandler).GetMethod("ResetUserTokensAsync", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(method); + + // Act + var task = Assert.IsAssignableFrom(method.Invoke(handler, + [new User { Id = "test-user", EmailAddress = "user@example.test", AuthenticationVersion = "current" }, "test", new DefaultHttpContext()])); + await task; + + // Assert + tokens.Verify(repository => repository.RemoveAllByUserIdAsync("test-user", null), Times.Once); + oauthTokens.Verify(repository => repository.RemoveAllByUserIdAsync("test-user", null), Times.Once); + tokens.VerifyNoOtherCalls(); + oauthTokens.VerifyNoOtherCalls(); + } + + private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository? tokenRepository = null, IOAuthTokenRepository? oauthTokenRepository = null) { var appOptions = GetService(); @@ -58,8 +86,8 @@ private AuthHandler CreateHandler(IUserRepository userRepository) appOptions.IntercomOptions, GetService(), userRepository, - GetService(), - GetService(), + tokenRepository ?? GetService(), + oauthTokenRepository ?? GetService(), GetService(), GetService(), GetService(), diff --git a/tests/Exceptionless.Tests/Serializer/Models/OAuthTokenSerializerTests.cs b/tests/Exceptionless.Tests/Serializer/Models/OAuthTokenSerializerTests.cs index bf7ff09673..cd746ec394 100644 --- a/tests/Exceptionless.Tests/Serializer/Models/OAuthTokenSerializerTests.cs +++ b/tests/Exceptionless.Tests/Serializer/Models/OAuthTokenSerializerTests.cs @@ -8,6 +8,7 @@ namespace Exceptionless.Tests.Serializer.Models; public class OAuthTokenSerializerTests : TestWithServices { + private const string AuthenticationVersion = "current-version"; private readonly ITextSerializer _serializer; public OAuthTokenSerializerTests(ITestOutputHelper output) : base(output) @@ -15,13 +16,17 @@ public OAuthTokenSerializerTests(ITestOutputHelper output) : base(output) _serializer = GetService(); } - [Fact] - public void RoundTrip_WithOAuthAccessToken_PreservesValues() + [Theory] + [InlineData(null)] + [InlineData(AuthenticationVersion)] + public void RoundTrip_WithOAuthAccessToken_PreservesValues(string? authenticationVersion) { + // Arrange var token = new OAuthToken { Id = "650000000000000000000005", UserId = "660000000000000000000001", + AuthenticationVersion = authenticationVersion, ClientId = "test-oauth-client", GrantId = "test-grant-id", Resource = "http://localhost:7110/api/v2", @@ -36,11 +41,16 @@ public void RoundTrip_WithOAuthAccessToken_PreservesValues() UpdatedUtc = DateTime.UtcNow }; + // Act string? json = _serializer.SerializeToString(token); var result = _serializer.Deserialize(json); + // Assert Assert.NotNull(result); + if (authenticationVersion is null) + Assert.DoesNotContain("authentication_version", json); Assert.Equal("test-oauth-client", result.ClientId); + Assert.Equal(authenticationVersion, result.AuthenticationVersion); Assert.Equal("test-grant-id", result.GrantId); Assert.Equal("http://localhost:7110/api/v2", result.Resource); Assert.Equal(OAuthService.CreateTokenHash("serializer-access-token"), result.AccessTokenHash); diff --git a/tests/Exceptionless.Tests/Serializer/Models/TokenSerializerTests.cs b/tests/Exceptionless.Tests/Serializer/Models/TokenSerializerTests.cs index 9b86475713..fb812a2075 100644 --- a/tests/Exceptionless.Tests/Serializer/Models/TokenSerializerTests.cs +++ b/tests/Exceptionless.Tests/Serializer/Models/TokenSerializerTests.cs @@ -50,9 +50,11 @@ public void RoundTrip_WithAccessToken_PreservesValues() public void RoundTrip_WithUserScopedToken_PreservesValues() { // Arrange + const string authenticationVersion = "current-version"; var token = new Token { Id = "650000000000000000000002", + AuthenticationVersion = authenticationVersion, OrganizationId = "", ProjectId = "", UserId = "660000000000000000000001", @@ -72,6 +74,7 @@ public void RoundTrip_WithUserScopedToken_PreservesValues() // Assert Assert.NotNull(result); Assert.Equal("660000000000000000000001", result.UserId); + Assert.Equal(authenticationVersion, result.AuthenticationVersion); Assert.Equal("540000000000000000000001", result.DefaultProjectId); Assert.Equal(TokenType.Authentication, result.Type); Assert.Equal("refresh_token_abc", result.Refresh); @@ -118,6 +121,7 @@ public void Deserialize_SnakeCaseJson_ParsesCorrectly() // Assert Assert.NotNull(result); Assert.Equal("650000000000000000000004", result.Id); + Assert.Null(result.AuthenticationVersion); Assert.Equal("550000000000000000000001", result.OrganizationId); Assert.Equal(TokenType.Access, result.Type); Assert.Equal(2, result.Scopes.Count); diff --git a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs index 8abd964e15..37d82962e2 100644 --- a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs +++ b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs @@ -230,6 +230,7 @@ public void Deserialize_SnakeCaseJson_PreservesOrganizationIds() // Assert - Critical: organization_ids must be populated Assert.NotNull(user); Assert.Equal("user-from-cache", user.Id); + Assert.Null(user.AuthenticationVersion); Assert.Equal(2, user.OrganizationIds.Count); Assert.Contains("cached-org-1", user.OrganizationIds); Assert.Contains("cached-org-2", user.OrganizationIds); @@ -419,9 +420,11 @@ public void Deserialize_UserWithEmptyCollections_ReturnsEmptyCollections() public void Deserialize_UserWithAllProperties_PreservesAllValues() { // Arrange + const string authenticationVersion = "current-version"; var original = new User { Id = "full-user", + AuthenticationVersion = authenticationVersion, FullName = "Full Test User", EmailAddress = "full@example.com", Password = "hashedpassword", @@ -444,6 +447,7 @@ public void Deserialize_UserWithAllProperties_PreservesAllValues() // Assert Assert.NotNull(deserialized); Assert.Equal("full-user", deserialized.Id); + Assert.Equal(authenticationVersion, deserialized.AuthenticationVersion); Assert.Equal("Full Test User", deserialized.FullName); Assert.Equal("full@example.com", deserialized.EmailAddress); Assert.Equal("hashedpassword", deserialized.Password); From 33490fd76bb7288368373c6d014813063eed8872 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 06:44:47 -0500 Subject: [PATCH 2/9] Revoke displaced user sessions when transferring external login --- .../Api/Handlers/AuthHandler.cs | 4 +- .../Api/Handlers/AuthHandlerTests.cs | 69 ++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index 4265f06900..6307682f4e 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -555,12 +555,14 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit var currentUser = httpContext.Request.GetUser(); if (existingUser is not null) { - if (existingUser.Id != currentUser.Id) + if (!String.Equals(existingUser.Id, currentUser.Id, StringComparison.Ordinal)) { if (!existingUser.RemoveOAuthAccount(userInfo.ProviderName, userInfo.Id)) throw new Exception($"Unable to remove existing oauth account for existing user: {existingUser.EmailAddress}"); + existingUser.AuthenticationVersion = Core.Extensions.StringExtensions.GetNewToken(); await userRepository.SaveAsync(existingUser, o => o.Cache()); + await ResetUserTokensAsync(existingUser, "ExternalLoginTransfer", httpContext); } else { diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index ae65279821..4e0e1731a2 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -1,17 +1,22 @@ using System.Reflection; +using System.Security.Claims; using Exceptionless.Core; using Exceptionless.Core.Authentication; using Exceptionless.Core.Configuration; +using Exceptionless.Core.Extensions; using Exceptionless.Core.Mail; using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; using Exceptionless.Core.Services; using Exceptionless.Web.Api.Handlers; using Exceptionless.Web.Api.Messages; +using Exceptionless.Web.Extensions; using Exceptionless.Web.Models; using Exceptionless.Web.Security; using Foundatio.Caching; using Foundatio.Mediator; +using Foundatio.Repositories; +using Foundatio.Repositories.Models; using Microsoft.AspNetCore.Http; using Moq; using Xunit; @@ -22,6 +27,66 @@ public sealed class AuthHandlerTests : TestWithServices { public AuthHandlerTests(ITestOutputHelper output) : base(output) { } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Handle_ExternalLoginTransfer_RevokesDisplacedUserSessionsBeforeLinking(bool cleanupFails) + { + // Arrange + const string provider = "github"; + const string providerId = "provider-user"; + const string previousVersion = "previous-version"; + var displaced = new User { Id = "displaced-user", EmailAddress = "displaced@example.test", AuthenticationVersion = previousVersion }; + displaced.AddOAuthAccount(provider, providerId, displaced.EmailAddress); + displaced.AddOAuthAccount("google", "retained-provider", displaced.EmailAddress); + var current = new User { Id = "current-user", EmailAddress = "current@example.test", AuthenticationVersion = "current-version" }; + var context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)) }; + context.Request.SetUser(current); + var calls = new List(); + string? persistedVersion = null; + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetUserByOAuthProviderAsync(provider, providerId)).ReturnsAsync(displaced); + users.Setup(repository => repository.SaveAsync(displaced, It.IsAny>())) + .Callback(() => { persistedVersion = displaced.AuthenticationVersion; calls.Add("save-displaced"); }).ReturnsAsync(displaced); + users.Setup(repository => repository.SaveAsync(current, It.IsAny>())) + .Callback(() => calls.Add("save-current")).ReturnsAsync(current); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.RemoveAllByUserIdAsync(displaced.Id, null)).Returns(() => + { + calls.Add("remove-tokens"); + return cleanupFails ? Task.FromException(new IOException("Token store unavailable.")) : Task.FromResult(1L); + }); + tokens.Setup(repository => repository.GetByTypeAndUserIdAsync(TokenType.Authentication, current.Id, null)).ReturnsAsync(FindResults.Empty); + tokens.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())) + .ReturnsAsync((Token token, CommandOptionsDescriptor _) => token); + var oauthTokens = new Mock(MockBehavior.Strict); + oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync(displaced.Id, null)) + .Callback(() => calls.Add("remove-oauth-tokens")).ReturnsAsync(1L); + var providerClient = new Mock(MockBehavior.Strict); + providerClient.Setup(client => client.GetGitHubUserInfoAsync(It.IsAny(), "test-client", "test-secret")) + .ReturnsAsync(new OAuth2.Models.UserInfo { ProviderName = provider, Id = providerId, Email = displaced.EmailAddress }); + var options = GetService().AuthOptions; + options.GitHubId = "test-client"; + options.GitHubSecret = "test-secret"; + var handler = CreateHandler(users.Object, tokens.Object, oauthTokens.Object, providerClient.Object); + + // Act + var result = await handler.Handle(new GitHubLogin(new ExternalAuthInfo(), context)); + + // Assert + Assert.Equal(ResultStatus.Success, result.Status); + Assert.False(String.IsNullOrEmpty(persistedVersion)); + Assert.False(String.Equals(previousVersion, persistedVersion, StringComparison.Ordinal)); + Assert.Equal(["save-displaced", "remove-tokens", "remove-oauth-tokens", "save-current"], calls); + Assert.DoesNotContain(displaced.OAuthAccounts, account => String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal)); + Assert.Contains(current.OAuthAccounts, account => String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal)); + Assert.Equal("current-version", current.AuthenticationVersion); + users.VerifyAll(); + tokens.VerifyAll(); + oauthTokens.VerifyAll(); + providerClient.VerifyAll(); + } + [Theory] [MemberData(nameof(LoginRepositoryExceptions))] public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exception exception) @@ -77,7 +142,7 @@ public async Task ResetUserTokensAsync_OneStoreThrows_StillAttemptsBothStores() oauthTokens.VerifyNoOtherCalls(); } - private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository? tokenRepository = null, IOAuthTokenRepository? oauthTokenRepository = null) + private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository? tokenRepository = null, IOAuthTokenRepository? oauthTokenRepository = null, IOAuthProviderClient? providerClient = null) { var appOptions = GetService(); @@ -88,7 +153,7 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito userRepository, tokenRepository ?? GetService(), oauthTokenRepository ?? GetService(), - GetService(), + providerClient ?? GetService(), GetService(), GetService(), GetService(), From 8f45631ec8619901eb979c64e7d9ee866577fabb Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 06:49:29 -0500 Subject: [PATCH 3/9] Make authentication test dependencies explicit and cover legacy transfer --- .../Api/Handlers/AuthHandlerTests.cs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 4e0e1731a2..023a33adb3 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -28,14 +28,15 @@ public sealed class AuthHandlerTests : TestWithServices public AuthHandlerTests(ITestOutputHelper output) : base(output) { } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task Handle_ExternalLoginTransfer_RevokesDisplacedUserSessionsBeforeLinking(bool cleanupFails) + [InlineData(false, null)] + [InlineData(true, null)] + [InlineData(false, "previous-version")] + [InlineData(true, "previous-version")] + public async Task Handle_ExternalLoginTransfer_RevokesDisplacedUserSessionsBeforeLinking(bool cleanupFails, string? previousVersion) { // Arrange const string provider = "github"; const string providerId = "provider-user"; - const string previousVersion = "previous-version"; var displaced = new User { Id = "displaced-user", EmailAddress = "displaced@example.test", AuthenticationVersion = previousVersion }; displaced.AddOAuthAccount(provider, providerId, displaced.EmailAddress); displaced.AddOAuthAccount("google", "retained-provider", displaced.EmailAddress); @@ -95,7 +96,7 @@ public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exce var userRepository = new Mock(); userRepository.Setup(repository => repository.GetByEmailAddressAsync("test@example.com")) .ThrowsAsync(exception); - var handler = CreateHandler(userRepository.Object); + var handler = CreateHandler(userRepository.Object, GetService(), GetService(), GetService()); var message = new LoginMessage( new Login { Email = "test@example.com", Password = "password" }, new DefaultHttpContext()); @@ -126,7 +127,7 @@ public async Task ResetUserTokensAsync_OneStoreThrows_StillAttemptsBothStores() .Throws(new InvalidOperationException("Token store unavailable.")); oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync("test-user", null)) .ReturnsAsync(0L); - var handler = CreateHandler(Mock.Of(), tokens.Object, oauthTokens.Object); + var handler = CreateHandler(Mock.Of(), tokens.Object, oauthTokens.Object, GetService()); var method = typeof(AuthHandler).GetMethod("ResetUserTokensAsync", BindingFlags.Instance | BindingFlags.NonPublic); Assert.NotNull(method); @@ -142,7 +143,7 @@ public async Task ResetUserTokensAsync_OneStoreThrows_StillAttemptsBothStores() oauthTokens.VerifyNoOtherCalls(); } - private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository? tokenRepository = null, IOAuthTokenRepository? oauthTokenRepository = null, IOAuthProviderClient? providerClient = null) + private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository tokenRepository, IOAuthTokenRepository oauthTokenRepository, IOAuthProviderClient providerClient) { var appOptions = GetService(); @@ -151,9 +152,9 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito appOptions.IntercomOptions, GetService(), userRepository, - tokenRepository ?? GetService(), - oauthTokenRepository ?? GetService(), - providerClient ?? GetService(), + tokenRepository, + oauthTokenRepository, + providerClient, GetService(), GetService(), GetService(), From 051996d217d0694f171fc702729fc1bd5c7249c5 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 06:51:52 -0500 Subject: [PATCH 4/9] Document session generation lifecycle and organize model properties --- src/Exceptionless.Core/Models/OAuthToken.cs | 50 ++++++++------ src/Exceptionless.Core/Models/Token.cs | 49 +++++++++----- src/Exceptionless.Core/Models/User.cs | 74 +++++++++++++-------- 3 files changed, 109 insertions(+), 64 deletions(-) diff --git a/src/Exceptionless.Core/Models/OAuthToken.cs b/src/Exceptionless.Core/Models/OAuthToken.cs index 32b31c86f1..941bb3b3bf 100644 --- a/src/Exceptionless.Core/Models/OAuthToken.cs +++ b/src/Exceptionless.Core/Models/OAuthToken.cs @@ -7,48 +7,60 @@ namespace Exceptionless.Core.Models; public class OAuthToken : IIdentity, IHaveDates, IValidatableObject { [Required] - [ObjectId] - public string Id { get; set; } = null!; + [MaxLength(100)] + public string AccessTokenHash { get; set; } = null!; - [Required] - [ObjectId] - public string UserId { get; set; } = null!; + /// + /// The user session generation captured when this OAuth token is issued. + /// A legacy null value matches only an active user whose generation is also null; it does not describe a password format. + /// public string? AuthenticationVersion { get; set; } [Required] [MaxLength(2048)] public string ClientId { get; set; } = null!; + [Required] + [ObjectId] + public string CreatedBy { get; set; } = null!; + + public DateTime CreatedUtc { get; set; } + + public DateTime? ExpiresUtc { get; set; } + [Required] [MaxLength(100)] public string GrantId { get; set; } = null!; [Required] - [MaxLength(2048)] - public string Resource { get; set; } = null!; + [ObjectId] + public string Id { get; set; } = null!; - [Required] - [MaxLength(100)] - public string AccessTokenHash { get; set; } = null!; + public bool IsDisabled { get; set; } + + public bool IsSuspended { get; set; } + + public HashSet OrganizationIds { get; set; } = new(StringComparer.Ordinal); + + public DateTime? RefreshExpiresUtc { get; set; } [MaxLength(100)] public string? RefreshTokenHash { get; set; } - public DateTime? ExpiresUtc { get; set; } - public DateTime? RefreshExpiresUtc { get; set; } public DateTime? RefreshTokenUsedUtc { get; set; } - public HashSet OrganizationIds { get; set; } = new(StringComparer.Ordinal); - public HashSet Scopes { get; set; } = new(StringComparer.Ordinal); - public bool IsDisabled { get; set; } - public bool IsSuspended { get; set; } [Required] - [ObjectId] - public string CreatedBy { get; set; } = null!; + [MaxLength(2048)] + public string Resource { get; set; } = null!; + + public HashSet Scopes { get; set; } = new(StringComparer.Ordinal); - public DateTime CreatedUtc { get; set; } public DateTime UpdatedUtc { get; set; } + [Required] + [ObjectId] + public string UserId { get; set; } = null!; + public IEnumerable Validate(ValidationContext validationContext) { if (CreatedUtc == DateTime.MinValue) diff --git a/src/Exceptionless.Core/Models/Token.cs b/src/Exceptionless.Core/Models/Token.cs index d5ec4ea4e4..477ad98210 100644 --- a/src/Exceptionless.Core/Models/Token.cs +++ b/src/Exceptionless.Core/Models/Token.cs @@ -6,9 +6,33 @@ namespace Exceptionless.Core.Models; public class Token : IOwnedByOrganizationAndProjectWithIdentity, IHaveDates, IValidatableObject { + /// + /// The user session generation captured when this token is issued. + /// A legacy null value matches only an active user whose generation is also null; it does not describe a password format. + /// + public string? AuthenticationVersion { get; set; } + + public string CreatedBy { get; set; } = null!; + + public DateTime CreatedUtc { get; set; } + + /// + /// The user's preferred default project. Cannot be set when ProjectId is defined. + /// + [ObjectId] + public string? DefaultProjectId { get; set; } + + public DateTime? ExpiresUtc { get; set; } + [Required] public string Id { get; set; } = null!; + public bool IsDisabled { get; set; } + + public bool IsSuspended { get; set; } + + public string? Notes { get; set; } + /// /// Null for user-scoped tokens (where UserId is set instead). /// Required when ProjectId is set. @@ -23,29 +47,20 @@ public class Token : IOwnedByOrganizationAndProjectWithIdentity, IHaveDates, IVa [ObjectId] public string ProjectId { get; set; } = null!; + public string? Refresh { get; set; } + + public HashSet Scopes { get; set; } = new(); + + public TokenType Type { get; set; } + + public DateTime UpdatedUtc { get; set; } + /// /// Set for user-scoped tokens. Cannot be set together with ProjectId. /// [ObjectId] public string? UserId { get; set; } - /// - /// The user's preferred default project. Cannot be set when ProjectId is defined. - /// - [ObjectId] - public string? DefaultProjectId { get; set; } - public string? Refresh { get; set; } - public string? AuthenticationVersion { get; set; } - public TokenType Type { get; set; } - public HashSet Scopes { get; set; } = new(); - public DateTime? ExpiresUtc { get; set; } - public string? Notes { get; set; } - public bool IsDisabled { get; set; } - public bool IsSuspended { get; set; } - public string CreatedBy { get; set; } = null!; - public DateTime CreatedUtc { get; set; } - public DateTime UpdatedUtc { get; set; } - public IEnumerable Validate(ValidationContext validationContext) { // OrganizationId is required for all non-user-scoped tokens. diff --git a/src/Exceptionless.Core/Models/User.cs b/src/Exceptionless.Core/Models/User.cs index 7786c21019..11b729c08d 100644 --- a/src/Exceptionless.Core/Models/User.cs +++ b/src/Exceptionless.Core/Models/User.cs @@ -8,26 +8,25 @@ namespace Exceptionless.Core.Models; public record User : IIdentity, IHaveDates, IValidatableObject { - /// - /// Unique id that identifies an user. - /// - [ObjectId] - public string Id { get; set; } = null!; + public bool? AssistantConversationSharingEnabled { get; set; } /// - /// The organizations that the user has access to. + /// Session revocation generation, rotated when a password changes or resets, or an external login is removed or transferred away. + /// Remains null until rotation; active users with a null generation accept only matching legacy null token generations. + /// This value does not describe the password hashing format. /// - public ISet OrganizationIds { get; init; } = new HashSet(); - - public string? Password { get; set; } public string? AuthenticationVersion { get; set; } - public string? Salt { get; set; } - public string? PasswordResetToken { get; set; } - public DateTime PasswordResetTokenExpiration { get; set; } - public ICollection OAuthAccounts { get; init; } = new Collection(); - public ICollection OrganizationPreferences { get; init; } = new Collection(); - public ICollection SavedViewOrders { get; init; } = new Collection(); - public IDictionary ProductTours { get; init; } = new Dictionary(StringComparer.Ordinal); + + [StringLength(2000)] + public string? AvatarFileName { get; set; } + + public DateTime CreatedUtc { get; set; } + + [Required] + [EmailAddress] + public string EmailAddress { get; set; } = null!; + + public bool EmailNotificationsEnabled { get; set; } = true; /// /// Gets or sets the users Full Name. @@ -35,29 +34,48 @@ public record User : IIdentity, IHaveDates, IValidatableObject [Required] public string FullName { get; set; } = null!; - [Required] - [EmailAddress] - public string EmailAddress { get; set; } = null!; + /// + /// Unique id that identifies an user. + /// + [ObjectId] + public string Id { get; set; } = null!; - [StringLength(2000)] - public string? AvatarFileName { get; set; } + /// + /// Gets or sets the users active state. + /// + public bool IsActive { get; init; } = true; - public bool EmailNotificationsEnabled { get; set; } = true; - public bool? AssistantConversationSharingEnabled { get; set; } public bool IsEmailAddressVerified { get; set; } - public string? VerifyEmailAddressToken { get; set; } - public DateTime VerifyEmailAddressTokenExpiration { get; set; } + + public ICollection OAuthAccounts { get; init; } = new Collection(); /// - /// Gets or sets the users active state. + /// The organizations that the user has access to. /// - public bool IsActive { get; init; } = true; + public ISet OrganizationIds { get; init; } = new HashSet(); + + public ICollection OrganizationPreferences { get; init; } = new Collection(); + + public string? Password { get; set; } + + public string? PasswordResetToken { get; set; } + + public DateTime PasswordResetTokenExpiration { get; set; } + + public IDictionary ProductTours { get; init; } = new Dictionary(StringComparer.Ordinal); public ISet Roles { get; init; } = new HashSet(); - public DateTime CreatedUtc { get; set; } + public string? Salt { get; set; } + + public ICollection SavedViewOrders { get; init; } = new Collection(); + public DateTime UpdatedUtc { get; set; } + public string? VerifyEmailAddressToken { get; set; } + + public DateTime VerifyEmailAddressTokenExpiration { get; set; } + public IEnumerable Validate(ValidationContext validationContext) { if (IsEmailAddressVerified) From af8856d652d43e398f2d7c527a9463e6b4693096 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 07:05:07 -0500 Subject: [PATCH 5/9] Prove user cache race and simplify session generation validation --- .../Extensions/UserExtensions.cs | 14 +- .../Services/OAuthService.cs | 19 +-- .../Api/Handlers/AuthHandler.cs | 10 +- .../Api/Handlers/TokenHandler.cs | 11 +- .../Security/ApiKeyAuthenticationHandler.cs | 4 +- .../Exceptionless.Tests/Api/Data/openapi.json | 149 +++++++++--------- .../Api/Handlers/AuthHandlerTests.cs | 62 ++++++++ .../ApiKeyAuthenticationHandlerTests.cs | 54 +++++++ .../OAuthAuthorizationValidationTests.cs | 55 +++++++ 9 files changed, 276 insertions(+), 102 deletions(-) diff --git a/src/Exceptionless.Core/Extensions/UserExtensions.cs b/src/Exceptionless.Core/Extensions/UserExtensions.cs index 31691cdb38..a04982adbc 100644 --- a/src/Exceptionless.Core/Extensions/UserExtensions.cs +++ b/src/Exceptionless.Core/Extensions/UserExtensions.cs @@ -1,10 +1,22 @@ -using Exceptionless.Core.Models; +using System.Diagnostics.CodeAnalysis; +using Exceptionless.Core.Models; using Exceptionless.DateTimeExtensions; namespace Exceptionless.Core.Extensions; public static class UserExtensions { + /// + /// Checks whether an active user's session generation matches the presented credential. + /// A missing generation matches only another missing generation for legacy credentials. + /// This method does not load or refresh the user. + /// + public static bool HasValidAuthenticationVersion([NotNullWhen(true)] this User? user, string? authenticationVersion) + { + return user is { IsActive: true } + && String.Equals(user.AuthenticationVersion, authenticationVersion, StringComparison.Ordinal); + } + public static bool IsCorrectPassword(this User user, string password) { if (String.IsNullOrEmpty(user.Salt) || String.IsNullOrEmpty(user.Password)) diff --git a/src/Exceptionless.Core/Services/OAuthService.cs b/src/Exceptionless.Core/Services/OAuthService.cs index 4f0ecd9b12..39e815f6b7 100644 --- a/src/Exceptionless.Core/Services/OAuthService.cs +++ b/src/Exceptionless.Core/Services/OAuthService.cs @@ -383,13 +383,6 @@ public async Task ValidateAuthorizationRequestAsync(OAuth return OAuthValidationResult.Valid(client, requestedScopes); } - public async Task CreateAuthorizationCodeAsync(OAuthAuthorizeRequest request, string userId, IReadOnlyCollection organizationIds) - { - var user = await userRepository.GetByIdAsync(userId) - ?? throw new InvalidOperationException("The user does not exist."); - return await CreateAuthorizationCodeAsync(request, user, organizationIds); - } - public async Task CreateAuthorizationCodeAsync(OAuthAuthorizeRequest request, User user, IReadOnlyCollection organizationIds) { await oauthApplicationRepository.AddOrganizationIdsAsync(request.ClientId, organizationIds, o => o.ImmediateConsistency().Notifications(false)); @@ -441,7 +434,7 @@ public async Task ExchangeAuthorizationCodeAsync(OAuthTok return OAuthTokenIssueResult.Invalid("invalid_grant", "Invalid PKCE verifier."); var user = await userRepository.GetByIdAsync(code.UserId); - if (user is not { IsActive: true } || !String.Equals(user.AuthenticationVersion, code.AuthenticationVersion, StringComparison.Ordinal)) + if (!user.HasValidAuthenticationVersion(code.AuthenticationVersion)) return OAuthTokenIssueResult.Invalid("invalid_grant", "Authorization code is invalid or expired."); return OAuthTokenIssueResult.Success(await CreateTokenAsync(code.UserId, code.ClientId, code.Resource, code.Scopes, code.OrganizationIds, authenticationVersion: code.AuthenticationVersion)); @@ -486,7 +479,7 @@ public async Task RefreshAsync(OAuthTokenRequest request) } var user = await userRepository.GetByIdAsync(token.UserId); - if (user is null || !user.IsActive || !String.Equals(user.AuthenticationVersion, token.AuthenticationVersion, StringComparison.Ordinal)) + if (!user.HasValidAuthenticationVersion(token.AuthenticationVersion)) { await RevokeOAuthGrantFamilyAsync(token); return OAuthTokenIssueResult.Invalid("invalid_grant", "Refresh token is invalid."); @@ -878,13 +871,13 @@ public record OAuthAuthorizationCode { public string? AuthenticationVersion { get; init; } public required string ClientId { get; init; } - public required string RedirectUri { get; init; } - public required string UserId { get; init; } public required string CodeChallenge { get; init; } - public required string Resource { get; init; } - public IReadOnlyCollection Scopes { get; set; } = []; public DateTime CreatedUtc { get; init; } public IReadOnlyCollection OrganizationIds { get; init; } = []; + public required string RedirectUri { get; init; } + public required string Resource { get; init; } + public IReadOnlyCollection Scopes { get; set; } = []; + public required string UserId { get; init; } } diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index 6307682f4e..83c3b21456 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -504,6 +504,7 @@ private async Task AddGlobalAdminRoleIfFirstUserAsync(User user) private async Task> ExternalLoginAsync(ExternalAuthInfo authInfo, HttpContext httpContext, string? appId, string? appSecret, Func> getUserInfoAsync) { using var _ = logger.BeginScope(new ExceptionlessState().Tag("External Login").SetHttpContext(httpContext)); + if (String.IsNullOrEmpty(appId) || String.IsNullOrEmpty(appSecret)) throw new ConfigurationErrorsException("Missing Configuration for OAuth provider"); @@ -626,6 +627,7 @@ private async Task AddInvitedUserToOrganizationAsync(string? token, User user, H return; using var _ = logger.BeginScope(new ExceptionlessState().Tag("Invite").Identity(user.EmailAddress).SetHttpContext(httpContext)); + var organization = await organizationRepository.GetByInviteTokenAsync(token); var invite = organization?.GetInvite(token); if (organization is null || invite is null) @@ -655,6 +657,7 @@ private async Task AddInvitedUserToOrganizationAsync(string? token, User user, H private async Task ChangePasswordAsync(User user, string password, string tag, HttpContext httpContext) { using var _ = logger.BeginScope(new ExceptionlessState().Tag(tag).Identity(user.EmailAddress).SetHttpContext(httpContext)); + if (String.IsNullOrEmpty(user.Salt)) user.Salt = Core.Extensions.StringExtensions.GetNewToken(); @@ -677,6 +680,7 @@ private async Task ChangePasswordAsync(User user, string password, string tag, H private async Task ResetUserTokensAsync(User user, string tag, HttpContext httpContext) { using var _ = logger.BeginScope(new ExceptionlessState().Tag(tag).Identity(user.EmailAddress).SetHttpContext(httpContext)); + long total = 0; try { @@ -684,7 +688,7 @@ private async Task ResetUserTokensAsync(User user, string tag, HttpContext httpC } catch (Exception ex) { - logger.LogError(ex, "Error removing user tokens for {UserId}: {Message}", user.Id, ex.Message); + logger.LogCritical(ex, "Error removing user tokens for {EmailAddress} ({UserId}): {Message}", user.EmailAddress, user.Id, ex.Message); } try @@ -693,7 +697,7 @@ private async Task ResetUserTokensAsync(User user, string tag, HttpContext httpC } catch (Exception ex) { - logger.LogError(ex, "Error removing OAuth tokens for {UserId}: {Message}", user.Id, ex.Message); + logger.LogCritical(ex, "Error removing OAuth tokens for {EmailAddress} ({UserId}): {Message}", user.EmailAddress, user.Id, ex.Message); } logger.RemovedUserTokens(total, user.EmailAddress); @@ -705,7 +709,7 @@ private async Task GetOrCreateAuthenticationTokenAsync(User user) var utcNow = timeProvider.GetUtcNow().UtcDateTime; var validAccessToken = userTokens.Documents.FirstOrDefault(token => !token.IsDisabled && !token.IsSuspended - && String.Equals(token.AuthenticationVersion, user.AuthenticationVersion, StringComparison.Ordinal) + && user.HasValidAuthenticationVersion(token.AuthenticationVersion) && (!token.ExpiresUtc.HasValue || token.ExpiresUtc > utcNow)); if (validAccessToken is not null) return validAccessToken.Id; diff --git a/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs b/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs index 6511f11258..aef15d8f04 100644 --- a/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/TokenHandler.cs @@ -17,7 +17,6 @@ namespace Exceptionless.Web.Api.Handlers; public class TokenHandler( ITokenRepository repository, - IUserRepository userRepository, IProjectRepository projectRepository, ApiMapper mapper, IAppQueryValidator validator, @@ -267,18 +266,12 @@ private async Task> CreateTokenImplAsync(NewToken value) return null; } - private async Task AddModelAsync(Token value) + private Task AddModelAsync(Token value) { value.Id = StringExtensions.GetNewToken(); value.CreatedUtc = value.UpdatedUtc = timeProvider.GetUtcNow().UtcDateTime; value.Type = TokenType.Access; value.CreatedBy = GetCurrentUserId(); - if (!String.IsNullOrEmpty(value.UserId)) - { - var user = String.Equals(value.UserId, GetCurrentUserId(), StringComparison.Ordinal) ? HttpContext.Request.GetUser() - : await userRepository.GetByIdAsync(value.UserId); - value.AuthenticationVersion = user?.AuthenticationVersion; - } if (value.Scopes.Contains(AuthorizationRoles.GlobalAdmin)) value.Scopes.Add(AuthorizationRoles.User); @@ -286,7 +279,7 @@ private async Task AddModelAsync(Token value) if (value.Scopes.Contains(AuthorizationRoles.User)) value.Scopes.Add(AuthorizationRoles.Client); - return await repository.AddAsync(value, o => o.Cache()); + return repository.AddAsync(value, o => o.Cache()); } private async Task CanDeleteAsync(Token value) diff --git a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs index edecc8755c..98116af59e 100644 --- a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs +++ b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs @@ -136,7 +136,7 @@ protected override async Task HandleAuthenticateAsync() return AuthenticateResult.Fail("Token is not valid"); } - if (!user.IsActive || !String.Equals(user.AuthenticationVersion, tokenRecord.AuthenticationVersion, StringComparison.Ordinal)) + if (!user.HasValidAuthenticationVersion(tokenRecord.AuthenticationVersion)) return AuthenticateResult.Fail("Token is not valid"); return AuthenticateResult.Success(CreateUserAuthenticationTicket(user, tokenRecord)); @@ -249,7 +249,7 @@ private async Task AuthenticateOAuthBearerAsync(string token return AuthenticateResult.Fail("Token is not valid"); } - if (!user.IsActive || !String.Equals(user.AuthenticationVersion, tokenRecord.AuthenticationVersion, StringComparison.Ordinal)) + if (!user.HasValidAuthenticationVersion(tokenRecord.AuthenticationVersion)) { await DisableOAuthTokenAsync(tokenRecord); return AuthenticateResult.Fail("User is not valid"); diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index 432e5f5321..33a401951e 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -14090,130 +14090,113 @@ }, "User": { "required": [ - "full_name", "email_address", + "full_name", + "created_utc", + "email_notifications_enabled", "id", - "organization_ids", - "password_reset_token_expiration", + "is_active", + "is_email_address_verified", "o_auth_accounts", + "organization_ids", "organization_preferences", - "saved_view_orders", + "password_reset_token_expiration", "product_tours", - "email_notifications_enabled", - "is_email_address_verified", - "verify_email_address_token_expiration", - "is_active", "roles", - "created_utc", - "updated_utc" + "saved_view_orders", + "updated_utc", + "verify_email_address_token_expiration" ], "type": "object", "properties": { - "id": { - "maxLength": 24, - "minLength": 24, - "pattern": "^[a-fA-F0-9]{24}$", - "type": "string", - "description": "Unique id that identifies an user." - }, - "organization_ids": { - "uniqueItems": true, - "type": "array", - "items": { - "type": "string" - }, - "description": "The organizations that the user has access to." - }, - "password": { + "assistant_conversation_sharing_enabled": { "type": [ "null", - "string" + "boolean" ] }, "authentication_version": { "type": [ "null", "string" - ] - }, - "salt": { - "type": [ - "null", - "string" - ] + ], + "description": "Session revocation generation, rotated when a password changes or resets, or an external login is removed or transferred away.\nRemains null until rotation; active users with a null generation accept only matching legacy null token generations.\nThis value does not describe the password hashing format." }, - "password_reset_token": { + "avatar_file_name": { + "maxLength": 2000, + "minLength": 0, "type": [ "null", "string" ] }, - "password_reset_token_expiration": { + "created_utc": { "type": "string", "format": "date-time" }, + "email_address": { + "type": "string", + "format": "email" + }, + "email_notifications_enabled": { + "type": "boolean" + }, + "full_name": { + "type": "string", + "description": "Gets or sets the users Full Name." + }, + "id": { + "maxLength": 24, + "minLength": 24, + "pattern": "^[a-fA-F0-9]{24}$", + "type": "string", + "description": "Unique id that identifies an user." + }, + "is_active": { + "type": "boolean", + "description": "Gets or sets the users active state." + }, + "is_email_address_verified": { + "type": "boolean" + }, "o_auth_accounts": { "type": "array", "items": { "$ref": "#/components/schemas/OAuthAccount" } }, - "organization_preferences": { + "organization_ids": { + "uniqueItems": true, "type": "array", "items": { - "$ref": "#/components/schemas/UserOrganizationPreference" - } + "type": "string" + }, + "description": "The organizations that the user has access to." }, - "saved_view_orders": { + "organization_preferences": { "type": "array", "items": { - "$ref": "#/components/schemas/UserSavedViewOrderPreference" + "$ref": "#/components/schemas/UserOrganizationPreference" } }, - "product_tours": { - "type": "object" - }, - "full_name": { - "type": "string", - "description": "Gets or sets the users Full Name." - }, - "email_address": { - "type": "string", - "format": "email" - }, - "avatar_file_name": { - "maxLength": 2000, - "minLength": 0, + "password": { "type": [ "null", "string" ] }, - "email_notifications_enabled": { - "type": "boolean" - }, - "assistant_conversation_sharing_enabled": { - "type": [ - "null", - "boolean" - ] - }, - "is_email_address_verified": { - "type": "boolean" - }, - "verify_email_address_token": { + "password_reset_token": { "type": [ "null", "string" ] }, - "verify_email_address_token_expiration": { + "password_reset_token_expiration": { "type": "string", "format": "date-time" }, - "is_active": { - "type": "boolean", - "description": "Gets or sets the users active state." + "product_tours": { + "type": "object" }, "roles": { "uniqueItems": true, @@ -14222,11 +14205,29 @@ "type": "string" } }, - "created_utc": { + "salt": { + "type": [ + "null", + "string" + ] + }, + "saved_view_orders": { + "type": "array", + "items": { + "$ref": "#/components/schemas/UserSavedViewOrderPreference" + } + }, + "updated_utc": { "type": "string", "format": "date-time" }, - "updated_utc": { + "verify_email_address_token": { + "type": [ + "null", + "string" + ] + }, + "verify_email_address_token_expiration": { "type": "string", "format": "date-time" } diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 023a33adb3..607bc7d563 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -1,5 +1,8 @@ using System.Reflection; using System.Security.Claims; +using System.Text; +using Elastic.Clients.Elasticsearch; +using Elastic.Transport; using Exceptionless.Core; using Exceptionless.Core.Authentication; using Exceptionless.Core.Configuration; @@ -7,6 +10,7 @@ using Exceptionless.Core.Mail; using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; +using Exceptionless.Core.Repositories.Configuration; using Exceptionless.Core.Services; using Exceptionless.Web.Api.Handlers; using Exceptionless.Web.Api.Messages; @@ -16,6 +20,7 @@ using Foundatio.Caching; using Foundatio.Mediator; using Foundatio.Repositories; +using Foundatio.Repositories.Elasticsearch.Configuration; using Foundatio.Repositories.Models; using Microsoft.AspNetCore.Http; using Moq; @@ -27,6 +32,52 @@ public sealed class AuthHandlerTests : TestWithServices { public AuthHandlerTests(ITestOutputHelper output) : base(output) { } + [Fact] + public async Task GetByIdAsync_ReadCompletesAfterSave_RepopulatesOldAuthenticationGeneration() + { + // Arrange + const string id = "123456789012345678901234"; + var readStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseRead = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + int gets = 0; + var invoker = new CacheRaceInvoker(async endpoint => + { + if (endpoint.Method == Elastic.Transport.HttpMethod.GET) + { + Interlocked.Increment(ref gets); + readStarted.TrySetResult(); + await releaseRead.Task; + return "{\"_index\":\"users-v1\",\"_id\":\"" + id + "\",\"found\":true,\"_source\":{\"id\":\"" + id + "\",\"emailAddress\":\"user@example.test\",\"authenticationVersion\":\"old\"}}"; + } + Assert.Equal(Elastic.Transport.HttpMethod.PUT, endpoint.Method); + return "{\"_index\":\"users-v1\",\"_id\":\"" + id + "\",\"_version\":2,\"result\":\"updated\",\"_shards\":{\"total\":1,\"successful\":1,\"failed\":0},\"_seq_no\":2,\"_primary_term\":1}"; + }); + var client = new ElasticsearchClient(new ElasticsearchClientSettings(new SingleNodePool(new Uri("http://localhost:9200")), invoker).DisablePing()); + var configuration = GetService(); + typeof(ElasticConfiguration).GetField("_client", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(configuration, new Lazy(() => client)); + typeof(Foundatio.Repositories.Elasticsearch.Configuration.Index).GetField("_isEnsured", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(configuration.Users, true); + var repository = GetService(); + var current = new User { Id = id, EmailAddress = "user@example.test", FullName = "Cache Test", AuthenticationVersion = "new" }; + + // Act + var delayedRead = repository.GetByIdAsync(id, o => o.Cache()); + await readStarted.Task.WaitAsync(TimeSpan.FromSeconds(10), TestCancellationToken); + await repository.SaveAsync(current, o => o.Cache().Originals(false).Validation(false).Notifications(false)); + var immediatelyAfterSave = await repository.GetByIdAsync(id, o => o.Cache()); + var emailImmediatelyAfterSave = await repository.GetByEmailAddressAsync(current.EmailAddress); + releaseRead.TrySetResult(); + await delayedRead; + var afterDelayedRead = await repository.GetByIdAsync(id, o => o.Cache()); + var emailAfterDelayedRead = await repository.GetByEmailAddressAsync(current.EmailAddress); + + // Assert + Assert.Equal("new", immediatelyAfterSave!.AuthenticationVersion); + Assert.Equal("new", emailImmediatelyAfterSave!.AuthenticationVersion); + Assert.Equal("old", afterDelayedRead!.AuthenticationVersion); + Assert.Equal("old", emailAfterDelayedRead!.AuthenticationVersion); + Assert.Equal(1, gets); + } + [Theory] [InlineData(false, null)] [InlineData(true, null)] @@ -162,4 +213,15 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito TimeProvider, Log.CreateLogger()); } + + private sealed class CacheRaceInvoker(Func> respond) + : InMemoryRequestInvoker(null, 200, null, "application/json", new Dictionary> { ["x-elastic-product"] = ["Elasticsearch"] }), IRequestInvoker + { + async Task IRequestInvoker.RequestAsync(Elastic.Transport.Endpoint endpoint, BoundConfiguration boundConfiguration, PostData? postData, CancellationToken cancellationToken) + { + string json = await respond(endpoint); + return await BuildResponseAsync(endpoint, boundConfiguration, postData, cancellationToken, Encoding.UTF8.GetBytes(json), 200); + } + } + } diff --git a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs index 3a223e9ebc..4f3c4272e4 100644 --- a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs @@ -9,6 +9,7 @@ using Exceptionless.Core.Services; using Exceptionless.Web.Security; using Foundatio.Caching; +using Foundatio.Repositories; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Options; @@ -24,6 +25,59 @@ public sealed class ApiKeyAuthenticationHandlerTests(ITestOutputHelper output) : private const string Salt = "1234567890123456"; private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(10); + [Theory] + [InlineData("Bearer", true, null, null, true)] + [InlineData("Basic", true, null, null, true)] + [InlineData("Bearer", true, "current", null, false)] + [InlineData("Basic", true, "current", null, false)] + [InlineData("Bearer", true, "current", "CURRENT", false)] + [InlineData("Basic", true, "current", "CURRENT", false)] + [InlineData("Bearer", false, "current", "current", false)] + [InlineData("Basic", false, "current", "current", false)] + [InlineData("Bearer", true, "current", "current", true)] + [InlineData("Basic", true, "current", "current", true)] + public async Task AuthenticateAsync_AuthenticationVersion_RequiresActiveUserAndMatchingGeneration(string scheme, bool isActive, string? userVersion, string? tokenVersion, bool expectedSuccess) + { + // Arrange + const string tokenId = "legacy-user-token"; + var user = new User + { + Id = "123456789012345678901234", + EmailAddress = EmailAddress, + FullName = "Token Test User", + IsActive = isActive, + AuthenticationVersion = userVersion, + Roles = new HashSet { AuthorizationRoles.User } + }; + var token = new Token { Id = tokenId, UserId = user.Id, Type = TokenType.Authentication, AuthenticationVersion = tokenVersion }; + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByIdAsync(user.Id, (ICommandOptions?)null)).ReturnsAsync(user); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.GetByIdAsync(tokenId, It.IsAny>())).ReturnsAsync(token); + var oauthTokens = new Mock(MockBehavior.Strict); + var options = new Mock>(MockBehavior.Strict); + options.Setup(monitor => monitor.Get(It.IsAny())).Returns(new ApiKeyAuthenticationOptions()); + var handler = new ApiKeyAuthenticationHandler(tokens.Object, oauthTokens.Object, GetService(), GetService(), users.Object, + GetService(), GetService(), options.Object, TimeProvider, Log, UrlEncoder.Default); + var context = new DefaultHttpContext { RequestServices = GetService() }; + context.Request.Path = "/api/v2/users/me"; + string credentials = String.Equals(scheme, "Basic", StringComparison.Ordinal) + ? Convert.ToBase64String(Encoding.UTF8.GetBytes($"client:{tokenId}")) : tokenId; + context.Request.Headers.Authorization = $"{scheme} {credentials}"; + await handler.InitializeAsync(new AuthenticationScheme(ApiKeyAuthenticationOptions.ApiKeySchema, null, typeof(ApiKeyAuthenticationHandler)), context); + + // Act + var result = await handler.AuthenticateAsync(); + + // Assert + Assert.Equal(expectedSuccess, result.Succeeded); + users.Verify(repository => repository.GetByIdAsync(user.Id, (ICommandOptions?)null), Times.Once); + tokens.Verify(repository => repository.GetByIdAsync(tokenId, It.IsAny>()), Times.Once); + users.VerifyNoOtherCalls(); + tokens.VerifyNoOtherCalls(); + oauthTokens.VerifyNoOtherCalls(); + } + [Theory] [InlineData(false, true, 5)] [InlineData(true, true, 15)] diff --git a/tests/Exceptionless.Tests/Services/OAuthAuthorizationValidationTests.cs b/tests/Exceptionless.Tests/Services/OAuthAuthorizationValidationTests.cs index 4899d4ac8e..8e877f4f86 100644 --- a/tests/Exceptionless.Tests/Services/OAuthAuthorizationValidationTests.cs +++ b/tests/Exceptionless.Tests/Services/OAuthAuthorizationValidationTests.cs @@ -9,6 +9,7 @@ using Exceptionless.Tests.Utility; using Foundatio.Caching; using Foundatio.Lock; +using Foundatio.Repositories; using Foundatio.Repositories.Utility; using Moq; using Xunit; @@ -28,6 +29,60 @@ protected override void RegisterServices(IServiceCollection services, AppOptions services.AddSingleton(new OAuthServerOptions()); } + [Theory] + [InlineData(true, null, null, true)] + [InlineData(true, null, "current", false)] + [InlineData(true, "CURRENT", "current", false)] + [InlineData(false, "current", "current", false)] + [InlineData(true, "current", "current", true)] + public async Task ExchangeAuthorizationCodeAsync_AccountGeneration_RequiresActiveUserAndMatchingGeneration(bool isActive, string? issuedVersion, string? currentVersion, bool expectedSuccess) + { + // Arrange + string verifier = new('a', 43); + var issuedUser = new User { Id = TestConstants.UserId, AuthenticationVersion = issuedVersion }; + var currentUser = issuedUser with { IsActive = isActive, AuthenticationVersion = currentVersion }; + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByIdAsync(issuedUser.Id, (ICommandOptions?)null)).ReturnsAsync(currentUser); + var applications = new Mock(MockBehavior.Strict); + applications.Setup(repository => repository.GetByClientIdAsync("test-client", null)) + .ReturnsAsync(new OAuthApplication { ClientId = "test-client", RedirectUris = [RedirectUri], Scopes = [AuthorizationRoles.McpRead] }); + applications.Setup(repository => repository.AddOrganizationIdsAsync("test-client", It.IsAny>(), It.IsAny>())) + .ReturnsAsync(1L); + OAuthToken? storedToken = null; + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())) + .Callback((OAuthToken token, CommandOptionsDescriptor _) => storedToken = token) + .ReturnsAsync((OAuthToken token, CommandOptionsDescriptor _) => token); + var service = new OAuthService(GetService(), GetService(), GetService(), + applications.Object, GetService(), tokens.Object, users.Object, TimeProvider); + string code = await service.CreateAuthorizationCodeAsync(CreateRequest(AuthorizationRoles.McpRead) with + { + CodeChallenge = OAuthService.CreateCodeChallenge(verifier) + }, issuedUser, [TestConstants.OrganizationId]); + var request = new OAuthTokenRequest + { + GrantType = OAuthGrantTypes.AuthorizationCode, + ClientId = "test-client", + Code = code, + CodeVerifier = verifier, + RedirectUri = RedirectUri, + Resource = Resource + }; + + // Act + var result = await service.ExchangeAuthorizationCodeAsync(request); + + // Assert + Assert.Equal(expectedSuccess, result.IsSuccess); + Assert.Equal(expectedSuccess ? null : "invalid_grant", result.Error); + Assert.Equal(expectedSuccess, storedToken is not null); + Assert.Equal(expectedSuccess ? issuedVersion : null, storedToken?.AuthenticationVersion); + tokens.Verify(repository => repository.AddAsync(It.IsAny(), It.IsAny>()), Times.Exactly(expectedSuccess ? 1 : 0)); + users.Verify(repository => repository.GetByIdAsync(issuedUser.Id, (ICommandOptions?)null), Times.Once); + tokens.VerifyNoOtherCalls(); + users.VerifyNoOtherCalls(); + } + [Fact] public void GetActiveOAuthOrganizationIds_RemovedMembership_ReturnsNoOrganizations() { From 77b3704200f316ae877e96ca54a1d92006d0c488 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 07:39:10 -0500 Subject: [PATCH 6/9] Preserve existing model property order --- src/Exceptionless.Core/Models/OAuthToken.cs | 55 +++---- src/Exceptionless.Core/Models/Token.cs | 48 +++--- src/Exceptionless.Core/Models/User.cs | 72 ++++----- .../Services/OAuthService.cs | 10 +- .../Exceptionless.Tests/Api/Data/openapi.json | 150 +++++++++--------- 5 files changed, 153 insertions(+), 182 deletions(-) diff --git a/src/Exceptionless.Core/Models/OAuthToken.cs b/src/Exceptionless.Core/Models/OAuthToken.cs index 941bb3b3bf..1cbba61017 100644 --- a/src/Exceptionless.Core/Models/OAuthToken.cs +++ b/src/Exceptionless.Core/Models/OAuthToken.cs @@ -7,59 +7,52 @@ namespace Exceptionless.Core.Models; public class OAuthToken : IIdentity, IHaveDates, IValidatableObject { [Required] - [MaxLength(100)] - public string AccessTokenHash { get; set; } = null!; + [ObjectId] + public string Id { get; set; } = null!; - /// - /// The user session generation captured when this OAuth token is issued. - /// A legacy null value matches only an active user whose generation is also null; it does not describe a password format. - /// - public string? AuthenticationVersion { get; set; } + [Required] + [ObjectId] + public string UserId { get; set; } = null!; [Required] [MaxLength(2048)] public string ClientId { get; set; } = null!; - [Required] - [ObjectId] - public string CreatedBy { get; set; } = null!; - - public DateTime CreatedUtc { get; set; } - - public DateTime? ExpiresUtc { get; set; } - [Required] [MaxLength(100)] public string GrantId { get; set; } = null!; [Required] - [ObjectId] - public string Id { get; set; } = null!; - - public bool IsDisabled { get; set; } - - public bool IsSuspended { get; set; } + [MaxLength(2048)] + public string Resource { get; set; } = null!; - public HashSet OrganizationIds { get; set; } = new(StringComparer.Ordinal); + [Required] + [MaxLength(100)] + public string AccessTokenHash { get; set; } = null!; - public DateTime? RefreshExpiresUtc { get; set; } + /// + /// The user session generation captured when this OAuth token is issued. + /// A legacy null value matches only an active user whose generation is also null; it does not describe a password format. + /// + public string? AuthenticationVersion { get; set; } [MaxLength(100)] public string? RefreshTokenHash { get; set; } + public DateTime? ExpiresUtc { get; set; } + public DateTime? RefreshExpiresUtc { get; set; } public DateTime? RefreshTokenUsedUtc { get; set; } - - [Required] - [MaxLength(2048)] - public string Resource { get; set; } = null!; - + public HashSet OrganizationIds { get; set; } = new(StringComparer.Ordinal); public HashSet Scopes { get; set; } = new(StringComparer.Ordinal); - - public DateTime UpdatedUtc { get; set; } + public bool IsDisabled { get; set; } + public bool IsSuspended { get; set; } [Required] [ObjectId] - public string UserId { get; set; } = null!; + public string CreatedBy { get; set; } = null!; + + public DateTime CreatedUtc { get; set; } + public DateTime UpdatedUtc { get; set; } public IEnumerable Validate(ValidationContext validationContext) { diff --git a/src/Exceptionless.Core/Models/Token.cs b/src/Exceptionless.Core/Models/Token.cs index 477ad98210..23b4bddb44 100644 --- a/src/Exceptionless.Core/Models/Token.cs +++ b/src/Exceptionless.Core/Models/Token.cs @@ -6,33 +6,15 @@ namespace Exceptionless.Core.Models; public class Token : IOwnedByOrganizationAndProjectWithIdentity, IHaveDates, IValidatableObject { + [Required] + public string Id { get; set; } = null!; + /// /// The user session generation captured when this token is issued. /// A legacy null value matches only an active user whose generation is also null; it does not describe a password format. /// public string? AuthenticationVersion { get; set; } - public string CreatedBy { get; set; } = null!; - - public DateTime CreatedUtc { get; set; } - - /// - /// The user's preferred default project. Cannot be set when ProjectId is defined. - /// - [ObjectId] - public string? DefaultProjectId { get; set; } - - public DateTime? ExpiresUtc { get; set; } - - [Required] - public string Id { get; set; } = null!; - - public bool IsDisabled { get; set; } - - public bool IsSuspended { get; set; } - - public string? Notes { get; set; } - /// /// Null for user-scoped tokens (where UserId is set instead). /// Required when ProjectId is set. @@ -47,20 +29,28 @@ public class Token : IOwnedByOrganizationAndProjectWithIdentity, IHaveDates, IVa [ObjectId] public string ProjectId { get; set; } = null!; - public string? Refresh { get; set; } - - public HashSet Scopes { get; set; } = new(); - - public TokenType Type { get; set; } - - public DateTime UpdatedUtc { get; set; } - /// /// Set for user-scoped tokens. Cannot be set together with ProjectId. /// [ObjectId] public string? UserId { get; set; } + /// + /// The user's preferred default project. Cannot be set when ProjectId is defined. + /// + [ObjectId] + public string? DefaultProjectId { get; set; } + public string? Refresh { get; set; } + public TokenType Type { get; set; } + public HashSet Scopes { get; set; } = new(); + public DateTime? ExpiresUtc { get; set; } + public string? Notes { get; set; } + public bool IsDisabled { get; set; } + public bool IsSuspended { get; set; } + public string CreatedBy { get; set; } = null!; + public DateTime CreatedUtc { get; set; } + public DateTime UpdatedUtc { get; set; } + public IEnumerable Validate(ValidationContext validationContext) { // OrganizationId is required for all non-user-scoped tokens. diff --git a/src/Exceptionless.Core/Models/User.cs b/src/Exceptionless.Core/Models/User.cs index 11b729c08d..b234f57e25 100644 --- a/src/Exceptionless.Core/Models/User.cs +++ b/src/Exceptionless.Core/Models/User.cs @@ -8,7 +8,16 @@ namespace Exceptionless.Core.Models; public record User : IIdentity, IHaveDates, IValidatableObject { - public bool? AssistantConversationSharingEnabled { get; set; } + /// + /// Unique id that identifies an user. + /// + [ObjectId] + public string Id { get; set; } = null!; + + /// + /// The organizations that the user has access to. + /// + public ISet OrganizationIds { get; init; } = new HashSet(); /// /// Session revocation generation, rotated when a password changes or resets, or an external login is removed or transferred away. @@ -17,16 +26,14 @@ public record User : IIdentity, IHaveDates, IValidatableObject /// public string? AuthenticationVersion { get; set; } - [StringLength(2000)] - public string? AvatarFileName { get; set; } - - public DateTime CreatedUtc { get; set; } - - [Required] - [EmailAddress] - public string EmailAddress { get; set; } = null!; - - public bool EmailNotificationsEnabled { get; set; } = true; + public string? Password { get; set; } + public string? Salt { get; set; } + public string? PasswordResetToken { get; set; } + public DateTime PasswordResetTokenExpiration { get; set; } + public ICollection OAuthAccounts { get; init; } = new Collection(); + public ICollection OrganizationPreferences { get; init; } = new Collection(); + public ICollection SavedViewOrders { get; init; } = new Collection(); + public IDictionary ProductTours { get; init; } = new Dictionary(StringComparer.Ordinal); /// /// Gets or sets the users Full Name. @@ -34,48 +41,29 @@ public record User : IIdentity, IHaveDates, IValidatableObject [Required] public string FullName { get; set; } = null!; - /// - /// Unique id that identifies an user. - /// - [ObjectId] - public string Id { get; set; } = null!; + [Required] + [EmailAddress] + public string EmailAddress { get; set; } = null!; - /// - /// Gets or sets the users active state. - /// - public bool IsActive { get; init; } = true; + [StringLength(2000)] + public string? AvatarFileName { get; set; } + public bool EmailNotificationsEnabled { get; set; } = true; + public bool? AssistantConversationSharingEnabled { get; set; } public bool IsEmailAddressVerified { get; set; } - - public ICollection OAuthAccounts { get; init; } = new Collection(); + public string? VerifyEmailAddressToken { get; set; } + public DateTime VerifyEmailAddressTokenExpiration { get; set; } /// - /// The organizations that the user has access to. + /// Gets or sets the users active state. /// - public ISet OrganizationIds { get; init; } = new HashSet(); - - public ICollection OrganizationPreferences { get; init; } = new Collection(); - - public string? Password { get; set; } - - public string? PasswordResetToken { get; set; } - - public DateTime PasswordResetTokenExpiration { get; set; } - - public IDictionary ProductTours { get; init; } = new Dictionary(StringComparer.Ordinal); + public bool IsActive { get; init; } = true; public ISet Roles { get; init; } = new HashSet(); - public string? Salt { get; set; } - - public ICollection SavedViewOrders { get; init; } = new Collection(); - + public DateTime CreatedUtc { get; set; } public DateTime UpdatedUtc { get; set; } - public string? VerifyEmailAddressToken { get; set; } - - public DateTime VerifyEmailAddressTokenExpiration { get; set; } - public IEnumerable Validate(ValidationContext validationContext) { if (IsEmailAddressVerified) diff --git a/src/Exceptionless.Core/Services/OAuthService.cs b/src/Exceptionless.Core/Services/OAuthService.cs index 39e815f6b7..9572bd409e 100644 --- a/src/Exceptionless.Core/Services/OAuthService.cs +++ b/src/Exceptionless.Core/Services/OAuthService.cs @@ -869,15 +869,15 @@ public record OAuthClientRegistrationResponse public record OAuthAuthorizationCode { - public string? AuthenticationVersion { get; init; } public required string ClientId { get; init; } - public required string CodeChallenge { get; init; } - public DateTime CreatedUtc { get; init; } - public IReadOnlyCollection OrganizationIds { get; init; } = []; public required string RedirectUri { get; init; } + public required string UserId { get; init; } + public required string CodeChallenge { get; init; } public required string Resource { get; init; } + public string? AuthenticationVersion { get; init; } public IReadOnlyCollection Scopes { get; set; } = []; - public required string UserId { get; init; } + public DateTime CreatedUtc { get; init; } + public IReadOnlyCollection OrganizationIds { get; init; } = []; } diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index 33a401951e..17f157531e 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -14090,30 +14090,39 @@ }, "User": { "required": [ - "email_address", "full_name", - "created_utc", - "email_notifications_enabled", + "email_address", "id", - "is_active", - "is_email_address_verified", - "o_auth_accounts", "organization_ids", - "organization_preferences", "password_reset_token_expiration", + "o_auth_accounts", + "organization_preferences", + "saved_view_orders", "product_tours", + "email_notifications_enabled", + "is_email_address_verified", + "verify_email_address_token_expiration", + "is_active", "roles", - "saved_view_orders", - "updated_utc", - "verify_email_address_token_expiration" + "created_utc", + "updated_utc" ], "type": "object", "properties": { - "assistant_conversation_sharing_enabled": { - "type": [ - "null", - "boolean" - ] + "id": { + "maxLength": 24, + "minLength": 24, + "pattern": "^[a-fA-F0-9]{24}$", + "type": "string", + "description": "Unique id that identifies an user." + }, + "organization_ids": { + "uniqueItems": true, + "type": "array", + "items": { + "type": "string" + }, + "description": "The organizations that the user has access to." }, "authentication_version": { "type": [ @@ -14122,42 +14131,27 @@ ], "description": "Session revocation generation, rotated when a password changes or resets, or an external login is removed or transferred away.\nRemains null until rotation; active users with a null generation accept only matching legacy null token generations.\nThis value does not describe the password hashing format." }, - "avatar_file_name": { - "maxLength": 2000, - "minLength": 0, + "password": { "type": [ "null", "string" ] }, - "created_utc": { - "type": "string", - "format": "date-time" - }, - "email_address": { - "type": "string", - "format": "email" - }, - "email_notifications_enabled": { - "type": "boolean" + "salt": { + "type": [ + "null", + "string" + ] }, - "full_name": { - "type": "string", - "description": "Gets or sets the users Full Name." + "password_reset_token": { + "type": [ + "null", + "string" + ] }, - "id": { - "maxLength": 24, - "minLength": 24, - "pattern": "^[a-fA-F0-9]{24}$", + "password_reset_token_expiration": { "type": "string", - "description": "Unique id that identifies an user." - }, - "is_active": { - "type": "boolean", - "description": "Gets or sets the users active state." - }, - "is_email_address_verified": { - "type": "boolean" + "format": "date-time" }, "o_auth_accounts": { "type": "array", @@ -14165,38 +14159,62 @@ "$ref": "#/components/schemas/OAuthAccount" } }, - "organization_ids": { - "uniqueItems": true, + "organization_preferences": { "type": "array", "items": { - "type": "string" - }, - "description": "The organizations that the user has access to." + "$ref": "#/components/schemas/UserOrganizationPreference" + } }, - "organization_preferences": { + "saved_view_orders": { "type": "array", "items": { - "$ref": "#/components/schemas/UserOrganizationPreference" + "$ref": "#/components/schemas/UserSavedViewOrderPreference" } }, - "password": { + "product_tours": { + "type": "object" + }, + "full_name": { + "type": "string", + "description": "Gets or sets the users Full Name." + }, + "email_address": { + "type": "string", + "format": "email" + }, + "avatar_file_name": { + "maxLength": 2000, + "minLength": 0, "type": [ "null", "string" ] }, - "password_reset_token": { + "email_notifications_enabled": { + "type": "boolean" + }, + "assistant_conversation_sharing_enabled": { + "type": [ + "null", + "boolean" + ] + }, + "is_email_address_verified": { + "type": "boolean" + }, + "verify_email_address_token": { "type": [ "null", "string" ] }, - "password_reset_token_expiration": { + "verify_email_address_token_expiration": { "type": "string", "format": "date-time" }, - "product_tours": { - "type": "object" + "is_active": { + "type": "boolean", + "description": "Gets or sets the users active state." }, "roles": { "uniqueItems": true, @@ -14205,29 +14223,11 @@ "type": "string" } }, - "salt": { - "type": [ - "null", - "string" - ] - }, - "saved_view_orders": { - "type": "array", - "items": { - "$ref": "#/components/schemas/UserSavedViewOrderPreference" - } - }, - "updated_utc": { + "created_utc": { "type": "string", "format": "date-time" }, - "verify_email_address_token": { - "type": [ - "null", - "string" - ] - }, - "verify_email_address_token_expiration": { + "updated_utc": { "type": "string", "format": "date-time" } From 4504e74af59c2ee382e09ece2f196239563ef0d3 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 08:32:02 -0500 Subject: [PATCH 7/9] Preserve cached authentication reads and simplify handler tests --- .../Services/OAuthService.cs | 5 + .../Api/Handlers/AuthHandler.cs | 2 +- .../Security/ApiKeyAuthenticationHandler.cs | 4 +- .../Api/Endpoints/AuthEndpointTests.cs | 5 +- .../Api/Handlers/AuthHandlerTests.cs | 95 +------------------ .../ApiKeyAuthenticationHandlerTests.cs | 12 ++- 6 files changed, 24 insertions(+), 99 deletions(-) diff --git a/src/Exceptionless.Core/Services/OAuthService.cs b/src/Exceptionless.Core/Services/OAuthService.cs index 9572bd409e..fb0d7ce3c4 100644 --- a/src/Exceptionless.Core/Services/OAuthService.cs +++ b/src/Exceptionless.Core/Services/OAuthService.cs @@ -385,6 +385,11 @@ public async Task ValidateAuthorizationRequestAsync(OAuth public async Task CreateAuthorizationCodeAsync(OAuthAuthorizeRequest request, User user, IReadOnlyCollection organizationIds) { + ArgumentNullException.ThrowIfNull(request); + ArgumentNullException.ThrowIfNull(user); + ArgumentNullException.ThrowIfNull(organizationIds); + ArgumentException.ThrowIfNullOrWhiteSpace(user.Id); + await oauthApplicationRepository.AddOrganizationIdsAsync(request.ClientId, organizationIds, o => o.ImmediateConsistency().Notifications(false)); string code = StringExtensions.GetNewToken(); diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index 83c3b21456..fc37943515 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -563,7 +563,7 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit existingUser.AuthenticationVersion = Core.Extensions.StringExtensions.GetNewToken(); await userRepository.SaveAsync(existingUser, o => o.Cache()); - await ResetUserTokensAsync(existingUser, "ExternalLoginTransfer", httpContext); + await ResetUserTokensAsync(existingUser, "Link external account to another user", httpContext); } else { diff --git a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs index 98116af59e..4d6c6c5b11 100644 --- a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs +++ b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs @@ -129,7 +129,7 @@ protected override async Task HandleAuthenticateAsync() if (!String.IsNullOrEmpty(tokenRecord.UserId)) { - var user = await _userRepository.GetByIdAsync(tokenRecord.UserId); + var user = await _userRepository.GetByIdAsync(tokenRecord.UserId, o => o.Cache()); if (user is null) { Logger.LogInformation("Could not find user {UserId} for token on {Path}", tokenRecord.UserId, Request.Path); @@ -242,7 +242,7 @@ private async Task AuthenticateOAuthBearerAsync(string token if (!await _oauthService.IsAccessTokenClientValidAsync(tokenRecord.ClientId)) return AuthenticateResult.Fail("OAuth client is not valid"); - var user = await _userRepository.GetByIdAsync(tokenRecord.UserId); + var user = await _userRepository.GetByIdAsync(tokenRecord.UserId, o => o.Cache()); if (user is null) { Logger.LogInformation("Could not find user {UserId} for OAuth token on {Path}", tokenRecord.UserId, Request.Path); diff --git a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs index 58d47d4f2a..9e834ef68c 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs @@ -1043,13 +1043,12 @@ public async Task AuthenticateAsync_InactiveUserToken_ReturnsUnauthorized() { // Arrange const string email = "inactive-token-user@exceptionless.test"; - const string password = "Password1$"; const string salt = "1234567890123456"; var user = new User { EmailAddress = email, - Password = password.ToSaltedHash(salt), + Password = TestConstants.UserPassword.ToSaltedHash(salt), Salt = salt, FullName = "Inactive Token User", Roles = AuthorizationRoles.AllScopes @@ -1060,7 +1059,7 @@ public async Task AuthenticateAsync_InactiveUserToken_ReturnsUnauthorized() var login = await SendRequestAsAsync(r => r .Post() .AppendPath("auth/login") - .Content(new Login { Email = email, Password = password }) + .Content(new Login { Email = email, Password = TestConstants.UserPassword }) .StatusCodeShouldBeOk()); Assert.NotNull(login); Assert.False(String.IsNullOrEmpty(login.Token)); diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 607bc7d563..94114de03b 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -1,8 +1,4 @@ -using System.Reflection; using System.Security.Claims; -using System.Text; -using Elastic.Clients.Elasticsearch; -using Elastic.Transport; using Exceptionless.Core; using Exceptionless.Core.Authentication; using Exceptionless.Core.Configuration; @@ -10,8 +6,8 @@ using Exceptionless.Core.Mail; using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; -using Exceptionless.Core.Repositories.Configuration; using Exceptionless.Core.Services; +using Exceptionless.Tests.Utility; using Exceptionless.Web.Api.Handlers; using Exceptionless.Web.Api.Messages; using Exceptionless.Web.Extensions; @@ -20,7 +16,6 @@ using Foundatio.Caching; using Foundatio.Mediator; using Foundatio.Repositories; -using Foundatio.Repositories.Elasticsearch.Configuration; using Foundatio.Repositories.Models; using Microsoft.AspNetCore.Http; using Moq; @@ -32,58 +27,12 @@ public sealed class AuthHandlerTests : TestWithServices { public AuthHandlerTests(ITestOutputHelper output) : base(output) { } - [Fact] - public async Task GetByIdAsync_ReadCompletesAfterSave_RepopulatesOldAuthenticationGeneration() - { - // Arrange - const string id = "123456789012345678901234"; - var readStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var releaseRead = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - int gets = 0; - var invoker = new CacheRaceInvoker(async endpoint => - { - if (endpoint.Method == Elastic.Transport.HttpMethod.GET) - { - Interlocked.Increment(ref gets); - readStarted.TrySetResult(); - await releaseRead.Task; - return "{\"_index\":\"users-v1\",\"_id\":\"" + id + "\",\"found\":true,\"_source\":{\"id\":\"" + id + "\",\"emailAddress\":\"user@example.test\",\"authenticationVersion\":\"old\"}}"; - } - Assert.Equal(Elastic.Transport.HttpMethod.PUT, endpoint.Method); - return "{\"_index\":\"users-v1\",\"_id\":\"" + id + "\",\"_version\":2,\"result\":\"updated\",\"_shards\":{\"total\":1,\"successful\":1,\"failed\":0},\"_seq_no\":2,\"_primary_term\":1}"; - }); - var client = new ElasticsearchClient(new ElasticsearchClientSettings(new SingleNodePool(new Uri("http://localhost:9200")), invoker).DisablePing()); - var configuration = GetService(); - typeof(ElasticConfiguration).GetField("_client", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(configuration, new Lazy(() => client)); - typeof(Foundatio.Repositories.Elasticsearch.Configuration.Index).GetField("_isEnsured", BindingFlags.Instance | BindingFlags.NonPublic)!.SetValue(configuration.Users, true); - var repository = GetService(); - var current = new User { Id = id, EmailAddress = "user@example.test", FullName = "Cache Test", AuthenticationVersion = "new" }; - - // Act - var delayedRead = repository.GetByIdAsync(id, o => o.Cache()); - await readStarted.Task.WaitAsync(TimeSpan.FromSeconds(10), TestCancellationToken); - await repository.SaveAsync(current, o => o.Cache().Originals(false).Validation(false).Notifications(false)); - var immediatelyAfterSave = await repository.GetByIdAsync(id, o => o.Cache()); - var emailImmediatelyAfterSave = await repository.GetByEmailAddressAsync(current.EmailAddress); - releaseRead.TrySetResult(); - await delayedRead; - var afterDelayedRead = await repository.GetByIdAsync(id, o => o.Cache()); - var emailAfterDelayedRead = await repository.GetByEmailAddressAsync(current.EmailAddress); - - // Assert - Assert.Equal("new", immediatelyAfterSave!.AuthenticationVersion); - Assert.Equal("new", emailImmediatelyAfterSave!.AuthenticationVersion); - Assert.Equal("old", afterDelayedRead!.AuthenticationVersion); - Assert.Equal("old", emailAfterDelayedRead!.AuthenticationVersion); - Assert.Equal(1, gets); - } - [Theory] [InlineData(false, null)] [InlineData(true, null)] [InlineData(false, "previous-version")] [InlineData(true, "previous-version")] - public async Task Handle_ExternalLoginTransfer_RevokesDisplacedUserSessionsBeforeLinking(bool cleanupFails, string? previousVersion) + public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOwnerSessions(bool cleanupFails, string? previousVersion) { // Arrange const string provider = "github"; @@ -144,12 +93,12 @@ public async Task Handle_ExternalLoginTransfer_RevokesDisplacedUserSessionsBefor public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exception exception) { // Arrange - var userRepository = new Mock(); + var userRepository = new Mock(MockBehavior.Strict); userRepository.Setup(repository => repository.GetByEmailAddressAsync("test@example.com")) .ThrowsAsync(exception); var handler = CreateHandler(userRepository.Object, GetService(), GetService(), GetService()); var message = new LoginMessage( - new Login { Email = "test@example.com", Password = "password" }, + new Login { Email = "test@example.com", Password = TestConstants.UserPassword }, new DefaultHttpContext()); // Act @@ -168,32 +117,6 @@ public async Task Handle_LoginRepositoryException_ReturnsUnauthorizedResult(Exce new OperationCanceledException("Repository operation was canceled.") }; - [Fact] - public async Task ResetUserTokensAsync_OneStoreThrows_StillAttemptsBothStores() - { - // Arrange - var tokens = new Mock(); - var oauthTokens = new Mock(); - tokens.Setup(repository => repository.RemoveAllByUserIdAsync("test-user", null)) - .Throws(new InvalidOperationException("Token store unavailable.")); - oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync("test-user", null)) - .ReturnsAsync(0L); - var handler = CreateHandler(Mock.Of(), tokens.Object, oauthTokens.Object, GetService()); - var method = typeof(AuthHandler).GetMethod("ResetUserTokensAsync", BindingFlags.Instance | BindingFlags.NonPublic); - Assert.NotNull(method); - - // Act - var task = Assert.IsAssignableFrom(method.Invoke(handler, - [new User { Id = "test-user", EmailAddress = "user@example.test", AuthenticationVersion = "current" }, "test", new DefaultHttpContext()])); - await task; - - // Assert - tokens.Verify(repository => repository.RemoveAllByUserIdAsync("test-user", null), Times.Once); - oauthTokens.Verify(repository => repository.RemoveAllByUserIdAsync("test-user", null), Times.Once); - tokens.VerifyNoOtherCalls(); - oauthTokens.VerifyNoOtherCalls(); - } - private AuthHandler CreateHandler(IUserRepository userRepository, ITokenRepository tokenRepository, IOAuthTokenRepository oauthTokenRepository, IOAuthProviderClient providerClient) { var appOptions = GetService(); @@ -214,14 +137,4 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito Log.CreateLogger()); } - private sealed class CacheRaceInvoker(Func> respond) - : InMemoryRequestInvoker(null, 200, null, "application/json", new Dictionary> { ["x-elastic-product"] = ["Elasticsearch"] }), IRequestInvoker - { - async Task IRequestInvoker.RequestAsync(Elastic.Transport.Endpoint endpoint, BoundConfiguration boundConfiguration, PostData? postData, CancellationToken cancellationToken) - { - string json = await respond(endpoint); - return await BuildResponseAsync(endpoint, boundConfiguration, postData, cancellationToken, Encoding.UTF8.GetBytes(json), 200); - } - } - } diff --git a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs index 4f3c4272e4..1c35b3096f 100644 --- a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs @@ -10,6 +10,7 @@ using Exceptionless.Web.Security; using Foundatio.Caching; using Foundatio.Repositories; +using Foundatio.Repositories.Options; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Options; @@ -51,7 +52,14 @@ public async Task AuthenticateAsync_AuthenticationVersion_RequiresActiveUserAndM }; var token = new Token { Id = tokenId, UserId = user.Id, Type = TokenType.Authentication, AuthenticationVersion = tokenVersion }; var users = new Mock(MockBehavior.Strict); - users.Setup(repository => repository.GetByIdAsync(user.Id, (ICommandOptions?)null)).ReturnsAsync(user); + users.Setup(repository => repository.GetByIdAsync(user.Id, It.IsAny>())) + .Callback>((_, configure) => + { + var commandOptions = new CommandOptions(); + configure(commandOptions); + Assert.True(commandOptions.ShouldReadCache()); + }) + .ReturnsAsync(user); var tokens = new Mock(MockBehavior.Strict); tokens.Setup(repository => repository.GetByIdAsync(tokenId, It.IsAny>())).ReturnsAsync(token); var oauthTokens = new Mock(MockBehavior.Strict); @@ -71,7 +79,7 @@ public async Task AuthenticateAsync_AuthenticationVersion_RequiresActiveUserAndM // Assert Assert.Equal(expectedSuccess, result.Succeeded); - users.Verify(repository => repository.GetByIdAsync(user.Id, (ICommandOptions?)null), Times.Once); + users.Verify(repository => repository.GetByIdAsync(user.Id, It.IsAny>()), Times.Once); tokens.Verify(repository => repository.GetByIdAsync(tokenId, It.IsAny>()), Times.Once); users.VerifyNoOtherCalls(); tokens.VerifyNoOtherCalls(); From d879f1b94f4ae9bc83628315f35777006e997554 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 08:55:50 -0500 Subject: [PATCH 8/9] Reject stale OAuth generations without disabling valid tokens --- .../Security/ApiKeyAuthenticationHandler.cs | 5 +- .../Api/Handlers/AuthHandlerTests.cs | 1 - .../ApiKeyAuthenticationHandlerTests.cs | 69 +++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs index 4d6c6c5b11..99d1f44c4f 100644 --- a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs +++ b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs @@ -249,12 +249,15 @@ private async Task AuthenticateOAuthBearerAsync(string token return AuthenticateResult.Fail("Token is not valid"); } - if (!user.HasValidAuthenticationVersion(tokenRecord.AuthenticationVersion)) + if (!user.IsActive) { await DisableOAuthTokenAsync(tokenRecord); return AuthenticateResult.Fail("User is not valid"); } + if (!user.HasValidAuthenticationVersion(tokenRecord.AuthenticationVersion)) + return AuthenticateResult.Fail("User is not valid"); + var activeOAuthOrganizationIds = user.GetActiveOAuthOrganizationIds(tokenRecord); if (activeOAuthOrganizationIds.Count == 0) { diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 94114de03b..e4aabc5edc 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -136,5 +136,4 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito TimeProvider, Log.CreateLogger()); } - } diff --git a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs index 1c35b3096f..4b104a4d30 100644 --- a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs @@ -3,12 +3,14 @@ using System.Text.Encodings.Web; using Exceptionless.Core; using Exceptionless.Core.Authorization; +using Exceptionless.Core.Configuration; using Exceptionless.Core.Extensions; using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; using Exceptionless.Core.Services; using Exceptionless.Web.Security; using Foundatio.Caching; +using Foundatio.Lock; using Foundatio.Repositories; using Foundatio.Repositories.Options; using Microsoft.AspNetCore.Authentication; @@ -26,6 +28,73 @@ public sealed class ApiKeyAuthenticationHandlerTests(ITestOutputHelper output) : private const string Salt = "1234567890123456"; private static readonly TimeSpan TestTimeout = TimeSpan.FromSeconds(10); + [Theory] + [InlineData(true, null)] + [InlineData(true, "previous")] + [InlineData(false, "current")] + public async Task AuthenticateAsync_OAuthCachedUser_RejectsMismatchWithoutRevokingActiveUserToken(bool isActive, string? cachedVersion) + { + // Arrange + const string tokenId = "oauth-token"; + const string clientId = "generation-test-client"; + const string refreshHash = "valid-refresh-hash"; + string accessToken = new('a', OAuthService.OAuthTokenLength); + var user = new User { Id = "123456789012345678901234", IsActive = isActive, AuthenticationVersion = cachedVersion }; + var appOptions = GetService(); + string origin = (String.IsNullOrWhiteSpace(appOptions.ApiUrl) ? appOptions.BaseURL : appOptions.ApiUrl).GetOrigin(); + var token = new OAuthToken + { + Id = tokenId, UserId = user.Id, ClientId = clientId, AuthenticationVersion = "current", + AccessTokenHash = OAuthService.CreateTokenHash(accessToken), RefreshTokenHash = refreshHash, + Resource = OAuthService.CreateResourceUri(origin, OAuthService.McpResource), OrganizationIds = ["organization"] + }; + var cache = GetService(); + await cache.SetAsync($"oauth:access:{token.AccessTokenHash}", tokenId); + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByIdAsync(user.Id, It.IsAny>())) + .Callback>((_, configure) => + { + var commandOptions = new CommandOptions(); + configure(commandOptions); + Assert.True(commandOptions.ShouldReadCache()); + }).ReturnsAsync(user); + var tokens = new Mock(MockBehavior.Strict); + var oauthTokens = new Mock(MockBehavior.Strict); + oauthTokens.Setup(repository => repository.GetByIdAsync(tokenId, It.IsAny>())).ReturnsAsync(token); + oauthTokens.Setup(repository => repository.SaveAsync(token, It.IsAny>())).ReturnsAsync(token); + var applications = new Mock(MockBehavior.Strict); + applications.Setup(repository => repository.GetByClientIdAsync(clientId, null)).ReturnsAsync(new OAuthApplication { ClientId = clientId }); + var oauthService = new OAuthService(new OAuthServerOptions(), cache, GetService(), applications.Object, + GetService(), oauthTokens.Object, users.Object, TimeProvider); + var options = new Mock>(MockBehavior.Strict); + options.Setup(monitor => monitor.Get(It.IsAny())).Returns(new ApiKeyAuthenticationOptions()); + var handler = new ApiKeyAuthenticationHandler(tokens.Object, oauthTokens.Object, cache, GetService(), users.Object, + oauthService, appOptions, options.Object, TimeProvider, Log, UrlEncoder.Default); + var context = new DefaultHttpContext { RequestServices = GetService() }; + context.Request.Path = OAuthService.McpResource.Path; + context.Request.Headers.Authorization = $"Bearer {accessToken}"; + await handler.InitializeAsync(new AuthenticationScheme(ApiKeyAuthenticationOptions.ApiKeySchema, null, typeof(ApiKeyAuthenticationHandler)), context); + + // Act + var result = await handler.AuthenticateAsync(); + await handler.ChallengeAsync(new AuthenticationProperties()); + + // Assert + Assert.False(result.Succeeded); + Assert.NotNull(result.Failure); + Assert.Equal(StatusCodes.Status401Unauthorized, context.Response.StatusCode); + Assert.Equal(!isActive, token.IsDisabled); + Assert.Equal(isActive ? refreshHash : null, token.RefreshTokenHash); + oauthTokens.Verify(repository => repository.SaveAsync(token, It.IsAny>()), Times.Exactly(isActive ? 0 : 1)); + oauthTokens.Verify(repository => repository.GetByIdAsync(tokenId, It.IsAny>()), Times.Once); + users.Verify(repository => repository.GetByIdAsync(user.Id, It.IsAny>()), Times.Once); + applications.Verify(repository => repository.GetByClientIdAsync(clientId, null), Times.Once); + oauthTokens.VerifyNoOtherCalls(); + users.VerifyNoOtherCalls(); + applications.VerifyNoOtherCalls(); + tokens.VerifyNoOtherCalls(); + } + [Theory] [InlineData("Bearer", true, null, null, true)] [InlineData("Basic", true, null, null, true)] From a178e7b0717cb7fe9d7d8816ce79073e863a4832 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 21:54:02 -0500 Subject: [PATCH 9/9] Use validated fixtures for session transfer tests --- .../Api/Handlers/AuthHandlerTests.cs | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index e4aabc5edc..14f653cb09 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -7,6 +7,7 @@ using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; using Exceptionless.Core.Services; +using Exceptionless.Core.Validation; using Exceptionless.Tests.Utility; using Exceptionless.Web.Api.Handlers; using Exceptionless.Web.Api.Messages; @@ -28,19 +29,25 @@ public sealed class AuthHandlerTests : TestWithServices public AuthHandlerTests(ITestOutputHelper output) : base(output) { } [Theory] - [InlineData(false, null)] - [InlineData(true, null)] - [InlineData(false, "previous-version")] - [InlineData(true, "previous-version")] - public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOwnerSessions(bool cleanupFails, string? previousVersion) + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOwnerSessions(bool cleanupFails, bool hasPreviousVersion) { // Arrange const string provider = "github"; - const string providerId = "provider-user"; - var displaced = new User { Id = "displaced-user", EmailAddress = "displaced@example.test", AuthenticationVersion = previousVersion }; + const string providerId = "123456789"; + string? previousVersion = hasPreviousVersion ? StringExtensions.GetNewToken() : null; + string currentVersion = StringExtensions.GetNewToken(); + var displaced = GetService().GenerateUser(id: TestConstants.UserId, emailAddress: TestConstants.UserEmail) with { AuthenticationVersion = previousVersion }; displaced.AddOAuthAccount(provider, providerId, displaced.EmailAddress); - displaced.AddOAuthAccount("google", "retained-provider", displaced.EmailAddress); - var current = new User { Id = "current-user", EmailAddress = "current@example.test", AuthenticationVersion = "current-version" }; + displaced.AddOAuthAccount("google", "104567890123456789012", displaced.EmailAddress); + var current = GetService().GenerateUser(id: TestConstants.UserId2, emailAddress: TestConstants.UserEmail2) with { AuthenticationVersion = currentVersion }; + displaced.MarkEmailAddressVerified(); + current.MarkEmailAddressVerified(); + await GetService().ValidateAndThrowAsync(displaced); + await GetService().ValidateAndThrowAsync(current); var context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)) }; context.Request.SetUser(current); var calls = new List(); @@ -48,7 +55,11 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw var users = new Mock(MockBehavior.Strict); users.Setup(repository => repository.GetUserByOAuthProviderAsync(provider, providerId)).ReturnsAsync(displaced); users.Setup(repository => repository.SaveAsync(displaced, It.IsAny>())) - .Callback(() => { persistedVersion = displaced.AuthenticationVersion; calls.Add("save-displaced"); }).ReturnsAsync(displaced); + .Callback(() => + { + persistedVersion = displaced.AuthenticationVersion; + calls.Add("save-displaced"); + }).ReturnsAsync(displaced); users.Setup(repository => repository.SaveAsync(current, It.IsAny>())) .Callback(() => calls.Add("save-current")).ReturnsAsync(current); var tokens = new Mock(MockBehavior.Strict); @@ -81,7 +92,7 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw Assert.Equal(["save-displaced", "remove-tokens", "remove-oauth-tokens", "save-current"], calls); Assert.DoesNotContain(displaced.OAuthAccounts, account => String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal)); Assert.Contains(current.OAuthAccounts, account => String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal)); - Assert.Equal("current-version", current.AuthenticationVersion); + Assert.Equal(currentVersion, current.AuthenticationVersion); users.VerifyAll(); tokens.VerifyAll(); oauthTokens.VerifyAll();