Skip to content

Commit 59e0f0e

Browse files
brendan-kellamclaude
brendan-kellam
and
claude
authored
chore(web): guard OAuth API routes against 307/308 redirects (#1163)
* chore(web): guard OAuth API routes against 307/308 redirects Adds `oauthApiHandler`, a thin wrapper around `apiHandler` that throws if the wrapped handler ever returns an HTTP 307 or 308 response. Per RFC 9700 §4.12, an OAuth authorization server must not use 307/308 on redirects that could carry user credentials, since those status codes preserve the request method and body. Wires `oauthApiHandler` into all five authorization-server route handlers — the token, register, and revoke endpoints under `/api/ee/oauth/*`, plus the two RFC 8414 / RFC 9728 discovery endpoints under `/api/ee/.well-known/*` — so any future change that accidentally introduces a 307/308 from these routes throws at request time rather than silently shipping. Includes unit tests verifying the wrapper passes through 200/302/303/400 responses unchanged and throws on 307 and 308. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * changelog --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d691e90 commit 59e0f0e

8 files changed

Lines changed: 96 additions & 10 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2020
- Added `/api/avatar` to resolve user profile pictures. [#1159](https://github.com/sourcebot-dev/sourcebot/pull/1159)
2121
- Hardened post-auth redirects with an explicit same-origin `redirect` callback in the NextAuth config, and switched the legacy `/~/...` URL rewrite from a 308 to a 301. [#1161](https://github.com/sourcebot-dev/sourcebot/pull/1161)
2222
- Made the Auth.js JWT session lifetime and OAuth token TTLs configurable via `AUTH_SESSION_MAX_AGE_SECONDS`, `AUTH_SESSION_UPDATE_AGE_SECONDS`, `OAUTH_AUTHORIZATION_CODE_TTL_SECONDS`, `OAUTH_ACCESS_TOKEN_TTL_SECONDS`, and `OAUTH_REFRESH_TOKEN_TTL_SECONDS`. Defaults preserve existing behavior. [#1162](https://github.com/sourcebot-dev/sourcebot/pull/1162)
23+
- Guarded all OAuth authorization-server route handlers with a runtime assertion that rejects HTTP 307 and 308 responses, per RFC 9700 §4.12. [#1163](https://github.com/sourcebot-dev/sourcebot/pull/1163)
2324

2425
### Fixed
2526
- Bumped `postcss` to `8.5.10`. [#1155](https://github.com/sourcebot-dev/sourcebot/pull/1155)

‎packages/web/src/app/api/(server)/ee/.well-known/oauth-authorization-server/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { apiHandler } from '@/lib/apiHandler';
1+
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
22
import { env, hasEntitlement } from '@sourcebot/shared';
33
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';
44

55
// RFC 8414: OAuth 2.0 Authorization Server Metadata
66
// @see: https://datatracker.ietf.org/doc/html/rfc8414
7-
export const GET = apiHandler(async () => {
7+
export const GET = oauthApiHandler(async () => {
88
if (!hasEntitlement('oauth')) {
99
return Response.json(
1010
{ error: 'not_found', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },

‎packages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/[...path]/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { apiHandler } from '@/lib/apiHandler';
1+
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
22
import { env, hasEntitlement } from '@sourcebot/shared';
33
import { NextRequest } from 'next/server';
44
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';
@@ -10,7 +10,7 @@ const PROTECTED_RESOURCES = new Set([
1010
'api/mcp'
1111
]);
1212

13-
export const GET = apiHandler(async (_request: NextRequest, { params }: { params: Promise<{ path: string[] }> }) => {
13+
export const GET = oauthApiHandler(async (_request: NextRequest, { params }: { params: Promise<{ path: string[] }> }) => {
1414
if (!hasEntitlement('oauth')) {
1515
return Response.json(
1616
{ error: 'not_found', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },

‎packages/web/src/app/api/(server)/ee/oauth/register/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { apiHandler } from '@/lib/apiHandler';
1+
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
22
import { requestBodySchemaValidationError, serviceErrorResponse } from '@/lib/serviceError';
33
import { __unsafePrisma } from '@/prisma';
44
import { hasEntitlement } from '@sourcebot/shared';
@@ -14,7 +14,7 @@ const registerRequestSchema = z.object({
1414
logo_uri: z.string().url().nullish(),
1515
});
1616

17-
export const POST = apiHandler(async (request: NextRequest) => {
17+
export const POST = oauthApiHandler(async (request: NextRequest) => {
1818
if (!hasEntitlement('oauth')) {
1919
return Response.json(
2020
{ error: 'access_denied', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },

‎packages/web/src/app/api/(server)/ee/oauth/revoke/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { revokeToken } from '@/ee/features/oauth/server';
2-
import { apiHandler } from '@/lib/apiHandler';
2+
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
33
import { hasEntitlement } from '@sourcebot/shared';
44
import { NextRequest } from 'next/server';
55
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';
66

77
// RFC 7009: OAuth 2.0 Token Revocation
88
// Always returns 200 regardless of whether the token existed.
99
// @see: https://datatracker.ietf.org/doc/html/rfc7009
10-
export const POST = apiHandler(async (request: NextRequest) => {
10+
export const POST = oauthApiHandler(async (request: NextRequest) => {
1111
if (!hasEntitlement('oauth')) {
1212
return Response.json(
1313
{ error: 'access_denied', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },

‎packages/web/src/app/api/(server)/ee/oauth/token/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { verifyAndExchangeCode, verifyAndRotateRefreshToken } from '@/ee/features/oauth/server';
2-
import { apiHandler } from '@/lib/apiHandler';
2+
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
33
import { env, hasEntitlement } from '@sourcebot/shared';
44
import { NextRequest } from 'next/server';
55
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';
66

77
// OAuth 2.0 Token Endpoint
88
// Supports grant_type=authorization_code with PKCE (RFC 7636).
99
// @see: https://datatracker.ietf.org/doc/html/rfc6749#section-3.2
10-
export const POST = apiHandler(async (request: NextRequest) => {
10+
export const POST = oauthApiHandler(async (request: NextRequest) => {
1111
if (!hasEntitlement('oauth')) {
1212
return Response.json(
1313
{ error: 'access_denied', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },

‎packages/web/src/ee/features/oauth/apiHandler.test.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { describe, expect, test, vi } from 'vitest';
2+
import { NextRequest } from 'next/server';
3+
import { oauthApiHandler } from './apiHandler';
4+
5+
vi.mock('@/lib/posthog', () => ({
6+
captureEvent: vi.fn(),
7+
}));
8+
9+
const makeRequest = () => new NextRequest('http://localhost/api/ee/oauth/test', { method: 'POST' });
10+
11+
describe('oauthApiHandler', () => {
12+
test('passes through a 200 response unchanged', async () => {
13+
const handler = oauthApiHandler(async (_req: NextRequest) => Response.json({ ok: true }, { status: 200 }));
14+
const res = await handler(makeRequest());
15+
expect(res.status).toBe(200);
16+
});
17+
18+
test('passes through a 400 response unchanged', async () => {
19+
const handler = oauthApiHandler(async (_req: NextRequest) => Response.json({ error: 'bad' }, { status: 400 }));
20+
const res = await handler(makeRequest());
21+
expect(res.status).toBe(400);
22+
});
23+
24+
test('passes through a 302 redirect unchanged', async () => {
25+
const handler = oauthApiHandler(async (_req: NextRequest) =>
26+
new Response(null, { status: 302, headers: { Location: '/elsewhere' } })
27+
);
28+
const res = await handler(makeRequest());
29+
expect(res.status).toBe(302);
30+
});
31+
32+
test('passes through a 303 redirect unchanged (the spec-recommended status)', async () => {
33+
const handler = oauthApiHandler(async (_req: NextRequest) =>
34+
new Response(null, { status: 303, headers: { Location: '/elsewhere' } })
35+
);
36+
const res = await handler(makeRequest());
37+
expect(res.status).toBe(303);
38+
});
39+
40+
test('throws when the inner handler returns 307', async () => {
41+
const handler = oauthApiHandler(async (_req: NextRequest) =>
42+
new Response(null, { status: 307, headers: { Location: '/elsewhere' } })
43+
);
44+
await expect(handler(makeRequest())).rejects.toThrow(/RFC 9700/);
45+
});
46+
47+
test('throws when the inner handler returns 308', async () => {
48+
const handler = oauthApiHandler(async (_req: NextRequest) =>
49+
new Response(null, { status: 308, headers: { Location: '/elsewhere' } })
50+
);
51+
await expect(handler(makeRequest())).rejects.toThrow(/RFC 9700/);
52+
});
53+
});

‎packages/web/src/ee/features/oauth/apiHandler.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { apiHandler } from '@/lib/apiHandler';
2+
3+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4+
type AnyHandler = (...args: any[]) => Promise<Response> | Response;
5+
6+
/**
7+
* Wraps `apiHandler` for routes that are part of the OAuth Authorization Server
8+
* (`/api/ee/oauth/*`).
9+
*
10+
* Per RFC 9700 §4.12, the authorization server MUST avoid forwarding user
11+
* credentials accidentally on redirect. 307 and 308 preserve the request
12+
* method and body, so they MUST NOT be used for authorization-server
13+
* redirects. This wrapper asserts that handlers never emit either status,
14+
* giving us a runtime guarantee.
15+
*
16+
* @see https://datatracker.ietf.org/doc/html/rfc9700#section-4.12
17+
*/
18+
export function oauthApiHandler<H extends AnyHandler>(handler: H): H {
19+
const wrapped = apiHandler(async (...args: Parameters<H>) => {
20+
const response = await handler(...args);
21+
if (response.status === 307 || response.status === 308) {
22+
throw new Error(
23+
`OAuth authorization server emitted HTTP ${response.status} redirect; ` +
24+
`per RFC 9700 §4.12 the authorization server MUST NOT use 307/308. ` +
25+
`Use 303 (See Other) instead.`
26+
);
27+
}
28+
return response;
29+
});
30+
31+
return wrapped as H;
32+
}

0 commit comments

Comments
 (0)