refactor: extract ITokenService port to remove Fastify from AuthResolver - #20
Conversation
AuthResolver was importing FastifyInstance directly to sign and verify JWTs, coupling the interface-adapter layer to the framework. Introduce ITokenService port and FastifyJwtTokenService infrastructure implementation so the resolver has no framework dependency and is fully unit-testable via a plain mock.
WalkthroughAuthentication token operations are now defined by an ChangesAuthentication token service
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. Comment |
![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 (1)
apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts (1)
8-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo direct unit test coverage for
FastifyJwtTokenService.The provided file set only shows
AuthResolver.test.tsmockingITokenService; no test exercises the real Fastify JWT signing/verification path (e.g. missing-secret behaviour, expiry,UNAUTHORIZEDmapping). Worth adding a focused unit test for this class given it's the security-critical boundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts` around lines 8 - 32, The FastifyJwtTokenService security boundary lacks direct unit tests. Add focused tests for FastifyJwtTokenService.sign and verifyRefresh, covering access and refresh token creation, missing refresh-secret behavior, token expiry, successful verification, and invalid-token mapping to an error with code UNAUTHORIZED; mock or provide the Fastify JWT dependency and isolate environment configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts`:
- Around line 8-32: Update FastifyJwtTokenService’s constructor to read and
validate JWT_REFRESH_SECRET immediately, throwing when it is unset, then cache
the validated value in a private field. Replace the lazy
process.env.JWT_REFRESH_SECRET! reads in sign() and verifyRefresh() with that
field.
---
Nitpick comments:
In `@apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts`:
- Around line 8-32: The FastifyJwtTokenService security boundary lacks direct
unit tests. Add focused tests for FastifyJwtTokenService.sign and verifyRefresh,
covering access and refresh token creation, missing refresh-secret behavior,
token expiry, successful verification, and invalid-token mapping to an error
with code UNAUTHORIZED; mock or provide the Fastify JWT dependency and isolate
environment configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb949b4c-1b21-4e63-a466-949e5b3c9203
📒 Files selected for processing (5)
apps/api/src/__tests__/interface-adapters/resolvers/AuthResolver.test.tsapps/api/src/http/container.tsapps/api/src/infrastructure/auth/FastifyJwtTokenService.tsapps/api/src/interface-adapters/resolvers/AuthResolver.tsapps/api/src/use-cases/ports/ITokenService.ts
| export class FastifyJwtTokenService implements ITokenService { | ||
| constructor(private readonly deps: Deps) {} | ||
|
|
||
| sign(userId: string, email: string): TokenPair { | ||
| const accessToken = this.deps.fastify.jwt.sign( | ||
| { sub: userId, email }, | ||
| { expiresIn: '15m' }, | ||
| ); | ||
| const refreshToken = this.deps.fastify.jwt.sign( | ||
| { sub: userId, email }, | ||
| { key: process.env.JWT_REFRESH_SECRET!, expiresIn: '7d' }, | ||
| ); | ||
| return { accessToken, refreshToken }; | ||
| } | ||
|
|
||
| verifyRefresh(token: string): { sub: string; email: string } { | ||
| try { | ||
| return this.deps.fastify.jwt.verify<{ sub: string; email: string }>(token, { | ||
| key: process.env.JWT_REFRESH_SECRET!, | ||
| }); | ||
| } catch { | ||
| throw Object.assign(new Error('Invalid refresh token'), { code: 'UNAUTHORIZED' }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file listing ==\n'
git ls-files 'apps/api/src/**/*' | sed -n '1,200p'
printf '\n== locate JWT_REFRESH_SECRET usage ==\n'
rg -n "JWT_REFRESH_SECRET|refresh secret|JWT_REFRESH" apps/api/src apps/api -g '!**/dist/**' -g '!**/build/**'
printf '\n== inspect target file ==\n'
cat -n apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts
printf '\n== inspect likely env/schema files ==\n'
fd -a -t f '.*(env|schema|config|validate).*' apps/api/src apps/api 2>/dev/null | sed -n '1,200p'Repository: mankatcheung/job-finder
Length of output: 12864
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== search for env validation/config ==\n'
rg -n "process\.env|JWT_REFRESH_SECRET|zod.*env|env schema|dotenv|createEnv|safeEnv|validate.*env" \
. \
-g '!**/dist/**' -g '!**/build/**' -g '!**/node_modules/**'
printf '\n== inspect HTTP/container wiring around token service ==\n'
cat -n apps/api/src/http/container.ts | sed -n '1,260p'
printf '\n== inspect auth plugin ==\n'
cat -n apps/api/src/http/plugins/auth.plugin.ts | sed -n '1,220p'Repository: mankatcheung/job-finder
Length of output: 18657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app bootstrap ==\n'
cat -n apps/api/src/app.ts | sed -n '1,240p'
printf '\n== server entrypoint ==\n'
cat -n apps/api/src/index.ts | sed -n '1,200p'
printf '\n== token service usages ==\n'
rg -n "tokenService|FastifyJwtTokenService|resolve\\(['\"]tokenService|cradle\\.tokenService|diContainer\\.resolve\\(['\"]tokenService" \
apps/api/src -g '!**/dist/**' -g '!**/build/**'Repository: mankatcheung/job-finder
Length of output: 6056
Validate JWT_REFRESH_SECRET in the constructor (apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts:8-32).
process.env.JWT_REFRESH_SECRET! is read lazily in sign() and verifyRefresh(), and this service is only resolved when auth flows run, so a missing secret lets the app boot and then breaks the first login/refresh request. Cache it in a private field and throw if it is unset.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/infrastructure/auth/FastifyJwtTokenService.ts` around lines 8 -
32, Update FastifyJwtTokenService’s constructor to read and validate
JWT_REFRESH_SECRET immediately, throwing when it is unset, then cache the
validated value in a private field. Replace the lazy
process.env.JWT_REFRESH_SECRET! reads in sign() and verifyRefresh() with that
field.
1 participant
Summary
ITokenServiceport (use-cases/ports/ITokenService.ts) withsign()andverifyRefresh()methodsFastifyJwtTokenServicein the infrastructure layer (infrastructure/auth/FastifyJwtTokenService.ts) — the only place that touchesfastify.jwtFastifyInstancefromAuthResolver's dependencies; resolver now depends only onITokenService, making it fully framework-agnosticcontainer.tsto registertokenServiceas a singletonAuthResolvertests to mockITokenServicedirectly — no more fake Fastify object neededWhy
AuthResolversat in the interface-adapter layer but was importingFastifyInstanceto sign and verify JWTs — infrastructure work that belongs one layer out. This violates Clean Architecture: interface-adapters should orchestrate use cases, not reach into framework internals.Test plan
pnpm --filter @job-finder/api typecheck— no errorspnpm --filter @job-finder/api test— 189 tests passpnpm dev+ manual login/register/refresh — verify tokens still work end-to-endSummary by CodeRabbit
Security & Authentication
Reliability