[PM-28795] Refactor organization accept-init flow with atomic database operations and upfront validation #6831
Conversation
…nitialization Introduces a new method that consolidates organization initialization, user confirmation, email verification, and collection creation into a single operation with upfront validation. Key improvements: - All validation performed before any state changes - Returns typed errors via CommandResult instead of throwing exceptions - Reduces three separate command calls into one unified flow - Maintains backward compatibility via feature flag The existing InitPendingOrganizationAsync is marked obsolete and will be removed after feature flag rollout completes.
Introduces comprehensive unit tests for the InitPendingOrganizationVNextAsync method, covering various scenarios including valid data initialization, error handling for invalid tokens, organization status checks, and user confirmation requirements. Key additions: - Tests for successful organization initialization and user confirmation. - Error handling tests for invalid tokens, already enabled organizations, and mismatched organization IDs. - Validation for existing keys and email mismatches. - Support for creating default collections during initialization. These tests enhance the reliability and robustness of the organization initialization process.
…nitialization Updated the AcceptInit method in OrganizationUsersController to return an IResult type and handle organization initialization based on a feature flag. If the feature is enabled, it utilizes the new InitPendingOrganizationVNextAsync method for atomic organization setup and user confirmation. Integration tests were added to verify the behavior under both feature flag states, ensuring proper initialization and error handling.
…ture flag support Updated the ValidatePoliciesAsync method to enforce the Automatic User Confirmation Policy when the feature flag is enabled. Added new unit tests to cover scenarios for automatic user confirmation and single organization policy violations, ensuring comprehensive validation during organization initialization. This improves error handling and maintains compliance with organizational policies.
… initialization Implemented the InitializePendingOrganizationAsync method in both Dapper and Entity Framework repositories to streamline the initialization of pending organizations. This method enables the organization, confirms the first owner, updates user details, and optionally creates a default collection, all within a single atomic transaction. Enhanced error handling ensures robustness during the initialization process.
Introduced several unit tests for the InitializePendingOrganizationAsync method, covering scenarios such as successful organization initialization with and without collections, exception handling for invalid organization IDs, and rollback behavior on errors. These tests enhance the reliability of the organization initialization process and ensure proper handling of various edge cases.
…ePendingOrganizationAsync method Replaced multiple asynchronous calls for organization initialization with a single call to the new InitializePendingOrganizationAsync method. This change streamlines the process by encapsulating organization setup, user confirmation, and collection creation into one atomic operation, enhancing maintainability and reducing complexity in the command logic.
…efactor existing ones Added a new test case for InitPendingOrganizationVNextAsync to validate organization initialization with a collection name. Refactored existing tests to improve clarity and maintainability, including the removal of redundant assertions and the consolidation of organization setup logic. This update strengthens the test coverage for the organization initialization process and ensures proper handling of various scenarios.
|
Claude finished @r-tome's task in 6m 20s —— View job Code Review SummaryPR: PM-28795 Refactor organization accept-init flow with atomic database operations and upfront validation I have completed my review of this pull request. OverviewThis PR refactors the organization accept-init flow to use a delegate-based pattern for atomic database operations, following the
Findings
Review AssessmentNo new issues identified. The code is well-structured and addresses previous review feedback:
Previous thread resolution:
Code Quality
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| Guid organizationUserId, | ||
| Guid userId, | ||
| string userKey, | ||
| string collectionName) |
There was a problem hiding this comment.
CRITICAL: Method signature mismatch with interface
The EF implementation has string collectionName (non-nullable), but the interface IOrganizationRepository defines it as string? collectionName (nullable). This violates the Liskov Substitution Principle and will cause compilation errors.
The Dapper implementation at line 266 correctly uses string? collectionName.
Fix: Change to string? collectionName to match the interface.
| await transaction.CommitAsync(); | ||
| } | ||
| catch | ||
| { |
There was a problem hiding this comment.
IMPORTANT: Generic catch block may hide critical errors
The catch block re-throws all exceptions, which is good for preserving the exception chain. However, this generic catch could mask specific database-related errors (deadlocks, constraint violations, connection issues) that might need special handling or logging.
Consideration: Add logging before the rollback to capture the exception type and message for debugging atomic transaction failures. This is especially important since this operation involves multiple critical state transitions.
Example:
catch (Exception ex)
{
_logger.LogError(ex, "Failed to initialize pending organization {OrganizationId}. Rolling back transaction.", organizationId);
await transaction.RollbackAsync();
throw;
}
|
|
||
| await transaction.CommitAsync(); | ||
| } | ||
| catch |
There was a problem hiding this comment.
IMPORTANT: Generic catch block may hide critical errors
The catch block re-throws all exceptions, which is good for preserving the exception chain. However, this generic catch could mask specific database-related errors (EF exceptions, concurrency issues, constraint violations) that might need special handling or logging.
Consideration: Add logging before the rollback to capture the exception type and message for debugging atomic transaction failures. This is especially important since this operation involves multiple critical state transitions.
The Dapper implementation should also have the same logging improvement for consistency.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6831 +/- ##
==========================================
+ Coverage 56.06% 60.20% +4.14%
==========================================
Files 1969 1971 +2
Lines 87065 87415 +350
Branches 7755 7778 +23
==========================================
+ Hits 48811 52630 +3819
+ Misses 36446 32889 -3557
- Partials 1808 1896 +88 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
New Issues (2)Checkmarx found the following issues in this Pull Request
|
… nullable collectionName parameter Updated the IOrganizationRepository and OrganizationRepository interfaces to change the collectionName parameter from nullable to non-nullable. This change enforces stricter parameter requirements and improves data integrity during organization initialization processes.
| validationError = ValidateOrganizationState(org); | ||
| if (validationError != null) | ||
| { | ||
| return validationError; | ||
| } | ||
|
|
||
| validationError = await ValidatePoliciesAsync(user, organizationId, org, orgUser); | ||
| if (validationError != null) | ||
| { | ||
| return validationError; | ||
| } | ||
|
|
||
| await _organizationRepository.InitializePendingOrganizationAsync( |
There was a problem hiding this comment.
❌ CRITICAL: Race Condition Between Validation and Transaction
The organization state is validated at line 217-220 outside the database transaction that starts inside InitializePendingOrganizationAsync (line 229). Between validation and transaction start, another concurrent request could modify the organization state.
Attack scenario:
- Request A validates: org is Pending, no keys ✓
- Request B validates: org is Pending, no keys ✓
- Request A's transaction sets keys to KeysA
- Request B's transaction overwrites with KeysB
⚠️
Impact:
- Organization keys could be overwritten
- Already-enabled orgs could be re-initialized
- Data corruption in concurrent scenarios
Solution:
The repository method should re-validate the organization state within the transaction before making changes:
// Inside InitializePendingOrganizationAsync, after fetching org:
if (organization.Enabled)
{
throw new InvalidOperationException("Organization is already enabled.");
}
if (organization.Status != OrganizationStatusType.Pending)
{
throw new InvalidOperationException("Organization is not in Pending status.");
}
if (!string.IsNullOrEmpty(organization.PublicKey) || !string.IsNullOrEmpty(organization.PrivateKey))
{
throw new InvalidOperationException("Organization already has encryption keys.");
}This ensures the validation check happens under transaction isolation, preventing TOCTOU (Time-of-Check-Time-of-Use) race conditions.
| catch | ||
| { | ||
| await transaction.RollbackAsync(); | ||
| throw; | ||
| } |
There was a problem hiding this comment.
⚠️ IMPORTANT: Add Logging Before Transaction Rollback
The generic catch block doesn't log any information before rolling back the transaction. This makes debugging transaction failures very difficult in production.
Recommendation:
catch (Exception ex)
{
_logger.LogError(ex,
"Failed to initialize pending organization {OrganizationId}. Rolling back transaction.",
organizationId);
await transaction.RollbackAsync();
throw;
}Why this matters:
- Helps diagnose which step failed (org update, user update, collection creation)
- Captures the exception details before rethrow
- Provides context (organizationId) for correlation with other logs
- Critical for production incident response
This same pattern should be applied to the EF implementation as well (OrganizationRepository.cs line 536-540).
| catch | ||
| { | ||
| await transaction.RollbackAsync(); | ||
| throw; | ||
| } |
There was a problem hiding this comment.
⚠️ IMPORTANT: Add Logging Before Transaction Rollback
Same issue as the Dapper implementation - add logging before rollback:
catch (Exception ex)
{
_logger.LogError(ex,
"Failed to initialize pending organization {OrganizationId}. Rolling back transaction.",
organizationId);
await transaction.RollbackAsync();
throw;
}This provides critical debugging information when transaction failures occur in production.
| await dbContext.SaveChangesAsync(); | ||
|
|
||
| // 2. Update OrganizationUser | ||
| var organizationUser = await dbContext.OrganizationUsers.FindAsync(organizationUserId); | ||
| if (organizationUser == null) | ||
| { | ||
| throw new InvalidOperationException($"OrganizationUser with ID {organizationUserId} not found."); | ||
| } | ||
|
|
||
| organizationUser.Status = OrganizationUserStatusType.Confirmed; | ||
| organizationUser.UserId = userId; | ||
| organizationUser.Key = userKey; | ||
| organizationUser.Email = null; | ||
|
|
||
| await dbContext.SaveChangesAsync(); | ||
|
|
||
| // 3. Update User (verify email if needed) | ||
| var user = await dbContext.Users.FindAsync(userId); | ||
| if (user == null) | ||
| { | ||
| throw new InvalidOperationException($"User with ID {userId} not found."); | ||
| } | ||
|
|
||
| if (user.EmailVerified == false) | ||
| { | ||
| user.EmailVerified = true; | ||
| user.RevisionDate = DateTime.UtcNow; | ||
|
|
||
| await dbContext.SaveChangesAsync(); | ||
| } | ||
|
|
||
| // 4. Create default collection if name provided | ||
| if (!string.IsNullOrWhiteSpace(collectionName)) | ||
| { | ||
| var collection = new Collection | ||
| { | ||
| Id = CoreHelpers.GenerateComb(), | ||
| Name = collectionName, | ||
| OrganizationId = organizationId, | ||
| CreationDate = DateTime.UtcNow, | ||
| RevisionDate = DateTime.UtcNow | ||
| }; | ||
|
|
||
| await dbContext.Collections.AddAsync(collection); | ||
| await dbContext.SaveChangesAsync(); | ||
|
|
||
| // Create CollectionUser association with Manage permissions | ||
| var collectionUser = new CollectionUser | ||
| { | ||
| CollectionId = collection.Id, | ||
| OrganizationUserId = organizationUserId, | ||
| HidePasswords = false, | ||
| ReadOnly = false, | ||
| Manage = true | ||
| }; | ||
|
|
||
| await dbContext.CollectionUsers.AddAsync(collectionUser); | ||
| await dbContext.SaveChangesAsync(); |
There was a problem hiding this comment.
🎨 SUGGESTED: Optimize Entity Framework SaveChanges Pattern
The current implementation calls SaveChangesAsync() multiple times within the transaction (lines 474, 488, 502, 518, 531). While this works correctly, Entity Framework is designed to batch changes efficiently.
Current pattern:
// Update Organization
organization.Enabled = true;
await dbContext.SaveChangesAsync(); // Save #1
// Update OrganizationUser
organizationUser.Status = OrganizationUserStatusType.Confirmed;
await dbContext.SaveChangesAsync(); // Save #2
// ... etcOptimized pattern:
// Update Organization
organization.Enabled = true;
// (no save yet)
// Update OrganizationUser
organizationUser.Status = OrganizationUserStatusType.Confirmed;
// (no save yet)
// ... all other changes ...
// Single save before commit
await dbContext.SaveChangesAsync();
await transaction.CommitAsync();Benefits:
- Fewer database roundtrips
- Better performance
- EF's change tracker batches all operations efficiently
Note: This is a performance optimization, not a correctness issue. The current code works as expected.
…s during transaction rollback Updated the OrganizationRepository in both Dapper and Entity Framework implementations to log detailed error messages when exceptions occur during the initialization of pending organizations. This enhancement improves traceability and debugging capabilities by providing context on failures, ensuring better maintainability of the organization initialization process.
Updated the OrganizationRepository to reduce multiple SaveChangesAsync calls into a single call at the end of the transaction. This change enhances performance and ensures that all changes are committed atomically, improving the overall maintainability of the organization initialization process.
| var organization = (await connection.QueryAsync<Organization>( | ||
| "[dbo].[Organization_ReadById]", | ||
| new { Id = organizationId }, | ||
| commandType: CommandType.StoredProcedure, | ||
| transaction: transaction)).SingleOrDefault(); | ||
|
|
||
| if (organization == null) | ||
| { | ||
| throw new InvalidOperationException($"Organization with ID {organizationId} not found."); | ||
| } | ||
|
|
||
| organization.Enabled = true; | ||
| organization.Status = OrganizationStatusType.Created; | ||
| organization.PublicKey = publicKey; | ||
| organization.PrivateKey = privateKey; | ||
| organization.RevisionDate = DateTime.UtcNow; |
There was a problem hiding this comment.
CRITICAL - Race Condition: Organization state not validated within transaction
The organization state is validated in InitPendingOrganizationCommand BEFORE the transaction starts, but NOT re-validated here within the transaction. This creates a race condition:
Attack Scenario:
- Request A validates: org is pending, no keys ✓
- Request B validates: org is pending, no keys ✓
- Request A starts transaction, sets keys, commits
- Request B starts transaction, overwrites keys with different values
Impact:
- Organization keys could be overwritten by concurrent requests
- Users could be confirmed with wrong keys
- Breaks zero-knowledge encryption guarantees
Required Fix:
Add validation within the transaction after reading the organization:
var organization = (await connection.QueryAsync<Organization>(...)).SingleOrDefault();
if (organization == null)
{
throw new InvalidOperationException($"Organization with ID {organizationId} not found.");
}
// CRITICAL: Re-validate state within transaction to prevent race conditions
if (organization.Enabled)
{
throw new InvalidOperationException("Organization is already enabled.");
}
if (organization.Status != OrganizationStatusType.Pending)
{
throw new InvalidOperationException("Organization is not in Pending status.");
}
if (!string.IsNullOrEmpty(organization.PublicKey) || !string.IsNullOrEmpty(organization.PrivateKey))
{
throw new InvalidOperationException("Organization already has encryption keys.");
}
organization.Enabled = true;
// ... continue with updateAlternative: Use database-level constraints or optimistic concurrency with row versioning.
| var organization = await dbContext.Organizations.FindAsync(organizationId); | ||
| if (organization == null) | ||
| { | ||
| throw new InvalidOperationException($"Organization with ID {organizationId} not found."); | ||
| } | ||
|
|
||
| organization.Enabled = true; | ||
| organization.Status = OrganizationStatusType.Created; | ||
| organization.PublicKey = publicKey; | ||
| organization.PrivateKey = privateKey; | ||
| organization.RevisionDate = DateTime.UtcNow; |
There was a problem hiding this comment.
CRITICAL - Race Condition: Organization state not validated within transaction
Same critical race condition exists in the EF implementation. The organization state must be validated within the transaction after it's been read.
Required Fix:
var organization = await dbContext.Organizations.FindAsync(organizationId);
if (organization == null)
{
throw new InvalidOperationException($"Organization with ID {organizationId} not found.");
}
// CRITICAL: Re-validate state within transaction to prevent race conditions
if (organization.Enabled)
{
throw new InvalidOperationException("Organization is already enabled.");
}
if (organization.Status != OrganizationStatusType.Pending)
{
throw new InvalidOperationException("Organization is not in Pending status.");
}
if (!string.IsNullOrEmpty(organization.PublicKey) || !string.IsNullOrEmpty(organization.PrivateKey))
{
throw new InvalidOperationException("Organization already has encryption keys.");
}
organization.Enabled = true;
// ... continue with updateNote: EF Core's default behavior does NOT prevent this race condition even with transactions. Consider using:
- Explicit row locking:
dbContext.Organizations.FromSqlRaw("SELECT * FROM Organization WITH (UPDLOCK) WHERE Id = {0}", organizationId) - Or optimistic concurrency with
[Timestamp]attribute on Organization entity
| await _organizationRepository.InitializePendingOrganizationAsync( | ||
| organizationId, | ||
| publicKey, | ||
| privateKey, | ||
| organizationUserId, | ||
| user.Id, | ||
| userKey, | ||
| collectionName); | ||
|
|
||
| await SendNotificationsAsync(org, orgUser, user, organizationId); | ||
|
|
||
| return new None(); |
There was a problem hiding this comment.
WARNING - Gap between validation and transaction execution
The validations performed above (lines 200-227) occur BEFORE the repository transaction starts. While the repository now handles the database operations atomically, there's still a time gap where the organization state could change.
Current Flow:
- Line 189-227: Validate token, email, org state, policies (NO transaction)
- Line 229-236: Execute atomic repository transaction
Risk:
Between validation and execution, another request could initialize the organization. The repository implementations currently don't re-validate state within the transaction (see comments on repository files).
Recommendation:
This is acceptable IF the repository implementations add state validation within their transactions (as noted in separate comments). The validation here serves as a fast-fail mechanism, while repository validation ensures correctness.
Consider: Document this two-phase validation approach in the method summary to clarify that repository-level validation is required for correctness.
# Conflicts: # src/Core/Constants.cs
|
|
||
| private async Task SendNotificationsAsync(Organization org, OrganizationUser orgUser, User user, Guid organizationId) | ||
| { | ||
| await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); |
There was a problem hiding this comment.
The new flow only logs OrganizationUser_Confirmed but the original flow logged both OrganizationUser_Accepted and OrganizationUser_Confirmed events.
The original flow in the controller called:
InitPendingOrganizationAsync- no eventAcceptOrgUserByEmailTokenAsync- logsOrganizationUser_AcceptedConfirmUserAsync- logsOrganizationUser_Confirmed
This missing event could affect:
- Audit trails and compliance reporting
- Downstream systems that react to the
Acceptedevent - Historical data consistency between users onboarded before and after this change
Consider adding the missing event:
| await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); | |
| await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Accepted); | |
| await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); |
| Task<CommandResult> InitPendingOrganizationVNextAsync( | ||
| User user, Guid organizationId, Guid organizationUserId, string publicKey, | ||
| string privateKey, string collectionName, string emailToken, string userKey); |
There was a problem hiding this comment.
This should use a request model so that these can be named properties rather than a list of parameters. Avoids mixing up different strings, for example.
| Task InitializePendingOrganizationAsync( | ||
| Guid organizationId, | ||
| string publicKey, | ||
| string privateKey, | ||
| Guid organizationUserId, | ||
| Guid userId, | ||
| string userKey, | ||
| string collectionName); |
There was a problem hiding this comment.
This should also take an object.
There was a problem hiding this comment.
I considered this but I couldn't find that pattern elsewhere in our repositories, so keeping separate parameters seems more consistent (though I agree there are too many). Open to being convinced if you feel strongly about it though!
There was a problem hiding this comment.
Maybe the issue is that there are just too many parameters here. Repository calls usually take a database object (entity) or just a couple of params. This may parameters indicates that it's too complex. Once you split this up per my comment below, it'll probably be OK because it'll be comprised of 4 separate repository methods that will only have a few parameters each.
| commandType: CommandType.StoredProcedure); | ||
| } | ||
|
|
||
| public async Task InitializePendingOrganizationAsync( |
There was a problem hiding this comment.
This is a tricky one. A few thoughts...
- I think performing all write operations in one transaction makes sense - you want them to succeed or fail together
- however, with default isolation levels, I don't think there's any benefit to re-fetching the data within the transaction. And it moves a lot of the data arrangement down into the repository layer, giving it more responsibility beyond just interacting with the db.
- this touches on 4 different tables: Organization, OrganizationUser, User and Collections. But it's located in the Organization repository. We don't have any great way of handling this today, the best I've seen is how RotateUserAccountKeysCommand builds up the
saveEncryptedDataActionslist of delegates, which are defined by different repositories, then executes them in the same transaction. It has some boilerplate, but it keeps each repository in charge of its own table, which I think is worth it.
| transaction: transaction); | ||
| } | ||
|
|
||
| // 4. Create default collection if name provided |
There was a problem hiding this comment.
Unfortunate that we confused the naming for this...
| // 4. Create default collection if name provided | |
| // 4. Create default collection if name provided | |
| // (this is the initial collection for the organization, not a My Items collection) |
There was a problem hiding this comment.
Most of these integration tests are for the current implementation, which will be removed soon, and only 2 are for the new implementation. It's OK to have some tests for the current code (e.g. to ensure parity) but we should have at least as much coverage for the new implementation.
|
|
||
| public async Task<CommandResult> InitPendingOrganizationVNextAsync( |
There was a problem hiding this comment.
It may be worth clarifying/noting something like...
The user initializing the organization is the first user to access it - there is no existing owner or provider who can change its settings. Therefore, the validation in this command assumes a default state - for example, it does not enforce policies for this organization, because none will be enabled.
This took me a little bit of staring at the screen to realise.
There was a problem hiding this comment.
This command has a lot of validation, I think it would help to move it into a separate validator class. It makes for easier testing.
And especially if you move data arrangement back into the command, it separates the validation vs. the action of the command.
| if (org.PlanType == PlanType.Free && | ||
| (orgUser.Type == OrganizationUserType.Owner || orgUser.Type == OrganizationUserType.Admin)) | ||
| { | ||
| var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(user.Id); | ||
| if (adminCount > 0) | ||
| { | ||
| return new FreeOrgAdminLimitError(); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: this is in ValidatePoliciesAsync but this is not a policy.
| Task InitPendingOrganizationAsync(User user, Guid organizationId, Guid organizationUserId, string publicKey, string privateKey, string collectionName, string emailToken); | ||
|
|
||
| /// <summary> | ||
| /// Initializes a pending organization and confirms the first owner with upfront validation. |
There was a problem hiding this comment.
The business context for this is useful, including a link to the command earlier in this overall flow:
| /// Initializes a pending organization and confirms the first owner with upfront validation. | |
| /// Initializes a pending organization created via the Bitwarden Portal on behalf of a Reseller. | |
| /// See <see cref="ResellerClientOrganizationSignUpCommand"/>. | |
| /// It also confirms the first owner. |
# Conflicts: # src/Core/Constants.cs
…nitPendingOrganizationVNextAsync method - Created InitPendingOrganizationRequest to encapsulate parameters for initializing a pending organization. - Refactored InitPendingOrganizationVNextAsync method to accept the new request model instead of multiple parameters. - Updated OrganizationUsersController to use the new request model for improved readability and maintainability. - Adjusted related tests to accommodate the new request structure.
…tations in UserRepository classes - Introduced a new method in IUserRepository to create an action for verifying user emails. - Implemented the method in both Dapper and Entity Framework UserRepository classes to update the email verification status of users. - Ensured that the method checks if the user's email is already verified before updating.
…y and implementations in CollectionRepository classes - Introduced a new method in ICollectionRepository to build an action for creating a default collection with user access. - Implemented the method in both Dapper and Entity Framework CollectionRepository classes to handle collection creation and user access assignments. - Enhanced the functionality to support transaction execution for database operations.
…ository and implementations in OrganizationUserRepository classes - Introduced a new method in IOrganizationUserRepository to build an action for confirming an organization user. - Implemented the method in both Dapper and Entity Framework OrganizationUserRepository classes to handle user confirmation and status updates. - Enhanced the functionality to support transaction execution for database operations.
…ry and implementations - Introduced BuildUpdateOrganizationAction method to create an action for updating organization properties during initialization. - Replaced the InitializePendingOrganizationAsync method with ExecuteOrganizationInitializationUpdatesAsync to handle multiple update actions in a single transaction. - Updated Dapper and Entity Framework implementations to support the new action-based approach for organization initialization, enhancing transaction management and code clarity.
…itialization process - Introduced methods for preparing organization and organization user for initialization. - Replaced direct calls to repository methods with a new action-based approach for executing multiple database updates in a single transaction. - Enhanced test cases to validate the new initialization logic and ensure proper handling of organization states.
- Converted existing tests to use [Theory] with [InlineData] for feature flag variations. - Updated assertions to reflect expected status codes based on feature flag state. - Enhanced user confirmation checks to ensure proper linking and email verification after acceptance. - Improved test coverage for organization initialization scenarios with and without collections.
…user ID - Updated IUserRepository and its implementations to change the parameter of BuildVerifyUserEmailAction from Guid userId to User user. - Modified related repository methods in Dapper and Entity Framework to utilize the User entity for email verification. - Adjusted tests to reflect the new method signature, ensuring proper functionality and integration with the updated user verification process.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-28795
📔 Objective
📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes