[PM-11162] Assign to Collection Permission Update #4844
Conversation
|
Fixed Issues
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4844 +/- ##
==========================================
- Coverage 43.69% 43.68% -0.02%
==========================================
Files 1469 1472 +3
Lines 67898 68022 +124
Branches 6156 6170 +14
==========================================
+ Hits 29670 29713 +43
- Misses 36934 37012 +78
- Partials 1294 1297 +3 ☔ View full report in Codecov by Sentry. |
| [Reprompt], | ||
| [Key], | ||
| [OrganizationUseTotp] | ||
| , MAX ([Edit]) AS [Edit] |
There was a problem hiding this comment.
⛏️ Can we fix the formatting here so that the , comes at the end of each line (to match the rest)?
| [Reprompt], | ||
| [Key], | ||
| [OrganizationUseTotp] | ||
|
|
There was a problem hiding this comment.
⛏️ Extra whitespace we can remove
There was a problem hiding this comment.
🎨 We'll want to update the date in this migration script to todays date since we already have a few others on main with a more recent date.
| AND O.[Enabled] = 1 | ||
| AND OU.[Status] = 2 -- Confirmed | ||
| AND ( | ||
| CU.[ReadOnly] = 0 |
There was a problem hiding this comment.
💭 We should ask QA to test a few scenarios where a cipher has both group and user assignments with mixed ReadOnly and HidePasswords permissions to be sure the users is still allowed to perform the action with the most permissive relationship.
I think this query will still work as expected for those scenarios, but its probably something we should confirm with some additional testing.
There was a problem hiding this comment.
Merge can be problematic with excessive locking, better just update or delete:
`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
)
-- Common CTE for available collections
WITH [AvailableCollectionsCTE] AS(
SELECT
C.[Id]
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
) AND (
CU.[HidePasswords] = 0
OR CG.[HidePasswords] = 0
)
)
-- Insert new collection assignments
INSERT INTO [dbo].[CollectionCipher] (
[CollectionId],
[CipherId]
)
SELECT
[Id],
@CipherId
FROM @CollectionIds
WHERE [Id] IN (SELECT [Id] FROM [AvailableCollectionsCTE])
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 [AvailableCollectionsCTE])
AND CC.[CollectionId] NOT IN (SELECT [Id] FROM @CollectionIds);
IF @OrgId IS NOT NULL
BEGIN
EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId
END
END
GO`
Jingo88
requested review from
shane-melton and
rkac-bw
and removed request for
rkac-bw
|
After speaking with product and design we have decided to move away from hiding |
🎟️ Tracking
PM-11162
📔 Objective
Users with
Can Edit Except Passwordwill not be allowed to assign collections to ciphers.Updates made to
CipherDetails_ReadByIdUserId.sqlto get accurateViewPasswordvalue when user assigns/unassigns collections (This addresses bug PM-14046)NOTE: EntityFramework update being looked at later on.
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes