Protect user updates and clean up orphaned private views

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<JobResult> RunInternalAsync(JobContext context)
         await CleanupSyntheticOrganizationsAsync(context);
         await CleanupSyntheticUsersAsync(context);
         await CleanupSoftDeletedOrganizationsAsync(context);
+        await CleanupOrphanedPrivateSavedViewsAsync(context);
         await CleanupSoftDeletedProjectsAsync(context);
         await CleanupSoftDeletedStacksAsync(context);
 
@@ -122,6 +127,59 @@ protected override async Task<JobResult> 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();
+
+                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
+                    && (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));

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<OrganizationService>();
     }
 
+    public Task<ILock> AcquireUserMembershipLockAsync(string userId, CancellationToken cancellationToken = default)
+        => _lockProvider.AcquireAsync(GetUserMembershipLockKey(userId), cancellationToken: cancellationToken);
+
+    public Task<ILock?> 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);
@@ -202,15 +213,16 @@ public async Task SoftDeleteOrganizationAsync(Organization organization, string
         if (organization.IsDeleted)
             return;
 
+        await RemoveUsersAsync(organization, currentUserId);
+
+        organization.IsDeleted = true;
+        await _organizationRepository.SaveAsync(organization, options => options.Cache());
+
         await RemoveTokensAsync(organization);
         await RemoveWebHooksAsync(organization);
         await RemoveSavedViewsAsync(organization);
         await CancelSubscriptionsAsync(organization);
-        await RemoveUsersAsync(organization, currentUserId);
         await CleanupProjectNotificationSettingsAsync(organization, []);
-
-        organization.IsDeleted = true;
-        await _organizationRepository.SaveAsync(organization);
     }
 
     private async Task<HashSet<string>> GetValidNotificationUserIdsAsync(string organizationId, IReadOnlyCollection<string> userIds, CancellationToken cancellationToken)

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)

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,23 @@ public async Task<Result<User>> Handle(AddOrganizationUser message)
         {
             if (!user.OrganizationIds.Contains(organization.Id))
             {
-                user.OrganizationIds.Add(organization.Id);
-                await userRepository.SaveAsync(user, o => o.Cache());
+                await using var membershipLock = await organizationService.AcquireUserMembershipLockAsync(user.Id, message.Context.RequestAborted);
+                try
+                {
+                    bool patched = await userRepository.PatchAsync(user.Id, new ActionPatch<User>(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 +700,14 @@ public async Task<Result> 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 +718,45 @@ public async Task<Result> 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<User>(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 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));
+        if (hasOrganizationAccess)
+            return Result.Success();
+
+        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();
     }
 
@@ -836,10 +877,35 @@ private async Task<Organization> 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());
 
-        user.OrganizationIds.Add(organization.Id);
-        await userRepository.SaveAsync(user, o => o.Cache());
+        try
+        {
+            bool patched = await userRepository.PatchAsync(user.Id, new ActionPatch<User>(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,

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<IReadOnlyCollection<SavedView>> 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));
 

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<Result> Handle(UnverifyEmailAddresses message)
 
     public async Task<Result> 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.");

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);