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',