Skip to content

Commit 7fb8e4a

Browse files
committed
Catch and format standard error types in responses
1 parent 57fe3b3 commit 7fb8e4a

File tree

8 files changed

+265
-195
lines changed

8 files changed

+265
-195
lines changed

src/server/auth/errors.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,14 @@ export class TooManyRequestsError extends OAuthError {
169169
constructor(message: string, errorUri?: string) {
170170
super("too_many_requests", message, errorUri);
171171
}
172+
}
173+
174+
/**
175+
* Invalid client metadata error - The client metadata is invalid.
176+
* (Custom error for dynamic client registration - RFC 7591)
177+
*/
178+
export class InvalidClientMetadataError extends OAuthError {
179+
constructor(message: string, errorUri?: string) {
180+
super("invalid_client_metadata", message, errorUri);
181+
}
172182
}

src/server/auth/handlers/authorize.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ describe('Authorization Handler', () => {
113113
.query({ client_id: 'nonexistent-client' });
114114

115115
expect(response.status).toBe(400);
116-
expect(response.text).toContain('invalid client_id');
117116
});
118117
});
119118

@@ -144,7 +143,6 @@ describe('Authorization Handler', () => {
144143
});
145144

146145
expect(response.status).toBe(400);
147-
expect(response.text).toContain('missing redirect_uri');
148146
});
149147

150148
it('validates redirect_uri against client registered URIs', async () => {
@@ -159,7 +157,6 @@ describe('Authorization Handler', () => {
159157
});
160158

161159
expect(response.status).toBe(400);
162-
expect(response.text).toContain('unregistered redirect_uri');
163160
});
164161

165162
it('accepts valid redirect_uri that client registered with', async () => {

src/server/auth/handlers/authorize.ts

Lines changed: 98 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ import express from "express";
44
import { OAuthServerProvider } from "../provider.js";
55
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
66
import { allowedMethods } from "../middleware/allowedMethods.js";
7+
import {
8+
InvalidRequestError,
9+
InvalidClientError,
10+
InvalidScopeError,
11+
ServerError,
12+
TooManyRequestsError,
13+
OAuthError
14+
} from "../errors.js";
715

816
export type AuthorizationHandlerOptions = {
917
provider: OAuthServerProvider;
@@ -42,81 +50,116 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
4250
max: 100, // 100 requests per windowMs
4351
standardHeaders: true,
4452
legacyHeaders: false,
45-
message: {
46-
error: 'too_many_requests',
47-
error_description: 'You have exceeded the rate limit for authorization requests'
48-
},
53+
message: new TooManyRequestsError('You have exceeded the rate limit for authorization requests').toResponseObject(),
4954
...rateLimitConfig
5055
}));
5156
}
5257

