mirror of
https://github.com/bitwarden/server
synced 2025-12-10 05:13:48 +00:00
[PM-22558] Update IOrganizationUserRepository.ReplaceAsync to preserve existing access to collections of the type DefaultUserCollection (#6037)
* feat: exclude DefaultUserCollection from GetManyByOrganizationIdWithPermissionsAsync Updated EF implementation, SQL procedure, and unit test to verify that default user collections are filtered from results * Update the public CollectionsController.Get method to return a NotFoundResult for collections of type DefaultUserCollection. * Add unit tests for the public CollectionsController * Update ICollectionRepository.GetManyByOrganizationIdAsync to exclude results of the type DefaultUserCollection Modified the SQL stored procedure and the EF query to reflect this change and added a new integration test to ensure the functionality works as expected. * Refactor CollectionsController to remove unused IApplicationCacheService dependency * Update IOrganizationUserRepository.GetDetailsByIdWithCollectionsAsync to exclude DefaultUserCollections * Update IOrganizationUserRepository.GetManyDetailsByOrganizationAsync to exclude DefaultUserCollections * Undo change to GetByIdWithCollectionsAsync * Update integration test to verify exclusion of DefaultUserCollection in OrganizationUserRepository.GetDetailsByIdWithCollectionsAsync * Clarify documentation in ICollectionRepository to specify that GetManyByOrganizationIdWithAccessAsync returns only shared collections belonging to the organization. * Update IOrganizationUserRepository.ReplaceAsync to preserve existing access to collections of the type DefaultUserCollection
This commit is contained in:
@@ -516,9 +516,11 @@ public class OrganizationUserRepository : Repository<Core.Entities.OrganizationU
|
|||||||
{
|
{
|
||||||
var dbContext = GetDatabaseContext(scope);
|
var dbContext = GetDatabaseContext(scope);
|
||||||
|
|
||||||
var existingCollectionUsers = await dbContext.CollectionUsers
|
// Retrieve all collection assignments, excluding DefaultUserCollection
|
||||||
.Where(cu => cu.OrganizationUserId == obj.Id)
|
var existingCollectionUsers = await (from cu in dbContext.CollectionUsers
|
||||||
.ToListAsync();
|
join c in dbContext.Collections on cu.CollectionId equals c.Id
|
||||||
|
where cu.OrganizationUserId == obj.Id && c.Type != CollectionType.DefaultUserCollection
|
||||||
|
select cu).ToListAsync();
|
||||||
|
|
||||||
foreach (var requestedCollection in requestedCollections)
|
foreach (var requestedCollection in requestedCollections)
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -72,8 +72,11 @@ BEGIN
|
|||||||
CU
|
CU
|
||||||
FROM
|
FROM
|
||||||
[dbo].[CollectionUser] CU
|
[dbo].[CollectionUser] CU
|
||||||
|
INNER JOIN
|
||||||
|
[dbo].[Collection] C ON C.[Id] = CU.[CollectionId]
|
||||||
WHERE
|
WHERE
|
||||||
CU.[OrganizationUserId] = @Id
|
CU.[OrganizationUserId] = @Id
|
||||||
|
AND C.[Type] != 1 -- Don't delete default collections
|
||||||
AND NOT EXISTS (
|
AND NOT EXISTS (
|
||||||
SELECT
|
SELECT
|
||||||
1
|
1
|
||||||
|
|||||||
@@ -1166,4 +1166,67 @@ public class OrganizationUserRepositoryTests
|
|||||||
Assert.NotNull(responseModel);
|
Assert.NotNull(responseModel);
|
||||||
Assert.Empty(responseModel);
|
Assert.Empty(responseModel);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[DatabaseTheory, DatabaseData]
|
||||||
|
public async Task ReplaceAsync_PreservesDefaultCollections_WhenUpdatingCollectionAccess(
|
||||||
|
IUserRepository userRepository,
|
||||||
|
IOrganizationRepository organizationRepository,
|
||||||
|
IOrganizationUserRepository organizationUserRepository,
|
||||||
|
ICollectionRepository collectionRepository)
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var organization = await organizationRepository.CreateTestOrganizationAsync();
|
||||||
|
var user = await userRepository.CreateTestUserAsync();
|
||||||
|
var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user);
|
||||||
|
|
||||||
|
// Create a regular collection and a default collection
|
||||||
|
var regularCollection = await collectionRepository.CreateTestCollectionAsync(organization);
|
||||||
|
|
||||||
|
// Manually create default collection since CreateTestCollectionAsync doesn't support type parameter
|
||||||
|
var defaultCollection = new Collection
|
||||||
|
{
|
||||||
|
OrganizationId = organization.Id,
|
||||||
|
Name = $"Default Collection {Guid.NewGuid()}",
|
||||||
|
Type = CollectionType.DefaultUserCollection
|
||||||
|
};
|
||||||
|
await collectionRepository.CreateAsync(defaultCollection);
|
||||||
|
|
||||||
|
var newCollection = await collectionRepository.CreateTestCollectionAsync(organization);
|
||||||
|
|
||||||
|
// Set up initial collection access: user has access to both regular and default collections
|
||||||
|
await organizationUserRepository.ReplaceAsync(orgUser, [
|
||||||
|
new CollectionAccessSelection { Id = regularCollection.Id, ReadOnly = false, HidePasswords = false, Manage = false },
|
||||||
|
new CollectionAccessSelection { Id = defaultCollection.Id, ReadOnly = false, HidePasswords = false, Manage = true }
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Verify initial state
|
||||||
|
var (_, initialCollections) = await organizationUserRepository.GetByIdWithCollectionsAsync(orgUser.Id);
|
||||||
|
Assert.Equal(2, initialCollections.Count);
|
||||||
|
Assert.Contains(initialCollections, c => c.Id == regularCollection.Id);
|
||||||
|
Assert.Contains(initialCollections, c => c.Id == defaultCollection.Id);
|
||||||
|
|
||||||
|
// Act: Update collection access with only the new collection
|
||||||
|
// This should preserve the default collection but remove the regular collection
|
||||||
|
await organizationUserRepository.ReplaceAsync(orgUser, [
|
||||||
|
new CollectionAccessSelection { Id = newCollection.Id, ReadOnly = false, HidePasswords = false, Manage = true }
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
var (actualOrgUser, actualCollections) = await organizationUserRepository.GetByIdWithCollectionsAsync(orgUser.Id);
|
||||||
|
Assert.NotNull(actualOrgUser);
|
||||||
|
Assert.Equal(2, actualCollections.Count); // Should have default collection + new collection
|
||||||
|
|
||||||
|
// Default collection should be preserved
|
||||||
|
var preservedDefaultCollection = actualCollections.FirstOrDefault(c => c.Id == defaultCollection.Id);
|
||||||
|
Assert.NotNull(preservedDefaultCollection);
|
||||||
|
Assert.True(preservedDefaultCollection.Manage); // Original permissions preserved
|
||||||
|
|
||||||
|
// New collection should be added
|
||||||
|
var addedNewCollection = actualCollections.FirstOrDefault(c => c.Id == newCollection.Id);
|
||||||
|
Assert.NotNull(addedNewCollection);
|
||||||
|
Assert.True(addedNewCollection.Manage);
|
||||||
|
|
||||||
|
// Regular collection should be removed
|
||||||
|
Assert.DoesNotContain(actualCollections, c => c.Id == regularCollection.Id);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,89 @@
|
|||||||
|
CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_UpdateWithCollections]
|
||||||
|
@Id UNIQUEIDENTIFIER,
|
||||||
|
@OrganizationId UNIQUEIDENTIFIER,
|
||||||
|
@UserId UNIQUEIDENTIFIER,
|
||||||
|
@Email NVARCHAR(256),
|
||||||
|
@Key VARCHAR(MAX),
|
||||||
|
@Status SMALLINT,
|
||||||
|
@Type TINYINT,
|
||||||
|
@ExternalId NVARCHAR(300),
|
||||||
|
@CreationDate DATETIME2(7),
|
||||||
|
@RevisionDate DATETIME2(7),
|
||||||
|
@Permissions NVARCHAR(MAX),
|
||||||
|
@ResetPasswordKey VARCHAR(MAX),
|
||||||
|
@Collections AS [dbo].[CollectionAccessSelectionType] READONLY,
|
||||||
|
@AccessSecretsManager BIT = 0
|
||||||
|
AS
|
||||||
|
BEGIN
|
||||||
|
SET NOCOUNT ON
|
||||||
|
|
||||||
|
EXEC [dbo].[OrganizationUser_Update] @Id, @OrganizationId, @UserId, @Email, @Key, @Status, @Type, @ExternalId, @CreationDate, @RevisionDate, @Permissions, @ResetPasswordKey, @AccessSecretsManager
|
||||||
|
-- Update
|
||||||
|
UPDATE
|
||||||
|
[Target]
|
||||||
|
SET
|
||||||
|
[Target].[ReadOnly] = [Source].[ReadOnly],
|
||||||
|
[Target].[HidePasswords] = [Source].[HidePasswords],
|
||||||
|
[Target].[Manage] = [Source].[Manage]
|
||||||
|
FROM
|
||||||
|
[dbo].[CollectionUser] AS [Target]
|
||||||
|
INNER JOIN
|
||||||
|
@Collections AS [Source] ON [Source].[Id] = [Target].[CollectionId]
|
||||||
|
WHERE
|
||||||
|
[Target].[OrganizationUserId] = @Id
|
||||||
|
AND (
|
||||||
|
[Target].[ReadOnly] != [Source].[ReadOnly]
|
||||||
|
OR [Target].[HidePasswords] != [Source].[HidePasswords]
|
||||||
|
OR [Target].[Manage] != [Source].[Manage]
|
||||||
|
)
|
||||||
|
|
||||||
|
-- Insert
|
||||||
|
INSERT INTO [dbo].[CollectionUser]
|
||||||
|
(
|
||||||
|
[CollectionId],
|
||||||
|
[OrganizationUserId],
|
||||||
|
[ReadOnly],
|
||||||
|
[HidePasswords],
|
||||||
|
[Manage]
|
||||||
|
)
|
||||||
|
SELECT
|
||||||
|
[Source].[Id],
|
||||||
|
@Id,
|
||||||
|
[Source].[ReadOnly],
|
||||||
|
[Source].[HidePasswords],
|
||||||
|
[Source].[Manage]
|
||||||
|
FROM
|
||||||
|
@Collections AS [Source]
|
||||||
|
INNER JOIN
|
||||||
|
[dbo].[Collection] C ON C.[Id] = [Source].[Id] AND C.[OrganizationId] = @OrganizationId
|
||||||
|
WHERE
|
||||||
|
NOT EXISTS (
|
||||||
|
SELECT
|
||||||
|
1
|
||||||
|
FROM
|
||||||
|
[dbo].[CollectionUser]
|
||||||
|
WHERE
|
||||||
|
[CollectionId] = [Source].[Id]
|
||||||
|
AND [OrganizationUserId] = @Id
|
||||||
|
)
|
||||||
|
|
||||||
|
-- Delete
|
||||||
|
DELETE
|
||||||
|
CU
|
||||||
|
FROM
|
||||||
|
[dbo].[CollectionUser] CU
|
||||||
|
INNER JOIN
|
||||||
|
[dbo].[Collection] C ON C.[Id] = CU.[CollectionId]
|
||||||
|
WHERE
|
||||||
|
CU.[OrganizationUserId] = @Id
|
||||||
|
AND C.[Type] != 1 -- Don't delete default collections
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT
|
||||||
|
1
|
||||||
|
FROM
|
||||||
|
@Collections
|
||||||
|
WHERE
|
||||||
|
[Id] = CU.[CollectionId]
|
||||||
|
)
|
||||||
|
END
|
||||||
|
GO
|
||||||
Reference in New Issue
Block a user