Slice 7: Auth — Register + Login (JWT issue) - #31
Merged
Conversation
Implements Slice 7: POST /api/v1/auth/register and /api/v1/auth/login.
- IPasswordHasher/BCryptPasswordHasher (BCrypt.Net-Next, enhanced hashing)
- IJwtTokenService/JwtTokenService: HS256 access tokens (15min) via
System.IdentityModel.Tokens.Jwt, cryptographically random refresh tokens
(SHA256-hashed before storage, 30-day expiry, plaintext only ever
returned to the client once at login)
- IAuthService/AuthService: orchestrates registration (case-insensitive
duplicate-email check -> 409) and login (password verify -> issue +
persist token pair)
- AuthController + FluentValidation validators (manual ValidateAsync
calls, not auto-validation middleware - the FluentValidation.AspNetCore
team now recommends against auto-validation)
- Program.cs: DbContext, JWT bearer auth, and validator DI registration,
all reading configuration lazily via DI (not eager top-level reads) so
WebApplicationFactory-based tests can override ConnectionStrings:Default
and Jwt:Secret after Program.cs runs
- appsettings.json: empty ConnectionStrings:Default / Jwt:Secret
placeholders - real values via user-secrets locally, env vars deployed,
never committed
Api.Tests gets its own Testcontainers-backed WebApplicationFactory
(ApiWebApplicationFactory) exercising the full real HTTP pipeline against
a real Postgres container. AuthControllerTests covers the 4 required
cases plus 3 more (nonexistent email, invalid email format, and a full
JWT decode+signature-verification+claims check standing in for the
issue's "manual curl smoke test").
Deviation: hardcoded the "api/v1" route prefix literally rather than
wiring Asp.Versioning.Mvc's dynamic {version:apiVersion} route constraint,
to guarantee the exact documented endpoint paths with no format-string
ambiguity. Full API versioning setup deferred to whenever a v2 is
actually needed.
Closes #8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
POST /api/v1/auth/registerandPOST /api/v1/auth/login— the first API/auth slice. A user can create an account and sign in, receiving an HS256 JWT access token (15 min) and a refresh token (30 days, stored hashed).Implements Slice 7 of the backend foundation PRD (#1).
Changes
Application (
src/SubVora.Application/Auth/):RegisterRequest/LoginRequest/AuthTokenResponse/RegisteredUserResponseDTOs,RegisterResult/LoginResultoutcome types,RegisterRequestValidator/LoginRequestValidator(FluentValidation), and 3 pure interfaces —IPasswordHasher,IJwtTokenService,IAuthService— no EF/DB dependency at this layer.Infrastructure (
src/SubVora.Infrastructure/Auth/):BCryptPasswordHasher—BCrypt.Net-Next, enhanced hashing (work factor 12)JwtTokenService— HS256 access tokens viaSystem.IdentityModel.Tokens.Jwt; refresh tokens are 32 random bytes (base64url), SHA256-hashed before storage — the plaintext is returned to the client exactly once, never persistedAuthService— orchestrates both endpoints againstAppDbContext: register does a case-insensitive duplicate-email check before insert; login verifies the password hash, then issues + persists the token pairApi:
AuthController(manualIValidator<T>.ValidateAsynccalls rather than FluentValidation's auto-validation middleware — the FluentValidation.AspNetCore maintainers now recommend against auto-validation, see their docs).Program.csgets its first real wiring:AppDbContextDI registration, JWT bearer authentication, and validator registration.Tests:
ApiWebApplicationFactory(Testcontainers-backedWebApplicationFactory<Program>, own pgvector Postgres instance, migrates on startup) +AuthControllerTests— 7 tests covering the 4 required cases plus nonexistent-email, invalid-email-format, and a full JWT decode/signature-verify/claims check (see below).A real bug caught by the tests, not by inspection
Initially wired
Program.csto readConnectionStrings:DefaultandJwt:Secretinto local variables at the top level (var connectionString = builder.Configuration.GetConnectionString("Default")). This works fine at runtime, but silently broke test isolation:WebApplicationFactory's configuration overrides (pointing at the Testcontainers instance) are applied atBuild()time, after those top-level reads had already captured the real (empty/unconfigured) values. Every test failed with a null-reference deep in Npgsql's connection string parsing.Fixed by deferring all configuration reads into DI-resolved lambdas (
sp.GetRequiredService<IConfiguration>()insideAddScoped(...), andAddOptions<JwtBearerOptions>().Configure<IConfiguration>(...)) so they run lazily, well after the test host's configuration overrides are in place. This is also just better practice for a testableProgram.csgenerally, independent of this bug.Deviation from the issue
Hardcoded the
api/v1route prefix as a literal string ([Route("api/v1/auth")]) rather than wiringAsp.Versioning.Mvc's dynamic{version:apiVersion}route constraint. The dynamic constraint's default rendering fornew ApiVersion(1, 0)wasn't something I wanted to gamble on matchingtechnical_requirements.md's documented/api/v1/...paths exactly without a live check, and this slice's acceptance criteria only cares about the literal endpoint paths working. FullAsp.Versioningsetup is deferred to whenever a real v2 is needed — flagging here since it's a decision worth revisiting once versioning actually matters.Verification
dotnet build SubVora.slnx— 0 warnings, 0 errors (including after bumpingFluentValidationto 12.1.1 in Application — no version-conflict warnings this time, unlike the EF Core saga in Slice 2)dotnet test SubVora.slnx— 25/25 pass (2 smoke + 17 Infrastructure.Tests + 7 new in Api.Tests, up from 0)Register_WithValidEmailAndPassword_Returns201AndCreatesUser,Register_WithDuplicateEmail_Returns409,Login_WithValidCredentials_ReturnsAccessAndRefreshToken,Login_WithWrongPassword_Returns401— the 4 required casesLogin_WithNonexistentEmail_Returns401,Register_WithInvalidEmail_Returns400Login_WithValidCredentials_...also decodes the returned access token withJwtSecurityTokenHandlerusing the test's known signing secret, validates issuer/audience/lifetime/signature, and asserts theemailclaim matches — the automated equivalent of the issue's "manual curl smoke test" verification stepAcceptance criteria (from #8)
POST /api/v1/auth/registercreates a user with a securely hashed password and rejects duplicate emails with 409.POST /api/v1/auth/loginreturns a valid HS256 JWT access token and a refresh token on correct credentials, and 401 on incorrect credentials.refresh_tokenswith a 30-day expiry.HITL note (carried from the issue)
Real
Jwt:Secret/ConnectionStrings:Defaultvalues are not committed —appsettings.jsonhas empty placeholders, real values go throughdotnet user-secretslocally / env vars in deployed environments, pertechnical_requirements.mddecision 25 andCLAUDE.md. Someone still needs to rundotnet user-secrets set Jwt:Secret "<value>"(and the connection string) beforedotnet runwill actually start locally — tests don't need this sinceApiWebApplicationFactorysupplies its own test-only values.Closes #8