[PM-11162] Assign to Collection Permission Update (#4844)

Only users with Manage/Edit permissions will be allowed to Assign To Collections. If the user has Can Edit Except Password the collections dropdown will be disabled. --------- Co-authored-by: Matt Bishop <[email protected]> Co-authored-by: kejaeger <[email protected]>
bitwarden · Feb 4, 2025 · 412c6f9 · 412c6f9
1 parent 90680f4
commit 412c6f9
Show file tree

Hide file tree

Showing 6 changed files with 233 additions and 39 deletions.
diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs
@@ -424,6 +424,59 @@ private async Task<bool> CanAccessUnassignedCiphersAsync(Guid organizationId)
         return false;
     }
 
+    /// <summary>
+    /// TODO: Move this to its own authorization handler or equivalent service - AC-2062
+    /// </summary>
+    private async Task<bool> CanModifyCipherCollectionsAsync(Guid organizationId, IEnumerable<Guid> 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;
+    }
+
     /// <summary>
     /// TODO: Move this to its own authorization handler or equivalent service - AC-2062
     /// </summary>
@@ -579,7 +632,7 @@ public async Task<OptionalCipherDetailsResponseModel> 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();
         }
@@ -634,7 +687,7 @@ public async Task<CipherMiniDetailsResponseModel> PutCollectionsAdmin(string id,
     [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();

diff --git a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs
@@ -98,7 +98,7 @@ public async Task<ICollection<CipherDetails>> GetManyByUserIdAsync(Guid userId,
 
             return results
                 .GroupBy(c => c.Id)
-                .Select(g => g.OrderByDescending(og => og.Edit).First())
+                .Select(g => g.OrderByDescending(og => og.Edit).ThenByDescending(og => og.ViewPassword).First())
                 .ToList();
         }
     }

diff --git 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,40 @@ 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
-END
+    GROUP BY
+        [Id],
+        [UserId],
+        [OrganizationId],
+        [Type],
+        [Data],
+        [Attachments],
+        [CreationDate],
+        [RevisionDate],
+        [Favorite],
+        [FolderId],
+        [DeletedDate],
+        [Reprompt],
+        [Key],
+        [OrganizationUseTotp]
+END
diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections.sql
@@ -14,10 +14,9 @@ BEGIN
         WHERE
             [Id] = @CipherId
     )
-
-    ;WITH [AvailableCollectionsCTE] AS(
-        SELECT
+    SELECT
             C.[Id]
+            INTO #TempAvailableCollections
         FROM
             [dbo].[Collection] C
         INNER JOIN
@@ -40,38 +39,33 @@ BEGIN
                 CU.[ReadOnly] = 0
                 OR CG.[ReadOnly] = 0
             )
