feat: implement token based authentication and authorization #15
Conversation
…ss authentication token
|
Warning Rate limit exceeded@osm-Jatin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis set of changes introduces JWT-based authentication and improves API response consistency, error handling, and documentation across the application. Controllers for users and roles now return structured Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserController
participant UserService
participant TokenService
participant UserRepository
Client->>UserController: POST /login (LoginRequestDto)
UserController->>UserService: Login(email, password)
UserService->>UserRepository: GetUserByEmailAsync(email)
UserRepository-->>UserService: User
UserService->>UserService: VerifyPassword
alt Valid credentials
UserService->>TokenService: GenerateToken(user)
TokenService-->>UserService: JWT Token
UserService-->>UserController: Token string
UserController-->>Client: 200 OK (token in BaseResponse)
else Invalid credentials
UserService-->>UserController: null
UserController-->>Client: 400 BadRequest (error BaseResponse)
end
sequenceDiagram
participant Client
participant RoleController
participant RoleService
participant RoleRepository
Client->>RoleController: GET /roles?pageNumber=1&pageSize=10
RoleController->>RoleService: GetAllRolesAsync(pageNumber, pageSize)
RoleService->>RoleRepository: GetAllRolesAsync(pageNumber, pageSize)
RoleRepository-->>RoleService: List<Role>
RoleService->>RoleRepository: GetTotalRolesCountAsync()
RoleRepository-->>RoleService: int
RoleService-->>RoleController: List<RoleResponseDto>, totalCount
RoleController-->>Client: 200 OK (BaseResponse with pagination metadata)
Possibly related PRs
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🔭 Outside diff range comments (1)
src/Api/Controllers/RoleController.cs (1)
1-12: 🛠️ Refactor suggestionController missing
[Authorize]Unlike
UserController, none of theRoleControllerendpoints are protected. Unless roles are intended to be publicly modifiable, decorate the controller or individual actions with[Authorize](and role policy if needed).
🧹 Nitpick comments (15)
src/Core/Services/Interfaces/ITokenService.cs (1)
1-8: Token service interface looks good.The interface is clean and follows the single responsibility principle. The implementation in
TokenService.cscorrectly generates JWT tokens with appropriate claims and validates configuration values.Consider enhancing this interface in the future with additional methods for token validation and refresh token functionality, which are common requirements in authentication systems.
src/Core/Entities/DTOs/UserDTOs.cs (1)
37-44: Consider adding minimum length validation for Password.The
Passwordproperty inLoginRequestDtoonly has the[Required]attribute without any length constraints. For consistency withUserCreateDto.Password(line 22-23), consider adding the[MinLength(8)]attribute to maintain the same validation rules across the application.[Required] + [MinLength(8)] public string Password { get; set; } = string.Empty;src/Core/Services/Interfaces/IRoleService.cs (2)
9-10: Add XML documentation for pagination methods.The pagination functionality is a good addition, but consider adding XML documentation to explain the parameters and their expected values:
+/// <summary> +/// Gets the total count of roles in the system. +/// </summary> +/// <returns>The total number of roles.</returns> Task<int> GetTotalRolesCountAsync(); +/// <summary> +/// Gets a paginated list of roles. +/// </summary> +/// <param name="pageNumber">The page number, starting from 1.</param> +/// <param name="pageSize">The number of items per page.</param> +/// <returns>A collection of role DTOs for the specified page.</returns> Task<IEnumerable<RoleResponseDto>> GetAllRolesAsync(int pageNumber, int pageSize);
10-10: Consider adding sorting parameters to GetAllRolesAsync.For enhanced flexibility, consider adding sorting parameters to the
GetAllRolesAsyncmethod:-Task<IEnumerable<RoleResponseDto>> GetAllRolesAsync(int pageNumber, int pageSize); +Task<IEnumerable<RoleResponseDto>> GetAllRolesAsync(int pageNumber, int pageSize, string? sortBy = null, bool ascending = true);This would allow API consumers to sort results by different properties, improving the usability of your paginated endpoints.
src/Core/Services/TokenService.cs (1)
20-54: Consider refactoring token generation for better testability and maintainability.The GenerateToken method handles multiple responsibilities: configuration retrieval, validation, claims creation, and token generation. Consider extracting these into smaller, focused methods.
public string GenerateToken(User user) { - IConfigurationSection jwtSettings = _configuration.GetSection("JwtSettings"); - SymmetricSecurityKey key = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSettings["Key"]!)); - SigningCredentials credentials = new SigningCredentials(key, SecurityAlgorithms.HmacSha256); + IConfigurationSection jwtSettings = GetAndValidateJwtSettings(); + SigningCredentials credentials = CreateSigningCredentials(jwtSettings); + List<Claim> claims = CreateUserClaims(user); + double minutes = GetAndValidateExpiryMinutes(jwtSettings); + SecurityTokenDescriptor tokenDescriptor = CreateTokenDescriptor(claims, minutes, credentials, jwtSettings); + + JwtSecurityTokenHandler tokenHandler = new JwtSecurityTokenHandler(); + SecurityToken token = tokenHandler.CreateToken(tokenDescriptor); + return tokenHandler.WriteToken(token); +} + +private IConfigurationSection GetAndValidateJwtSettings() +{ + IConfigurationSection jwtSettings = _configuration.GetSection("JwtSettings"); + if (jwtSettings == null || !jwtSettings.GetChildren().Any()) + { + throw new InvalidOperationException("JwtSettings section is missing from configuration"); + } + return jwtSettings; +} + +private SigningCredentials CreateSigningCredentials(IConfigurationSection jwtSettings) +{ + string? key = jwtSettings["Key"]; + if (string.IsNullOrEmpty(key) || key.Length < 32) + { + throw new InvalidOperationException("JWT Key is missing or insufficient length (min 32 characters required for HMAC-SHA256)"); + } + SymmetricSecurityKey securityKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(key)); + return new SigningCredentials(securityKey, SecurityAlgorithms.HmacSha256); +} + +private List<Claim> CreateUserClaims(User user) +{ List<Claim> claims = new List<Claim> { new(ClaimTypes.NameIdentifier, user.Id.ToString()), new(ClaimTypes.Email, user.Email), }; if (user.Role != null) { claims.Add(new Claim(ClaimTypes.Role, user.Role.Name)); } + return claims; +} + +private double GetAndValidateExpiryMinutes(IConfigurationSection jwtSettings) +{ string? expiryMinutes = jwtSettings["ExpiryMinutes"]; if (string.IsNullOrEmpty(expiryMinutes) || !double.TryParse(expiryMinutes, out double minutes)) { throw new InvalidOperationException("JWT ExpiryMinutes is not properly configured"); } + return minutes; +} + +private SecurityTokenDescriptor CreateTokenDescriptor(List<Claim> claims, double minutes, SigningCredentials credentials, IConfigurationSection jwtSettings) +{ + string? issuer = jwtSettings["Issuer"]; + string? audience = jwtSettings["Audience"]; + if (string.IsNullOrEmpty(issuer) || string.IsNullOrEmpty(audience)) + { + throw new InvalidOperationException("JWT Issuer and Audience must be configured"); + } + SecurityTokenDescriptor tokenDescriptor = new SecurityTokenDescriptor { Subject = new ClaimsIdentity(claims), Expires = DateTime.UtcNow.AddMinutes(minutes), - Issuer = jwtSettings["Issuer"], - Audience = jwtSettings["Audience"], + Issuer = issuer, + Audience = audience, SigningCredentials = credentials }; - - JwtSecurityTokenHandler tokenHandler = new JwtSecurityTokenHandler(); - SecurityToken token = tokenHandler.CreateToken(tokenDescriptor); - return tokenHandler.WriteToken(token); + return tokenDescriptor; }src/Core/Repositories/RoleRepository.cs (1)
38-38: Use nameof() for property names in OrderBy.Using nameof() for property names in OrderBy makes the code more maintainable and refactoring-friendly.
return await _context.Roles .AsNoTracking() - .OrderBy(r => r.Id) + .OrderBy(r => r.Id) // Consider using OrderBy(r => EF.Property<int>(r, nameof(Role.Id))) .Skip((pageNumber - 1) * pageSize) .Take(pageSize) .ToListAsync() .ConfigureAwait(false);src/Core/Repositories/Interfaces/IUserRepository.cs (1)
9-12: Consider documenting pagination & return‑type semanticsThe interface change looks good, but the behaviour of
pageNumber(0‑ vs 1‑based) and the meaning of anullreturn from Add/Update are not obvious to future implementers / consumers.
Adding XML‑docs (or at least summary comments) clarifying:
pageNumberis 1‑based and must be ≥ 1pageSizemust be ≥ 1nullindicates a persistence failure / validation conflictwould make the contract self‑describing and help avoid misuse.
src/Core/Repositories/UserRepository.cs (2)
44-48: Eager‑loadRolebefore returning to avoid incomplete DTO mapping
createdUseris returned straight afterSaveChangesAsync, but EF won’t populate navigation properties on the detached entity.
Down‑stream mapping toUserResponseDtowill therefore yieldRole = null.Two lightweight options:
bool success = await _context.SaveChangesAsync().ConfigureAwait(false) > 0; -return success ? user : null; +return success + ? await _context.Users.Include(u => u.Role) + .AsNoTracking() + .FirstOrDefaultAsync(u => u.Id == user.Id) + .ConfigureAwait(false) + : null;(or project the role name directly in the mapper).
50-55: Same eager‑load concern applies toUpdateUserAsyncAfter
Update, reload the entity (or use.Entry(user).Reference(u => u.Role).LoadAsync()) before returning, so the service layer receives a fully populated object.src/Core/Services/Interfaces/IUserService.cs (1)
8-14: Return a richer result fromLogininstead of raw/null stringA
string?forces the controller to guess whynullwas returned (bad credentials? locked account? …).
Consider returning a small result object:public record LoginResult(bool Succeeded, string? Token, string? Error); Task<LoginResult> LoginAsync(string email, string password);This keeps the service pure and lets the API layer craft the appropriate HTTP response.
src/Core/Services/UserService.cs (4)
22-31: Timing‑attack mitigation & uniform error handling
VerifyPasswordis fine, but returningnullimmediately after a failed password check can create a small timing difference between “email not found” and “password wrong”.
You can unify the paths (e.g., always callVerifyPasswordwith a dummy hash when the user is missing) to reduce user‑enumeration vectors.Not urgent, yet easy to address:
private static readonly string DummyHash = BCrypt.Net.BCrypt.HashPassword(Guid.NewGuid().ToString()); ... if (user == null || !VerifyPassword(password, user.PasswordHash)) { // Still call verification to equalise timing when user == null if (user == null) _ = VerifyPassword(password, DummyHash); return null; }
45-54: Propagate the pagination guard to the service levelEven when the repository sanitises inputs, it’s better to fail‑fast at the service boundary so controllers immediately get a validation error:
if (pageNumber < 1 || pageSize < 1) throw new ArgumentOutOfRangeException("PageNumber and PageSize must be positive integers.");This keeps the contract explicit and avoids silent auto‑correction that clients may not expect.
56-69: Race condition on duplicate‑email checkBetween the duplicate‑email probe and
SaveChangesAsync, another request could insert the same email, leading to a DB unique‑constraint violation.
WrappingSaveChangesAsyncin atry‑catchforDbUpdateException(and translating it to a domain error) will make the service resilient:try { User? createdUser = await _userRepository.AddUserAsync(user).ConfigureAwait(false); return createdUser == null ? null : _mapper.Map<UserResponseDto>(createdUser); } catch (DbUpdateException ex) when (ex.InnerException?.Message.Contains("UNIQUE") == true) { return null; // or throw DuplicateEmailException }
102-110: Expose BCrypt work factor via configuration
BCrypt.HashPassword(password)uses the library default cost (currently 10). For long‑lived systems you’ll want to tune this parameter without code changes.-private static string HashPassword(string password) -{ - return BCrypt.Net.BCrypt.HashPassword(password); -} +private readonly int _bcryptWorkFactor = 11; // inject from IConfiguration + +private string HashPassword(string password) +{ + return BCrypt.Net.BCrypt.HashPassword(password, _bcryptWorkFactor); +}Same applies to
VerifyPassword.src/Api/Models/Common/BaseResponse.cs (1)
79-108: Encapsulate paging calculations inPaginationMetadata
PageNumber,TotalPages,HasPreviousPage, andHasNextPageare set by every controller. Moving that calculation into a constructor or a static factory eliminates repetition and risk of mis‑calculation (e.g., division by zero whenpageSize == 0).public sealed record PaginationMetadata(int PageNumber, int PageSize, int TotalCount) { public int TotalPages => (int)Math.Ceiling(TotalCount / (double)PageSize); public bool HasPreviousPage => PageNumber > 1; public bool HasNextPage => PageNumber < TotalPages; }Controllers would then only pass the three primitives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/Api/Api.csproj(2 hunks)src/Api/Controllers/RoleController.cs(2 hunks)src/Api/Controllers/UserController.cs(2 hunks)src/Api/Models/Common/BaseResponse.cs(2 hunks)src/Api/Program.cs(5 hunks)src/Api/appsettings.Development.json(1 hunks)src/Api/appsettings.json(1 hunks)src/Core/Core.csproj(1 hunks)src/Core/Entities/DTOs/UserDTOs.cs(1 hunks)src/Core/Repositories/Interfaces/IRoleRepository.cs(1 hunks)src/Core/Repositories/Interfaces/IUserRepository.cs(1 hunks)src/Core/Repositories/RoleRepository.cs(2 hunks)src/Core/Repositories/UserRepository.cs(1 hunks)src/Core/Services/Interfaces/IRoleService.cs(1 hunks)src/Core/Services/Interfaces/ITokenService.cs(1 hunks)src/Core/Services/Interfaces/IUserService.cs(1 hunks)src/Core/Services/RoleService.cs(2 hunks)src/Core/Services/TokenService.cs(1 hunks)src/Core/Services/UserService.cs(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
src/Api/appsettings.json (1)
Learnt from: osm-Jatin
PR: OsmosysSoftware/dotnet-foundation-v2#8
File: src/Api/appsettings.Development.json:19-19
Timestamp: 2025-03-05T07:10:16.015Z
Learning: In the .NET Foundation project, JWT key placeholders with clear instructions (like "PLACEHOLDER_REPLACE_WITH_STRONG_KEY_MIN_32CHARS_BEFORE_USE") are intentionally kept in development configuration files (appsettings.Development.json) to guide developers on required replacements.
src/Api/appsettings.Development.json (1)
Learnt from: osm-Jatin
PR: OsmosysSoftware/dotnet-foundation-v2#8
File: src/Api/appsettings.Development.json:19-19
Timestamp: 2025-03-05T07:10:16.015Z
Learning: In the .NET Foundation project, JWT key placeholders with clear instructions (like "PLACEHOLDER_REPLACE_WITH_STRONG_KEY_MIN_32CHARS_BEFORE_USE") are intentionally kept in development configuration files (appsettings.Development.json) to guide developers on required replacements.
🧬 Code Graph Analysis (6)
src/Core/Services/Interfaces/ITokenService.cs (1)
src/Core/Services/TokenService.cs (1)
GenerateToken(20-54)
src/Core/Services/TokenService.cs (1)
src/Core/Services/Interfaces/ITokenService.cs (1)
GenerateToken(7-7)
src/Core/Services/Interfaces/IUserService.cs (1)
src/Core/Entities/DTOs/UserDTOs.cs (3)
UserResponseDto(28-35)UserCreateDto(20-24)UserUpdateDto(26-26)
src/Core/Repositories/RoleRepository.cs (3)
src/Core/Repositories/Interfaces/IRoleRepository.cs (7)
Task(7-7)Task(8-8)Task(9-9)Task(10-10)Task(11-11)Task(12-12)Task(13-13)src/Core/Services/Interfaces/IRoleService.cs (6)
Task(8-8)Task(9-9)Task(10-10)Task(11-11)Task(12-12)Task(13-13)src/Core/Services/RoleService.cs (3)
Task(23-27)Task(29-32)Task(34-38)
src/Core/Services/Interfaces/IRoleService.cs (1)
src/Core/Entities/DTOs/RoleDTOs.cs (3)
RoleResponseDto(20-23)RoleCreateDto(12-14)RoleUpdateDto(16-18)
src/Core/Services/UserService.cs (4)
src/Core/Services/Interfaces/IUserService.cs (8)
Task(8-8)Task(9-9)Task(10-10)Task(11-11)Task(12-12)Task(13-13)Task(14-14)Task(15-15)src/Core/Repositories/Interfaces/IUserRepository.cs (7)
Task(7-7)Task(8-8)Task(9-9)Task(10-10)Task(11-11)Task(12-12)Task(13-13)src/Core/Entities/DTOs/UserDTOs.cs (3)
UserResponseDto(28-35)UserCreateDto(20-24)UserUpdateDto(26-26)src/Core/Entities/Models/User.cs (1)
SetRole(46-50)
🔇 Additional comments (14)
src/Api/appsettings.Development.json (1)
18-23: JWT settings section added correctly.The JWT configuration looks good with appropriate placeholders. I see that using placeholder keys with clear instructions is a known practice in this project (based on retrieved learnings).
Remember that the placeholder key needs to be replaced with a strong key of at least 32 characters before deployment to any environment.
src/Core/Core.csproj (1)
11-11: JWT authentication package added.The Microsoft.AspNetCore.Authentication.JwtBearer package is correctly added for JWT authentication support.
src/Api/Api.csproj (1)
11-11: The JWT authentication packages are properly configured.The added packages are appropriate for implementing JWT-based authentication and enhancing API documentation:
Microsoft.AspNetCore.Authentication.JwtBearerprovides middleware for token validationSwashbuckle.AspNetCore.Annotationsallows for enhanced Swagger documentationSystem.IdentityModel.Tokens.Jwtprovides JWT token handling capabilitiesThese packages align perfectly with the PR objective of implementing token-based authentication.
Also applies to: 25-26
src/Api/Program.cs (3)
37-37: Service registration looks good.The
ITokenServiceis correctly registered as a scoped service, which is appropriate for services that maintain state during a request but not across requests.
57-103: Swagger configuration is well-structured for JWT authentication.The OpenAPI configuration properly sets up JWT authentication in Swagger UI with clear documentation and security requirements. Using annotations and including XML comments enhances API documentation quality.
158-158: Authentication middleware placement is correct.The
UseAuthentication()middleware is correctly placed beforeUseAuthorization(), which is essential for the authentication flow to work properly.src/Core/Services/Interfaces/IRoleService.cs (1)
12-12: Improved return type for UpdateRoleAsync.Changing the return type from
Task<bool>toTask<RoleResponseDto?>is a good improvement. It provides more information to the caller about the updated entity rather than just a success/failure flag.src/Core/Repositories/Interfaces/IRoleRepository.cs (2)
9-10: Good implementation of pagination support.Adding pagination capabilities through
GetTotalRolesCountAsync()and modifyingGetAllRolesAsync()to accept pagination parameters is a good practice. This approach improves performance by limiting the data retrieved in a single query and provides better user experience for dealing with large datasets.
12-12: LGTM: Improved return type for UpdateRoleAsync.Changing the return type from
Task<bool>toTask<Role?>provides more detailed information about the update operation. This is consistent with theAddRoleAsyncmethod and allows callers to access the updated entity without additional queries.src/Core/Repositories/RoleRepository.cs (2)
29-32: LGTM: Clean implementation of GetTotalRolesCountAsync().The implementation correctly uses EF Core's CountAsync() with ConfigureAwait(false) for async optimization.
52-57: LGTM: Consistent implementation of UpdateRoleAsync.The updated implementation properly returns the Role object or null based on the success of the update operation, which is consistent with AddRoleAsync and follows the interface changes.
src/Core/Services/RoleService.cs (1)
29-32: LGTM: Clean implementation of GetTotalRolesCountAsync().The implementation correctly delegates to the repository method with ConfigureAwait(false) for async optimization.
src/Api/Models/Common/BaseResponse.cs (1)
60-70: Possible emptyErrorsdictionaryIf this helper is called while
ModelState.IsValid == true, the produced response still reportsResponseStatus.Errorwith an emptyErrorscollection. Consider short‑circuiting when no validation error exists to avoid false negatives.src/Api/Controllers/UserController.cs (1)
88-104: Duplicate enumeration inGetAllUsers
users.Any()is called twice. Cache the result as shown in theRoleControllercomment to avoid double database hits.
osm-Jatin
force-pushed
the
feat/authentication
branch
from
e8e40d9 to
1c0287c
Compare
Task ID
REST-1102
Description:
This PR introduces token-based authentication and authorization using JWT in the application.
Changes Included:
Login API Implementation
JWT Token Service
Secure Endpoints with Authorize Attribute
Authentication & Authorization Configuration in Program.cs
Updates to appsettings.json
Summary by CodeRabbit
New Features
Improvements
Configuration