From dbf8907bfccf4ed42e3c2f77bc53a8beb28d3865 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 6 Dec 2023 11:10:39 +1000 Subject: [PATCH] [AC-1330] [AC-1816] Deprecate AccessAll in CollectionCipher sprocs (#3480) --- .../Vault/Controllers/CiphersController.cs | 8 +- src/Api/Vault/Controllers/SyncController.cs | 2 +- .../ICollectionCipherRepository.cs | 8 +- .../Services/Implementations/CipherService.cs | 8 +- .../CollectionCipherRepository.cs | 32 ++- .../CollectionCipherRepository.cs | 46 +++- ...llectionCipherReadByUserIdCipherIdQuery.cs | 2 +- .../CollectionCipherReadByUserIdQuery.cs | 47 +++- ...lectionsReadByOrganizationIdUserIdQuery.cs | 44 ++++ ...llectionCipher_ReadByUserIdCipherId_V2.sql | 31 +++ .../CollectionCipher_ReadByUserId_V2.sql | 29 +++ ...nCipher_UpdateCollectionsForCiphers_V2.sql | 62 +++++ .../CollectionCipher_UpdateCollections_V2.sql | 77 +++++++ .../Vault/Controllers/SyncControllerTests.cs | 8 +- .../Vault/Services/CipherServiceTests.cs | 8 +- ...00_DeprecateAccessAll_CollectionCipher.sql | 212 ++++++++++++++++++ 16 files changed, 582 insertions(+), 42 deletions(-) create mode 100644 src/Infrastructure.EntityFramework/Repositories/Queries/CollectionsReadByOrganizationIdUserIdQuery.cs create mode 100644 src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserIdCipherId_V2.sql create mode 100644 src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserId_V2.sql create mode 100644 src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollectionsForCiphers_V2.sql create mode 100644 src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections_V2.sql create mode 100644 util/Migrator/DbScripts/2023-12-01_00_DeprecateAccessAll_CollectionCipher.sql diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 7ce970988..a99d2b01c 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -106,7 +106,7 @@ public class CiphersController : Controller throw new NotFoundException(); } - var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, id); + var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, id, UseFlexibleCollections); return new CipherDetailsResponseModel(cipher, _globalSettings, collectionCiphers); } @@ -120,7 +120,7 @@ public class CiphersController : Controller Dictionary> collectionCiphersGroupDict = null; if (hasOrgs) { - var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdAsync(userId); + var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdAsync(userId, UseFlexibleCollections); collectionCiphersGroupDict = collectionCiphers.GroupBy(c => c.CipherId).ToDictionary(s => s.Key); } @@ -189,7 +189,7 @@ public class CiphersController : Controller ValidateClientVersionForItemLevelEncryptionSupport(cipher); ValidateClientVersionForFido2CredentialSupport(cipher); - var collectionIds = (await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, id)).Select(c => c.CollectionId).ToList(); + var collectionIds = (await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, id, UseFlexibleCollections)).Select(c => c.CollectionId).ToList(); var modelOrgId = string.IsNullOrWhiteSpace(model.OrganizationId) ? (Guid?)null : new Guid(model.OrganizationId); if (cipher.OrganizationId != modelOrgId) @@ -220,7 +220,7 @@ public class CiphersController : Controller throw new NotFoundException(); } - var collectionIds = (await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, id)).Select(c => c.CollectionId).ToList(); + var collectionIds = (await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, id, UseFlexibleCollections)).Select(c => c.CollectionId).ToList(); // object cannot be a descendant of CipherDetails, so let's clone it. var cipherClone = model.ToCipher(cipher).Clone(); await _cipherService.SaveAsync(cipherClone, userId, model.LastKnownRevisionDate, collectionIds, true, false); diff --git a/src/Api/Vault/Controllers/SyncController.cs b/src/Api/Vault/Controllers/SyncController.cs index 9d8cc5d57..7bf26e714 100644 --- a/src/Api/Vault/Controllers/SyncController.cs +++ b/src/Api/Vault/Controllers/SyncController.cs @@ -95,7 +95,7 @@ public class SyncController : Controller if (hasEnabledOrgs) { collections = await _collectionRepository.GetManyByUserIdAsync(user.Id); - var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdAsync(user.Id); + var collectionCiphers = await _collectionCipherRepository.GetManyByUserIdAsync(user.Id, UseFlexibleCollections); collectionCiphersGroupDict = collectionCiphers.GroupBy(c => c.CipherId).ToDictionary(s => s.Key); } diff --git a/src/Core/Repositories/ICollectionCipherRepository.cs b/src/Core/Repositories/ICollectionCipherRepository.cs index 272128810..5bf00c614 100644 --- a/src/Core/Repositories/ICollectionCipherRepository.cs +++ b/src/Core/Repositories/ICollectionCipherRepository.cs @@ -4,11 +4,11 @@ namespace Bit.Core.Repositories; public interface ICollectionCipherRepository { - Task> GetManyByUserIdAsync(Guid userId); + Task> GetManyByUserIdAsync(Guid userId, bool useFlexibleCollections); Task> GetManyByOrganizationIdAsync(Guid organizationId); - Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId); - Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds); + Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId, bool useFlexibleCollections); + Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds, bool useFlexibleCollections); Task UpdateCollectionsForAdminAsync(Guid cipherId, Guid organizationId, IEnumerable collectionIds); Task UpdateCollectionsForCiphersAsync(IEnumerable cipherIds, Guid userId, Guid organizationId, - IEnumerable collectionIds); + IEnumerable collectionIds, bool useFlexibleCollections); } diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index cea17856f..5517be168 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -585,11 +585,11 @@ public class CipherService : ICipherService originalCipher.SetAttachments(originalAttachments); } - var currentCollectionsForCipher = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(sharingUserId, originalCipher.Id); + var currentCollectionsForCipher = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(sharingUserId, originalCipher.Id, UseFlexibleCollections); var currentCollectionIdsForCipher = currentCollectionsForCipher.Select(c => c.CollectionId).ToList(); currentCollectionIdsForCipher.RemoveAll(id => collectionIds.Contains(id)); - await _collectionCipherRepository.UpdateCollectionsAsync(originalCipher.Id, sharingUserId, currentCollectionIdsForCipher); + await _collectionCipherRepository.UpdateCollectionsAsync(originalCipher.Id, sharingUserId, currentCollectionIdsForCipher, UseFlexibleCollections); await _cipherRepository.ReplaceAsync(originalCipher); } @@ -634,7 +634,7 @@ public class CipherService : ICipherService await _cipherRepository.UpdateCiphersAsync(sharingUserId, cipherInfos.Select(c => c.cipher)); await _collectionCipherRepository.UpdateCollectionsForCiphersAsync(cipherIds, sharingUserId, - organizationId, collectionIds); + organizationId, collectionIds, UseFlexibleCollections); var events = cipherInfos.Select(c => new Tuple(c.cipher, EventType.Cipher_Shared, null)); @@ -675,7 +675,7 @@ public class CipherService : ICipherService { throw new BadRequestException("You do not have permissions to edit this."); } - await _collectionCipherRepository.UpdateCollectionsAsync(cipher.Id, savingUserId, collectionIds); + await _collectionCipherRepository.UpdateCollectionsAsync(cipher.Id, savingUserId, collectionIds, UseFlexibleCollections); } await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_UpdatedCollections); diff --git a/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs b/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs index 96ee52bed..7d80ee129 100644 --- a/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs @@ -17,12 +17,16 @@ public class CollectionCipherRepository : BaseRepository, ICollectionCipherRepos : base(connectionString, readOnlyConnectionString) { } - public async Task> GetManyByUserIdAsync(Guid userId) + public async Task> GetManyByUserIdAsync(Guid userId, bool useFlexibleCollections) { + var sprocName = useFlexibleCollections + ? "[dbo].[CollectionCipher_ReadByUserId_V2]" + : "[dbo].[CollectionCipher_ReadByUserId]"; + using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.QueryAsync( - "[dbo].[CollectionCipher_ReadByUserId]", + sprocName, new { UserId = userId }, commandType: CommandType.StoredProcedure); @@ -43,12 +47,16 @@ public class CollectionCipherRepository : BaseRepository, ICollectionCipherRepos } } - public async Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId) + public async Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId, bool useFlexibleCollections) { + var sprocName = useFlexibleCollections + ? "[dbo].[CollectionCipher_ReadByUserIdCipherId_V2]" + : "[dbo].[CollectionCipher_ReadByUserIdCipherId]"; + using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.QueryAsync( - "[dbo].[CollectionCipher_ReadByUserIdCipherId]", + sprocName, new { UserId = userId, CipherId = cipherId }, commandType: CommandType.StoredProcedure); @@ -56,12 +64,16 @@ public class CollectionCipherRepository : BaseRepository, ICollectionCipherRepos } } - public async Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds) + public async Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds, bool useFlexibleCollections) { + var sprocName = useFlexibleCollections + ? "[dbo].[CollectionCipher_UpdateCollections_V2]" + : "[dbo].[CollectionCipher_UpdateCollections]"; + using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.ExecuteAsync( - "[dbo].[CollectionCipher_UpdateCollections]", + sprocName, new { CipherId = cipherId, UserId = userId, CollectionIds = collectionIds.ToGuidIdArrayTVP() }, commandType: CommandType.StoredProcedure); } @@ -79,12 +91,16 @@ public class CollectionCipherRepository : BaseRepository, ICollectionCipherRepos } public async Task UpdateCollectionsForCiphersAsync(IEnumerable cipherIds, Guid userId, - Guid organizationId, IEnumerable collectionIds) + Guid organizationId, IEnumerable collectionIds, bool useFlexibleCollections) { + var sprocName = useFlexibleCollections + ? "[dbo].[CollectionCipher_UpdateCollectionsForCiphers_V2]" + : "[dbo].[CollectionCipher_UpdateCollectionsForCiphers]"; + using (var connection = new SqlConnection(ConnectionString)) { var results = await connection.ExecuteAsync( - "[dbo].[CollectionCipher_UpdateCollectionsForCiphers]", + sprocName, new { CipherIds = cipherIds.ToGuidIdArrayTVP(), diff --git a/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs b/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs index f929b8d92..7ef9b1967 100644 --- a/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs @@ -46,31 +46,31 @@ public class CollectionCipherRepository : BaseEntityFrameworkRepository, ICollec } } - public async Task> GetManyByUserIdAsync(Guid userId) + public async Task> GetManyByUserIdAsync(Guid userId, bool useFlexibleCollections) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); - var data = await new CollectionCipherReadByUserIdQuery(userId) + var data = await new CollectionCipherReadByUserIdQuery(userId, useFlexibleCollections) .Run(dbContext) .ToArrayAsync(); return data; } } - public async Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId) + public async Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId, bool useFlexibleCollections) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); - var data = await new CollectionCipherReadByUserIdCipherIdQuery(userId, cipherId) + var data = await new CollectionCipherReadByUserIdCipherIdQuery(userId, cipherId, useFlexibleCollections) .Run(dbContext) .ToArrayAsync(); return data; } } - public async Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds) + public async Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds, bool useFlexibleCollections) { using (var scope = ServiceScopeFactory.CreateScope()) { @@ -81,7 +81,17 @@ public class CollectionCipherRepository : BaseEntityFrameworkRepository, ICollec .Select(c => c.OrganizationId) .FirstAsync(); - var availableCollections = await (from c in dbContext.Collections + List availableCollections; + if (useFlexibleCollections) + { + var availableCollectionsQuery = new CollectionsReadByOrganizationIdUserIdQuery(organizationId, userId); + availableCollections = await availableCollectionsQuery + .Run(dbContext) + .Select(c => c.Id).ToListAsync(); + } + else + { + availableCollections = await (from c in dbContext.Collections join o in dbContext.Organizations on c.OrganizationId equals o.Id join ou in dbContext.OrganizationUsers on new { OrganizationId = o.Id, UserId = (Guid?)userId } equals @@ -104,6 +114,8 @@ public class CollectionCipherRepository : BaseEntityFrameworkRepository, ICollec && (ou.AccessAll || !cu.ReadOnly || g.AccessAll || !cg.ReadOnly) select c.Id).ToListAsync(); + } + var collectionCiphers = await (from cc in dbContext.CollectionCiphers where cc.CipherId == cipherId select cc).ToListAsync(); @@ -176,12 +188,22 @@ public class CollectionCipherRepository : BaseEntityFrameworkRepository, ICollec } } - public async Task UpdateCollectionsForCiphersAsync(IEnumerable cipherIds, Guid userId, Guid organizationId, IEnumerable collectionIds) + public async Task UpdateCollectionsForCiphersAsync(IEnumerable cipherIds, Guid userId, Guid organizationId, IEnumerable collectionIds, bool useFlexibleCollections) { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); - var availableCollections = from c in dbContext.Collections + + IQueryable availableCollections; + if (useFlexibleCollections) + { + var availableCollectionsQuery = new CollectionsReadByOrganizationIdUserIdQuery(organizationId, userId); + availableCollections = availableCollectionsQuery + .Run(dbContext); + } + else + { + availableCollections = from c in dbContext.Collections join o in dbContext.Organizations on c.OrganizationId equals o.Id join ou in dbContext.OrganizationUsers @@ -204,8 +226,10 @@ public class CollectionCipherRepository : BaseEntityFrameworkRepository, ICollec where !g.AccessAll && cg.CollectionId == c.Id && (o.Id == organizationId && o.Enabled && ou.Status == OrganizationUserStatusType.Confirmed && (ou.AccessAll || !cu.ReadOnly || g.AccessAll || !cg.ReadOnly)) - select new { c, o, ou, cu, gu, g, cg }; - var count = await availableCollections.CountAsync(); + select c; + + } + if (await availableCollections.CountAsync() < 1) { return; @@ -213,7 +237,7 @@ public class CollectionCipherRepository : BaseEntityFrameworkRepository, ICollec var insertData = from collectionId in collectionIds from cipherId in cipherIds - where availableCollections.Select(x => x.c.Id).Contains(collectionId) + where availableCollections.Select(c => c.Id).Contains(collectionId) select new Models.CollectionCipher { CollectionId = collectionId, diff --git a/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdCipherIdQuery.cs b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdCipherIdQuery.cs index e494aec1f..3ba559436 100644 --- a/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdCipherIdQuery.cs +++ b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdCipherIdQuery.cs @@ -6,7 +6,7 @@ public class CollectionCipherReadByUserIdCipherIdQuery : CollectionCipherReadByU { private readonly Guid _cipherId; - public CollectionCipherReadByUserIdCipherIdQuery(Guid userId, Guid cipherId) : base(userId) + public CollectionCipherReadByUserIdCipherIdQuery(Guid userId, Guid cipherId, bool useFlexibleCollections) : base(userId, useFlexibleCollections) { _cipherId = cipherId; } diff --git a/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdQuery.cs b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdQuery.cs index 1347a706f..3adfe7ffa 100644 --- a/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdQuery.cs +++ b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionCipherReadByUserIdQuery.cs @@ -6,13 +6,58 @@ namespace Bit.Infrastructure.EntityFramework.Repositories.Queries; public class CollectionCipherReadByUserIdQuery : IQuery { private readonly Guid _userId; + private readonly bool _useFlexibleCollections; - public CollectionCipherReadByUserIdQuery(Guid userId) + public CollectionCipherReadByUserIdQuery(Guid userId, bool useFlexibleCollections) { _userId = userId; + _useFlexibleCollections = useFlexibleCollections; } public virtual IQueryable Run(DatabaseContext dbContext) + { + return _useFlexibleCollections + ? Run_VNext(dbContext) + : Run_VCurrent(dbContext); + } + + private IQueryable Run_VNext(DatabaseContext dbContext) + { + var query = from cc in dbContext.CollectionCiphers + + join c in dbContext.Collections + on cc.CollectionId equals c.Id + + join ou in dbContext.OrganizationUsers + on new { c.OrganizationId, UserId = (Guid?)_userId } equals + new { ou.OrganizationId, ou.UserId } + + join cu in dbContext.CollectionUsers + on new { CollectionId = c.Id, OrganizationUserId = ou.Id } equals + new { cu.CollectionId, cu.OrganizationUserId } into cu_g + from cu in cu_g.DefaultIfEmpty() + + join gu in dbContext.GroupUsers + on new { CollectionId = (Guid?)cu.CollectionId, OrganizationUserId = ou.Id } equals + new { CollectionId = (Guid?)null, gu.OrganizationUserId } into gu_g + from gu in gu_g.DefaultIfEmpty() + + join g in dbContext.Groups + on gu.GroupId equals g.Id into g_g + from g in g_g.DefaultIfEmpty() + + join cg in dbContext.CollectionGroups + on new { CollectionId = c.Id, gu.GroupId } equals + new { cg.CollectionId, cg.GroupId } into cg_g + from cg in cg_g.DefaultIfEmpty() + + where ou.Status == OrganizationUserStatusType.Confirmed && + (cu.CollectionId != null || cg.CollectionId != null) + select cc; + return query; + } + + private IQueryable Run_VCurrent(DatabaseContext dbContext) { var query = from cc in dbContext.CollectionCiphers diff --git a/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionsReadByOrganizationIdUserIdQuery.cs b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionsReadByOrganizationIdUserIdQuery.cs new file mode 100644 index 000000000..d6d94e6e3 --- /dev/null +++ b/src/Infrastructure.EntityFramework/Repositories/Queries/CollectionsReadByOrganizationIdUserIdQuery.cs @@ -0,0 +1,44 @@ +using Bit.Core.Enums; +using Bit.Infrastructure.EntityFramework.Models; + +namespace Bit.Infrastructure.EntityFramework.Repositories.Queries; + +public class CollectionsReadByOrganizationIdUserIdQuery : IQuery +{ + private readonly Guid? _organizationId; + private readonly Guid _userId; + + public CollectionsReadByOrganizationIdUserIdQuery(Guid? organizationId, Guid userId) + { + _organizationId = organizationId; + _userId = userId; + } + + public virtual IQueryable Run(DatabaseContext dbContext) + { + var query = from c in dbContext.Collections + join o in dbContext.Organizations on c.OrganizationId equals o.Id + join ou in dbContext.OrganizationUsers + on new { OrganizationId = o.Id, UserId = (Guid?)_userId } equals + new { ou.OrganizationId, ou.UserId } + join cu in dbContext.CollectionUsers + on new { CollectionId = c.Id, OrganizationUserId = ou.Id } equals + new { cu.CollectionId, cu.OrganizationUserId } into cu_g + from cu in cu_g.DefaultIfEmpty() + join gu in dbContext.GroupUsers + on new { CollectionId = (Guid?)cu.CollectionId, OrganizationUserId = ou.Id } equals + new { CollectionId = (Guid?)null, gu.OrganizationUserId } into gu_g + from gu in gu_g.DefaultIfEmpty() + join g in dbContext.Groups on gu.GroupId equals g.Id into g_g + from g in g_g.DefaultIfEmpty() + join cg in dbContext.CollectionGroups + on new { CollectionId = c.Id, gu.GroupId } equals + new { cg.CollectionId, cg.GroupId } into cg_g + from cg in cg_g.DefaultIfEmpty() + where o.Id == _organizationId && o.Enabled && ou.Status == OrganizationUserStatusType.Confirmed + && (!cu.ReadOnly || !cg.ReadOnly) + select c; + + return query; + } +} diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserIdCipherId_V2.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserIdCipherId_V2.sql new file mode 100644 index 000000000..de8cb6e8f --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserIdCipherId_V2.sql @@ -0,0 +1,31 @@ +CREATE PROCEDURE [dbo].[CollectionCipher_ReadByUserIdCipherId_V2] + @UserId UNIQUEIDENTIFIER, + @CipherId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + CC.* + FROM + [dbo].[CollectionCipher] CC + INNER JOIN + [dbo].[Collection] S ON S.[Id] = CC.[CollectionId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[OrganizationId] = S.[OrganizationId] AND OU.[UserId] = @UserId + LEFT JOIN + [dbo].[CollectionUser] CU ON CU.[CollectionId] = S.[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] = CC.[CollectionId] AND CG.[GroupId] = GU.[GroupId] + WHERE + CC.[CipherId] = @CipherId + AND OU.[Status] = 2 -- Confirmed + AND ( + CU.[CollectionId] IS NOT NULL + OR CG.[CollectionId] IS NOT NULL + ) +END diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserId_V2.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserId_V2.sql new file mode 100644 index 000000000..6a0b444c3 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadByUserId_V2.sql @@ -0,0 +1,29 @@ +CREATE PROCEDURE [dbo].[CollectionCipher_ReadByUserId_V2] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + CC.* + FROM + [dbo].[CollectionCipher] CC + INNER JOIN + [dbo].[Collection] S ON S.[Id] = CC.[CollectionId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[OrganizationId] = S.[OrganizationId] AND OU.[UserId] = @UserId + LEFT JOIN + [dbo].[CollectionUser] CU ON CU.[CollectionId] = S.[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] = CC.[CollectionId] AND CG.[GroupId] = GU.[GroupId] + WHERE + OU.[Status] = 2 -- Confirmed + AND ( + CU.[CollectionId] IS NOT NULL + OR CG.[CollectionId] IS NOT NULL + ) +END diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollectionsForCiphers_V2.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollectionsForCiphers_V2.sql new file mode 100644 index 000000000..ab06d65e4 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollectionsForCiphers_V2.sql @@ -0,0 +1,62 @@ +CREATE PROCEDURE [dbo].[CollectionCipher_UpdateCollectionsForCiphers_V2] + @CipherIds AS [dbo].[GuidIdArray] READONLY, + @OrganizationId UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @CollectionIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + CREATE TABLE #AvailableCollections ( + [Id] UNIQUEIDENTIFIER + ) + + INSERT INTO #AvailableCollections + 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] = @OrganizationId + AND O.[Enabled] = 1 + AND OU.[Status] = 2 -- Confirmed + AND ( + CU.[ReadOnly] = 0 + OR CG.[ReadOnly] = 0 + ) + + IF (SELECT COUNT(1) FROM #AvailableCollections) < 1 + BEGIN + -- No writable collections available to share with in this organization. + RETURN + END + + INSERT INTO [dbo].[CollectionCipher] + ( + [CollectionId], + [CipherId] + ) + SELECT + [Collection].[Id], + [Cipher].[Id] + FROM + @CollectionIds [Collection] + INNER JOIN + @CipherIds [Cipher] ON 1 = 1 + WHERE + [Collection].[Id] IN (SELECT [Id] FROM #AvailableCollections) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId +END diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections_V2.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections_V2.sql new file mode 100644 index 000000000..c540c1737 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_UpdateCollections_V2.sql @@ -0,0 +1,77 @@ +CREATE PROCEDURE [dbo].[CollectionCipher_UpdateCollections_V2] + @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 + ) + + ;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 + ) + ), + [CollectionCiphersCTE] AS( + SELECT + [CollectionId], + [CipherId] + FROM + [dbo].[CollectionCipher] + WHERE + [CipherId] = @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 + ; + + IF @OrgId IS NOT NULL + BEGIN + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId + END +END diff --git a/test/Api.Test/Vault/Controllers/SyncControllerTests.cs b/test/Api.Test/Vault/Controllers/SyncControllerTests.cs index c46dcd73a..0b525a1d3 100644 --- a/test/Api.Test/Vault/Controllers/SyncControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/SyncControllerTests.cs @@ -116,7 +116,7 @@ public class SyncControllerTests // Returns for methods only called if we have enabled orgs collectionRepository.GetManyByUserIdAsync(user.Id).Returns(collections); - collectionCipherRepository.GetManyByUserIdAsync(user.Id).Returns(new List()); + collectionCipherRepository.GetManyByUserIdAsync(user.Id, Arg.Any()).Returns(new List()); // Back to standard test setup userService.TwoFactorIsEnabledAsync(user).Returns(false); @@ -281,7 +281,7 @@ public class SyncControllerTests // Returns for methods only called if we have enabled orgs collectionRepository.GetManyByUserIdAsync(user.Id).Returns(collections); - collectionCipherRepository.GetManyByUserIdAsync(user.Id).Returns(new List()); + collectionCipherRepository.GetManyByUserIdAsync(user.Id, Arg.Any()).Returns(new List()); // Back to standard test setup userService.TwoFactorIsEnabledAsync(user).Returns(false); @@ -346,7 +346,7 @@ public class SyncControllerTests await collectionRepository.ReceivedWithAnyArgs(1) .GetManyByUserIdAsync(default); await collectionCipherRepository.ReceivedWithAnyArgs(1) - .GetManyByUserIdAsync(default); + .GetManyByUserIdAsync(default, default); } else { @@ -354,7 +354,7 @@ public class SyncControllerTests await collectionRepository.ReceivedWithAnyArgs(0) .GetManyByUserIdAsync(default); await collectionCipherRepository.ReceivedWithAnyArgs(0) - .GetManyByUserIdAsync(default); + .GetManyByUserIdAsync(default, default); } await userService.ReceivedWithAnyArgs(1) diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 15b03d198..fc64f8021 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -432,7 +432,7 @@ public class CipherServiceTests sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); var attachmentStorageService = sutProvider.GetDependency(); var collectionCipherRepository = sutProvider.GetDependency(); - collectionCipherRepository.GetManyByUserIdCipherIdAsync(cipher.UserId.Value, cipher.Id).Returns( + collectionCipherRepository.GetManyByUserIdCipherIdAsync(cipher.UserId.Value, cipher.Id, Arg.Any()).Returns( Task.FromResult((ICollection)new List { new CollectionCipher @@ -512,7 +512,7 @@ public class CipherServiceTests Assert.Contains("ex from StartShareAttachmentAsync", exception.Message); await collectionCipherRepository.Received().UpdateCollectionsAsync(cipher.Id, cipher.UserId.Value, - Arg.Is>(ids => ids.Count == 1 && ids[0] != collectionIds[0])); + Arg.Is>(ids => ids.Count == 1 && ids[0] != collectionIds[0]), Arg.Any()); await cipherRepository.Received().ReplaceAsync(Arg.Is(c => c.GetAttachments()[v0AttachmentId].Key == null @@ -537,7 +537,7 @@ public class CipherServiceTests var attachmentStorageService = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); var collectionCipherRepository = sutProvider.GetDependency(); - collectionCipherRepository.GetManyByUserIdCipherIdAsync(cipher.UserId.Value, cipher.Id).Returns( + collectionCipherRepository.GetManyByUserIdCipherIdAsync(cipher.UserId.Value, cipher.Id, Arg.Any()).Returns( Task.FromResult((ICollection)new List { new CollectionCipher @@ -644,7 +644,7 @@ public class CipherServiceTests Assert.Contains("ex from StartShareAttachmentAsync", exception.Message); await collectionCipherRepository.Received().UpdateCollectionsAsync(cipher.Id, cipher.UserId.Value, - Arg.Is>(ids => ids.Count == 1 && ids[0] != collectionIds[0])); + Arg.Is>(ids => ids.Count == 1 && ids[0] != collectionIds[0]), Arg.Any()); await cipherRepository.Received().ReplaceAsync(Arg.Is(c => c.GetAttachments()[v0AttachmentId1].Key == null diff --git a/util/Migrator/DbScripts/2023-12-01_00_DeprecateAccessAll_CollectionCipher.sql b/util/Migrator/DbScripts/2023-12-01_00_DeprecateAccessAll_CollectionCipher.sql new file mode 100644 index 000000000..1783f2e3a --- /dev/null +++ b/util/Migrator/DbScripts/2023-12-01_00_DeprecateAccessAll_CollectionCipher.sql @@ -0,0 +1,212 @@ +-- Flexible Collections: create new CollectionCipher sprocs that don't use AccessAll logic + +-- CollectionCipher_ReadByUserId_V2 +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_ReadByUserId_V2] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + CC.* + FROM + [dbo].[CollectionCipher] CC + INNER JOIN + [dbo].[Collection] S ON S.[Id] = CC.[CollectionId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[OrganizationId] = S.[OrganizationId] AND OU.[UserId] = @UserId + LEFT JOIN + [dbo].[CollectionUser] CU ON CU.[CollectionId] = S.[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] = CC.[CollectionId] AND CG.[GroupId] = GU.[GroupId] + WHERE + OU.[Status] = 2 -- Confirmed + AND ( + CU.[CollectionId] IS NOT NULL + OR CG.[CollectionId] IS NOT NULL + ) +END +GO + +-- CollectionCipher_ReadByUserIdCipherId_V2 +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_ReadByUserIdCipherId_V2] + @UserId UNIQUEIDENTIFIER, + @CipherId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + CC.* + FROM + [dbo].[CollectionCipher] CC + INNER JOIN + [dbo].[Collection] S ON S.[Id] = CC.[CollectionId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[OrganizationId] = S.[OrganizationId] AND OU.[UserId] = @UserId + LEFT JOIN + [dbo].[CollectionUser] CU ON CU.[CollectionId] = S.[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] = CC.[CollectionId] AND CG.[GroupId] = GU.[GroupId] + WHERE + CC.[CipherId] = @CipherId + AND OU.[Status] = 2 -- Confirmed + AND ( + CU.[CollectionId] IS NOT NULL + OR CG.[CollectionId] IS NOT NULL + ) +END +GO + +-- CollectionCipher_UpdateCollections_V2 +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_UpdateCollections_V2] + @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 + ) + + ;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 + ) + ), + [CollectionCiphersCTE] AS( + SELECT + [CollectionId], + [CipherId] + FROM + [dbo].[CollectionCipher] + WHERE + [CipherId] = @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 + ; + + IF @OrgId IS NOT NULL + BEGIN + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrgId + END +END +GO + +-- CollectionCipher_UpdateCollectionsForCiphers_V2 +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_UpdateCollectionsForCiphers_V2] + @CipherIds AS [dbo].[GuidIdArray] READONLY, + @OrganizationId UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @CollectionIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + CREATE TABLE #AvailableCollections ( + [Id] UNIQUEIDENTIFIER + ) + + INSERT INTO #AvailableCollections + 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] = @OrganizationId + AND O.[Enabled] = 1 + AND OU.[Status] = 2 -- Confirmed + AND ( + CU.[ReadOnly] = 0 + OR CG.[ReadOnly] = 0 + ) + + IF (SELECT COUNT(1) FROM #AvailableCollections) < 1 + BEGIN + -- No writable collections available to share with in this organization. + RETURN + END + + INSERT INTO [dbo].[CollectionCipher] + ( + [CollectionId], + [CipherId] + ) + SELECT + [Collection].[Id], + [Cipher].[Id] + FROM + @CollectionIds [Collection] + INNER JOIN + @CipherIds [Cipher] ON 1 = 1 + WHERE + [Collection].[Id] IN (SELECT [Id] FROM #AvailableCollections) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId +END +GO