Skip to content

chores: implemented jwt authentication middleware#21

Merged
codebestia merged 4 commits into
ShadeProtocol:mainfrom
KodeSage:feat/jwt_auth
Jun 29, 2026
Merged

chores: implemented jwt authentication middleware#21
codebestia merged 4 commits into
ShadeProtocol:mainfrom
KodeSage:feat/jwt_auth

Conversation

@KodeSage

@KodeSage KodeSage commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

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) in auth.services.ts), but the middleware never actually verified them — it did a refreshToken database session lookup instead. That meant the signed access token was issued but never checked anywhere.

This PR makes authenticateMerchant a real JWT guard: it verifies the Bearer token against JWT_SECRET, loads the merchant referenced by the token's sub claim, attaches it to req.merchant, and returns clear, distinct 401s for every failure mode.

What changed

File Change
src/middlewares/auth.middleware.ts Rewrote to verify the JWT, decode sub, load the merchant, and attach req.merchant; spec-compliant 401 messages
tests/unit/auth.middleware.test.ts New — 7 unit tests covering every acceptance criterion
tests/integration/invoice.routes.test.ts Authenticate with a real signed JWT + mock merchant.findUnique (was mocking the old refresh-token lookup)
tests/integration/merchant.register.test.ts Same JWT-based auth update + assert the new 401 messages

Behavior

Case Response
Missing or non-Bearer Authorization header 401 { "error": "Authentication required" }
Malformed / expired / wrong-secret JWT 401 { "error": "Invalid or expired token" }
Valid JWT but merchant no longer in DB 401 { "error": "Invalid or expired token" }
Valid JWT + merchant exists req.merchant populated, next() called

The middleware is applied at the router levelinvoice.routes.ts uses router.use(authenticateMerchant), and the protected merchant /register route guards via the same middleware — so new protected route groups only need a single router.use(authenticateMerchant) to opt in.

Notes / deviations

  • Path convention: the task description referenced src/middleware/ (singular), but the project already uses src/middlewares/ (plural) and every route imports from there, so the existing file was updated rather than forking a parallel path.
  • Types: src/types/express.d.ts already augments Express.Request with merchant?: Merchant, so req.merchant is typed project-wide; no type changes were needed.
  • Out of scope: the refreshToken table 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/refresh endpoint can follow separately.

Testing

Test Suites: 12 passed, 12 total
Tests:       112 passed, 112 total
  • Added 7 focused unit tests (valid token, missing header, non-Bearer scheme, malformed token, expired token, forged-secret token, deleted merchant).
  • npx tsc --noEmit is clean for all src/ code. (Pre-existing, unrelated: missing @types/urijs declarations inside node_modules/@stellar/stellar-sdk.)

Summary by CodeRabbit

  • Bug Fixes
    • Improved authentication handling to resolve merchants from signed Bearer tokens (and other supported token types), rather than relying on session/refresh-token validation.
    • Standardized unauthenticated responses: missing/invalid Bearer now returns 401 { error: 'Authentication required' }, and invalid/expired tokens return 401 { error: 'Invalid or expired token' }.
  • Tests
    • Updated integration and unit tests to generate real JWTs dynamically and to assert the new authentication error messages and merchant lookup behavior.

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a566eb6b-b915-414b-9c3e-a878f7433f64

📥 Commits

Reviewing files that changed from the base of the PR and between 8700c13 and 85d43b4.

📒 Files selected for processing (3)
  • src/middlewares/auth.middleware.ts
  • tests/integration/auth.email-otp.test.ts
  • tests/integration/merchant.register.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/integration/merchant.register.test.ts

📝 Walkthrough

Walkthrough

authenticateMerchant now verifies Bearer JWTs with jwtSecret, loads merchants by token subject, and returns specific 401 errors for missing or invalid tokens. Related unit and integration tests now sign JWTs and assert the updated responses.

Changes

JWT Auth Middleware Rewrite

Layer / File(s) Summary
JWT middleware implementation
src/middlewares/auth.middleware.ts
Adds bearer extraction helpers, token-type routing, JWT and refresh-token validation, merchant lookup, and distinct 401 responses for missing versus invalid tokens.
Unit tests for authenticateMerchant
tests/unit/auth.middleware.test.ts
Adds coverage for valid JWT authentication, missing and non-Bearer headers, malformed and expired tokens, wrong-secret tokens, and a missing merchant record.
Integration test updates
tests/integration/invoice.routes.test.ts, tests/integration/merchant.register.test.ts, tests/integration/auth.email-otp.test.ts
Switches integration tests to signed JWTs using environment.jwtSecret, mocks merchant lookup, and updates expected authentication error payloads and authorized requests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • codebestia

Poem

🐇 A JWT hopped into the gate,
With merchant claims and secret fate.
If tokens drift or vanish away,
The burrow says, “Not today!”

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning JWT auth and tests were added, but the required req.merchant type extension and router-level application for protected groups aren't shown. Add req.merchant typing in src/types/express.d.ts and apply the middleware at the protected router level for merchants and invoices.
Out of Scope Changes check ⚠️ Warning The middleware also resolves API keys and refresh tokens, which is broader than the JWT-only auth guard requested. Remove the extra auth-scheme routing from this PR, or move it to a separate change focused on non-JWT authentication.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: implementing JWT authentication middleware.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: one or more packages not found in the registry.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
tests/integration/merchant.register.test.ts (1)

53-58: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Return the auth header from authenticateAs.

Keeping authHeader fixed to baseMerchant lets 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

📥 Commits

Reviewing files that changed from the base of the PR and between c0e30a5 and 6925628.

📒 Files selected for processing (4)
  • src/middlewares/auth.middleware.ts
  • tests/integration/invoice.routes.test.ts
  • tests/integration/merchant.register.test.ts
  • tests/unit/auth.middleware.test.ts

Comment thread src/middlewares/auth.middleware.ts Outdated
@codebestia

Copy link
Copy Markdown
Contributor

@KodeSage
Great job so far.

Please add the authentication mock to the tests for email otp.
This was merged into your branch.

Ensure to pull from the branch before you continue.

@codebestia codebestia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thank you for your contribution.

@codebestia codebestia merged commit c3e05f8 into ShadeProtocol:main Jun 29, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auth Middleware (JWT Guard for Protected Routes)

2 participants