[PM-21752] Add granular events for collection management settings (#6269)

* Add new event types for collection management settings in EventType enum * Refactor collection management settings update process in OrganizationsController and IOrganizationService. Introduced UpdateCollectionManagementSettingsAsync method to streamline updates and logging for collection management settings. * Add unit tests for collection management settings updates in OrganizationsController and OrganizationService. Implemented tests to verify the successful update of collection management settings and the logging of specific events when settings are changed. Added error handling for cases where the organization is not found. * Refactor collection management settings handling in OrganizationsController and IOrganizationService. Updated the UpdateCollectionManagementSettingsAsync method to accept a single settings object, simplifying the parameter list and improving code readability. Introduced a new OrganizationCollectionManagementSettings model to encapsulate collection management settings. Adjusted related tests to reflect these changes. * Add Obsolete attribute to Organization_CollectionManagement_Updated event in EventType enum
2025-12-06 00:03:34 +00:00 · 2025-09-05 12:01:14 +01:00
parent 1b0be3e87f
commit 2c0c113420
8 changed files with 242 additions and 26 deletions
--- a/src/Api/AdminConsole/Controllers/OrganizationsController.cs
+++ b/src/Api/AdminConsole/Controllers/OrganizationsController.cs
@@ -554,18 +554,12 @@ public class OrganizationsController : Controller
    [HttpPut("{id}/collection-management")]
    public async Task<OrganizationResponseModel> PutCollectionManagement(Guid id, [FromBody] OrganizationCollectionManagementUpdateRequestModel model)
    {
-        var organization = await _organizationRepository.GetByIdAsync(id);
-        if (organization == null)
-        {
-            throw new NotFoundException();
-        }
-
        if (!await _currentContext.OrganizationOwner(id))
        {
            throw new NotFoundException();
        }

-        await _organizationService.UpdateAsync(model.ToOrganization(organization, _featureService), eventType: EventType.Organization_CollectionManagement_Updated);
+        var organization = await _organizationService.UpdateCollectionManagementSettingsAsync(id, model.ToSettings());
        var plan = await _pricingClient.GetPlan(organization.PlanType);
        return new OrganizationResponseModel(organization, plan);
    }
--- a/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs
+++ b/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs
@@ -1,5 +1,4 @@
-using Bit.Core.AdminConsole.Entities;
-using Bit.Core.Services;
+using Bit.Core.AdminConsole.Models.Business;

 namespace Bit.Api.Models.Request.Organizations;

@@ -10,12 +9,11 @@ public class OrganizationCollectionManagementUpdateRequestModel
    public bool LimitItemDeletion { get; set; }
    public bool AllowAdminAccessToAllCollectionItems { get; set; }

-    public virtual Organization ToOrganization(Organization existingOrganization, IFeatureService featureService)
+    public OrganizationCollectionManagementSettings ToSettings() => new()
    {
-        existingOrganization.LimitCollectionCreation = LimitCollectionCreation;
-        existingOrganization.LimitCollectionDeletion = LimitCollectionDeletion;
-        existingOrganization.LimitItemDeletion = LimitItemDeletion;
-        existingOrganization.AllowAdminAccessToAllCollectionItems = AllowAdminAccessToAllCollectionItems;
-        return existingOrganization;
-    }
+        LimitCollectionCreation = LimitCollectionCreation,
+        LimitCollectionDeletion = LimitCollectionDeletion,
+        LimitItemDeletion = LimitItemDeletion,
+        AllowAdminAccessToAllCollectionItems = AllowAdminAccessToAllCollectionItems
+    };
 }
--- a/src/Core/AdminConsole/Enums/EventType.cs
+++ b/src/Core/AdminConsole/Enums/EventType.cs
@@ -70,7 +70,16 @@ public enum EventType : int
    Organization_EnabledKeyConnector = 1606,
    Organization_DisabledKeyConnector = 1607,
    Organization_SponsorshipsSynced = 1608,
