diff --git a/src/Core/AdminConsole/Collections/DuplicateDefaultCollectionException.cs b/src/Core/AdminConsole/Collections/DuplicateDefaultCollectionException.cs new file mode 100644 index 0000000000..abe1cdedb0 --- /dev/null +++ b/src/Core/AdminConsole/Collections/DuplicateDefaultCollectionException.cs @@ -0,0 +1,5 @@ +namespace Bit.Core.AdminConsole.Collections; + +public class DuplicateDefaultCollectionException() + : Exception("A My Items collection already exists for one or more of the specified organization members."); + diff --git a/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs b/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs index 70f2de9d71..3340fec8f1 100644 --- a/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/CollectionRepository.cs @@ -1,13 +1,12 @@ using System.Data; using System.Diagnostics.CodeAnalysis; using System.Text.Json; +using Bit.Core.AdminConsole.Collections; using Bit.Core.AdminConsole.OrganizationFeatures.Collections; using Bit.Core.Entities; -using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Settings; -using Bit.Core.Utilities; using Bit.Infrastructure.Dapper.AdminConsole.Helpers; using Dapper; using Microsoft.Data.SqlClient; @@ -372,16 +371,23 @@ public class CollectionRepository : Repository, ICollectionRep await using var connection = new SqlConnection(ConnectionString); await connection.OpenAsync(); - var organizationUserIdsJson = JsonSerializer.Serialize(organizationUserIds); - await connection.ExecuteAsync( - "[dbo].[Collection_CreateDefaultCollections]", - new - { - OrganizationId = organizationId, - DefaultCollectionName = defaultCollectionName, - OrganizationUserIdsJson = organizationUserIdsJson - }, - commandType: CommandType.StoredProcedure); + try + { + var organizationUserIdsJson = JsonSerializer.Serialize(organizationUserIds); + await connection.ExecuteAsync( + "[dbo].[Collection_CreateDefaultCollections]", + new + { + OrganizationId = organizationId, + DefaultCollectionName = defaultCollectionName, + OrganizationUserIdsJson = organizationUserIdsJson + }, + commandType: CommandType.StoredProcedure); + } + catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex)) + { + throw new DuplicateDefaultCollectionException(); + } } public async Task CreateDefaultCollectionsBulkAsync(Guid organizationId, IEnumerable organizationUserIds, string defaultCollectionName) @@ -417,6 +423,11 @@ public class CollectionRepository : Repository, ICollectionRep transaction.Commit(); } + catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex)) + { + transaction.Rollback(); + throw new DuplicateDefaultCollectionException(); + } catch { transaction.Rollback(); diff --git a/src/Infrastructure.Dapper/Repositories/DatabaseExceptionHelpers.cs b/src/Infrastructure.Dapper/Repositories/DatabaseExceptionHelpers.cs new file mode 100644 index 0000000000..4403fb4bfc --- /dev/null +++ b/src/Infrastructure.Dapper/Repositories/DatabaseExceptionHelpers.cs @@ -0,0 +1,17 @@ +namespace Bit.Infrastructure.Dapper.Repositories; + +#nullable enable + +internal static class DatabaseExceptionHelpers +{ + /// + /// Determines if an exception represents a SQL Server duplicate key constraint violation. + /// + public static bool IsDuplicateKeyException(Exception exception) + { + ArgumentNullException.ThrowIfNull(exception); + + return exception is Microsoft.Data.SqlClient.SqlException { Errors: not null } msEx && + msEx.Errors.Cast().Any(error => error.Number == 2627); + } +} diff --git a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs index 09ec901aba..fb0539b3c3 100644 --- a/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/CollectionRepository.cs @@ -1,9 +1,10 @@ -using AutoMapper; +using System.Data.Common; +using AutoMapper; +using Bit.Core.AdminConsole.Collections; using Bit.Core.AdminConsole.OrganizationFeatures.Collections; using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Repositories; -using Bit.Core.Utilities; using Bit.Infrastructure.EntityFramework.AdminConsole.Models; using Bit.Infrastructure.EntityFramework.Models; using Bit.Infrastructure.EntityFramework.Repositories.Queries; @@ -810,20 +811,27 @@ public class CollectionRepository : Repository new DefaultCollectionSemaphore + try { - OrganizationUserId = c.OrganizationUserId, - CreationDate = now - }).ToList(); + // CRITICAL: Insert semaphore entries BEFORE collections + // Database will throw on duplicate primary key (OrganizationUserId) + var now = DateTime.UtcNow; + var semaphores = collectionUsers.Select(c => new DefaultCollectionSemaphore + { + OrganizationUserId = c.OrganizationUserId, + CreationDate = now + }).ToList(); - await dbContext.BulkCopyAsync(semaphores); - await dbContext.BulkCopyAsync(Mapper.Map>(collections)); - await dbContext.BulkCopyAsync(Mapper.Map>(collectionUsers)); + await dbContext.BulkCopyAsync(semaphores); + await dbContext.BulkCopyAsync(Mapper.Map>(collections)); + await dbContext.BulkCopyAsync(Mapper.Map>(collectionUsers)); - await dbContext.SaveChangesAsync(); + await dbContext.SaveChangesAsync(); + } + catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex)) + { + throw new DuplicateDefaultCollectionException(); + } } public async Task CreateDefaultCollectionsBulkAsync(Guid organizationId, IEnumerable organizationUserIds, string defaultCollectionName) diff --git a/src/Infrastructure.EntityFramework/Repositories/DatabaseExceptionHelpers.cs b/src/Infrastructure.EntityFramework/Repositories/DatabaseExceptionHelpers.cs new file mode 100644 index 0000000000..e5c9ecbd9a --- /dev/null +++ b/src/Infrastructure.EntityFramework/Repositories/DatabaseExceptionHelpers.cs @@ -0,0 +1,37 @@ +using System.Data.Common; +using Microsoft.EntityFrameworkCore; + +namespace Bit.Infrastructure.EntityFramework.Repositories; + +#nullable enable + +internal static class DatabaseExceptionHelpers +{ + /// + /// Determines if a DbUpdateException represents a duplicate key constraint violation. + /// Works with MySQL, SQL Server, PostgreSQL, and SQLite. + /// + public static bool IsDuplicateKeyException(Exception exception) + { + ArgumentNullException.ThrowIfNull(exception); + + switch (exception) + { + // MySQL + case MySqlConnector.MySqlException myEx: + return myEx.ErrorCode == MySqlConnector.MySqlErrorCode.DuplicateKeyEntry; + // SQL Server + case Microsoft.Data.SqlClient.SqlException msEx: + return msEx.Errors != null && + msEx.Errors.Cast().Any(error => error.Number == 2627); + // PostgreSQL + case Npgsql.PostgresException pgEx: + return pgEx.SqlState == "23505"; + // SQLite + case Microsoft.Data.Sqlite.SqliteException liteEx: + return liteEx is { SqliteErrorCode: 19, SqliteExtendedErrorCode: 1555 }; + default: + return false; + } + } +} diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsBulkTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsBulkTests.cs index 5851d8d468..b841681cf9 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsBulkTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsBulkTests.cs @@ -1,4 +1,6 @@ -using Bit.Core.AdminConsole.Entities; + +using Bit.Core.AdminConsole.Collections; +using Bit.Core.AdminConsole.Entities; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; @@ -96,8 +98,8 @@ public class CreateDefaultCollectionsBulkTests await CreateUsersWithExistingDefaultCollectionsAsync(collectionRepository, organization.Id, affectedOrgUserIds, defaultCollectionName, resultOrganizationUsers); - // Act - Try to create again, should throw database constraint exception - await Assert.ThrowsAnyAsync(() => + // Act - Try to create again, should throw specific duplicate collection exception + await Assert.ThrowsAsync(() => collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, affectedOrgUserIds, defaultCollectionName)); // Assert - Original collections should remain unchanged @@ -125,7 +127,7 @@ public class CreateDefaultCollectionsBulkTests await collectionRepository.CreateDefaultCollectionsBulkAsync(organization.Id, [existingUser.Id], defaultCollectionName); // Act - Try to create for both without filtering (incorrect usage) - await Assert.ThrowsAnyAsync(() => + await Assert.ThrowsAsync(() => collectionRepository.CreateDefaultCollectionsBulkAsync( organization.Id, [existingUser.Id, newUser.Id], diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsTests.cs index b8ca61a3c7..120951b302 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CreateDefaultCollectionsTests.cs @@ -1,4 +1,5 @@ -using Bit.Core.Enums; +using Bit.Core.AdminConsole.Collections; +using Bit.Core.Enums; using Bit.Core.Repositories; using Xunit; @@ -87,8 +88,8 @@ public class CreateDefaultCollectionsTests [orgUser.Id], "My Items"); - // Second call should throw and should not create duplicate - await Assert.ThrowsAnyAsync(() => + // Second call should throw specific exception and should not create duplicate + await Assert.ThrowsAsync(() => collectionRepository.CreateDefaultCollectionsAsync( organization.Id, [orgUser.Id],