diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs index 67b5f0da80..6ffb54696a 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs @@ -79,19 +79,10 @@ public class AutomaticallyConfirmOrganizationUserCommand(IOrganizationUserReposi return; } - await collectionRepository.CreateAsync( - new Collection - { - OrganizationId = request.Organization!.Id, - Name = request.DefaultUserCollectionName, - Type = CollectionType.DefaultUserCollection - }, - groups: null, - [new CollectionAccessSelection - { - Id = request.OrganizationUser!.Id, - Manage = true - }]); + await collectionRepository.UpsertDefaultCollectionAsync( + request.Organization!.Id, + request.OrganizationUser!.Id, + request.DefaultUserCollectionName); } catch (Exception ex) { diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs index 2fbe6be5c6..658f301e23 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommand.cs @@ -274,21 +274,10 @@ public class ConfirmOrganizationUserCommand : IConfirmOrganizationUserCommand return; } - var defaultCollection = new Collection - { - OrganizationId = organizationUser.OrganizationId, - Name = defaultUserCollectionName, - Type = CollectionType.DefaultUserCollection - }; - var collectionUser = new CollectionAccessSelection - { - Id = organizationUser.Id, - ReadOnly = false, - HidePasswords = false, - Manage = true - }; - - await _collectionRepository.CreateAsync(defaultCollection, groups: null, users: [collectionUser]); + await _collectionRepository.UpsertDefaultCollectionAsync( + organizationUser.OrganizationId, + organizationUser.Id, + defaultUserCollectionName); } /// diff --git a/src/Core/Repositories/ICollectionRepository.cs b/src/Core/Repositories/ICollectionRepository.cs index f86147ca7d..4e0af70729 100644 --- a/src/Core/Repositories/ICollectionRepository.cs +++ b/src/Core/Repositories/ICollectionRepository.cs @@ -71,4 +71,14 @@ public interface ICollectionRepository : IRepository /// The encrypted string to use as the default collection name. /// Task UpsertDefaultCollectionsAsync(Guid organizationId, IEnumerable organizationUserIds, string defaultCollectionName); + + /// + /// Creates a default user collection for the specified organization user if they do not already have one. + /// This operation is idempotent - calling it multiple times will not create duplicate collections. + /// + /// The Organization ID. + /// The Organization User ID to create/find a default collection for. + /// The encrypted string to use as the default collection name. + /// True if a new collection was created; false if the user already had a default collection. + Task UpsertDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName); } diff --git a/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs b/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs index c2a59f75aa..d95fe15974 100644 --- a/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs @@ -363,6 +363,27 @@ public class CollectionRepository : Repository, ICollectionRep } } + public async Task UpsertDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var parameters = new DynamicParameters(); + parameters.Add("@OrganizationId", organizationId); + parameters.Add("@OrganizationUserId", organizationUserId); + parameters.Add("@Name", defaultCollectionName); + parameters.Add("@CreationDate", DateTime.UtcNow); + parameters.Add("@RevisionDate", DateTime.UtcNow); + parameters.Add("@WasCreated", dbType: DbType.Boolean, direction: ParameterDirection.Output); + + await connection.ExecuteAsync( + $"[{Schema}].[Collection_UpsertDefaultCollection]", + parameters, + commandType: CommandType.StoredProcedure); + + return parameters.Get("@WasCreated"); + } + } + private async Task> GetOrgUserIdsWithDefaultCollectionAsync(SqlConnection connection, SqlTransaction transaction, Guid organizationId) { const string sql = @" diff --git a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs index 5aa156d1f8..25e3c693d6 100644 --- a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs @@ -821,6 +821,69 @@ public class CollectionRepository : Repository UpsertDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName) + { + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + + // Check if this organization user already has a default collection + var existingDefaultCollection = await dbContext.OrganizationUsers + .Where(ou => ou.Id == organizationUserId && ou.OrganizationId == organizationId) + .Join( + dbContext.CollectionUsers, + ou => ou.Id, + cu => cu.OrganizationUserId, + (ou, cu) => cu) + .Join( + dbContext.Collections, + cu => cu.CollectionId, + c => c.Id, + (cu, c) => c) + .Where(c => c.Type == CollectionType.DefaultUserCollection) + .FirstOrDefaultAsync(); + + // If collection already exists, return false (not created) + if (existingDefaultCollection != null) + { + return false; + } + + // Create new default collection + var collectionId = Guid.NewGuid(); + var now = DateTime.UtcNow; + + var collection = new Collection + { + Id = collectionId, + OrganizationId = organizationId, + Name = defaultCollectionName, + ExternalId = null, + CreationDate = now, + RevisionDate = now, + Type = CollectionType.DefaultUserCollection, + DefaultUserCollectionEmail = null + }; + + var collectionUser = new CollectionUser + { + CollectionId = collectionId, + OrganizationUserId = organizationUserId, + ReadOnly = false, + HidePasswords = false, + Manage = true + }; + + await dbContext.Collections.AddAsync(collection); + await dbContext.CollectionUsers.AddAsync(collectionUser); + await dbContext.SaveChangesAsync(); + + // Bump user account revision dates + await dbContext.UserBumpAccountRevisionDateByCollectionIdAsync(collectionId, organizationId); + await dbContext.SaveChangesAsync(); + + return true; + } + private async Task> GetOrgUserIdsWithDefaultCollectionAsync(DatabaseContext dbContext, Guid organizationId) { var results = await dbContext.OrganizationUsers diff --git a/src/Sql/dbo/Stored Procedures/Collection_UpsertDefaultCollection.sql b/src/Sql/dbo/Stored Procedures/Collection_UpsertDefaultCollection.sql new file mode 100644 index 0000000000..189f67f85a --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/Collection_UpsertDefaultCollection.sql @@ -0,0 +1,78 @@ +CREATE PROCEDURE [dbo].[Collection_UpsertDefaultCollection] + @OrganizationId UNIQUEIDENTIFIER, + @OrganizationUserId UNIQUEIDENTIFIER, + @Name VARCHAR(MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7), + @WasCreated BIT OUTPUT +AS +BEGIN + SET NOCOUNT ON + + DECLARE @CollectionId UNIQUEIDENTIFIER; + + -- Check if this organization user already has a default collection + SELECT @CollectionId = c.Id + FROM [dbo].[Collection] c + INNER JOIN [dbo].[CollectionUser] cu ON cu.CollectionId = c.Id + WHERE cu.OrganizationUserId = @OrganizationUserId + AND c.OrganizationId = @OrganizationId + AND c.Type = 1; -- CollectionType.DefaultUserCollection + + -- If no default collection exists, create one + IF @CollectionId IS NULL + BEGIN + SET @CollectionId = NEWID(); + SET @WasCreated = 1; + + -- Insert Collection + INSERT INTO [dbo].[Collection] + ( + [Id], + [OrganizationId], + [Name], + [ExternalId], + [CreationDate], + [RevisionDate], + [DefaultUserCollectionEmail], + [Type] + ) + VALUES + ( + @CollectionId, + @OrganizationId, + @Name, + NULL, -- ExternalId + @CreationDate, + @RevisionDate, + NULL, -- DefaultUserCollectionEmail + 1 -- CollectionType.DefaultUserCollection + ); + + -- Insert CollectionUser + INSERT INTO [dbo].[CollectionUser] + ( + [CollectionId], + [OrganizationUserId], + [ReadOnly], + [HidePasswords], + [Manage] + ) + VALUES + ( + @CollectionId, + @OrganizationUserId, + 0, -- ReadOnly = false + 0, -- HidePasswords = false + 1 -- Manage = true + ); + + -- Bump user account revision dates + EXEC [dbo].[User_BumpAccountRevisionDateByCollectionId] @CollectionId, @OrganizationId; + END + ELSE + BEGIN + SET @WasCreated = 0; + END +END +GO diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUsers/AutomaticallyConfirmUsersCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUsers/AutomaticallyConfirmUsersCommandTests.cs index 1035d5c578..4d84d2b0c5 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUsers/AutomaticallyConfirmUsersCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUsers/AutomaticallyConfirmUsersCommandTests.cs @@ -203,14 +203,10 @@ public class AutomaticallyConfirmUsersCommandTests await sutProvider.GetDependency() .Received(1) - .CreateAsync( - Arg.Is(c => - c.OrganizationId == organization.Id && - c.Name == defaultCollectionName && - c.Type == CollectionType.DefaultUserCollection), - Arg.Is>(groups => groups == null), - Arg.Is>(access => - access.FirstOrDefault(x => x.Id == organizationUser.Id && x.Manage) != null)); + .UpsertDefaultCollectionAsync( + organization.Id, + organizationUser.Id, + defaultCollectionName); } [Theory] @@ -252,9 +248,10 @@ public class AutomaticallyConfirmUsersCommandTests await sutProvider.GetDependency() .DidNotReceive() - .CreateAsync(Arg.Any(), - Arg.Any>(), - Arg.Any>()); + .UpsertDefaultCollectionAsync( + Arg.Any(), + Arg.Any(), + Arg.Any()); } [Theory] @@ -290,9 +287,10 @@ public class AutomaticallyConfirmUsersCommandTests var collectionException = new Exception("Collection creation failed"); sutProvider.GetDependency() - .CreateAsync(Arg.Any(), - Arg.Any>(), - Arg.Any>()) + .UpsertDefaultCollectionAsync( + Arg.Any(), + Arg.Any(), + Arg.Any()) .ThrowsAsync(collectionException); // Act diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs index 86b068b88f..c83a5ddd62 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/ConfirmOrganizationUserCommandTests.cs @@ -491,15 +491,10 @@ public class ConfirmOrganizationUserCommandTests await sutProvider.GetDependency() .Received(1) - .CreateAsync( - Arg.Is(c => - c.Name == collectionName && - c.OrganizationId == organization.Id && - c.Type == CollectionType.DefaultUserCollection), - Arg.Any>(), - Arg.Is>(cu => - cu.Single().Id == orgUser.Id && - cu.Single().Manage)); + .UpsertDefaultCollectionAsync( + organization.Id, + orgUser.Id, + collectionName); } [Theory, BitAutoData] diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/UpsertDefaultCollectionTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/UpsertDefaultCollectionTests.cs new file mode 100644 index 0000000000..4ee7a9ed5d --- /dev/null +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/UpsertDefaultCollectionTests.cs @@ -0,0 +1,175 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Xunit; + +namespace Bit.Infrastructure.IntegrationTest.AdminConsole.Repositories.CollectionRepository; + +public class UpsertDefaultCollectionTests +{ + [Theory, DatabaseData] + public async Task UpsertDefaultCollectionAsync_ShouldCreateCollection_WhenUserDoesNotHaveDefaultCollection( + IOrganizationRepository organizationRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + ICollectionRepository collectionRepository) + { + // Arrange + var organization = await organizationRepository.CreateTestOrganizationAsync(); + var user = await userRepository.CreateTestUserAsync(); + var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + var defaultCollectionName = $"My Items - {organization.Id}"; + + // Act + var wasCreated = await collectionRepository.UpsertDefaultCollectionAsync( + organization.Id, + orgUser.Id, + defaultCollectionName); + + // Assert + Assert.True(wasCreated); + + var collectionDetails = await collectionRepository.GetManyByUserIdAsync(user.Id); + var defaultCollection = collectionDetails.SingleOrDefault(c => + c.OrganizationId == organization.Id && + c.Type == CollectionType.DefaultUserCollection); + + Assert.NotNull(defaultCollection); + Assert.Equal(defaultCollectionName, defaultCollection.Name); + Assert.True(defaultCollection.Manage); + Assert.False(defaultCollection.ReadOnly); + Assert.False(defaultCollection.HidePasswords); + + // Cleanup + await CleanupAsync(organizationRepository, userRepository, organization, orgUser); + } + + [Theory, DatabaseData] + public async Task UpsertDefaultCollectionAsync_ShouldReturnFalse_WhenUserAlreadyHasDefaultCollection( + IOrganizationRepository organizationRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + ICollectionRepository collectionRepository) + { + // Arrange + var organization = await organizationRepository.CreateTestOrganizationAsync(); + var user = await userRepository.CreateTestUserAsync(); + var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + var defaultCollectionName = $"My Items - {organization.Id}"; + + // Create initial collection + var firstWasCreated = await collectionRepository.UpsertDefaultCollectionAsync( + organization.Id, + orgUser.Id, + defaultCollectionName); + + // Act - Call again with same parameters + var secondWasCreated = await collectionRepository.UpsertDefaultCollectionAsync( + organization.Id, + orgUser.Id, + defaultCollectionName); + + // Assert + Assert.True(firstWasCreated); + Assert.False(secondWasCreated); + + // Verify only one default collection exists + var collectionDetails = await collectionRepository.GetManyByUserIdAsync(user.Id); + var defaultCollections = collectionDetails.Where(c => + c.OrganizationId == organization.Id && + c.Type == CollectionType.DefaultUserCollection).ToList(); + + Assert.Single(defaultCollections); + + // Cleanup + await CleanupAsync(organizationRepository, userRepository, organization, orgUser); + } + + [Theory, DatabaseData] + public async Task UpsertDefaultCollectionAsync_ShouldBeIdempotent_WhenCalledMultipleTimes( + IOrganizationRepository organizationRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + ICollectionRepository collectionRepository) + { + // Arrange + var organization = await organizationRepository.CreateTestOrganizationAsync(); + var user = await userRepository.CreateTestUserAsync(); + var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + var defaultCollectionName = $"My Items - {organization.Id}"; + + // Act - Call method 5 times + var results = new List(); + for (int i = 0; i < 5; i++) + { + var wasCreated = await collectionRepository.UpsertDefaultCollectionAsync( + organization.Id, + orgUser.Id, + defaultCollectionName); + results.Add(wasCreated); + } + + // Assert + Assert.True(results[0]); // First call should create + Assert.All(results.Skip(1), wasCreated => Assert.False(wasCreated)); // Rest should return false + + // Verify only one collection exists + var collectionDetails = await collectionRepository.GetManyByUserIdAsync(user.Id); + var defaultCollections = collectionDetails.Where(c => + c.OrganizationId == organization.Id && + c.Type == CollectionType.DefaultUserCollection).ToList(); + + Assert.Single(defaultCollections); + + // Cleanup + await CleanupAsync(organizationRepository, userRepository, organization, orgUser); + } + + [Theory, DatabaseData] + public async Task UpsertDefaultCollectionAsync_ShouldSetCorrectPermissions_ForNewCollection( + IOrganizationRepository organizationRepository, + IUserRepository userRepository, + IOrganizationUserRepository organizationUserRepository, + ICollectionRepository collectionRepository) + { + // Arrange + var organization = await organizationRepository.CreateTestOrganizationAsync(); + var user = await userRepository.CreateTestUserAsync(); + var orgUser = await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + var defaultCollectionName = $"My Items - {organization.Id}"; + + // Act + await collectionRepository.UpsertDefaultCollectionAsync( + organization.Id, + orgUser.Id, + defaultCollectionName); + + // Assert + var collectionDetails = await collectionRepository.GetManyByUserIdAsync(user.Id); + var defaultCollection = collectionDetails.Single(c => + c.OrganizationId == organization.Id && + c.Type == CollectionType.DefaultUserCollection); + + Assert.True(defaultCollection.Manage); + Assert.False(defaultCollection.ReadOnly); + Assert.False(defaultCollection.HidePasswords); + + // Cleanup + await CleanupAsync(organizationRepository, userRepository, organization, orgUser); + } + + private static async Task CleanupAsync( + IOrganizationRepository organizationRepository, + IUserRepository userRepository, + Organization organization, + OrganizationUser organizationUser) + { + await organizationRepository.DeleteAsync(organization); + + if (organizationUser.UserId != null) + { + await userRepository.DeleteAsync(new User { Id = organizationUser.UserId.Value }); + } + } +} diff --git a/util/Migrator/DbScripts/2025-12-02_00_Collection_UpsertDefaultCollection.sql b/util/Migrator/DbScripts/2025-12-02_00_Collection_UpsertDefaultCollection.sql new file mode 100644 index 0000000000..cbf1c68394 --- /dev/null +++ b/util/Migrator/DbScripts/2025-12-02_00_Collection_UpsertDefaultCollection.sql @@ -0,0 +1,82 @@ +-- Create the idempotent stored procedure for creating default collections +-- This procedure prevents duplicate "My Items" collections for users by checking +-- if a default collection already exists before attempting to create one. + +CREATE PROCEDURE [dbo].[Collection_UpsertDefaultCollection] + @OrganizationId UNIQUEIDENTIFIER, + @OrganizationUserId UNIQUEIDENTIFIER, + @Name VARCHAR(MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7), + @WasCreated BIT OUTPUT +AS +BEGIN + SET NOCOUNT ON + + DECLARE @CollectionId UNIQUEIDENTIFIER; + + -- Check if this organization user already has a default collection + SELECT @CollectionId = c.Id + FROM [dbo].[Collection] c + INNER JOIN [dbo].[CollectionUser] cu ON cu.CollectionId = c.Id + WHERE cu.OrganizationUserId = @OrganizationUserId + AND c.OrganizationId = @OrganizationId + AND c.Type = 1; -- CollectionType.DefaultUserCollection + + -- If no default collection exists, create one + IF @CollectionId IS NULL + BEGIN + SET @CollectionId = NEWID(); + SET @WasCreated = 1; + + -- Insert Collection + INSERT INTO [dbo].[Collection] + ( + [Id], + [OrganizationId], + [Name], + [ExternalId], + [CreationDate], + [RevisionDate], + [DefaultUserCollectionEmail], + [Type] + ) + VALUES + ( + @CollectionId, + @OrganizationId, + @Name, + NULL, -- ExternalId + @CreationDate, + @RevisionDate, + NULL, -- DefaultUserCollectionEmail + 1 -- CollectionType.DefaultUserCollection + ); + + -- Insert CollectionUser + INSERT INTO [dbo].[CollectionUser] + ( + [CollectionId], + [OrganizationUserId], + [ReadOnly], + [HidePasswords], + [Manage] + ) + VALUES + ( + @CollectionId, + @OrganizationUserId, + 0, -- ReadOnly = false + 0, -- HidePasswords = false + 1 -- Manage = true + ); + + -- Bump user account revision dates + EXEC [dbo].[User_BumpAccountRevisionDateByCollectionId] @CollectionId, @OrganizationId; + END + ELSE + BEGIN + SET @WasCreated = 0; + END +END +GO