1
0
mirror of https://github.com/bitwarden/server synced 2025-12-06 00:03:34 +00:00

[PM-24357] Do not purge ciphers in the default collection (#6320)

* do not purge ciphers in the default collection

* Update `DeleteByOrganizationId` procedure to be more performant based on PR review feedback

* update EF integration for purge to match new SQL implementation

* update Cipher_DeleteByOrganizationId based on PR feedback from dbops team
This commit is contained in:
Nick Krantz
2025-09-24 12:52:04 -05:00
committed by GitHub
parent 6e4f05ebd3
commit 6edab46d97
4 changed files with 345 additions and 49 deletions

View File

@@ -281,17 +281,20 @@ public class CipherRepository : Repository<Core.Vault.Entities.Cipher, Cipher, G
{ {
var dbContext = GetDatabaseContext(scope); var dbContext = GetDatabaseContext(scope);
var collectionCiphers = from cc in dbContext.CollectionCiphers var ciphersToDelete = from c in dbContext.Ciphers
join c in dbContext.Collections where c.OrganizationId == organizationId
on cc.CollectionId equals c.Id && !c.CollectionCiphers.Any(cc =>
where c.OrganizationId == organizationId cc.Collection.Type == CollectionType.DefaultUserCollection)
select cc; select c;
dbContext.RemoveRange(collectionCiphers); dbContext.RemoveRange(ciphersToDelete);
var ciphers = from c in dbContext.Ciphers var collectionCiphersToRemove = from cc in dbContext.CollectionCiphers
where c.OrganizationId == organizationId join col in dbContext.Collections on cc.CollectionId equals col.Id
select c; join c in dbContext.Ciphers on cc.CipherId equals c.Id
dbContext.RemoveRange(ciphers); where col.Type != CollectionType.DefaultUserCollection
&& c.OrganizationId == organizationId
select cc;
dbContext.RemoveRange(collectionCiphersToRemove);
await OrganizationUpdateStorage(organizationId); await OrganizationUpdateStorage(organizationId);
await dbContext.UserBumpAccountRevisionDateByOrganizationIdAsync(organizationId); await dbContext.UserBumpAccountRevisionDateByOrganizationIdAsync(organizationId);

View File

@@ -1,49 +1,91 @@
CREATE PROCEDURE [dbo].[Cipher_DeleteByOrganizationId] CREATE PROCEDURE [dbo].[Cipher_DeleteByOrganizationId]
@OrganizationId AS UNIQUEIDENTIFIER @OrganizationId UNIQUEIDENTIFIER
AS AS
BEGIN BEGIN
SET NOCOUNT ON SET NOCOUNT ON;
DECLARE @BatchSize INT = 100 DECLARE @BatchSize INT = 1000;
-- Delete collection ciphers BEGIN TRY
WHILE @BatchSize > 0 BEGIN TRANSACTION;
BEGIN
BEGIN TRANSACTION Cipher_DeleteByOrganizationId_CC
DELETE TOP(@BatchSize) CC ---------------------------------------------------------------------
FROM -- 1. Delete organization ciphers that are NOT in any default
[dbo].[CollectionCipher] CC -- user collection (Collection.Type = 1).
INNER JOIN ---------------------------------------------------------------------
[dbo].[Collection] C ON C.[Id] = CC.[CollectionId] WHILE 1 = 1
WHERE BEGIN
C.[OrganizationId] = @OrganizationId ;WITH Target AS
(
SELECT TOP (@BatchSize) C.Id
FROM dbo.Cipher C
WHERE C.OrganizationId = @OrganizationId
AND NOT EXISTS (
SELECT 1
FROM dbo.CollectionCipher CC2
INNER JOIN dbo.Collection Col2
ON Col2.Id = CC2.CollectionId
AND Col2.Type = 1 -- Default user collection
WHERE CC2.CipherId = C.Id
)
ORDER BY C.Id -- Deterministic ordering (matches clustered index)
)
DELETE C
FROM dbo.Cipher C
INNER JOIN Target T ON T.Id = C.Id;
SET @BatchSize = @@ROWCOUNT IF @@ROWCOUNT = 0 BREAK;
END
COMMIT TRANSACTION Cipher_DeleteByOrganizationId_CC ---------------------------------------------------------------------
END -- 2. Remove remaining CollectionCipher rows that reference
-- non-default (Type = 0 / shared) collections, for ciphers
-- that were preserved because they belong to at least one
-- default (Type = 1) collection.
---------------------------------------------------------------------
SET @BatchSize = 1000;
WHILE 1 = 1
BEGIN
;WITH ToDelete AS
(
SELECT TOP (@BatchSize)
CC.CipherId,
CC.CollectionId
FROM dbo.CollectionCipher CC
INNER JOIN dbo.Collection Col
ON Col.Id = CC.CollectionId
AND Col.Type = 0 -- Non-default collections
INNER JOIN dbo.Cipher C
ON C.Id = CC.CipherId
WHERE C.OrganizationId = @OrganizationId
ORDER BY CC.CollectionId, CC.CipherId -- Matches clustered index
)
DELETE CC
FROM dbo.CollectionCipher CC
INNER JOIN ToDelete TD
ON CC.CipherId = TD.CipherId
AND CC.CollectionId = TD.CollectionId;
-- Reset batch size IF @@ROWCOUNT = 0 BREAK;
SET @BatchSize = 100 END
-- Delete ciphers ---------------------------------------------------------------------
WHILE @BatchSize > 0 -- 3. Bump revision date (inside transaction for consistency)
BEGIN ---------------------------------------------------------------------
BEGIN TRANSACTION Cipher_DeleteByOrganizationId EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId;
DELETE TOP(@BatchSize) COMMIT TRANSACTION ;
FROM
[dbo].[Cipher]
WHERE
[OrganizationId] = @OrganizationId
SET @BatchSize = @@ROWCOUNT ---------------------------------------------------------------------
-- 4. Update storage usage (outside the transaction to avoid
COMMIT TRANSACTION Cipher_DeleteByOrganizationId -- holding locks during long-running calculation)
END ---------------------------------------------------------------------
EXEC [dbo].[Organization_UpdateStorage] @OrganizationId;
-- Cleanup organization END TRY
EXEC [dbo].[Organization_UpdateStorage] @OrganizationId BEGIN CATCH
EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId IF @@TRANCOUNT > 0
END ROLLBACK TRANSACTION;
THROW;
END CATCH
END
GO

View File

@@ -1241,4 +1241,164 @@ public class CipherRepositoryTests
Assert.NotNull(archivedCipher); Assert.NotNull(archivedCipher);
Assert.NotNull(archivedCipher.ArchivedDate); Assert.NotNull(archivedCipher.ArchivedDate);
} }
[DatabaseTheory, DatabaseData]
public async Task DeleteByOrganizationIdAsync_ExcludesDefaultCollectionCiphers(
IOrganizationRepository organizationRepository,
IUserRepository userRepository,
ICipherRepository cipherRepository,
ICollectionRepository collectionRepository,
ICollectionCipherRepository collectionCipherRepository)
{
var user = await userRepository.CreateAsync(new User
{
Name = "Test User",
Email = $"test+{Guid.NewGuid()}@email.com",
ApiKey = "TEST",
SecurityStamp = "stamp",
});
var organization = await organizationRepository.CreateAsync(new Organization
{
Name = "Test Organization",
BillingEmail = user.Email,
Plan = "Test"
});
var defaultCollection = await collectionRepository.CreateAsync(new Collection
{
Name = "Default Collection",
OrganizationId = organization.Id,
Type = CollectionType.DefaultUserCollection
});
var sharedCollection = await collectionRepository.CreateAsync(new Collection
{
Name = "Shared Collection",
OrganizationId = organization.Id,
});
async Task<Cipher> CreateOrgCipherAsync() => await cipherRepository.CreateAsync(new Cipher
{
Type = CipherType.Login,
OrganizationId = organization.Id,
Data = ""
});
var cipherInDefaultCollection = await CreateOrgCipherAsync();
var cipherInSharedCollection = await CreateOrgCipherAsync();
var cipherInBothCollections = await CreateOrgCipherAsync();
var unassignedCipher = await CreateOrgCipherAsync();
await collectionCipherRepository.UpdateCollectionsForAdminAsync(cipherInDefaultCollection.Id, organization.Id,
new List<Guid> { defaultCollection.Id });
await collectionCipherRepository.UpdateCollectionsForAdminAsync(cipherInSharedCollection.Id, organization.Id,
new List<Guid> { sharedCollection.Id });
await collectionCipherRepository.UpdateCollectionsForAdminAsync(cipherInBothCollections.Id, organization.Id,
new List<Guid> { defaultCollection.Id, sharedCollection.Id });
await cipherRepository.DeleteByOrganizationIdAsync(organization.Id);
var remainingCipherInDefault = await cipherRepository.GetByIdAsync(cipherInDefaultCollection.Id);
var deletedCipherInShared = await cipherRepository.GetByIdAsync(cipherInSharedCollection.Id);
var remainingCipherInBoth = await cipherRepository.GetByIdAsync(cipherInBothCollections.Id);
var deletedUnassignedCipher = await cipherRepository.GetByIdAsync(unassignedCipher.Id);
Assert.Null(deletedCipherInShared);
Assert.Null(deletedUnassignedCipher);
Assert.NotNull(remainingCipherInDefault);
Assert.NotNull(remainingCipherInBoth);
var remainingCollectionCiphers = await collectionCipherRepository.GetManyByOrganizationIdAsync(organization.Id);
// Should still have the default collection cipher relationships
Assert.Contains(remainingCollectionCiphers, cc =>
cc.CipherId == cipherInDefaultCollection.Id && cc.CollectionId == defaultCollection.Id);
Assert.Contains(remainingCollectionCiphers, cc =>
cc.CipherId == cipherInBothCollections.Id && cc.CollectionId == defaultCollection.Id);
// Should not have the shared collection cipher relationships
Assert.DoesNotContain(remainingCollectionCiphers, cc => cc.CollectionId == sharedCollection.Id);
}
[DatabaseTheory, DatabaseData]
public async Task DeleteByOrganizationIdAsync_DeletesAllWhenNoDefaultCollections(
IOrganizationRepository organizationRepository,
IUserRepository userRepository,
ICipherRepository cipherRepository,
ICollectionRepository collectionRepository,
ICollectionCipherRepository collectionCipherRepository)
{
// Arrange
var user = await userRepository.CreateAsync(new User
{
Name = "Test User",
Email = $"test+{Guid.NewGuid()}@email.com",
ApiKey = "TEST",
SecurityStamp = "stamp",
});
var organization = await organizationRepository.CreateAsync(new Organization
{
Name = "Test Organization",
BillingEmail = user.Email,
Plan = "Test"
});
var sharedCollection1 = await collectionRepository.CreateAsync(new Collection
{
Name = "Shared Collection 1",
OrganizationId = organization.Id,
Type = CollectionType.SharedCollection
});
var sharedCollection2 = await collectionRepository.CreateAsync(new Collection
{
Name = "Shared Collection 2",
OrganizationId = organization.Id,
Type = CollectionType.SharedCollection
});
// Create ciphers
var cipherInSharedCollection1 = await cipherRepository.CreateAsync(new Cipher
{
Type = CipherType.Login,
OrganizationId = organization.Id,
Data = ""
});
var cipherInSharedCollection2 = await cipherRepository.CreateAsync(new Cipher
{
Type = CipherType.Login,
OrganizationId = organization.Id,
Data = ""
});
var unassignedCipher = await cipherRepository.CreateAsync(new Cipher
{
Type = CipherType.Login,
OrganizationId = organization.Id,
Data = ""
});
await collectionCipherRepository.UpdateCollectionsForAdminAsync(cipherInSharedCollection1.Id, organization.Id,
new List<Guid> { sharedCollection1.Id });
await collectionCipherRepository.UpdateCollectionsForAdminAsync(cipherInSharedCollection2.Id, organization.Id,
new List<Guid> { sharedCollection2.Id });
await cipherRepository.DeleteByOrganizationIdAsync(organization.Id);
var deletedCipher1 = await cipherRepository.GetByIdAsync(cipherInSharedCollection1.Id);
var deletedCipher2 = await cipherRepository.GetByIdAsync(cipherInSharedCollection2.Id);
var deletedUnassignedCipher = await cipherRepository.GetByIdAsync(unassignedCipher.Id);
Assert.Null(deletedCipher1);
Assert.Null(deletedCipher2);
Assert.Null(deletedUnassignedCipher);
// All collection cipher relationships should be removed
var remainingCollectionCiphers = await collectionCipherRepository.GetManyByOrganizationIdAsync(organization.Id);
Assert.Empty(remainingCollectionCiphers);
}
} }

