From 519b3dea2483ccd06bd9dd9019ef23e96173ee61 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Sat, 2 Dec 2023 01:28:10 +1000 Subject: [PATCH] [AC-1873] Fix: restore logic assigning Managers to new collections server-side (#3498) * Restore pre-flexible collections logic to assign managers to new collections * Dont overwrite existing access * Fix and add tests --- src/Api/Controllers/CollectionsController.cs | 31 +++++++++- .../LegacyCollectionsControllerTests.cs | 59 ++++++++++++++++++- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index 9a5a12cff..8d8e737a6 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -4,7 +4,9 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core; using Bit.Core.Context; using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Data; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -26,6 +28,7 @@ public class CollectionsController : Controller private readonly ICurrentContext _currentContext; private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand; private readonly IFeatureService _featureService; + private readonly IOrganizationUserRepository _organizationUserRepository; public CollectionsController( ICollectionRepository collectionRepository, @@ -35,7 +38,8 @@ public class CollectionsController : Controller IAuthorizationService authorizationService, ICurrentContext currentContext, IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand, - IFeatureService featureService) + IFeatureService featureService, + IOrganizationUserRepository organizationUserRepository) { _collectionRepository = collectionRepository; _collectionService = collectionService; @@ -45,6 +49,7 @@ public class CollectionsController : Controller _currentContext = currentContext; _bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand; _featureService = featureService; + _organizationUserRepository = organizationUserRepository; } private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); @@ -168,7 +173,29 @@ public class CollectionsController : Controller } var groups = model.Groups?.Select(g => g.ToSelectionReadOnly()); - var users = model.Users?.Select(g => g.ToSelectionReadOnly()); + var users = model.Users?.Select(g => g.ToSelectionReadOnly()).ToList() ?? new List(); + + // Pre-flexible collections logic assigned Managers to collections they create + var assignUserToCollection = + !FlexibleCollectionsIsEnabled && + !await _currentContext.EditAnyCollection(orgId) && + await _currentContext.EditAssignedCollections(orgId); + var isNewCollection = collection.Id == default; + + if (assignUserToCollection && isNewCollection && _currentContext.UserId.HasValue) + { + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, _currentContext.UserId.Value); + // don't add duplicate access if the user has already specified it themselves + var existingAccess = users.Any(u => u.Id == orgUser.Id); + if (orgUser is { Status: OrganizationUserStatusType.Confirmed } && !existingAccess) + { + users.Add(new CollectionAccessSelection + { + Id = orgUser.Id, + ReadOnly = false + }); + } + } await _collectionService.SaveAsync(collection, groups, users); return new CollectionResponseModel(collection); diff --git a/test/Api.Test/Controllers/LegacyCollectionsControllerTests.cs b/test/Api.Test/Controllers/LegacyCollectionsControllerTests.cs index c07ead63f..07c27d4ab 100644 --- a/test/Api.Test/Controllers/LegacyCollectionsControllerTests.cs +++ b/test/Api.Test/Controllers/LegacyCollectionsControllerTests.cs @@ -3,6 +3,7 @@ using Bit.Api.Models.Request; using Bit.Core.AdminConsole.Entities; using Bit.Core.Context; using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Data; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; @@ -12,6 +13,8 @@ using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; +using Collection = Bit.Core.Entities.Collection; +using User = Bit.Core.Entities.User; namespace Bit.Api.Test.Controllers; @@ -24,8 +27,11 @@ namespace Bit.Api.Test.Controllers; public class LegacyCollectionsControllerTests { [Theory, BitAutoData] - public async Task Post_Success(Guid orgId, SutProvider sutProvider) + public async Task Post_Manager_AssignsToCollection_Success(Guid orgId, OrganizationUser orgUser, SutProvider sutProvider) { + orgUser.Type = OrganizationUserType.Manager; + orgUser.Status = OrganizationUserStatusType.Confirmed; + sutProvider.GetDependency() .OrganizationManager(orgId) .Returns(true); @@ -34,6 +40,15 @@ public class LegacyCollectionsControllerTests .EditAnyCollection(orgId) .Returns(false); + sutProvider.GetDependency() + .EditAssignedCollections(orgId) + .Returns(true); + + sutProvider.GetDependency().UserId = orgUser.UserId; + + sutProvider.GetDependency().GetByOrganizationAsync(orgId, orgUser.UserId.Value) + .Returns(orgUser); + var collectionRequest = new CollectionRequestModel { Name = "encrypted_string", @@ -42,9 +57,49 @@ public class LegacyCollectionsControllerTests _ = await sutProvider.Sut.Post(orgId, collectionRequest); + var test = sutProvider.GetDependency().ReceivedCalls(); await sutProvider.GetDependency() .Received(1) - .SaveAsync(Arg.Any(), Arg.Any>(), null); + .SaveAsync(Arg.Any(), Arg.Any>(), + Arg.Is>(users => users.Any(u => u.Id == orgUser.Id && !u.ReadOnly && !u.HidePasswords && !u.Manage))); + } + + [Theory, BitAutoData] + public async Task Post_Owner_DoesNotAssignToCollection_Success(Guid orgId, OrganizationUser orgUser, SutProvider sutProvider) + { + orgUser.Type = OrganizationUserType.Owner; + orgUser.Status = OrganizationUserStatusType.Confirmed; + + sutProvider.GetDependency() + .OrganizationManager(orgId) + .Returns(true); + + sutProvider.GetDependency() + .EditAnyCollection(orgId) + .Returns(true); + + sutProvider.GetDependency() + .EditAssignedCollections(orgId) + .Returns(true); + + sutProvider.GetDependency().UserId = orgUser.UserId; + + sutProvider.GetDependency().GetByOrganizationAsync(orgId, orgUser.UserId.Value) + .Returns(orgUser); + + var collectionRequest = new CollectionRequestModel + { + Name = "encrypted_string", + ExternalId = "my_external_id" + }; + + _ = await sutProvider.Sut.Post(orgId, collectionRequest); + + var test = sutProvider.GetDependency().ReceivedCalls(); + await sutProvider.GetDependency() + .Received(1) + .SaveAsync(Arg.Any(), Arg.Any>(), + Arg.Is>(users => !users.Any())); } [Theory, BitAutoData]