From 4c43e48efb2856ec8c6df7efa0ecd642434bfb44 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 21:30:44 -0500 Subject: [PATCH 1/6] Protect user saves with optimistic concurrency --- src/Exceptionless.Core/Models/User.cs | 7 +- .../Repositories/UserRepository.cs | 44 ++++++ .../JsonSerializerOptionsExtensions.cs | 10 ++ .../Api/Endpoints/AuthEndpoints.cs | 18 +++ .../Api/Handlers/AuthHandler.cs | 49 +++++- .../Api/Handlers/OrganizationHandler.cs | 47 ++++-- .../Exceptionless.Tests/Api/Data/openapi.json | 90 +++++++++++ .../Api/Endpoints/AuthEndpointTests.cs | 7 + .../Endpoints/OrganizationEndpointTests.cs | 97 ++++++++++++ .../Api/Endpoints/UserEndpointTests.cs | 147 ++++++++++++++++++ .../Api/Handlers/AuthHandlerTests.cs | 91 ++++++++++- .../Serializer/Models/UserSerializerTests.cs | 12 +- 12 files changed, 604 insertions(+), 15 deletions(-) diff --git a/src/Exceptionless.Core/Models/User.cs b/src/Exceptionless.Core/Models/User.cs index b234f57e25..ae4dcc5bd9 100644 --- a/src/Exceptionless.Core/Models/User.cs +++ b/src/Exceptionless.Core/Models/User.cs @@ -6,7 +6,7 @@ namespace Exceptionless.Core.Models; -public record User : IIdentity, IHaveDates, IValidatableObject +public record User : IIdentity, IHaveDates, IVersioned, IValidatableObject { /// /// Unique id that identifies an user. @@ -54,6 +54,11 @@ public record User : IIdentity, IHaveDates, IValidatableObject public string? VerifyEmailAddressToken { get; set; } public DateTime VerifyEmailAddressTokenExpiration { get; set; } + /// + /// Repository concurrency metadata used to reject stale writes, preserved in cached snapshots. + /// + public string Version { get; set; } = null!; + /// /// Gets or sets the users active state. /// diff --git a/src/Exceptionless.Core/Repositories/UserRepository.cs b/src/Exceptionless.Core/Repositories/UserRepository.cs index 4b32a48e5a..6f5b37a23d 100644 --- a/src/Exceptionless.Core/Repositories/UserRepository.cs +++ b/src/Exceptionless.Core/Repositories/UserRepository.cs @@ -2,6 +2,7 @@ using Exceptionless.Core.Repositories.Configuration; using Exceptionless.Core.Validation; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Models; using Foundatio.Repositories.Options; using User = Exceptionless.Core.Models.User; @@ -19,6 +20,43 @@ public UserRepository(ExceptionlessElasticConfiguration configuration, MiniValid AddRequiredField(u => u.EmailAddress, u => u.OrganizationIds); } + public override Task SaveAsync(IEnumerable documents, ICommandOptions? options = null) + { + ArgumentNullException.ThrowIfNull(documents); + + var users = documents.ToArray(); + if (users.Any(user => user is null)) + throw new ArgumentException("Users cannot contain null values.", nameof(documents)); + if (users.Any(user => String.IsNullOrWhiteSpace(user.Version))) + throw new VersionConflictDocumentException("Read the current user record before saving changes."); + + return base.SaveAsync(users, options); + } + + public override async Task GetByIdAsync(Id id, ICommandOptions? options = null) + { + var user = await base.GetByIdAsync(id, options); + if (user is null || !String.IsNullOrWhiteSpace(user.Version)) + return user; + + // Replace legacy cached snapshots, never attach a current version to stale fields. + await InvalidateCacheAsync(user); + return await base.GetByIdAsync(id, new CommandOptions().Cache(false)); + } + + public override async Task> GetByIdsAsync(Ids ids, ICommandOptions? options = null) + { + var users = await base.GetByIdsAsync(ids, options); + var legacyUsers = users.Where(user => String.IsNullOrWhiteSpace(user.Version)).ToArray(); + if (legacyUsers.Length == 0) + return users; + + foreach (var user in legacyUsers) + await InvalidateCacheAsync(user); + + return await base.GetByIdsAsync(ids, new CommandOptions().Cache(false)); + } + public async Task RecordProductTourAsync(User user, string stateKey, DateTime recordedUtc) { const string script = """ @@ -111,6 +149,12 @@ public Task SetSavedViewOrdersAsync(User user, CommandOptionsDescriptor q.FieldEquals(u => u.EmailAddress, emailAddress), o => o.Cache(EmailCacheKey(emailAddress))); + if (hit?.Document is { } user && String.IsNullOrWhiteSpace(user.Version)) + { + await InvalidateCacheAsync(user); + hit = await FindOneAsync(q => q.FieldEquals(u => u.EmailAddress, emailAddress), o => o.Cache(false)); + } + return hit?.Document; } diff --git a/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs b/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs index cd27fb785e..87dec1f9cb 100644 --- a/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs +++ b/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs @@ -3,6 +3,7 @@ using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; using System.Text.Unicode; +using Exceptionless.Core.Models; namespace Exceptionless.Core.Serialization; @@ -35,6 +36,15 @@ public static JsonSerializerOptions ConfigureExceptionlessDefaults(this JsonSeri public static JsonSerializerOptions ConfigureExceptionlessApiDefaults(this JsonSerializerOptions options) { ConfigureExceptionlessDefaults(options, skipEmptyCollections: false); + options.TypeInfoResolver = options.TypeInfoResolver!.WithAddedModifier(typeInfo => + { + if (typeInfo.Type != typeof(User)) + return; + + var version = typeInfo.Properties.FirstOrDefault(property => String.Equals(property.Name, "version", StringComparison.Ordinal)); + if (version is not null) + typeInfo.Properties.Remove(version); + }); return options; } diff --git a/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs b/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs index 2e117f7a64..5102415460 100644 --- a/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs +++ b/src/Exceptionless.Web/Api/Endpoints/AuthEndpoints.cs @@ -106,6 +106,7 @@ headers api_key input box. .Produces() .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with GitHub") .WithMetadata(new EndpointDocumentation { @@ -113,6 +114,7 @@ headers api_key input box. ["200"] = "User Authentication Token", ["401"] = "Sign-in failed", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -126,6 +128,7 @@ headers api_key input box. .Produces() .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with Google") .WithMetadata(new EndpointDocumentation { @@ -133,6 +136,7 @@ headers api_key input box. ["200"] = "User Authentication Token", ["401"] = "Sign-in failed", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -146,6 +150,7 @@ headers api_key input box. .Produces() .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with Facebook") .WithMetadata(new EndpointDocumentation { @@ -153,6 +158,7 @@ headers api_key input box. ["200"] = "User Authentication Token", ["401"] = "Sign-in failed", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -166,6 +172,7 @@ headers api_key input box. .Produces() .ProducesProblem(StatusCodes.Status401Unauthorized) .ProducesProblem(StatusCodes.Status403Forbidden) + .ProducesProblem(StatusCodes.Status409Conflict) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) .WithSummary("Sign in with Microsoft") .WithMetadata(new EndpointDocumentation { @@ -173,6 +180,7 @@ headers api_key input box. ["200"] = "User Authentication Token", ["401"] = "Sign-in failed", ["403"] = "Account Creation is currently disabled", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -182,6 +190,7 @@ headers api_key input box. .Accepts>("application/json", "application/*+json") .Produces() .ProducesProblem(StatusCodes.Status400BadRequest) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Removes an external login provider from the account") .WithMetadata(new EndpointDocumentation { RequestBodyDescription = "The provider user id.", @@ -191,6 +200,7 @@ headers api_key input box. ResponseDescriptions = new() { ["200"] = "User Authentication Token", ["400"] = "Invalid provider name.", + ["409"] = "The account changed while the request was being processed.", } }); @@ -201,10 +211,12 @@ headers api_key input box. .Accepts("application/json", "application/*+json") .Produces() .ProducesProblem(StatusCodes.Status422UnprocessableEntity) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Change password") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "User Authentication Token", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Validation error", } }); @@ -219,6 +231,7 @@ headers api_key input box. .AllowAnonymous() .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status400BadRequest) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Forgot password") .WithMetadata(new EndpointDocumentation { ParameterDescriptions = new() { @@ -227,6 +240,7 @@ headers api_key input box. ResponseDescriptions = new() { ["200"] = "Forgot password email was sent.", ["400"] = "Invalid email address.", + ["409"] = "The account changed while the request was being processed.", } }); @@ -238,10 +252,12 @@ headers api_key input box. .Accepts("application/json", "application/*+json") .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status422UnprocessableEntity) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Reset password") .WithMetadata(new EndpointDocumentation { ResponseDescriptions = new() { ["200"] = "Password reset email was sent.", + ["409"] = "The account changed while the request was being processed.", ["422"] = "Invalid reset password model.", } }); @@ -257,6 +273,7 @@ headers api_key input box. .AllowAnonymous() .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status400BadRequest) + .ProducesProblem(StatusCodes.Status409Conflict) .WithSummary("Cancel reset password") .WithMetadata(new EndpointDocumentation { ParameterDescriptions = new() { @@ -265,6 +282,7 @@ headers api_key input box. ResponseDescriptions = new() { ["200"] = "Password reset email was cancelled.", ["400"] = "Invalid password reset token.", + ["409"] = "The account changed while the request was being processed.", } }); diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index f6f3fc0a96..c912888bcb 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -563,15 +563,20 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit { var currentUser = httpContext.Request.GetUser(); EnsureUserIsActive(currentUser); + OAuthAccount? removedAccount = null; + string? removedAccountVersion = null; if (existingUser is not null) { if (!String.Equals(existingUser.Id, currentUser.Id, StringComparison.Ordinal)) { + removedAccount = existingUser.OAuthAccounts.FirstOrDefault(account => String.Equals(account.Provider, userInfo.ProviderName, StringComparison.OrdinalIgnoreCase) + && String.Equals(account.ProviderUserId, userInfo.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()); + existingUser = await userRepository.SaveAsync(existingUser, o => o.Cache()); + removedAccountVersion = existingUser.Version; await ResetUserTokensAsync(existingUser, "Link external account to another user", httpContext); } else @@ -581,7 +586,17 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit } currentUser.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email); - return await userRepository.SaveAsync(currentUser, o => o.Cache()); + try + { + return await userRepository.SaveAsync(currentUser, o => o.Cache()); + } + catch + { + if (existingUser is not null && removedAccount is not null) + await RestoreTransferredExternalLoginAsync(existingUser, removedAccountVersion, removedAccount, currentUser.Id); + + throw; + } } if (existingUser is not null) @@ -637,6 +652,36 @@ private async Task IsAccountCreationEnabledAsync(string? token) return organization is not null; } + private async Task RestoreTransferredExternalLoginAsync(User previousOwner, string? removedAccountVersion, OAuthAccount account, string destinationUserId) + { + try + { + // A failed save may have committed before a cache or transport error. + var destination = await userRepository.GetByIdAsync(destinationUserId); + if (destination is not null && destination.OAuthAccounts.Any(existing => String.Equals(existing.Provider, account.Provider, StringComparison.Ordinal) + && String.Equals(existing.ProviderUserId, account.ProviderUserId, StringComparison.Ordinal))) + return; + + var currentOwner = await userRepository.GetByIdAsync(previousOwner.Id); + if (currentOwner is null || String.IsNullOrEmpty(removedAccountVersion) + || !String.Equals(currentOwner.Version, removedAccountVersion, StringComparison.Ordinal) + || !String.Equals(currentOwner.AuthenticationVersion, previousOwner.AuthenticationVersion, StringComparison.Ordinal) + || currentOwner.OAuthAccounts.Any(existing => String.Equals(existing.Provider, account.Provider, StringComparison.Ordinal) + && String.Equals(existing.ProviderUserId, account.ProviderUserId, StringComparison.Ordinal))) + { + logger.LogWarning("External account restoration skipped for {EmailAddress} ({UserId}) because the previous owner changed", previousOwner.EmailAddress, previousOwner.Id); + return; + } + + currentOwner.OAuthAccounts.Add(account); + await userRepository.SaveAsync(currentOwner, o => o.Cache()); + } + catch (Exception ex) + { + logger.LogCritical(ex, "Error restoring external account for {EmailAddress} ({UserId}): {Message}", previousOwner.EmailAddress, previousOwner.Id, ex.Message); + } + } + private async Task AddInvitedUserToOrganizationAsync(string? token, User user, HttpContext httpContext) { if (String.IsNullOrWhiteSpace(token)) diff --git a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs index 574c7cb899..9a972e0a73 100644 --- a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs @@ -641,8 +641,8 @@ public async Task> Handle(AddOrganizationUser message) { if (!user.OrganizationIds.Contains(organization.Id)) { + await userRepository.PatchAsync(user.Id, new ActionPatch(current => current.OrganizationIds.Add(organization.Id)), o => o.Cache()); user.OrganizationIds.Add(organization.Id); - await userRepository.SaveAsync(user, o => o.Cache()); await messagePublisher.PublishAsync(new UserMembershipChanged { ChangeType = ChangeType.Added, @@ -683,6 +683,13 @@ public async Task Handle(RemoveOrganizationUser message) var user = await userRepository.GetByEmailAddressAsync(message.Email); if (user is null || !user.OrganizationIds.Contains(message.Id)) { + // Retrying a completed membership removal must also retry its cleanup. + if (user is not null) + { + await organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); + await organizationService.RemoveUserSavedViewsAsync(organization.Id, user.Id); + } + var invite = organization.Invites.FirstOrDefault(i => String.Equals(i.EmailAddress, message.Email, StringComparison.OrdinalIgnoreCase)); if (invite is null) return Result.Success(); @@ -699,15 +706,17 @@ public async Task Handle(RemoveOrganizationUser message) if (organizationUsers.Total is 1) return Result.BadRequest("An organization must contain at least one user."); + await userRepository.PatchAsync(user.Id, new ActionPatch(current => + { + current.OrganizationIds.Remove(organization.Id); + foreach (var preference in current.OrganizationPreferences.Where(preference => String.Equals(preference.OrganizationId, organization.Id, StringComparison.Ordinal)).ToList()) + current.OrganizationPreferences.Remove(preference); + foreach (var preference in current.SavedViewOrders.Where(preference => String.Equals(preference.OrganizationId, organization.Id, StringComparison.Ordinal)).ToList()) + current.SavedViewOrders.Remove(preference); + }), o => o.Cache()); + await organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); await organizationService.RemoveUserSavedViewsAsync(organization.Id, user.Id); - - user.OrganizationIds.Remove(organization.Id); - foreach (var preference in user.OrganizationPreferences.Where(preference => String.Equals(preference.OrganizationId, organization.Id, StringComparison.Ordinal)).ToList()) - user.OrganizationPreferences.Remove(preference); - foreach (var preference in user.SavedViewOrders.Where(preference => String.Equals(preference.OrganizationId, organization.Id, StringComparison.Ordinal)).ToList()) - user.SavedViewOrders.Remove(preference); - await userRepository.SaveAsync(user, o => o.Cache()); await messagePublisher.PublishAsync(new UserMembershipChanged { ChangeType = ChangeType.Removed, @@ -838,8 +847,28 @@ private async Task AddModelAsync(Organization value, HttpContext h var organization = await repository.AddAsync(value, o => o.Cache()); + try + { + await userRepository.PatchAsync(user.Id, new ActionPatch(current => current.OrganizationIds.Add(organization.Id)), o => o.Cache()); + } + catch + { + try + { + // A cache/notification failure may follow a committed membership write. + var current = await userRepository.GetByIdAsync(user.Id); + if (current is null || !current.OrganizationIds.Contains(organization.Id)) + await repository.RemoveAsync(organization); + } + catch (Exception ex) + { + _logger.LogCritical(ex, "Unable to clean up organization {OrganizationId} after adding user {EmailAddress} failed: {Message}", organization.Id, user.EmailAddress, ex.Message); + } + + throw; + } + user.OrganizationIds.Add(organization.Id); - await userRepository.SaveAsync(user, o => o.Cache()); await messagePublisher.PublishAsync(new UserMembershipChanged { UserId = user.Id, diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index ac44626d14..c21695d4ff 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -1751,6 +1751,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1816,6 +1826,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1881,6 +1901,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -1946,6 +1976,16 @@ } } }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Validation error", "content": { @@ -2058,6 +2098,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -2093,6 +2143,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -2131,6 +2191,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -2166,6 +2236,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -2311,6 +2391,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } diff --git a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs index d2f0666968..2bab90e426 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/AuthEndpointTests.cs @@ -22,6 +22,7 @@ using FluentRest; using Foundatio.Queues; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Utility; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; @@ -1337,6 +1338,8 @@ public async Task ChangePassword_StoredOldTokenSurvivesCleanup_CannotAuthenticat Assert.NotNull(login); var oldToken = await _tokenRepository.GetByIdAsync(login.Token, o => o.Cache(false)); Assert.NotNull(oldToken); + var staleUser = await _userRepository.GetByIdAsync(oldToken.UserId!, o => o.Cache(false)); + Assert.NotNull(staleUser); await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeOk()); // Act @@ -1350,11 +1353,15 @@ public async Task ChangePassword_StoredOldTokenSurvivesCleanup_CannotAuthenticat // Assert Assert.NotNull(changed); Assert.NotEqual(login.Token, changed.Token); + staleUser.FullName = "Stale profile update"; + await Assert.ThrowsAsync(() => _userRepository.SaveAsync(staleUser)); 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.True(user.IsCorrectPassword(newPassword)); + Assert.False(user.IsCorrectPassword(SampleDataService.TEST_USER_PASSWORD)); Assert.False(String.IsNullOrEmpty(user.AuthenticationVersion)); Assert.Equal(user.AuthenticationVersion, (await _tokenRepository.GetByIdAsync(changed.Token))!.AuthenticationVersion); } diff --git a/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs index dbd4f791cb..2592d5f773 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs @@ -5,13 +5,17 @@ using Exceptionless.Core.Models; using Exceptionless.Core.Models.Billing; using Exceptionless.Core.Repositories; +using Exceptionless.Core.Repositories.Configuration; using Exceptionless.Core.Services; using Exceptionless.Core.Utility; +using Exceptionless.Core.Validation; using Exceptionless.Tests.Extensions; using Exceptionless.Tests.Utility; using Exceptionless.Web.Models; using Exceptionless.Web.Utility; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; +using Foundatio.Repositories.Models; using Foundatio.Repositories.Utility; using Microsoft.Extensions.DependencyInjection; using Stripe; @@ -42,6 +46,7 @@ protected override void RegisterServices(IServiceCollection services) { base.RegisterServices(services); services.ReplaceSingleton(); + services.ReplaceSingleton(); } private async Task WithBillingEnabledAsync(Func action) @@ -403,6 +408,43 @@ public async Task PostAsync_NewOrganization_MapsToOrganizationAndCreates() Assert.Equal("Test Organization", organization.Name); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task PostAsync_MembershipConflicts_PreservesPasswordChangeAndCompensatesFailure(bool failMembership) + { + // Arrange + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(user); + string generation = StringExtensions.GetNewToken(); + const string organizationName = "Membership concurrency"; + var users = Assert.IsType(_userRepository); + users.BeforeNextPatch = async () => + { + var current = await _userRepository.GetByIdAsync(user.Id); + Assert.NotNull(current); + current.AuthenticationVersion = generation; + await _userRepository.SaveAsync(current); + if (failMembership) + throw new VersionConflictDocumentException("Concurrent membership update."); + }; + + // Act + await SendRequestAsync(r => r.AsTestOrganizationUser().Post().AppendPath("organizations") + .Content(new NewOrganization { Name = organizationName }) + .ExpectedStatus(failMembership ? HttpStatusCode.Conflict : HttpStatusCode.Created)); + + // Assert + var persisted = await _userRepository.GetByIdAsync(user.Id); + Assert.NotNull(persisted); + Assert.Equal(generation, persisted.AuthenticationVersion); + var organizations = await _organizationRepository.FindAsync(q => q.FieldEquals(organization => organization.Name, organizationName)); + if (failMembership) + Assert.Empty(organizations.Documents); + else + Assert.Contains(Assert.Single(organizations.Documents).Id, persisted.OrganizationIds); + } + [Fact] public async Task PostAsync_NewOrganization_ReturnsAbsoluteLocation() { @@ -1179,6 +1221,45 @@ public Task AddUserAsync_PlanLimitReached_ReturnsUpgradeRequired() ); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RemoveUserAsync_MembershipConflicts_DoesNotCleanUpBeforePersistence(bool failMembership) + { + // Arrange + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(user); + var project = await _projectRepository.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + project.NotificationSettings[user.Id] = new NotificationSettings { SendDailySummary = true }; + await _projectRepository.SaveAsync(project, o => o.ImmediateConsistency()); + string generation = StringExtensions.GetNewToken(); + var users = Assert.IsType(_userRepository); + users.BeforeNextPatch = async () => + { + var current = await _userRepository.GetByIdAsync(user.Id); + Assert.NotNull(current); + current.AuthenticationVersion = generation; + await _userRepository.SaveAsync(current); + if (failMembership) + throw new VersionConflictDocumentException("Concurrent membership update."); + }; + + // Act + await SendRequestAsync(r => r.AsGlobalAdminUser().Delete() + .AppendPaths("organizations", SampleDataService.TEST_ORG_ID, "users", SampleDataService.TEST_ORG_USER_EMAIL) + .ExpectedStatus(failMembership ? HttpStatusCode.Conflict : HttpStatusCode.OK)); + + // Assert + user = await _userRepository.GetByIdAsync(user.Id); + project = await _projectRepository.GetByIdAsync(project.Id); + Assert.NotNull(user); + Assert.NotNull(project); + Assert.Equal(generation, user.AuthenticationVersion); + Assert.Equal(failMembership, user.OrganizationIds.Contains(SampleDataService.TEST_ORG_ID)); + Assert.Equal(failMembership, project.NotificationSettings.ContainsKey(user.Id)); + } + [Fact] public async Task RemoveUserAsync_UserWithNotificationSettings_CleansUpNotificationSettings() { @@ -2782,6 +2863,22 @@ public async Task PatchAsync_EmptyJsonBody_ReturnsOriginalOrganizationUnchanged( Assert.Equal(originalOrganization.UpdatedUtc, updated.UpdatedUtc); } + private sealed class MembershipUserRepository(ExceptionlessElasticConfiguration configuration, MiniValidationValidator validator, AppOptions options) + : UserRepository(configuration, validator, options) + { + public Func? BeforeNextPatch { get; set; } + + public override async Task PatchAsync(Id id, IPatchOperation operation, ICommandOptions? options = null) + { + var beforePatch = BeforeNextPatch; + BeforeNextPatch = null; + if (beforePatch is not null) + await beforePatch(); + + return await base.PatchAsync(id, operation, options); + } + } + private static MultipartFormDataContent CreateProfileImageContent() { byte[] bytes = new byte[256 * 1024]; diff --git a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs index 8493e66ab5..ee72810708 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Text; using System.Text.Json; +using GetRequest = Elastic.Clients.Elasticsearch.GetRequest; using Exceptionless.Core; using Exceptionless.Core.Authorization; using Exceptionless.Core.Extensions; @@ -17,6 +18,7 @@ using Exceptionless.Web.Utility; using FluentRest; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Models; using Foundatio.Repositories.Utility; using Xunit; @@ -423,6 +425,49 @@ public async Task GetCurrentUserAsync_TestOrganizationUser_ReturnsCurrentUser() Assert.True(user.IsActive); } + [Theory] + [InlineData("id")] + [InlineData("ids")] + [InlineData("email")] + public async Task GetAsync_LegacyCachedUser_ReloadsVersionBeforeSave(string lookup) + { + // Arrange + var user = await GetTestOrganizationUserAsync(); + var legacy = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + Assert.NotNull(legacy); + legacy.Version = null!; + legacy.FullName = "Legacy cached profile"; + var repository = Assert.IsType(_userRepository); + await repository.CacheUserAsync(legacy); + + // Act + var loaded = lookup switch + { + "ids" => Assert.Single(await _userRepository.GetByIdsAsync(new[] { user.Id }, o => o.Cache())), + "email" => await _userRepository.GetByEmailAddressAsync(user.EmailAddress), + _ => await _userRepository.GetByIdAsync(user.Id, o => o.Cache()) + }; + Assert.NotNull(loaded); + Assert.Equal(user.FullName, loaded.FullName); + loaded.FullName = "Updated profile"; + await _userRepository.SaveAsync(loaded, o => o.ImmediateConsistency().Cache()); + var persisted = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + int getRequests = repository.GetRequestCount; + var cached = await _userRepository.GetByIdAsync(user.Id, o => o.Cache()); + var cachedAgain = await _userRepository.GetByIdAsync(user.Id, o => o.Cache()); + + // Assert + Assert.False(String.IsNullOrEmpty(loaded.Version)); + Assert.NotNull(persisted); + Assert.Equal("Updated profile", persisted.FullName); + Assert.Equal(loaded.Version, persisted.Version); + Assert.NotNull(cached); + Assert.NotNull(cachedAgain); + Assert.Equal(loaded.Version, cached.Version); + Assert.Equal(loaded.Version, cachedAgain.Version); + Assert.Equal(getRequests, repository.GetRequestCount); + } + [Fact] public async Task GetCurrentUserAsync_CachedUser_ReturnsCachedProfileUntilInvalidated() { @@ -805,6 +850,70 @@ public async Task PatchAsync_NullFullName_PreservesExistingName() Assert.Equal(currentUser.FullName, persisted.FullName); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task PatchAsync_FieldUpdate_PreservesAuthenticationVersion(bool script) + { + // Arrange + string authenticationVersion = StringExtensions.GetNewToken(); + var profile = await GetTestOrganizationUserAsync(); + var user = await _userRepository.GetByIdAsync(profile.Id, o => o.Cache(false)); + Assert.NotNull(user); + user.AuthenticationVersion = authenticationVersion; + await _userRepository.SaveAsync(user, o => o.ImmediateConsistency()); + IPatchOperation patch = script + ? new ScriptPatch("ctx._source.full_name = 'Updated profile'") + : new PartialPatch(new { full_name = "Updated profile" }); + + // Act + await _userRepository.PatchAsync(user.Id, patch, o => o.ImmediateConsistency()); + var persisted = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + + // Assert + Assert.NotNull(persisted); + Assert.Equal(authenticationVersion, persisted.AuthenticationVersion); + Assert.Equal("Updated profile", persisted.FullName); + Assert.NotEqual(user.Version, persisted.Version); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task PatchAllAsync_InterveningAuthenticationChange_PreservesNewGeneration(bool jsonPatch) + { + // Arrange + string authenticationVersion = StringExtensions.GetNewToken(); + const string updatedName = "Patched profile"; + var user = await GetTestOrganizationUserAsync(); + var repository = Assert.IsType(_userRepository); + bool intervened = false; + using var subscription = repository.AfterQuery.AddHandler(async (_, args) => + { + if (intervened || args.Result is not FindResults results || !results.Documents.Any(u => String.Equals(u.Id, user.Id, StringComparison.Ordinal))) + return; + + intervened = true; + var current = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + Assert.NotNull(current); + current.AuthenticationVersion = authenticationVersion; + await _userRepository.SaveAsync(current, o => o.ImmediateConsistency()); + }); + IPatchOperation patch = jsonPatch + ? new JsonPatch(new PatchDocument(new ReplaceOperation { Path = "full_name", Value = updatedName })) + : new ActionPatch(u => u.FullName = updatedName); + + // Act + await _userRepository.PatchAllAsync(q => q.Id(user.Id), patch, o => o.ImmediateConsistency()); + var persisted = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + + // Assert + Assert.True(intervened); + Assert.NotNull(persisted); + Assert.Equal(authenticationVersion, persisted.AuthenticationVersion); + Assert.Equal(updatedName, persisted.FullName); + } + [Theory] [InlineData(true)] [InlineData(false)] @@ -1056,6 +1165,36 @@ public async Task UpdateEmailAddressAsync_ValidEmail_ReturnsResult() Assert.NotNull(result); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SaveAsync_StaleOrMissingVersion_PreservesNewAuthenticationVersion(bool missingVersion) + { + // Arrange + string authenticationVersion = StringExtensions.GetNewToken(); + var user = await GetTestOrganizationUserAsync(); + var stale = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + var current = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + Assert.NotNull(stale); + Assert.NotNull(current); + Assert.False(String.IsNullOrEmpty(current.Version)); + current.AuthenticationVersion = authenticationVersion; + await _userRepository.SaveAsync(current, o => o.ImmediateConsistency().Cache()); + stale.FullName = "Stale profile"; + if (missingVersion) + stale.Version = null!; + + // Act + await Assert.ThrowsAsync(() => _userRepository.SaveAsync(stale)); + var persisted = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); + + // Assert + Assert.NotNull(persisted); + Assert.Equal(authenticationVersion, persisted.AuthenticationVersion); + Assert.Equal(current.FullName, persisted.FullName); + Assert.Equal(current.Version, persisted.Version); + } + [Fact] public async Task VerifyAsync_ExpiredToken_ReturnsValidationProblem() { @@ -1216,6 +1355,14 @@ private sealed class PausingProfileUserRepository(ExceptionlessElasticConfigurat { private ProfileReadPause? _pause; + public int GetRequestCount { get; private set; } + + protected override void ConfigureGetRequest(GetRequest request, ICommandOptions options) + { + GetRequestCount++; + base.ConfigureGetRequest(request, options); + } + public ProfileReadPause PauseNextRead(string userId) => _pause = new ProfileReadPause(userId); public Task CacheUserAsync(User user) => AddDocumentsToCacheAsync(user, new CommandOptions().Cache(), false); diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 14f653cb09..04c3d48b3d 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -17,6 +17,8 @@ using Foundatio.Caching; using Foundatio.Mediator; using Foundatio.Repositories; +using Foundatio.Repositories.Elasticsearch; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Models; using Microsoft.AspNetCore.Http; using Moq; @@ -28,6 +30,90 @@ public sealed class AuthHandlerTests : TestWithServices { public AuthHandlerTests(ITestOutputHelper output) : base(output) { } + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Handle_ExternalAccountDestinationSaveFails_RestoresOnlyUnchangedPreviousOwner(bool sourceChanged, bool destinationCommitted) + { + // Arrange + const string provider = "github"; + const string providerId = "123456789"; + string previousGeneration = StringExtensions.GetNewToken(); + var displaced = GetService().GenerateUser(id: TestConstants.UserId, emailAddress: TestConstants.UserEmail) + with { AuthenticationVersion = previousGeneration, Version = new ElasticDocumentVersion(1, 1) }; + displaced.AddOAuthAccount(provider, providerId, displaced.EmailAddress); + var destination = GetService().GenerateUser(id: TestConstants.UserId2, emailAddress: TestConstants.UserEmail2) + with { Version = new ElasticDocumentVersion(1, 1) }; + displaced.MarkEmailAddressVerified(); + destination.MarkEmailAddressVerified(); + await GetService().ValidateAndThrowAsync(displaced); + await GetService().ValidateAndThrowAsync(destination); + var storedDestination = destination with { OAuthAccounts = [] }; + if (destinationCommitted) + storedDestination.AddOAuthAccount(provider, providerId, displaced.EmailAddress); + + var context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)) }; + context.Request.SetUser(destination); + Exception failure = destinationCommitted + ? new IOException("Destination save failed.") + : new VersionConflictDocumentException("Destination user changed."); + string? rotatedGeneration = null; + User? currentOwner = null; + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetUserByOAuthProviderAsync(provider, providerId)).ReturnsAsync(displaced); + users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, displaced)), It.IsAny>())) + .Returns((User user, CommandOptionsDescriptor _) => + { + user.Version = new ElasticDocumentVersion(1, 2); + rotatedGeneration = user.AuthenticationVersion; + currentOwner = user with + { + OAuthAccounts = [], + Version = new ElasticDocumentVersion(1, sourceChanged ? 3 : 2), + FullName = sourceChanged ? "Updated Name" : user.FullName + }; + return Task.FromResult(user); + }); + users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, destination)), It.IsAny>())) + .ThrowsAsync(failure); + users.Setup(repository => repository.GetByIdAsync(destination.Id, (ICommandOptions?)null)).ReturnsAsync(storedDestination); + if (!destinationCommitted) + users.Setup(repository => repository.GetByIdAsync(displaced.Id, (ICommandOptions?)null)).ReturnsAsync(() => currentOwner); + if (!sourceChanged && !destinationCommitted) + users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, currentOwner)), It.IsAny>())) + .ReturnsAsync((User user, CommandOptionsDescriptor _) => user); + + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.RemoveAllByUserIdAsync(displaced.Id, null)).ReturnsAsync(1L); + var oauthTokens = new Mock(MockBehavior.Strict); + oauthTokens.Setup(repository => repository.RemoveAllByUserIdAsync(displaced.Id, null)).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 actual = await Assert.ThrowsAnyAsync(() => handler.Handle(new GitHubLogin(new ExternalAuthInfo(), context))); + + // Assert + Assert.Same(failure, actual); + Assert.NotNull(currentOwner); + Assert.False(String.IsNullOrEmpty(rotatedGeneration)); + Assert.NotEqual(previousGeneration, rotatedGeneration); + Assert.Equal(rotatedGeneration, currentOwner.AuthenticationVersion); + Assert.Equal(!sourceChanged && !destinationCommitted, currentOwner.OAuthAccounts.Any(account => String.Equals(account.Provider, provider, StringComparison.Ordinal) + && String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal))); + Assert.Equal(sourceChanged ? "Updated Name" : displaced.FullName, currentOwner.FullName); + users.VerifyAll(); + tokens.VerifyAll(); + oauthTokens.VerifyAll(); + providerClient.VerifyAll(); + } + [Theory] [InlineData(false, false)] [InlineData(true, false)] @@ -40,10 +126,10 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw 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 }; + var displaced = GetService().GenerateUser(id: TestConstants.UserId, emailAddress: TestConstants.UserEmail) with { AuthenticationVersion = previousVersion, Version = new ElasticDocumentVersion(1, 1) }; displaced.AddOAuthAccount(provider, providerId, displaced.EmailAddress); displaced.AddOAuthAccount("google", "104567890123456789012", displaced.EmailAddress); - var current = GetService().GenerateUser(id: TestConstants.UserId2, emailAddress: TestConstants.UserEmail2) with { AuthenticationVersion = currentVersion }; + var current = GetService().GenerateUser(id: TestConstants.UserId2, emailAddress: TestConstants.UserEmail2) with { AuthenticationVersion = currentVersion, Version = new ElasticDocumentVersion(1, 1) }; displaced.MarkEmailAddressVerified(); current.MarkEmailAddressVerified(); await GetService().ValidateAndThrowAsync(displaced); @@ -58,6 +144,7 @@ public async Task Handle_LinkExternalAccountOwnedByAnotherUser_RevokesPreviousOw .Callback(() => { persistedVersion = displaced.AuthenticationVersion; + displaced.Version = new ElasticDocumentVersion(1, 2); calls.Add("save-displaced"); }).ReturnsAsync(displaced); users.Setup(repository => repository.SaveAsync(current, It.IsAny>())) diff --git a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs index 37d82962e2..26d5a75280 100644 --- a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs +++ b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs @@ -1,5 +1,8 @@ using System.Text.Json; +using Exceptionless.Core.Extensions; using Exceptionless.Core.Models; +using Exceptionless.Core.Serialization; +using Foundatio.Repositories.Elasticsearch; using Foundatio.Serializer; using Xunit; @@ -231,6 +234,7 @@ public void Deserialize_SnakeCaseJson_PreservesOrganizationIds() Assert.NotNull(user); Assert.Equal("user-from-cache", user.Id); Assert.Null(user.AuthenticationVersion); + Assert.Null(user.Version); Assert.Equal(2, user.OrganizationIds.Count); Assert.Contains("cached-org-1", user.OrganizationIds); Assert.Contains("cached-org-2", user.OrganizationIds); @@ -420,11 +424,13 @@ public void Deserialize_UserWithEmptyCollections_ReturnsEmptyCollections() public void Deserialize_UserWithAllProperties_PreservesAllValues() { // Arrange - const string authenticationVersion = "current-version"; + string authenticationVersion = StringExtensions.GetNewToken(); + string documentVersion = new ElasticDocumentVersion(primaryTerm: 1, sequenceNumber: 42); var original = new User { Id = "full-user", AuthenticationVersion = authenticationVersion, + Version = documentVersion, FullName = "Full Test User", EmailAddress = "full@example.com", Password = "hashedpassword", @@ -443,11 +449,15 @@ public void Deserialize_UserWithAllProperties_PreservesAllValues() // Act string? json = _serializer.SerializeToString(original); var deserialized = _serializer.Deserialize(json); + using var apiJson = JsonDocument.Parse(JsonSerializer.Serialize(original, new JsonSerializerOptions().ConfigureExceptionlessApiDefaults())); // Assert Assert.NotNull(deserialized); Assert.Equal("full-user", deserialized.Id); Assert.Equal(authenticationVersion, deserialized.AuthenticationVersion); + Assert.Equal(documentVersion, deserialized.Version); + Assert.False(apiJson.RootElement.TryGetProperty("version", out _)); + Assert.Equal(authenticationVersion, apiJson.RootElement.GetProperty("authentication_version").GetString()); Assert.Equal("Full Test User", deserialized.FullName); Assert.Equal("full@example.com", deserialized.EmailAddress); Assert.Equal("hashedpassword", deserialized.Password); From d209e348f45c2dce8256caa149571715ac7caf2a Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Tue, 13 Oct 2026 21:48:33 -0500 Subject: [PATCH 2/6] Correct concurrency regression test setup and snapshot encoding --- tests/Exceptionless.Tests/Api/Data/openapi.json | 8 ++++---- .../Api/Endpoints/OrganizationEndpointTests.cs | 1 + .../Api/Endpoints/UserEndpointTests.cs | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index c21695d4ff..8fb2b1add4 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -1754,7 +1754,7 @@ "409": { "description": "The account changed while the request was being processed.", "content": { - "application/problem+json": { + "application/problem\u002Bjson": { "schema": { "$ref": "#/components/schemas/ProblemDetails" } @@ -1829,7 +1829,7 @@ "409": { "description": "The account changed while the request was being processed.", "content": { - "application/problem+json": { + "application/problem\u002Bjson": { "schema": { "$ref": "#/components/schemas/ProblemDetails" } @@ -1904,7 +1904,7 @@ "409": { "description": "The account changed while the request was being processed.", "content": { - "application/problem+json": { + "application/problem\u002Bjson": { "schema": { "$ref": "#/components/schemas/ProblemDetails" } @@ -1979,7 +1979,7 @@ "409": { "description": "The account changed while the request was being processed.", "content": { - "application/problem+json": { + "application/problem\u002Bjson": { "schema": { "$ref": "#/components/schemas/ProblemDetails" } diff --git a/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs index 2592d5f773..d58940cc21 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs @@ -433,6 +433,7 @@ public async Task PostAsync_MembershipConflicts_PreservesPasswordChangeAndCompen await SendRequestAsync(r => r.AsTestOrganizationUser().Post().AppendPath("organizations") .Content(new NewOrganization { Name = organizationName }) .ExpectedStatus(failMembership ? HttpStatusCode.Conflict : HttpStatusCode.Created)); + await RefreshDataAsync(); // Assert var persisted = await _userRepository.GetByIdAsync(user.Id); diff --git a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs index ee72810708..177566a1f9 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs @@ -863,7 +863,7 @@ public async Task PatchAsync_FieldUpdate_PreservesAuthenticationVersion(bool scr user.AuthenticationVersion = authenticationVersion; await _userRepository.SaveAsync(user, o => o.ImmediateConsistency()); IPatchOperation patch = script - ? new ScriptPatch("ctx._source.full_name = 'Updated profile'") + ? new ScriptPatch("ctx._source.full_name = 'Updated profile';") : new PartialPatch(new { full_name = "Updated profile" }); // Act From 52e8db1f9d09f5ae89ef5c62e56731e99a92051f Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Thu, 15 Oct 2026 20:59:35 -0500 Subject: [PATCH 3/6] Document external account unlink conflict response --- tests/Exceptionless.Tests/Api/Data/openapi.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index 8fb2b1add4..6038cd904e 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -2053,6 +2053,16 @@ } } } + }, + "409": { + "description": "The account changed while the request was being processed.", + "content": { + "application/problem\u002Bjson": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } From 1d8b2c2d8b6dd29e26baa779d82ccc9d5781b941 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Thu, 15 Oct 2026 21:15:56 -0500 Subject: [PATCH 4/6] Remove duplicate organization token conflict response --- tests/Exceptionless.Tests/Api/Data/openapi.json | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/Exceptionless.Tests/Api/Data/openapi.json b/tests/Exceptionless.Tests/Api/Data/openapi.json index 6038cd904e..6e671ac749 100644 --- a/tests/Exceptionless.Tests/Api/Data/openapi.json +++ b/tests/Exceptionless.Tests/Api/Data/openapi.json @@ -2401,16 +2401,6 @@ } } } - }, - "409": { - "description": "The account changed while the request was being processed.", - "content": { - "application/problem\u002Bjson": { - "schema": { - "$ref": "#/components/schemas/ProblemDetails" - } - } - } } } } From 0407c06be803e61486ecc129122cdd46b0db4dd3 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Thu, 15 Oct 2026 22:20:38 -0500 Subject: [PATCH 5/6] Keep membership revocation ahead of fallible cleanup --- .../Repositories/UserRepository.cs | 1 + .../JsonSerializerOptionsExtensions.cs | 1 + .../Api/Handlers/AuthHandler.cs | 3 + .../Api/Handlers/OrganizationHandler.cs | 16 +- .../Api/Handlers/AuthHandlerTests.cs | 31 ++- .../Api/Handlers/OrganizationHandlerTests.cs | 186 ++++++++++++++++++ .../Serializer/Models/UserSerializerTests.cs | 12 +- 7 files changed, 237 insertions(+), 13 deletions(-) create mode 100644 tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs diff --git a/src/Exceptionless.Core/Repositories/UserRepository.cs b/src/Exceptionless.Core/Repositories/UserRepository.cs index 6f5b37a23d..1d4c6a2495 100644 --- a/src/Exceptionless.Core/Repositories/UserRepository.cs +++ b/src/Exceptionless.Core/Repositories/UserRepository.cs @@ -27,6 +27,7 @@ public override Task SaveAsync(IEnumerable documents, ICommandOptions? opt var users = documents.ToArray(); if (users.Any(user => user is null)) throw new ArgumentException("Users cannot contain null values.", nameof(documents)); + if (users.Any(user => String.IsNullOrWhiteSpace(user.Version))) throw new VersionConflictDocumentException("Read the current user record before saving changes."); diff --git a/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs b/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs index 87dec1f9cb..b41205415d 100644 --- a/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs +++ b/src/Exceptionless.Core/Serialization/JsonSerializerOptionsExtensions.cs @@ -45,6 +45,7 @@ public static JsonSerializerOptions ConfigureExceptionlessApiDefaults(this JsonS if (version is not null) typeInfo.Properties.Remove(version); }); + return options; } diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index c912888bcb..a2e81a853c 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -565,12 +565,14 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit EnsureUserIsActive(currentUser); OAuthAccount? removedAccount = null; string? removedAccountVersion = null; + if (existingUser is not null) { if (!String.Equals(existingUser.Id, currentUser.Id, StringComparison.Ordinal)) { removedAccount = existingUser.OAuthAccounts.FirstOrDefault(account => String.Equals(account.Provider, userInfo.ProviderName, StringComparison.OrdinalIgnoreCase) && String.Equals(account.ProviderUserId, userInfo.Id, StringComparison.Ordinal)); + if (!existingUser.RemoveOAuthAccount(userInfo.ProviderName, userInfo.Id)) throw new Exception($"Unable to remove existing oauth account for existing user: {existingUser.EmailAddress}"); @@ -586,6 +588,7 @@ private async Task FromExternalLoginAsync(UserInfo userInfo, string? invit } currentUser.AddOAuthAccount(userInfo.ProviderName, userInfo.Id, userInfo.Email); + try { return await userRepository.SaveAsync(currentUser, o => o.Cache()); diff --git a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs index 9a972e0a73..3812229832 100644 --- a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs @@ -683,9 +683,16 @@ public async Task Handle(RemoveOrganizationUser message) var user = await userRepository.GetByEmailAddressAsync(message.Email); if (user is null || !user.OrganizationIds.Contains(message.Id)) { - // Retrying a completed membership removal must also retry its cleanup. + // Retrying a completed removal must also retry revocation and cleanup. if (user is not null) { + await messagePublisher.PublishAsync(new UserMembershipChanged + { + ChangeType = ChangeType.Removed, + UserId = user.Id, + OrganizationId = organization.Id + }); + await organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); await organizationService.RemoveUserSavedViewsAsync(organization.Id, user.Id); } @@ -709,20 +716,23 @@ public async Task Handle(RemoveOrganizationUser message) await userRepository.PatchAsync(user.Id, new ActionPatch(current => { current.OrganizationIds.Remove(organization.Id); + foreach (var preference in current.OrganizationPreferences.Where(preference => String.Equals(preference.OrganizationId, organization.Id, StringComparison.Ordinal)).ToList()) current.OrganizationPreferences.Remove(preference); + foreach (var preference in current.SavedViewOrders.Where(preference => String.Equals(preference.OrganizationId, organization.Id, StringComparison.Ordinal)).ToList()) current.SavedViewOrders.Remove(preference); }), o => o.Cache()); - await organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); - await organizationService.RemoveUserSavedViewsAsync(organization.Id, user.Id); await messagePublisher.PublishAsync(new UserMembershipChanged { ChangeType = ChangeType.Removed, UserId = user.Id, OrganizationId = organization.Id }); + + await organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); + await organizationService.RemoveUserSavedViewsAsync(organization.Id, user.Id); } return Result.Success(); diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 04c3d48b3d..8feafdf745 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -31,10 +31,11 @@ public sealed class AuthHandlerTests : TestWithServices public AuthHandlerTests(ITestOutputHelper output) : base(output) { } [Theory] - [InlineData(false, false)] - [InlineData(true, false)] - [InlineData(false, true)] - public async Task Handle_ExternalAccountDestinationSaveFails_RestoresOnlyUnchangedPreviousOwner(bool sourceChanged, bool destinationCommitted) + [InlineData(false, false, false)] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + [InlineData(false, false, true)] + public async Task Handle_ExternalAccountDestinationSaveFails_RestoresOnlyUnchangedPreviousOwner(bool sourceChanged, bool destinationCommitted, bool recoveryReadFails) { // Arrange const string provider = "github"; @@ -58,6 +59,7 @@ public async Task Handle_ExternalAccountDestinationSaveFails_RestoresOnlyUnchang Exception failure = destinationCommitted ? new IOException("Destination save failed.") : new VersionConflictDocumentException("Destination user changed."); + var recoveryFailure = new IOException("Recovery read failed."); string? rotatedGeneration = null; User? currentOwner = null; var users = new Mock(MockBehavior.Strict); @@ -77,10 +79,16 @@ public async Task Handle_ExternalAccountDestinationSaveFails_RestoresOnlyUnchang }); users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, destination)), It.IsAny>())) .ThrowsAsync(failure); - users.Setup(repository => repository.GetByIdAsync(destination.Id, (ICommandOptions?)null)).ReturnsAsync(storedDestination); - if (!destinationCommitted) + var recoveryRead = users.Setup(repository => repository.GetByIdAsync(destination.Id, (ICommandOptions?)null)); + if (recoveryReadFails) + recoveryRead.ThrowsAsync(recoveryFailure); + else + recoveryRead.ReturnsAsync(storedDestination); + + if (!destinationCommitted && !recoveryReadFails) users.Setup(repository => repository.GetByIdAsync(displaced.Id, (ICommandOptions?)null)).ReturnsAsync(() => currentOwner); - if (!sourceChanged && !destinationCommitted) + + if (!sourceChanged && !destinationCommitted && !recoveryReadFails) users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, currentOwner)), It.IsAny>())) .ReturnsAsync((User user, CommandOptionsDescriptor _) => user); @@ -105,9 +113,16 @@ public async Task Handle_ExternalAccountDestinationSaveFails_RestoresOnlyUnchang Assert.False(String.IsNullOrEmpty(rotatedGeneration)); Assert.NotEqual(previousGeneration, rotatedGeneration); Assert.Equal(rotatedGeneration, currentOwner.AuthenticationVersion); - Assert.Equal(!sourceChanged && !destinationCommitted, currentOwner.OAuthAccounts.Any(account => String.Equals(account.Provider, provider, StringComparison.Ordinal) + Assert.Equal(!sourceChanged && !destinationCommitted && !recoveryReadFails, currentOwner.OAuthAccounts.Any(account => String.Equals(account.Provider, provider, StringComparison.Ordinal) && String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal))); Assert.Equal(sourceChanged ? "Updated Name" : displaced.FullName, currentOwner.FullName); + if (recoveryReadFails) + { + var entry = Assert.Single(Log.LogEntries, entry => ReferenceEquals(entry.Exception, recoveryFailure)); + Assert.Equal(LogLevel.Critical, entry.LogLevel); + Assert.Equal($"Error restoring external account for {displaced.EmailAddress} ({displaced.Id}): {recoveryFailure.Message}", entry.Message); + } + users.VerifyAll(); tokens.VerifyAll(); oauthTokens.VerifyAll(); diff --git a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs new file mode 100644 index 0000000000..d7ab1fc89d --- /dev/null +++ b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs @@ -0,0 +1,186 @@ +using System.Security.Claims; +using Exceptionless.Core; +using Exceptionless.Core.Authorization; +using Exceptionless.Core.Billing; +using Exceptionless.Core.Extensions; +using Exceptionless.Core.Mail; +using Exceptionless.Core.Messaging.Models; +using Exceptionless.Core.Models; +using Exceptionless.Core.Models.Billing; +using Exceptionless.Core.Repositories; +using Exceptionless.Core.Services; +using Exceptionless.Tests.Utility; +using Exceptionless.Web.Api.Handlers; +using Exceptionless.Web.Api.Messages; +using Exceptionless.Web.Extensions; +using Exceptionless.Web.Mapping; +using Exceptionless.Web.Models; +using Foundatio.Caching; +using Foundatio.Lock; +using Foundatio.Messaging; +using Foundatio.Repositories; +using Foundatio.Repositories.Models; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Moq; +using Xunit; + +namespace Exceptionless.Tests.Api.Handlers; + +public sealed class OrganizationHandlerTests : TestWithServices +{ + public OrganizationHandlerTests(ITestOutputHelper output) : base(output) { } + + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Handle_CreateOrganizationMembershipFails_PreservesCommittedMembershipAndOriginalFailure(bool membershipCommitted, bool cleanupReadFails) + { + // Arrange + var user = new User { Id = "000000000000000000000002", EmailAddress = "owner@example.test" }; + var context = CreateContext(); + context.Request.SetUser(user); + var organization = new Organization { Id = "000000000000000000000001", Name = "Test" }; + var persisted = user with { OrganizationIds = new HashSet() }; + if (membershipCommitted) + persisted.OrganizationIds.Add(organization.Id); + + var failure = new IOException("Synthetic membership failure"); + var cleanupFailure = new IOException("Synthetic recovery read failure"); + var organizations = new Mock(MockBehavior.Strict); + organizations.Setup(repository => repository.GetByIdsAsync(It.IsAny(), (ICommandOptions?)null)).ReturnsAsync(Array.Empty()); + organizations.Setup(repository => repository.AddAsync(It.IsAny(), It.IsAny>())).ReturnsAsync(organization); + if (!membershipCommitted && !cleanupReadFails) + organizations.Setup(repository => repository.RemoveAsync(organization, (ICommandOptions?)null)).Returns(Task.CompletedTask); + + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.PatchAsync(user.Id, It.IsAny(), It.IsAny>())).ThrowsAsync(failure); + var recoveryRead = users.Setup(repository => repository.GetByIdAsync(user.Id, (ICommandOptions?)null)); + if (cleanupReadFails) + recoveryRead.ThrowsAsync(cleanupFailure); + else + recoveryRead.ReturnsAsync(persisted); + + var handler = CreateHandler(organizations.Object, users.Object, + new Mock(MockBehavior.Strict).Object, + new Mock(MockBehavior.Strict).Object, + new Mock(MockBehavior.Strict).Object, context); + + // Act + var actual = await Record.ExceptionAsync(async () => + { + _ = await handler.Handle(new CreateOrganization(new NewOrganization { Name = "Test" }, context)); + }); + + // Assert + Assert.Same(failure, actual); + if (cleanupReadFails) + { + var entry = Assert.Single(Log.LogEntries, entry => entry.LogLevel == LogLevel.Critical); + Assert.Same(cleanupFailure, entry.Exception); + Assert.Equal($"Unable to clean up organization {organization.Id} after adding user {user.EmailAddress} failed: {cleanupFailure.Message}", entry.Message); + } + + organizations.VerifyAll(); + users.VerifyAll(); + } + + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Handle_RemoveOrganizationUserFailure_PublishesRemovalOnlyAfterPersistence(bool alreadyRemoved, bool persistenceFails) + { + // Arrange + var organization = new Organization { Id = "000000000000000000000001", Name = "Test" }; + var user = new User { Id = "000000000000000000000002", EmailAddress = "removed@example.test" }; + if (!alreadyRemoved) + user.OrganizationIds.Add(organization.Id); + + var failure = new IOException("Synthetic removal failure"); + var calls = new List(); + var published = new List(); + var organizations = new Mock(MockBehavior.Strict); + organizations.Setup(repository => repository.GetByIdAsync(organization.Id, It.IsAny>())).ReturnsAsync(organization); + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByEmailAddressAsync(user.EmailAddress)).ReturnsAsync(user); + if (!alreadyRemoved) + { + users.Setup(repository => repository.GetByOrganizationIdAsync(organization.Id, null)).ReturnsAsync(new FindResults([], 2)); + users.Setup(repository => repository.PatchAsync(user.Id, It.IsAny(), It.IsAny>())) + .Callback(() => calls.Add("persist")) + .Returns(() => persistenceFails ? Task.FromException(failure) : Task.FromResult(true)); + } + + var projects = new Mock(MockBehavior.Strict); + var publisher = new Mock(MockBehavior.Strict); + if (!persistenceFails) + { + projects.Setup(repository => repository.GetByOrganizationIdAsync(organization.Id, It.IsAny>())) + .Callback(() => calls.Add("cleanup")) + .ThrowsAsync(failure); + publisher.Setup(client => client.PublishAsync(typeof(UserMembershipChanged), It.IsAny(), null, default)) + .Callback((Type _, object message, MessageOptions? _, CancellationToken _) => + { + published.Add((UserMembershipChanged)message); + calls.Add("publish"); + }) + .Returns(Task.CompletedTask); + } + + var context = CreateContext(); + var handler = CreateHandler(organizations.Object, users.Object, projects.Object, + new Mock(MockBehavior.Strict).Object, publisher.Object, context); + + // Act + var actual = await Record.ExceptionAsync(async () => + { + _ = await handler.Handle(new RemoveOrganizationUser(organization.Id, user.EmailAddress, context)); + }); + + // Assert + Assert.Same(failure, actual); + if (persistenceFails) + { + Assert.Equal(["persist"], calls); + Assert.Empty(published); + } + else + { + Assert.Equal(alreadyRemoved ? ["publish", "cleanup"] : ["persist", "publish", "cleanup"], calls); + var notification = Assert.Single(published); + Assert.Equal(user.Id, notification.UserId); + Assert.Equal(organization.Id, notification.OrganizationId); + Assert.Equal(ChangeType.Removed, notification.ChangeType); + } + + organizations.VerifyAll(); + users.VerifyAll(); + projects.VerifyAll(); + publisher.VerifyAll(); + } + + private static DefaultHttpContext CreateContext() + { + return new DefaultHttpContext + { + User = new ClaimsPrincipal(new ClaimsIdentity([new Claim(ClaimTypes.Role, AuthorizationRoles.GlobalAdmin)], IdentityUtils.UserAuthenticationType)) + }; + } + + private OrganizationHandler CreateHandler(IOrganizationRepository organizations, IUserRepository users, IProjectRepository projects, + ISavedViewRepository savedViews, IMessagePublisher publisher, HttpContext context) + { + var usage = GetService(); + var stripe = new FakeStripeBillingClient(); + var service = new OrganizationService(organizations, projects, savedViews, GetService(), users, + GetService(), stripe, usage, Log); + var billing = new BillingManager(GetService(), organizations, projects, users, GetService(), TimeProvider); + + return new OrganizationHandler(service, organizations, GetService(), GetService(), users, projects, + billing, GetService(), usage, stripe, GetService(), publisher, new ApiMapper(TimeProvider), + GetService(), TimeProvider, new HttpContextAccessor { HttpContext = context }, + new Mock(MockBehavior.Strict).Object, Log); + } +} diff --git a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs index 26d5a75280..2c36f08a19 100644 --- a/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs +++ b/tests/Exceptionless.Tests/Serializer/Models/UserSerializerTests.cs @@ -447,9 +447,12 @@ public void Deserialize_UserWithAllProperties_PreservesAllValues() original.Roles.Add("user"); // Act - string? json = _serializer.SerializeToString(original); + string json = _serializer.SerializeToString(original)!; var deserialized = _serializer.Deserialize(json); - using var apiJson = JsonDocument.Parse(JsonSerializer.Serialize(original, new JsonSerializerOptions().ConfigureExceptionlessApiDefaults())); + var apiOptions = new JsonSerializerOptions().ConfigureExceptionlessApiDefaults(); + using var apiJson = JsonDocument.Parse(JsonSerializer.Serialize(original, apiOptions)); + var apiUser = JsonSerializer.Deserialize(json, apiOptions); + var nestedApiJson = JsonSerializer.SerializeToElement(new { user = (object)original, users = new[] { original } }, apiOptions); // Assert Assert.NotNull(deserialized); @@ -457,6 +460,11 @@ public void Deserialize_UserWithAllProperties_PreservesAllValues() Assert.Equal(authenticationVersion, deserialized.AuthenticationVersion); Assert.Equal(documentVersion, deserialized.Version); Assert.False(apiJson.RootElement.TryGetProperty("version", out _)); + Assert.NotNull(apiUser); + Assert.Null(apiUser.Version); + Assert.Equal(original.FullName, apiUser.FullName); + Assert.False(nestedApiJson.GetProperty("user").TryGetProperty("version", out _)); + Assert.False(nestedApiJson.GetProperty("users")[0].TryGetProperty("version", out _)); Assert.Equal(authenticationVersion, apiJson.RootElement.GetProperty("authentication_version").GetString()); Assert.Equal("Full Test User", deserialized.FullName); Assert.Equal("full@example.com", deserialized.EmailAddress); From 3bbd51ae6d8384e1ae256b6fea78f07ea63e5acc Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Thu, 15 Oct 2026 22:26:20 -0500 Subject: [PATCH 6/6] Verify committed memberships prevent organization cleanup --- .../Api/Handlers/OrganizationHandlerTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs index d7ab1fc89d..fd1451e4a7 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs @@ -83,6 +83,8 @@ public async Task Handle_CreateOrganizationMembershipFails_PreservesCommittedMem } organizations.VerifyAll(); + organizations.Verify(repository => repository.RemoveAsync(It.IsAny(), It.IsAny()), + membershipCommitted || cleanupReadFails ? Times.Never() : Times.Once()); users.VerifyAll(); }