[PM-25982] Restrict Ciphers being assigned to Default from Shared collections (#6382)

* validate that any change in collection does not allow only shared ciphers to migrate to a default cipher

* refactor order of checks to avoid any unnecessary calls

* remove unneeded conditional
This commit is contained in:
Nick Krantz 2025-09-29 13:06:52 -05:00 committed by GitHub
parent f1af331a0c
commit 46958cc838
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 54 additions and 0 deletions

View File

@ -887,6 +887,9 @@ public class CiphersController : Controller
[HttpPost("bulk-collections")]
public async Task PostBulkCollections([FromBody] CipherBulkUpdateCollectionsRequestModel model)
{
var userId = _userService.GetProperUserId(User).Value;
await _cipherService.ValidateBulkCollectionAssignmentAsync(model.CollectionIds, model.CipherIds, userId);
if (!await CanModifyCipherCollectionsAsync(model.OrganizationId, model.CipherIds) ||
!await CanEditItemsInCollections(model.OrganizationId, model.CollectionIds))
{

View File

@ -37,4 +37,5 @@ public interface ICipherService
Task UploadFileForExistingAttachmentAsync(Stream stream, Cipher cipher, CipherAttachment.MetaData attachmentId);
Task<AttachmentResponseData> GetAttachmentDownloadDataAsync(Cipher cipher, string attachmentId);
Task<bool> ValidateCipherAttachmentFile(Cipher cipher, CipherAttachment.MetaData attachmentData);
Task ValidateBulkCollectionAssignmentAsync(IEnumerable<Guid> collectionIds, IEnumerable<Guid> cipherIds, Guid userId);
}

View File

@ -170,6 +170,7 @@ public class CipherService : ICipherService
{
ValidateCipherLastKnownRevisionDateAsync(cipher, lastKnownRevisionDate);
cipher.RevisionDate = DateTime.UtcNow;
await ValidateChangeInCollectionsAsync(cipher, collectionIds, savingUserId);
await ValidateViewPasswordUserAsync(cipher);
await _cipherRepository.ReplaceAsync(cipher);
await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_Updated);
@ -539,6 +540,7 @@ public class CipherService : ICipherService
try
{
await ValidateCipherCanBeShared(cipher, sharingUserId, organizationId, lastKnownRevisionDate);
await ValidateChangeInCollectionsAsync(cipher, collectionIds, sharingUserId);
// Sproc will not save this UserId on the cipher. It is used limit scope of the collectionIds.
cipher.UserId = sharingUserId;
@ -678,6 +680,7 @@ public class CipherService : ICipherService
{
throw new BadRequestException("Cipher must belong to an organization.");
}
await ValidateChangeInCollectionsAsync(cipher, collectionIds, savingUserId);
cipher.RevisionDate = DateTime.UtcNow;
@ -820,6 +823,15 @@ public class CipherService : ICipherService
return restoringCiphers;
}
public async Task ValidateBulkCollectionAssignmentAsync(IEnumerable<Guid> collectionIds, IEnumerable<Guid> cipherIds, Guid userId)
{
foreach (var cipherId in cipherIds)
{
var cipher = await _cipherRepository.GetByIdAsync(cipherId);
await ValidateChangeInCollectionsAsync(cipher, collectionIds, userId);
}
}
private async Task<bool> UserCanEditAsync(Cipher cipher, Guid userId)
{
if (!cipher.OrganizationId.HasValue && cipher.UserId.HasValue && cipher.UserId.Value == userId)
@ -1038,6 +1050,44 @@ public class CipherService : ICipherService
}
}
// Validates that a cipher is not being added to a default collection when it is only currently only in shared collections
private async Task ValidateChangeInCollectionsAsync(Cipher updatedCipher, IEnumerable<Guid> newCollectionIds, Guid userId)
{
if (updatedCipher.Id == Guid.Empty || !updatedCipher.OrganizationId.HasValue)
{
return;
}
var currentCollectionsForCipher = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, updatedCipher.Id);
if (!currentCollectionsForCipher.Any())
{
// When a cipher is not currently in any collections it can be assigned to any type of collection
return;
}
var currentCollections = await _collectionRepository.GetManyByManyIdsAsync(currentCollectionsForCipher.Select(c => c.CollectionId));
var currentCollectionsContainDefault = currentCollections.Any(c => c.Type == CollectionType.DefaultUserCollection);
// When the current cipher already contains the default collection, no check is needed for if they added or removed
// a default collection, because it is already there.
if (currentCollectionsContainDefault)
{
return;
}
var newCollections = await _collectionRepository.GetManyByManyIdsAsync(newCollectionIds);
var newCollectionsContainDefault = newCollections.Any(c => c.Type == CollectionType.DefaultUserCollection);
if (newCollectionsContainDefault)
{
// User is trying to add the default collection when the cipher is only in shared collections
throw new BadRequestException("The cipher(s) cannot be assigned to a default collection when only assigned to non-default collections.");
}
}
private string SerializeCipherData(CipherData data)
{
return data switch