Browse Source

[AC-1174] Refactor permission logic to target master

- Remove references to "Manage" flag on CollectionUser and CollectionGroups
- Revert to current collection permission checks in the auth handler
- Fix unit tests
- Remove references to "Manage" in SQL sprocs and EF queries
ac/ac-1174/master-bulk-collection-management
Shane Melton 2 years ago
parent
commit
91b9af62e2
No known key found for this signature in database
  1. 11
      src/Api/Vault/AuthorizationHandlers/CollectionAuthorizationHandler.cs
  2. 6
      src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs
  3. 26
      src/Sql/dbo/Stored Procedures/Collection_CreateOrUpdateAccessForMany.sql
  4. 28
      test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs
  5. 2
      test/Core.Test/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommandTests.cs
  6. 2
      test/Core.Test/Vault/AutoFixture/CollectionFixture.cs
  7. 26
      util/Migrator/DbScripts/2023-08-25_00_BulkAddCollectionAccess.sql

11
src/Api/Vault/AuthorizationHandlers/CollectionAuthorizationHandler.cs

@ -71,10 +71,17 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio @@ -71,10 +71,17 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
return;
}
// List of collection Ids the acting user is allowed to manage
// Acting user does not have permission to edit assigned collections, fail
if (!org.Permissions.EditAssignedCollections)
{
context.Fail();
return;
}
// List of assigned collection Ids for the acting user
var manageableCollectionIds =
(await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value))
.Where(c => c.Manage && c.OrganizationId == targetOrganizationId)
.Where(c => c.OrganizationId == targetOrganizationId)
.Select(c => c.Id)
.ToHashSet();

6
src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs

@ -491,7 +491,6 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect @@ -491,7 +491,6 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
OrganizationUserId = requestedUser.Id,
HidePasswords = requestedUser.HidePasswords,
ReadOnly = requestedUser.ReadOnly,
Manage = requestedUser.Manage
});
continue;
}
@ -499,7 +498,6 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect @@ -499,7 +498,6 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
// It already exists, update it
existingCollectionUser.HidePasswords = requestedUser.HidePasswords;
existingCollectionUser.ReadOnly = requestedUser.ReadOnly;
existingCollectionUser.Manage = requestedUser.Manage;
dbContext.CollectionUsers.Update(existingCollectionUser);
}
}
@ -528,8 +526,7 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect @@ -528,8 +526,7 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
CollectionId = collectionId,
GroupId = requestedGroup.Id,
HidePasswords = requestedGroup.HidePasswords,
ReadOnly = requestedGroup.ReadOnly,
Manage = requestedGroup.Manage
ReadOnly = requestedGroup.ReadOnly
});
continue;
}
@ -537,7 +534,6 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect @@ -537,7 +534,6 @@ public class CollectionRepository : Repository<Core.Entities.Collection, Collect
// It already exists, update it
existingCollectionGroup.HidePasswords = requestedGroup.HidePasswords;
existingCollectionGroup.ReadOnly = requestedGroup.ReadOnly;
existingCollectionGroup.Manage = requestedGroup.Manage;
dbContext.CollectionGroups.Update(existingCollectionGroup);
}
}

26
src/Sql/dbo/Stored Procedures/Collection_CreateOrUpdateAccessForMany.sql