-    Organization_CollectionManagement_Updated = 1609,
+    [Obsolete("Use other specific Organization_CollectionManagement events instead")]
+    Organization_CollectionManagement_Updated = 1609, // TODO: Will be removed in PM-25315
+    Organization_CollectionManagement_LimitCollectionCreationEnabled = 1610,
+    Organization_CollectionManagement_LimitCollectionCreationDisabled = 1611,
+    Organization_CollectionManagement_LimitCollectionDeletionEnabled = 1612,
+    Organization_CollectionManagement_LimitCollectionDeletionDisabled = 1613,
+    Organization_CollectionManagement_LimitItemDeletionEnabled = 1614,
+    Organization_CollectionManagement_LimitItemDeletionDisabled = 1615,
+    Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsEnabled = 1616,
+    Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsDisabled = 1617,

    Policy_Updated = 1700,

--- a/src/Core/AdminConsole/Models/Business/OrganizationCollectionManagementSettings.cs
+++ b/src/Core/AdminConsole/Models/Business/OrganizationCollectionManagementSettings.cs
@@ -0,0 +1,9 @@
+namespace Bit.Core.AdminConsole.Models.Business;
+
+public record OrganizationCollectionManagementSettings
+{
+    public bool LimitCollectionCreation { get; set; }
+    public bool LimitCollectionDeletion { get; set; }
+    public bool LimitItemDeletion { get; set; }
+    public bool AllowAdminAccessToAllCollectionItems { get; set; }
+}
--- a/src/Core/AdminConsole/Services/IOrganizationService.cs
+++ b/src/Core/AdminConsole/Services/IOrganizationService.cs
@@ -2,6 +2,7 @@
 #nullable disable

 using Bit.Core.AdminConsole.Entities;
+using Bit.Core.AdminConsole.Models.Business;
 using Bit.Core.Auth.Enums;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
@@ -19,7 +20,8 @@ public interface IOrganizationService
    Task<string> AdjustSeatsAsync(Guid organizationId, int seatAdjustment);
    Task VerifyBankAsync(Guid organizationId, int amount1, int amount2);
    Task UpdateExpirationDateAsync(Guid organizationId, DateTime? expirationDate);
-    Task UpdateAsync(Organization organization, bool updateBilling = false, EventType eventType = EventType.Organization_Updated);
+    Task UpdateAsync(Organization organization, bool updateBilling = false);
+    Task<Organization> UpdateCollectionManagementSettingsAsync(Guid organizationId, OrganizationCollectionManagementSettings settings);
    Task UpdateTwoFactorProviderAsync(Organization organization, TwoFactorProviderType type);
    Task DisableTwoFactorProviderAsync(Organization organization, TwoFactorProviderType type);
    Task<OrganizationUser> InviteUserAsync(Guid organizationId, Guid? invitingUserId, EventSystemUser? systemUser,
--- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs
+++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs
@@ -5,6 +5,7 @@ using System.Text.Json;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Enums.Provider;
+using Bit.Core.AdminConsole.Models.Business;
 using Bit.Core.AdminConsole.Models.Data.Organizations.Policies;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers;
@@ -378,8 +379,7 @@ public class OrganizationService : IOrganizationService
        }
    }

-    public async Task UpdateAsync(Organization organization, bool updateBilling = false,
-        EventType eventType = EventType.Organization_Updated)
+    public async Task UpdateAsync(Organization organization, bool updateBilling = false)
    {
        if (organization.Id == default(Guid))
        {
@@ -395,7 +395,7 @@ public class OrganizationService : IOrganizationService
            }
        }

-        await ReplaceAndUpdateCacheAsync(organization, eventType);
+        await ReplaceAndUpdateCacheAsync(organization, EventType.Organization_Updated);

        if (updateBilling && !string.IsNullOrWhiteSpace(organization.GatewayCustomerId))
        {
@@ -420,11 +420,35 @@ public class OrganizationService : IOrganizationService
                    },
                });
        }
