Skip to content
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f54a77d
First pass
eliykat Dec 2, 2025
dd39159
Merge remote-tracking branch 'origin/main' into ac/pm-28555/server-crโ€ฆ
eliykat Dec 3, 2025
5fd5c51
Update EF method to use query syntax and GenerateComb
eliykat Dec 3, 2025
63b4f1b
Adjust repository methods
eliykat Dec 3, 2025
cecb022
clean up tests
eliykat Dec 3, 2025
3bbf375
fix CREATE OR ALTER
eliykat Dec 3, 2025
25189b8
set serializable transaction to avoid race condition
eliykat Dec 3, 2025
f5393fa
linting
eliykat Dec 3, 2025
1c03ef1
Configure EF to gracefully handle deadlocks
eliykat Dec 3, 2025
d001aa4
Merge remote-tracking branch 'origin/main' into ac/pm-28555/server-crโ€ฆ
eliykat Dec 4, 2025
a9b24d7
Merge remote-tracking branch 'origin/main' into ac/pm-28555/server-crโ€ฆ
eliykat Dec 17, 2025
6adb30e
Merge remote-tracking branch 'origin/main' into ac/pm-28555/server-crโ€ฆ
eliykat Dec 20, 2025
0a28dbf
First pass at unique constraint
eliykat Dec 20, 2025
184677d
add xmldoc
eliykat Dec 20, 2025
b0a7884
Undo previous changes to EF configuration
eliykat Dec 20, 2025
a3369ab
Add constraint in EF
eliykat Dec 20, 2025
868ecf0
Resolve competing cascade action: manually set FKID to null
eliykat Dec 20, 2025
ed49d7d
dotnet format
eliykat Dec 20, 2025
7577fa6
fix sql style
eliykat Dec 20, 2025
aad01ec
Remove OR ALTER from sproc
eliykat Dec 20, 2025
dc1dda6
add todo
eliykat Dec 20, 2025
8582ab3
Merge remote-tracking branch 'origin/main' into ac/pm-28555/server-crโ€ฆ
eliykat Dec 26, 2025
5c56767
Rename DefaultCollectionOwner -> DefaultCollectionOwnerId
eliykat Dec 26, 2025
e2cd3c6
EF migrations
eliykat Dec 26, 2025
afaaea0
First pass at handling existing data and duplicates
eliykat Dec 26, 2025
b6ed81e
Catch errors from all db types
eliykat Dec 27, 2025
dca7695
Fix filter in EF dbs
eliykat Dec 27, 2025
1f931bd
Fix bulk tests
eliykat Dec 27, 2025
ce7d727
Move new sproc to separate PR
eliykat Dec 27, 2025
03690cf
dotnet format
eliykat Dec 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Models.Data;
using Bit.Core.Platform.Push;
using Bit.Core.Repositories;
using Bit.Core.Services;
Expand Down Expand Up @@ -79,19 +77,10 @@ private async Task CreateDefaultCollectionsAsync(AutomaticallyConfirmOrganizatio
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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data;
using Bit.Core.Platform.Push;
using Bit.Core.Repositories;
using Bit.Core.Services;
Expand Down Expand Up @@ -297,21 +296,10 @@ private async Task CreateDefaultCollectionAsync(OrganizationUser organizationUse
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);
}

/// <summary>
Expand Down
50 changes: 50 additions & 0 deletions src/Core/Entities/Collection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,66 @@ namespace Bit.Core.Entities;