@ -13,8 +13,7 @@ BEGIN @@ -13,8 +13,7 @@ BEGIN
cId.[Id] AS [CollectionId],
gu.[Id] AS [GroupId],
gu.[ReadOnly],
gu.[HidePasswords],
gu.[Manage]
gu.[HidePasswords]
FROM
@Groups AS gu
CROSS JOIN
@ -32,21 +31,19 @@ BEGIN @@ -32,21 +31,19 @@ BEGIN
[Target].[CollectionId] = [Source].[CollectionId]
AND [Target].[GroupId] = [Source].[GroupId]
WHEN MATCHED AND EXISTS(
SELECT [Source].[ReadOnly], [Source].[HidePasswords], [Source].[Manage]
SELECT [Source].[ReadOnly], [Source].[HidePasswords]
EXCEPT
SELECT [Target].[ReadOnly], [Target].[HidePasswords], [Target].[Manage]
SELECT [Target].[ReadOnly], [Target].[HidePasswords]
) THEN UPDATE SET
[Target].[ReadOnly] = [Source].[ReadOnly],
[Target].[HidePasswords] = [Source].[HidePasswords],
[Target].[Manage] = [Source].[Manage]
[Target].[HidePasswords] = [Source].[HidePasswords]
WHEN NOT MATCHED BY TARGET
THEN INSERT VALUES
(
[Source].[CollectionId],
[Source].[GroupId],
[Source].[ReadOnly],
[Source].[HidePasswords],
[Source].[Manage]
[Source].[HidePasswords]
);
-- Users
@ -55,8 +52,7 @@ BEGIN @@ -55,8 +52,7 @@ BEGIN
cId.[Id] AS [CollectionId],
cu.[Id] AS [OrganizationUserId],
cu.[ReadOnly],
cu.[HidePasswords],
cu.[Manage]
cu.[HidePasswords]
FROM
@Users AS cu
CROSS JOIN
@ -74,21 +70,19 @@ BEGIN @@ -74,21 +70,19 @@ BEGIN
[Target].[CollectionId] = [Source].[CollectionId]
AND [Target].[OrganizationUserId] = [Source].[OrganizationUserId]
WHEN MATCHED AND EXISTS(
SELECT [Source].[ReadOnly], [Source].[HidePasswords], [Source].[Manage]
SELECT [Source].[ReadOnly], [Source].[HidePasswords]
EXCEPT
SELECT [Target].[ReadOnly], [Target].[HidePasswords], [Target].[Manage]
SELECT [Target].[ReadOnly], [Target].[HidePasswords]
) THEN UPDATE SET
[Target].[ReadOnly] = [Source].[ReadOnly],
[Target].[HidePasswords] = [Source].[HidePasswords],
[Target].[Manage] = [Source].[Manage]
[Target].[HidePasswords] = [Source].[HidePasswords]
WHEN NOT MATCHED BY TARGET
THEN INSERT VALUES
(
[Source].[CollectionId],
[Source].[OrganizationUserId],
[Source].[ReadOnly],
[Source].[HidePasswords],
[Source].[Manage]
[Source].[HidePasswords]
);
EXEC [dbo].[User_BumpAccountRevisionDateByCollectionIds] @CollectionIds, @OrganizationId

28
test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs

@ -25,16 +25,19 @@ public class CollectionAuthorizationHandlerTests @@ -25,16 +25,19 @@ public class CollectionAuthorizationHandlerTests
[BitAutoData(OrganizationUserType.Custom, true, false)]
[BitAutoData(OrganizationUserType.Owner, true, true)]
public async Task CanManageCollectionAccessAsync_Success(
OrganizationUserType userType, bool editAnyCollection, bool manageCollections,
OrganizationUserType userType, bool editAnyCollection, bool manageAssignedCollections,
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
ICollection<CollectionDetails> collectionDetails,
CurrentContentOrganization organization)
IList<CollectionDetails> collectionDetails,
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
foreach (var collectionDetail in collectionDetails)
if (!manageAssignedCollections)
{
collectionDetail.Manage = manageCollections;
// Simulate the user not being assigned to a collection
collectionDetails.RemoveAt(0);
organization.Permissions.EditAssignedCollections = false;
}
organization.Type = userType;
@ -102,7 +105,7 @@ public class CollectionAuthorizationHandlerTests @@ -102,7 +105,7 @@ public class CollectionAuthorizationHandlerTests
public async Task CanManageCollectionAccessAsync_MissingOrgMembership_Failure(
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
CurrentContentOrganization organization)
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
@ -128,17 +131,14 @@ public class CollectionAuthorizationHandlerTests @@ -128,17 +131,14 @@ public class CollectionAuthorizationHandlerTests
public async Task CanManageCollectionAccessAsync_MissingManageCollectionPermission_Failure(
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
ICollection<CollectionDetails> collectionDetails,
CurrentContentOrganization organization)
IList<CollectionDetails> collectionDetails,
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
foreach (var collectionDetail in collectionDetails)
{
collectionDetail.Manage = true;
}
// Simulate one collection missing the manage permission
collectionDetails.First().Manage = false;
// Simulate the user not being assigned to a collection
collectionDetails.RemoveAt(0);
organization.Permissions.EditAssignedCollections = true;
// Ensure the user is not an owner/admin and does not have edit any collection permission
organization.Type = OrganizationUserType.User;

2
test/Core.Test/OrganizationFeatures/OrganizationCollections/BulkAddCollectionAccessCommandTests.cs

