1
0
mirror of https://github.com/bitwarden/server synced 2026-01-27 06:43:19 +00:00

Return uniform error

This commit is contained in:
Thomas Rittson
2026-01-01 15:16:48 +10:00
parent a9e840988d
commit ebbdc946a1
7 changed files with 113 additions and 32 deletions

View File

@@ -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.");

View File

@@ -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<Collection, Guid>, 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<Guid> organizationUserIds, string defaultCollectionName)
@@ -417,6 +423,11 @@ public class CollectionRepository : Repository<Collection, Guid>, ICollectionRep
transaction.Commit();
}
catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex))
{
transaction.Rollback();
throw new DuplicateDefaultCollectionException();
}
catch
{
transaction.Rollback();

View File

@@ -0,0 +1,17 @@
namespace Bit.Infrastructure.Dapper.Repositories;
#nullable enable
internal static class DatabaseExceptionHelpers
{
/// <summary>
/// Determines if an exception represents a SQL Server duplicate key constraint violation.
/// </summary>
public static bool IsDuplicateKeyException(Exception exception)
{
ArgumentNullException.ThrowIfNull(exception);
return exception is Microsoft.Data.SqlClient.SqlException { Errors: not null } msEx &&
msEx.Errors.Cast<Microsoft.Data.SqlClient.SqlError>().Any(error => error.Number == 2627);
}
}

View File

@@ -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<Core.Entities.Collection, Collect
using var scope = ServiceScopeFactory.CreateScope();
var dbContext = GetDatabaseContext(scope);
// 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
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<IEnumerable<Collection>>(collections));
await dbContext.BulkCopyAsync(Mapper.Map<IEnumerable<CollectionUser>>(collectionUsers));
await dbContext.BulkCopyAsync(semaphores);
await dbContext.BulkCopyAsync(Mapper.Map<IEnumerable<Collection>>(collections));
await dbContext.BulkCopyAsync(Mapper.Map<IEnumerable<CollectionUser>>(collectionUsers));
await dbContext.SaveChangesAsync();
await dbContext.SaveChangesAsync();
}
catch (Exception ex) when (DatabaseExceptionHelpers.IsDuplicateKeyException(ex))
{
throw new DuplicateDefaultCollectionException();
}
}
public async Task CreateDefaultCollectionsBulkAsync(Guid organizationId, IEnumerable<Guid> organizationUserIds, string defaultCollectionName)

View File

@@ -0,0 +1,37 @@
using System.Data.Common;
using Microsoft.EntityFrameworkCore;
namespace Bit.Infrastructure.EntityFramework.Repositories;
#nullable enable
internal static class DatabaseExceptionHelpers
{
/// <summary>
/// Determines if a DbUpdateException represents a duplicate key constraint violation.
/// Works with MySQL, SQL Server, PostgreSQL, and SQLite.
/// </summary>
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<Microsoft.Data.SqlClient.SqlError>().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;
}
}
}

View File

@@ -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<Exception>(() =>
// Act - Try to create again, should throw specific duplicate collection exception
await Assert.ThrowsAsync<DuplicateDefaultCollectionException>(() =>
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<Exception>(() =>
await Assert.ThrowsAsync<DuplicateDefaultCollectionException>(() =>
collectionRepository.CreateDefaultCollectionsBulkAsync(
organization.Id,
[existingUser.Id, newUser.Id],

View File

@@ -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<Exception>(() =>
// Second call should throw specific exception and should not create duplicate
await Assert.ThrowsAsync<DuplicateDefaultCollectionException>(() =>
collectionRepository.CreateDefaultCollectionsAsync(
organization.Id,
[orgUser.Id],