Development feature - User Management #19
Conversation
…s "user" add: register, login, profile. Added authentication, based on JWT. Added Identity seeder for MongoDB.
|
Thank you very much for your pull request and for taking the time to contribute to the project! I noticed that you've added the AspNetCore.Identity.MongoDbCore NuGet package. Could you please clarify if this library is essential for the feature we're implementing? The last release of this library was in 2021, and it seems that it might no longer be actively maintained. This raises some concerns, especially when dealing with security features like authentication and authorization. Given the importance of security in our project, I'd like to suggest taking a look at a previous project of mine where I implemented a similar feature without relying on third-party libraries. I focused on using the built-in .NET APIs to handle authentication. You can find the implementation here . Additionally, the codebase already includes an existing implementation of the IContext interface within the Shared project. This interface provides access to request information and the authenticated user. It might be more consistent to use this rather than the IHttpContextAccessor. Finally, I think it would be great if we could also implement a few additional features, either in this PR or through separate issues/PRs:
I’ve also left a few comments directly within the code for further feedback. If you have any questions or need further clarification, please feel free to reach out! Thanks again for your work on this! Looking forward to your thoughts. |
|
|
||
| internal class GetProfileHandler : ICommandHandler<GetProfile, GetProfileResponse> | ||
| { | ||
| private readonly IHttpContextAccessor _httpContextAccessor; |
There was a problem hiding this comment.
Consider using IContext interface from the Shared project instead of IHttpContextAccessor. IContext is already designed to handle request information and the authenticated user, so it might be a more consistent approach across the codebase.
| MongoDbSettings = new MongoDbSettings | ||
| { | ||
| ConnectionString = options.CONNECTION_STRING, | ||
| DatabaseName = "StellarChat" |
There was a problem hiding this comment.
Avoid hardcoding values directly in the code. Instead, consider using constant variables or values from appsettings.json. For example, the database name can be retrieved from the mongo:database setting in appsettings.json. This approach will make the code more maintainable and configurable.
| await _userManager.AccessFailedAsync(user); | ||
|
|
||
| if (await _userManager.IsLockedOutAsync(user)) | ||
| return new LoginUserResponse(false, "Your account is locked out. Please try again later.", null); |
There was a problem hiding this comment.
Instead of returning a record for error handling, consider using custom exceptions. This approach will help to standardize error handling across the application and make it easier to manage and trace exceptions.
|
I completely agree with all your suggestions. Often the fastest solution is not the best. I hope I will soon find time to make this functionality the right way. Thank you for your cooperation! See you soon, with a new PR. |
💻 Change Type
🔀 Description
Feature - User Management (for current user application)
🔄 Changes
https://github.com/alexandre-spieser/AspNetCore.Identity.MongoDbCore
🖼️ Screenshots (optional)
(If applicable, add screenshots or other visual information that helps to understand the changes.)
📝 Additional Notes
(Anything else you'd like to add that might be relevant for the review or implementation.)