Skip to content

Httponly cookies migration. continuation of UI PR.#34

Closed
YounBrand wants to merge 42 commits intomainfrom
httponly-cookies
Closed

Httponly cookies migration. continuation of UI PR.#34
YounBrand wants to merge 42 commits intomainfrom
httponly-cookies

Conversation

@YounBrand
Copy link
Copy Markdown
Collaborator

Summary

  • Migrate auth token storage from localStorage/sessionStorage to httpOnly cookies, eliminating XSS token theft as tokens are no longer accessible to JavaScript
  • Fix onboarding re-entry bug where authenticated users with no Plaid link were sent to /dashboard with no path to link a bank account

What changed

Server

  • Registered @fastify/cookie plugin in app.ts
  • login sets accessToken and refreshToken as httpOnly, Secure, SameSite=Strict cookies; response body now contains only { user }
  • logout reads refreshToken from cookie instead of request body; clears both cookies on success
  • refresh reads refreshToken from cookie, rotates both tokens via Set-Cookie, returns { success: true }
  • verifyJWT preHandler reads from request.cookies.accessToken instead of Authorization header
  • Removed refreshToken from logoutSchema and refreshSchema request bodies; updated response schemas

Client

  • Removed all token storage code (sessionStorage/localStorage, setTokens, clearTokens, getAccessToken)
  • Added credentials: 'include' to all fetch() calls so the browser automatically sends cookies
  • AuthProvider always checks session on mount via GET /api/auth/verify; login() reads user from response body
  • Landing.tsx now redirects to /link-bank when agentBudgetApproved is false instead of always sending authenticated users to /dashboard

Tests

  • All unit and integration tests updated: headers: { authorization: 'Bearer ...' }cookies: { accessToken: token }
  • @fastify/cookie registered in every test app builder that exercises protected routes
  • Logout/refresh tests updated to pass tokens via cookies instead of request body
  • Schema and controller tests updated to reflect cookie-based token delivery and new response shapes

Test plan

  • Fresh page load with no session → lands on login page (401 on /api/auth/verify is expected and silent)
  • Login → cookies set, redirected to dashboard or /link-bank based on onboarding state
  • Refresh page after login → session persists, dashboard loads correctly
  • Close tab and reopen → session persists (cookies survive tab close unlike sessionStorage)
  • Logout → cookies cleared, redirected to login; subsequent protected requests return 401
  • Login with wrong password → 401 with "Invalid email or password"
  • All 52 test files pass: npm test in /server
  • Client lint and build pass: npm run lint && npm run build in /client
  • Server lint and build pass: npm run lint && npm run build in /server

TimPolk and others added 30 commits March 30, 2026 12:20
gburger5
gburger5 previously approved these changes Apr 10, 2026
@gburger5 gburger5 dismissed their stale review April 10, 2026 14:29

The merge-base changed after approval.

gburger5
gburger5 previously approved these changes Apr 10, 2026
@YounBrand YounBrand dismissed gburger5’s stale review April 10, 2026 14:30

The merge-base changed after approval.

gburger5
gburger5 previously approved these changes Apr 10, 2026
@YounBrand YounBrand dismissed gburger5’s stale review April 10, 2026 14:43

The merge-base changed after approval.

TimPolk
TimPolk previously approved these changes Apr 12, 2026
@YounBrand YounBrand dismissed TimPolk’s stale review April 12, 2026 16:07

The merge-base changed after approval.

TimPolk
TimPolk previously approved these changes Apr 12, 2026
Copy link
Copy Markdown
Collaborator

@TimPolk TimPolk left a comment

Choose a reason for hiding this comment

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

The changes made look good

@YounBrand YounBrand requested a review from TimPolk April 12, 2026 16:12
@YounBrand YounBrand dismissed TimPolk’s stale review April 12, 2026 16:12

The merge-base changed after approval.

Copy link
Copy Markdown
Owner

@gburger5 gburger5 left a comment

Choose a reason for hiding this comment

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

Looks good

@gburger5 gburger5 removed the request for review from TimPolk April 12, 2026 16:15
gburger5
gburger5 previously approved these changes Apr 12, 2026
@YounBrand YounBrand dismissed gburger5’s stale review April 12, 2026 16:21

The merge-base changed after approval.

TimPolk
TimPolk previously approved these changes Apr 12, 2026
@YounBrand YounBrand dismissed TimPolk’s stale review April 12, 2026 16:26

The merge-base changed after approval.

gburger5
gburger5 previously approved these changes Apr 12, 2026
@YounBrand YounBrand dismissed gburger5’s stale review April 12, 2026 16:31

The merge-base changed after approval.

@YounBrand YounBrand closed this Apr 12, 2026
@YounBrand YounBrand deleted the httponly-cookies branch April 12, 2026 21:00
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.

4 participants