From 4bff6f7288bb7ba17ad2d74e89cbef01b21c1a8c 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) --- docs/api/errors/invalid-argument-error.md | 21 +++++++- lib/errors/invalid-argument-error.js | 22 +++++++- lib/handlers/authorize-handler.js | 6 ++- lib/handlers/token-handler.js | 6 ++- .../handlers/authorize-handler_test.js | 49 ++++++++++++++++++ .../handlers/token-handler_test.js | 50 +++++++++++++++++++ 6 files changed, 150 insertions(+), 4 deletions(-) diff --git a/docs/api/errors/invalid-argument-error.md b/docs/api/errors/invalid-argument-error.md index 5d080939..a5d8649b 100644 --- a/docs/api/errors/invalid-argument-error.md +++ b/docs/api/errors/invalid-argument-error.md @@ -1,7 +1,26 @@ ## InvalidArgumentError -"An argument to a function or constructor is missing or of wrong type" +Thrown when an argument to a function or constructor is missing or of the +wrong type — i.e. a programming or configuration error in the code that +integrates this library (for example, a missing server option, or a `model` +that does not implement a required method). + +`InvalidArgumentError` is **not** an OAuth 2.0 protocol error: its +`invalid_argument` code (HTTP 500) is not one of the error codes defined by +RFC 6749 and must never be sent to an OAuth client. How it surfaces therefore +depends on *when* it is raised: + +- **At construction / configuration time** (missing options, a `model` missing + a required method, or `request`/`response` not being instances of this + library's `Request`/`Response`) it is thrown to your code unchanged, before + any response is produced — so you can catch it while wiring up the server. +- **While handling a request** (for example, when a `model` returns malformed + token data) the `token` and `authorize` handlers normalise it to a + `ServerError` (`server_error`) before the response reaches the client, + preserving the original error as `.inner`. So when you `catch` errors from + `OAuth2Server#token` or `OAuth2Server#authorize`, expect a `ServerError` + (inspect its `.inner`) for these cases — not an `InvalidArgumentError`. **Kind**: global class diff --git a/lib/errors/invalid-argument-error.js b/lib/errors/invalid-argument-error.js index 3a1a4002..3a5c9e3d 100644 --- a/lib/errors/invalid-argument-error.js +++ b/lib/errors/invalid-argument-error.js @@ -7,8 +7,28 @@ const OAuthError = require('./oauth-error'); /** + * Thrown when an argument to a function or constructor is missing or of the + * wrong type — i.e. a programming or configuration error in the code that + * integrates this library (for example, a missing server option, or a `model` + * that does not implement a required method). + * + * `InvalidArgumentError` is **not** an OAuth 2.0 protocol error: its + * `invalid_argument` code (HTTP 500) is not one of the error codes defined by + * RFC 6749 and must never be sent to an OAuth client. How it surfaces therefore + * depends on *when* it is raised: + * + * - **At construction / configuration time** (missing options, a `model` missing + * a required method, or `request`/`response` not being instances of this + * library's `Request`/`Response`) it is thrown to your code unchanged, before + * any response is produced — so you can catch it while wiring up the server. + * - **While handling a request** (for example, when a `model` returns malformed + * token data) the `token` and `authorize` handlers normalise it to a + * `ServerError` (`server_error`) before the response reaches the client, + * preserving the original error as `.inner`. So when you `catch` errors from + * `OAuth2Server#token` or `OAuth2Server#authorize`, expect a `ServerError` + * (inspect its `.inner`) for these cases — not an `InvalidArgumentError`. + * * @class - * @classDesc "An argument to a function or constructor is missing or of wrong type" */ class InvalidArgumentError extends OAuthError { diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js index 35a5c22b..4aef3492 100644 --- a/lib/handlers/authorize-handler.js +++ b/lib/handlers/authorize-handler.js @@ -128,7 +128,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 0fd81e7c..895ef9c8 100644 --- a/lib/handlers/token-handler.js +++ b/lib/handlers/token-handler.js @@ -100,7 +100,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 717c4cce..aa81ab6a 100644 --- a/test/integration/handlers/authorize-handler_test.js +++ b/test/integration/handlers/authorize-handler_test.js @@ -354,6 +354,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 010a9292..4fc52ca0 100644 --- a/test/integration/handlers/token-handler_test.js +++ b/test/integration/handlers/token-handler_test.js @@ -398,6 +398,56 @@ 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',