From b311fb44a4d2404c2dfc8cbb1cbccf4adf4c8b1a Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 09:05:47 -0500 Subject: [PATCH 01/15] Protect user updates and clean up orphaned private views --- src/Exceptionless.Core/Jobs/CleanupDataJob.cs | 54 ++++ .../Services/OrganizationService.cs | 2 +- .../Api/Handlers/OrganizationHandler.cs | 99 +++++-- .../Api/Endpoints/SavedViewEndpointTests.cs | 117 ++++++++ .../Api/Endpoints/UserEndpointTests.cs | 155 ++++------- .../Api/Handlers/AuthHandlerTests.cs | 81 +++++- .../Api/Handlers/OrganizationHandlerTests.cs | 262 ++++++++++++++++++ .../Jobs/CleanupDataJobTests.cs | 163 +++++++++++ .../Jobs/WebHooksJobTests.cs | 4 + 9 files changed, 811 insertions(+), 126 deletions(-) create mode 100644 tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs diff --git a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs index 0b043edaeb..53de767113 100644 --- a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs +++ b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs @@ -1,3 +1,4 @@ +using Exceptionless.Core.Authorization; using Exceptionless.Core.Billing; using Exceptionless.Core.Extensions; using Exceptionless.Core.Models; @@ -34,6 +35,7 @@ public class CleanupDataJob : JobWithLockBase, IHealthCheck private readonly IOrganizationRepository _organizationRepository; private readonly OrganizationService _organizationService; private readonly IUserRepository _userRepository; + private readonly ISavedViewRepository _savedViewRepository; private readonly IProjectRepository _projectRepository; private readonly IStackRepository _stackRepository; private readonly IEventRepository _eventRepository; @@ -55,6 +57,7 @@ public CleanupDataJob( IOrganizationRepository organizationRepository, OrganizationService organizationService, IUserRepository userRepository, + ISavedViewRepository savedViewRepository, IProjectRepository projectRepository, IStackRepository stackRepository, IEventRepository eventRepository, @@ -78,6 +81,7 @@ ILoggerFactory loggerFactory _organizationRepository = organizationRepository; _organizationService = organizationService; _userRepository = userRepository; + _savedViewRepository = savedViewRepository; _projectRepository = projectRepository; _stackRepository = stackRepository; _eventRepository = eventRepository; @@ -112,6 +116,7 @@ protected override async Task RunInternalAsync(JobContext context) await CleanupSyntheticOrganizationsAsync(context); await CleanupSyntheticUsersAsync(context); await CleanupSoftDeletedOrganizationsAsync(context); + await CleanupOrphanedPrivateSavedViewsAsync(context); await CleanupSoftDeletedProjectsAsync(context); await CleanupSoftDeletedStacksAsync(context); @@ -122,6 +127,55 @@ protected override async Task RunInternalAsync(JobContext context) return JobResult.Success; } + private async Task CleanupOrphanedPrivateSavedViewsAsync(JobContext context) + { + var views = await _savedViewRepository.FindAsync( + query => query.FieldHasValue(view => view.UserId!) + .FieldNotEquals(view => view.UserId!, String.Empty) + .Include(view => view.Id, view => view.OrganizationId, view => view.UserId!) + .SortAscending(view => view.Id), + options => options.SearchAfterPaging().PageLimit(500)); + + long removed = 0; + while (views.Documents.Count > 0 && !context.CancellationToken.IsCancellationRequested) + { + string[] userIds = views.Documents.Select(view => view.UserId!).Distinct(StringComparer.Ordinal).ToArray(); + string[] organizationIds = views.Documents.Select(view => view.OrganizationId).Distinct(StringComparer.Ordinal).ToArray(); + var users = await _userRepository.GetByIdsAsync(userIds, options => options.Cache(false)); + var usersById = users.ToDictionary(user => user.Id, StringComparer.Ordinal); + var organizations = await _organizationRepository.GetByIdsAsync(organizationIds, options => options.Cache(false)); + var existingOrganizationIds = organizations.Select(organization => organization.Id).ToHashSet(StringComparer.Ordinal); + + var orphaned = views.Documents.Where(view => + !existingOrganizationIds.Contains(view.OrganizationId) + || !usersById.TryGetValue(view.UserId!, out var user) + || (!user.OrganizationIds.Contains(view.OrganizationId) && !user.Roles.Contains(AuthorizationRoles.GlobalAdmin))); + foreach (var group in orphaned.GroupBy(view => (view.OrganizationId, view.UserId))) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + var currentUser = await _userRepository.GetByIdAsync(group.Key.UserId!, options => options.Cache(false)); + var currentOrganization = await _organizationRepository.GetByIdAsync(group.Key.OrganizationId, options => options.Cache(false)); + if (currentUser is not null && currentOrganization is not null + && (currentUser.OrganizationIds.Contains(group.Key.OrganizationId) || currentUser.Roles.Contains(AuthorizationRoles.GlobalAdmin))) + continue; + + // Recheck ownership and visibility so a view changed since the scan is retained. + removed += await _savedViewRepository.RemoveAllAsync( + query => query.Id(group.Select(view => view.Id).ToArray()) + .Organization(group.Key.OrganizationId) + .FieldEquals(view => view.UserId!, group.Key.UserId!), + options => options.Cache(false).ImmediateConsistency()); + } + + await RenewLockAsync(context); + if (!await views.NextPageAsync()) + break; + } + + _logger.LogInformation("Removed {SavedViewCount} orphaned private saved views", removed); + } + private async Task MarkTokensSuspended(JobContext context) { var suspendedOrganizations = await _organizationRepository.FindAsync(q => q.FieldEquals(o => o.IsSuspended, true).OnlyIds(), o => o.SearchAfterPaging().PageLimit(1000)); diff --git a/src/Exceptionless.Core/Services/OrganizationService.cs b/src/Exceptionless.Core/Services/OrganizationService.cs index ae715ebdb1..9072ed3320 100644 --- a/src/Exceptionless.Core/Services/OrganizationService.cs +++ b/src/Exceptionless.Core/Services/OrganizationService.cs @@ -202,11 +202,11 @@ public async Task SoftDeleteOrganizationAsync(Organization organization, string if (organization.IsDeleted) return; + await RemoveUsersAsync(organization, currentUserId); await RemoveTokensAsync(organization); await RemoveWebHooksAsync(organization); await RemoveSavedViewsAsync(organization); await CancelSubscriptionsAsync(organization); - await RemoveUsersAsync(organization, currentUserId); await CleanupProjectNotificationSettingsAsync(organization, []); organization.IsDeleted = true; diff --git a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs index 574c7cb899..db618941c1 100644 --- a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs @@ -20,6 +20,7 @@ using Foundatio.Mediator; using Foundatio.Messaging; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Models; using Stripe; using DataDictionary = Exceptionless.Core.Models.DataDictionary; @@ -641,8 +642,22 @@ public async Task> Handle(AddOrganizationUser message) { if (!user.OrganizationIds.Contains(organization.Id)) { - user.OrganizationIds.Add(organization.Id); - await userRepository.SaveAsync(user, o => o.Cache()); + try + { + bool patched = await userRepository.PatchAsync(user.Id, new ActionPatch(current => current.OrganizationIds.Add(organization.Id)), o => o.Cache()); + + if (!patched) + return Result.NotFound("User not found."); + } + catch (DocumentNotFoundException) + { + return Result.NotFound("User not found."); + } + + user = await userRepository.GetByIdAsync(user.Id, o => o.Cache()); + if (user is null) + return Result.NotFound("User not found."); + await messagePublisher.PublishAsync(new UserMembershipChanged { ChangeType = ChangeType.Added, @@ -684,11 +699,14 @@ public async Task Handle(RemoveOrganizationUser message) if (user is null || !user.OrganizationIds.Contains(message.Id)) { var invite = organization.Invites.FirstOrDefault(i => String.Equals(i.EmailAddress, message.Email, StringComparison.OrdinalIgnoreCase)); - if (invite is null) - return Result.Success(); + if (invite is not null) + { + organization.Invites.Remove(invite); + await repository.SaveAsync(organization, o => o.Cache()); + } - organization.Invites.Remove(invite); - await repository.SaveAsync(organization, o => o.Cache()); + if (user is null || user.Roles.Contains(AuthorizationRoles.GlobalAdmin)) + return Result.Success(); } else { @@ -699,23 +717,38 @@ public async Task Handle(RemoveOrganizationUser message) if (organizationUsers.Total is 1) return Result.BadRequest("An organization must contain at least one user."); - await organizationService.CleanupProjectNotificationSettingsAsync(organization, [user.Id]); - await organizationService.RemoveUserSavedViewsAsync(organization.Id, user.Id); + try + { + bool patched = 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()); - 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 + if (!patched) + return Result.NotFound("User not found."); + } + catch (DocumentNotFoundException) { - ChangeType = ChangeType.Removed, - UserId = user.Id, - OrganizationId = organization.Id - }); + return Result.NotFound("User not found."); + } } + 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(); } @@ -838,8 +871,32 @@ private async Task AddModelAsync(Organization value, HttpContext h var organization = await repository.AddAsync(value, o => o.Cache()); - user.OrganizationIds.Add(organization.Id); - await userRepository.SaveAsync(user, o => o.Cache()); + try + { + bool patched = await userRepository.PatchAsync(user.Id, new ActionPatch(current => current.OrganizationIds.Add(organization.Id)), o => o.Cache()); + + if (!patched) + throw new DocumentNotFoundException("User not found."); + } + catch (DocumentException ex) when (ex is DocumentNotFoundException or VersionConflictDocumentException) + { + try + { + await repository.RemoveAsync(organization); + } + catch (Exception cleanupException) + { + _logger.LogCritical(cleanupException, "Unable to clean up organization {OrganizationId} after adding user {EmailAddress} failed: {Message}", organization.Id, user.EmailAddress, cleanupException.Message); + } + + throw; + } + + var updatedUser = await userRepository.GetByIdAsync(user.Id, o => o.Cache()); + if (updatedUser is null) + throw new DocumentNotFoundException("User not found."); + + httpContext.Request.SetUser(updatedUser); await messagePublisher.PublishAsync(new UserMembershipChanged { UserId = user.Id, diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index b18138251a..ccd02d0023 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Text.Json; using Exceptionless.Core.Models; +using Exceptionless.Core.Authorization; using Exceptionless.Core.Repositories; using Exceptionless.Core.Seed; using Exceptionless.Core.Services; @@ -12,6 +13,7 @@ using FluentRest; using Foundatio.Jobs; using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; using Xunit; namespace Exceptionless.Tests.Api.Endpoints; @@ -2003,6 +2005,79 @@ await SendRequestAsync(r => r + [Fact] + public async Task RemoveUser_PreviouslyCommittedRemoval_CompletesNotificationAndPrivateViewCleanup() + { + // Arrange + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(user); + var projects = GetService(); + var project = await projects.GetByIdAsync(SampleDataService.TEST_PROJECT_ID); + Assert.NotNull(project); + project.NotificationSettings[user.Id] = new NotificationSettings { ReportNewErrors = true }; + await projects.SaveAsync(project, options => options.ImmediateConsistency()); + var view = await _savedViewRepository.AddAsync(new SavedView + { + OrganizationId = SampleDataService.TEST_ORG_ID, UserId = user.Id, CreatedByUserId = user.Id, + Name = "Interrupted cleanup", Slug = "interrupted-cleanup", ViewType = "events" + }, options => options.ImmediateConsistency()); + user.OrganizationIds.Remove(SampleDataService.TEST_ORG_ID); + await _userRepository.SaveAsync(user); + + // Act + await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() + .AppendPaths("organizations", SampleDataService.TEST_ORG_ID, "users", user.EmailAddress).StatusCodeShouldBeOk()); + var remainingView = await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false)); + var updatedProject = await projects.GetByIdAsync(project.Id, options => options.Cache(false)); + + // Assert + Assert.Null(remainingView); + Assert.NotNull(updatedProject); + Assert.False(updatedProject.NotificationSettings.ContainsKey(user.Id)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RemoveUser_NonmemberGlobalAdministrator_PreservesPrivateViews(bool hasInvite) + { + // Arrange + var administrator = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(administrator); + Assert.Contains(AuthorizationRoles.GlobalAdmin, administrator.Roles); + administrator.OrganizationIds.Remove(SampleDataService.TEST_ORG_ID); + await _userRepository.SaveAsync(administrator); + var organization = await _organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID); + Assert.NotNull(organization); + if (hasInvite) + { + organization.Invites.Add(new Invite + { + EmailAddress = administrator.EmailAddress, + Token = Guid.NewGuid().ToString("N"), + DateAdded = TimeProvider.GetUtcNow().UtcDateTime + }); + await _organizationRepository.SaveAsync(organization); + } + + var view = await _savedViewRepository.AddAsync(new SavedView + { + OrganizationId = organization.Id, UserId = administrator.Id, CreatedByUserId = administrator.Id, + Name = "Administrator private view", Slug = "administrator-private-view", ViewType = "events" + }, options => options.ImmediateConsistency()); + + // Act + await SendRequestAsync(request => request.Delete().AsTestOrganizationUser() + .AppendPaths("organizations", organization.Id, "users", administrator.EmailAddress).StatusCodeShouldBeOk()); + var retained = await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false)); + var updatedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false)); + + // Assert + Assert.NotNull(retained); + Assert.NotNull(updatedOrganization); + Assert.DoesNotContain(updatedOrganization.Invites, invite => invite.EmailAddress == administrator.EmailAddress); + } + [Fact] public async Task RemoveUser_DeletesPrivateSavedViews_ButPreservesOrganizationWideViews() { @@ -2065,6 +2140,48 @@ await SendRequestAsync(r => r Assert.Contains(testUser.SavedViewOrders, preference => preference.OrganizationId == SampleDataService.FREE_ORG_ID); } + [Fact] + public async Task SoftDeleteOrganization_UserVersionConflict_PreservesOrganizationResources() + { + // Arrange + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + var organization = await _organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID); + Assert.NotNull(user); + Assert.NotNull(organization); + var view = await _savedViewRepository.AddAsync(new SavedView + { + OrganizationId = organization.Id, CreatedByUserId = user.Id, + Name = "Retained on conflict", Slug = "retained-on-conflict", ViewType = "events" + }, options => options.ImmediateConsistency()); + bool changed = false; + using var subscription = _userRepository.DocumentsSaving.AddHandler(async (_, args) => + { + if (changed || !args.Documents.Any(document => document.Value.Id == user.Id)) + return; + + changed = true; + var current = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + Assert.NotNull(current); + current.FullName = "Concurrent profile update"; + await _userRepository.SaveAsync(current); + }); + + // Act + var exception = await Record.ExceptionAsync(() => _organizationService.SoftDeleteOrganizationAsync(organization, user.Id)); + var retained = await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false)); + var storedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false)); + var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + + // Assert + Assert.True(changed); + Assert.IsType(exception); + Assert.NotNull(retained); + Assert.NotNull(storedOrganization); + Assert.False(storedOrganization.IsDeleted); + Assert.NotNull(storedUser); + Assert.Equal("Concurrent profile update", storedUser.FullName); + } + [Fact] public async Task SoftDeleteOrganization_WithSavedViews_RemovesAllSavedViews() { diff --git a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs index 8493e66ab5..f44267d7fb 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/UserEndpointTests.cs @@ -9,7 +9,6 @@ using Exceptionless.Core.Repositories.Configuration; using Exceptionless.Core.Services; using Exceptionless.Core.Utility; -using Exceptionless.Core.Validation; using Exceptionless.Tests.Extensions; using Exceptionless.Web.Api.Results; using Exceptionless.Web.Models; @@ -36,12 +35,6 @@ public UserEndpointTests(ITestOutputHelper output, AppWebHostFactory factory) : _oauthTokenRepository = GetService(); } - protected override void RegisterServices(IServiceCollection services) - { - base.RegisterServices(services); - services.ReplaceSingleton(); - } - protected override async Task ResetDataAsync() { await base.ResetDataAsync(); @@ -426,13 +419,14 @@ public async Task GetCurrentUserAsync_TestOrganizationUser_ReturnsCurrentUser() [Fact] public async Task GetCurrentUserAsync_CachedUser_ReturnsCachedProfileUntilInvalidated() { - // Arrange: give the cache a distinct profile without changing the stored user. + // Arrange var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); Assert.NotNull(user); - string persistedName = user.FullName; - user.FullName = "Cached profile"; - var repository = Assert.IsType(_userRepository); - await repository.CacheUserAsync(user); + string cachedName = user.FullName; + const string persistedName = "Updated stored profile"; + var configuration = GetService(); + var update = await configuration.Client.UpdateAsync(configuration.Users.VersionedName, user.Id, + request => request.Doc(new { full_name = persistedName }), TestCancellationToken); // Act var cached = await GetTestOrganizationUserAsync(); @@ -440,7 +434,8 @@ public async Task GetCurrentUserAsync_CachedUser_ReturnsCachedProfileUntilInvali var refreshed = await GetTestOrganizationUserAsync(); // Assert - Assert.Equal("Cached profile", cached.FullName); + Assert.True(update.IsValidResponse); + Assert.Equal(cachedName, cached.FullName); Assert.Equal(persistedName, refreshed.FullName); } @@ -806,84 +801,67 @@ public async Task PatchAsync_NullFullName_PreservesExistingName() } [Theory] - [InlineData(true)] [InlineData(false)] - public async Task PatchAsync_TourRecordedAfterProfileRead_PreservesProgressAndUnchangedFields(bool updateFullName) + [InlineData(true)] + public async Task PatchAsync_FieldUpdate_PreservesAuthenticationVersion(bool script) { // Arrange - var currentUser = await GetTestOrganizationUserAsync(); - var user = await _userRepository.GetByIdAsync(currentUser.Id, o => o.Cache(false)); + string authenticationVersion = StringExtensions.GetNewToken(); + var profile = await GetTestOrganizationUserAsync(); + var user = await _userRepository.GetByIdAsync(profile.Id, o => o.Cache(false)); Assert.NotNull(user); - var repository = Assert.IsType(_userRepository); - var pause = repository.PauseNextRead(user.Id); - var recordedUtc = TimeProvider.GetUtcNow().UtcDateTime; - object changes = updateFullName - ? new { FullName = "Updated Name" } - : new { EmailNotificationsEnabled = false }; - var update = SendRequestAsAsync(r => r.Patch().AsGlobalAdminUser() - .AppendPaths("users", user.Id).Content(changes).StatusCodeShouldBeOk()); - - // Act: save progress after the handler's read, then allow its profile write. - try - { - await pause.SnapshotRead.Task.WaitAsync(TimeSpan.FromSeconds(10), TestCancellationToken); - await _userRepository.RecordProductTourAsync(user, "app_overview", recordedUtc); - await _userRepository.GetByIdAsync(user.Id, o => o.Cache()); - await _userRepository.GetByEmailAddressAsync(user.EmailAddress); - } - finally - { - pause.ResumeRead.TrySetResult(); - } - var response = await update; + 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)); - var byId = await _userRepository.GetByIdAsync(user.Id, o => o.Cache()); - var byEmail = await _userRepository.GetByEmailAddressAsync(user.EmailAddress); // Assert - Assert.NotNull(response); - Assert.Equal(updateFullName ? "Updated Name" : user.FullName, response.FullName); - Assert.Equal(updateFullName && user.EmailNotificationsEnabled, response.EmailNotificationsEnabled); - foreach (var result in new[] { persisted, byId, byEmail }) - { - Assert.NotNull(result); - Assert.Equal(recordedUtc, result.ProductTours["app_overview"].GetDateTime()); - Assert.Equal(response.FullName, result.FullName); - Assert.Equal(response.EmailNotificationsEnabled, result.EmailNotificationsEnabled); - Assert.Equal(user.EmailAddress, result.EmailAddress); - Assert.Equal(user.CreatedUtc, result.CreatedUtc); - Assert.Equal(user.Roles, result.Roles); - } + 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 UpdateUserAsync_DeletedAfterProfileRead_ReturnsNotFound(bool usePut) + public async Task PatchAllAsync_InterveningAuthenticationChange_PreservesNewGeneration(bool jsonPatch) { - var currentUser = await GetTestOrganizationUserAsync(); - var user = await _userRepository.GetByIdAsync(currentUser.Id, o => o.Cache(false)); - Assert.NotNull(user); - var repository = Assert.IsType(_userRepository); - var pause = repository.PauseNextRead(user.Id); - var request = SendRequestAsync(r => + // 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) => { - r = usePut ? r.Put() : r.Patch(); - r.AsGlobalAdminUser().AppendPaths("users", user.Id) - .Content(new { FullName = "Updated Name" }).StatusCodeShouldBeNotFound(); + 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); - try - { - await pause.SnapshotRead.Task.WaitAsync(TimeSpan.FromSeconds(10), TestCancellationToken); - await _userRepository.RemoveAsync(user); - } - finally - { - pause.ResumeRead.TrySetResult(); - } + // Act + await _userRepository.PatchAllAsync(q => q.Id(user.Id), patch, o => o.ImmediateConsistency()); + var persisted = await _userRepository.GetByIdAsync(user.Id, o => o.Cache(false)); - await request; + // Assert + Assert.True(intervened); + Assert.NotNull(persisted); + Assert.Equal(authenticationVersion, persisted.AuthenticationVersion); + Assert.Equal(updatedName, persisted.FullName); } [Fact] @@ -1211,35 +1189,6 @@ private async Task GetTestOrganizationUserAsync() return user; } - private sealed class PausingProfileUserRepository(ExceptionlessElasticConfiguration configuration, MiniValidationValidator validator, AppOptions options) - : UserRepository(configuration, validator, options) - { - private ProfileReadPause? _pause; - - public ProfileReadPause PauseNextRead(string userId) => _pause = new ProfileReadPause(userId); - - public Task CacheUserAsync(User user) => AddDocumentsToCacheAsync(user, new CommandOptions().Cache(), false); - - public override async Task GetByIdAsync(Id id, ICommandOptions? options = null) - { - var user = await base.GetByIdAsync(id, options); - if (_pause is { } pause && user?.Id == pause.UserId) - { - _pause = null; - pause.SnapshotRead.TrySetResult(); - await pause.ResumeRead.Task; - } - return user; - } - } - - private sealed class ProfileReadPause(string userId) - { - public string UserId { get; } = userId; - public TaskCompletionSource SnapshotRead { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); - public TaskCompletionSource ResumeRead { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); - } - private static MultipartFormDataContent CreateProfileImageContent() { byte[] bytes = new byte[256 * 1024]; diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index fff2299667..531404f0d3 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -1,3 +1,5 @@ +using Foundatio.Repositories.Exceptions; +using Foundatio.Repositories.Elasticsearch; using System.Security.Claims; using Exceptionless.Core; using Exceptionless.Core.Authentication; @@ -29,6 +31,84 @@ public sealed class AuthHandlerTests : TestWithServices { public AuthHandlerTests(ITestOutputHelper output) : base(output) { } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Handle_ExternalAccountDestinationSaveFails_DoesNotRestoreOwnership(bool ambiguousFailure) + { + // 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 context = new DefaultHttpContext { User = new ClaimsPrincipal(new ClaimsIdentity(IdentityUtils.UserAuthenticationType)) }; + context.Request.SetUser(destination); + Exception failure = ambiguousFailure + ? 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, It.IsAny())).ReturnsAsync(displaced); + users.Setup(repository => repository.GetByIdAsync(displaced.Id, It.IsAny>())).ReturnsAsync(displaced); + users.Setup(repository => repository.GetByIdAsync(destination.Id, It.IsAny>())).ReturnsAsync(destination); + 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, 2) + }; + return Task.FromResult(user); + }); + users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, destination)), It.IsAny>())) + .ThrowsAsync(failure); + 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 Record.ExceptionAsync(async () => + { + _ = await 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.DoesNotContain(currentOwner.OAuthAccounts, account => String.Equals(account.Provider, provider, StringComparison.Ordinal) + && String.Equals(account.ProviderUserId, providerId, StringComparison.Ordinal)); + users.Verify(repository => repository.GetByIdAsync(displaced.Id, It.IsAny>()), Times.Once()); + users.Verify(repository => repository.GetByIdAsync(destination.Id, It.IsAny>()), Times.Once()); + users.Verify(repository => repository.SaveAsync(It.IsAny(), It.IsAny>()), Times.Exactly(2)); + users.VerifyAll(); + tokens.VerifyAll(); + oauthTokens.VerifyAll(); + providerClient.VerifyAll(); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -78,7 +158,6 @@ public async Task Handle_ExternalAccountLockUnavailable_IsRejectedBeforeReposito } } - [Theory] [InlineData(false, false, false)] [InlineData(true, false, false)] diff --git a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs new file mode 100644 index 0000000000..3f76bbabc6 --- /dev/null +++ b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs @@ -0,0 +1,262 @@ +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.Mediator; +using Foundatio.Repositories; +using Foundatio.Repositories.Exceptions; +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_AddOrganizationUser_UsesPersistedUserAndChecksPatchResult(bool patched, bool userDeleted) + { + // Arrange + var organization = new Organization { Id = "000000000000000000000001", Name = "Test", MaxUsers = -1 }; + var user = new User { Id = "000000000000000000000002", EmailAddress = "added@example.test", Version = "1:1" }; + var persisted = user with { Version = "1:2", OrganizationIds = new HashSet { organization.Id } }; + var context = CreateContext(); + context.Request.SetUser(user); + 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.GetByOrganizationIdAsync(organization.Id, null)).ReturnsAsync(new FindResults([], 1)); + users.Setup(repository => repository.GetByEmailAddressAsync(user.EmailAddress)).ReturnsAsync(user); + users.Setup(repository => repository.PatchAsync(user.Id, It.IsAny(), It.IsAny>())) + .Returns(() => userDeleted ? Task.FromException(new DocumentNotFoundException("User deleted")) : Task.FromResult(patched)); + users.Setup(repository => repository.GetByIdAsync(user.Id, It.IsAny>())).ReturnsAsync(persisted); + var publisher = new Mock(); + var mailer = new Mock(); + var handler = CreateHandler(organizations.Object, users.Object, new Mock().Object, + new Mock().Object, publisher.Object, context, mailer.Object); + + // Act + var result = await handler.Handle(new AddOrganizationUser(organization.Id, user.EmailAddress, context)); + + // Assert + Assert.Empty(user.OrganizationIds); + Assert.Equal(patched ? ResultStatus.Success : ResultStatus.NotFound, result.Status); + mailer.Verify(client => client.SendOrganizationAddedAsync(It.IsAny(), organization, persisted), patched ? Times.Once() : Times.Never()); + publisher.Verify(client => client.PublishAsync(typeof(UserMembershipChanged), It.IsAny(), null, default), patched ? Times.Once() : Times.Never()); + } + + [Theory] + [InlineData("transport", false)] + [InlineData("conflict", false)] + [InlineData("conflict", true)] + [InlineData("missing", false)] + [InlineData("not-patched", false)] + public async Task Handle_CreateOrganizationMembershipFails_CleansUpOnlyDefiniteRejections(string outcome, bool cleanupFails) + { + // 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" }; + bool definiteRejection = outcome != "transport"; + Exception failure = outcome switch + { + "conflict" => new VersionConflictDocumentException("Membership conflict"), + "missing" => new DocumentNotFoundException("User deleted"), + _ => new IOException("Membership transport failure") + }; + var cleanupFailure = new IOException("Synthetic cleanup 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 (definiteRejection) + organizations.Setup(repository => repository.RemoveAsync(organization, (ICommandOptions?)null)) + .Returns(() => cleanupFails ? Task.FromException(cleanupFailure) : Task.CompletedTask); + + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.PatchAsync(user.Id, It.IsAny(), It.IsAny>())) + .Returns(() => outcome == "not-patched" ? Task.FromResult(false) : Task.FromException(failure)); + 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 + if (outcome == "not-patched") + Assert.IsType(actual); + else + Assert.Same(failure, actual); + if (cleanupFails) + { + 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(); + organizations.Verify(repository => repository.RemoveAsync(It.IsAny(), It.IsAny()), + definiteRejection ? Times.Once() : Times.Never()); + users.VerifyAll(); + users.VerifyNoOtherCalls(); + } + + [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 + if (persistenceFails) + { + Assert.Same(failure, actual); + Assert.Equal(["persist"], calls); + Assert.Empty(published); + } + else + { + Assert.Same(failure, actual); + 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(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Handle_RemoveOrganizationUserPatchRejected_DoesNotPublishOrCleanUp(bool userDeleted) + { + // Arrange + var organization = new Organization { Id = "000000000000000000000001", Name = "Test" }; + var user = new User { Id = "000000000000000000000002", EmailAddress = "removed@example.test" }; + user.OrganizationIds.Add(organization.Id); + 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); + users.Setup(repository => repository.GetByOrganizationIdAsync(organization.Id, null)).ReturnsAsync(new FindResults([], 2)); + users.Setup(repository => repository.PatchAsync(user.Id, It.IsAny(), It.IsAny>())) + .Returns(() => userDeleted ? Task.FromException(new DocumentNotFoundException("User deleted")) : Task.FromResult(false)); + var context = CreateContext(); + 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 result = await handler.Handle(new RemoveOrganizationUser(organization.Id, user.EmailAddress, context)); + + // Assert + Assert.Equal(ResultStatus.NotFound, result.Status); + Assert.Contains(organization.Id, user.OrganizationIds); + users.VerifyAll(); + users.VerifyNoOtherCalls(); + } + + 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, IMailer? mailer = null) + { + 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, mailer ?? GetService(), publisher, new ApiMapper(TimeProvider), + GetService(), TimeProvider, new HttpContextAccessor { HttpContext = context }, + new Mock(MockBehavior.Strict).Object, Log); + } +} diff --git a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs index 64d1ab18ac..e3d5639a97 100644 --- a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs +++ b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs @@ -60,6 +60,140 @@ public CleanupDataJobTests(ITestOutputHelper output, AppWebHostFactory factory) _fileStorage = GetService(); } + [Fact] + public async Task RunAsync_OrphanedPrivateSavedViews_RemovesEveryPageAndPreservesAccessibleViews() + { + // Arrange + var savedViews = GetService(); + var organization = await _organizationRepository.AddAsync(_organizationData.GenerateOrganization(_billingManager, _plans, id: TestConstants.OrganizationId)); + var otherOrganization = await _organizationRepository.AddAsync(_organizationData.GenerateOrganization(_billingManager, _plans, id: TestConstants.OrganizationId2)); + var deletedUser = _userData.GenerateUser(generateId: true, organizationId: organization.Id); + var formerMember = _userData.GenerateUser(generateId: true, organizationId: organization.Id); + formerMember.OrganizationIds.Add(otherOrganization.Id); + var member = _userData.GenerateUser(generateId: true, organizationId: organization.Id); + var disabledMember = _userData.GenerateUser(generateId: true, organizationId: organization.Id) with { IsActive = false }; + var administrator = _userData.GenerateUser(generateId: true, organizationId: otherOrganization.Id, roles: [AuthorizationRoles.GlobalAdmin]); + var users = new[] { deletedUser, formerMember, member, disabledMember, administrator }; + foreach (var user in users) + user.MarkEmailAddressVerified(); + await _userRepository.AddAsync(users); + + var orphaned = Enumerable.Range(0, 501) + .Select(index => CreateView(organization.Id, deletedUser.Id, $"deleted-owner-{index}")) + .ToList(); + orphaned.Add(CreateView(organization.Id, formerMember.Id, "former-member")); + var removedOrganization = await _organizationRepository.AddAsync(_organizationData.GenerateOrganization(_billingManager, _plans, generateId: true)); + member.OrganizationIds.Add(removedOrganization.Id); + await _userRepository.SaveAsync(member); + orphaned.Add(CreateView(removedOrganization.Id, member.Id, "removed-organization")); + var retained = new[] + { + CreateView(organization.Id, null, "shared-view", deletedUser.Id), + CreateView(organization.Id, member.Id, "member-view"), + CreateView(otherOrganization.Id, formerMember.Id, "other-membership"), + CreateView(organization.Id, disabledMember.Id, "disabled-member"), + CreateView(organization.Id, administrator.Id, "administrator-view") + }; + await savedViews.AddAsync(orphaned.Concat(retained), options => options.ImmediateConsistency()); + await _userRepository.RemoveAsync(deletedUser); + formerMember.OrganizationIds.Remove(organization.Id); + await _userRepository.SaveAsync(formerMember); + await _organizationRepository.RemoveAsync(removedOrganization); + + // Act + var result = await _job.RunAsync(TestCancellationToken); + await RefreshDataAsync(); + var remaining = await savedViews.GetAllAsync(options => options.PageLimit(1000)); + + // Assert + Assert.True(result.IsSuccess); + Assert.Equal(retained.Select(view => view.Id).Order(), remaining.Documents.Select(view => view.Id).Order()); + Assert.Equal(retained.Length, remaining.Total); + + SavedView CreateView(string organizationId, string? userId, string name, string? createdByUserId = null) => new() + { + OrganizationId = organizationId, + UserId = userId, + CreatedByUserId = createdByUserId ?? userId!, + Name = name, + Slug = name, + ViewType = "events" + }; + } + + [Fact] + public async Task RunAsync_PrivateViewMadeSharedAfterScan_PreservesView() + { + // Arrange + var savedViews = Assert.IsType(GetService()); + var organization = await _organizationRepository.AddAsync(_organizationData.GenerateOrganization(_billingManager, _plans)); + var view = await savedViews.AddAsync(new SavedView + { + OrganizationId = organization.Id, + UserId = TestConstants.UserId, + CreatedByUserId = TestConstants.UserId, + Name = "Former private view", + Slug = "former-private-view", + ViewType = "events" + }, options => options.ImmediateConsistency()); + bool madeShared = false; + using var subscription = savedViews.AfterQuery.AddHandler(async (_, args) => + { + if (madeShared || args.Result is not FindResults results || !results.Documents.Any(candidate => candidate.Id == view.Id)) + return; + + madeShared = true; + await savedViews.SaveAsync(view with { UserId = null }, options => options.ImmediateConsistency()); + }); + + // Act + var result = await _job.RunAsync(TestCancellationToken); + var retained = await savedViews.GetByIdAsync(view.Id, options => options.Cache(false)); + + // Assert + Assert.True(result.IsSuccess); + Assert.True(madeShared); + Assert.NotNull(retained); + Assert.Null(retained.UserId); + } + + [Fact] + public async Task RunAsync_MembershipRestoredAfterOwnerRead_PreservesPrivateView() + { + // Arrange + var savedViews = GetService(); + var organizations = Assert.IsType(_organizationRepository); + var organization = await organizations.AddAsync(_organizationData.GenerateOrganization(_billingManager, _plans)); + var user = _userData.GenerateUser(); + user.OrganizationIds.Clear(); + user.MarkEmailAddressVerified(); + await _userRepository.AddAsync(user); + var view = await savedViews.AddAsync(new SavedView + { + OrganizationId = organization.Id, UserId = user.Id, CreatedByUserId = user.Id, + Name = "Restored member", Slug = "restored-member", ViewType = "events" + }, options => options.ImmediateConsistency()); + bool restored = false; + using var subscription = organizations.BeforeGet.AddHandler(async (_, _) => + { + if (restored) + return; + + restored = true; + user.OrganizationIds.Add(organization.Id); + await _userRepository.SaveAsync(user); + }); + + // Act + var result = await _job.RunAsync(TestCancellationToken); + var retained = await savedViews.GetByIdAsync(view.Id, options => options.Cache(false)); + + // Assert + Assert.True(result.IsSuccess); + Assert.True(restored); + Assert.NotNull(retained); + } + [Fact] public async Task RunAsync_AbandonedOAuthApplications_RemovesOnlyOldAutomaticRegistrationsWithoutAuthorization() { @@ -325,6 +459,26 @@ public async Task CanCleanupSoftDeletedOrganization_MultiTenant_OnlyDeletedOrgan var project1 = await _projectRepository.AddAsync(_projectData.GenerateProject(id: TestConstants.ProjectId, organizationId: organization1.Id), o => o.ImmediateConsistency()); var project2 = await _projectRepository.AddAsync(_projectData.GenerateProject(generateId: true, organizationId: organization2.Id), o => o.ImmediateConsistency()); + var user = _userData.GenerateUser(); + user.MarkEmailAddressVerified(); + user.OrganizationIds.Clear(); + user.OrganizationIds.UnionWith([organization1.Id, organization2.Id]); + await _userRepository.AddAsync(user); + project1.NotificationSettings[user.Id] = new NotificationSettings { ReportNewErrors = true }; + project2.NotificationSettings[user.Id] = new NotificationSettings { ReportNewErrors = true }; + await _projectRepository.SaveAsync([project1, project2]); + var savedViews = GetService(); + var deletedView = await savedViews.AddAsync(new SavedView + { + OrganizationId = organization1.Id, UserId = user.Id, CreatedByUserId = user.Id, + Name = "Deleted private view", Slug = "deleted-private-view", ViewType = "events" + }, options => options.ImmediateConsistency()); + var retainedView = await savedViews.AddAsync(new SavedView + { + OrganizationId = organization2.Id, UserId = user.Id, CreatedByUserId = user.Id, + Name = "Retained private view", Slug = "retained-private-view", ViewType = "events" + }, options => options.ImmediateConsistency()); + var stack1 = await _stackRepository.AddAsync(_stackData.GenerateStack(id: TestConstants.StackId, organizationId: organization1.Id, projectId: project1.Id), o => o.ImmediateConsistency()); var stack2 = await _stackRepository.AddAsync(_stackData.GenerateStack(generateId: true, organizationId: organization2.Id, projectId: project2.Id), o => o.ImmediateConsistency()); @@ -359,6 +513,15 @@ public async Task CanCleanupSoftDeletedOrganization_MultiTenant_OnlyDeletedOrgan Assert.Null(await _projectRepository.GetByIdAsync(deletedProject.Id, o => o.IncludeSoftDeletes())); Assert.Null(await _stackRepository.GetByIdAsync(stack1.Id, o => o.IncludeSoftDeletes())); Assert.Null(await _eventRepository.GetByIdAsync(event1.Id, o => o.IncludeSoftDeletes())); + Assert.Null(await savedViews.GetByIdAsync(deletedView.Id)); + Assert.NotNull(await savedViews.GetByIdAsync(retainedView.Id)); + var retainedUser = await _userRepository.GetByIdAsync(user.Id); + Assert.NotNull(retainedUser); + Assert.DoesNotContain(organization1.Id, retainedUser.OrganizationIds); + Assert.Contains(organization2.Id, retainedUser.OrganizationIds); + var retainedProject = await _projectRepository.GetByIdAsync(project2.Id); + Assert.NotNull(retainedProject); + Assert.Contains(user.Id, retainedProject.NotificationSettings.Keys); Assert.NotNull(await _organizationRepository.GetByIdAsync(organization2.Id)); Assert.NotNull(await _projectRepository.GetByIdAsync(project2.Id)); diff --git a/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs b/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs index 858938188d..0da0106109 100644 --- a/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs +++ b/tests/Exceptionless.Tests/Jobs/WebHooksJobTests.cs @@ -25,6 +25,10 @@ namespace Exceptionless.Tests.Jobs; +[CollectionDefinition("Webhook telemetry", DisableParallelization = true)] +public sealed class WebHookTelemetryCollection; + +[Collection("Webhook telemetry")] public sealed class WebHooksJobTests : TestWithServices { private const string DestinationUrl = "https://example.com/webhook"; From ee7ef3e6f262bafbda5689e93e93931f66cc9e76 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 09:24:01 -0500 Subject: [PATCH 02/15] Persist organization deletion before removing members --- src/Exceptionless.Core/Jobs/CleanupDataJob.cs | 2 +- src/Exceptionless.Core/Models/Organization.cs | 6 +++ .../Services/OrganizationService.cs | 7 +-- .../Api/Endpoints/SavedViewEndpointTests.cs | 24 +++++++-- .../Jobs/CleanupDataJobTests.cs | 4 ++ .../Services/OrganizationServiceTests.cs | 50 +++++++++++++++++++ 6 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs index 53de767113..cfc523028a 100644 --- a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs +++ b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs @@ -405,7 +405,7 @@ private async Task RemoveOrganizationAsync(Organization organization, JobContext await _organizationService.RemoveWebHooksAsync(organization); await _organizationService.RemoveSavedViewsAsync(organization); await _organizationService.CancelSubscriptionsAsync(organization); - await _organizationService.RemoveUsersAsync(organization, null); + await _organizationService.RemoveUsersAsync(organization, organization.DeletedByUserId); await RenewLockAsync(context); long removedEvents = await _eventRepository.RemoveAllByOrganizationIdAsync(organization.Id); diff --git a/src/Exceptionless.Core/Models/Organization.cs b/src/Exceptionless.Core/Models/Organization.cs index 4cfa22dac8..0bce9ebb2f 100644 --- a/src/Exceptionless.Core/Models/Organization.cs +++ b/src/Exceptionless.Core/Models/Organization.cs @@ -201,6 +201,12 @@ public Organization() public DateTime UpdatedUtc { get; set; } public bool IsDeleted { get; set; } + /// + /// User whose account must be preserved when organization deletion resumes. + /// + [ObjectId] + public string? DeletedByUserId { get; set; } + string IOwnedByOrganization.OrganizationId { get { return Id; } set { Id = value; } } public IEnumerable Validate(ValidationContext validationContext) diff --git a/src/Exceptionless.Core/Services/OrganizationService.cs b/src/Exceptionless.Core/Services/OrganizationService.cs index 9072ed3320..df915bdd30 100644 --- a/src/Exceptionless.Core/Services/OrganizationService.cs +++ b/src/Exceptionless.Core/Services/OrganizationService.cs @@ -202,15 +202,16 @@ public async Task SoftDeleteOrganizationAsync(Organization organization, string if (organization.IsDeleted) return; + organization.IsDeleted = true; + organization.DeletedByUserId = currentUserId; + await _organizationRepository.SaveAsync(organization, options => options.Cache()); + await RemoveUsersAsync(organization, currentUserId); await RemoveTokensAsync(organization); await RemoveWebHooksAsync(organization); await RemoveSavedViewsAsync(organization); await CancelSubscriptionsAsync(organization); await CleanupProjectNotificationSettingsAsync(organization, []); - - organization.IsDeleted = true; - await _organizationRepository.SaveAsync(organization); } private async Task> GetValidNotificationUserIdsAsync(string organizationId, IReadOnlyCollection userIds, CancellationToken cancellationToken) diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index ccd02d0023..f7592929bd 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Text.Json; using Exceptionless.Core.Models; +using Exceptionless.Core.Jobs; using Exceptionless.Core.Authorization; using Exceptionless.Core.Repositories; using Exceptionless.Core.Seed; @@ -2141,13 +2142,16 @@ await SendRequestAsync(r => r } [Fact] - public async Task SoftDeleteOrganization_UserVersionConflict_PreservesOrganizationResources() + public async Task SoftDeleteOrganization_UserVersionConflict_CleanupJobCompletesDeletionAndPreservesRequester() { // Arrange var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); var organization = await _organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID); Assert.NotNull(user); Assert.NotNull(organization); + user.OrganizationIds.Clear(); + user.OrganizationIds.Add(organization.Id); + await _userRepository.SaveAsync(user); var view = await _savedViewRepository.AddAsync(new SavedView { OrganizationId = organization.Id, CreatedByUserId = user.Id, @@ -2169,7 +2173,7 @@ public async Task SoftDeleteOrganization_UserVersionConflict_PreservesOrganizati // Act var exception = await Record.ExceptionAsync(() => _organizationService.SoftDeleteOrganizationAsync(organization, user.Id)); var retained = await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false)); - var storedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false)); + var storedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false).IncludeSoftDeletes()); var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); // Assert @@ -2177,9 +2181,23 @@ public async Task SoftDeleteOrganization_UserVersionConflict_PreservesOrganizati Assert.IsType(exception); Assert.NotNull(retained); Assert.NotNull(storedOrganization); - Assert.False(storedOrganization.IsDeleted); + Assert.True(storedOrganization.IsDeleted); Assert.NotNull(storedUser); Assert.Equal("Concurrent profile update", storedUser.FullName); + + // Act - the existing cleanup job resumes the persisted deletion. + await RefreshDataAsync(); + var result = await GetService().RunAsync(TestCancellationToken); + var requester = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + var deletedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false).IncludeSoftDeletes()); + + // Assert + Assert.True(result.IsSuccess); + Assert.Null(deletedOrganization); + Assert.Null(await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false))); + Assert.NotNull(requester); + Assert.Equal("Concurrent profile update", requester.FullName); + Assert.DoesNotContain(organization.Id, requester.OrganizationIds); } [Fact] diff --git a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs index e3d5639a97..95175732f6 100644 --- a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs +++ b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs @@ -464,6 +464,9 @@ public async Task CanCleanupSoftDeletedOrganization_MultiTenant_OnlyDeletedOrgan user.OrganizationIds.Clear(); user.OrganizationIds.UnionWith([organization1.Id, organization2.Id]); await _userRepository.AddAsync(user); + var legacyUser = _userData.GenerateUser(generateId: true, organizationId: organization1.Id); + legacyUser.MarkEmailAddressVerified(); + await _userRepository.AddAsync(legacyUser); project1.NotificationSettings[user.Id] = new NotificationSettings { ReportNewErrors = true }; project2.NotificationSettings[user.Id] = new NotificationSettings { ReportNewErrors = true }; await _projectRepository.SaveAsync([project1, project2]); @@ -515,6 +518,7 @@ public async Task CanCleanupSoftDeletedOrganization_MultiTenant_OnlyDeletedOrgan Assert.Null(await _eventRepository.GetByIdAsync(event1.Id, o => o.IncludeSoftDeletes())); Assert.Null(await savedViews.GetByIdAsync(deletedView.Id)); Assert.NotNull(await savedViews.GetByIdAsync(retainedView.Id)); + Assert.Null(await _userRepository.GetByIdAsync(legacyUser.Id)); var retainedUser = await _userRepository.GetByIdAsync(user.Id); Assert.NotNull(retainedUser); Assert.DoesNotContain(organization1.Id, retainedUser.OrganizationIds); diff --git a/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs b/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs index cf09fe90d8..4ca11982d3 100644 --- a/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs +++ b/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs @@ -2,9 +2,13 @@ using Exceptionless.Core.Billing; using Exceptionless.Core.Extensions; using Exceptionless.Core.Models; +using Exceptionless.Core.Repositories; using Exceptionless.Core.Services; using Exceptionless.Tests.Utility; +using Foundatio.Repositories; +using Foundatio.Repositories.Models; using Microsoft.Extensions.DependencyInjection; +using Moq; using Stripe; using Xunit; @@ -20,6 +24,52 @@ protected override void RegisterServices(IServiceCollection services, AppOptions services.ReplaceSingleton(); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SoftDeleteOrganization_Failure_PersistsDeletionBeforeChangingMembers(bool deletionSaveFails) + { + // Arrange + var organization = new Organization { Id = TestConstants.OrganizationId, Name = "Deleting organization" }; + var persistenceFailure = new IOException("Deletion marker failed"); + var cleanupFailure = new IOException("Token cleanup failed"); + bool persistedDeleted = false; + var organizations = new Mock(MockBehavior.Strict); + organizations.Setup(repository => repository.SaveAsync(organization, It.IsAny>())) + .Returns(() => + { + if (deletionSaveFails) + return Task.FromException(persistenceFailure); + + persistedDeleted = organization.IsDeleted; + return Task.FromResult(organization); + }); + var users = new Mock(MockBehavior.Strict); + users.Setup(repository => repository.GetByOrganizationIdAsync(organization.Id, It.IsAny>())) + .ReturnsAsync(FindResults.Empty); + var tokens = new Mock(MockBehavior.Strict); + tokens.Setup(repository => repository.RemoveAllByOrganizationIdAsync(organization.Id)).ThrowsAsync(cleanupFailure); + var service = new OrganizationService(organizations.Object, + new Mock(MockBehavior.Strict).Object, + new Mock(MockBehavior.Strict).Object, + tokens.Object, users.Object, + new Mock(MockBehavior.Strict).Object, + new Mock(MockBehavior.Strict).Object, + GetService(), Log); + + // Act + var exception = await Record.ExceptionAsync(() => service.SoftDeleteOrganizationAsync(organization, TestConstants.UserId)); + + // Assert + Assert.Same(deletionSaveFails ? persistenceFailure : cleanupFailure, exception); + Assert.Equal(!deletionSaveFails, persistedDeleted); + organizations.Verify(repository => repository.SaveAsync(organization, It.IsAny>()), Times.Once()); + users.Verify(repository => repository.GetByOrganizationIdAsync(organization.Id, It.IsAny>()), + deletionSaveFails ? Times.Never() : Times.Once()); + tokens.Verify(repository => repository.RemoveAllByOrganizationIdAsync(organization.Id), + deletionSaveFails ? Times.Never() : Times.Once()); + } + [Fact] public async Task CancelSubscriptionsAsync_WithoutStripeCustomer_DoesNotCallStripe() { From 23aac29038975f96b2d01c81be172989af634df1 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 09:35:07 -0500 Subject: [PATCH 03/15] Recheck current access before membership cleanup --- .../Api/Handlers/OrganizationHandler.cs | 6 ++++ .../Api/Endpoints/SavedViewEndpointTests.cs | 36 ++++++++++++++++--- .../Api/Handlers/OrganizationHandlerTests.cs | 2 ++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs index db618941c1..9a7d3dd1a7 100644 --- a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs @@ -739,6 +739,12 @@ public async Task Handle(RemoveOrganizationUser message) } } + var currentUser = await userRepository.GetByIdAsync(user.Id, options => options.Cache()); + bool hasOrganizationAccess = currentUser is not null + && (currentUser.OrganizationIds.Contains(organization.Id) || currentUser.Roles.Contains(AuthorizationRoles.GlobalAdmin)); + if (hasOrganizationAccess) + return Result.Success(); + await messagePublisher.PublishAsync(new UserMembershipChanged { ChangeType = ChangeType.Removed, diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index f7592929bd..271832937a 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -15,6 +15,7 @@ using Foundatio.Jobs; using Foundatio.Repositories; using Foundatio.Repositories.Exceptions; +using Foundatio.Repositories.Models; using Xunit; namespace Exceptionless.Tests.Api.Endpoints; @@ -2006,8 +2007,10 @@ await SendRequestAsync(r => r - [Fact] - public async Task RemoveUser_PreviouslyCommittedRemoval_CompletesNotificationAndPrivateViewCleanup() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RemoveUser_PreviouslyCommittedRemoval_CleansOnlyOrphanedResources(bool restoreMembership) { // Arrange var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); @@ -2024,6 +2027,26 @@ public async Task RemoveUser_PreviouslyCommittedRemoval_CompletesNotificationAnd }, options => options.ImmediateConsistency()); user.OrganizationIds.Remove(SampleDataService.TEST_ORG_ID); await _userRepository.SaveAsync(user); + var organization = await _organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID); + Assert.NotNull(organization); + if (restoreMembership) + { + organization.Invites.Add(new Invite + { + EmailAddress = user.EmailAddress, Token = Guid.NewGuid().ToString("N"), DateAdded = TimeProvider.GetUtcNow().UtcDateTime + }); + await _organizationRepository.SaveAsync(organization); + } + + bool restored = false; + using var subscription = _organizationRepository.DocumentsSaved.AddHandler(async (_, args) => + { + if (!restoreMembership || restored || !args.Documents.Any(document => document.Value.Id == organization.Id)) + return; + + restored = await _userRepository.PatchAsync(user.Id, + new ActionPatch(current => current.OrganizationIds.Add(organization.Id)), options => options.Cache()); + }); // Act await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() @@ -2031,10 +2054,15 @@ await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() var remainingView = await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false)); var updatedProject = await projects.GetByIdAsync(project.Id, options => options.Cache(false)); + var currentUser = await _userRepository.GetByIdAsync(user.Id, options => options.Cache()); + // Assert - Assert.Null(remainingView); + Assert.Equal(restoreMembership, restored); + Assert.Equal(restoreMembership, remainingView is not null); Assert.NotNull(updatedProject); - Assert.False(updatedProject.NotificationSettings.ContainsKey(user.Id)); + Assert.Equal(restoreMembership, updatedProject.NotificationSettings.ContainsKey(user.Id)); + Assert.NotNull(currentUser); + Assert.Equal(restoreMembership, currentUser.OrganizationIds.Contains(organization.Id)); } [Theory] diff --git a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs index 3f76bbabc6..e2edbca397 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs @@ -160,6 +160,8 @@ public async Task Handle_RemoveOrganizationUserFailure_PublishesRemovalOnlyAfter var publisher = new Mock(MockBehavior.Strict); if (!persistenceFails) { + users.Setup(repository => repository.GetByIdAsync(user.Id, It.IsAny>())) + .ReturnsAsync(new User { Id = user.Id, EmailAddress = user.EmailAddress }); projects.Setup(repository => repository.GetByOrganizationIdAsync(organization.Id, It.IsAny>())) .Callback(() => calls.Add("cleanup")) .ThrowsAsync(failure); From 3c0e469024729f74bf2d6790e03bc7ff3b676aff Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 09:46:40 -0500 Subject: [PATCH 04/15] Document organization deletion conflict responses --- .../Api/Endpoints/SavedViewEndpointTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index 271832937a..9f134f0eee 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -14,7 +14,6 @@ using FluentRest; using Foundatio.Jobs; using Foundatio.Repositories; -using Foundatio.Repositories.Exceptions; using Foundatio.Repositories.Models; using Xunit; @@ -2170,7 +2169,7 @@ await SendRequestAsync(r => r } [Fact] - public async Task SoftDeleteOrganization_UserVersionConflict_CleanupJobCompletesDeletionAndPreservesRequester() + public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndCleanupJobPreservesRequester() { // Arrange var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); @@ -2199,14 +2198,15 @@ public async Task SoftDeleteOrganization_UserVersionConflict_CleanupJobCompletes }); // Act - var exception = await Record.ExceptionAsync(() => _organizationService.SoftDeleteOrganizationAsync(organization, user.Id)); + using var response = await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() + .AppendPaths("organizations", organization.Id).ExpectedStatus(HttpStatusCode.Conflict)); var retained = await _savedViewRepository.GetByIdAsync(view.Id, options => options.Cache(false)); var storedOrganization = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false).IncludeSoftDeletes()); var storedUser = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); // Assert Assert.True(changed); - Assert.IsType(exception); + Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); Assert.NotNull(retained); Assert.NotNull(storedOrganization); Assert.True(storedOrganization.IsDeleted); From e953eb22c9d4a7a2f6bdb16c805905a922f9b94f Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 09:55:48 -0500 Subject: [PATCH 05/15] Keep unsaved user versions optional in the API contract --- tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs b/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs index 1cee7b91a4..a2b0642db0 100644 --- a/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs +++ b/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs @@ -172,7 +172,7 @@ public async Task SaveAsync_VersionChecks_RejectStaleOrMissingVersion(bool missi await _repository.AddAsync(user, options => options.ImmediateConsistency()); var stale = user with { FullName = "Stale profile" }; user.AuthenticationVersion = "rotated-generation"; - string originalVersion = user.Version; + string originalVersion = Assert.IsType(user.Version); var saved = await _repository.SaveAsync(user, options => options.ImmediateConsistency()); if (missingVersion) stale.Version = null!; From 3d3af3360034e29e642bd8453425c591a5774f7d Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 19:07:37 -0500 Subject: [PATCH 06/15] Use standard user versioning without persisted deletion requester state --- src/Exceptionless.Core/Jobs/CleanupDataJob.cs | 2 +- src/Exceptionless.Core/Models/Organization.cs | 6 ------ .../Services/OrganizationService.cs | 4 ++-- .../Api/Endpoints/OrganizationEndpointTests.cs | 1 + .../Api/Endpoints/SavedViewEndpointTests.cs | 8 +++++--- .../Repositories/UserRepositoryTests.cs | 2 +- .../Services/OrganizationServiceTests.cs | 14 +++++++++++--- 7 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs index cfc523028a..53de767113 100644 --- a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs +++ b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs @@ -405,7 +405,7 @@ private async Task RemoveOrganizationAsync(Organization organization, JobContext await _organizationService.RemoveWebHooksAsync(organization); await _organizationService.RemoveSavedViewsAsync(organization); await _organizationService.CancelSubscriptionsAsync(organization); - await _organizationService.RemoveUsersAsync(organization, organization.DeletedByUserId); + await _organizationService.RemoveUsersAsync(organization, null); await RenewLockAsync(context); long removedEvents = await _eventRepository.RemoveAllByOrganizationIdAsync(organization.Id); diff --git a/src/Exceptionless.Core/Models/Organization.cs b/src/Exceptionless.Core/Models/Organization.cs index 0bce9ebb2f..4cfa22dac8 100644 --- a/src/Exceptionless.Core/Models/Organization.cs +++ b/src/Exceptionless.Core/Models/Organization.cs @@ -201,12 +201,6 @@ public Organization() public DateTime UpdatedUtc { get; set; } public bool IsDeleted { get; set; } - /// - /// User whose account must be preserved when organization deletion resumes. - /// - [ObjectId] - public string? DeletedByUserId { get; set; } - string IOwnedByOrganization.OrganizationId { get { return Id; } set { Id = value; } } public IEnumerable Validate(ValidationContext validationContext) diff --git a/src/Exceptionless.Core/Services/OrganizationService.cs b/src/Exceptionless.Core/Services/OrganizationService.cs index df915bdd30..11ccccd71e 100644 --- a/src/Exceptionless.Core/Services/OrganizationService.cs +++ b/src/Exceptionless.Core/Services/OrganizationService.cs @@ -202,11 +202,11 @@ public async Task SoftDeleteOrganizationAsync(Organization organization, string if (organization.IsDeleted) return; + await RemoveUsersAsync(organization, currentUserId); + organization.IsDeleted = true; - organization.DeletedByUserId = currentUserId; await _organizationRepository.SaveAsync(organization, options => options.Cache()); - await RemoveUsersAsync(organization, currentUserId); await RemoveTokensAsync(organization); await RemoveWebHooksAsync(organization); await RemoveSavedViewsAsync(organization); diff --git a/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs index dbd4f791cb..86969be5c3 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/OrganizationEndpointTests.cs @@ -1052,6 +1052,7 @@ public async Task AddUserAsync_NewEmail_AddsInvite() // Assert Assert.NotNull(user); Assert.Equal(emailAddress, user.EmailAddress); + Assert.Equal(String.Empty, user.Version); var organization = await _organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID); Assert.NotNull(organization); diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index 9f134f0eee..29bdc364f8 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -2169,7 +2169,7 @@ await SendRequestAsync(r => r } [Fact] - public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndCleanupJobPreservesRequester() + public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndRetryPreservesRequester() { // Arrange var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); @@ -2209,11 +2209,13 @@ public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndClean Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); Assert.NotNull(retained); Assert.NotNull(storedOrganization); - Assert.True(storedOrganization.IsDeleted); + Assert.False(storedOrganization.IsDeleted); Assert.NotNull(storedUser); Assert.Equal("Concurrent profile update", storedUser.FullName); - // Act - the existing cleanup job resumes the persisted deletion. + // Act - retry completes membership removal before scheduling cleanup. + using var retryResponse = await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() + .AppendPaths("organizations", organization.Id).StatusCodeShouldBeOk()); await RefreshDataAsync(); var result = await GetService().RunAsync(TestCancellationToken); var requester = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); diff --git a/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs b/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs index a2b0642db0..1cee7b91a4 100644 --- a/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs +++ b/tests/Exceptionless.Tests/Repositories/UserRepositoryTests.cs @@ -172,7 +172,7 @@ public async Task SaveAsync_VersionChecks_RejectStaleOrMissingVersion(bool missi await _repository.AddAsync(user, options => options.ImmediateConsistency()); var stale = user with { FullName = "Stale profile" }; user.AuthenticationVersion = "rotated-generation"; - string originalVersion = Assert.IsType(user.Version); + string originalVersion = user.Version; var saved = await _repository.SaveAsync(user, options => options.ImmediateConsistency()); if (missingVersion) stale.Version = null!; diff --git a/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs b/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs index 4ca11982d3..6ed58f17e9 100644 --- a/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs +++ b/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs @@ -27,17 +27,20 @@ protected override void RegisterServices(IServiceCollection services, AppOptions [Theory] [InlineData(false)] [InlineData(true)] - public async Task SoftDeleteOrganization_Failure_PersistsDeletionBeforeChangingMembers(bool deletionSaveFails) + public async Task SoftDeleteOrganization_Failure_PersistsDeletionAfterMembersBeforeResourceCleanup(bool deletionSaveFails) { // Arrange var organization = new Organization { Id = TestConstants.OrganizationId, Name = "Deleting organization" }; var persistenceFailure = new IOException("Deletion marker failed"); var cleanupFailure = new IOException("Token cleanup failed"); bool persistedDeleted = false; + bool membersProcessed = false; + bool membersProcessedBeforeSave = false; var organizations = new Mock(MockBehavior.Strict); organizations.Setup(repository => repository.SaveAsync(organization, It.IsAny>())) .Returns(() => { + membersProcessedBeforeSave = membersProcessed; if (deletionSaveFails) return Task.FromException(persistenceFailure); @@ -46,7 +49,11 @@ public async Task SoftDeleteOrganization_Failure_PersistsDeletionBeforeChangingM }); var users = new Mock(MockBehavior.Strict); users.Setup(repository => repository.GetByOrganizationIdAsync(organization.Id, It.IsAny>())) - .ReturnsAsync(FindResults.Empty); + .Returns(() => + { + membersProcessed = true; + return Task.FromResult(FindResults.Empty); + }); var tokens = new Mock(MockBehavior.Strict); tokens.Setup(repository => repository.RemoveAllByOrganizationIdAsync(organization.Id)).ThrowsAsync(cleanupFailure); var service = new OrganizationService(organizations.Object, @@ -63,9 +70,10 @@ public async Task SoftDeleteOrganization_Failure_PersistsDeletionBeforeChangingM // Assert Assert.Same(deletionSaveFails ? persistenceFailure : cleanupFailure, exception); Assert.Equal(!deletionSaveFails, persistedDeleted); + Assert.True(membersProcessedBeforeSave); organizations.Verify(repository => repository.SaveAsync(organization, It.IsAny>()), Times.Once()); users.Verify(repository => repository.GetByOrganizationIdAsync(organization.Id, It.IsAny>()), - deletionSaveFails ? Times.Never() : Times.Once()); + Times.Once()); tokens.Verify(repository => repository.RemoveAllByOrganizationIdAsync(organization.Id), deletionSaveFails ? Times.Never() : Times.Once()); } From 2d825f7961294f958e7ea444b959eea082b1f276 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 19:08:23 -0500 Subject: [PATCH 07/15] Refresh persisted conflict state before deletion retry --- .../Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index 29bdc364f8..04275b93df 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -2214,6 +2214,7 @@ public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndRetry Assert.Equal("Concurrent profile update", storedUser.FullName); // Act - retry completes membership removal before scheduling cleanup. + await RefreshDataAsync(); using var retryResponse = await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() .AppendPaths("organizations", organization.Id).StatusCodeShouldBeOk()); await RefreshDataAsync(); From a975b92383f4ea01f8558cd75cb13973f938dca0 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 19:20:27 -0500 Subject: [PATCH 08/15] Expect the existing accepted response when retrying organization deletion --- .../Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index 04275b93df..cf11469d5c 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -2216,7 +2216,7 @@ public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndRetry // Act - retry completes membership removal before scheduling cleanup. await RefreshDataAsync(); using var retryResponse = await SendRequestAsync(request => request.Delete().AsGlobalAdminUser() - .AppendPaths("organizations", organization.Id).StatusCodeShouldBeOk()); + .AppendPaths("organizations", organization.Id).StatusCodeShouldBeAccepted()); await RefreshDataAsync(); var result = await GetService().RunAsync(TestCancellationToken); var requester = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); From dc470e1ea251a35bde815007339a7f0103253a39 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 19:28:36 -0500 Subject: [PATCH 09/15] Reproduce membership restoration during the final cleanup organization lookup --- .../Jobs/CleanupDataJobTests.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs index 95175732f6..1c174902e0 100644 --- a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs +++ b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs @@ -157,8 +157,10 @@ public async Task RunAsync_PrivateViewMadeSharedAfterScan_PreservesView() Assert.Null(retained.UserId); } - [Fact] - public async Task RunAsync_MembershipRestoredAfterOwnerRead_PreservesPrivateView() + [Theory] + [InlineData(1)] + [InlineData(2)] + public async Task RunAsync_MembershipRestoredDuringOrganizationLookup_PreservesPrivateView(int restoreOnOrganizationRead) { // Arrange var savedViews = GetService(); @@ -174,9 +176,14 @@ public async Task RunAsync_MembershipRestoredAfterOwnerRead_PreservesPrivateView Name = "Restored member", Slug = "restored-member", ViewType = "events" }, options => options.ImmediateConsistency()); bool restored = false; - using var subscription = organizations.BeforeGet.AddHandler(async (_, _) => + int organizationReads = 0; + using var subscription = organizations.BeforeGet.AddHandler(async (_, args) => { - if (restored) + if (!args.Ids.Any(id => id.Value == organization.Id)) + return; + + organizationReads++; + if (restored || organizationReads != restoreOnOrganizationRead) return; restored = true; From 2b701e97d00dec781c2c8decefed7db84ece38ec Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 19:37:05 -0500 Subject: [PATCH 10/15] Read current membership after the final organization lookup in cleanup --- src/Exceptionless.Core/Jobs/CleanupDataJob.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs index 53de767113..f2ca8100e4 100644 --- a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs +++ b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs @@ -154,8 +154,8 @@ private async Task CleanupOrphanedPrivateSavedViewsAsync(JobContext context) { context.CancellationToken.ThrowIfCancellationRequested(); - var currentUser = await _userRepository.GetByIdAsync(group.Key.UserId!, options => options.Cache(false)); var currentOrganization = await _organizationRepository.GetByIdAsync(group.Key.OrganizationId, options => options.Cache(false)); + var currentUser = await _userRepository.GetByIdAsync(group.Key.UserId!, options => options.Cache(false)); if (currentUser is not null && currentOrganization is not null && (currentUser.OrganizationIds.Contains(group.Key.OrganizationId) || currentUser.Roles.Contains(AuthorizationRoles.GlobalAdmin))) continue; From 42905e4b8e01b8458457deeea1bdb55d29c0434a Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 19:52:19 -0500 Subject: [PATCH 11/15] Reproduce invitations lost during predefined saved-view initialization --- .../Api/Endpoints/SavedViewEndpointTests.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs index cf11469d5c..3be703e484 100644 --- a/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs +++ b/tests/Exceptionless.Tests/Api/Endpoints/SavedViewEndpointTests.cs @@ -2168,6 +2168,53 @@ await SendRequestAsync(r => r Assert.Contains(testUser.SavedViewOrders, preference => preference.OrganizationId == SampleDataService.FREE_ORG_ID); } + [Fact] + public async Task GetByOrganization_PredefinedInitializationOverlapsInvite_PreservesInvitationAndOrganizationData() + { + // Arrange + var organization = await _organizationRepository.GetByIdAsync(SampleDataService.TEST_ORG_ID); + Assert.NotNull(organization); + organization.Data = new DataDictionary { ["existing_setting"] = "existing" }; + await _organizationRepository.SaveAsync(organization, options => options.Cache().ImmediateConsistency()); + await _savedViewRepository.RemoveAllAsync(query => query.Organization(organization.Id), options => options.ImmediateConsistency()); + var invite = new Invite + { + Token = Exceptionless.Core.Extensions.StringExtensions.GetNewToken(), + EmailAddress = "concurrent-invite@localhost.test", + DateAdded = TimeProvider.GetUtcNow().UtcDateTime + }; + bool invited = false; + using var subscription = _savedViewRepository.DocumentsAdding.AddHandler(async (_, args) => + { + if (invited || !args.Documents.Any(view => view.OrganizationId == organization.Id)) + return; + + invited = true; + var current = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false)); + Assert.NotNull(current); + current.Invites.Add(invite); + current.Data!["concurrent_setting"] = "concurrent"; + await _organizationRepository.SaveAsync(current, options => options.Cache().ImmediateConsistency()); + }); + + // Act + using var response = await SendRequestAsync(request => request.AsGlobalAdminUser() + .AppendPaths("organizations", organization.Id, "saved-views").StatusCodeShouldBeOk()); + var stored = await _organizationRepository.GetByIdAsync(organization.Id, options => options.Cache(false)); + var invitedOrganization = await _organizationRepository.GetByInviteTokenAsync(invite.Token); + + // Assert + Assert.True(invited); + Assert.NotNull(stored); + Assert.Contains(stored.Invites, candidate => candidate.Token == invite.Token); + Assert.NotNull(stored.Data); + Assert.Equal("existing", stored.Data["existing_setting"]?.ToString()); + Assert.Equal("concurrent", stored.Data["concurrent_setting"]?.ToString()); + Assert.True(stored.Data.ContainsKey(PredefinedSavedViewsDataSeed.PredefinedSavedViewsContentHashDataKey)); + Assert.NotNull(invitedOrganization); + Assert.Equal(organization.Id, invitedOrganization.Id); + } + [Fact] public async Task DeleteOrganization_UserVersionConflict_ReturnsConflictAndRetryPreservesRequester() { From a041485dfe9081f10744a3c886354da27252f54e Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 20:00:40 -0500 Subject: [PATCH 12/15] Patch predefined-view metadata without overwriting organization invitations --- .../Api/Handlers/SavedViewHandler.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Exceptionless.Web/Api/Handlers/SavedViewHandler.cs b/src/Exceptionless.Web/Api/Handlers/SavedViewHandler.cs index 45baae424c..7d37568409 100644 --- a/src/Exceptionless.Web/Api/Handlers/SavedViewHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/SavedViewHandler.cs @@ -19,6 +19,7 @@ using Foundatio.Mediator; using Foundatio.Queues; using Foundatio.Repositories; +using Foundatio.Repositories.Models; using DataDictionary = Exceptionless.Core.Models.DataDictionary; namespace Exceptionless.Web.Api.Handlers; @@ -702,9 +703,13 @@ private async Task> UpsertPredefinedSavedViewsAsy if (!upsertResult.HasSkippedCustomizations) { - organization.Data ??= new DataDictionary(); - organization.Data[PredefinedSavedViewsDataSeed.PredefinedSavedViewsContentHashDataKey] = definitionsContentHash; - await organizationRepository.SaveAsync(organization, o => o.Cache().ImmediateConsistency()); + await organizationRepository.PatchAsync(organizationId, new PartialPatch(new + { + data = new DataDictionary + { + [PredefinedSavedViewsDataSeed.PredefinedSavedViewsContentHashDataKey] = definitionsContentHash + } + }), o => o.Cache().ImmediateConsistency()); } }, TimeSpan.FromSeconds(10), TimeSpan.FromSeconds(15)); From 4261fa06b34b695115be243ed5c699dc7d4ce93a Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 20:15:37 -0500 Subject: [PATCH 13/15] test: reproduce private view loss during invitation acceptance --- .../Jobs/CleanupDataJobTests.cs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs index 1c174902e0..2eb9c83ce2 100644 --- a/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs +++ b/tests/Exceptionless.Tests/Jobs/CleanupDataJobTests.cs @@ -9,6 +9,7 @@ using Exceptionless.Core.Utility; using Exceptionless.DateTimeExtensions; using Exceptionless.Tests.Utility; +using Exceptionless.Web.Models; using Foundatio.Repositories; using Foundatio.Repositories.Models; using Foundatio.Repositories.Utility; @@ -157,6 +158,63 @@ public async Task RunAsync_PrivateViewMadeSharedAfterScan_PreservesView() Assert.Null(retained.UserId); } + [Fact] + public async Task RunAsync_InviteAcceptanceInProgress_PreservesPrivateView() + { + // Arrange + await GetService().CreateDataAsync(); + var user = await _userRepository.GetByEmailAddressAsync(SampleDataService.TEST_ORG_USER_EMAIL); + Assert.NotNull(user); + var organization = _organizationData.GenerateOrganization(_billingManager, _plans, generateId: true); + var invite = new Invite + { + EmailAddress = user.EmailAddress, Token = StringExtensions.GetNewToken(), + DateAdded = TimeProvider.GetUtcNow().UtcDateTime + }; + organization.Invites.Add(invite); + await _organizationRepository.AddAsync(organization, options => options.ImmediateConsistency()); + var savedViews = GetService(); + var view = await savedViews.AddAsync(new SavedView + { + OrganizationId = organization.Id, UserId = user.Id, CreatedByUserId = user.Id, + Name = "Returning member", Slug = "returning-member", ViewType = "events" + }, options => options.ImmediateConsistency()); + var grantStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var finishGrant = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var subscription = _userRepository.DocumentsSaving.AddHandler(async (_, args) => + { + if (!args.Documents.Any(document => document.Value.Id == user.Id && document.Value.OrganizationIds.Contains(organization.Id))) + return; + + grantStarted.TrySetResult(); + await finishGrant.Task.WaitAsync(TestCancellationToken); + }); + + // Act + var grant = SendRequestAsync(request => request.Post().AppendPath("auth/login") + .Content(new Login { Email = user.EmailAddress, Password = SampleDataService.TEST_ORG_USER_PASSWORD, InviteToken = invite.Token }) + .ExpectedStatus(System.Net.HttpStatusCode.OK)); + Foundatio.Jobs.JobResult result; + try + { + await grantStarted.Task.WaitAsync(TimeSpan.FromSeconds(30), TestCancellationToken); + result = await _job.RunAsync(TestCancellationToken); + } + finally + { + finishGrant.TrySetResult(); + } + using var response = await grant; + var member = await _userRepository.GetByIdAsync(user.Id, options => options.Cache(false)); + var retained = await savedViews.GetByIdAsync(view.Id, options => options.Cache(false)); + + // Assert + Assert.True(result.IsSuccess); + Assert.NotNull(member); + Assert.Contains(organization.Id, member.OrganizationIds); + Assert.NotNull(retained); + } + [Theory] [InlineData(1)] [InlineData(2)] From 234e538ce42e1b8b6a756aab5930d21317567dc0 Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Mon, 19 Oct 2026 20:22:39 -0500 Subject: [PATCH 14/15] fix: coordinate membership grants with private view cleanup --- src/Exceptionless.Core/Jobs/CleanupDataJob.cs | 4 ++++ .../Services/OrganizationService.cs | 13 ++++++++++++- src/Exceptionless.Web/Api/Handlers/AuthHandler.cs | 2 ++ .../Api/Handlers/OrganizationHandler.cs | 3 +++ src/Exceptionless.Web/Api/Handlers/UserHandler.cs | 3 +++ .../Api/Handlers/AuthHandlerTests.cs | 9 +++++++++ .../Api/Handlers/OrganizationHandlerTests.cs | 2 +- .../Services/OrganizationServiceTests.cs | 2 +- 8 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs index f2ca8100e4..98b2700dbb 100644 --- a/src/Exceptionless.Core/Jobs/CleanupDataJob.cs +++ b/src/Exceptionless.Core/Jobs/CleanupDataJob.cs @@ -154,6 +154,10 @@ private async Task CleanupOrphanedPrivateSavedViewsAsync(JobContext context) { context.CancellationToken.ThrowIfCancellationRequested(); + await using var membershipLock = await _organizationService.TryAcquireUserMembershipLockAsync(group.Key.UserId!); + if (membershipLock is null) + continue; + var currentOrganization = await _organizationRepository.GetByIdAsync(group.Key.OrganizationId, options => options.Cache(false)); var currentUser = await _userRepository.GetByIdAsync(group.Key.UserId!, options => options.Cache(false)); if (currentUser is not null && currentOrganization is not null diff --git a/src/Exceptionless.Core/Services/OrganizationService.cs b/src/Exceptionless.Core/Services/OrganizationService.cs index 11ccccd71e..8fd2f3d3e8 100644 --- a/src/Exceptionless.Core/Services/OrganizationService.cs +++ b/src/Exceptionless.Core/Services/OrganizationService.cs @@ -2,6 +2,7 @@ using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; using Foundatio.Extensions.Hosting.Startup; +using Foundatio.Lock; using Foundatio.Repositories; using Foundatio.Repositories.Models; using Microsoft.Extensions.Logging; @@ -21,8 +22,9 @@ public class OrganizationService : IStartupAction private readonly IStripeBillingClient _stripeBillingClient; private readonly UsageService _usageService; private readonly ILogger _logger; + private readonly ILockProvider _lockProvider; - public OrganizationService(IOrganizationRepository organizationRepository, IProjectRepository projectRepository, ISavedViewRepository savedViewRepository, ITokenRepository tokenRepository, IUserRepository userRepository, IWebHookRepository webHookRepository, IStripeBillingClient stripeBillingClient, UsageService usageService, ILoggerFactory loggerFactory) + public OrganizationService(IOrganizationRepository organizationRepository, IProjectRepository projectRepository, ISavedViewRepository savedViewRepository, ITokenRepository tokenRepository, IUserRepository userRepository, IWebHookRepository webHookRepository, IStripeBillingClient stripeBillingClient, UsageService usageService, ILockProvider lockProvider, ILoggerFactory loggerFactory) { _organizationRepository = organizationRepository; _projectRepository = projectRepository; @@ -32,9 +34,18 @@ public OrganizationService(IOrganizationRepository organizationRepository, IProj _webHookRepository = webHookRepository; _stripeBillingClient = stripeBillingClient; _usageService = usageService; + _lockProvider = lockProvider; _logger = loggerFactory.CreateLogger(); } + public Task AcquireUserMembershipLockAsync(string userId, CancellationToken cancellationToken = default) + => _lockProvider.AcquireAsync(GetUserMembershipLockKey(userId), cancellationToken: cancellationToken); + + public Task TryAcquireUserMembershipLockAsync(string userId) + => _lockProvider.TryAcquireAsync(GetUserMembershipLockKey(userId), timeUntilExpires: null, acquireTimeout: TimeSpan.Zero); + + private static string GetUserMembershipLockKey(string userId) => $"user-membership:{userId}"; + public Task RunAsync(CancellationToken shutdownToken = default) { _organizationRepository.DocumentsSaved.AddHandler(OnOrganizationSavedAsync); diff --git a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs index f11620e263..b3e3f3147e 100644 --- a/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/AuthHandler.cs @@ -35,6 +35,7 @@ public class AuthHandler( ICacheClient cacheClient, ILockProvider lockProvider, AuthService authService, + OrganizationService organizationService, IMailer mailer, IDomainLoginProvider domainLoginProvider, TimeProvider timeProvider, @@ -721,6 +722,7 @@ private async Task AddInvitedUserToOrganizationAsync(string? token, User user, H using var _ = logger.BeginScope(new ExceptionlessState().Tag("Invite").Identity(user.EmailAddress).SetHttpContext(httpContext)); + await using var membershipLock = await organizationService.AcquireUserMembershipLockAsync(user.Id, httpContext.RequestAborted); var organization = await organizationRepository.GetByInviteTokenAsync(token); var invite = organization?.GetInvite(token); if (organization is null || invite is null) diff --git a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs index 9a7d3dd1a7..d1ce6a42ad 100644 --- a/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/OrganizationHandler.cs @@ -642,6 +642,7 @@ public async Task> Handle(AddOrganizationUser message) { if (!user.OrganizationIds.Contains(organization.Id)) { + await using var membershipLock = await organizationService.AcquireUserMembershipLockAsync(user.Id, message.Context.RequestAborted); try { bool patched = await userRepository.PatchAsync(user.Id, new ActionPatch(current => current.OrganizationIds.Add(organization.Id)), o => o.Cache()); @@ -739,6 +740,7 @@ public async Task Handle(RemoveOrganizationUser message) } } + await using var membershipLock = await organizationService.AcquireUserMembershipLockAsync(user.Id, message.Context.RequestAborted); var currentUser = await userRepository.GetByIdAsync(user.Id, options => options.Cache()); bool hasOrganizationAccess = currentUser is not null && (currentUser.OrganizationIds.Contains(organization.Id) || currentUser.Roles.Contains(AuthorizationRoles.GlobalAdmin)); @@ -875,6 +877,7 @@ private async Task AddModelAsync(Organization value, HttpContext h : plans.FreePlan; billingManager.ApplyBillingPlan(value, plan, user); + await using var membershipLock = await organizationService.AcquireUserMembershipLockAsync(user.Id, httpContext.RequestAborted); var organization = await repository.AddAsync(value, o => o.Cache()); try diff --git a/src/Exceptionless.Web/Api/Handlers/UserHandler.cs b/src/Exceptionless.Web/Api/Handlers/UserHandler.cs index 9a8a6e9787..e979469143 100644 --- a/src/Exceptionless.Web/Api/Handlers/UserHandler.cs +++ b/src/Exceptionless.Web/Api/Handlers/UserHandler.cs @@ -5,6 +5,7 @@ using Exceptionless.Core.Mail; using Exceptionless.Core.Models; using Exceptionless.Core.Repositories; +using Exceptionless.Core.Services; using Exceptionless.Core.Validation; using Exceptionless.DateTimeExtensions; using Exceptionless.Web.Api.Infrastructure; @@ -25,6 +26,7 @@ namespace Exceptionless.Web.Api.Handlers; public class UserHandler( IUserRepository repository, + OrganizationService organizationService, IOrganizationRepository organizationRepository, ITokenRepository tokenRepository, IOAuthTokenRepository oauthTokenRepository, @@ -365,6 +367,7 @@ public async Task Handle(UnverifyEmailAddresses message) public async Task Handle(AddAdminRole message) { + await using var membershipLock = await organizationService.AcquireUserMembershipLockAsync(message.Id, HttpContext.RequestAborted); var user = await GetModelAsync(message.Id, false); if (user is null) return Result.NotFound("User not found."); diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index 531404f0d3..d30d6f95f7 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -3,6 +3,7 @@ using System.Security.Claims; using Exceptionless.Core; using Exceptionless.Core.Authentication; +using Exceptionless.Core.Billing; using Exceptionless.Core.Configuration; using Exceptionless.Core.Extensions; using Exceptionless.Core.Mail; @@ -22,6 +23,7 @@ using Foundatio.Repositories; using Foundatio.Repositories.Models; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Moq; using Xunit; @@ -31,6 +33,12 @@ public sealed class AuthHandlerTests : TestWithServices { public AuthHandlerTests(ITestOutputHelper output) : base(output) { } + protected override void RegisterServices(IServiceCollection services, AppOptions options) + { + base.RegisterServices(services, options); + services.ReplaceSingleton(); + } + [Theory] [InlineData(false)] [InlineData(true)] @@ -325,6 +333,7 @@ private AuthHandler CreateHandler(IUserRepository userRepository, ITokenReposito GetService(), lockProvider ?? GetService(), GetService(), + GetService(), GetService(), GetService(), TimeProvider, diff --git a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs index e2edbca397..e8e47f41e5 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/OrganizationHandlerTests.cs @@ -253,7 +253,7 @@ private OrganizationHandler CreateHandler(IOrganizationRepository organizations, var usage = GetService(); var stripe = new FakeStripeBillingClient(); var service = new OrganizationService(organizations, projects, savedViews, GetService(), users, - GetService(), stripe, usage, Log); + GetService(), stripe, usage, GetService(), Log); var billing = new BillingManager(GetService(), organizations, projects, users, GetService(), TimeProvider); return new OrganizationHandler(service, organizations, GetService(), GetService(), users, projects, diff --git a/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs b/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs index 6ed58f17e9..527f85722f 100644 --- a/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs +++ b/tests/Exceptionless.Tests/Services/OrganizationServiceTests.cs @@ -62,7 +62,7 @@ public async Task SoftDeleteOrganization_Failure_PersistsDeletionAfterMembersBef tokens.Object, users.Object, new Mock(MockBehavior.Strict).Object, new Mock(MockBehavior.Strict).Object, - GetService(), Log); + GetService(), GetService(), Log); // Act var exception = await Record.ExceptionAsync(() => service.SoftDeleteOrganizationAsync(organization, TestConstants.UserId)); From 4e9c9d5c19c4a262600227d645bb975c829610fd Mon Sep 17 00:00:00 2001 From: Blake Niemyjski Date: Wed, 21 Oct 2026 22:09:04 -0500 Subject: [PATCH 15/15] Adapt stacked transfer regressions to the data-only identity lookup --- tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs index d30d6f95f7..97380d10f8 100644 --- a/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs +++ b/tests/Exceptionless.Tests/Api/Handlers/AuthHandlerTests.cs @@ -65,7 +65,7 @@ public async Task Handle_ExternalAccountDestinationSaveFails_DoesNotRestoreOwner string? rotatedGeneration = null; User? currentOwner = null; var users = new Mock(MockBehavior.Strict); - users.Setup(repository => repository.GetUserByOAuthProviderAsync(provider, providerId, It.IsAny())).ReturnsAsync(displaced); + users.Setup(repository => repository.GetUsersByOAuthProviderAsync(provider, providerId, It.IsAny())).ReturnsAsync(new[] { displaced }); users.Setup(repository => repository.GetByIdAsync(displaced.Id, It.IsAny>())).ReturnsAsync(displaced); users.Setup(repository => repository.GetByIdAsync(destination.Id, It.IsAny>())).ReturnsAsync(destination); users.Setup(repository => repository.SaveAsync(It.Is(user => ReferenceEquals(user, displaced)), It.IsAny>()))