public class Collection : ITableObject<Guid>
{
/// <summary>
/// The unique identifier for the collection.
/// </summary>
public Guid Id { get; set; }

/// <summary>
/// The identifier of the organization that owns this collection.
/// </summary>
public Guid OrganizationId { get; set; }

/// <summary>
/// The name of the collection, encrypted with the organization symmetric key.
/// </summary>
public string Name { get; set; } = null!;

/// <summary>
/// An external identifier for integration with external systems.
/// </summary>
[MaxLength(300)]
public string? ExternalId { get; set; }

/// <summary>
/// The date and time when the collection was created.
/// </summary>
public DateTime CreationDate { get; set; } = DateTime.UtcNow;

/// <summary>
/// The date and time when the collection was last revised.
/// </summary>
public DateTime RevisionDate { get; set; } = DateTime.UtcNow;

/// <summary>
/// The type of collection - see <see cref="CollectionType"/>.
/// </summary>
public CollectionType Type { get; set; } = CollectionType.SharedCollection;

/// <summary>
/// The email address for the OrganizationUser associated with a DefaultUserCollection collection type.
/// </summary>
/// <remarks>
/// This is only populated at the time the OrganizationUser leaves or is removed from the organization.
/// It is then used as the collection name so that administrators can identify the collection.
/// It is null for all other collection types and at all other times.
/// </remarks>
public string? DefaultUserCollectionEmail { get; set; }

/// <summary>
/// The ID for the OrganizationUser associated with this default collection.
/// INTERNAL DATABASE USE ONLY - DO NOT USE - SEE REMARKS.
/// </summary>
/// <remarks>
/// This is used to enforce uniqueness so that an OrganizationUser can only have 1 default collection
/// in a given organization.
/// It should NOT be used for any other purpose, used in the application code, or exposed to the front-end.
/// In particular, refer to the <see cref="CollectionUser"/> to evaluate user assignment and permissions.
/// Only populated for collections of type DefaultUserCollection.
/// Set to null when the OrganizationUser entry is deleted.
/// </remarks>
public Guid? DefaultCollectionOwner { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public Guid? DefaultCollectionOwner { get; set; }
public Guid? DefaultCollectionOwnerId { get; set; }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


public void SetNewId()
{
Id = CoreHelpers.GenerateComb();
Expand Down
10 changes: 10 additions & 0 deletions src/Core/Enums/CollectionType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

public enum CollectionType
{
/// <summary>
/// A standard collection used to share items within an organization.
/// </summary>
SharedCollection = 0,

/// <summary>
/// The default collection for an OrganizationUser, called a "My Items" collection in the product.
/// This is used by an OrganizationUser to store their items within the organization and is not
/// accessible by other members.
/// It is only created if the Organization Data Ownership policy is enabled.
/// </summary>
DefaultUserCollection = 1,
}
10 changes: 10 additions & 0 deletions src/Core/Repositories/ICollectionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,14 @@ Task CreateOrUpdateAccessForManyAsync(Guid organizationId, IEnumerable<Guid> col
/// <param name="defaultCollectionName">The encrypted string to use as the default collection name.</param>
/// <returns></returns>
Task UpsertDefaultCollectionsAsync(Guid organizationId, IEnumerable<Guid> organizationUserIds, string defaultCollectionName);

/// <summary>
/// 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.
/// </summary>
/// <param name="organizationId">The Organization ID.</param>
/// <param name="organizationUserId">The Organization User ID to create/find a default collection for.</param>
/// <param name="defaultCollectionName">The encrypted string to use as the default collection name.</param>
/// <returns>True if a new collection was created; false if the user already had a default collection.</returns>
Task<bool> UpsertDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName);
}
28 changes: 26 additions & 2 deletions src/Infrastructure.Dapper/Repositories/CollectionRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,30 @@ public async Task UpsertDefaultCollectionsAsync(Guid organizationId, IEnumerable
}
}

public async Task<bool> UpsertDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName)
{
using (var connection = new SqlConnection(ConnectionString))
{
var collectionId = CoreHelpers.GenerateComb();
var now = DateTime.UtcNow;
var parameters = new DynamicParameters();
parameters.Add("@CollectionId", collectionId);
parameters.Add("@OrganizationId", organizationId);
parameters.Add("@OrganizationUserId", organizationUserId);
parameters.Add("@Name", defaultCollectionName);
parameters.Add("@CreationDate", now);
parameters.Add("@RevisionDate", now);
parameters.Add("@WasCreated", dbType: DbType.Boolean, direction: ParameterDirection.Output);

await connection.ExecuteAsync(
$"[{Schema}].[Collection_UpsertDefaultCollection]",
parameters,
commandType: CommandType.StoredProcedure);

return parameters.Get<bool>("@WasCreated");
}
}