+    }

-        if (eventType == EventType.Organization_CollectionManagement_Updated)
+    public async Task<Organization> UpdateCollectionManagementSettingsAsync(Guid organizationId, OrganizationCollectionManagementSettings settings)
+    {
+        var existingOrganization = await _organizationRepository.GetByIdAsync(organizationId);
+        if (existingOrganization == null)
        {
-            await _pushNotificationService.PushSyncOrganizationCollectionManagementSettingsAsync(organization);
+            throw new NotFoundException();
        }
+
+        // Create logging actions based on what will change
+        var loggingActions = CreateCollectionManagementLoggingActions(existingOrganization, settings);
+
+        existingOrganization.LimitCollectionCreation = settings.LimitCollectionCreation;
+        existingOrganization.LimitCollectionDeletion = settings.LimitCollectionDeletion;
+        existingOrganization.LimitItemDeletion = settings.LimitItemDeletion;
+        existingOrganization.AllowAdminAccessToAllCollectionItems = settings.AllowAdminAccessToAllCollectionItems;
+        existingOrganization.RevisionDate = DateTime.UtcNow;
+
+        await ReplaceAndUpdateCacheAsync(existingOrganization);
+
+        if (loggingActions.Any())
+        {
+            await Task.WhenAll(loggingActions.Select(action => action()));
+        }
+
+        await _pushNotificationService.PushSyncOrganizationCollectionManagementSettingsAsync(existingOrganization);
+
+        return existingOrganization;
    }

    public async Task UpdateTwoFactorProviderAsync(Organization organization, TwoFactorProviderType type)
@@ -1214,4 +1238,44 @@ public class OrganizationService : IOrganizationService

        return status;
    }
+
+    private List<Func<Task>> CreateCollectionManagementLoggingActions(
+        Organization existingOrganization, OrganizationCollectionManagementSettings settings)
+    {
+        var loggingActions = new List<Func<Task>>();
+
+        if (existingOrganization.LimitCollectionCreation != settings.LimitCollectionCreation)
+        {
+            var eventType = settings.LimitCollectionCreation
+                ? EventType.Organization_CollectionManagement_LimitCollectionCreationEnabled
+                : EventType.Organization_CollectionManagement_LimitCollectionCreationDisabled;
+            loggingActions.Add(() => _eventService.LogOrganizationEventAsync(existingOrganization, eventType));
+        }
+
+        if (existingOrganization.LimitCollectionDeletion != settings.LimitCollectionDeletion)
+        {
+            var eventType = settings.LimitCollectionDeletion
+                ? EventType.Organization_CollectionManagement_LimitCollectionDeletionEnabled
+                : EventType.Organization_CollectionManagement_LimitCollectionDeletionDisabled;
+            loggingActions.Add(() => _eventService.LogOrganizationEventAsync(existingOrganization, eventType));
+        }
+
+        if (existingOrganization.LimitItemDeletion != settings.LimitItemDeletion)
+        {
+            var eventType = settings.LimitItemDeletion
+                ? EventType.Organization_CollectionManagement_LimitItemDeletionEnabled
+                : EventType.Organization_CollectionManagement_LimitItemDeletionDisabled;
+            loggingActions.Add(() => _eventService.LogOrganizationEventAsync(existingOrganization, eventType));
+        }
+
+        if (existingOrganization.AllowAdminAccessToAllCollectionItems != settings.AllowAdminAccessToAllCollectionItems)
+        {
+            var eventType = settings.AllowAdminAccessToAllCollectionItems
+                ? EventType.Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsEnabled
+                : EventType.Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsDisabled;
+            loggingActions.Add(() => _eventService.LogOrganizationEventAsync(existingOrganization, eventType));
+        }
+
+        return loggingActions;
+    }
 }
