From c147b5bd5a68e4631e1c9344ba2865c1425b5132 Mon Sep 17 00:00:00 2001 From: jng Date: Wed, 2 Oct 2024 16:23:25 -0400 Subject: [PATCH 1/4] updated collections_v2 and bulk-collections endpoint for viewPassword permission on ciphers --- .../Vault/Controllers/CiphersController.cs | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 769ba34a16d1..4d52307ab10a 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -420,6 +420,59 @@ private async Task CanAccessUnassignedCiphersAsync(Guid organizationId) return false; } + + /// + /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 + /// + private async Task CanModifyCipherCollectionsAsync(Guid organizationId, IEnumerable cipherIds) + { + // If the user can edit all ciphers for the organization, just check they all belong to the org + if (await CanEditAllCiphersAsync(organizationId)) + { + // TODO: This can likely be optimized to only query the requested ciphers and then checking they belong to the org + var orgCiphers = (await _cipherRepository.GetManyByOrganizationIdAsync(organizationId)).ToDictionary(c => c.Id); + + // Ensure all requested ciphers are in orgCiphers + if (cipherIds.Any(c => !orgCiphers.ContainsKey(c))) + { + return false; + } + + return true; + } + + // The user cannot access any ciphers for the organization, we're done + if (!await CanAccessOrganizationCiphersAsync(organizationId)) + { + return false; + } + + var userId = _userService.GetProperUserId(User).Value; + // Select all editable ciphers for this user belonging to the organization + var editableOrgCipherList = (await _cipherRepository.GetManyByUserIdAsync(userId, true)) + .Where(c => c.OrganizationId == organizationId && c.UserId == null && c.Edit && c.ViewPassword).ToList(); + + // Special case for unassigned ciphers + if (await CanAccessUnassignedCiphersAsync(organizationId)) + { + var unassignedCiphers = + (await _cipherRepository.GetManyUnassignedOrganizationDetailsByOrganizationIdAsync( + organizationId)); + + // Users that can access unassigned ciphers can also edit them + editableOrgCipherList.AddRange(unassignedCiphers.Select(c => new CipherDetails(c) { Edit = true })); + } + + var editableOrgCiphers = editableOrgCipherList + .ToDictionary(c => c.Id); + + if (cipherIds.Any(c => !editableOrgCiphers.ContainsKey(c))) + { + return false; + } + + return true; + } /// /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 @@ -576,7 +629,7 @@ public async Task PutCollections_vNext(Guid var userId = _userService.GetProperUserId(User).Value; var cipher = await GetByIdAsync(id, userId); if (cipher == null || !cipher.OrganizationId.HasValue || - !await _currentContext.OrganizationUser(cipher.OrganizationId.Value)) + !await _currentContext.OrganizationUser(cipher.OrganizationId.Value) || !cipher.ViewPassword) { throw new NotFoundException(); } @@ -626,7 +679,7 @@ public async Task PutCollectionsAdmin(string id, [FromBody] CipherCollectionsReq [HttpPost("bulk-collections")] public async Task PostBulkCollections([FromBody] CipherBulkUpdateCollectionsRequestModel model) { - if (!await CanEditCiphersAsync(model.OrganizationId, model.CipherIds) || + if (!await CanModifyCipherCollectionsAsync(model.OrganizationId, model.CipherIds) || !await CanEditItemsInCollections(model.OrganizationId, model.CollectionIds)) { throw new NotFoundException(); From f5412856b2afcca70f455e992d50932cbc999643 Mon Sep 17 00:00:00 2001 From: jng Date: Fri, 4 Oct 2024 15:41:31 -0400 Subject: [PATCH 2/4] update ciphers mock in CiphersControllerTests --- test/Api.Test/Vault/Controllers/CiphersControllerTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index e7c5cd9ef586..2afce14ac570 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -127,6 +127,7 @@ private CipherDetails CreateCipherDetailsMock(Guid id, Guid userId) UserId = userId, OrganizationId = Guid.NewGuid(), Type = CipherType.Login, + ViewPassword = true, Data = @" { ""Uris"": [ From 0df254ae851d6daa0f21d6d283a7f19ba7128d8b Mon Sep 17 00:00:00 2001 From: jng Date: Fri, 4 Oct 2024 15:50:04 -0400 Subject: [PATCH 3/4] formatted --- src/Api/Vault/Controllers/CiphersController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 4d52307ab10a..6520f3bdfb0f 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -420,7 +420,7 @@ private async Task CanAccessUnassignedCiphersAsync(Guid organizationId) return false; } - + /// /// TODO: Move this to its own authorization handler or equivalent service - AC-2062 /// From a9d1df64bc78de418c46f58fc788c83285635720 Mon Sep 17 00:00:00 2001 From: jng Date: Thu, 31 Oct 2024 11:58:51 -0400 Subject: [PATCH 4/4] updates to CipherDetails query for accurate ViewPassword value --- .../Cipher/CipherDetails_ReadByIdUserId.sql | 37 +++++++++++++-- ...2024-10-29_00_UpdateCipherDetailsQuery.sql | 45 +++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 util/Migrator/DbScripts/2024-10-29_00_UpdateCipherDetailsQuery.sql diff --git a/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql b/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql index e2fb2629bd3a..20eecbf3ccd8 100644 --- a/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql +++ b/src/Sql/Vault/dbo/Stored Procedures/Cipher/CipherDetails_ReadByIdUserId.sql @@ -5,12 +5,41 @@ AS BEGIN SET NOCOUNT ON - SELECT TOP 1 - * +SELECT + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp] + , MAX ([Edit]) AS [Edit] + , MAX ([ViewPassword]) AS [ViewPassword] FROM [dbo].[UserCipherDetails](@UserId) WHERE [Id] = @Id - ORDER BY - [Edit] DESC + GROUP BY + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp] + END \ No newline at end of file diff --git a/util/Migrator/DbScripts/2024-10-29_00_UpdateCipherDetailsQuery.sql b/util/Migrator/DbScripts/2024-10-29_00_UpdateCipherDetailsQuery.sql new file mode 100644 index 000000000000..f433d62372ee --- /dev/null +++ b/util/Migrator/DbScripts/2024-10-29_00_UpdateCipherDetailsQuery.sql @@ -0,0 +1,45 @@ +CREATE OR ALTER PROCEDURE [dbo].[CipherDetails_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + +SELECT + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp], + MAX ([Edit]) AS [Edit], + MAX ([ViewPassword]) AS [ViewPassword] +FROM + [dbo].[UserCipherDetails](@UserId) +WHERE + [Id] = @Id +GROUP BY + [Id], + [UserId], + [OrganizationId], + [Type], + [Data], + [Attachments], + [CreationDate], + [RevisionDate], + [Favorite], + [FolderId], + [DeletedDate], + [Reprompt], + [Key], + [OrganizationUseTotp] +END +GO