-    ),
-    [CollectionCiphersCTE] AS(
-        SELECT
-            [CollectionId],
-            [CipherId]
-        FROM
-            [dbo].[CollectionCipher]
-        WHERE
-            [CipherId] = @CipherId
+    -- Insert new collection assignments
+    INSERT INTO [dbo].[CollectionCipher] (
+        [CollectionId],
+        [CipherId]
     )
-    MERGE
-        [CollectionCiphersCTE] AS [Target]
-    USING
-        @CollectionIds AS [Source]
-    ON
-        [Target].[CollectionId] = [Source].[Id]
-        AND [Target].[CipherId] = @CipherId
-    WHEN NOT MATCHED BY TARGET
-    AND [Source].[Id] IN (SELECT [Id] FROM [AvailableCollectionsCTE]) THEN
-        INSERT VALUES
-        (
-            [Source].[Id],
-            @CipherId
-        )
-    WHEN NOT MATCHED BY SOURCE
-    AND [Target].[CipherId] = @CipherId
-    AND [Target].[CollectionId] IN (SELECT [Id] FROM [AvailableCollectionsCTE]) THEN
-        DELETE
-    ;
+    SELECT 
+        [Id],
+        @CipherId
+    FROM @CollectionIds
+    WHERE [Id] IN (SELECT [Id] FROM [#TempAvailableCollections])
+    AND NOT EXISTS (
+        SELECT 1 
+        FROM [dbo].[CollectionCipher]
+        WHERE [CollectionId] = [@CollectionIds].[Id]
+        AND [CipherId] = @CipherId
+    );
+
+    -- Delete removed collection assignments
+    DELETE CC
+    FROM [dbo].[CollectionCipher] CC
+    WHERE CC.[CipherId] = @CipherId
+    AND CC.[CollectionId] IN (SELECT [Id] FROM [#TempAvailableCollections])
+    AND CC.[CollectionId] NOT IN (SELECT [Id] FROM @CollectionIds);
 
     IF @OrgId IS NOT NULL
     BEGIN
         EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId
     END
+    DROP TABLE #TempAvailableCollections;
 END
diff --git 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"": [

diff --git a/util/Migrator/DbScripts/2025-02-04_00_CollectionPermissionEditExceptPWPerm.sql b/util/Migrator/DbScripts/2025-02-04_00_CollectionPermissionEditExceptPWPerm.sql
@@ -0,0 +1,118 @@
+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
+
+CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_UpdateCollections]
+    @CipherId UNIQUEIDENTIFIER,
+    @UserId UNIQUEIDENTIFIER,
+    @CollectionIds AS [dbo].[GuidIdArray] READONLY
+AS
+BEGIN
+    SET NOCOUNT ON
+
+    DECLARE @OrgId UNIQUEIDENTIFIER = (
+        SELECT TOP 1
+            [OrganizationId]
+        FROM
+            [dbo].[Cipher]
+        WHERE
+            [Id] = @CipherId
+    )
+    SELECT
+            C.[Id]
+            INTO #TempAvailableCollections
+        FROM
+            [dbo].[Collection] C
+        INNER JOIN
+            [Organization] O ON O.[Id] = C.[OrganizationId]
+        INNER JOIN
+            [dbo].[OrganizationUser] OU ON OU.[OrganizationId] = O.[Id] AND OU.[UserId] = @UserId
+        LEFT JOIN
+            [dbo].[CollectionUser] CU ON CU.[CollectionId] = C.[Id] AND CU.[OrganizationUserId] = OU.[Id]
+        LEFT JOIN
+            [dbo].[GroupUser] GU ON CU.[CollectionId] IS NULL AND GU.[OrganizationUserId] = OU.[Id]
+        LEFT JOIN
+            [dbo].[Group] G ON G.[Id] = GU.[GroupId]
+        LEFT JOIN
+            [dbo].[CollectionGroup] CG ON CG.[CollectionId] = C.[Id] AND CG.[GroupId] = GU.[GroupId]
+        WHERE
+            O.[Id] = @OrgId
+            AND O.[Enabled] = 1
+            AND OU.[Status] = 2 -- Confirmed
+            AND (
+                CU.[ReadOnly] = 0
+                OR CG.[ReadOnly] = 0
+            )
+    -- Insert new collection assignments
+    INSERT INTO [dbo].[CollectionCipher] (
+        [CollectionId],
+        [CipherId]
+    )
+    SELECT 
+        [Id],
+        @CipherId
+    FROM @CollectionIds
+    WHERE [Id] IN (SELECT [Id] FROM [#TempAvailableCollections])
+    AND NOT EXISTS (
+        SELECT 1 
+        FROM [dbo].[CollectionCipher]
+        WHERE [CollectionId] = [@CollectionIds].[Id]
+        AND [CipherId] = @CipherId
+    );
+
+    -- Delete removed collection assignments
+    DELETE CC
+    FROM [dbo].[CollectionCipher] CC
+    WHERE CC.[CipherId] = @CipherId
+    AND CC.[CollectionId] IN (SELECT [Id] FROM [#TempAvailableCollections])
+    AND CC.[CollectionId] NOT IN (SELECT [Id] FROM @CollectionIds);
+
+    IF @OrgId IS NOT NULL
+    BEGIN
+        EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId
+    END
+    DROP TABLE #TempAvailableCollections;
+END
+GO