--- a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs
+++ b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs
@@ -2,10 +2,12 @@
 using AutoFixture.Xunit2;
 using Bit.Api.AdminConsole.Controllers;
 using Bit.Api.Auth.Models.Request.Accounts;
+using Bit.Api.Models.Request.Organizations;
 using Bit.Core;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.Enums;
 using Bit.Core.AdminConsole.Enums.Provider;
+using Bit.Core.AdminConsole.Models.Business;
 using Bit.Core.AdminConsole.Models.Business.Tokenables;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationApiKeys.Interfaces;
 using Bit.Core.AdminConsole.OrganizationFeatures.Organizations;
@@ -29,6 +31,7 @@ using Bit.Core.Exceptions;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
 using Bit.Core.Tokens;
+using Bit.Core.Utilities;
 using Bit.Infrastructure.EntityFramework.AdminConsole.Models.Provider;
 using NSubstitute;
 using Xunit;
@@ -293,4 +296,40 @@ public class OrganizationsControllerTests : IDisposable

        Assert.True(result.ResetPasswordEnabled);
    }
+
+    [Theory, AutoData]
+    public async Task PutCollectionManagement_ValidRequest_Success(
+        Organization organization,
+        OrganizationCollectionManagementUpdateRequestModel model)
+    {
+        // Arrange
+        _currentContext.OrganizationOwner(organization.Id).Returns(true);
+
+        var plan = StaticStore.GetPlan(PlanType.EnterpriseAnnually);
+        _pricingClient.GetPlan(Arg.Any<PlanType>()).Returns(plan);
+
+        _organizationService
+            .UpdateCollectionManagementSettingsAsync(
+                organization.Id,
+                Arg.Is<OrganizationCollectionManagementSettings>(s =>
+                    s.LimitCollectionCreation == model.LimitCollectionCreation &&
+                    s.LimitCollectionDeletion == model.LimitCollectionDeletion &&
+                    s.LimitItemDeletion == model.LimitItemDeletion &&
+                    s.AllowAdminAccessToAllCollectionItems == model.AllowAdminAccessToAllCollectionItems))
+            .Returns(organization);
+
+        // Act
+        await _sut.PutCollectionManagement(organization.Id, model);
+
+        // Assert
+        await _organizationService
+            .Received(1)
+            .UpdateCollectionManagementSettingsAsync(
+                organization.Id,
+                Arg.Is<OrganizationCollectionManagementSettings>(s =>
+                    s.LimitCollectionCreation == model.LimitCollectionCreation &&
+                    s.LimitCollectionDeletion == model.LimitCollectionDeletion &&
+                    s.LimitItemDeletion == model.LimitItemDeletion &&
+                    s.AllowAdminAccessToAllCollectionItems == model.AllowAdminAccessToAllCollectionItems));
+    }
 }
--- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs
+++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs
@@ -1,6 +1,7 @@
 using System.Text.Json;
 using Bit.Core.AdminConsole.Entities.Provider;
 using Bit.Core.AdminConsole.Enums.Provider;
+using Bit.Core.AdminConsole.Models.Business;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers;
 using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models;
@@ -27,7 +28,6 @@ using Bit.Test.Common.AutoFixture.Attributes;
 using Bit.Test.Common.Fakes;
 using NSubstitute;
 using NSubstitute.ExceptionExtensions;
-using NSubstitute.ReceivedExtensions;
 using NSubstitute.ReturnsExtensions;
 using Stripe;
 using Xunit;
@@ -42,8 +42,6 @@ public class OrganizationServiceTests
 {
    private readonly IDataProtectorTokenFactory<OrgUserInviteTokenable> _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory<OrgUserInviteTokenable>();

-
-
    [Theory]
    [OrganizationInviteCustomize(InviteeUserType = OrganizationUserType.User,
         InvitorUserType = OrganizationUserType.Owner), OrganizationCustomize, BitAutoData]
@@ -1229,6 +1227,109 @@ public class OrganizationServiceTests
            .GetByIdentifierAsync(Arg.Is<string>(id => id == organization.Identifier));
    }

