Skip to content

[PM-22344] - fix Error: Cannot Decrypt when moving a vault item to a collection #5911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/Api/Vault/Controllers/CiphersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,7 +1064,7 @@ public async Task MoveMany([FromBody] CipherBulkMoveRequestModel model)

[HttpPut("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);
if (!await _currentContext.OrganizationUser(organizationId))
Expand All @@ -1086,7 +1086,7 @@ public async Task<CipherMiniResponseModel[]> PutShareMany([FromBody] CipherBulkS
}
}

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))
Expand All @@ -1096,7 +1096,7 @@ public async Task<CipherMiniResponseModel[]> PutShareMany([FromBody] CipherBulkS

ValidateClientVersionForFido2CredentialSupport(existingCipher);

shareCiphers.Add(((Cipher)existingCipher, cipher.LastKnownRevisionDate));
shareCiphers.Add((cipher.ToCipherDetails(existingCipher), cipher.LastKnownRevisionDate));
}

var updated = await _cipherService.ShareManyAsync(
Expand All @@ -1106,7 +1106,8 @@ public async Task<CipherMiniResponseModel[]> PutShareMany([FromBody] CipherBulkS
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")]
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Vault/Services/ICipherService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Task CreateAttachmentShareAsync(Cipher cipher, Stream stream, string fileName, s
Task DeleteFolderAsync(Folder folder);
Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organizationId, IEnumerable<Guid> collectionIds,
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);
Task SaveCollectionsAsync(Cipher cipher, IEnumerable<Guid> collectionIds, Guid savingUserId, bool orgAdmin);
Task SoftDeleteAsync(CipherDetails cipherDetails, Guid deletingUserId, bool orgAdmin = false);
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Vault/Services/Implementations/CipherService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ await _attachmentStorageService.RollbackShareAttachmentAsync(cipher.Id, organiza
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)
{
var cipherIds = new List<Guid>();
Expand Down
49 changes: 35 additions & 14 deletions test/Api.Test/Vault/Controllers/CiphersControllerTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -1774,16 +1775,16 @@ public async Task PutShareMany_ShouldShareCiphersAndReturnRevisionDateMap(
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<CipherDetails> { 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<ICurrentContext>()
.OrganizationUser(organizationId)
Expand All @@ -1801,19 +1802,39 @@ public async Task PutShareMany_ShouldShareCiphersAndReturnRevisionDateMap(

sutProvider.GetDependency<ICipherService>()
.ShareManyAsync(
Arg.Any<IEnumerable<(Cipher, DateTime?)>>(),
Arg.Any<IEnumerable<(CipherDetails, DateTime?)>>(),
organizationId,
Arg.Any<IEnumerable<Guid>>(),
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,
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<CipherFieldModel>();
m.PasswordHistory = Array.Empty<CipherPasswordHistoryModel>();
}

// similar for SecureNote, Card, etc., if you ever hit those branches
return m;
}).ToList();

var model = new CipherBulkShareRequestModel
Expand All @@ -1824,15 +1845,15 @@ public async Task PutShareMany_ShouldShareCiphersAndReturnRevisionDateMap(

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<ICipherService>()
.Received(1)
.ShareManyAsync(
Arg.Is<IEnumerable<(Cipher, DateTime?)>>(list =>
Arg.Is<IEnumerable<(CipherDetails, DateTime?)>>(list =>
list.Select(x => x.Item1.Id).OrderBy(id => id)
.SequenceEqual(new[] { detail1.Id, detail2.Id }.OrderBy(id => id))
),
Expand Down
8 changes: 4 additions & 4 deletions test/Core.Test/Vault/Services/CipherServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public async Task ShareAsync_WrongRevisionDate_Throws(SutProvider<CipherService>

[Theory, BitAutoData]
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)
.Returns(new Organization
Expand Down Expand Up @@ -651,7 +651,7 @@ await attachmentStorageService.Received().RollbackShareAttachmentAsync(cipher.Id
[BitAutoData("")]
[BitAutoData("Correct Time")]
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)
.Returns(new Organization
Expand Down Expand Up @@ -1173,7 +1173,7 @@ await sutProvider.GetDependency<IPushNotificationService>()

[Theory, BitAutoData]
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
{
Expand All @@ -1194,7 +1194,7 @@ public async Task ShareManyAsync_FreeOrgWithAttachment_Throws(SutProvider<Cipher

[Theory, BitAutoData]
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)
.Returns(new Organization
Expand Down
Loading