From 8855aa99f7293813a1b9ac63a794a8b9328d5c16 Mon Sep 17 00:00:00 2001
From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com>
Date: Fri, 3 Apr 2026 12:02:56 +0200
Subject: [PATCH] fix: redirect bank OAuth callback to /settings/banking (#165)

* fix: redirect bank OAuth callback to /settings/banking

The callback was redirecting to /settings which immediately redirects to
/settings/company, dropping the bank_connected and connection_id query
params. The auto-sync code that triggers the initial 90-day transaction
fetch lives in /settings/banking and never received the params.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: add coverage for invalid_code_format redirect path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 .../callback/__tests__/route.test.ts               | 14 ++++++++++++++
 .../extensions/enable-banking/callback/route.ts    | 12 ++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts
index 9b0a8a5b..7f8d1d95 100644
--- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts
+++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts
@@ -57,6 +57,7 @@ describe('GET /api/extensions/enable-banking/callback', () => {
 
     expect(response.status).toBe(307)
     const location = response.headers.get('location') || ''
+    expect(location).toContain('/settings/banking?')
     expect(location).toContain('bank_error=invalid_state')
   })
 
@@ -87,6 +88,7 @@ describe('GET /api/extensions/enable-banking/callback', () => {
 
     expect(response.status).toBe(307)
     const location = response.headers.get('location') || ''
+    expect(location).toContain('/settings/banking?')
     expect(location).toContain('bank_connected=true')
     expect(location).toContain('connection_id=conn-1')
   })
@@ -96,6 +98,7 @@ describe('GET /api/extensions/enable-banking/callback', () => {
 
     expect(response.status).toBe(307)
     const location = response.headers.get('location') || ''
+    expect(location).toContain('/settings/banking?')
     expect(location).toContain('bank_error=User%20cancelled')
     // No state → no DB cleanup attempted
     expect(mockFrom).not.toHaveBeenCalled()
@@ -114,6 +117,7 @@ describe('GET /api/extensions/enable-banking/callback', () => {
 
     expect(response.status).toBe(307)
     const location = response.headers.get('location') || ''
+    expect(location).toContain('/settings/banking?')
     expect(location).toContain('bank_error=Denied%20data%20sharing%20consent')
     // Should clean up the pending row
     expect(mockFrom).toHaveBeenCalledWith('bank_connections')
@@ -124,6 +128,16 @@ describe('GET /api/extensions/enable-banking/callback', () => {
 
     expect(response.status).toBe(307)
     const location = response.headers.get('location') || ''
+    expect(location).toContain('/settings/banking?')
     expect(location).toContain('bank_error=missing_parameters')
   })
+
+  it('redirects with error when code fails format validation', async () => {
+    const response = await GET(makeRequest({ code: '!!bad!!', state: 'some-state' }))
+
+    expect(response.status).toBe(307)
+    const location = response.headers.get('location') || ''
+    expect(location).toContain('/settings/banking?')
+    expect(location).toContain('bank_error=invalid_code_format')
+  })
 })
diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts
index 47dbace5..c5cea534 100644
--- a/app/api/extensions/enable-banking/callback/route.ts
+++ b/app/api/extensions/enable-banking/callback/route.ts
@@ -61,18 +61,18 @@ export async function GET(request: Request) {
     }
 
     return NextResponse.redirect(
-      `${baseUrl}/settings?bank_error=${encodeURIComponent(errorMessage)}`
+      `${baseUrl}/settings/banking?bank_error=${encodeURIComponent(errorMessage)}`
     )
   }
 
   if (!code || !state) {
-    return NextResponse.redirect(`${baseUrl}/settings?bank_error=missing_parameters`)
+    return NextResponse.redirect(`${baseUrl}/settings/banking?bank_error=missing_parameters`)
   }
 
   // Validate authorization code format
   const codePattern = /^[a-zA-Z0-9._~+\/-]{8,2048}$/
   if (!codePattern.test(code)) {
-    return NextResponse.redirect(`${baseUrl}/settings?bank_error=invalid_code_format`)
+    return NextResponse.redirect(`${baseUrl}/settings/banking?bank_error=invalid_code_format`)
   }
 
   const supabase = await createServiceClient()
@@ -93,7 +93,7 @@ export async function GET(request: Request) {
         hasCode: !!code,
       })
       return NextResponse.redirect(
-        `${baseUrl}/settings?bank_error=${encodeURIComponent('invalid_state')}`
+        `${baseUrl}/settings/banking?bank_error=${encodeURIComponent('invalid_state')}`
       )
     }
 
@@ -171,7 +171,7 @@ export async function GET(request: Request) {
       .single()
 
     const redirectTarget = userSettings?.onboarding_complete
-      ? `/settings?bank_connected=true&connection_id=${connectionId}`
+      ? `/settings/banking?bank_connected=true&connection_id=${connectionId}`
       : `/onboarding?bank_connected=true&connection_id=${connectionId}`
 
     return NextResponse.redirect(`${baseUrl}${redirectTarget}`)
@@ -197,7 +197,7 @@ export async function GET(request: Request) {
     }
 
     return NextResponse.redirect(
-      `${baseUrl}/settings?bank_error=${encodeURIComponent('Connection failed')}`
+      `${baseUrl}/settings/banking?bank_error=${encodeURIComponent('Connection failed')}`
     )
   }
 }