Skip to content

Commit a9777cb

Browse files
committed
Preserve protected app redirects
1 parent 70ebb2b commit a9777cb

6 files changed

Lines changed: 170 additions & 4 deletions

File tree

apps/web/app/app/layout.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { redirect } from 'next/navigation';
2+
import { headers } from 'next/headers';
23
import { checkBetaAccess } from '@/lib/auth/beta-access';
4+
import { getProtectedAppRedirectTarget } from '@/lib/auth/app-redirect';
35

46
export const dynamic = 'force-dynamic';
57

@@ -9,14 +11,19 @@ export default async function AppLayout({
911
children: React.ReactNode;
1012
}) {
1113
const { user, betaApproved, status } = await checkBetaAccess();
14+
const requestHeaders = await headers();
15+
const redirectTarget = getProtectedAppRedirectTarget(
16+
requestHeaders.get('x-th-pathname'),
17+
requestHeaders.get('x-th-search')
18+
);
1219

1320
if (status === 'unavailable') {
1421
throw new Error('Authentication service unavailable');
1522
}
1623

1724
// Not authenticated
1825
if (!user || status === 'unauthenticated') {
19-
redirect('/login?redirect=' + encodeURIComponent('/app'));
26+
redirect('/login?redirect=' + encodeURIComponent(redirectTarget));
2027
}
2128

2229
// Authenticated but not approved for beta

apps/web/lib/auth/app-redirect.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export const APP_REDIRECT_FALLBACK = '/app'
2+
3+
export function isProtectedAppPath(pathname: string) {
4+
return pathname === '/app' || pathname.startsWith('/app/')
5+
}
6+
7+
export function getProtectedAppRedirectTarget(
8+
pathname: string | null,
9+
search: string | null
10+
) {
11+
const hasSafePathname = pathname != null && isProtectedAppPath(pathname)
12+
const safePathname = hasSafePathname
13+
? pathname
14+
: APP_REDIRECT_FALLBACK
15+
const safeSearch = hasSafePathname && search?.startsWith('?') ? search : ''
16+
17+
return `${safePathname}${safeSearch}`
18+
}

apps/web/middleware.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
import { createServerClient } from '@supabase/ssr'
22
import { NextResponse, type NextRequest } from 'next/server'
3+
import { isProtectedAppPath } from './lib/auth/app-redirect'
34

45
export async function middleware(request: NextRequest) {
5-
let supabaseResponse = NextResponse.next({ request })
6+
const requestHeaders = new Headers(request.headers)
7+
requestHeaders.set('x-th-pathname', request.nextUrl.pathname)
8+
requestHeaders.set('x-th-search', request.nextUrl.search)
9+
10+
let supabaseResponse = NextResponse.next({
11+
request: { headers: requestHeaders },
12+
})
613
const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL
714
const supabaseAnonKey = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY
815

@@ -20,7 +27,9 @@ export async function middleware(request: NextRequest) {
2027
},
2128
setAll(cookiesToSet) {
2229
cookiesToSet.forEach(({ name, value }) => request.cookies.set(name, value))
23-
supabaseResponse = NextResponse.next({ request })
30+
supabaseResponse = NextResponse.next({
31+
request: { headers: requestHeaders },
32+
})
2433
cookiesToSet.forEach(({ name, value, options }) =>
2534
supabaseResponse.cookies.set(name, value, options)
2635
)
@@ -31,7 +40,20 @@ export async function middleware(request: NextRequest) {
3140

3241
// IMPORTANT: getUser() validates JWT and refreshes if needed
3342
// Do NOT use getSession() - it only validates locally
34-
await supabase.auth.getUser()
43+
const {
44+
data: { user },
45+
} = await supabase.auth.getUser()
46+
47+
if (!user && isProtectedAppPath(request.nextUrl.pathname)) {
48+
const url = new URL(request.nextUrl.toString())
49+
url.pathname = '/login'
50+
url.search = ''
51+
url.searchParams.set(
52+
'redirect',
53+
`${request.nextUrl.pathname}${request.nextUrl.search}`
54+
)
55+
return NextResponse.redirect(url)
56+
}
3557

3658
return supabaseResponse
3759
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
import AppLayout from '@/app/app/layout'
3+
import { checkBetaAccess } from '@/lib/auth/beta-access'
4+
import { headers } from 'next/headers'
5+
import { redirect } from 'next/navigation'
6+
7+
vi.mock('@/lib/auth/beta-access', () => ({
8+
checkBetaAccess: vi.fn(),
9+
}))
10+
11+
vi.mock('next/headers', () => ({
12+
headers: vi.fn(),
13+
}))
14+
15+
vi.mock('next/navigation', () => ({
16+
redirect: vi.fn((url: string) => {
17+
throw new Error(`redirect:${url}`)
18+
}),
19+
}))
20+
21+
describe('AppLayout', () => {
22+
beforeEach(() => {
23+
vi.clearAllMocks()
24+
vi.mocked(headers).mockResolvedValue(new Headers() as never)
25+
})
26+
27+
it('preserves nested protected app routes in the login redirect', async () => {
28+
vi.mocked(headers).mockResolvedValue(
29+
new Headers([
30+
['x-th-pathname', '/app/admin/beta'],
31+
['x-th-search', ''],
32+
]) as never
33+
)
34+
vi.mocked(checkBetaAccess).mockResolvedValue({
35+
user: null,
36+
betaApproved: false,
37+
status: 'unauthenticated',
38+
isAdmin: false,
39+
error: 'No authenticated user',
40+
})
41+
42+
await expect(AppLayout({ children: <div /> })).rejects.toThrow(
43+
'redirect:/login?redirect=%2Fapp%2Fadmin%2Fbeta'
44+
)
45+
expect(redirect).toHaveBeenCalledWith(
46+
'/login?redirect=%2Fapp%2Fadmin%2Fbeta'
47+
)
48+
})
49+
})

apps/web/tests/integration/middleware-public-routes.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,17 @@ const nextResponse = {
1313
},
1414
}
1515

16+
const redirectResponse = {
17+
status: 307,
18+
cookies: {
19+
set: vi.fn(),
20+
},
21+
}
22+
1623
vi.mock('next/server', () => ({
1724
NextResponse: {
1825
next: vi.fn(() => nextResponse),
26+
redirect: vi.fn(() => redirectResponse),
1927
},
2028
}))
2129

@@ -73,4 +81,40 @@ describe('root middleware public-route resilience', () => {
7381
)
7482
expect(getUser).toHaveBeenCalled()
7583
})
84+
85+
it('forwards the attempted path to app layout redirects', async () => {
86+
delete process.env.NEXT_PUBLIC_SUPABASE_URL
87+
delete process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY
88+
89+
await middleware(requestFor('/app/admin/beta?tab=invites'))
90+
91+
const { NextResponse } = await import('next/server')
92+
const nextOptions = vi.mocked(NextResponse.next).mock.calls[0]?.[0]
93+
const headers = nextOptions?.request?.headers
94+
95+
expect(headers).toBeInstanceOf(Headers)
96+
expect(headers?.get('x-th-pathname')).toBe('/app/admin/beta')
97+
expect(headers?.get('x-th-search')).toBe('?tab=invites')
98+
})
99+
100+
it('preserves the protected app path in the login redirect', async () => {
101+
process.env.NEXT_PUBLIC_SUPABASE_URL = 'https://example.supabase.co'
102+
process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY = 'anon-key'
103+
vi.mocked(createServerClient).mockReturnValue({
104+
auth: {
105+
getUser: vi.fn().mockResolvedValue({ data: { user: null }, error: null }),
106+
},
107+
} as unknown as SupabaseClient)
108+
109+
const response = await middleware(requestFor('/app/admin/beta'))
110+
111+
expect(response.status).toBe(307)
112+
const { NextResponse } = await import('next/server')
113+
expect(NextResponse.redirect).toHaveBeenCalledWith(
114+
expect.objectContaining({
115+
pathname: '/login',
116+
search: `?redirect=${encodeURIComponent('/app/admin/beta')}`,
117+
})
118+
)
119+
})
76120
})
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from 'vitest'
2+
import {
3+
getProtectedAppRedirectTarget,
4+
isProtectedAppPath,
5+
} from '@/lib/auth/app-redirect'
6+
7+
describe('getProtectedAppRedirectTarget', () => {
8+
it('preserves nested app routes and query strings', () => {
9+
expect(
10+
getProtectedAppRedirectTarget('/app/admin/beta', '?tab=invites')
11+
).toBe('/app/admin/beta?tab=invites')
12+
})
13+
14+
it('keeps the app dashboard as a valid target', () => {
15+
expect(getProtectedAppRedirectTarget('/app', '')).toBe('/app')
16+
})
17+
18+
it('falls back to app dashboard for non-app paths', () => {
19+
expect(getProtectedAppRedirectTarget('/login', '?redirect=/evil')).toBe('/app')
20+
})
21+
22+
it('does not treat app-like public paths as protected app routes', () => {
23+
expect(isProtectedAppPath('/apple')).toBe(false)
24+
expect(isProtectedAppPath('/appetite')).toBe(false)
25+
})
26+
})

0 commit comments

Comments
 (0)