From 53ed608ba1118d29d026db3706075bb7fe02d201 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 21 May 2024 14:40:05 +1000 Subject: [PATCH] [AC-2604] Fix aggregation of CollectionGroup permissions (#4097) * Fix aggregation of CollectionGroup permissions - use MAX on Manage column instead of MIN --- .../Repositories/CollectionRepository.cs | 4 +- .../Collection_ReadByIdUserId.sql | 2 +- .../Collection_ReadByIdUserId_V2.sql | 2 +- .../Collection_ReadByUserId.sql | 2 +- .../Collection_ReadByUserId_V2.sql | 2 +- .../2024-05-20_00_FixManageAggregation.sql | 124 ++++++++++++++++++ 6 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 util/Migrator/DbScripts/2024-05-20_00_FixManageAggregation.sql diff --git a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs index ee6f1e8794..8773a69a81 100644 --- a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs @@ -340,7 +340,7 @@ public class CollectionRepository : Repository Convert.ToInt32(c.ReadOnly))), HidePasswords = Convert.ToBoolean(collectionGroup.Min(c => Convert.ToInt32(c.HidePasswords))), - Manage = Convert.ToBoolean(collectionGroup.Min(c => Convert.ToInt32(c.Manage))), + Manage = Convert.ToBoolean(collectionGroup.Max(c => Convert.ToInt32(c.Manage))), }) .ToList(); } @@ -365,7 +365,7 @@ public class CollectionRepository : Repository Convert.ToInt32(c.ReadOnly))), HidePasswords = Convert.ToBoolean(collectionGroup.Min(c => Convert.ToInt32(c.HidePasswords))), - Manage = Convert.ToBoolean(collectionGroup.Min(c => Convert.ToInt32(c.Manage))), + Manage = Convert.ToBoolean(collectionGroup.Max(c => Convert.ToInt32(c.Manage))), }).ToListAsync(); } } diff --git a/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId.sql b/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId.sql index 85f2584359..b38709507e 100644 --- a/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId.sql +++ b/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId.sql @@ -13,7 +13,7 @@ BEGIN ExternalId, MIN([ReadOnly]) AS [ReadOnly], MIN([HidePasswords]) AS [HidePasswords], - MIN([Manage]) AS [Manage] + MAX([Manage]) AS [Manage] FROM [dbo].[UserCollectionDetails](@UserId) WHERE diff --git a/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId_V2.sql b/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId_V2.sql index 42b61671a6..e753fc89ac 100644 --- a/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId_V2.sql +++ b/src/Sql/dbo/Stored Procedures/Collection_ReadByIdUserId_V2.sql @@ -13,7 +13,7 @@ BEGIN ExternalId, MIN([ReadOnly]) AS [ReadOnly], MIN([HidePasswords]) AS [HidePasswords], - MIN([Manage]) AS [Manage] + MAX([Manage]) AS [Manage] FROM [dbo].[UserCollectionDetails_V2](@UserId) WHERE diff --git a/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId.sql b/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId.sql index b31c0c2b4e..f0eab509ac 100644 --- a/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId.sql +++ b/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId.sql @@ -13,7 +13,7 @@ BEGIN ExternalId, MIN([ReadOnly]) AS [ReadOnly], MIN([HidePasswords]) AS [HidePasswords], - MIN([Manage]) AS [Manage] + MAX([Manage]) AS [Manage] FROM [dbo].[UserCollectionDetails](@UserId) GROUP BY diff --git a/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId_V2.sql b/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId_V2.sql index adcca40a6d..4538dc8da0 100644 --- a/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId_V2.sql +++ b/src/Sql/dbo/Stored Procedures/Collection_ReadByUserId_V2.sql @@ -13,7 +13,7 @@ BEGIN ExternalId, MIN([ReadOnly]) AS [ReadOnly], MIN([HidePasswords]) AS [HidePasswords], - MIN([Manage]) AS [Manage] + MAX([Manage]) AS [Manage] FROM [dbo].[UserCollectionDetails_V2](@UserId) GROUP BY diff --git a/util/Migrator/DbScripts/2024-05-20_00_FixManageAggregation.sql b/util/Migrator/DbScripts/2024-05-20_00_FixManageAggregation.sql new file mode 100644 index 0000000000..d4bd662787 --- /dev/null +++ b/util/Migrator/DbScripts/2024-05-20_00_FixManageAggregation.sql @@ -0,0 +1,124 @@ +-- We were aggregating CollectionGroup permissions using MIN([Manage]) instead of MAX. +-- If the user is a member of multiple groups with overlapping collection permissions, they should get the most +-- generous permissions, not the least. This is consistent with ReadOnly and HidePasswords columns. +-- Updating both current and V2 sprocs out of caution and because they still need to be reviewed/cleaned up. + +-- Collection_ReadByIdUserId +CREATE OR ALTER PROCEDURE [dbo].[Collection_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + SELECT + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId, + MIN([ReadOnly]) AS [ReadOnly], + MIN([HidePasswords]) AS [HidePasswords], + MAX([Manage]) AS [Manage] + FROM + [dbo].[UserCollectionDetails](@UserId) + WHERE + [Id] = @Id + GROUP BY + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId +END +GO; + +-- Collection_ReadByIdUserId_V2 +CREATE OR ALTER PROCEDURE [dbo].[Collection_ReadByIdUserId_V2] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + SELECT + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId, + MIN([ReadOnly]) AS [ReadOnly], + MIN([HidePasswords]) AS [HidePasswords], + MAX([Manage]) AS [Manage] + FROM + [dbo].[UserCollectionDetails_V2](@UserId) + WHERE + [Id] = @Id + GROUP BY + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId +END +GO; + +-- Collection_ReadByUserId +CREATE OR ALTER PROCEDURE [dbo].[Collection_ReadByUserId] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId, + MIN([ReadOnly]) AS [ReadOnly], + MIN([HidePasswords]) AS [HidePasswords], + MAX([Manage]) AS [Manage] + FROM + [dbo].[UserCollectionDetails](@UserId) + GROUP BY + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId +END +GO; + +-- Collection_ReadByUserId_V2 +CREATE OR ALTER PROCEDURE [dbo].[Collection_ReadByUserId_V2] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId, + MIN([ReadOnly]) AS [ReadOnly], + MIN([HidePasswords]) AS [HidePasswords], + MAX([Manage]) AS [Manage] + FROM + [dbo].[UserCollectionDetails_V2](@UserId) + GROUP BY + Id, + OrganizationId, + [Name], + CreationDate, + RevisionDate, + ExternalId +END +GO;