From 2e14a46ceeddafc56e9ddc6c67c897325b5d6315 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Tue, 3 Jun 2025 14:02:13 -0700 Subject: [PATCH] [PM-22344] - fix Error: Cannot Decrypt when moving a vault item to a collection (#5911) * use ToCipher instead of casting * return ListResponseModel * fix test * remove ToArray * have ShareManyAsync return CipherDetails * fix test * fix tests * fix test * fix test --- .../Vault/Controllers/CiphersController.cs | 9 ++-- src/Core/Vault/Services/ICipherService.cs | 2 +- .../Services/Implementations/CipherService.cs | 2 +- .../Controllers/CiphersControllerTests.cs | 49 +++++++++++++------ .../Vault/Services/CipherServiceTests.cs | 8 +-- 5 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/Api/Vault/Controllers/CiphersController.cs b/src/Api/Vault/Controllers/CiphersController.cs index 92b611f588..7b302f3724 100644 --- a/src/Api/Vault/Controllers/CiphersController.cs +++ b/src/Api/Vault/Controllers/CiphersController.cs @@ -1064,7 +1064,7 @@ public class CiphersController : Controller [HttpPut("share")] [HttpPost("share")] - public async Task PutShareMany([FromBody] CipherBulkShareRequestModel model) + public async Task> PutShareMany([FromBody] CipherBulkShareRequestModel model) { var organizationId = new Guid(model.Ciphers.First().OrganizationId); if (!await _currentContext.OrganizationUser(organizationId)) @@ -1086,7 +1086,7 @@ public class CiphersController : Controller } } - var shareCiphers = new List<(Cipher, DateTime?)>(); + var shareCiphers = new List<(CipherDetails, DateTime?)>(); foreach (var cipher in model.Ciphers) { if (!ciphersDict.TryGetValue(cipher.Id.Value, out var existingCipher)) @@ -1096,7 +1096,7 @@ public class CiphersController : Controller ValidateClientVersionForFido2CredentialSupport(existingCipher); - shareCiphers.Add(((Cipher)existingCipher, cipher.LastKnownRevisionDate)); + shareCiphers.Add((cipher.ToCipherDetails(existingCipher), cipher.LastKnownRevisionDate)); } var updated = await _cipherService.ShareManyAsync( @@ -1106,7 +1106,8 @@ public class CiphersController : Controller userId ); - return updated.Select(c => new CipherMiniResponseModel(c, _globalSettings, false)).ToArray(); + var response = updated.Select(c => new CipherMiniResponseModel(c, _globalSettings, c.OrganizationUseTotp)); + return new ListResponseModel(response); } [HttpPost("purge")] diff --git a/src/Core/Vault/Services/ICipherService.cs b/src/Core/Vault/Services/ICipherService.cs index 3721f592ba..d3f8d20c90 100644 --- a/src/Core/Vault/Services/ICipherService.cs +++ b/src/Core/Vault/Services/ICipherService.cs @@ -24,7 +24,7 @@ public interface ICipherService Task DeleteFolderAsync(Folder folder); Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organizationId, IEnumerable collectionIds, Guid userId, DateTime? lastKnownRevisionDate); - Task> ShareManyAsync(IEnumerable<(Cipher cipher, DateTime? lastKnownRevisionDate)> ciphers, Guid organizationId, + Task> ShareManyAsync(IEnumerable<(CipherDetails cipher, DateTime? lastKnownRevisionDate)> ciphers, Guid organizationId, IEnumerable collectionIds, Guid sharingUserId); Task SaveCollectionsAsync(Cipher cipher, IEnumerable collectionIds, Guid savingUserId, bool orgAdmin); Task SoftDeleteAsync(CipherDetails cipherDetails, Guid deletingUserId, bool orgAdmin = false); diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index cf4e1fda86..413aee3e0d 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -625,7 +625,7 @@ public class CipherService : ICipherService await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds); } - public async Task> ShareManyAsync(IEnumerable<(Cipher cipher, DateTime? lastKnownRevisionDate)> cipherInfos, + public async Task> ShareManyAsync(IEnumerable<(CipherDetails cipher, DateTime? lastKnownRevisionDate)> cipherInfos, Guid organizationId, IEnumerable collectionIds, Guid sharingUserId) { var cipherIds = new List(); diff --git a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs index 14f795e571..bca6bbc048 100644 --- a/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs +++ b/test/Api.Test/Vault/Controllers/CiphersControllerTests.cs @@ -1,6 +1,7 @@ using System.Security.Claims; using System.Text.Json; using Bit.Api.Vault.Controllers; +using Bit.Api.Vault.Models; using Bit.Api.Vault.Models.Request; using Bit.Api.Vault.Models.Response; using Bit.Core; @@ -1774,16 +1775,16 @@ public class CiphersControllerTests Id = Guid.NewGuid(), UserId = userId, OrganizationId = organizationId, - Type = CipherType.SecureNote, - Data = JsonSerializer.Serialize(new CipherSecureNoteData()), + Type = CipherType.Login, + Data = JsonSerializer.Serialize(new CipherLoginData()), RevisionDate = oldDate2 }; var preloadedDetails = new List { detail1, detail2 }; var newDate1 = oldDate1.AddMinutes(5); var newDate2 = oldDate2.AddMinutes(5); - var updatedCipher1 = new Cipher { Id = detail1.Id, RevisionDate = newDate1, Type = detail1.Type, Data = detail1.Data }; - var updatedCipher2 = new Cipher { Id = detail2.Id, RevisionDate = newDate2, Type = detail2.Type, Data = detail2.Data }; + var updatedCipher1 = new CipherDetails { Id = detail1.Id, RevisionDate = newDate1, Type = detail1.Type, Data = detail1.Data }; + var updatedCipher2 = new CipherDetails { Id = detail2.Id, RevisionDate = newDate2, Type = detail2.Type, Data = detail2.Data }; sutProvider.GetDependency() .OrganizationUser(organizationId) @@ -1801,19 +1802,39 @@ public class CiphersControllerTests sutProvider.GetDependency() .ShareManyAsync( - Arg.Any>(), + Arg.Any>(), organizationId, Arg.Any>(), userId ) - .Returns(Task.FromResult>(new[] { updatedCipher1, updatedCipher2 })); + .Returns(Task.FromResult>(new[] { updatedCipher1, updatedCipher2 })); - var cipherRequests = preloadedDetails.Select(d => new CipherWithIdRequestModel + var cipherRequests = preloadedDetails.Select(d => { - Id = d.Id, - OrganizationId = d.OrganizationId!.Value.ToString(), - LastKnownRevisionDate = d.RevisionDate, - Type = d.Type + var m = new CipherWithIdRequestModel + { + Id = d.Id, + OrganizationId = d.OrganizationId!.Value.ToString(), + LastKnownRevisionDate = d.RevisionDate, + Type = d.Type, + }; + + if (d.Type == CipherType.Login) + { + m.Login = new CipherLoginModel + { + Username = "", + Password = "", + Uris = [], + }; + m.Name = ""; + m.Notes = ""; + m.Fields = Array.Empty(); + m.PasswordHistory = Array.Empty(); + } + + // similar for SecureNote, Card, etc., if you ever hit those branches + return m; }).ToList(); var model = new CipherBulkShareRequestModel @@ -1824,15 +1845,15 @@ public class CiphersControllerTests var result = await sutProvider.Sut.PutShareMany(model); - Assert.Equal(2, result.Length); - var revisionDates = result.Select(r => r.RevisionDate).ToList(); + Assert.Equal(2, result.Data.Count()); + var revisionDates = result.Data.Select(x => x.RevisionDate).ToList(); Assert.Contains(newDate1, revisionDates); Assert.Contains(newDate2, revisionDates); await sutProvider.GetDependency() .Received(1) .ShareManyAsync( - Arg.Is>(list => + Arg.Is>(list => list.Select(x => x.Item1.Id).OrderBy(id => id) .SequenceEqual(new[] { detail1.Id, detail2.Id }.OrderBy(id => id)) ), diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 061d90bcc3..95fd8179e3 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -72,7 +72,7 @@ public class CipherServiceTests [Theory, BitAutoData] public async Task ShareManyAsync_WrongRevisionDate_Throws(SutProvider sutProvider, - IEnumerable ciphers, Guid organizationId, List collectionIds) + IEnumerable ciphers, Guid organizationId, List collectionIds) { sutProvider.GetDependency().GetByIdAsync(organizationId) .Returns(new Organization @@ -651,7 +651,7 @@ public class CipherServiceTests [BitAutoData("")] [BitAutoData("Correct Time")] public async Task ShareManyAsync_CorrectRevisionDate_Passes(string revisionDateString, - SutProvider sutProvider, IEnumerable ciphers, Organization organization, List collectionIds) + SutProvider sutProvider, IEnumerable ciphers, Organization organization, List collectionIds) { sutProvider.GetDependency().GetByIdAsync(organization.Id) .Returns(new Organization @@ -1173,7 +1173,7 @@ public class CipherServiceTests [Theory, BitAutoData] public async Task ShareManyAsync_FreeOrgWithAttachment_Throws(SutProvider sutProvider, - IEnumerable ciphers, Guid organizationId, List collectionIds) + IEnumerable ciphers, Guid organizationId, List collectionIds) { sutProvider.GetDependency().GetByIdAsync(organizationId).Returns(new Organization { @@ -1194,7 +1194,7 @@ public class CipherServiceTests [Theory, BitAutoData] public async Task ShareManyAsync_PaidOrgWithAttachment_Passes(SutProvider sutProvider, - IEnumerable ciphers, Guid organizationId, List collectionIds) + IEnumerable ciphers, Guid organizationId, List collectionIds) { sutProvider.GetDependency().GetByIdAsync(organizationId) .Returns(new Organization