Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c147b5b
updated collections_v2 and bulk-collections endpoint for viewPasswordโ€ฆ
Jingo88 Oct 2, 2024
c705b40
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Oct 2, 2024
f541285
update ciphers mock in CiphersControllerTests
Jingo88 Oct 4, 2024
0df254a
formatted
Jingo88 Oct 4, 2024
441d2e7
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Oct 28, 2024
a9d1df6
updates to CipherDetails query for accurate ViewPassword value
Jingo88 Oct 31, 2024
2b55485
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Nov 6, 2024
786b054
format nit and migration date nit
Jingo88 Nov 6, 2024
5c672ec
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Nov 8, 2024
5f7fb0c
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
withinfocus Nov 11, 2024
f841ec5
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
kejaeger Nov 13, 2024
f7135fb
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Nov 14, 2024
710b0e2
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Nov 15, 2024
7f15dc3
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
kejaeger Nov 21, 2024
2215d45
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Nov 22, 2024
23d8790
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Dec 2, 2024
c5d772e
add HidePassword check to CollectionCipher_UpdateCollections
Jingo88 Dec 2, 2024
a86a45a
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Dec 3, 2024
e498acb
update cipher repository getManyByUserIdAsync to look at Edit and Vieโ€ฆ
Jingo88 Dec 3, 2024
5eb650f
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
kejaeger Dec 9, 2024
92cd2ba
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Dec 10, 2024
2e92275
update migration file
Jingo88 Dec 10, 2024
2cd32b6
update related sproc
Jingo88 Dec 10, 2024
7de47e4
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Dec 11, 2024
ced2997
update sproc and migration file for collectionCipher_updateCollection
Jingo88 Dec 11, 2024
a240df3
Merge branch 'main' into PM-11162-assign-to-collection-perm-update
Jingo88 Dec 23, 2024
c40ce6d
remove hidepassword logic from CollectionCipher_UpdateCollections
Jingo88 Dec 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions src/Api/Vault/Controllers/CiphersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,59 @@
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)
{

Check warning on line 431 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L431

Added line #L431 was not covered by tests
// If the user can edit all ciphers for the organization, just check they all belong to the org
if (await CanEditAllCiphersAsync(organizationId))
{

Check warning on line 434 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L434

Added line #L434 was not covered by tests
// 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);

Check warning on line 436 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L436

Added line #L436 was not covered by tests

// Ensure all requested ciphers are in orgCiphers
if (cipherIds.Any(c => !orgCiphers.ContainsKey(c)))
{
return false;

Check warning on line 441 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L439-L441

Added lines #L439 - L441 were not covered by tests
}

return true;

Check warning on line 444 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L444

Added line #L444 was not covered by tests
}

// The user cannot access any ciphers for the organization, we're done
if (!await CanAccessOrganizationCiphersAsync(organizationId))
{
return false;

Check warning on line 450 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L449-L450

Added lines #L449 - L450 were not covered by tests
}

var userId = _userService.GetProperUserId(User).Value;

Check warning on line 453 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L453

Added line #L453 was not covered by tests
// Select all editable ciphers for this user belonging to the organization
var editableOrgCipherList = (await _cipherRepository.GetManyByUserIdAsync(userId, true))

Check warning on line 455 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L455

Added line #L455 was not covered by tests
shane-melton marked this conversation as resolved.
Show resolved Hide resolved
.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));

Check warning on line 463 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L460-L463

Added lines #L460 - L463 were not covered by tests

// Users that can access unassigned ciphers can also edit them
editableOrgCipherList.AddRange(unassignedCiphers.Select(c => new CipherDetails(c) { Edit = true }));
}

Check warning on line 467 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L466-L467

Added lines #L466 - L467 were not covered by tests

var editableOrgCiphers = editableOrgCipherList
.ToDictionary(c => c.Id);

Check warning on line 470 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L469-L470

Added lines #L469 - L470 were not covered by tests

if (cipherIds.Any(c => !editableOrgCiphers.ContainsKey(c)))
{
return false;

Check warning on line 474 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L472-L474

Added lines #L472 - L474 were not covered by tests
}

return true;
}

Check warning on line 478 in src/Api/Vault/Controllers/CiphersController.cs

View check run for this annotation

Codecov / codecov/patch

src/Api/Vault/Controllers/CiphersController.cs#L477-L478

Added lines #L477 - L478 were not covered by tests

/// <summary>
/// TODO: Move this to its own authorization handler or equivalent service - AC-2062
/// </summary>
Expand Down Expand Up @@ -579,7 +632,7 @@
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();
}
Expand Down Expand Up @@ -634,7 +687,7 @@
[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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@

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())

Check warning on line 101 in src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs#L101

Added line #L101 was not covered by tests
.ToList();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from cg in cg_g.DefaultIfEmpty()
where o.Id == _organizationId && o.Enabled && ou.Status == OrganizationUserStatusType.Confirmed
&& (!cu.ReadOnly || !cg.ReadOnly)
&& (!cu.HidePasswords || !cg.HidePasswords)

Check warning on line 43 in src/Infrastructure.EntityFramework/Repositories/Queries/CollectionsReadByOrganizationIdUserIdQuery.cs

View check run for this annotation

Codecov / codecov/patch

src/Infrastructure.EntityFramework/Repositories/Queries/CollectionsReadByOrganizationIdUserIdQuery.cs#L43

Added line #L43 was not covered by tests
select c;

return query;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ BEGIN
WHERE
[Id] = @CipherId
)

;WITH [AvailableCollectionsCTE] AS(
SELECT
SELECT
C.[Id]
INTO #TempAvailableCollections
FROM
[dbo].[Collection] C
INNER JOIN
Expand All @@ -39,39 +38,37 @@ BEGIN
AND (
CU.[ReadOnly] = 0
OR CG.[ReadOnly] = 0
) AND (
CU.[HidePasswords] = 0
OR CG.[HidePasswords] = 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
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ private CipherDetails CreateCipherDetailsMock(Guid id, Guid userId)
UserId = userId,
OrganizationId = Guid.NewGuid(),
Type = CipherType.Login,
ViewPassword = true,
Data = @"
{
""Uris"": [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
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
) 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 [#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
Loading