Skip to content

Commit 6a484fe

Browse files
dhensbyclaude
andcommitted
fix!: normalise InvalidArgumentError to server_error on the wire
InvalidArgumentError carries the non-standard `invalid_argument` code (defined in neither RFC 6749 §5.2 nor §4.1.2.1) but extends OAuthError, so the token and authorize handler catch blocks let it slip through their "wrap any non-OAuthError as ServerError" guard. As a result, a misbehaving model that returns malformed token data (e.g. a non-Date `accessTokenExpiresAt`) or a falsy authorization code caused the server to emit `error=invalid_argument` with HTTP 500 to the client instead of a standard `server_error`. Treat InvalidArgumentError like any other internal error at the serialisation boundary: normalise it to ServerError before it reaches the client, preserving the original as `.inner`. Construction- and request-validation guards (missing options, model-capability checks, request/response instance checks) are thrown before the try block and are unaffected — they still surface InvalidArgumentError to the integrator as a descriptive developer-facing error. BREAKING CHANGE: when a model returns malformed token data or a falsy authorization code at request time, the OAuth error response now reports `error: server_error` with HTTP status 503 instead of `error: invalid_argument` with HTTP status 500. Setup-time InvalidArgumentErrors thrown to integrator code are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4cd878b commit 6a484fe

4 files changed

Lines changed: 97 additions & 2 deletions

File tree

lib/handlers/authorize-handler.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,11 @@ class AuthorizeHandler {
121121
} catch (err) {
122122
let e = err;
123123

124-
if (!(e instanceof OAuthError)) {
124+
// `InvalidArgumentError` denotes a programming or configuration error and
125+
// its `invalid_argument` code is not a valid OAuth 2.0 error code, so -
126+
// like any non-OAuth error - it is normalised to a `server_error` before
127+
// reaching the client. The original error is retained as `.inner`.
128+
if (!(e instanceof OAuthError) || e instanceof InvalidArgumentError) {
125129
e = new ServerError(e);
126130
}
127131
const redirectUri = this.buildErrorRedirectUri(uri, e);

lib/handlers/token-handler.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,11 @@ class TokenHandler {
9797
} catch (err) {
9898
let e = err;
9999

100-
if (!(e instanceof OAuthError)) {
100+
// `InvalidArgumentError` denotes a programming or configuration error and
101+
// its `invalid_argument` code is not a valid OAuth 2.0 error code, so -
102+
// like any non-OAuth error - it is normalised to a `server_error` before
103+
// reaching the client. The original error is retained as `.inner`.
104+
if (!(e instanceof OAuthError) || e instanceof InvalidArgumentError) {
101105
e = new ServerError(e);
102106
}
103107

test/integration/handlers/authorize-handler_test.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,55 @@ describe('AuthorizeHandler integration', function() {
308308
}
309309
});
310310

311+
it('should redirect with a `server_error` if an `InvalidArgumentError` is thrown while handling the request', async function() {
312+
// A model returning a falsy `authorizationCode` makes `CodeResponseType`
313+
// throw an `InvalidArgumentError`. That is an internal error, not an OAuth
314+
// error, so it must reach the client as a standard `server_error` rather
315+
// than the non-standard `invalid_argument`.
316+
const model = createModel({
317+
getAccessToken: async function() {
318+
return {
319+
user: {},
320+
accessTokenExpiresAt: new Date(new Date().getTime() + 10000)
321+
};
322+
},
323+
getClient: async function() {
324+
return { grants: ['authorization_code'], redirectUris: ['http://example.com/cb'] };
325+
},
326+
saveAuthorizationCode: async function() {
327+
return { authorizationCode: undefined, client: {} };
328+
}
329+
});
330+
const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model });
331+
const request = new Request({
332+
body: {
333+
client_id: 12345,
334+
response_type: 'code'
335+
},
336+
headers: {
337+
'Authorization': 'Bearer foo'
338+
},
339+
method: {},
340+
query: {
341+
state: 'foobar'
342+
}
343+
});
344+
const response = new Response({ body: {}, headers: {} });
345+
346+
try {
347+
await handler.handle(request, response);
348+
should.fail();
349+
} catch (e) {
350+
e.should.be.an.instanceOf(ServerError);
351+
e.inner.should.be.an.instanceOf(InvalidArgumentError);
352+
e.message.should.equal('Missing parameter: `code`');
353+
const location = new URL(response.get('location'));
354+
location.searchParams.get('error').should.equal('server_error');
355+
location.searchParams.get('error_description').should.equal('Missing parameter: `code`');
356+
location.searchParams.get('state').should.equal('foobar');
357+
}
358+
});
359+
311360
it('should redirect to a successful response with `code` and `state` if successful', async function() {
312361
const client = {
313362
id: 'client-12343434',

test/integration/handlers/token-handler_test.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,44 @@ describe('TokenHandler integration', function() {
298298
});
299299
});
300300

301+
it('should normalise an `InvalidArgumentError` thrown while handling the request to a `server_error`', function() {
302+
// A model that returns malformed token data makes `TokenModel` throw an
303+
// `InvalidArgumentError`. That is an internal error, not an OAuth error, so
304+
// it must reach the client as a standard `server_error` rather than the
305+
// non-standard `invalid_argument`.
306+
const model = Model.from({
307+
getClient: function() { return { grants: ['password'] }; },
308+
getUser: function() { return {}; },
309+
saveToken: function() { return { accessToken: 'foo', client: {}, user: {}, accessTokenExpiresAt: 'not-a-date' }; },
310+
validateScope: function() { return ['baz']; }
311+
});
312+
const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 });
313+
const request = new Request({
314+
body: {
315+
client_id: 12345,
316+
client_secret: 'secret',
317+
username: 'foo',
318+
password: 'bar',
319+
grant_type: 'password',
320+
scope: 'baz'
321+
},
322+
headers: { 'content-type': 'application/x-www-form-urlencoded', 'transfer-encoding': 'chunked' },
323+
method: 'POST',
324+
query: {}
325+
});
326+
const response = new Response({ body: {}, headers: {} });
327+
328+
return handler.handle(request, response)
329+
.then(should.fail)
330+
.catch(function(e) {
331+
e.should.be.an.instanceOf(ServerError);
332+
e.inner.should.be.an.instanceOf(InvalidArgumentError);
333+
e.message.should.equal('Invalid parameter: `accessTokenExpiresAt`');
334+
response.body.should.eql({ error: 'server_error', error_description: 'Invalid parameter: `accessTokenExpiresAt`' });
335+
response.status.should.equal(503);
336+
});
337+
});
338+
301339
it('should return a bearer token if successful', function() {
302340
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['foobar'], user: {} };
303341
const model = Model.from({

0 commit comments

Comments
 (0)