chores: implemented jwt authentication middleware#21
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesJWT Auth Middleware Rewrite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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
tests/integration/merchant.register.test.tsParsing error: "parserOptions.project" has been provided for Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/integration/merchant.register.test.ts (1)
53-58: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReturn the auth header from
authenticateAs.Keeping
authHeaderfixed tobaseMerchantlets the token subject drift from the mocked merchant when tests pass a different merchant. Make the helper produce both the mock and matching header.♻️ Proposed refactor
const authenticateAs = (merchant: Record<string, unknown>) => { prismaMock.merchant.findUnique.mockResolvedValue(merchant as any); + return `Bearer ${tokenFor(merchant)}`; }; -const authHeader = `Bearer ${tokenFor(baseMerchant)}`; +const authHeader = authenticateAs(baseMerchant);🤖 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 `@tests/integration/merchant.register.test.ts` around lines 53 - 58, The authentication helper is only mocking the merchant lookup and the auth header is still always generated from baseMerchant, so the token subject can mismatch the merchant under test. Update authenticateAs in merchant.register.test.ts to return the matching Bearer header for the provided merchant while also setting prismaMock.merchant.findUnique, and use that returned header at call sites instead of the fixed authHeader constant.
🤖 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 `@src/middlewares/auth.middleware.ts`:
- Line 42: The auth middleware currently verifies tokens with
environment.jwtSecret, which can fall back to the development secret if
JWT_SECRET is unset. Update the configuration path used by auth.middleware.ts
and the environment.jwtSecret source so production startup fails fast when
JWT_SECRET is missing instead of allowing the fallback; keep jwt.verify in
AuthMiddleware using only a validated secret and ensure the config check happens
before the middleware can accept requests.
---
Nitpick comments:
In `@tests/integration/merchant.register.test.ts`:
- Around line 53-58: The authentication helper is only mocking the merchant
lookup and the auth header is still always generated from baseMerchant, so the
token subject can mismatch the merchant under test. Update authenticateAs in
merchant.register.test.ts to return the matching Bearer header for the provided
merchant while also setting prismaMock.merchant.findUnique, and use that
returned header at call sites instead of the fixed authHeader constant.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a908776e-0343-4407-9c77-01ed9eea5fd5
📒 Files selected for processing (4)
src/middlewares/auth.middleware.tstests/integration/invoice.routes.test.tstests/integration/merchant.register.test.tstests/unit/auth.middleware.test.ts
|
@KodeSage Please add the authentication mock to the tests for email otp. Ensure to pull from the branch before you continue. |
JWT authentication middleware for merchant-facing routes(CLOSES #8)
Summary
Merchant-facing endpoints (invoices, registration, and future dashboard/profile/API-key routes) need to be gated behind a valid access token. The codebase already issued JWT access tokens (
jwt.sign({ sub: merchant.id, address }, JWT_SECRET)inauth.services.ts), but the middleware never actually verified them — it did arefreshTokendatabase session lookup instead. That meant the signed access token was issued but never checked anywhere.This PR makes
authenticateMerchanta real JWT guard: it verifies the Bearer token againstJWT_SECRET, loads the merchant referenced by the token'ssubclaim, attaches it toreq.merchant, and returns clear, distinct 401s for every failure mode.What changed
src/middlewares/auth.middleware.tssub, load the merchant, and attachreq.merchant; spec-compliant 401 messagestests/unit/auth.middleware.test.tstests/integration/invoice.routes.test.tsmerchant.findUnique(was mocking the old refresh-token lookup)tests/integration/merchant.register.test.tsBehavior
BearerAuthorizationheader401 { "error": "Authentication required" }401 { "error": "Invalid or expired token" }401 { "error": "Invalid or expired token" }req.merchantpopulated,next()calledThe middleware is applied at the router level —
invoice.routes.tsusesrouter.use(authenticateMerchant), and the protected merchant/registerroute guards via the same middleware — so new protected route groups only need a singlerouter.use(authenticateMerchant)to opt in.Notes / deviations
src/middleware/(singular), but the project already usessrc/middlewares/(plural) and every route imports from there, so the existing file was updated rather than forking a parallel path.src/types/express.d.tsalready augmentsExpress.Requestwithmerchant?: Merchant, soreq.merchantis typed project-wide; no type changes were needed.refreshTokentable and refresh-token rotation (exchanging a refresh token for a new 15-minute access token) are untouched — the middleware only validates access tokens. A/auth/refreshendpoint can follow separately.Testing
npx tsc --noEmitis clean for allsrc/code. (Pre-existing, unrelated: missing@types/urijsdeclarations insidenode_modules/@stellar/stellar-sdk.)Summary by CodeRabbit
401 { error: 'Authentication required' }or401 { error: 'Invalid or expired token' }.