private async Task<HashSet<Guid>> GetOrgUserIdsWithDefaultCollectionAsync(SqlConnection connection, SqlTransaction transaction, Guid organizationId)
{
const string sql = @"
Expand Down Expand Up @@ -438,8 +462,8 @@ INNER JOIN
CreationDate = DateTime.UtcNow,
RevisionDate = DateTime.UtcNow,
Type = CollectionType.DefaultUserCollection,
DefaultUserCollectionEmail = null

DefaultUserCollectionEmail = null,
DefaultCollectionOwner = orgUserId
});

collectionUsers.Add(new CollectionUser
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
๏ปฟusing Bit.Infrastructure.EntityFramework.Models;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace Bit.Infrastructure.EntityFramework.AdminConsole.Configurations;

public class CollectionEntityTypeConfiguration : IEntityTypeConfiguration<Collection>
{
public void Configure(EntityTypeBuilder<Collection> builder)
{
builder
.Property(c => c.Id)
.ValueGeneratedNever();

builder
.HasIndex(c => new { c.DefaultCollectionOwner, c.OrganizationId, c.Type })
.IsUnique()
.HasFilter("[Type] = 1")
.HasDatabaseName("IX_Collection_DefaultCollectionOwner_OrganizationId_Type");

// Configure FK with NO ACTION delete behavior to prevent cascade conflicts
// Cleanup is handled explicitly in OrganizationUserRepository.DeleteAsync
builder
.HasOne<OrganizationUser>()
.WithMany()
.HasForeignKey(c => c.DefaultCollectionOwner)
.OnDelete(DeleteBehavior.NoAction);

builder.ToTable(nameof(Collection));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ await dbContext.Collections
.SetProperty(c => c.Type, CollectionType.SharedCollection)
.SetProperty(c => c.RevisionDate, utcNow)
.SetProperty(c => c.DefaultUserCollectionEmail,
c => c.DefaultUserCollectionEmail == null ? email : c.DefaultUserCollectionEmail));
c => c.DefaultUserCollectionEmail == null ? email : c.DefaultUserCollectionEmail)
.SetProperty(c => c.DefaultCollectionOwner, (Guid?)null));

await dbContext.CollectionUsers
.Where(cu => cu.OrganizationUserId == organizationUser.Id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,63 @@ public async Task UpsertDefaultCollectionsAsync(Guid organizationId, IEnumerable
await dbContext.SaveChangesAsync();
}

public async Task<bool> UpsertDefaultCollectionAsync(Guid organizationId, Guid organizationUserId, string defaultCollectionName)
{
using var scope = ServiceScopeFactory.CreateScope();
var dbContext = GetDatabaseContext(scope);

try
{
// Create new default collection
var collectionId = CoreHelpers.GenerateComb();
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,
DefaultCollectionOwner = organizationUserId
};

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;
}
catch (DbUpdateException ex) when (IsUniqueConstraintViolation(ex))
{
// Collection already exists, return false
return false;
}
}

private static bool IsUniqueConstraintViolation(DbUpdateException ex)
{
// Check if the inner exception is a SqlException with error 2601 or 2627
return ex.InnerException is Microsoft.Data.SqlClient.SqlException sqlEx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works for SQL, we should either add the other supported DB types or just return false for any DbUpdateException

Copy link
Member Author

@eliykat eliykat Dec 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this but I ended up removing the sproc from this PR. It became too large. I'm still figuring out the best order to do things in.

&& (sqlEx.Number == 2601 || sqlEx.Number == 2627);
}

private async Task<HashSet<Guid>> GetOrgUserIdsWithDefaultCollectionAsync(DatabaseContext dbContext, Guid organizationId)
{
var results = await dbContext.OrganizationUsers
Expand Down Expand Up @@ -861,8 +918,8 @@ private async Task<HashSet<Guid>> GetOrgUserIdsWithDefaultCollectionAsync(Databa
CreationDate = DateTime.UtcNow,
RevisionDate = DateTime.UtcNow,
Type = CollectionType.DefaultUserCollection,
DefaultUserCollectionEmail = null

DefaultUserCollectionEmail = null,
DefaultCollectionOwner = orgUserId
});

collectionUsers.Add(new CollectionUser
Expand Down
Loading
Loading