T3: Identity API controller + JWT auth#63
Conversation
- Add JWT authentication middleware with bearer token validation - Implement IdentityService with BCrypt password hashing and JWT generation - Replace scaffold controller with full register/login/users endpoints - Add [Authorize] protection to user listing endpoints - Configure JWT settings in appsettings.json - Register DI services for IIdentityService
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| public async Task<IEnumerable<AppUserDto>> GetAllUsersAsync() | ||
| { | ||
| var users = await _context.Users.ToListAsync(); | ||
| return users.Select(MapToDto); | ||
| } |
There was a problem hiding this comment.
🚩 GetAllUsersAsync loads entire user table into memory without pagination
The GetAllUsersAsync method at IdentityService.cs:82 calls ToListAsync() on the entire Users table with no pagination, filtering, or limit. As the user base grows, this will load all user records into memory in a single query. The GetUsers controller endpoint (IdentityController.cs:46-49) is protected by [Authorize] but not by any role restriction, so any authenticated user can trigger this. This could become a performance concern at scale.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — this is a known limitation for now. Pagination would require changes to the IIdentityService interface in the Domain layer (which is out of scope for T3 per the task constraints). Can be addressed in a follow-up task.
Check user.IsEnabled after credential verification before issuing JWT token.
| if (user == null || !BCrypt.Net.BCrypt.Verify(request.Password, user.PasswordHash)) | ||
| { | ||
| return new LoginResult { Success = false, ErrorMessage = "Invalid username or password." }; | ||
| } | ||
|
|
||
| if (!user.IsEnabled) | ||
| { | ||
| return new LoginResult { Success = false, ErrorMessage = "This account has been disabled." }; | ||
| } |
There was a problem hiding this comment.
🚩 Login verifies password before checking if account is disabled
In LoginAsync, the BCrypt password verification (IdentityService.cs:64) runs before the IsEnabled check (IdentityService.cs:69). This means: (1) expensive BCrypt work is performed for disabled accounts unnecessarily, and (2) a correct password on a disabled account yields a different error message ('This account has been disabled') than an incorrect one ('Invalid username or password'), which reveals account state to the caller. This may be intentional UX, but if not, reversing the order (check IsEnabled first, then verify password) would be both more efficient and more uniform in error responses.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is intentional UX — a legitimate user whose account was disabled should see a clear "account disabled" message rather than a generic credentials error (which would lead them to think they mistyped their password). The information disclosure tradeoff is acceptable here since username enumeration is already possible via the register endpoint's uniqueness check. The BCrypt cost on disabled accounts is negligible in practice.
1 participant
Summary
Implements the full Identity API controller with JWT-based authentication and BCrypt password hashing.
Key changes:
IdentityController— four endpoints:POST /api/identity/register→ validates username/email uniqueness, BCrypt-hashes password, persistsAppUser, returns 201 +AppUserDtoPOST /api/identity/login→ verifies credentials viaBCrypt.Verify, checksIsEnabled, issues JWT with claims (sub,name,email,role), returns tokenGET /api/identity/users([Authorize]) → returns all usersGET /api/identity/users/{id:guid}([Authorize]) → returns single user or 404IdentityService(Infrastructure) — implementsIIdentityService; JWT generation usesHmacSha256with config-driven secret/issuer/audience/expiration; login rejects disabled accountsProgram.cs— registersJwtBearerDefaultsauthentication scheme withTokenValidationParameters, addsUseAuthentication()/UseAuthorization()middleware, registersIIdentityService→IdentityServiceDI bindingNuGet additions:
BCrypt.Net-Next 4.*,Microsoft.AspNetCore.Authentication.JwtBearer 10.*,System.IdentityModel.Tokens.Jwt 8.*,Microsoft.Extensions.Configuration.Abstractions 10.*Build passes. Validated: register→201, login→200+token, unauthenticated→401, authenticated→200.
Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/0f07b7a8f1014aec962930431c54da9f
Requested by: @mbatchelor81