diff --git a/.changeset/tasty-spoons-pay.md b/.changeset/tasty-spoons-pay.md new file mode 100644 index 00000000..1b211175 --- /dev/null +++ b/.changeset/tasty-spoons-pay.md @@ -0,0 +1,8 @@ +--- +'@interledger/openapi': major +--- + +Changes to the `createValidatorMiddleware` behaviour: + +- The middleware now throws a `OpenAPIValidatorMiddlewareError` instead of directly using Koa's `ctx.throw` function. This error class has `message` and `status` properties that describe the exact validation error as well as the status code that should be thrown. +- The middleware now takes in an optional `validationOptions` argument that determines whether to validate just the request, the response or both. By default, both are validated, and the middleware no longer uses `process.env.NODE_ENV` to determine whether to validate the response or not. diff --git a/packages/open-payments/src/client/requests.ts b/packages/open-payments/src/client/requests.ts index 6c4cb6a7..c7fe9a76 100644 --- a/packages/open-payments/src/client/requests.ts +++ b/packages/open-payments/src/client/requests.ts @@ -133,7 +133,7 @@ const handleError = ( errorStatus = error.response?.status } else if (isValidationError(error)) { errorDescription = 'Could not validate OpenAPI response' - validationErrors = error.errors + validationErrors = error.errors.map((e) => e.message) errorStatus = error.status } else if (error instanceof Error) { errorDescription = error.message diff --git a/packages/open-payments/src/test/helpers.ts b/packages/open-payments/src/test/helpers.ts index 50547846..8b341171 100644 --- a/packages/open-payments/src/test/helpers.ts +++ b/packages/open-payments/src/test/helpers.ts @@ -20,7 +20,7 @@ import { DIDDocument } from '../types' import { v4 as uuid } from 'uuid' -import { ResponseValidator } from '@interledger/openapi' +import { ResponseValidator, ValidationError } from '@interledger/openapi' import base64url from 'base64url' import { BaseDeps } from '../client' @@ -41,10 +41,11 @@ export const mockOpenApiResponseValidators = () => ({ // eslint-disable-next-line @typescript-eslint/no-explicit-any true) as ResponseValidator, failedValidator: ((data: unknown): data is unknown => { - throw { - errors: ['Failed to validate response'], - message: 'Failed to validate response' - } // to mock validationError + const err: ValidationError = { + errors: [{ message: 'Failed to validate response' }] + } + + throw err // eslint-disable-next-line @typescript-eslint/no-explicit-any }) as ResponseValidator }) diff --git a/packages/openapi/README.md b/packages/openapi/README.md index f0188cc9..9fa689ac 100644 --- a/packages/openapi/README.md +++ b/packages/openapi/README.md @@ -12,6 +12,8 @@ npm install @interledger/openapi ## Usage +### Validators + First, instantiate an `OpenAPI` validator object with a reference to your OpenAPI spec: ```ts @@ -40,7 +42,7 @@ validateResponse({ body: response.body, status }) // throws or returns true > > The underlying response & request validator [packages](https://github.com/kogosoftwarellc/open-api/tree/master/packages) use the [Ajv schema validator](https://ajv.js.org) library. Each time validators are created via `createRequestValidator` and `createResponseValidator`, a `new Ajv()` instance is also [created](https://github.com/kogosoftwarellc/open-api/blob/master/packages/openapi-response-validator/index.ts). Since Ajv [recommends](https://ajv.js.org/guide/managing-schemas.html#compiling-during-initialization) instantiating once at initialization, these validators should also be instantiated just once during the lifecycle of the application to avoid any issues. -
+### Middleware Likewise, you can validate both requests and responses inside a [Koa](https://github.com/koajs/koa) middleware method, using `createValidatorMiddleware`: @@ -49,9 +51,28 @@ const openApi = await createOpenAPI(OPEN_API_URL) const router = new SomeRouter() router.get( '/resource/{id}', - createValidatorMiddleware(openApi, { - path: '/resource/{id}', - method: HttpMethod.GET - }) + createValidatorMiddleware( + openApi, + { + path: '/resource/{id}', + method: HttpMethod.GET + }, + { validateRequest: true, validateResponse: false } // optional arguments to determine what you want the middleware to validate. Both properties are true by default. Setting both variables to false results in a noop middleware. + ) ) ``` + +If a validation error occurs, the middleware will throw an `OpenAPIValidatorMiddlewareError`: + +```ts +app.use(async (ctx, next) => { + try { + await next() + } catch (err) { + if (err instanceof OpenAPIValidatorMiddlewareError) { + console.log(err.message) // e.g. Received error validating OpenAPI response: response.receivedAmount.value must match format "uint64" + console.log(err.status) // e.g. 400 + } + } +}) +``` diff --git a/packages/openapi/src/index.ts b/packages/openapi/src/index.ts index 124b2f52..9bfa4708 100644 --- a/packages/openapi/src/index.ts +++ b/packages/openapi/src/index.ts @@ -155,10 +155,14 @@ class OpenAPIImpl implements OpenAPI { } } +interface TransformedError { + message: string +} + const errorTransformer = ( _openapiError: OpenAPIResponseValidatorError, ajvError: ErrorObject -) => { +): TransformedError => { // Remove preceding 'data/' // Delineate subfields with '.' const message = ajv.errorsText([ajvError]).slice(5).replace(/\//g, '.') @@ -182,9 +186,9 @@ const customFormats = { } } -interface ValidationError { +export interface ValidationError { status?: number - errors: string[] + errors: TransformedError[] } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/openapi/src/middleware.test.ts b/packages/openapi/src/middleware.test.ts index 29e08e2a..164ab2ce 100644 --- a/packages/openapi/src/middleware.test.ts +++ b/packages/openapi/src/middleware.test.ts @@ -1,9 +1,18 @@ import Koa from 'koa' import * as httpMocks from 'node-mocks-http' import { v4 as uuid } from 'uuid' - -import { createOpenAPI, OpenAPI, HttpMethod } from './' -import { createValidatorMiddleware } from './middleware' +import assert from 'assert' +import { + createOpenAPI, + OpenAPI, + HttpMethod, + RequestValidator, + ResponseValidator +} from './' +import { + createValidatorMiddleware, + OpenAPIValidatorMiddlewareError +} from './middleware' import * as path from 'path' declare module 'koa' { @@ -94,34 +103,55 @@ describe('OpenAPI Validator', (): void => { {} ) addTestSignatureHeaders(ctx) - await expect(validateListMiddleware(ctx, next)).rejects.toMatchObject({ - status: 400, - message: 'first must be integer' - }) + + try { + await validateListMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + 'Received error validating OpenAPI request: first must be integer' + ) + expect(err.status).toBe(400) + } expect(next).not.toHaveBeenCalled() }) - test.each` - headers | status | message | description - ${{ Accept: 'text/plain' }} | ${406} | ${'must accept json'} | ${'Accept'} - ${{ 'Content-Type': 'text/plain' }} | ${415} | ${'Unsupported Content-Type text/plain'} | ${'Content-Type'} - `( - 'returns $status on invalid $description header', - async ({ headers, status, message }): Promise => { - const ctx = createContext( - { - headers - }, - {} + test('returns 406 on invalid Accept header', async (): Promise => { + const ctx = createContext({ headers: { Accept: 'text/plain' } }, {}) + addTestSignatureHeaders(ctx) + + try { + await validatePostMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + 'Received error validating OpenAPI request: Must accept application/json' ) - addTestSignatureHeaders(ctx) - await expect(validatePostMiddleware(ctx, next)).rejects.toMatchObject({ - status, - message - }) - expect(next).not.toHaveBeenCalled() + expect(err.status).toBe(406) } - ) + + expect(next).not.toHaveBeenCalled() + }) + + test('returns 415 on invalid Content-Type header', async (): Promise => { + const ctx = createContext( + { headers: { 'Content-Type': 'text/plain' } }, + {} + ) + addTestSignatureHeaders(ctx) + + try { + await validatePostMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + 'Received error validating OpenAPI request: Unsupported Content-Type text/plain' + ) + expect(err.status).toBe(415) + } + + expect(next).not.toHaveBeenCalled() + }) test.each` body | message | description @@ -145,13 +175,20 @@ describe('OpenAPI Validator', (): void => { {} ) addTestSignatureHeaders(ctx) - ctx.request.body = body + ctx.request['body'] = body ? { ...body, walletAddress: WALLET_ADDRESS } : body - await expect(validatePostMiddleware(ctx, next)).rejects.toMatchObject({ - status: 400, - message - }) + + try { + await validatePostMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + `Received error validating OpenAPI request: ${message}` + ) + expect(err.status).toBe(400) + } + expect(next).not.toHaveBeenCalled() } ) @@ -201,14 +238,74 @@ describe('OpenAPI Validator', (): void => { ] } }) - await expect(validateListMiddleware(ctx, next)).rejects.toMatchObject({ - status: 500, - message: - 'response.result.0 must NOT have additional properties: additionalProp' - }) + + try { + await validateListMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + 'Received error validating OpenAPI response: response.result.0 must NOT have additional properties: additionalProp' + ) + expect(err.status).toBe(500) + } + expect(next).toHaveBeenCalled() }) + test.each` + validateRequest | validateResponse + ${true} | ${true} + ${true} | ${false} + ${false} | ${true} + ${false} | ${false} + `( + 'calls validators correctly with validateRequest=$validateRequest and validateResponse=$validateResponse', + async ({ validateRequest, validateResponse }): Promise => { + const ctx = createContext( + { + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + } + }, + {} + ) + addTestSignatureHeaders(ctx) + ctx.request['body'] = { walletAddress: WALLET_ADDRESS } + + const mockRequestValidator = jest.fn() + + jest.spyOn(openApi, 'createRequestValidator').mockReturnValueOnce( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockRequestValidator as unknown as RequestValidator + ) + + const mockResponseValidator = jest.fn() + + jest.spyOn(openApi, 'createResponseValidator').mockReturnValueOnce( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockResponseValidator as unknown as ResponseValidator + ) + + await createValidatorMiddleware( + openApi, + { + path: PATH, + method: HttpMethod.POST + }, + { validateRequest, validateResponse } + )(ctx, next) + + expect(mockRequestValidator).toHaveBeenCalledTimes( + validateRequest ? 1 : 0 + ) + expect(mockResponseValidator).toHaveBeenCalledTimes( + validateResponse ? 1 : 0 + ) + expect(next).toHaveBeenCalled() + } + ) + const body = { id: `https://${accountId}/incoming-payments/${uuid()}`, walletAddress: 'https://openpayments.guide/alice', @@ -244,15 +341,21 @@ describe('OpenAPI Validator', (): void => { {} ) addTestSignatureHeaders(ctx) - ctx.request.body = { walletAddress: WALLET_ADDRESS } + ctx.request['body'] = { walletAddress: WALLET_ADDRESS } const next = jest.fn().mockImplementation(() => { ctx.status = status ctx.response.body = body }) - await expect(validatePostMiddleware(ctx, next)).rejects.toMatchObject({ - status: 500, - message - }) + try { + await validatePostMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + `Received error validating OpenAPI response: ${message}` + ) + expect(err.status).toBe(500) + } + expect(next).toHaveBeenCalled() } ) @@ -269,8 +372,6 @@ describe('OpenAPI Validator', (): void => { test.each` body | description - ${{ receiver: 'ht999tp://something.com/incoming-payments' }} | ${'invalid receiver, unknown protocol'} - ${{ receiver: 'http://something.com/incoming-payments' }} | ${'invalid receiver, missing incoming payment id'} ${{ receiver: 'http://something.com/connections/c3a0d182-b221-4612-a500-07ad106b5f5d' }} | ${'invalid receiver, wrong path'} `( 'returns 400 on invalid quote body ($description)', @@ -285,18 +386,21 @@ describe('OpenAPI Validator', (): void => { {} ) addTestSignatureHeaders(ctx) - ctx.request.body = { + ctx.request['body'] = { ...body, walletAddress: WALLET_ADDRESS, method: 'ilp' } - await expect( - validateQuotePostMiddleware(ctx, next) - ).rejects.toMatchObject({ - status: 400, - message: - 'body.receiver must match pattern "^(https|http):..(.+).incoming-payments.(.+)$"' - }) + try { + await validateQuotePostMiddleware(ctx, next) + } catch (err) { + assert.ok(err instanceof OpenAPIValidatorMiddlewareError) + expect(err.message).toBe( + 'Received error validating OpenAPI request: body.receiver must match pattern "^(https|http):..(.+).incoming-payments.(.+)$"' + ) + expect(err.status).toBe(400) + } + expect(next).not.toHaveBeenCalled() } ) @@ -318,13 +422,13 @@ describe('OpenAPI Validator', (): void => { {} ) addTestSignatureHeaders(ctx) - ctx.request.body = { + ctx.request['body'] = { receiver, walletAddress: WALLET_ADDRESS, method: 'ilp' } const next = jest.fn().mockImplementation(() => { - expect(ctx.request.body.receiver).toEqual(receiver) + expect(ctx.request['body'].receiver).toEqual(receiver) ctx.status = 201 ctx.response.body = { id: 'https://something-else/quotes/3b461206-daae-4d97-88b0-abffbcaa6f96', diff --git a/packages/openapi/src/middleware.ts b/packages/openapi/src/middleware.ts index ef83f139..ed08b103 100644 --- a/packages/openapi/src/middleware.ts +++ b/packages/openapi/src/middleware.ts @@ -2,39 +2,74 @@ import { OpenAPI, RequestOptions, isValidationError } from './' import Koa from 'koa' +interface ValidationOptions { + validateRequest?: boolean + validateResponse?: boolean +} + export function createValidatorMiddleware( spec: OpenAPI, - options: RequestOptions + options: RequestOptions, + validationOptions: ValidationOptions | undefined = { + validateRequest: true, + validateResponse: true + } ): (ctx: Koa.Context, next: () => Promise) => Promise { - const validateRequest = spec.createRequestValidator(options) - const validateResponse = - process.env.NODE_ENV !== 'production' && - spec.createResponseValidator(options) + const requestValidator = spec.createRequestValidator(options) + const responseValidator = spec.createResponseValidator(options) return async ( ctx: Koa.Context, next: () => Promise ): Promise => { - // TODO: Allow 'application/*+json' - ctx.assert(ctx.accepts('application/json'), 406, 'must accept json') - try { - if (validateRequest(ctx.request)) { - await next() - if (validateResponse && !validateResponse(ctx.response)) { - throw new Error('unreachable') + if (validationOptions?.validateRequest) { + // TODO: Allow 'application/*+json' + if (!ctx.accepts('application/json')) { + throw new OpenAPIValidatorMiddlewareError( + 'Received error validating OpenAPI request: Must accept application/json', + 406 + ) + } + + try { + requestValidator(ctx.request) + } catch (err) { + if (isValidationError(err)) { + throw new OpenAPIValidatorMiddlewareError( + `Received error validating OpenAPI request: ${err.errors[0]?.message}`, + err.status || 500 + ) } - } else { - throw new Error('unreachable') + + throw err // Should not be possible (only ValidationError is thrown in requestValidator) } - } catch (err) { - if (err instanceof Koa.HttpError) { - throw err - } else if (isValidationError(err)) { - ctx.throw(err.status ?? 500, err.errors[0]) - } else { - console.log('err=', err) - ctx.throw(500) + } + + await next() + + if (validationOptions?.validateResponse) { + try { + responseValidator(ctx.response) + } catch (err) { + if (isValidationError(err)) { + throw new OpenAPIValidatorMiddlewareError( + `Received error validating OpenAPI response: ${err.errors[0]?.message}`, + err.status || 500 + ) + } + + throw err // Should not be possible (only ValidationError is thrown in responseValidator) } } } } + +export class OpenAPIValidatorMiddlewareError extends Error { + public status?: number + + constructor(message: string, status?: number) { + super(message) + this.name = 'OpenAPIValidatorMiddlewareError' + this.status = status + } +}