Feature/issue 2346 #2637
Conversation
YaroslavRosnovskyi
requested review from
IceStorman,
Michael-Kolpakov,
YuliiaAndreieva,
ZhmudAnastasiia,
ihor-yatsyshyn,
ilko-dev,
vlad-zhadan and
yaroslav-vasylyshyn
WalkthroughThis pull request adds new email confirmation and token validation features. It introduces a new constant value for email confirmation titles, new DTOs for handling email confirmation and token validation, and MediatR commands/handlers to support these functionalities. The authentication flow has been updated in login, registration, and Google login handlers to incorporate email verification, including generating and processing confirmation tokens. Custom token providers for email confirmation and password reset have been implemented, and resource files have been updated accordingly. Integration and unit tests now reflect the confirmed email status for users. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant MediatR
participant ConfirmEmailHandler
participant UserManager
Client->>AuthController: POST /confirmEmail (ConfirmEmailDTO)
AuthController->>MediatR: Send ConfirmEmailCommand
MediatR->>ConfirmEmailHandler: Dispatch command
ConfirmEmailHandler->>UserManager: Find user and confirm token
UserManager-->>ConfirmEmailHandler: Return user (or null)
ConfirmEmailHandler-->>MediatR: Result (success/failure)
MediatR-->>AuthController: Command result
AuthController-->>Client: Response
sequenceDiagram
participant Client
participant AuthController
participant MediatR
participant ValidateTokenHandler
participant UserManager
Client->>AuthController: POST /validateToken (ValidateTokenDto)
AuthController->>MediatR: Send ValidateTokenCommand
MediatR->>ValidateTokenHandler: Dispatch command
ValidateTokenHandler->>UserManager: Retrieve user and verify token
UserManager-->>ValidateTokenHandler: Return user (or null)
ValidateTokenHandler-->>MediatR: Result (valid/invalid)
MediatR-->>AuthController: Command result
AuthController-->>Client: Response
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProviderOptions.cs (1)
1-8: Overall, the new class is well-structured.
This file introduces theCustomPasswordResetTokenProviderOptionsclass by extendingDataProtectionTokenProviderOptions, making it ready for use with custom token lifespans—such as the 15-minute expiration requirement. Although the class is currently empty, its design allows for future extensions, and its simplicity keeps things clear. For improved maintainability, consider adding XML documentation to explain its intended use.Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProvider.cs (1)
1-19: Implementation looks clean but consider adding documentationThe custom token provider implementation follows proper inheritance patterns for ASP.NET Core Identity token providers. The constructor correctly passes all parameters to the base class.
Since this is a specialized token provider that will handle password reset functionality with a 15-minute expiration (as mentioned in the PR objectives), I'd recommend adding XML documentation comments to clarify:
- The purpose of this provider
- How it differs from the default token provider
- The expected token lifetime
This would help other developers understand why this custom implementation exists rather than using the built-in providers.
+/// <summary> +/// Custom token provider for password reset functionality with a 15-minute expiration. +/// </summary> +/// <typeparam name="TUser">The type representing a user in the system</typeparam> public class CustomPasswordResetTokenProvider<TUser> : DataProtectorTokenProvider<TUser> where TUser : class { + /// <summary> + /// Initializes a new instance of the <see cref="CustomPasswordResetTokenProvider{TUser}"/> class. + /// </summary> + /// <param name="dataProtectionProvider">The data protection provider used to protect the tokens.</param> + /// <param name="options">The options for this token provider.</param> + /// <param name="logger">The logger used for logging.</param> public CustomPasswordResetTokenProvider( IDataProtectionProvider dataProtectionProvider, IOptions<CustomPasswordResetTokenProviderOptions> options, ILogger<DataProtectorTokenProvider<TUser>> logger) : base(dataProtectionProvider, options, logger) { } }Streetcode/Streetcode.BLL/Resources/SharedResource.UserSharedResource.en-US.resx (1)
147-149: Minor grammatical issue in the email confirmation messageThe resource message has a small grammatical error: "We have resent you email" should be "We have resent your email" to properly indicate possession.
- <value>Email not confirmed. We have resent you email</value> + <value>Email not confirmed. We have resent your email</value>Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProviderOptions.cs (1)
5-7: Consider adding XML documentationWhile the class implementation is correct and supports the PR objective of adding token expiration, adding XML documentation would improve developer understanding of why this class exists and how it contributes to the 15-minute token expiration feature mentioned in the PR objectives.
+ /// <summary> + /// Options for the custom email confirmation token provider. + /// Allows configuring specific settings for email confirmation tokens separate from other token types. + /// </summary> public class CustomEmailConfirmationTokenProviderOptions : DataProtectionTokenProviderOptions { }Streetcode/Streetcode.BLL/DTO/Authentication/ValidateToken/ValidateTokenDto.cs (1)
3-12: Add documentation and consider validation attributesThis DTO is structured well with appropriate default values. For improved clarity and robustness, consider adding XML documentation and validation attributes for required fields.
+ /// <summary> + /// Data transfer object for token validation requests. + /// </summary> public class ValidateTokenDto { + /// <summary> + /// The token to validate. + /// </summary> + [Required(ErrorMessage = "Token is required")] public string Token { get; set; } = string.Empty; + /// <summary> + /// The purpose of the token (e.g., EmailConfirmation, ResetPassword). + /// </summary> + [Required(ErrorMessage = "Purpose is required")] public string Purpose { get; set; } = string.Empty; + /// <summary> + /// The username associated with the token. + /// </summary> + [Required(ErrorMessage = "UserName is required")] public string UserName { get; set; } = string.Empty; + /// <summary> + /// The provider used to generate the token. + /// </summary> public string TokenProvider { get; set; } = "Default"; }Don't forget to add the necessary using statement:
using System.ComponentModel.DataAnnotations;Streetcode/Streetcode.BLL/Models/Email/Messages/ConfirmEmailMessageData.cs (2)
9-12: Consider validating the Token propertyThe
Tokenproperty doesn't have a default value assigned, unlike the other string properties. This could potentially lead to a null reference exception if the property isn't explicitly set when creating an instance.public string From { get; set; } = string.Empty; -public string Token { get; set; } +public string Token { get; set; } = string.Empty; public string Username { get; set; } = string.Empty; public string CurrentDomain { get; set; } = string.Empty;
16-19: Consider adding a display name for the email senderUsing an empty string for the display name in
new MailboxAddress("", From)might impact user experience. Consider adding a meaningful display name like "StreetCode Team" or "HistoryCode".-emailMessage.From.Add(new MailboxAddress("", From)); +emailMessage.From.Add(new MailboxAddress("Команда Historycode", From));Streetcode/Streetcode.WebApi/Controllers/Authentication/AuthController.cs (2)
73-77: Implemented email confirmation endpointThe
ConfirmEmailendpoint correctly handles the email confirmation process by receiving the DTO and delegating to the MediatR handler. However, it's missing response type documentation and authorization attributes.Consider adding appropriate attributes for better API documentation and security:
[HttpPost] +[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(bool))] +[ProducesResponseType(StatusCodes.Status400BadRequest)] +[Route("confirm-email")] public async Task<IActionResult> ConfirmEmail(ConfirmEmailDTO confirmEmailDto) { return HandleResult(await Mediator.Send(new ConfirmEmailCommand(confirmEmailDto))); }
79-83: Implemented token validation endpointThe
ValidateTokenendpoint correctly handles token validation using MediatR, but lacks response type documentation, route attribute, and parameter name consistency.Consider these improvements for clarity and consistent API documentation:
[HttpPost] +[ProducesResponseType(StatusCodes.Status200OK, Type = typeof(bool))] +[ProducesResponseType(StatusCodes.Status400BadRequest)] +[Route("validate-token")] -public async Task<IActionResult> ValidateToken(ValidateTokenDto confirmEmailDto) +public async Task<IActionResult> ValidateToken(ValidateTokenDto validateTokenDto) { - return HandleResult(await Mediator.Send(new ValidateTokenCommand(confirmEmailDto))); + return HandleResult(await Mediator.Send(new ValidateTokenCommand(validateTokenDto))); }Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenHandler.cs (1)
40-59: Token validation conditional logicThe token validation logic handles different token providers correctly but contains duplicate code that could be simplified.
Consider refactoring to reduce code duplication:
try { - bool result; - if (request.ValidateTokenDto.TokenProvider == "Default") - { - result = await _userManager.VerifyUserTokenAsync( - user, - TokenOptions.DefaultProvider, - request.ValidateTokenDto.Purpose, - request.ValidateTokenDto.Token); - } - else - { - result = await _userManager.VerifyUserTokenAsync( - user, - request.ValidateTokenDto.TokenProvider, - request.ValidateTokenDto.Purpose, - request.ValidateTokenDto.Token); - } + string tokenProvider = request.ValidateTokenDto.TokenProvider == "Default" + ? TokenOptions.DefaultProvider + : request.ValidateTokenDto.TokenProvider; + + bool result = await _userManager.VerifyUserTokenAsync( + user, + tokenProvider, + request.ValidateTokenDto.Purpose, + request.ValidateTokenDto.Token);Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailHandler.cs (1)
27-53: Consider handling the scenario when the email is already confirmed.
The logic correctly decodes inputs, verifies the user, and confirms the email token. However, you might want to gracefully handle or inform the user if the email has already been confirmed, to avoid confusion.Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Login/HandleLoginTest.cs (1)
164-215: Helper methods for user manager and captcha setups aid test clarity.
These private methods provide a consistent, maintainable approach for configuring each test scenario. Continue to ensure each possibility (exception throwing, invalid password, unconfirmed email, etc.) is covered.Streetcode/Streetcode.BLL/MediatR/Authentication/Register/RegisterHandler.cs (1)
69-82: Great use of token generation and email sending.
Generating an email confirmation token, forming a proper URL-safe token, and sending the email are well-sequenced. Consider logging any email delivery failures or introducing retries if needed.Streetcode/Streetcode.BLL/MediatR/Authentication/Login/LoginHandler.cs (2)
38-41: Constructor dependencies are consistent.
The constructor now includes localization, email, message data, and HTTP context dependencies, making the code more robust. Keep an eye on constructor bloat, but for now it looks manageable.Consider using a design pattern like a factory or a separate orchestrator if more dependencies arise in the future.
85-102: Email token generation & sending flow.
Creating and escaping the token and username, then sending via_emailServiceis well-sequenced. The domain retrieval fromHttpContextHelperis a neat approach. Just remember to verify or handle domain edge cases.Consider adding error handling or retry logic around the email-sending process to handle transient network failures.
Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (1)
114-118: Token lifespan for email confirmation.
The 15-minuteTokenLifespanis clear, balancing user convenience and security.If your application has longer or shorter confirmation flows, consider making this lifespan configurable via environment variables.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
Streetcode/Streetcode.BLL/Constants/MessageData/MessageDataConstants.cs(1 hunks)Streetcode/Streetcode.BLL/DTO/Authentication/ConfirmEmail/ConfirmEmailDto.cs(1 hunks)Streetcode/Streetcode.BLL/DTO/Authentication/ValidateToken/ValidateTokenDto.cs(1 hunks)Streetcode/Streetcode.BLL/Factories/MessageDataFactory/Abstracts/IMessageDataAbstractFactory.cs(1 hunks)Streetcode/Streetcode.BLL/Factories/MessageDataFactory/Concretes/MessageDataConcreteFactory.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailCommand.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailHandler.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/Login/LoginHandler.cs(3 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/LoginGoogle/LoginGoogleHandler.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/Register/RegisterHandler.cs(3 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenCommand.cs(1 hunks)Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenHandler.cs(1 hunks)Streetcode/Streetcode.BLL/Models/Email/Messages/ConfirmEmailMessageData.cs(1 hunks)Streetcode/Streetcode.BLL/Resources/Services.Users.TokenService.en-US.resx(0 hunks)Streetcode/Streetcode.BLL/Resources/Services.Users.TokenService.uk-UA.resx(0 hunks)Streetcode/Streetcode.BLL/Resources/SharedResource.FailedToValidateSharedResource.en-US.resx(1 hunks)Streetcode/Streetcode.BLL/Resources/SharedResource.FailedToValidateSharedResource.uk-UA.resx(1 hunks)Streetcode/Streetcode.BLL/Resources/SharedResource.UserSharedResource.en-US.resx(1 hunks)Streetcode/Streetcode.BLL/Resources/SharedResource.UserSharedResource.uk-UA.resx(1 hunks)Streetcode/Streetcode.WebApi/Controllers/Authentication/AuthController.cs(2 hunks)Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs(2 hunks)Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProvider.cs(1 hunks)Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProviderOptions.cs(1 hunks)Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProvider.cs(1 hunks)Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProviderOptions.cs(1 hunks)Streetcode/Streetcode.XIntegrationTest/ControllerTests/Authentication/AuthControllerTests.cs(1 hunks)Streetcode/Streetcode.XIntegrationTest/ControllerTests/Users/UserControllerTests.cs(1 hunks)Streetcode/Streetcode.XIntegrationTest/ControllerTests/Utils/Extracter/Authentication/UserExtracter.cs(2 hunks)Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Login/HandleLoginTest.cs(8 hunks)Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Register/HandleRegisterTest.cs(1 hunks)
💤 Files with no reviewable changes (2)
- Streetcode/Streetcode.BLL/Resources/Services.Users.TokenService.uk-UA.resx
- Streetcode/Streetcode.BLL/Resources/Services.Users.TokenService.en-US.resx
🧰 Additional context used
🧬 Code Definitions (12)
Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProvider.cs (1)
Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProviderOptions.cs (1)
CustomEmailConfirmationTokenProviderOptions(5-7)
Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenCommand.cs (1)
Streetcode/Streetcode.BLL/DTO/Authentication/ValidateToken/ValidateTokenDto.cs (1)
ValidateTokenDto(3-12)
Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailCommand.cs (1)
Streetcode/Streetcode.BLL/DTO/Authentication/ConfirmEmail/ConfirmEmailDto.cs (1)
ConfirmEmailDTO(3-7)
Streetcode/Streetcode.BLL/Factories/MessageDataFactory/Concretes/MessageDataConcreteFactory.cs (3)
Streetcode/Streetcode.BLL/Factories/MessageDataFactory/Abstracts/IMessageDataAbstractFactory.cs (3)
MessageData(7-7)MessageData(8-8)MessageData(9-9)Streetcode/Streetcode.BLL/Models/Email/Messages/Base/MessageData.cs (1)
MessageData(5-9)Streetcode/Streetcode.BLL/Models/Email/Messages/ConfirmEmailMessageData.cs (1)
ConfirmEmailMessageData(7-35)
Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Login/HandleLoginTest.cs (1)
Streetcode/Streetcode.BLL/MediatR/Authentication/Login/LoginHandler.cs (1)
Task(54-127)
Streetcode/Streetcode.BLL/MediatR/Authentication/Login/LoginHandler.cs (3)
Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Login/HandleLoginTest.cs (2)
LoginHandler(225-237)UserDTO(126-133)Streetcode/Streetcode.BLL/Services/Authentication/TokenService.cs (2)
User(87-94)SetNewRefreshTokenForUser(74-85)Streetcode/Streetcode.BLL/DTO/Users/UserDTO.cs (1)
UserDTO(3-7)
Streetcode/Streetcode.WebApi/Controllers/Authentication/AuthController.cs (4)
Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailHandler.cs (1)
Task(27-54)Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenHandler.cs (1)
Task(30-67)Streetcode/Streetcode.BLL/DTO/Authentication/ConfirmEmail/ConfirmEmailDto.cs (1)
ConfirmEmailDTO(3-7)Streetcode/Streetcode.BLL/DTO/Authentication/ValidateToken/ValidateTokenDto.cs (1)
ValidateTokenDto(3-12)
Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProvider.cs (1)
Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProviderOptions.cs (1)
CustomPasswordResetTokenProviderOptions(5-7)
Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (4)
Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProvider.cs (2)
CustomEmailConfirmationTokenProvider(7-18)CustomEmailConfirmationTokenProvider(11-17)Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProvider.cs (2)
CustomPasswordResetTokenProvider(7-18)CustomPasswordResetTokenProvider(11-17)Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProviderOptions.cs (1)
CustomEmailConfirmationTokenProviderOptions(5-7)Streetcode/Streetcode.WebApi/Utils/TokenProviders/ResetPassword/CustomPasswordResetTokenProviderOptions.cs (1)
CustomPasswordResetTokenProviderOptions(5-7)
Streetcode/Streetcode.XIntegrationTest/ControllerTests/Utils/Extracter/Authentication/UserExtracter.cs (1)
Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Register/HandleRegisterTest.cs (1)
User(180-190)
Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Register/HandleRegisterTest.cs (1)
Streetcode/Streetcode.BLL/MediatR/Authentication/Register/RegisterHandler.cs (5)
Task(51-89)Task(91-106)Task(108-131)RegisterHandler(20-132)RegisterHandler(31-49)
Streetcode/Streetcode.BLL/MediatR/Authentication/Register/RegisterHandler.cs (2)
Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Register/HandleRegisterTest.cs (2)
RegisterHandler(301-312)User(180-190)Streetcode/Streetcode.XIntegrationTest/ControllerTests/Utils/Extracter/Authentication/UserExtracter.cs (1)
User(9-34)
🔇 Additional comments (64)
Streetcode/Streetcode.BLL/Resources/SharedResource.FailedToValidateSharedResource.uk-UA.resx (1)
207-209: Resource addition for invalid token looks goodThe addition of the "InvalidToken" resource entry is well-structured and follows the same format as other validation messages in the file. This aligns well with the PR objective of implementing token validation for email confirmation.
Streetcode/Streetcode.BLL/Resources/SharedResource.UserSharedResource.uk-UA.resx (1)
147-149: Ukrainian localization for email confirmation looks goodThe localization entry for "EmailNotConfirmed" is properly added with the appropriate Ukrainian text. This ensures a consistent user experience across different languages.
Streetcode/Streetcode.WebApi/Utils/TokenProviders/EmailConfirmation/CustomEmailConfirmationTokenProvider.cs (1)
7-18:Details
❓ Verification inconclusive
Custom token provider implementation looks well-structured
The
CustomEmailConfirmationTokenProviderclass is correctly implemented as a generic class that inherits fromDataProtectorTokenProvider<TUser>. This implementation allows for customized token generation for email confirmation.Could you confirm that the token expiration time is set to 15 minutes as mentioned in the PR objectives? This would typically be configured in the
CustomEmailConfirmationTokenProviderOptionsclass or when registering the token provider in the startup configuration.
🏁 Script executed:
#!/bin/bash # Look for token provider registration in startup or program files echo "Searching for token provider registration..." rg -A 10 "CustomEmailConfirmationTokenProvider" --include "*.cs" | grep -i "expir\|time\|minute"Length of output: 637
Confirm Token Expiration Setting
The implementation of the custom email confirmation token provider is clean and well-structured. However, the automated search for a token expiration setting encountered an error (likely due to incorrect command flags), so I couldn’t confirm whether the token expiration is configured to 15 minutes as outlined in the PR objectives.
- Could you please manually verify that the token expiration time is set to 15 minutes? This is typically done in the configuration for the
CustomEmailConfirmationTokenProviderOptionsor during token provider registration in the startup configuration.Streetcode/Streetcode.BLL/Constants/MessageData/MessageDataConstants.cs (1)
7-7: LGTM: New constant properly supports email confirmation featureThe addition of
EmailConfirmationTitleconstant aligns perfectly with the PR objective of implementing email confirmation functionality. The constant follows the same naming pattern as existing constants and will serve as the email subject for confirmation messages.Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenCommand.cs (1)
1-8: Clear and concise implementation of the token validation command.The
ValidateTokenCommandrecord is well-structured, following the CQRS pattern with MediatR. It properly implementsIRequest<Result<bool>>for handling token validation operations and encapsulates all necessary data through the ValidateTokenDto parameter.Streetcode/Streetcode.BLL/Resources/SharedResource.FailedToValidateSharedResource.en-US.resx (1)
207-209: Resource entry correctly added for token validation messages.The "InvalidToken" resource entry is appropriately placed in the shared resource file, maintaining consistency with other validation message patterns. This will be useful for providing localized feedback when token validation fails.
Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailCommand.cs (1)
1-7: Effective implementation of the email confirmation command.The
ConfirmEmailCommandrecord follows the CQRS pattern with MediatR, matching the approach used for token validation. The implementation is clean and focused on its single responsibility.Streetcode/Streetcode.BLL/Factories/MessageDataFactory/Abstracts/IMessageDataAbstractFactory.cs (1)
9-9: Factory interface properly extended for email confirmation functionality.The new
CreateConfirmEmailMessageDatamethod follows the same pattern as the existingCreateForgotPasswordMessageDatamethod, maintaining consistency in the interface design. This addition appropriately supports the email confirmation feature described in the PR objectives.Streetcode/Streetcode.BLL/Factories/MessageDataFactory/Concretes/MessageDataConcreteFactory.cs (1)
42-52: Implementation is consistent with existing factory methods.The new
CreateConfirmEmailMessageDatamethod follows the established factory pattern in the codebase, providing a clean approach to instantiate and configure the email confirmation message data object.Streetcode/Streetcode.BLL/Models/Email/Messages/ConfirmEmailMessageData.cs (1)
20-34: Email body implementation looks goodThe HTML email template is well-structured and provides clear instructions to the user in Ukrainian. The confirmation link is properly constructed using the required parameters.
Streetcode/Streetcode.XIntegrationTest/ControllerTests/Authentication/AuthControllerTests.cs (1)
39-39: Appropriate test adaptation for email confirmationAdding the
isEmailConfirmed: trueparameter ensures that integration tests work correctly with the new email confirmation functionality. This is a necessary adjustment to make the tests compatible with the updated registration and login flow.Streetcode/Streetcode.BLL/MediatR/Authentication/LoginGoogle/LoginGoogleHandler.cs (1)
54-55: Setting EmailConfirmed to true is appropriate for Google authenticationUsers authenticating through Google already have verified emails, so setting
EmailConfirmed = trueis a logical approach that avoids an unnecessary confirmation step for these users. This enhances the user experience by streamlining the authentication process for Google sign-ins.Streetcode/Streetcode.XIntegrationTest/ControllerTests/Users/UserControllerTests.cs (1)
47-53: Updated method call to include email confirmation parameterThe addition of
isEmailConfirmed: trueparameter to theUserExtracter.Extractmethod call is consistent with the new email confirmation functionality being implemented. This ensures test users have confirmed email status, which is important for integration tests to properly simulate authenticated users.Streetcode/Streetcode.XIntegrationTest/ControllerTests/Utils/Extracter/Authentication/UserExtracter.cs (2)
9-9: Method signature updated to support email confirmationThe
Extractmethod signature has been expanded to include thebool isEmailConfirmedparameter, which aligns well with the PR objective of adding email confirmation functionality. This change enables test data to properly reflect the email confirmation status.
18-18: Added email confirmation to test user setupThe assignment of the
isEmailConfirmedparameter to the user object'sEmailConfirmedproperty correctly implements the email confirmation functionality in the test user creation process.Streetcode/Streetcode.WebApi/Controllers/Authentication/AuthController.cs (1)
4-16: Added imports for new email confirmation functionalityThe new import statements properly reference the required classes for email confirmation and token validation functionality, maintaining code organization.
Streetcode/Streetcode.BLL/MediatR/Authentication/ValidateToken/ValidateTokenHandler.cs (3)
11-28: Well-structured token validation handlerThe handler follows good practices by using dependency injection for services and separating responsibilities. The class properly implements the MediatR
IRequestHandlerinterface and has appropriate dependencies for token validation.
30-39: User validation logic in token handlerThe token validation properly checks for user existence first and returns appropriate error messages when the user is not found.
60-67: Token validation result handlingThe result handling and error catching is implemented well, with proper logging and error propagation.
Streetcode/Streetcode.BLL/MediatR/Authentication/ConfirmEmail/ConfirmEmailHandler.cs (2)
1-9: All necessary imports are present and relevant.
These using statements and namespace declarations are straightforward, and no extraneous references are visible. Good to go.
11-25: Constructor injection is clear and consistent.
The fields_userManager,_stringLocalizer, and_loggerare assigned properly, improving clarity and testability.Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Login/HandleLoginTest.cs (7)
23-31: Private fields for newly introduced mocks are well-defined.
Declaring_emailService,_httpContextAccessor, and_messageDataAbstractFactoryas private fields improves test arrangement clarity.
35-46: Constructor initialization for all mocked dependencies is clean.
Injecting and storing the new mocks (IEmailService,IMessageDataAbstractFactory,IHttpContextAccessor) aligns the tests with new email confirmation logic.
52-54: Ensuring email confirmation check in successful login scenario is valuable.
IncludingSetupMockUserManagerIsEmailConfirmed(true)here accurately simulates a confirmed account scenario. This test arrangement is logical and thorough.
68-69: User not found scenario is realistic.
CallingSetupMockCaptchaService(true)and then proceeding to handle ensures other factors (like captcha) are valid when a user is missing. Nicely isolates this failure mode.
82-83: Validating incorrect password test is suitably arranged.
Your approach ensures the captcha is valid while the password is invalid, providing a focused test case.
153-162: Overall setup for a successful test scenario is clear.
Gathering all dependent mock setups inSetupServicesForSuccessfulResult()improves code readability and reuse.
228-236: Centralizing handler creation is beneficial.
GetLoginHandler()consolidates mock dependencies into one place, keeping test code DRY and easier to maintain.Streetcode/Streetcode.BLL/MediatR/Authentication/Register/RegisterHandler.cs (2)
27-29: New private fields for email confirmation dependencies are appropriately introduced.
Having_emailService,_messageDataAbstractFactory, and_httpContextAccessorat the class level aligns with the newly added email confirmation workflow.
31-49: Constructor parameters align with introduced integrations.
The constructor properly assigns each new dependency, ensuring they can be utilized inHandle(). No issues spotted with naming or injection.Streetcode/Streetcode.BLL/MediatR/Authentication/Login/LoginHandler.cs (7)
5-5: Ensure references are used as intended.
These newusingdirectives pull in external references required for email-sending logic, message data creation, and HTTP context access. They appear necessary for the new email confirmation feature.Also applies to: 10-10, 12-12, 15-15
28-30: Appropriate injection of new dependencies.
Introducing_emailService,_messageDataAbstractFactory, and_httpContextAccessoris a clear move toward integrating email confirmation. Great job keeping these concerns separated and easy to maintain.
49-51: Dependency assignments align with fields.
Assigning_messageDataAbstractFactory,_httpContextAccessor, and_emailServicein the constructor ensures clarity and correctness.
67-73: Null-check logic for user ensures early exit.
This block gracefully handles cases where the user does not exist. It's straightforward and clear.
74-80: Password check logic is well-organized.
Separating the password validation from user existence checks leads to clearer error handling. Good job.
82-83: Email confirmation gating is sensible.
Ensuring email confirmation before generating tokens is a best practice to avoid unauthorized access.
105-120: Refresh token setting and response creation.
This block completes the login flow with new tokens and user data. It's well-structured and neatly returns the result.Streetcode/Streetcode.WebApi/Extensions/ServiceCollectionExtensions.cs (4)
44-45: New using statements for custom token providers.
These references are essential for introducing the new custom email and password token provider classes.
103-106: Custom token provider naming.
Settingoptions.Tokens.EmailConfirmationTokenProviderandoptions.Tokens.PasswordResetTokenProviderto custom strings is a clean approach, making it explicit.
110-111: Registering custom token providers.
IncludingCustomEmailConfirmationTokenProvider<User>andCustomPasswordResetTokenProvider<User>ensures identity can generate and validate these new tokens. This approach fosters maintainability.
119-123: Token lifespan for password reset.
Similarly, the 15-minute setting is consistent. Good job aligning both token providers with the same default.Streetcode/Streetcode.XUnitTest/MediatRTests/Authentication/Register/HandleRegisterTest.cs (23)
1-12: Imports align with new test logic.
Theseusingdirectives introduce necessary test dependencies and classes, enabling HTTP context mocking, message data factories, and email services. Nice setup.
13-22: Test namespace and fundamental references.
This block defines the test namespace, sets up expressions, mapper, identity, and mocking frameworks. It's well-named for clarity.
24-31: Mocks for unit testing dependencies.
These mocks ensure each service can be independently tested. Good usage of dependency injection andMock<>()to isolate behaviors.
33-49: Test constructor sets up essential mocks.
This ensures each dependent service is properly configured, including localizer returning relevant string resources. The usage ofUserManager<User>with a fakeIUserStore<User>is appropriate for testing identity scenarios.
51-66: Test for successful registration: 'ShouldReturnSuccess_NewUser'.
This test verifies a new user scenario with properly set up environment. The arrangement and act-assert structure is well organized.
67-89: Ensuring correct user data in 'ShouldReturnRegisterResponseDTOWithCorrectData_NewUser'.
This test method checks that mapped user fields match the expected user data, covering name, surname, and role resulting from the registration process.
91-106: Fail scenario: existing email.
Checks that the system returns failure if the email already exists, verifying the localized error message. Thorough negative path coverage.
108-123: Fail scenario: existing username.
This test ensures unique username validation is enforced. Good coverage for user uniqueness checks.
125-141: Mismatch scenario: 'CreateUserFails'.
Verifies that a failed user creation attempt triggers the correct localized error. Excellent job verifying negative outcomes.
143-159: Failure scenario with an internal exception.
Ensures exceptions thrown during user creation are handled gracefully and the correct message is returned.
161-178: Exception scenario in 'AddToRoleAsync'.
Verifies that role assignment failures are properly caught and result in a meaningful error message, fulfilling robust error handling.
180-190: Factory method 'GetSampleUser'.
Provides a convenient default user instance for tests. Clear approach for test data.
192-198: Factory method 'GetSampleRequestDTO'.
Creates a minimalRegisterRequestDTOoffering a starting point for test scenarios.
200-206: Factory method for existing email scenario.
Facilitates negative path testing. This improves clarity when verifying duplicate email behavior.
208-214: Factory method for existing username scenario.
Similarly, dedicated for negative path testing. Good approach for test clarity.
216-220: Central method for success scenario setup.
Combines repository calls and user-manager success paths. Reduces redundant code across test cases.
222-232: Repository mocking for user checks.
This ensures the test can simulate both scenarios of existing and non-existing user records.
233-267: Mapping logic control for tests.
The test can shape whether the email or username is recognized as duplicate. This flexible approach helps thoroughly cover edge cases.
269-275: User manager creation mock.
Simulates both successful or failed user creation. Clear approach for verifying registration flow outcomes.
277-283: Mock token generation.
Offers consistent token strings for testing email confirmation logic without external dependencies.
285-291: Exception mocking for user creation.
Demonstrates robust negative scenario coverage, verifying the system's error response.
293-299: Exception mocking for role assignment.
Ensures that the test covers potential role assignment issues. Great for completeness.
301-312: Handler factory.
ConstructsRegisterHandlerwith mocks, ensuring each test has an isolated environment. This is a clean approach to test setup.
|
| var token = await _tokenService.GenerateAccessTokenAsync(user!); | ||
| var stringToken = new JwtSecurityTokenHandler().WriteToken(token); | ||
|
|
||
| var userDTO = _mapper.Map<UserDTO>(user); |
There was a problem hiding this comment.
please, rename DTO's you are working with to Dto's, if possible
4 participants
dev
JIRA
Summary of issue
Summary of change
Front-end PR
ita-social-projects/StreetCode_Client#1563
Summary by CodeRabbit
New Features
Bug Fixes
Tests