+    [Theory]
+    [BitAutoData(false, true, false, true)]
+    [BitAutoData(true, false, true, false)]
+    public async Task UpdateCollectionManagementSettingsAsync_WhenSettingsChanged_LogsSpecificEvents(
+        bool newLimitCollectionCreation,
+        bool newLimitCollectionDeletion,
+        bool newLimitItemDeletion,
+        bool newAllowAdminAccessToAllCollectionItems,
+        Organization existingOrganization, SutProvider<OrganizationService> sutProvider)
+    {
+        // Arrange
+        existingOrganization.LimitCollectionCreation = false;
+        existingOrganization.LimitCollectionDeletion = false;
+        existingOrganization.LimitItemDeletion = false;
+        existingOrganization.AllowAdminAccessToAllCollectionItems = false;
+
+        sutProvider.GetDependency<IOrganizationRepository>()
+            .GetByIdAsync(existingOrganization.Id)
+            .Returns(existingOrganization);
+
+        var settings = new OrganizationCollectionManagementSettings
+        {
+            LimitCollectionCreation = newLimitCollectionCreation,
+            LimitCollectionDeletion = newLimitCollectionDeletion,
+            LimitItemDeletion = newLimitItemDeletion,
+            AllowAdminAccessToAllCollectionItems = newAllowAdminAccessToAllCollectionItems
+        };
+
+        // Act
+        await sutProvider.Sut.UpdateCollectionManagementSettingsAsync(existingOrganization.Id, settings);
+
+        // Assert
+        var eventService = sutProvider.GetDependency<IEventService>();
+        if (newLimitCollectionCreation)
+        {
+            await eventService.Received(1).LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_LimitCollectionCreationEnabled));
+        }
+        else
+        {
+            await eventService.DidNotReceive().LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_LimitCollectionCreationEnabled));
+        }
+
+        if (newLimitCollectionDeletion)
+        {
+            await eventService.Received(1).LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_LimitCollectionDeletionEnabled));
+        }
+        else
+        {
+            await eventService.DidNotReceive().LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_LimitCollectionDeletionEnabled));
+        }
+
+        if (newLimitItemDeletion)
+        {
+            await eventService.Received(1).LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_LimitItemDeletionEnabled));
+        }
+        else
+        {
+            await eventService.DidNotReceive().LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_LimitItemDeletionEnabled));
+        }
+
+        if (newAllowAdminAccessToAllCollectionItems)
+        {
+            await eventService.Received(1).LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsEnabled));
+        }
+        else
+        {
+            await eventService.DidNotReceive().LogOrganizationEventAsync(
+                Arg.Is<Organization>(org => org.Id == existingOrganization.Id),
+                Arg.Is<EventType>(e => e == EventType.Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsEnabled));
+        }
+    }
+
+    [Theory, BitAutoData]
+    public async Task UpdateCollectionManagementSettingsAsync_WhenOrganizationNotFound_ThrowsNotFoundException(
+        Guid organizationId, OrganizationCollectionManagementSettings settings, SutProvider<OrganizationService> sutProvider)
+    {
+        // Arrange
+        sutProvider.GetDependency<IOrganizationRepository>()
+            .GetByIdAsync(organizationId)
+            .Returns((Organization)null);
+
+        // Act/Assert
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.UpdateCollectionManagementSettingsAsync(organizationId, settings));
+
+        await sutProvider.GetDependency<IOrganizationRepository>()
+            .Received(1)
+            .GetByIdAsync(organizationId);
+    }
+
    // Must set real guids in order for dictionary of guids to not throw aggregate exceptions
    private void SetupOrgUserRepositoryCreateManyAsyncMock(IOrganizationUserRepository organizationUserRepository)
    {