53-
// Define the handler
5458
router.all("/", async (req, res) => {
5559
res.setHeader('Cache-Control', 'no-store');
5660

57-
let client_id, redirect_uri;
61+
// In the authorization flow, errors are split into two categories:
62+
// 1. Pre-redirect errors (direct response with 400)
63+
// 2. Post-redirect errors (redirect with error parameters)
64+
65+
// Phase 1: Validate client_id and redirect_uri. Any errors here must be direct responses.
66+
let client_id, redirect_uri, client;
5867
try {
59-
const data = req.method === 'POST' ? req.body : req.query;
60-
({ client_id, redirect_uri } = ClientAuthorizationParamsSchema.parse(data));
61-
} catch (error) {
62-
res.status(400).end(`Bad Request: ${error}`);
63-
return;
64-
}
68+
const result = ClientAuthorizationParamsSchema.safeParse(req.method === 'POST' ? req.body : req.query);
69+
if (!result.success) {
70+
throw new InvalidRequestError(result.error.message);
71+
}
6572

66-
const client = await provider.clientsStore.getClient(client_id);
67-
if (!client) {
68-
res.status(400).end("Bad Request: invalid client_id");
69-
return;
70-
}
73+
client_id = result.data.client_id;
74+
redirect_uri = result.data.redirect_uri;
7175

72-
if (redirect_uri !== undefined) {
73-
if (!client.redirect_uris.includes(redirect_uri)) {
74-
res.status(400).end("Bad Request: unregistered redirect_uri");
75-
return;
76+
client = await provider.clientsStore.getClient(client_id);
77+
if (!client) {
78+
throw new InvalidClientError("Invalid client_id");
7679
}
77-
} else if (client.redirect_uris.length === 1) {
78-
redirect_uri = client.redirect_uris[0];
79-
} else {
80-
res.status(400).end("Bad Request: missing redirect_uri");
81-
return;
82-
}
8380

84-
let params;
85-
try {
86-
const authData = req.method === 'POST' ? req.body : req.query;
87-
params = RequestAuthorizationParamsSchema.parse(authData);
81+
if (redirect_uri !== undefined) {
82+
if (!client.redirect_uris.includes(redirect_uri)) {
83+
throw new InvalidRequestError("Unregistered redirect_uri");
84+
}
85+
} else if (client.redirect_uris.length === 1) {
86+
redirect_uri = client.redirect_uris[0];
87+
} else {
88+
throw new InvalidRequestError("redirect_uri must be specified when client has multiple registered URIs");
89+
}
8890
} catch (error) {
89-
const errorUrl = new URL(redirect_uri);
90-
errorUrl.searchParams.set("error", "invalid_request");
91-
errorUrl.searchParams.set("error_description", String(error));
92-
res.redirect(302, errorUrl.href);
91+
// Pre-redirect errors - return direct response
92+
if (error instanceof OAuthError) {
93+
const status = error instanceof ServerError ? 500 : 400;
94+
res.status(status).end(error.message);
95+
} else {
96+
console.error("Unexpected error looking up client:", error);
97+
res.status(500).end("Internal Server Error");
98+
}
99+
93100
return;
94101
}
95102

96-
let requestedScopes: string[] = [];
97-
if (params.scope !== undefined && client.scope !== undefined) {
98-
requestedScopes = params.scope.split(" ");
99-
const allowedScopes = new Set(client.scope.split(" "));
100-
101-
// If any requested scope is not in the client's registered scopes, error out
102-
for (const scope of requestedScopes) {
103-
if (!allowedScopes.has(scope)) {
104-
const errorUrl = new URL(redirect_uri);
105-
errorUrl.searchParams.set("error", "invalid_scope");
106-
errorUrl.searchParams.set("error_description", `Client was not registered with scope ${scope}`);
107-
res.redirect(302, errorUrl.href);
108-
return;
103+
// Phase 2: Validate other parameters. Any errors here should go into redirect responses.
104+
let state;
105+
try {
106+
// Parse and validate authorization parameters
107+
const parseResult = RequestAuthorizationParamsSchema.safeParse(req.method === 'POST' ? req.body : req.query);
108+
if (!parseResult.success) {
109+
throw new InvalidRequestError(parseResult.error.message);
110+
}
111+
112+
const { scope, code_challenge } = parseResult.data;
113+
state = parseResult.data.state;
114+
115+
// Validate scopes
116+
let requestedScopes: string[] = [];
117+
if (scope !== undefined && client.scope !== undefined) {
118+
requestedScopes = scope.split(" ");
119+
const allowedScopes = new Set(client.scope.split(" "));
120+
121+
// Check each requested scope against allowed scopes
122+
for (const scope of requestedScopes) {
123+
if (!allowedScopes.has(scope)) {
124+
throw new InvalidScopeError(`Client was not registered with scope ${scope}`);
125+
}
109126
}
110127
}
111-
}
112128

113-
await provider.authorize(client, {
114-
state: params.state,
115-
scopes: requestedScopes,
116-
redirectUri: redirect_uri,
117-
codeChallenge: params.code_challenge,
118-
}, res);
129+
// All validation passed, proceed with authorization
130+
await provider.authorize(client, {
131+
state,
132+
scopes: requestedScopes,
133+
redirectUri: redirect_uri,
134+
codeChallenge: code_challenge,
135+
}, res);
136+
} catch (error) {
137+
// Post-redirect errors - redirect with error parameters
138+
if (error instanceof OAuthError) {
139+
res.redirect(302, createErrorRedirect(redirect_uri, error, state));
140+
} else {
141+
console.error("Unexpected error during authorization:", error);
142+
const serverError = new ServerError("Internal Server Error");
143+
res.redirect(302, createErrorRedirect(redirect_uri, serverError, state));
144+
}
145+
}
119146
});
120147

121148
return router;
149+
}
150+
151+
/**
152+
* Helper function to create redirect URL with error parameters
153+
*/
154+
function createErrorRedirect(redirectUri: string, error: OAuthError, state?: string): string {
155+
const errorUrl = new URL(redirectUri);
156+
errorUrl.searchParams.set("error", error.errorCode);
157+
errorUrl.searchParams.set("error_description", error.message);
158+
if (error.errorUri) {
159+
errorUrl.searchParams.set("error_uri", error.errorUri);
160+
}
161+
if (state) {
162+
errorUrl.searchParams.set("state", state);
163+
}
164+
return errorUrl.href;
122165
}

src/server/auth/handlers/register.ts

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import cors from 'cors';
55
import { OAuthRegisteredClientsStore } from "../clients.js";
66
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
77
import { allowedMethods } from "../middleware/allowedMethods.js";
8+
import {
9+
InvalidClientMetadataError,
10+
ServerError,
11+
TooManyRequestsError,
12+
OAuthError
13+
} from "../errors.js";
814

915
export type ClientRegistrationHandlerOptions = {
1016
/**
@@ -54,44 +60,49 @@ export function clientRegistrationHandler({
5460
max: 20, // 20 requests per hour - stricter as registration is sensitive
5561
standardHeaders: true,
5662
legacyHeaders: false,
57-
message: {
58-
error: 'too_many_requests',
59-
error_description: 'You have exceeded the rate limit for client registration requests'
60-
},
63+
message: new TooManyRequestsError('You have exceeded the rate limit for client registration requests').toResponseObject(),
6164
...rateLimitConfig
6265
}));
6366
}
6467

6568
router.post("/", async (req, res) => {
6669
res.setHeader('Cache-Control', 'no-store');
6770

68-
let clientMetadata;
6971
try {
70-
clientMetadata = OAuthClientMetadataSchema.parse(req.body);
72+
const parseResult = OAuthClientMetadataSchema.safeParse(req.body);
73+
if (!parseResult.success) {
74+
throw new InvalidClientMetadataError(parseResult.error.message);
75+
}
76+
77+
const clientMetadata = parseResult.data;
78+
79+
// Generate client credentials
80+
const clientId = crypto.randomUUID();
81+
const clientSecret = clientMetadata.token_endpoint_auth_method !== 'none'
82+
? crypto.randomBytes(32).toString('hex')
83+
: undefined;
84+
const clientIdIssuedAt = Math.floor(Date.now() / 1000);
85+
86+
let clientInfo: OAuthClientInformationFull = {
87+
...clientMetadata,
88+
client_id: clientId,
89+
client_secret: clientSecret,
90+
client_id_issued_at: clientIdIssuedAt,
91+
client_secret_expires_at: clientSecretExpirySeconds > 0 ? clientIdIssuedAt + clientSecretExpirySeconds : 0
92+
};
93+
94+
clientInfo = await clientsStore.registerClient!(clientInfo);
95+
res.status(201).json(clientInfo);
7196
} catch (error) {
72-
res.status(400).json({
73-
error: "invalid_client_metadata",
74-
error_description: String(error),
75-
});
76-
return;
97+
if (error instanceof OAuthError) {
98+
const status = error instanceof ServerError ? 500 : 400;
99+
res.status(status).json(error.toResponseObject());
100+
} else {
101+
console.error("Unexpected error registering client:", error);
102+
const serverError = new ServerError("Internal Server Error");
103+
res.status(500).json(serverError.toResponseObject());
104+
}
77105
}
78-
79-
const clientId = crypto.randomUUID();
80-
const clientSecret = clientMetadata.token_endpoint_auth_method !== 'none'
81-
? crypto.randomBytes(32).toString('hex')
82-
: undefined;
83-
const clientIdIssuedAt = Math.floor(Date.now() / 1000);
84-
85-
let clientInfo: OAuthClientInformationFull = {
86-
...clientMetadata,
87-
client_id: clientId,
88-
client_secret: clientSecret,
89-
client_id_issued_at: clientIdIssuedAt,
90-
client_secret_expires_at: clientSecretExpirySeconds > 0 ? clientIdIssuedAt + clientSecretExpirySeconds : 0
91-
};
92-
93-
clientInfo = await clientsStore.registerClient!(clientInfo);
94-
res.status(201).json(clientInfo);
95106
});
96107

97108
return router;

src/server/auth/handlers/revoke.ts

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import { authenticateClient } from "../middleware/clientAuth.js";
55
import { OAuthTokenRevocationRequestSchema } from "../../../shared/auth.js";
66
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
77
import { allowedMethods } from "../middleware/allowedMethods.js";
8+
import {
9+
InvalidRequestError,
10+
ServerError,
11+
TooManyRequestsError,
12+
OAuthError
13+
} from "../errors.js";
814

915
export type RevocationHandlerOptions = {
1016
provider: OAuthServerProvider;
@@ -36,10 +42,7 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
3642
max: 50, // 50 requests per windowMs
3743
standardHeaders: true,
3844
legacyHeaders: false,
39-
message: {
40-
error: 'too_many_requests',
41-
error_description: 'You have exceeded the rate limit for token revocation requests'
42-
},
45+
message: new TooManyRequestsError('You have exceeded the rate limit for token revocation requests').toResponseObject(),
4346
...rateLimitConfig
4447
}));
4548
}
@@ -50,27 +53,31 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
5053
router.post("/", async (req, res) => {
5154
res.setHeader('Cache-Control', 'no-store');
5255

53-
let revocationRequest;
5456
try {
55-
revocationRequest = OAuthTokenRevocationRequestSchema.parse(req.body);
56-
} catch (error) {
57-
res.status(400).json({
58-
error: "invalid_request",
59-
error_description: String(error),
60-
});
61-
return;
62-
}
57+
const parseResult = OAuthTokenRevocationRequestSchema.safeParse(req.body);
58+
if (!parseResult.success) {
59+
throw new InvalidRequestError(parseResult.error.message);
60+
}
6361

64-
const client = req.client;
65-
if (!client) {
66-
console.error("Missing client information after authentication");
67-
res.status(500).end("Internal Server Error");
68-
return;
69-
}
62+
const client = req.client;
63+
if (!client) {
64+
// This should never happen
65+
console.error("Missing client information after authentication");
66+
throw new ServerError("Internal Server Error");
67+
}
7068

71-
await provider.revokeToken!(client, revocationRequest);
72-
// Return empty response on success (per OAuth 2.0 spec)
73-
res.status(200).json({});
69+
await provider.revokeToken!(client, parseResult.data);
70+
res.status(200).json({});
71+
} catch (error) {
72+
if (error instanceof OAuthError) {
73+
const status = error instanceof ServerError ? 500 : 400;
74+
res.status(status).json(error.toResponseObject());
75+
} else {
76+
console.error("Unexpected error revoking token:", error);
77+
const serverError = new ServerError("Internal Server Error");
78+
res.status(500).json(serverError.toResponseObject());
79+
}
80+
}
7481
});
7582

7683
return router;

0 commit comments

Comments
 (0)