Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Added `/api/avatar` to resolve user profile pictures. [#1159](https://github.com/sourcebot-dev/sourcebot/pull/1159)
- 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)
- 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)
- 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)
Comment thread
brendan-kellam marked this conversation as resolved.

### Fixed
- Bumped `postcss` to `8.5.10`. [#1155](https://github.com/sourcebot-dev/sourcebot/pull/1155)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { apiHandler } from '@/lib/apiHandler';
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
import { env, hasEntitlement } from '@sourcebot/shared';
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';

// RFC 8414: OAuth 2.0 Authorization Server Metadata
// @see: https://datatracker.ietf.org/doc/html/rfc8414
export const GET = apiHandler(async () => {
export const GET = oauthApiHandler(async () => {
if (!hasEntitlement('oauth')) {
return Response.json(
{ error: 'not_found', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { apiHandler } from '@/lib/apiHandler';
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
import { env, hasEntitlement } from '@sourcebot/shared';
import { NextRequest } from 'next/server';
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';
Expand All @@ -10,7 +10,7 @@ const PROTECTED_RESOURCES = new Set([
'api/mcp'
]);

export const GET = apiHandler(async (_request: NextRequest, { params }: { params: Promise<{ path: string[] }> }) => {
export const GET = oauthApiHandler(async (_request: NextRequest, { params }: { params: Promise<{ path: string[] }> }) => {
if (!hasEntitlement('oauth')) {
return Response.json(
{ error: 'not_found', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },
Expand Down
4 changes: 2 additions & 2 deletions packages/web/src/app/api/(server)/ee/oauth/register/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { apiHandler } from '@/lib/apiHandler';
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
import { requestBodySchemaValidationError, serviceErrorResponse } from '@/lib/serviceError';
import { __unsafePrisma } from '@/prisma';
import { hasEntitlement } from '@sourcebot/shared';
Expand All @@ -14,7 +14,7 @@ const registerRequestSchema = z.object({
logo_uri: z.string().url().nullish(),
});

export const POST = apiHandler(async (request: NextRequest) => {
export const POST = oauthApiHandler(async (request: NextRequest) => {
if (!hasEntitlement('oauth')) {
return Response.json(
{ error: 'access_denied', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },
Expand Down
4 changes: 2 additions & 2 deletions packages/web/src/app/api/(server)/ee/oauth/revoke/route.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { revokeToken } from '@/ee/features/oauth/server';
import { apiHandler } from '@/lib/apiHandler';
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
import { hasEntitlement } from '@sourcebot/shared';
import { NextRequest } from 'next/server';
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';

// RFC 7009: OAuth 2.0 Token Revocation
// Always returns 200 regardless of whether the token existed.
// @see: https://datatracker.ietf.org/doc/html/rfc7009
export const POST = apiHandler(async (request: NextRequest) => {
export const POST = oauthApiHandler(async (request: NextRequest) => {
if (!hasEntitlement('oauth')) {
return Response.json(
{ error: 'access_denied', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },
Expand Down
4 changes: 2 additions & 2 deletions packages/web/src/app/api/(server)/ee/oauth/token/route.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { verifyAndExchangeCode, verifyAndRotateRefreshToken } from '@/ee/features/oauth/server';
import { apiHandler } from '@/lib/apiHandler';
import { oauthApiHandler } from '@/ee/features/oauth/apiHandler';
import { env, hasEntitlement } from '@sourcebot/shared';
import { NextRequest } from 'next/server';
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';

// OAuth 2.0 Token Endpoint
// Supports grant_type=authorization_code with PKCE (RFC 7636).
// @see: https://datatracker.ietf.org/doc/html/rfc6749#section-3.2
export const POST = apiHandler(async (request: NextRequest) => {
export const POST = oauthApiHandler(async (request: NextRequest) => {
if (!hasEntitlement('oauth')) {
return Response.json(
{ error: 'access_denied', error_description: OAUTH_NOT_SUPPORTED_ERROR_MESSAGE },
Expand Down
53 changes: 53 additions & 0 deletions packages/web/src/ee/features/oauth/apiHandler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { describe, expect, test, vi } from 'vitest';
import { NextRequest } from 'next/server';
import { oauthApiHandler } from './apiHandler';

vi.mock('@/lib/posthog', () => ({
captureEvent: vi.fn(),
}));

const makeRequest = () => new NextRequest('http://localhost/api/ee/oauth/test', { method: 'POST' });

describe('oauthApiHandler', () => {
test('passes through a 200 response unchanged', async () => {
const handler = oauthApiHandler(async (_req: NextRequest) => Response.json({ ok: true }, { status: 200 }));
const res = await handler(makeRequest());
expect(res.status).toBe(200);
});

test('passes through a 400 response unchanged', async () => {
const handler = oauthApiHandler(async (_req: NextRequest) => Response.json({ error: 'bad' }, { status: 400 }));
const res = await handler(makeRequest());
expect(res.status).toBe(400);
});

test('passes through a 302 redirect unchanged', async () => {
const handler = oauthApiHandler(async (_req: NextRequest) =>
new Response(null, { status: 302, headers: { Location: '/elsewhere' } })
);
const res = await handler(makeRequest());
expect(res.status).toBe(302);
});

test('passes through a 303 redirect unchanged (the spec-recommended status)', async () => {
const handler = oauthApiHandler(async (_req: NextRequest) =>
new Response(null, { status: 303, headers: { Location: '/elsewhere' } })
);
const res = await handler(makeRequest());
expect(res.status).toBe(303);
});

test('throws when the inner handler returns 307', async () => {
const handler = oauthApiHandler(async (_req: NextRequest) =>
new Response(null, { status: 307, headers: { Location: '/elsewhere' } })
);
await expect(handler(makeRequest())).rejects.toThrow(/RFC 9700/);
});

test('throws when the inner handler returns 308', async () => {
const handler = oauthApiHandler(async (_req: NextRequest) =>
new Response(null, { status: 308, headers: { Location: '/elsewhere' } })
);
await expect(handler(makeRequest())).rejects.toThrow(/RFC 9700/);
});
});
32 changes: 32 additions & 0 deletions packages/web/src/ee/features/oauth/apiHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { apiHandler } from '@/lib/apiHandler';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type AnyHandler = (...args: any[]) => Promise<Response> | Response;

/**
* Wraps `apiHandler` for routes that are part of the OAuth Authorization Server
* (`/api/ee/oauth/*`).
*
* Per RFC 9700 §4.12, the authorization server MUST avoid forwarding user
* credentials accidentally on redirect. 307 and 308 preserve the request
* method and body, so they MUST NOT be used for authorization-server
* redirects. This wrapper asserts that handlers never emit either status,
* giving us a runtime guarantee.
*
* @see https://datatracker.ietf.org/doc/html/rfc9700#section-4.12
*/
export function oauthApiHandler<H extends AnyHandler>(handler: H): H {
const wrapped = apiHandler(async (...args: Parameters<H>) => {
const response = await handler(...args);
if (response.status === 307 || response.status === 308) {
throw new Error(
`OAuth authorization server emitted HTTP ${response.status} redirect; ` +
`per RFC 9700 §4.12 the authorization server MUST NOT use 307/308. ` +
`Use 303 (See Other) instead.`
);
}
return response;
});

return wrapped as H;
}
Loading