From d94a54516eafe2d86ca6288d19b54adf7b7bf284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 2 Aug 2023 16:22:37 +0100 Subject: [PATCH] [AC-1344] Provider users unable to bulk restore vault items for client organizations (#2871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [AC-1344] Added method PutRestoreManyAdmin to CiphersController and refactored PutRestoreMany * [AC-1344] Fixed unit test * [AC-1344] Removed comment * [AC-1344] Fixed sql.csproj * [AC-1344] Added check for empty or null array; added more unit tests --- .../Vault/Controllers/CiphersController.cs | 30 ++++++-- .../Models/Request/CipherRequestModel.cs | 1 + .../Vault/Repositories/ICipherRepository.cs | 1 + src/Core/Vault/Services/ICipherService.cs | 2 +- .../Services/Implementations/CipherService.cs | 32 ++++++-- .../Vault/Repositories/CipherRepository.cs | 13 ++++ .../Vault/Repositories/CipherRepository.cs | 25 +++++++ .../Cipher_RestoreByIdsOrganizationId.sql | 29 ++++++++ .../Vault/Services/CipherServiceTests.cs | 74 ++++++++++++++++++- ...21_00_CipherRestoreByIdsOrganizationId.sql | 30 ++++++++ 10 files changed, 221 insertions(+), 16 deletions(-) create mode 100644 src/Sql/Vault/dbo/Stored Procedures/Cipher/Cipher_RestoreByIdsOrganizationId.sql create mode 100644 util/Migrator/DbScripts/2023-04-21_00_CipherRestoreByIdsOrganizationId.sql diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 4a416c6df..8bcd60389 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -451,7 +451,7 @@ public class CiphersController : Controller } [HttpPut("restore")] - public async Task> PutRestoreMany([FromBody] CipherBulkRestoreRequestModel model) + public async Task> PutRestoreMany([FromBody] CipherBulkRestoreRequestModel model) { if (!_globalSettings.SelfHosted && model.Ids.Count() > 500) { @@ -461,12 +461,30 @@ public class CiphersController : Controller var userId = _userService.GetProperUserId(User).Value; var cipherIdsToRestore = new HashSet(model.Ids.Select(i => new Guid(i))); - var ciphers = await _cipherRepository.GetManyByUserIdAsync(userId); - var restoringCiphers = ciphers.Where(c => cipherIdsToRestore.Contains(c.Id) && c.Edit); + var restoredCiphers = await _cipherService.RestoreManyAsync(cipherIdsToRestore, userId); + var responses = restoredCiphers.Select(c => new CipherMiniResponseModel(c, _globalSettings, c.OrganizationUseTotp)); + return new ListResponseModel(responses); + } - await _cipherService.RestoreManyAsync(restoringCiphers, userId); - var responses = restoringCiphers.Select(c => new CipherResponseModel(c, _globalSettings)); - return new ListResponseModel(responses); + [HttpPut("restore-admin")] + public async Task> PutRestoreManyAdmin([FromBody] CipherBulkRestoreRequestModel model) + { + if (!_globalSettings.SelfHosted && model.Ids.Count() > 500) + { + throw new BadRequestException("You can only restore up to 500 items at a time."); + } + + if (model == null || model.OrganizationId == default || !await _currentContext.EditAnyCollection(model.OrganizationId)) + { + throw new NotFoundException(); + } + + var userId = _userService.GetProperUserId(User).Value; + var cipherIdsToRestore = new HashSet(model.Ids.Select(i => new Guid(i))); + + var restoredCiphers = await _cipherService.RestoreManyAsync(cipherIdsToRestore, userId, model.OrganizationId, true); + var responses = restoredCiphers.Select(c => new CipherMiniResponseModel(c, _globalSettings, c.OrganizationUseTotp)); + return new ListResponseModel(responses); } [HttpPut("move")] diff --git a/src/Api/Vault/Models/Request/CipherRequestModel.cs b/src/Api/Vault/Models/Request/CipherRequestModel.cs index 842c99800..a7cc399fe 100644 --- a/src/Api/Vault/Models/Request/CipherRequestModel.cs +++ b/src/Api/Vault/Models/Request/CipherRequestModel.cs @@ -291,6 +291,7 @@ public class CipherBulkRestoreRequestModel { [Required] public IEnumerable Ids { get; set; } + public Guid OrganizationId { get; set; } } public class CipherBulkMoveRequestModel diff --git a/src/Core/Vault/Repositories/ICipherRepository.cs b/src/Core/Vault/Repositories/ICipherRepository.cs index 3df130aa8..401add13d 100644 --- a/src/Core/Vault/Repositories/ICipherRepository.cs +++ b/src/Core/Vault/Repositories/ICipherRepository.cs @@ -36,5 +36,6 @@ public interface ICipherRepository : IRepository Task SoftDeleteAsync(IEnumerable ids, Guid userId); Task SoftDeleteByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId); Task RestoreAsync(IEnumerable ids, Guid userId); + Task RestoreByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId); Task DeleteDeletedAsync(DateTime deletedDateBefore); } diff --git a/src/Core/Vault/Services/ICipherService.cs b/src/Core/Vault/Services/ICipherService.cs index 3168e50ea..83cd729e1 100644 --- a/src/Core/Vault/Services/ICipherService.cs +++ b/src/Core/Vault/Services/ICipherService.cs @@ -35,7 +35,7 @@ public interface ICipherService Task SoftDeleteAsync(Cipher cipher, Guid deletingUserId, bool orgAdmin = false); Task SoftDeleteManyAsync(IEnumerable cipherIds, Guid deletingUserId, Guid? organizationId = null, bool orgAdmin = false); Task RestoreAsync(Cipher cipher, Guid restoringUserId, bool orgAdmin = false); - Task RestoreManyAsync(IEnumerable ciphers, Guid restoringUserId); + Task> RestoreManyAsync(IEnumerable cipherIds, Guid restoringUserId, Guid? organizationId = null, bool orgAdmin = false); Task UploadFileForExistingAttachmentAsync(Stream stream, Cipher cipher, CipherAttachment.MetaData attachmentId); Task GetAttachmentDownloadDataAsync(Cipher cipher, string attachmentId); Task ValidateCipherAttachmentFile(Cipher cipher, CipherAttachment.MetaData attachmentData); diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index 5686497c6..82a6753a9 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -898,13 +898,33 @@ public class CipherService : ICipherService await _pushService.PushSyncCipherUpdateAsync(cipher, null); } - public async Task RestoreManyAsync(IEnumerable ciphers, Guid restoringUserId) + public async Task> RestoreManyAsync(IEnumerable cipherIds, Guid restoringUserId, Guid? organizationId = null, bool orgAdmin = false) { - var revisionDate = await _cipherRepository.RestoreAsync(ciphers.Select(c => c.Id), restoringUserId); - - var events = ciphers.Select(c => + if (cipherIds == null || !cipherIds.Any()) { - c.RevisionDate = revisionDate; + return new List(); + } + + var cipherIdsSet = new HashSet(cipherIds); + var restoringCiphers = new List(); + DateTime? revisionDate; + + if (orgAdmin && organizationId.HasValue) + { + var ciphers = await _cipherRepository.GetManyOrganizationDetailsByOrganizationIdAsync(organizationId.Value); + restoringCiphers = ciphers.Where(c => cipherIdsSet.Contains(c.Id)).ToList(); + revisionDate = await _cipherRepository.RestoreByIdsOrganizationIdAsync(restoringCiphers.Select(c => c.Id), organizationId.Value); + } + else + { + var ciphers = await _cipherRepository.GetManyByUserIdAsync(restoringUserId); + restoringCiphers = ciphers.Where(c => cipherIdsSet.Contains(c.Id) && c.Edit).Select(c => (CipherOrganizationDetails)c).ToList(); + revisionDate = await _cipherRepository.RestoreAsync(restoringCiphers.Select(c => c.Id), restoringUserId); + } + + var events = restoringCiphers.Select(c => + { + c.RevisionDate = revisionDate.Value; c.DeletedDate = null; return new Tuple(c, EventType.Cipher_Restored, null); }); @@ -915,6 +935,8 @@ public class CipherService : ICipherService // push await _pushService.PushSyncCiphersAsync(restoringUserId); + + return restoringCiphers; } public async Task<(IEnumerable, Dictionary>)> GetOrganizationCiphers(Guid userId, Guid organizationId) diff --git a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs index 31b8b10f5..cc1fd554e 100644 --- a/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs +++ b/src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs @@ -669,6 +669,19 @@ public class CipherRepository : Repository, ICipherRepository } } + public async Task RestoreByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.ExecuteScalarAsync( + $"[{Schema}].[Cipher_RestoreByIdsOrganizationId]", + new { Ids = ids.ToGuidIdArrayTVP(), OrganizationId = organizationId }, + commandType: CommandType.StoredProcedure); + + return results; + } + } + public async Task DeleteDeletedAsync(DateTime deletedDateBefore) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs b/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs index cf2b5085d..07d8576fe 100644 --- a/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs +++ b/src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs @@ -625,6 +625,31 @@ public class CipherRepository : Repository RestoreByIdsOrganizationIdAsync(IEnumerable ids, Guid organizationId) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var utcNow = DateTime.UtcNow; + var ciphers = from c in dbContext.Ciphers + where c.OrganizationId == organizationId && + ids.Contains(c.Id) + select c; + + await ciphers.ForEachAsync(cipher => + { + dbContext.Attach(cipher); + cipher.DeletedDate = null; + cipher.RevisionDate = utcNow; + }); + + await OrganizationUpdateStorage(organizationId); + await dbContext.UserBumpAccountRevisionDateByOrganizationIdAsync(organizationId); + await dbContext.SaveChangesAsync(); + return utcNow; + } + } + public async Task SoftDeleteAsync(IEnumerable ids, Guid userId) { await ToggleCipherStates(ids, userId, CipherStateAction.SoftDelete); diff --git a/src/Sql/Vault/dbo/Stored Procedures/Cipher/Cipher_RestoreByIdsOrganizationId.sql b/src/Sql/Vault/dbo/Stored Procedures/Cipher/Cipher_RestoreByIdsOrganizationId.sql new file mode 100644 index 000000000..1a0d5a8f5 --- /dev/null +++ b/src/Sql/Vault/dbo/Stored Procedures/Cipher/Cipher_RestoreByIdsOrganizationId.sql @@ -0,0 +1,29 @@ +CREATE PROCEDURE [dbo].[Cipher_RestoreByIdsOrganizationId] + @Ids AS [dbo].[GuidIdArray] READONLY, + @OrganizationId AS UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + IF (SELECT COUNT(1) FROM @Ids) < 1 + BEGIN + RETURN(-1) + END + + -- Delete ciphers + DECLARE @UtcNow DATETIME2(7) = GETUTCDATE(); + UPDATE + [dbo].[Cipher] + SET + [DeletedDate] = NULL, + [RevisionDate] = @UtcNow + WHERE + [Id] IN (SELECT * FROM @Ids) + AND OrganizationId = @OrganizationId + + -- Cleanup organization + EXEC [dbo].[Organization_UpdateStorage] @OrganizationId + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId + + SELECT @UtcNow +END \ No newline at end of file diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index c866fa282..ea3309fc6 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -1,4 +1,5 @@ using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; @@ -601,21 +602,23 @@ public class CipherServiceTests [Theory] [BitAutoData] - public async Task RestoreManyAsync_UpdatesCiphers(IEnumerable ciphers, + public async Task RestoreManyAsync_UpdatesCiphers(ICollection ciphers, SutProvider sutProvider) { + var cipherIds = ciphers.Select(c => c.Id).ToArray(); var restoringUserId = ciphers.First().UserId.Value; var previousRevisionDate = DateTime.UtcNow; foreach (var cipher in ciphers) { + cipher.Edit = true; cipher.RevisionDate = previousRevisionDate; } + sutProvider.GetDependency().GetManyByUserIdAsync(restoringUserId).Returns(ciphers); var revisionDate = previousRevisionDate + TimeSpan.FromMinutes(1); - sutProvider.GetDependency().RestoreAsync(Arg.Any>(), restoringUserId) - .Returns(revisionDate); + sutProvider.GetDependency().RestoreAsync(Arg.Any>(), restoringUserId).Returns(revisionDate); - await sutProvider.Sut.RestoreManyAsync(ciphers, restoringUserId); + await sutProvider.Sut.RestoreManyAsync(cipherIds, restoringUserId); foreach (var cipher in ciphers) { @@ -624,6 +627,58 @@ public class CipherServiceTests } } + [Theory] + [BitAutoData] + public async Task RestoreManyAsync_WithOrgAdmin_UpdatesCiphers(Guid organizationId, ICollection ciphers, + SutProvider sutProvider) + { + var cipherIds = ciphers.Select(c => c.Id).ToArray(); + var restoringUserId = ciphers.First().UserId.Value; + var previousRevisionDate = DateTime.UtcNow; + foreach (var cipher in ciphers) + { + cipher.RevisionDate = previousRevisionDate; + cipher.OrganizationId = organizationId; + } + + sutProvider.GetDependency().GetManyOrganizationDetailsByOrganizationIdAsync(organizationId).Returns(ciphers); + var revisionDate = previousRevisionDate + TimeSpan.FromMinutes(1); + sutProvider.GetDependency().RestoreByIdsOrganizationIdAsync(Arg.Is>(ids => ids.All(i => cipherIds.Contains(i))), organizationId).Returns(revisionDate); + + await sutProvider.Sut.RestoreManyAsync(cipherIds, restoringUserId, organizationId, true); + + foreach (var cipher in ciphers) + { + Assert.Null(cipher.DeletedDate); + Assert.Equal(revisionDate, cipher.RevisionDate); + } + + await sutProvider.GetDependency().Received(1).LogCipherEventsAsync(Arg.Is>>(events => events.All(e => cipherIds.Contains(e.Item1.Id)))); + await sutProvider.GetDependency().Received(1).PushSyncCiphersAsync(restoringUserId); + } + + [Theory] + [BitAutoData] + public async Task RestoreManyAsync_WithEmptyCipherIdsArray_DoesNothing(Guid restoringUserId, + SutProvider sutProvider) + { + var cipherIds = Array.Empty(); + + await sutProvider.Sut.RestoreManyAsync(cipherIds, restoringUserId); + + await AssertNoActionsAsync(sutProvider); + } + + [Theory] + [BitAutoData] + public async Task RestoreManyAsync_WithNullCipherIdsArray_DoesNothing(Guid restoringUserId, + SutProvider sutProvider) + { + await sutProvider.Sut.RestoreManyAsync(null, restoringUserId); + + await AssertNoActionsAsync(sutProvider); + } + [Theory, BitAutoData] public async Task ShareManyAsync_FreeOrgWithAttachment_Throws(SutProvider sutProvider, IEnumerable ciphers, Guid organizationId, List collectionIds) @@ -667,4 +722,15 @@ public class CipherServiceTests await sutProvider.GetDependency().Received(1).UpdateCiphersAsync(sharingUserId, Arg.Is>(arg => arg.Except(ciphers).IsNullOrEmpty())); } + + private async Task AssertNoActionsAsync(SutProvider sutProvider) + { + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyOrganizationDetailsByOrganizationIdAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreByIdsOrganizationIdAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreByIdsOrganizationIdAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByUserIdAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventsAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCiphersAsync(default); + } } diff --git a/util/Migrator/DbScripts/2023-04-21_00_CipherRestoreByIdsOrganizationId.sql b/util/Migrator/DbScripts/2023-04-21_00_CipherRestoreByIdsOrganizationId.sql new file mode 100644 index 000000000..e156984f6 --- /dev/null +++ b/util/Migrator/DbScripts/2023-04-21_00_CipherRestoreByIdsOrganizationId.sql @@ -0,0 +1,30 @@ +CREATE OR ALTER PROCEDURE [dbo].[Cipher_RestoreByIdsOrganizationId] + @Ids AS [dbo].[GuidIdArray] READONLY, + @OrganizationId AS UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + IF (SELECT COUNT(1) FROM @Ids) < 1 + BEGIN + RETURN(-1) + END + + -- Delete ciphers + DECLARE @UtcNow DATETIME2(7) = GETUTCDATE(); + UPDATE + [dbo].[Cipher] + SET + [DeletedDate] = NULL, + [RevisionDate] = @UtcNow + WHERE + [Id] IN (SELECT * FROM @Ids) + AND OrganizationId = @OrganizationId + + -- Cleanup organization + EXEC [dbo].[Organization_UpdateStorage] @OrganizationId + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId + + SELECT @UtcNow +END +GO \ No newline at end of file