@ -228,7 +228,6 @@ public class BulkAddCollectionAccessCommandTests @@ -228,7 +228,6 @@ public class BulkAddCollectionAccessCommandTests
return collectionUsers.Select(cu => new CollectionAccessSelection
{
Id = cu.OrganizationUserId,
Manage = cu.Manage,
HidePasswords = cu.HidePasswords,
ReadOnly = cu.ReadOnly
}).ToList();
@ -238,7 +237,6 @@ public class BulkAddCollectionAccessCommandTests @@ -238,7 +237,6 @@ public class BulkAddCollectionAccessCommandTests
return collectionGroups.Select(cg => new CollectionAccessSelection
{
Id = cg.GroupId,
Manage = cg.Manage,
HidePasswords = cg.HidePasswords,
ReadOnly = cg.ReadOnly
}).ToList();

2
test/Core.Test/Vault/AutoFixture/CollectionFixture.cs

@ -20,7 +20,7 @@ public class CollectionCustomization : ICustomization @@ -20,7 +20,7 @@ public class CollectionCustomization : ICustomization
fixture.Customize<Organization>(composer => composer
.With(o => o.Id, orgId));
fixture.Customize<CurrentContentOrganization>(composer => composer
fixture.Customize<CurrentContextOrganization>(composer => composer
.With(o => o.Id, orgId));
fixture.Customize<OrganizationUser>(composer => composer

26
util/Migrator/DbScripts/2023-08-25_00_BulkAddCollectionAccess.sql

@ -50,8 +50,7 @@ BEGIN @@ -50,8 +50,7 @@ BEGIN
cId.[Id] AS [CollectionId],
gu.[Id] AS [GroupId],
gu.[ReadOnly],
gu.[HidePasswords],
gu.[Manage]
gu.[HidePasswords]
FROM
@Groups AS gu
CROSS JOIN
@ -69,21 +68,19 @@ BEGIN @@ -69,21 +68,19 @@ BEGIN
[Target].[CollectionId] = [Source].[CollectionId]
AND [Target].[GroupId] = [Source].[GroupId]
WHEN MATCHED AND EXISTS(
SELECT [Source].[ReadOnly], [Source].[HidePasswords], [Source].[Manage]
SELECT [Source].[ReadOnly], [Source].[HidePasswords]
EXCEPT
SELECT [Target].[ReadOnly], [Target].[HidePasswords], [Target].[Manage]
SELECT [Target].[ReadOnly], [Target].[HidePasswords]
) THEN UPDATE SET
[Target].[ReadOnly] = [Source].[ReadOnly],
[Target].[HidePasswords] = [Source].[HidePasswords],
[Target].[Manage] = [Source].[Manage]
[Target].[HidePasswords] = [Source].[HidePasswords]
WHEN NOT MATCHED BY TARGET
THEN INSERT VALUES
(
[Source].[CollectionId],
[Source].[GroupId],
[Source].[ReadOnly],
[Source].[HidePasswords],
[Source].[Manage]
[Source].[HidePasswords]
);
-- Users
@ -92,8 +89,7 @@ BEGIN @@ -92,8 +89,7 @@ BEGIN
cId.[Id] AS [CollectionId],
cu.[Id] AS [OrganizationUserId],
cu.[ReadOnly],
cu.[HidePasswords],
cu.[Manage]
cu.[HidePasswords]
FROM
@Users AS cu
CROSS JOIN
@ -111,21 +107,19 @@ BEGIN @@ -111,21 +107,19 @@ BEGIN
[Target].[CollectionId] = [Source].[CollectionId]
AND [Target].[OrganizationUserId] = [Source].[OrganizationUserId]
WHEN MATCHED AND EXISTS(
SELECT [Source].[ReadOnly], [Source].[HidePasswords], [Source].[Manage]
SELECT [Source].[ReadOnly], [Source].[HidePasswords]
EXCEPT
SELECT [Target].[ReadOnly], [Target].[HidePasswords], [Target].[Manage]
SELECT [Target].[ReadOnly], [Target].[HidePasswords]
) THEN UPDATE SET
[Target].[ReadOnly] = [Source].[ReadOnly],
[Target].[HidePasswords] = [Source].[HidePasswords],
[Target].[Manage] = [Source].[Manage]
[Target].[HidePasswords] = [Source].[HidePasswords]
WHEN NOT MATCHED BY TARGET
THEN INSERT VALUES
(
[Source].[CollectionId],
[Source].[OrganizationUserId],
[Source].[ReadOnly],
[Source].[HidePasswords],
[Source].[Manage]
[Source].[HidePasswords]
);
EXEC [dbo].[User_BumpAccountRevisionDateByCollectionIds] @CollectionIds, @OrganizationId

Loading…
Cancel
Save