1
0
mirror of https://github.com/bitwarden/server.git synced 2025-06-07 11:40:31 -05:00

[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
This commit is contained in:
Jordan Aasen 2025-06-03 14:02:13 -07:00 committed by GitHub
parent 812fe94c16
commit 2e14a46cee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 46 additions and 24 deletions

View File

@ -1064,7 +1064,7 @@ public class CiphersController : Controller
[HttpPut("share")] [HttpPut("share")]
[HttpPost("share")] [HttpPost("share")]
public async Task<CipherMiniResponseModel[]> PutShareMany([FromBody] CipherBulkShareRequestModel model) public async Task<ListResponseModel<CipherMiniResponseModel>> PutShareMany([FromBody] CipherBulkShareRequestModel model)
{ {
var organizationId = new Guid(model.Ciphers.First().OrganizationId); var organizationId = new Guid(model.Ciphers.First().OrganizationId);
if (!await _currentContext.OrganizationUser(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) foreach (var cipher in model.Ciphers)
{ {
if (!ciphersDict.TryGetValue(cipher.Id.Value, out var existingCipher)) if (!ciphersDict.TryGetValue(cipher.Id.Value, out var existingCipher))
@ -1096,7 +1096,7 @@ public class CiphersController : Controller
ValidateClientVersionForFido2CredentialSupport(existingCipher); ValidateClientVersionForFido2CredentialSupport(existingCipher);
shareCiphers.Add(((Cipher)existingCipher, cipher.LastKnownRevisionDate)); shareCiphers.Add((cipher.ToCipherDetails(existingCipher), cipher.LastKnownRevisionDate));
} }
var updated = await _cipherService.ShareManyAsync( var updated = await _cipherService.ShareManyAsync(
@ -1106,7 +1106,8 @@ public class CiphersController : Controller
userId 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<CipherMiniResponseModel>(response);
} }
[HttpPost("purge")] [HttpPost("purge")]

View File

@ -24,7 +24,7 @@ public interface ICipherService
Task DeleteFolderAsync(Folder folder); Task DeleteFolderAsync(Folder folder);
Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organizationId, IEnumerable<Guid> collectionIds, Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organizationId, IEnumerable<Guid> collectionIds,
Guid userId, DateTime? lastKnownRevisionDate); Guid userId, DateTime? lastKnownRevisionDate);
Task<IEnumerable<Cipher>> ShareManyAsync(IEnumerable<(Cipher cipher, DateTime? lastKnownRevisionDate)> ciphers, Guid organizationId, Task<IEnumerable<CipherDetails>> ShareManyAsync(IEnumerable<(CipherDetails cipher, DateTime? lastKnownRevisionDate)> ciphers, Guid organizationId,
IEnumerable<Guid> collectionIds, Guid sharingUserId); IEnumerable<Guid> collectionIds, Guid sharingUserId);
Task SaveCollectionsAsync(Cipher cipher, IEnumerable<Guid> collectionIds, Guid savingUserId, bool orgAdmin); Task SaveCollectionsAsync(Cipher cipher, IEnumerable<Guid> collectionIds, Guid savingUserId, bool orgAdmin);
Task SoftDeleteAsync(CipherDetails cipherDetails, Guid deletingUserId, bool orgAdmin = false); Task SoftDeleteAsync(CipherDetails cipherDetails, Guid deletingUserId, bool orgAdmin = false);

View File

@ -625,7 +625,7 @@ public class CipherService : ICipherService
await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds); await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds);
} }
public async Task<IEnumerable<Cipher>> ShareManyAsync(IEnumerable<(Cipher cipher, DateTime? lastKnownRevisionDate)> cipherInfos, public async Task<IEnumerable<CipherDetails>> ShareManyAsync(IEnumerable<(CipherDetails cipher, DateTime? lastKnownRevisionDate)> cipherInfos,
Guid organizationId, IEnumerable<Guid> collectionIds, Guid sharingUserId) Guid organizationId, IEnumerable<Guid> collectionIds, Guid sharingUserId)
{ {
var cipherIds = new List<Guid>(); var cipherIds = new List<Guid>();

View File

@ -1,6 +1,7 @@
using System.Security.Claims; using System.Security.Claims;
using System.Text.Json; using System.Text.Json;
using Bit.Api.Vault.Controllers; using Bit.Api.Vault.Controllers;
using Bit.Api.Vault.Models;
using Bit.Api.Vault.Models.Request; using Bit.Api.Vault.Models.Request;
using Bit.Api.Vault.Models.Response; using Bit.Api.Vault.Models.Response;
using Bit.Core; using Bit.Core;
@ -1774,16 +1775,16 @@ public class CiphersControllerTests
Id = Guid.NewGuid(), Id = Guid.NewGuid(),
UserId = userId, UserId = userId,
OrganizationId = organizationId, OrganizationId = organizationId,
Type = CipherType.SecureNote, Type = CipherType.Login,
Data = JsonSerializer.Serialize(new CipherSecureNoteData()), Data = JsonSerializer.Serialize(new CipherLoginData()),
RevisionDate = oldDate2 RevisionDate = oldDate2
}; };
var preloadedDetails = new List<CipherDetails> { detail1, detail2 }; var preloadedDetails = new List<CipherDetails> { detail1, detail2 };
var newDate1 = oldDate1.AddMinutes(5); var newDate1 = oldDate1.AddMinutes(5);
var newDate2 = oldDate2.AddMinutes(5); var newDate2 = oldDate2.AddMinutes(5);
var updatedCipher1 = new Cipher { Id = detail1.Id, RevisionDate = newDate1, Type = detail1.Type, Data = detail1.Data }; var updatedCipher1 = new CipherDetails { 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 updatedCipher2 = new CipherDetails { Id = detail2.Id, RevisionDate = newDate2, Type = detail2.Type, Data = detail2.Data };
sutProvider.GetDependency<ICurrentContext>() sutProvider.GetDependency<ICurrentContext>()
.OrganizationUser(organizationId) .OrganizationUser(organizationId)
@ -1801,19 +1802,39 @@ public class CiphersControllerTests
sutProvider.GetDependency<ICipherService>() sutProvider.GetDependency<ICipherService>()
.ShareManyAsync( .ShareManyAsync(
Arg.Any<IEnumerable<(Cipher, DateTime?)>>(), Arg.Any<IEnumerable<(CipherDetails, DateTime?)>>(),
organizationId, organizationId,
Arg.Any<IEnumerable<Guid>>(), Arg.Any<IEnumerable<Guid>>(),
userId userId
) )
.Returns(Task.FromResult<IEnumerable<Cipher>>(new[] { updatedCipher1, updatedCipher2 })); .Returns(Task.FromResult<IEnumerable<CipherDetails>>(new[] { updatedCipher1, updatedCipher2 }));
var cipherRequests = preloadedDetails.Select(d => new CipherWithIdRequestModel var cipherRequests = preloadedDetails.Select(d =>
{ {
Id = d.Id, var m = new CipherWithIdRequestModel
OrganizationId = d.OrganizationId!.Value.ToString(), {
LastKnownRevisionDate = d.RevisionDate, Id = d.Id,
Type = d.Type 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<CipherFieldModel>();
m.PasswordHistory = Array.Empty<CipherPasswordHistoryModel>();
}
// similar for SecureNote, Card, etc., if you ever hit those branches
return m;
}).ToList(); }).ToList();
var model = new CipherBulkShareRequestModel var model = new CipherBulkShareRequestModel
@ -1824,15 +1845,15 @@ public class CiphersControllerTests
var result = await sutProvider.Sut.PutShareMany(model); var result = await sutProvider.Sut.PutShareMany(model);
Assert.Equal(2, result.Length); Assert.Equal(2, result.Data.Count());
var revisionDates = result.Select(r => r.RevisionDate).ToList(); var revisionDates = result.Data.Select(x => x.RevisionDate).ToList();
Assert.Contains(newDate1, revisionDates); Assert.Contains(newDate1, revisionDates);
Assert.Contains(newDate2, revisionDates); Assert.Contains(newDate2, revisionDates);
await sutProvider.GetDependency<ICipherService>() await sutProvider.GetDependency<ICipherService>()
.Received(1) .Received(1)
.ShareManyAsync( .ShareManyAsync(
Arg.Is<IEnumerable<(Cipher, DateTime?)>>(list => Arg.Is<IEnumerable<(CipherDetails, DateTime?)>>(list =>
list.Select(x => x.Item1.Id).OrderBy(id => id) list.Select(x => x.Item1.Id).OrderBy(id => id)
.SequenceEqual(new[] { detail1.Id, detail2.Id }.OrderBy(id => id)) .SequenceEqual(new[] { detail1.Id, detail2.Id }.OrderBy(id => id))
), ),

View File

@ -72,7 +72,7 @@ public class CipherServiceTests
[Theory, BitAutoData] [Theory, BitAutoData]
public async Task ShareManyAsync_WrongRevisionDate_Throws(SutProvider<CipherService> sutProvider, public async Task ShareManyAsync_WrongRevisionDate_Throws(SutProvider<CipherService> sutProvider,
IEnumerable<Cipher> ciphers, Guid organizationId, List<Guid> collectionIds) IEnumerable<CipherDetails> ciphers, Guid organizationId, List<Guid> collectionIds)
{ {
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organizationId) sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organizationId)
.Returns(new Organization .Returns(new Organization
@ -651,7 +651,7 @@ public class CipherServiceTests
[BitAutoData("")] [BitAutoData("")]
[BitAutoData("Correct Time")] [BitAutoData("Correct Time")]
public async Task ShareManyAsync_CorrectRevisionDate_Passes(string revisionDateString, public async Task ShareManyAsync_CorrectRevisionDate_Passes(string revisionDateString,
SutProvider<CipherService> sutProvider, IEnumerable<Cipher> ciphers, Organization organization, List<Guid> collectionIds) SutProvider<CipherService> sutProvider, IEnumerable<CipherDetails> ciphers, Organization organization, List<Guid> collectionIds)
{ {
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id) sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id)
.Returns(new Organization .Returns(new Organization
@ -1173,7 +1173,7 @@ public class CipherServiceTests
[Theory, BitAutoData] [Theory, BitAutoData]
public async Task ShareManyAsync_FreeOrgWithAttachment_Throws(SutProvider<CipherService> sutProvider, public async Task ShareManyAsync_FreeOrgWithAttachment_Throws(SutProvider<CipherService> sutProvider,
IEnumerable<Cipher> ciphers, Guid organizationId, List<Guid> collectionIds) IEnumerable<CipherDetails> ciphers, Guid organizationId, List<Guid> collectionIds)
{ {
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organizationId).Returns(new Organization sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organizationId).Returns(new Organization
{ {
@ -1194,7 +1194,7 @@ public class CipherServiceTests
[Theory, BitAutoData] [Theory, BitAutoData]
public async Task ShareManyAsync_PaidOrgWithAttachment_Passes(SutProvider<CipherService> sutProvider, public async Task ShareManyAsync_PaidOrgWithAttachment_Passes(SutProvider<CipherService> sutProvider,
IEnumerable<Cipher> ciphers, Guid organizationId, List<Guid> collectionIds) IEnumerable<CipherDetails> ciphers, Guid organizationId, List<Guid> collectionIds)
{ {
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organizationId) sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organizationId)
.Returns(new Organization .Returns(new Organization