View File

@@ -0,0 +1,91 @@
CREATE OR ALTER PROCEDURE [dbo].[Cipher_DeleteByOrganizationId]
@OrganizationId UNIQUEIDENTIFIER
AS
BEGIN
SET NOCOUNT ON;
DECLARE @BatchSize INT = 1000;
BEGIN TRY
BEGIN TRANSACTION;
---------------------------------------------------------------------
-- 1. Delete organization ciphers that are NOT in any default
-- user collection (Collection.Type = 1).
---------------------------------------------------------------------
WHILE 1 = 1
BEGIN
;WITH Target AS
(
SELECT TOP (@BatchSize) C.Id
FROM dbo.Cipher C
WHERE C.OrganizationId = @OrganizationId
AND NOT EXISTS (
SELECT 1
FROM dbo.CollectionCipher CC2
INNER JOIN dbo.Collection Col2
ON Col2.Id = CC2.CollectionId
AND Col2.Type = 1 -- Default user collection
WHERE CC2.CipherId = C.Id
)
ORDER BY C.Id -- Deterministic ordering (matches clustered index)
)
DELETE C
FROM dbo.Cipher C
INNER JOIN Target T ON T.Id = C.Id;
IF @@ROWCOUNT = 0 BREAK;
END
---------------------------------------------------------------------
-- 2. Remove remaining CollectionCipher rows that reference
-- non-default (Type = 0 / shared) collections, for ciphers
-- that were preserved because they belong to at least one
-- default (Type = 1) collection.
---------------------------------------------------------------------
SET @BatchSize = 1000;
WHILE 1 = 1
BEGIN
;WITH ToDelete AS
(
SELECT TOP (@BatchSize)
CC.CipherId,
CC.CollectionId
FROM dbo.CollectionCipher CC
INNER JOIN dbo.Collection Col
ON Col.Id = CC.CollectionId
AND Col.Type = 0 -- Non-default collections
INNER JOIN dbo.Cipher C
ON C.Id = CC.CipherId
WHERE C.OrganizationId = @OrganizationId
ORDER BY CC.CollectionId, CC.CipherId -- Matches clustered index
)
DELETE CC
FROM dbo.CollectionCipher CC
INNER JOIN ToDelete TD
ON CC.CipherId = TD.CipherId
AND CC.CollectionId = TD.CollectionId;
IF @@ROWCOUNT = 0 BREAK;
END
---------------------------------------------------------------------
-- 3. Bump revision date (inside transaction for consistency)
---------------------------------------------------------------------
EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationId] @OrganizationId;
COMMIT TRANSACTION ;
---------------------------------------------------------------------
-- 4. Update storage usage (outside the transaction to avoid
-- holding locks during long-running calculation)
---------------------------------------------------------------------
EXEC [dbo].[Organization_UpdateStorage] @OrganizationId;
END TRY
BEGIN CATCH
IF @@TRANCOUNT > 0
ROLLBACK TRANSACTION;
THROW;
END CATCH
END
GO