Skip to content

Commit 1724f63

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 7463db7 commit 1724f63

4 files changed

Lines changed: 109 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
@@ -99,7 +99,11 @@ class TokenHandler {
9999
} catch (err) {
100100
let e = err;
101101

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

test/integration/handlers/authorize-handler_test.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,55 @@ describe('AuthorizeHandler integration', function () {
354354
}
355355
});
356356

357+
it('should redirect with a `server_error` if an `InvalidArgumentError` is thrown while handling the request', async function () {
358+
// A model returning a falsy `authorizationCode` makes `CodeResponseType`
359+
// throw an `InvalidArgumentError`. That is an internal error, not an OAuth
360+
// error, so it must reach the client as a standard `server_error` rather
361+
// than the non-standard `invalid_argument`.
362+
const model = createModel({
363+
getAccessToken: async function () {
364+
return {
365+
user: {},
366+
accessTokenExpiresAt: new Date(new Date().getTime() + 10000),
367+
};
368+
},
369+
getClient: async function () {
370+
return { grants: ['authorization_code'], redirectUris: ['http://example.com/cb'] };
371+
},
372+
saveAuthorizationCode: async function () {
373+
return { authorizationCode: undefined, client: {} };
374+
},
375+
});
376+
const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model });
377+
const request = new Request({
378+
body: {
379+
client_id: 12345,
380+
response_type: 'code',
381+
},
382+
headers: {
383+
Authorization: 'Bearer foo',
384+
},
385+
method: {},
386+
query: {
387+
state: 'foobar',
388+
},
389+
});
390+
const response = new Response({ body: {}, headers: {} });
391+
392+
try {
393+
await handler.handle(request, response);
394+
should.fail();
395+
} catch (e) {
396+
e.should.be.an.instanceOf(ServerError);
397+
e.inner.should.be.an.instanceOf(InvalidArgumentError);
398+
e.message.should.equal('Missing parameter: `code`');
399+
const location = new URL(response.get('location'));
400+
location.searchParams.get('error').should.equal('server_error');
401+
location.searchParams.get('error_description').should.equal('Missing parameter: `code`');
402+
location.searchParams.get('state').should.equal('foobar');
403+
}
404+
});
405+
357406
it('should redirect to a successful response with `code` and `state` if successful', async function () {
358407
const client = {
359408
id: 'client-12343434',

test/integration/handlers/token-handler_test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,56 @@ describe('TokenHandler integration', function () {
398398
});
399399
});
400400

401+
it('should normalise an `InvalidArgumentError` thrown while handling the request to a `server_error`', function () {
402+
// A model that returns malformed token data makes `TokenModel` throw an
403+
// `InvalidArgumentError`. That is an internal error, not an OAuth error, so
404+
// it must reach the client as a standard `server_error` rather than the
405+
// non-standard `invalid_argument`.
406+
const model = Model.from({
407+
getClient: function () {
408+
return { grants: ['password'] };
409+
},
410+
getUser: function () {
411+
return {};
412+
},
413+
saveToken: function () {
414+
return { accessToken: 'foo', client: {}, user: {}, accessTokenExpiresAt: 'not-a-date' };
415+
},
416+
validateScope: function () {
417+
return ['baz'];
418+
},
419+
});
420+
const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 });
421+
const request = new Request({
422+
body: {
423+
client_id: 12345,
424+
client_secret: 'secret',
425+
username: 'foo',
426+
password: 'bar',
427+
grant_type: 'password',
428+
scope: 'baz',
429+
},
430+
headers: { 'content-type': 'application/x-www-form-urlencoded', 'transfer-encoding': 'chunked' },
431+
method: 'POST',
432+
query: {},
433+
});
434+
const response = new Response({ body: {}, headers: {} });
435+
436+
return handler
437+
.handle(request, response)
438+
.then(should.fail)
439+
.catch(function (e) {
440+
e.should.be.an.instanceOf(ServerError);
441+
e.inner.should.be.an.instanceOf(InvalidArgumentError);
442+
e.message.should.equal('Invalid parameter: `accessTokenExpiresAt`');
443+
response.body.should.eql({
444+
error: 'server_error',
445+
error_description: 'Invalid parameter: `accessTokenExpiresAt`',
446+
});
447+
response.status.should.equal(503);
448+
});
449+
});
450+
401451
it('should return a bearer token if successful', function () {
402452
const token = {
403453
accessToken: 'foo',

0 commit comments

Comments
 (0)