From 2c0c113420f393751fbc28dfdd77cf0036c2c2cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Fri, 5 Sep 2025 12:01:14 +0100 Subject: [PATCH] [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 --- .../Controllers/OrganizationsController.cs | 8 +- ...nCollectionManagementUpdateRequestModel.cs | 16 ++- src/Core/AdminConsole/Enums/EventType.cs | 11 +- ...rganizationCollectionManagementSettings.cs | 9 ++ .../Services/IOrganizationService.cs | 4 +- .../Implementations/OrganizationService.cs | 74 +++++++++++- .../OrganizationsControllerTests.cs | 39 +++++++ .../Services/OrganizationServiceTests.cs | 107 +++++++++++++++++- 8 files changed, 242 insertions(+), 26 deletions(-) create mode 100644 src/Core/AdminConsole/Models/Business/OrganizationCollectionManagementSettings.cs diff --git a/src/Api/AdminConsole/Controllers/OrganizationsController.cs b/src/Api/AdminConsole/Controllers/OrganizationsController.cs index 8b1a6243c3..17e6a60cd9 100644 --- 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 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); } diff --git a/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs b/src/Api/Models/Request/Organizations/OrganizationCollectionManagementUpdateRequestModel.cs index 829840c896..93866161c0 100644 --- 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 + }; } diff --git a/src/Core/AdminConsole/Enums/EventType.cs b/src/Core/AdminConsole/Enums/EventType.cs index 32ea4a64e9..81501fd6ec 100644 --- 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, diff --git a/src/Core/AdminConsole/Models/Business/OrganizationCollectionManagementSettings.cs b/src/Core/AdminConsole/Models/Business/OrganizationCollectionManagementSettings.cs new file mode 100644 index 0000000000..aff2244598 --- /dev/null +++ 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; } +} diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index 8c47ae049c..94df74afdf 100644 --- 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 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 UpdateCollectionManagementSettingsAsync(Guid organizationId, OrganizationCollectionManagementSettings settings); Task UpdateTwoFactorProviderAsync(Organization organization, TwoFactorProviderType type); Task DisableTwoFactorProviderAsync(Organization organization, TwoFactorProviderType type); Task InviteUserAsync(Guid organizationId, Guid? invitingUserId, EventSystemUser? systemUser, diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index f418737508..57eb4f51de 100644 --- 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 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> CreateCollectionManagementLoggingActions( + Organization existingOrganization, OrganizationCollectionManagementSettings settings) + { + var loggingActions = new List>(); + + 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; + } } diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationsControllerTests.cs index 3484c9a995..00fd3c3b4e 100644 --- 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()).Returns(plan); + + _organizationService + .UpdateCollectionManagementSettingsAsync( + organization.Id, + Arg.Is(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(s => + s.LimitCollectionCreation == model.LimitCollectionCreation && + s.LimitCollectionDeletion == model.LimitCollectionDeletion && + s.LimitItemDeletion == model.LimitItemDeletion && + s.AllowAdminAccessToAllCollectionItems == model.AllowAdminAccessToAllCollectionItems)); + } } diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index e3f26a898d..33f2e78799 100644 --- 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 _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); - - [Theory] [OrganizationInviteCustomize(InviteeUserType = OrganizationUserType.User, InvitorUserType = OrganizationUserType.Owner), OrganizationCustomize, BitAutoData] @@ -1229,6 +1227,109 @@ public class OrganizationServiceTests .GetByIdentifierAsync(Arg.Is(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 sutProvider) + { + // Arrange + existingOrganization.LimitCollectionCreation = false; + existingOrganization.LimitCollectionDeletion = false; + existingOrganization.LimitItemDeletion = false; + existingOrganization.AllowAdminAccessToAllCollectionItems = false; + + sutProvider.GetDependency() + .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(); + if (newLimitCollectionCreation) + { + await eventService.Received(1).LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_LimitCollectionCreationEnabled)); + } + else + { + await eventService.DidNotReceive().LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_LimitCollectionCreationEnabled)); + } + + if (newLimitCollectionDeletion) + { + await eventService.Received(1).LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_LimitCollectionDeletionEnabled)); + } + else + { + await eventService.DidNotReceive().LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_LimitCollectionDeletionEnabled)); + } + + if (newLimitItemDeletion) + { + await eventService.Received(1).LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_LimitItemDeletionEnabled)); + } + else + { + await eventService.DidNotReceive().LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_LimitItemDeletionEnabled)); + } + + if (newAllowAdminAccessToAllCollectionItems) + { + await eventService.Received(1).LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsEnabled)); + } + else + { + await eventService.DidNotReceive().LogOrganizationEventAsync( + Arg.Is(org => org.Id == existingOrganization.Id), + Arg.Is(e => e == EventType.Organization_CollectionManagement_AllowAdminAccessToAllCollectionItemsEnabled)); + } + } + + [Theory, BitAutoData] + public async Task UpdateCollectionManagementSettingsAsync_WhenOrganizationNotFound_ThrowsNotFoundException( + Guid organizationId, OrganizationCollectionManagementSettings settings, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency() + .GetByIdAsync(organizationId) + .Returns((Organization)null); + + // Act/Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateCollectionManagementSettingsAsync(organizationId, settings)); + + await sutProvider.GetDependency() + .Received(1) + .GetByIdAsync(organizationId); + } + // Must set real guids in order for dictionary of guids to not throw aggregate exceptions private void SetupOrgUserRepositoryCreateManyAsyncMock(IOrganizationUserRepository organizationUserRepository) {