diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 931dbed504a1..63784855e51f 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -952,8 +952,8 @@ public async Task PostShare(Guid id, [FromBody] CipherShare return await PutShare(id, model); } - [HttpPut("{id}/collections")] - public async Task PutCollections(Guid id, [FromBody] CipherCollectionsRequestModel model) + [HttpPut("{id}/shared-folders")] + public async Task PutSharedFolders(Guid id, [FromBody] CipherSharedFoldersRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); var cipher = await GetByIdAsync(id, user.Id); @@ -964,23 +964,36 @@ public async Task PutCollections(Guid id, [FromBody] } await _cipherService.SaveCollectionsAsync(cipher, - model.CollectionIds.Select(c => new Guid(c)), user.Id, false); + model.SharedFolderIds.Select(c => new Guid(c)), user.Id, false); var updatedCipher = await GetByIdAsync(id, user.Id); var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(user.Id, id); - return await BuildWriteReturnDetailsResponseAsync(updatedCipher, user, collectionCiphers); + var response = await BuildWriteReturnDetailsResponseAsync(updatedCipher, user, collectionCiphers); + response.UseSharedFolderTerms(); + return response; + } + + [HttpPut("{id}/collections")] + [Obsolete("Replaced by /shared-folders route; retained for backwards compatibility")] + public async Task PutCollections(Guid id, [FromBody] CipherCollectionsRequestModel model) + { + var response = await PutSharedFolders(id, ToSharedFoldersRequest(model)); + response.UseCollectionTerms(); + return response; } [HttpPost("{id}/collections")] [Obsolete("This endpoint is deprecated. Use PUT method instead.")] public async Task PostCollections(Guid id, [FromBody] CipherCollectionsRequestModel model) { - return await PutCollections(id, model); + var response = await PutSharedFolders(id, ToSharedFoldersRequest(model)); + response.UseCollectionTerms(); + return response; } - [HttpPut("{id}/collections_v2")] - public async Task PutCollections_vNext(Guid id, [FromBody] CipherCollectionsRequestModel model) + [HttpPut("{id}/shared-folders_v2")] + public async Task PutSharedFolders_vNext(Guid id, [FromBody] CipherSharedFoldersRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); var cipher = await GetByIdAsync(id, user.Id); @@ -991,7 +1004,7 @@ public async Task PutCollections_vNext(Guid } await _cipherService.SaveCollectionsAsync(cipher, - model.CollectionIds.Select(c => new Guid(c)), user.Id, false); + model.SharedFolderIds.Select(c => new Guid(c)), user.Id, false); var updatedCipher = await GetByIdAsync(id, user.Id); var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(user.Id, id); @@ -1004,6 +1017,16 @@ await _cipherService.SaveCollectionsAsync(cipher, ? null : await BuildWriteReturnDetailsResponseAsync(updatedCipher, user, collectionCiphers) }; + response.Cipher?.UseSharedFolderTerms(); + return response; + } + + [HttpPut("{id}/collections_v2")] + [Obsolete("Replaced by /shared-folders_v2 route; retained for backwards compatibility")] + public async Task PutCollections_vNext(Guid id, [FromBody] CipherCollectionsRequestModel model) + { + var response = await PutSharedFolders_vNext(id, ToSharedFoldersRequest(model)); + response.Cipher?.UseCollectionTerms(); return response; } @@ -1011,11 +1034,13 @@ await _cipherService.SaveCollectionsAsync(cipher, [Obsolete("This endpoint is deprecated. Use PUT method instead.")] public async Task PostCollections_vNext(Guid id, [FromBody] CipherCollectionsRequestModel model) { - return await PutCollections_vNext(id, model); + var response = await PutSharedFolders_vNext(id, ToSharedFoldersRequest(model)); + response.Cipher?.UseCollectionTerms(); + return response; } - [HttpPut("{id}/collections-admin")] - public async Task PutCollectionsAdmin(string id, [FromBody] CipherCollectionsRequestModel model) + [HttpPut("{id}/shared-folders-admin")] + public async Task PutSharedFoldersAdmin(string id, [FromBody] CipherSharedFoldersRequestModel model) { var userId = _userService.GetProperUserId(User).Value; var cipher = await _cipherRepository.GetOrganizationDetailsByIdAsync(new Guid(id)); @@ -1026,7 +1051,7 @@ public async Task PutCollectionsAdmin(string id, throw new NotFoundException(); } - var collectionIds = model.CollectionIds.Select(c => new Guid(c)).ToList(); + var collectionIds = model.SharedFolderIds.Select(c => new Guid(c)).ToList(); // In V1, we still need to check if the user can edit the collections they're submitting // This should only happen for unassigned ciphers (otherwise restricted admins would use the normal collections endpoint) @@ -1041,25 +1066,38 @@ public async Task PutCollectionsAdmin(string id, var collectionCiphersGroupDict = collectionCiphers.GroupBy(c => c.CipherId).ToDictionary(s => s.Key); var access = await AuthorizeAdminWriteReturnOrThrowAsync(userId, cipher.OrganizationId.Value, cipher); - return CipherMiniDetailsResponseModel.From(access, cipher, _globalSettings, + var response = CipherMiniDetailsResponseModel.From(access, cipher, _globalSettings, collectionCiphersGroupDict, cipher.OrganizationUseTotp); + response.UseSharedFolderTerms(); + return response; + } + + [HttpPut("{id}/collections-admin")] + [Obsolete("Replaced by /shared-folders-admin route; retained for backwards compatibility")] + public async Task PutCollectionsAdmin(string id, [FromBody] CipherCollectionsRequestModel model) + { + var response = await PutSharedFoldersAdmin(id, ToSharedFoldersRequest(model)); + response.UseCollectionTerms(); + return response; } [HttpPost("{id}/collections-admin")] [Obsolete("This endpoint is deprecated. Use PUT method instead.")] public async Task PostCollectionsAdmin(string id, [FromBody] CipherCollectionsRequestModel model) { - return await PutCollectionsAdmin(id, model); + var response = await PutSharedFoldersAdmin(id, ToSharedFoldersRequest(model)); + response.UseCollectionTerms(); + return response; } - [HttpPost("bulk-collections")] - public async Task PostBulkCollections([FromBody] CipherBulkUpdateCollectionsRequestModel model) + [HttpPost("bulk-shared-folders")] + public async Task PostBulkSharedFolders([FromBody] CipherBulkUpdateSharedFoldersRequestModel model) { var userId = _userService.GetProperUserId(User).Value; - await _cipherService.ValidateBulkCollectionAssignmentAsync(model.CollectionIds, model.CipherIds, userId, model.RemoveCollections); + await _cipherService.ValidateBulkCollectionAssignmentAsync(model.SharedFolderIds, model.CipherIds, userId, model.RemoveSharedFolders); if (!await CanModifyCipherCollectionsAsync(model.OrganizationId, model.CipherIds) || - !await CanEditItemsInCollections(model.OrganizationId, model.CollectionIds)) + !await CanEditItemsInCollections(model.OrganizationId, model.SharedFolderIds)) { throw new NotFoundException(); } @@ -1076,16 +1114,32 @@ public async Task PostBulkCollections([FromBody] CipherBulkUpdateCollectionsRequ await _cipherLeaseGate.EnsureCanMutateManyAsync(userId, ciphers); } - if (model.RemoveCollections) + if (model.RemoveSharedFolders) { - await _collectionCipherRepository.RemoveCollectionsForManyCiphersAsync(model.OrganizationId, model.CipherIds, model.CollectionIds); + await _collectionCipherRepository.RemoveCollectionsForManyCiphersAsync(model.OrganizationId, model.CipherIds, model.SharedFolderIds); } else { - await _collectionCipherRepository.AddCollectionsForManyCiphersAsync(model.OrganizationId, model.CipherIds, model.CollectionIds); + await _collectionCipherRepository.AddCollectionsForManyCiphersAsync(model.OrganizationId, model.CipherIds, model.SharedFolderIds); } } + [HttpPost("bulk-collections")] + [Obsolete("Replaced by /bulk-shared-folders route; retained for backwards compatibility")] + public async Task PostBulkCollections([FromBody] CipherBulkUpdateCollectionsRequestModel model) + { + await PostBulkSharedFolders(new CipherBulkUpdateSharedFoldersRequestModel + { + OrganizationId = model.OrganizationId, + CipherIds = model.CipherIds, + SharedFolderIds = model.CollectionIds, + RemoveSharedFolders = model.RemoveCollections, + }); + } + + private static CipherSharedFoldersRequestModel ToSharedFoldersRequest(CipherCollectionsRequestModel model) => + new() { SharedFolderIds = model.CollectionIds }; + [HttpPut("{id}/archive")] public async Task PutArchive(Guid id) { diff --git a/src/Api/Vault/Models/Request/CipherBulkUpdateSharedFoldersRequestModel.cs b/src/Api/Vault/Models/Request/CipherBulkUpdateSharedFoldersRequestModel.cs new file mode 100644 index 000000000000..4dd3ad08fd10 --- /dev/null +++ b/src/Api/Vault/Models/Request/CipherBulkUpdateSharedFoldersRequestModel.cs @@ -0,0 +1,18 @@ +// FIXME: Update this file to be null safe and then delete the line below +#nullable disable + +namespace Bit.Api.Vault.Models.Request; + +public class CipherBulkUpdateSharedFoldersRequestModel +{ + public Guid OrganizationId { get; set; } + + public IEnumerable CipherIds { get; set; } + + public IEnumerable SharedFolderIds { get; set; } + + /// + /// If true, the shared folders will be removed from the ciphers. Otherwise, they will be added. + /// + public bool RemoveSharedFolders { get; set; } +} diff --git a/src/Api/Vault/Models/Request/CipherRequestModel.cs b/src/Api/Vault/Models/Request/CipherRequestModel.cs index 78e5afdb512f..e2c9b0424fb5 100644 --- a/src/Api/Vault/Models/Request/CipherRequestModel.cs +++ b/src/Api/Vault/Models/Request/CipherRequestModel.cs @@ -577,6 +577,12 @@ public class CipherCollectionsRequestModel public IEnumerable CollectionIds { get; set; } } +public class CipherSharedFoldersRequestModel +{ + [Required] + public IEnumerable SharedFolderIds { get; set; } +} + public class CipherBulkArchiveRequestModel { [Required] diff --git a/src/Api/Vault/Models/Response/CipherResponseModel.cs b/src/Api/Vault/Models/Response/CipherResponseModel.cs index 3adbbb4748e3..0e14d653678b 100644 --- a/src/Api/Vault/Models/Response/CipherResponseModel.cs +++ b/src/Api/Vault/Models/Response/CipherResponseModel.cs @@ -406,7 +406,27 @@ public static CipherDetailsResponseModel From(FullCipherAccess access, CipherDet ? new FullCipherDetailsResponseModel(access, cipher, user, organizationAbility, globalSettings, obj) : new PartialCipherDetailsResponseModel(cipher, user, organizationAbility, obj); - public IEnumerable CollectionIds { get; set; } + /// Legacy term. Omitted from the payload when is in use. + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public IEnumerable? CollectionIds { get; set; } + + /// Shared-folder term for . Only populated by . + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public IEnumerable? SharedFolderIds { get; set; } + + /// Moves the collection ids to so the payload uses shared-folder terms. + public void UseSharedFolderTerms() + { + SharedFolderIds ??= CollectionIds; + CollectionIds = null; + } + + /// Reverses for legacy (collections) endpoints. + public void UseCollectionTerms() + { + CollectionIds ??= SharedFolderIds; + SharedFolderIds = null; + } } /// The reduced-data counterpart of . @@ -520,7 +540,27 @@ public static CipherMiniDetailsResponseModel From(FullCipherAccess access, string obj = "cipherMiniDetails") => From(access, cipher, globalSettings, cipher.OrganizationUseTotp, obj); - public IEnumerable CollectionIds { get; set; } + /// Legacy term. Omitted from the payload when is in use. + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public IEnumerable? CollectionIds { get; set; } + + /// Shared-folder term for . Only populated by . + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public IEnumerable? SharedFolderIds { get; set; } + + /// Moves the collection ids to so the payload uses shared-folder terms. + public void UseSharedFolderTerms() + { + SharedFolderIds ??= CollectionIds; + CollectionIds = null; + } + + /// Reverses for legacy (collections) endpoints. + public void UseCollectionTerms() + { + CollectionIds ??= SharedFolderIds; + SharedFolderIds = null; + } } /// The reduced-data counterpart of . diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index fce539b5f8fb..2aead5a6f9d0 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -31,6 +31,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; using NSubstitute.ExceptionExtensions; using NSubstitute.ReturnsExtensions; @@ -138,20 +139,20 @@ await sutProvider.GetDependency() } [Theory, BitAutoData] - public async Task PutCollections_vNextShouldThrowExceptionWhenCipherIsNullOrNoOrgValue(Guid id, CipherCollectionsRequestModel model, User user, + public async Task PutSharedFolders_vNextShouldThrowExceptionWhenCipherIsNullOrNoOrgValue(Guid id, CipherSharedFoldersRequestModel model, User user, SutProvider sutProvider) { sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); sutProvider.GetDependency().OrganizationUser(Guid.NewGuid()).Returns(false); sutProvider.GetDependency().GetByIdAsync(id, user.Id).ReturnsNull(); - var requestAction = async () => await sutProvider.Sut.PutCollections_vNext(id, model); + var requestAction = async () => await sutProvider.Sut.PutSharedFolders_vNext(id, model); await Assert.ThrowsAsync(requestAction); } [Theory, BitAutoData] - public async Task PutCollections_vNextShouldSaveUpdatedCipher(Guid id, CipherCollectionsRequestModel model, Guid userId, SutProvider sutProvider) + public async Task PutSharedFolders_vNextShouldSaveUpdatedCipher(Guid id, CipherSharedFoldersRequestModel model, Guid userId, SutProvider sutProvider) { SetupUserAndOrgMocks(id, userId, sutProvider); var cipherDetails = CreateCipherDetailsMock(id, userId); @@ -161,13 +162,13 @@ public async Task PutCollections_vNextShouldSaveUpdatedCipher(Guid id, CipherCol sutProvider.GetDependency().GetOrganizationAbilityAsync(cipherDetails.OrganizationId.Value).Returns(new OrganizationAbility { Id = cipherDetails.OrganizationId.Value }); var cipherService = sutProvider.GetDependency(); - await sutProvider.Sut.PutCollections_vNext(id, model); + await sutProvider.Sut.PutSharedFolders_vNext(id, model); await cipherService.ReceivedWithAnyArgs().SaveCollectionsAsync(default, default, default, default); } [Theory, BitAutoData] - public async Task PutCollections_vNextReturnOptionalDetailsCipherUnavailableFalse(Guid id, CipherCollectionsRequestModel model, Guid userId, SutProvider sutProvider) + public async Task PutSharedFolders_vNextReturnOptionalDetailsCipherUnavailableFalse(Guid id, CipherSharedFoldersRequestModel model, Guid userId, SutProvider sutProvider) { SetupUserAndOrgMocks(id, userId, sutProvider); var cipherDetails = CreateCipherDetailsMock(id, userId); @@ -176,14 +177,14 @@ public async Task PutCollections_vNextReturnOptionalDetailsCipherUnavailableFals sutProvider.GetDependency().GetManyByUserIdCipherIdAsync(userId, id).Returns((ICollection)new List()); sutProvider.GetDependency().GetOrganizationAbilityAsync(cipherDetails.OrganizationId.Value).Returns(new OrganizationAbility { Id = cipherDetails.OrganizationId.Value }); - var result = await sutProvider.Sut.PutCollections_vNext(id, model); + var result = await sutProvider.Sut.PutSharedFolders_vNext(id, model); Assert.IsType(result); Assert.False(result.Unavailable); } [Theory, BitAutoData] - public async Task PutCollections_vNextReturnOptionalDetailsCipherUnavailableTrue(Guid id, CipherCollectionsRequestModel model, Guid userId, SutProvider sutProvider) + public async Task PutSharedFolders_vNextReturnOptionalDetailsCipherUnavailableTrue(Guid id, CipherSharedFoldersRequestModel model, Guid userId, SutProvider sutProvider) { SetupUserAndOrgMocks(id, userId, sutProvider); var cipherDetails = CreateCipherDetailsMock(id, userId); @@ -191,12 +192,110 @@ public async Task PutCollections_vNextReturnOptionalDetailsCipherUnavailableTrue sutProvider.GetDependency().GetManyByUserIdCipherIdAsync(userId, id).Returns((ICollection)new List()); - var result = await sutProvider.Sut.PutCollections_vNext(id, model); + var result = await sutProvider.Sut.PutSharedFolders_vNext(id, model); Assert.IsType(result); Assert.True(result.Unavailable); } + [Theory, BitAutoData] + public async Task PutSharedFolders_vNext_ResponseUsesSharedFolderTerms(Guid id, Guid sharedFolderId, Guid userId, + SutProvider sutProvider) + { + var response = await InvokeVNextAsync(id, userId, sharedFolderId, sutProvider, + (sut, model) => sut.PutSharedFolders_vNext(id, new CipherSharedFoldersRequestModel { SharedFolderIds = model })); + + Assert.NotNull(response.Cipher); + Assert.Null(response.Cipher.CollectionIds); + Assert.Equal([sharedFolderId], response.Cipher.SharedFolderIds); + } + + [Theory, BitAutoData] + public async Task PutCollections_vNext_ForwardsRequestAndResponseUsesCollectionTerms(Guid id, Guid collectionId, Guid userId, + SutProvider sutProvider) + { + var response = await InvokeVNextAsync(id, userId, collectionId, sutProvider, + (sut, model) => sut.PutCollections_vNext(id, new CipherCollectionsRequestModel { CollectionIds = model })); + + Assert.NotNull(response.Cipher); + Assert.Null(response.Cipher.SharedFolderIds); + Assert.Equal([collectionId], response.Cipher.CollectionIds); + await sutProvider.GetDependency().Received(1).SaveCollectionsAsync( + Arg.Any(), Arg.Is>(ids => ids.SequenceEqual(new[] { collectionId })), userId, false); + } + + [Theory, BitAutoData] + public async Task PostBulkCollections_ForwardsRenamedFieldsToSharedFolders(CipherBulkUpdateCollectionsRequestModel model, + Guid userId, SutProvider sutProvider) + { + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); + sutProvider.GetDependency().GetOrganization(default).ReturnsNullForAnyArgs(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.PostBulkCollections(model)); + + await sutProvider.GetDependency().Received(1).ValidateBulkCollectionAssignmentAsync( + model.CollectionIds, model.CipherIds, userId, model.RemoveCollections); + } + + private async Task InvokeVNextAsync(Guid id, Guid userId, Guid folderId, + SutProvider sutProvider, + Func, Task> act) + { + SetupUserAndOrgMocks(id, userId, sutProvider); + var cipherDetails = CreateCipherDetailsMock(id, userId); + sutProvider.GetDependency().GetByIdAsync(id, userId).Returns(cipherDetails); + sutProvider.GetDependency().GetManyByUserIdCipherIdAsync(userId, id) + .Returns((ICollection)new List + { + new() { CipherId = id, CollectionId = folderId } + }); + sutProvider.GetDependency() + .GetOrganizationAbilityAsync(cipherDetails.OrganizationId.Value) + .Returns(new OrganizationAbility { Id = cipherDetails.OrganizationId.Value }); + + return await act(sutProvider.Sut, [folderId.ToString()]); + } + + [Theory, BitAutoData] + public async Task PutSharedFolders_ThrowsNotFoundWhenCipherIsNull(Guid id, CipherSharedFoldersRequestModel model, User user, + SutProvider sutProvider) + { + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().GetByIdAsync(id, user.Id).ReturnsNull(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.PutSharedFolders(id, model)); + } + + [Theory, BitAutoData] + public async Task PostBulkSharedFolders_ThrowsNotFoundWhenUserCannotModify(CipherBulkUpdateSharedFoldersRequestModel model, + Guid userId, SutProvider sutProvider) + { + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); + sutProvider.GetDependency().GetOrganization(default).ReturnsNullForAnyArgs(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.PostBulkSharedFolders(model)); + } + + [Theory] + [InlineData(nameof(CiphersController.PutCollections), nameof(CiphersController.PutSharedFolders), "{id}/shared-folders")] + [InlineData(nameof(CiphersController.PutCollections_vNext), nameof(CiphersController.PutSharedFolders_vNext), "{id}/shared-folders_v2")] + [InlineData(nameof(CiphersController.PutCollectionsAdmin), nameof(CiphersController.PutSharedFoldersAdmin), "{id}/shared-folders-admin")] + [InlineData(nameof(CiphersController.PostBulkCollections), nameof(CiphersController.PostBulkSharedFolders), "bulk-shared-folders")] + public void SharedFolderAliasRoutes_AreRoutedAndLegacyRoutesAreObsolete(string legacyName, string aliasName, string expectedTemplate) + { + var legacy = typeof(CiphersController).GetMethod(legacyName)!; + var alias = typeof(CiphersController).GetMethod(aliasName)!; + + var obsolete = Assert.Single(legacy.GetCustomAttributes(typeof(ObsoleteAttribute), false).Cast()); + Assert.Equal($"Replaced by /{expectedTemplate.Replace("{id}/", "")} route; retained for backwards compatibility", obsolete.Message); + Assert.Empty(alias.GetCustomAttributes(typeof(ObsoleteAttribute), false)); + + var legacyVerb = legacy.GetCustomAttributes(typeof(HttpMethodAttribute), false).Cast().Single(); + var aliasVerb = alias.GetCustomAttributes(typeof(HttpMethodAttribute), false).Cast().Single(); + Assert.Equal(legacyVerb.HttpMethods, aliasVerb.HttpMethods); + Assert.Equal(expectedTemplate, aliasVerb.Template); + } + private void SetupUserAndOrgMocks(Guid id, Guid userId, SutProvider sutProvider) { sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId);