From 6a484fe4471f36e5ac32185d22f42395b48f680b Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Wed, 17 Jun 2026 12:39:58 +0100 Subject: [PATCH] fix!: normalise InvalidArgumentError to server_error on the wire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- lib/handlers/authorize-handler.js | 6 ++- lib/handlers/token-handler.js | 6 ++- .../handlers/authorize-handler_test.js | 49 +++++++++++++++++++ .../handlers/token-handler_test.js | 38 ++++++++++++++ 4 files changed, 97 insertions(+), 2 deletions(-) diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js index c5f0db1..ed3b23e 100644 --- a/lib/handlers/authorize-handler.js +++ b/lib/handlers/authorize-handler.js @@ -121,7 +121,11 @@ class AuthorizeHandler { } catch (err) { let e = err; - if (!(e instanceof OAuthError)) { + // `InvalidArgumentError` denotes a programming or configuration error and + // its `invalid_argument` code is not a valid OAuth 2.0 error code, so - + // like any non-OAuth error - it is normalised to a `server_error` before + // reaching the client. The original error is retained as `.inner`. + if (!(e instanceof OAuthError) || e instanceof InvalidArgumentError) { e = new ServerError(e); } const redirectUri = this.buildErrorRedirectUri(uri, e); diff --git a/lib/handlers/token-handler.js b/lib/handlers/token-handler.js index 3b98307..ced1f00 100644 --- a/lib/handlers/token-handler.js +++ b/lib/handlers/token-handler.js @@ -97,7 +97,11 @@ class TokenHandler { } catch (err) { let e = err; - if (!(e instanceof OAuthError)) { + // `InvalidArgumentError` denotes a programming or configuration error and + // its `invalid_argument` code is not a valid OAuth 2.0 error code, so - + // like any non-OAuth error - it is normalised to a `server_error` before + // reaching the client. The original error is retained as `.inner`. + if (!(e instanceof OAuthError) || e instanceof InvalidArgumentError) { e = new ServerError(e); } diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js index d9004cd..db92a4b 100644 --- a/test/integration/handlers/authorize-handler_test.js +++ b/test/integration/handlers/authorize-handler_test.js @@ -308,6 +308,55 @@ describe('AuthorizeHandler integration', function() { } }); + it('should redirect with a `server_error` if an `InvalidArgumentError` is thrown while handling the request', async function() { + // A model returning a falsy `authorizationCode` makes `CodeResponseType` + // throw an `InvalidArgumentError`. That is an internal error, not an OAuth + // error, so it must reach the client as a standard `server_error` rather + // than the non-standard `invalid_argument`. + const model = createModel({ + getAccessToken: async function() { + return { + user: {}, + accessTokenExpiresAt: new Date(new Date().getTime() + 10000) + }; + }, + getClient: async function() { + return { grants: ['authorization_code'], redirectUris: ['http://example.com/cb'] }; + }, + saveAuthorizationCode: async function() { + return { authorizationCode: undefined, client: {} }; + } + }); + const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model }); + const request = new Request({ + body: { + client_id: 12345, + response_type: 'code' + }, + headers: { + 'Authorization': 'Bearer foo' + }, + method: {}, + query: { + state: 'foobar' + } + }); + const response = new Response({ body: {}, headers: {} }); + + try { + await handler.handle(request, response); + should.fail(); + } catch (e) { + e.should.be.an.instanceOf(ServerError); + e.inner.should.be.an.instanceOf(InvalidArgumentError); + e.message.should.equal('Missing parameter: `code`'); + const location = new URL(response.get('location')); + location.searchParams.get('error').should.equal('server_error'); + location.searchParams.get('error_description').should.equal('Missing parameter: `code`'); + location.searchParams.get('state').should.equal('foobar'); + } + }); + it('should redirect to a successful response with `code` and `state` if successful', async function() { const client = { id: 'client-12343434', diff --git a/test/integration/handlers/token-handler_test.js b/test/integration/handlers/token-handler_test.js index 7fc0a2b..d49ffc7 100644 --- a/test/integration/handlers/token-handler_test.js +++ b/test/integration/handlers/token-handler_test.js @@ -298,6 +298,44 @@ describe('TokenHandler integration', function() { }); }); + it('should normalise an `InvalidArgumentError` thrown while handling the request to a `server_error`', function() { + // A model that returns malformed token data makes `TokenModel` throw an + // `InvalidArgumentError`. That is an internal error, not an OAuth error, so + // it must reach the client as a standard `server_error` rather than the + // non-standard `invalid_argument`. + const model = Model.from({ + getClient: function() { return { grants: ['password'] }; }, + getUser: function() { return {}; }, + saveToken: function() { return { accessToken: 'foo', client: {}, user: {}, accessTokenExpiresAt: 'not-a-date' }; }, + validateScope: function() { return ['baz']; } + }); + const handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120 }); + const request = new Request({ + body: { + client_id: 12345, + client_secret: 'secret', + username: 'foo', + password: 'bar', + grant_type: 'password', + scope: 'baz' + }, + headers: { 'content-type': 'application/x-www-form-urlencoded', 'transfer-encoding': 'chunked' }, + method: 'POST', + query: {} + }); + const response = new Response({ body: {}, headers: {} }); + + return handler.handle(request, response) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(ServerError); + e.inner.should.be.an.instanceOf(InvalidArgumentError); + e.message.should.equal('Invalid parameter: `accessTokenExpiresAt`'); + response.body.should.eql({ error: 'server_error', error_description: 'Invalid parameter: `accessTokenExpiresAt`' }); + response.status.should.equal(503); + }); + }); + it('should return a bearer token if successful', function() { const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['foobar'], user: {} }; const model = Model.from({