Implement user management features with Keycloak integration#36
Implement user management features with Keycloak integration#36zribktad wants to merge 7 commits intofeature/usiness-logic-in-handlersfrom
Conversation
…tion, updates, and deletions - Added ChangeUserRoleCommand and its handler for changing user roles. - Introduced CreateUserCommand and handler for user creation with validation. - Implemented UpdateUserCommand for updating user details. - Added DeleteUserCommand for user deletion with Keycloak integration. - Created GetUserByIdQuery and GetUsersQuery for fetching user details. - Implemented KeycloakPasswordResetCommand for triggering password reset emails. - Added specifications for filtering users by email and username. - Introduced validators for user requests and filters. - Enhanced error handling and logging throughout user management features. - Updated global usings and removed unused references in Identity module.
…ead letter endpoints with authorization
…stributed consistency
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the user creation flow to implement a durable outbox pattern, ensuring distributed consistency between the local database and Keycloak. Key changes include the introduction of the ProvisionKeycloakUserEvent for asynchronous provisioning, the implementation of idempotency in the Keycloak administration service, and the reorganization of the Identity module's namespaces. Additionally, the PR adds comprehensive documentation regarding Wolverine message handling and updates the project's error handling policies. Feedback was provided regarding a version mismatch for the WolverineFx.Http package relative to other WolverineFx dependencies.
| <PackageVersion Include="WolverineFx" Version="5.27.0" /> | ||
| <PackageVersion Include="WolverineFx.EntityFrameworkCore" Version="5.27.0" /> | ||
| <PackageVersion Include="WolverineFx.FluentValidation" Version="5.22.0" /> | ||
| <PackageVersion Include="WolverineFx.Http" Version="5.24.0" /> |
There was a problem hiding this comment.
The version of WolverineFx.Http (5.24.0) is not aligned with other WolverineFx packages in this file, which are at version 5.27.0 (e.g., WolverineFx, WolverineFx.EntityFrameworkCore, WolverineFx.Postgresql). Using mismatched versions of packages from the same library can lead to subtle runtime errors or compatibility issues. It's recommended to use the same version for all related packages to ensure compatibility.
<PackageVersion Include="WolverineFx.Http" Version="5.27.0" />
There was a problem hiding this comment.
Pull request overview
This PR enhances the Identity module’s user management by shifting Keycloak user provisioning to a Wolverine durable-outbox workflow (DB-first, Keycloak-later), adding dead-letter tooling, and consolidating user feature types under a single namespace to simplify usage and validation.
Changes:
- Refactors user creation to persist
AppUserfirst and publish a durableProvisionKeycloakUserEvent, handled asynchronously to create/link the Keycloak account. - Adds Wolverine dead-letter HTTP endpoints + retry policy for transient
HttpRequestExceptionfailures, plus documentation on Wolverine message handling. - Normalizes Identity user feature namespaces (DTOs/validators/specifications/mappings) and adds helper validation utilities + unit tests for provisioning/create flows.
Reviewed changes
Copilot reviewed 39 out of 42 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| TODO.md | Documents distributed consistency risks and proposed fixes. |
| tests/APITemplate.Tests/Unit/Identity/ProvisionKeycloakUserHandlerTests.cs | Adds unit tests for outbox-driven Keycloak provisioning handler. |
| tests/APITemplate.Tests/Unit/Identity/CreateUserCommandHandlerTests.cs | Adds unit tests for new DB-first user creation + provisioning event emission. |
| src/Modules/Identity/Security/Keycloak/KeycloakAdminService.cs | Makes Keycloak user creation idempotent on 409 conflict via username lookup. |
| src/Modules/Identity/Repositories/UserRepository.cs | Removes now-unneeded using after namespace consolidation. |
| src/Modules/Identity/Logging/IdentityLogs.cs | Updates logging events for provisioning + password reset flows. |
| src/Modules/Identity/IdentityModule.cs | Removes obsolete validation namespace import. |
| src/Modules/Identity/Identity.Api.GlobalUsings.cs | Removes DTOs global using after namespace consolidation. |
| src/Modules/Identity/Features/User/UpdateUser/UpdateUserRequestValidator.cs | Moves validator into consolidated Identity.Features.User namespace. |
| src/Modules/Identity/Features/User/UpdateUser/UpdateUserRequest.cs | Moves request DTO into consolidated namespace. |
| src/Modules/Identity/Features/User/UpdateUser/UpdateUserCommand.cs | Splits validation into ValidateAsync and injects result into HandleAsync. |
| src/Modules/Identity/Features/User/Shared/UserValidationHelper.cs | Introduces shared uniqueness validation helpers for email/username. |
| src/Modules/Identity/Features/User/Shared/UserResponse.cs | Moves response DTO into consolidated namespace. |
| src/Modules/Identity/Features/User/Shared/UserMappings.cs | Moves mappings into consolidated namespace. |
| src/Modules/Identity/Features/User/Shared/UserByUsernameSpecification.cs | Moves specification into consolidated namespace. |
| src/Modules/Identity/Features/User/Shared/UserByEmailSpecification.cs | Moves specification into consolidated namespace. |
| src/Modules/Identity/Features/User/SetUserActive/SetUserActiveCommand.cs | Adds ValidateAsync and injects user into handler. |
| src/Modules/Identity/Features/User/ProvisionKeycloakUser/ProvisionKeycloakUserHandler.cs | Adds durable outbox handler to create Keycloak user + link back to AppUser. |
| src/Modules/Identity/Features/User/ProvisionKeycloakUser/ProvisionKeycloakUserEvent.cs | Adds outbox event contract for Keycloak provisioning. |
| src/Modules/Identity/Features/User/KeycloakPasswordReset/RequestPasswordResetRequest.cs | Moves request DTO into consolidated namespace. |
| src/Modules/Identity/Features/User/KeycloakPasswordReset/KeycloakPasswordResetCommand.cs | Adds password-reset command handler calling Keycloak admin service. |
| src/Modules/Identity/Features/User/GetUsers/UserSortFields.cs | Adds sortable field map for user queries. |
| src/Modules/Identity/Features/User/GetUsers/UserFilterValidator.cs | Moves validator into consolidated namespace. |
| src/Modules/Identity/Features/User/GetUsers/UserFilterSpecification.cs | Moves specification into consolidated namespace and removes redundant using. |
| src/Modules/Identity/Features/User/GetUsers/UserFilterCriteria.cs | Moves criteria helper into consolidated namespace. |
| src/Modules/Identity/Features/User/GetUsers/UserFilter.cs | Moves filter DTO into consolidated namespace. |
| src/Modules/Identity/Features/User/GetUsers/GetUsersQuery.cs | Removes redundant specifications using after consolidation. |
| src/Modules/Identity/Features/User/GetUserById/UserByIdSpecification.cs | Moves specification into consolidated namespace and removes redundant using. |
| src/Modules/Identity/Features/User/GetUserById/GetUserByIdQuery.cs | Removes redundant specifications using after consolidation. |
| src/Modules/Identity/Features/User/DeleteUser/DeleteUserCommand.cs | Adds ValidateAsync and injects user into handler. |
| src/Modules/Identity/Features/User/CreateUser/CreateUserRequestValidator.cs | Moves validator into consolidated namespace. |
| src/Modules/Identity/Features/User/CreateUser/CreateUserRequest.cs | Moves request DTO into consolidated namespace. |
| src/Modules/Identity/Features/User/CreateUser/CreateUserCommand.cs | Replaces create flow with DB-first + outbox provisioning event emission. |
| src/Modules/Identity/Features/User/Commands/CreateUserCommand.cs | Removes old Keycloak-first create flow and compensating delete logic. |
| src/Modules/Identity/Features/User/ChangeUserRole/ChangeUserRoleRequestValidator.cs | Moves validator into consolidated namespace. |
| src/Modules/Identity/Features/User/ChangeUserRole/ChangeUserRoleRequest.cs | Moves request DTO into consolidated namespace. |
| src/Modules/Identity/Features/User/ChangeUserRole/ChangeUserRoleCommand.cs | Adds ValidateAsync and injects user into handler. |
| src/Modules/Identity/Entities/AppUser.cs | Adds LinkKeycloak() to safely set Keycloak linkage once. |
| src/APITemplate/Api/Program.cs | Adds Wolverine HTTP + dead-letter endpoints + retry/dead-letter policy for HttpRequestException. |
| src/APITemplate/Api/APITemplate.csproj | Adds WolverineFx.Http dependency. |
| docs/wolverine-message-handling.md | Documents inline vs outbox execution, retries, and dead-letter behavior. |
| Directory.Packages.props | Pins WolverineFx.Http package version. |
Comments suppressed due to low confidence (4)
src/Modules/Identity/Features/User/UpdateUser/UpdateUserCommand.cs:64
UpdateUserCommandHandlerhas been refactored intoValidateAsync+HandleAsyncwith a newHandleAsyncsignature (now takesErrorOr<AppUser> userResultand no longer acceptsIMessageBus). There are existing unit tests/callers still invoking the oldHandleAsync(command, repository, unitOfWork, bus, ct)signature (e.g.,tests/APITemplate.Tests/Unit/Handlers/UserRequestHandlersTests.cs:298+), which will fail to compile until updated to the new pattern.
src/Modules/Identity/Features/User/SetUserActive/SetUserActiveCommand.cs:28SetUserActiveCommandHandler.HandleAsyncsignature changed to require an injectedErrorOr<AppUser> userResult(fromValidateAsync) and no longer accepts anIMessageBus. Existing unit tests/callers are still invoking the old signature with a bus parameter (e.g.,tests/APITemplate.Tests/Unit/Handlers/UserRequestHandlersTests.cs:399+), which will break compilation until updated to the new validation + handle pattern.
src/Modules/Identity/Features/User/DeleteUser/DeleteUserCommand.cs:31DeleteUserCommandHandler.HandleAsyncsignature changed to require an injectedErrorOr<AppUser> userResult(fromValidateAsync) and no longer accepts anIMessageBus. Existing unit tests/callers are still invoking the old signature with a bus parameter (e.g.,tests/APITemplate.Tests/Unit/Handlers/UserRequestHandlersTests.cs:521+), which will break compilation until updated.
src/Modules/Identity/Features/User/ChangeUserRole/ChangeUserRoleCommand.cs:27ChangeUserRoleCommandHandler.HandleAsyncsignature changed to require an injectedErrorOr<AppUser> userResult(fromValidateAsync) and no longer acceptsIMessageBus/ILogger. Existing unit tests/callers are still invoking the old signature with bus/logger parameters (e.g.,tests/APITemplate.Tests/Unit/Handlers/UserRequestHandlersTests.cs:470+), which will fail to compile until updated.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Wolverine lifecycle method — runs before HandleAsync; result is injected as a parameter | ||
| public static async Task<ErrorOr<Success>> ValidateAsync( | ||
| CreateUserCommand command, | ||
| IUserRepository repository, | ||
| CancellationToken ct | ||
| ) | ||
| { | ||
| ErrorOr<Success> emailResult = await UserValidationHelper.ValidateEmailUniqueAsync( | ||
| repository, | ||
| command.Request.Email, | ||
| ct | ||
| ); | ||
| if (emailResult.IsError) | ||
| return emailResult.Errors; | ||
|
|
||
| return await UserValidationHelper.ValidateUsernameUniqueAsync( | ||
| repository, | ||
| command.Request.Username, | ||
| ct | ||
| ); | ||
| } | ||
|
|
||
| public static async Task<(ErrorOr<UserResponse>, OutgoingMessages)> HandleAsync( | ||
| CreateUserCommand command, | ||
| IUserRepository repository, | ||
| IUnitOfWork<IdentityDbMarker> unitOfWork, | ||
| ErrorOr<Success> validation, | ||
| CancellationToken ct | ||
| ) |
There was a problem hiding this comment.
CreateUserCommandHandler now uses a ValidateAsync lifecycle method and a different HandleAsync signature. There are existing unit tests/callers still using the old signature that accepted IMessageBus, ILogger, and IKeycloakAdminService (e.g., tests/APITemplate.Tests/Unit/Handlers/UserRequestHandlersTests.cs:162+), which will fail to compile until updated to call ValidateAsync and pass its result into HandleAsync (or switch to Wolverine invocation).
| [LoggerMessage( | ||
| EventId = 2001, | ||
| Level = LogLevel.Error, | ||
| Message = "DB save failed after creating Keycloak user {KeycloakUserId}. Attempting compensating delete." | ||
| Level = LogLevel.Information, | ||
| Message = "Provisioned Keycloak account for AppUser {UserId} — KeycloakUserId={KeycloakUserId}" | ||
| )] | ||
| public static partial void CreateUserDbSaveFailed( | ||
| public static partial void KeycloakUserProvisioned( | ||
| this ILogger logger, | ||
| Exception exception, | ||
| [SensitiveData] string keycloakUserId | ||
| Guid userId, | ||
| string keycloakUserId | ||
| ); |
There was a problem hiding this comment.
IdentityLogs.KeycloakUserProvisioned logs KeycloakUserId without [SensitiveData], but other Identity logs treat Keycloak IDs/bodies as sensitive (e.g., DeleteUserDbDeleteFailed uses [SensitiveData]). If log redaction is expected for Keycloak identifiers, mark keycloakUserId as [SensitiveData] here as well to keep telemetry consistent.
| if (response.StatusCode == HttpStatusCode.Conflict) | ||
| { | ||
| // Idempotent retry: user already exists in Keycloak (e.g. handler ran twice due to | ||
| // outbox retry after a DB commit failure). Fetch the existing user ID by username. | ||
| keycloakUserId = await GetExistingUserIdByUsernameAsync(username, ct); | ||
| _logger.KeycloakUserCreated(username, keycloakUserId); | ||
| return keycloakUserId; | ||
| } |
There was a problem hiding this comment.
In CreateUserAsync, a 409 Conflict path logs _logger.KeycloakUserCreated(...) even though the user was not created in this execution (it already existed). This makes operational logs misleading during idempotent retries. Consider adding a distinct log message/event for the conflict/idempotent path (e.g., “Keycloak user already exists, resolved id”) or adjusting the existing log message to reflect the actual behavior.
| <PackageVersion Include="WolverineFx" Version="5.27.0" /> | ||
| <PackageVersion Include="WolverineFx.EntityFrameworkCore" Version="5.27.0" /> | ||
| <PackageVersion Include="WolverineFx.FluentValidation" Version="5.22.0" /> | ||
| <PackageVersion Include="WolverineFx.Http" Version="5.24.0" /> | ||
| <PackageVersion Include="WolverineFx.Postgresql" Version="5.27.0" /> |
There was a problem hiding this comment.
WolverineFx.Http is pinned to 5.24.0 while other Wolverine packages are at 5.27.0. This version skew can cause subtle runtime/compile incompatibilities if the packages expect the same shared abstractions. If possible, align WolverineFx.Http to the same WolverineFx minor/patch version used elsewhere (or document why it must differ).
…actor user command handlers for consistency
- Introduced a new enum `ProvisioningStatus` to track the provisioning state of external identity provider accounts. - Updated the `AppUser` entity to include a `ProvisioningStatus` field with a default value of "Pending". - Created a migration to add the `ProvisioningStatus` field to the `Users` table and ensure proper constraints and indices. - Added corresponding migration files for the new schema changes. refactor(productCatalog): enhance logging for orphaned product data cleanup - Updated logging messages for orphaned product data cleanup to provide clearer context. - Introduced new logging methods to track the marking and sweeping of orphaned product data. chore(migrations): add PendingDeletion field to product_data collection - Created a MongoDB migration to backfill existing `product_data` documents with a `PendingDeletion` field set to false. - Implemented methods to handle the addition and removal of the `PendingDeletion` field in the database.
…validation in CreateUserCommand
Enhance user management capabilities by implementing user provisioning, role changes, and CRUD operations. Introduce commands for user creation, updates, deletions, and password resets, along with improved error handling and logging. Add WolverineFx.Http package for dead letter endpoints and update the application configuration accordingly. Document identified issues and proposed fixes for distributed consistency.