From 5d8114c1d66dbbc8c4fac44e2e3d37fe1c57b089 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Fri, 5 Jul 2024 17:13:56 -0400 Subject: [PATCH 01/23] feat(openapi-tooling): custom validation server for test cases Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 3 + packages/backend-openapi-utils/src/index.ts | 2 +- .../backend-openapi-utils/src/proxy/setup.ts | 71 +++ .../src/schema/validation.ts | 587 ++++++++++++++++++ .../backend-openapi-utils/src/testUtils.ts | 17 + .../src/service/createRouter.test.ts | 4 +- .../src/schema/openapi.generated.ts | 2 +- .../search-backend/src/schema/openapi.yaml | 2 +- .../search-backend/src/service/router.test.ts | 6 +- yarn.lock | 126 +++- 10 files changed, 810 insertions(+), 10 deletions(-) create mode 100644 packages/backend-openapi-utils/src/proxy/setup.ts create mode 100644 packages/backend-openapi-utils/src/schema/validation.ts diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 8f6fd1b6eb..a173cbf17b 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -33,15 +33,18 @@ "test": "backstage-cli package test" }, "dependencies": { + "@apidevtools/swagger-parser": "^10.1.0", "@backstage/backend-plugin-api": "workspace:^", "@backstage/errors": "workspace:^", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", + "ajv": "^8.16.0", "express": "^4.17.1", "express-openapi-validator": "^5.0.4", "express-promise-router": "^4.1.0", "json-schema-to-ts": "^3.0.0", "lodash": "^4.17.21", + "mockttp": "^3.13.0", "openapi-merge": "^1.3.2", "openapi3-ts": "^3.1.2" }, diff --git a/packages/backend-openapi-utils/src/index.ts b/packages/backend-openapi-utils/src/index.ts index 9f5ccdc03b..57d9f755c2 100644 --- a/packages/backend-openapi-utils/src/index.ts +++ b/packages/backend-openapi-utils/src/index.ts @@ -32,4 +32,4 @@ export type { } from './utility'; export type { ApiRouter } from './router'; export { createValidatedOpenApiRouter, getOpenApiSpecRoute } from './stub'; -export { wrapInOpenApiTestServer } from './testUtils'; +export { wrapInOpenApiTestServer, wrapServer } from './testUtils'; diff --git a/packages/backend-openapi-utils/src/proxy/setup.ts b/packages/backend-openapi-utils/src/proxy/setup.ts new file mode 100644 index 0000000000..a7205a883a --- /dev/null +++ b/packages/backend-openapi-utils/src/proxy/setup.ts @@ -0,0 +1,71 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as mockttp from 'mockttp'; +import { OpenApiProxyValidator } from '../schema/validation'; + +export class Proxy { + server: mockttp.Mockttp; + #openRequests: Record = {}; + requestResponsePairs = new Map< + mockttp.CompletedRequest, + mockttp.CompletedResponse + >(); + validator: OpenApiProxyValidator; + constructor() { + this.server = mockttp.getLocal(); + this.validator = new OpenApiProxyValidator(); + } + + async setup() { + await this.server.start(); + this.server + .forAnyRequest() + .thenForwardTo(`http://localhost:${process.env.PORT}`); + await this.server.on('request', request => { + this.#openRequests[request.id] = request; + }); + await this.server.on('response', response => { + const request = this.#openRequests[response.id]; + if (request) { + this.requestResponsePairs.set(request, response); + } + delete this.#openRequests[response.id]; + try { + this.validator.validate(request, response); + } catch (err) { + console.error(err); + } + }); + } + + async initialize() { + await this.validator.initialize( + `http://localhost:${process.env.PORT}/openapi.json`, + ); + } + + stop() { + if (Object.keys(this.#openRequests).length > 0) { + throw new Error('There are still open requests'); + } + this.server.stop(); + } + + get url() { + return this.server.proxyEnv.HTTP_PROXY; + } +} diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts new file mode 100644 index 0000000000..104482a485 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -0,0 +1,587 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { CompletedRequest, CompletedResponse } from 'mockttp'; +import { + OpenAPIObject, + OperationObject, + ParameterObject, + ResponseObject, + SchemaObject, +} from 'openapi3-ts'; +import Ajv from 'ajv'; +import Parser from '@apidevtools/swagger-parser'; + +const ajv = new Ajv({ allErrors: true }); // options can be passed, e.g. {allErrors: true} + +interface RequestResponsePair { + request: CompletedRequest; + response: CompletedResponse; +} + +interface ValidatorParams { + pair: RequestResponsePair; + operationSchema: OperationObject; + path: string; +} + +interface Validator { + validate(pair: ValidatorParams): Promise; +} + +class RequestErrorFactory { + static createRequestError(request: CompletedRequest, message: string): Error { + return new Error(`[${request.url} (${request.method})]: ${message}`); + } +} + +export class ParameterValidator implements Validator { + schema: OpenAPIObject; + cache: Record = {}; + constructor(schema: OpenAPIObject) { + this.schema = schema; + } + + async validate({ + pair: { request, response }, + operationSchema, + path, + }: ValidatorParams) { + if (response.statusCode === 400) { + // If the response is a 400, then the request is invalid and we shouldn't validate the parameters + return; + } + const parameters = operationSchema.parameters; + const queryParameters: Record = {}; + const headerParameters: Record = {}; + const pathParameters: Record = {}; + for (const parameter of parameters || []) { + if ('$ref' in parameter) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported', + ); + } + if (parameter.in === 'query') { + queryParameters[parameter.name] = parameter; + } + if (parameter.in === 'header') { + headerParameters[parameter.name] = parameter; + } + if (parameter.in === 'path') { + pathParameters[parameter.name] = parameter; + } + } + this.validateQueryParameters(queryParameters, request); + this.validateHeaderParameters(headerParameters, request); + this.validatePathParameters(pathParameters, request, path); + } + + validateQueryParameters( + queryParameters: Record, + request: CompletedRequest, + ) { + const { searchParams } = new URL(request.url); + for (const [name, parameter] of Object.entries(queryParameters)) { + if (!parameter.schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Schema not found for query parameter', + ); + } + if ('$ref' in parameter.schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported for parameters', + ); + } + let param: any | null = this.#findQueryParameters( + request, + queryParameters, + searchParams, + name, + ); + if (parameter.schema.type !== 'array' && Array.isArray(param)) { + param = param.length > 0 ? param[0] : undefined; + } + + if (!param && parameter.required) { + throw RequestErrorFactory.createRequestError( + request, + `Required query parameter ${name} not found`, + ); + } else if (!param && !parameter.required) { + continue; + } + if (parameter.schema.type === 'integer') { + // Try to parse the integer as AJV won't do it for us. + param = parseInt(param, 10); + } + const validate = ajv.compile(parameter.schema); + const valid = validate(param); + if (!valid) { + console.log(param); + console.error(validate.errors); + throw RequestErrorFactory.createRequestError( + request, + 'Query parameter validation failed', + ); + } + } + } + + #findQueryParameters( + request: CompletedRequest, + parameters: Record, + searchParams: URLSearchParams, + name: string, + ) { + const parameter = parameters[name]; + const schema = parameter.schema as SchemaObject; + if (schema.type === 'array') { + if (parameter.style === 'form' || !parameter.style) { + if (parameter.explode || typeof parameter.explode === 'undefined') { + if (!searchParams.has(name) && searchParams.has(`${name}[0]`)) { + const values: string[] = []; + let index = 0; + while (searchParams.has(`${name}[${index}]`)) { + values.push(searchParams.get(`${name}[${index}]`)!); + index++; + } + return values; + } + return searchParams.getAll(name); + } + if (!searchParams.has(name) && searchParams.has(`${name}[]`)) { + return searchParams.getAll(`${name}[]`); + } + return searchParams.get(name)?.split(','); + } else if (parameter.style === 'spaceDelimited') { + return searchParams.get(name)?.split(' '); + } else if (parameter.style === 'pipeDelimited') { + return searchParams.get(name)?.split('|'); + } + throw RequestErrorFactory.createRequestError( + request, + 'Unsupported style for array parameter', + ); + } + if (schema.type === 'object') { + if (parameter.style === 'form' || !parameter.style) { + if (parameter.explode) { + const obj: Record = {}; + for (const [key, value] of searchParams.entries()) { + if (this.#matchesOtherQueryParameters(parameters, key)) { + continue; + } + obj[key] = value; + } + console.log(obj); + return obj; + } + const obj: Record = {}; + const value = searchParams.get(name); + if (value) { + const parts = value.split(','); + if (parts.length % 2 !== 0) { + throw RequestErrorFactory.createRequestError( + request, + 'Invalid object parameter', + ); + } + for (let i = 0; i < parts.length; i += 2) { + obj[parts[i]] = parts[i + 1]; + } + } + return obj; + } else if (parameter.style === 'deepObject') { + const obj: Record = {}; + for (const [key, value] of searchParams.entries()) { + if (key.startsWith(`${name}[`)) { + const parts = key.split('['); + let currentLayer = obj; + for (let partIndex = 0; partIndex < parts.length - 1; partIndex++) { + const part = parts[partIndex]; + const objKey = part.split(']')[0]; + if (!currentLayer[objKey]) { + currentLayer[objKey] = {}; + } + currentLayer = currentLayer[objKey]; + } + currentLayer[parts[parts.length - 1].split(']')[0]] = value; + } + } + return obj; + } + throw RequestErrorFactory.createRequestError( + request, + 'Unsupported style for object parameter', + ); + } + // For everything else, just return the value. + return searchParams.getAll(name); + } + + #matchesOtherQueryParameters( + parameters: Record, + nameToMatch: string, + ) { + for (const [name] of Object.entries(parameters)) { + if (name === nameToMatch) { + return true; + } + } + return false; + } + + validateHeaderParameters( + headerParameters: Record, + request: CompletedRequest, + ) { + for (const [name, parameter] of Object.entries(headerParameters)) { + if (!request.headers[name]) { + throw RequestErrorFactory.createRequestError( + request, + `Header parameter ${name} not found`, + ); + } + if (!parameter.schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Schema not found for path parameter', + ); + } + if ('$ref' in parameter.schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported for parameters', + ); + } + const validate = ajv.compile(parameter.schema); + const valid = validate(request.headers[name]); + + if (!valid) { + console.log(request.headers[name]); + console.error(validate.errors); + throw RequestErrorFactory.createRequestError( + request, + 'Header parameter validation failed', + ); + } + } + } + + validatePathParameters( + pathParameters: Record, + request: CompletedRequest, + path: string, + ) { + const { pathname } = new URL(request.url); + const params = parsePath({ request, path: pathname, schema: path }); + for (const [name, parameter] of Object.entries(pathParameters)) { + if (!params[name] && parameter.required) { + throw RequestErrorFactory.createRequestError( + request, + `Path parameter ${name} not found`, + ); + } + if (!parameter.schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Schema not found for path parameter', + ); + } + if ('$ref' in parameter.schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported for parameters', + ); + } + + const validate = ajv.compile(parameter.schema); + const valid = validate(params[name]); + + if (!valid) { + console.log(params); + console.error(validate.errors); + throw RequestErrorFactory.createRequestError( + request, + 'Path parameter validation failed', + ); + } + } + } +} + +function parsePath({ + request, + schema, + path, +}: { + request: CompletedRequest; + schema: string; + path: string; +}) { + const parts = path.split('/'); + const pathParts = schema.split('/'); + if (parts.length !== pathParts.length) { + throw RequestErrorFactory.createRequestError( + request, + 'Path parts do not match', + ); + } + const params: Record = {}; + for (let i = 0; i < parts.length; i++) { + if (pathParts[i] === parts[i]) { + continue; + } + if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { + params[pathParts[i].slice(1, -1)] = parts[i]; + continue; + } + break; + } + return params; +} + +export class RequestBodyValidator implements Validator { + schema: OpenAPIObject; + constructor(schema: OpenAPIObject) { + this.schema = schema; + } + + async validate({ + pair: { request, response }, + operationSchema, + }: ValidatorParams) { + if (response.statusCode === 400) { + // If the response is a 400, then the request is invalid and we shouldn't validate the request body + return; + } + const requestBody = operationSchema.requestBody; + const bodyText = await request.body.getText(); + if (!requestBody && bodyText?.length) { + throw RequestErrorFactory.createRequestError( + request, + `No request body found for ${request.url}`, + ); + } else if (!requestBody && !bodyText?.length) { + // If there is no request body in the schema and no body in the request, then the request is valid + return; + } + if ('$ref' in requestBody!) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported', + ); + } + if (!requestBody!.content) { + throw RequestErrorFactory.createRequestError( + request, + 'No content found in request body', + ); + } + if (!requestBody!.content['application/json']) { + throw RequestErrorFactory.createRequestError( + request, + 'No application/json content type found in request body', + ); + } + const contentType = request.headers['content-type']; + if (!contentType) { + throw RequestErrorFactory.createRequestError( + request, + 'Content type not found in request', + ); + } + if (contentType !== 'application/json') { + throw RequestErrorFactory.createRequestError( + request, + 'Content type is not application/json', + ); + } + const schema = requestBody!.content['application/json'].schema; + if (!schema) { + throw RequestErrorFactory.createRequestError( + request, + 'No schema found in request body', + ); + } + if ('$ref' in schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported', + ); + } + + const validate = ajv.compile(schema); + const body = await request.body.getJson(); + const valid = validate(body); + if (!valid) { + console.log(body); + console.error(validate.errors); + throw RequestErrorFactory.createRequestError( + request, + `Request body validation failed.`, + ); + } + } +} + +export class ResponseBodyValidator implements Validator { + schema: OpenAPIObject; + constructor(schema: OpenAPIObject) { + this.schema = schema; + } + + async validate(pair: ValidatorParams) { + const { + pair: { response, request }, + operationSchema, + } = pair; + const responseSchema = this.findResponseSchema(operationSchema, response); + if (!responseSchema) { + throw RequestErrorFactory.createRequestError( + request, + `No response schema found for ${response.statusCode}`, + ); + } + const body = await response.body.getText(); + if (!responseSchema.content && body?.length) { + throw RequestErrorFactory.createRequestError( + request, + 'No content found in response', + ); + } else if (!responseSchema.content && !body?.length) { + // If there is no content in the response schema and no body in the response, then the response is valid + return; + } + if (!responseSchema.content!['application/json']) { + throw RequestErrorFactory.createRequestError( + request, + 'No application/json content type found in response', + ); + } + const schema = responseSchema.content!['application/json'].schema; + if (!schema) { + throw RequestErrorFactory.createRequestError( + request, + 'No schema found in response', + ); + } + if ('$ref' in schema) { + throw RequestErrorFactory.createRequestError( + request, + 'Reference objects are not supported', + ); + } + + const validate = ajv.compile(schema); + const valid = validate(await response.body.getJson()); + if (!valid) { + console.log(await response.body.getJson()); + console.error(validate.errors); + throw RequestErrorFactory.createRequestError( + request, + 'Response body validation failed', + ); + } + } + + private findResponseSchema( + operationSchema: OperationObject, + response: CompletedResponse, + ): ResponseObject | undefined { + const { statusCode } = response; + return operationSchema.responses?.[statusCode]; + } +} + +export class OpenApiProxyValidator { + schema: OpenAPIObject | undefined; + validators: Validator[] | undefined; + + async initialize(url: string) { + this.schema = (await Parser.dereference(url)) as unknown as OpenAPIObject; + this.validators = [ + new ParameterValidator(this.schema), + new RequestBodyValidator(this.schema), + // new ResponseBodyValidator(this.schema), + ]; + } + + validate(request: CompletedRequest, response: CompletedResponse) { + const operation = this.findOperation(request); + if (!operation) { + throw RequestErrorFactory.createRequestError( + request, + `No operation schema found for ${request.url}`, + ); + } + + const [path, operationSchema] = operation; + + const validators = this.validators!; + for (const validator of validators) { + validator.validate({ + pair: { request, response }, + operationSchema, + path, + }); + } + } + + private findOperation( + request: CompletedRequest, + ): [string, OperationObject] | undefined { + const { url } = request; + const { pathname } = new URL(url); + + const parts = pathname.split('/'); + for (const [path, schema] of Object.entries(this.schema!.paths)) { + const pathParts = path.split('/'); + if (parts.length !== pathParts.length) { + continue; + } + let found = true; + for (let i = 0; i < parts.length; i++) { + if (pathParts[i] === parts[i]) { + continue; + } + if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { + continue; + } + found = false; + break; + } + if (!found) { + continue; + } + let matchingOperationType: OperationObject | undefined = undefined; + for (const [operationType, operation] of Object.entries(schema)) { + if (operationType === request.method.toLowerCase()) { + matchingOperationType = operation as OperationObject; + break; + } + } + if (!matchingOperationType) { + continue; + } + return [path, matchingOperationType]; + } + + return undefined; + } +} diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index 2ac5575fc4..f96a0b1b76 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -15,6 +15,23 @@ */ import { Express } from 'express'; import { Server } from 'http'; +import { Proxy } from './proxy/setup'; + +const proxy = new Proxy(); + +beforeAll(async () => { + await proxy.setup(); +}); + +afterAll(() => { + proxy.stop(); +}); + +export async function wrapServer(app: Express): Promise { + const server = app.listen(+process.env.PORT!); + await proxy.initialize(); + return { ...server, address: () => new URL(proxy.url) } as any; +} /** * !!! THIS CURRENTLY ONLY SUPPORTS SUPERTEST !!! diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 9cc0717807..6654187526 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -38,7 +38,7 @@ import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common/a import { CatalogProcessingOrchestrator } from '../processing/types'; import { z } from 'zod'; import { decodeCursor, encodeCursor } from './util'; -import { wrapInOpenApiTestServer } from '@backstage/backend-openapi-utils'; +import { wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; @@ -93,7 +93,7 @@ describe('createRouter readonly disabled', () => { locationAnalyzer, permissionsService: permissionsService, }); - app = wrapInOpenApiTestServer(express().use(router)); + app = await wrapServer(express().use(router)); }); beforeEach(() => { diff --git a/plugins/search-backend/src/schema/openapi.generated.ts b/plugins/search-backend/src/schema/openapi.generated.ts index 8ad27546d6..56aaeb2bbe 100644 --- a/plugins/search-backend/src/schema/openapi.generated.ts +++ b/plugins/search-backend/src/schema/openapi.generated.ts @@ -207,7 +207,7 @@ export const spec = { name: 'filters', in: 'query', required: false, - style: 'deepObject', + style: 'form', explode: true, allowReserved: true, schema: { diff --git a/plugins/search-backend/src/schema/openapi.yaml b/plugins/search-backend/src/schema/openapi.yaml index 358f94fcf6..a49b783b64 100644 --- a/plugins/search-backend/src/schema/openapi.yaml +++ b/plugins/search-backend/src/schema/openapi.yaml @@ -139,7 +139,7 @@ paths: - name: filters in: query required: false - style: deepObject + style: form explode: true allowReserved: true schema: diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index d22fc17e54..5b6900ede1 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -23,7 +23,7 @@ import { import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; -import { wrapInOpenApiTestServer } from '@backstage/backend-openapi-utils'; +import { wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, @@ -87,9 +87,7 @@ describe('createRouter', () => { auth: mockServices.auth(), httpAuth: mockServices.httpAuth(), }); - app = wrapInOpenApiTestServer( - express().use(router).use(mockErrorHandler()), - ); + app = await wrapServer(express().use(router).use(mockErrorHandler())); }); beforeEach(() => { diff --git a/yarn.lock b/yarn.lock index f5184e9149..036ac5c852 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3738,16 +3738,19 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/backend-openapi-utils@workspace:packages/backend-openapi-utils" dependencies: + "@apidevtools/swagger-parser": ^10.1.0 "@backstage/backend-plugin-api": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/errors": "workspace:^" "@types/express": ^4.17.6 "@types/express-serve-static-core": ^4.17.5 + ajv: ^8.16.0 express: ^4.17.1 express-openapi-validator: ^5.0.4 express-promise-router: ^4.1.0 json-schema-to-ts: ^3.0.0 lodash: ^4.17.21 + mockttp: ^3.13.0 openapi-merge: ^1.3.2 openapi3-ts: ^3.1.2 supertest: ^7.0.0 @@ -10313,6 +10316,15 @@ __metadata: languageName: node linkType: hard +"@httptoolkit/httpolyglot@npm:^2.2.1": + version: 2.2.1 + resolution: "@httptoolkit/httpolyglot@npm:2.2.1" + dependencies: + "@types/node": "*" + checksum: 5b3882657e37953bd7089d91ac6cd24cec36480deab114e6b69a4b3d9e4ab09db568500e5e96713869fb4a8fe40b5ecc1661cc39ee621ef40ed0e38b55e0257e + languageName: node + linkType: hard + "@httptoolkit/subscriptions-transport-ws@npm:^0.11.2": version: 0.11.2 resolution: "@httptoolkit/subscriptions-transport-ws@npm:0.11.2" @@ -20715,7 +20727,7 @@ __metadata: languageName: node linkType: hard -"ajv@npm:^8.0.0, ajv@npm:^8.10.0, ajv@npm:^8.11.0, ajv@npm:^8.12.0, ajv@npm:^8.17.1, ajv@npm:^8.6.0, ajv@npm:^8.6.3, ajv@npm:^8.9.0": +"ajv@npm:^8.0.0, ajv@npm:^8.10.0, ajv@npm:^8.11.0, ajv@npm:^8.12.0, ajv@npm:^8.16.0, ajv@npm:^8.17.1, ajv@npm:^8.6.0, ajv@npm:^8.6.3, ajv@npm:^8.9.0": version: 8.17.1 resolution: "ajv@npm:8.17.1" dependencies: @@ -21339,6 +21351,15 @@ __metadata: languageName: node linkType: hard +"async-mutex@npm:^0.5.0": + version: 0.5.0 + resolution: "async-mutex@npm:0.5.0" + dependencies: + tslib: ^2.4.0 + checksum: be1587f4875f3bb15e34e9fcce82eac2966daef4432c8d0046e61947fb9a1b95405284601bc7ce4869319249bc07c75100880191db6af11d1498931ac2a2f9ea + languageName: node + linkType: hard + "async-retry@npm:^1.3.3": version: 1.3.3 resolution: "async-retry@npm:1.3.3" @@ -22111,6 +22132,13 @@ __metadata: languageName: node linkType: hard +"brotli-wasm@npm:^3.0.0": + version: 3.0.1 + resolution: "brotli-wasm@npm:3.0.1" + checksum: 48191b27265de8ffc59c940f9efef3a931448b6a15c26a4e360192fc3f0968e073c11fe0926510d019c305cc1d9c6d65df4d3e5752648a91cb0bbcccff7a8460 + languageName: node + linkType: hard + "browser-headers@npm:^0.4.1": version: 0.4.1 resolution: "browser-headers@npm:0.4.1" @@ -24995,6 +25023,15 @@ __metadata: languageName: node linkType: hard +"destroyable-server@npm:^1.0.2": + version: 1.0.2 + resolution: "destroyable-server@npm:1.0.2" + dependencies: + "@types/node": "*" + checksum: 81fd70b9132d43c3633a7a819adfe1fc68b52a55154ff8a36f42f4655e7b71b8468559888caadfd324c1aa824f0d236796a8f356e8a00e7438649e647ea654b2 + languageName: node + linkType: hard + "detect-indent@npm:^6.0.0": version: 6.1.0 resolution: "detect-indent@npm:6.1.0" @@ -29301,6 +29338,17 @@ __metadata: languageName: node linkType: hard +"http-encoding@npm:^2.0.1": + version: 2.0.1 + resolution: "http-encoding@npm:2.0.1" + dependencies: + brotli-wasm: ^3.0.0 + pify: ^5.0.0 + zstd-codec: ^0.1.5 + checksum: c34a1cd81ad1c08e6c6aba5aef3f4d4bc4a6c84f8b3511776eb62006beeee48a104ce1630e3c8497f66d5c0913195dea596e776336dd5a598bd7fe06d27e1395 + languageName: node + linkType: hard + "http-errors@npm:2.0.0, http-errors@npm:^2.0.0": version: 2.0.0 resolution: "http-errors@npm:2.0.0" @@ -29459,6 +29507,16 @@ __metadata: languageName: node linkType: hard +"http2-wrapper@npm:^2.2.1": + version: 2.2.1 + resolution: "http2-wrapper@npm:2.2.1" + dependencies: + quick-lru: ^5.1.1 + resolve-alpn: ^1.2.0 + checksum: e95e55e22c6fd61182ce81fecb9b7da3af680d479febe8ad870d05f7ebbc9f076e455193766f4e7934e50913bf1d8da3ba121fb5cd2928892390b58cf9d5c509 + languageName: node + linkType: hard + "https-browserify@npm:^1.0.0": version: 1.0.0 resolution: "https-browserify@npm:1.0.0" @@ -34535,6 +34593,58 @@ __metadata: languageName: node linkType: hard +"mockttp@npm:^3.13.0": + version: 3.15.2 + resolution: "mockttp@npm:3.15.2" + dependencies: + "@graphql-tools/schema": ^8.5.0 + "@graphql-tools/utils": ^8.8.0 + "@httptoolkit/httpolyglot": ^2.2.1 + "@httptoolkit/subscriptions-transport-ws": ^0.11.2 + "@httptoolkit/websocket-stream": ^6.0.1 + "@types/cors": ^2.8.6 + "@types/node": "*" + async-mutex: ^0.5.0 + base64-arraybuffer: ^0.1.5 + body-parser: ^1.15.2 + cacheable-lookup: ^6.0.0 + common-tags: ^1.8.0 + connect: ^3.7.0 + cors: ^2.8.4 + cors-gate: ^1.1.3 + cross-fetch: ^3.1.5 + destroyable-server: ^1.0.2 + express: ^4.14.0 + fast-json-patch: ^3.1.1 + graphql: ^14.0.2 || ^15.5 + graphql-http: ^1.22.0 + graphql-subscriptions: ^1.1.0 + graphql-tag: ^2.12.6 + http-encoding: ^2.0.1 + http2-wrapper: ^2.2.1 + https-proxy-agent: ^5.0.1 + isomorphic-ws: ^4.0.1 + lodash: ^4.16.4 + lru-cache: ^7.14.0 + native-duplexpair: ^1.0.0 + node-forge: ^1.2.1 + pac-proxy-agent: ^7.0.0 + parse-multipart-data: ^1.4.0 + performance-now: ^2.1.0 + portfinder: ^1.0.32 + read-tls-client-hello: ^1.0.0 + semver: ^7.5.3 + socks-proxy-agent: ^7.0.0 + typed-error: ^3.0.2 + urlpattern-polyfill: ^8.0.0 + uuid: ^8.3.2 + ws: ^8.8.0 + bin: + mockttp: dist/admin/admin-bin.js + checksum: 96b90e0515e7ac1b73954e9e01010424d51d9563f8e850e620b06ba864bf064401e1a1af89e103724b956bfa3bee790cb452366df100bf01122f333e04c3aee8 + languageName: node + linkType: hard + "mockttp@npm:^3.9.1": version: 3.9.4 resolution: "mockttp@npm:3.9.4" @@ -43704,6 +43814,13 @@ __metadata: languageName: node linkType: hard +"urlpattern-polyfill@npm:^8.0.0": + version: 8.0.2 + resolution: "urlpattern-polyfill@npm:8.0.2" + checksum: d2cc0905a613c77e330c426e8697ee522dd9640eda79ac51160a0f6350e103f09b8c327623880989f8ba7325e8d95267b745aa280fdcc2aead80b023e16bd09d + languageName: node + linkType: hard + "urlpattern-polyfill@npm:^9.0.0": version: 9.0.0 resolution: "urlpattern-polyfill@npm:9.0.0" @@ -45259,6 +45376,13 @@ __metadata: languageName: node linkType: hard +"zstd-codec@npm:^0.1.5": + version: 0.1.5 + resolution: "zstd-codec@npm:0.1.5" + checksum: ba62bf643c3ca9759fedc090b73a0c3b1e506364fcae902a70b112c1f5b30bc6aabff3184808cc4430f2ab6644cabae979368152ae908c1d8ef39cd8c3223c85 + languageName: node + linkType: hard + "zwitch@npm:^2.0.0": version: 2.0.2 resolution: "zwitch@npm:2.0.2" From 5555a58d62324237a7eab4dd2478ad0477d892f5 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Fri, 5 Jul 2024 17:19:05 -0400 Subject: [PATCH 02/23] throw on failure correctly Signed-off-by: aramissennyeydd --- .../backend-openapi-utils/src/proxy/setup.ts | 9 +++++---- .../src/schema/validation.ts | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/backend-openapi-utils/src/proxy/setup.ts b/packages/backend-openapi-utils/src/proxy/setup.ts index a7205a883a..8f24aa2512 100644 --- a/packages/backend-openapi-utils/src/proxy/setup.ts +++ b/packages/backend-openapi-utils/src/proxy/setup.ts @@ -44,11 +44,12 @@ export class Proxy { this.requestResponsePairs.set(request, response); } delete this.#openRequests[response.id]; - try { - this.validator.validate(request, response); - } catch (err) { + this.validator.validate(request, response).catch(err => { + if (process.env.THROW) { + throw err; + } console.error(err); - } + }); }); } diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index 104482a485..9f2f22b9ac 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -522,7 +522,7 @@ export class OpenApiProxyValidator { ]; } - validate(request: CompletedRequest, response: CompletedResponse) { + async validate(request: CompletedRequest, response: CompletedResponse) { const operation = this.findOperation(request); if (!operation) { throw RequestErrorFactory.createRequestError( @@ -534,13 +534,15 @@ export class OpenApiProxyValidator { const [path, operationSchema] = operation; const validators = this.validators!; - for (const validator of validators) { - validator.validate({ - pair: { request, response }, - operationSchema, - path, - }); - } + await Promise.all( + validators.map(validator => + validator.validate({ + pair: { request, response }, + operationSchema, + path, + }), + ), + ); } private findOperation( From 3bf5285bd63e8c444e19015e7752034b4a061521 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 13:04:16 -0400 Subject: [PATCH 03/23] adjusting validation library format and adding test cases Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 3 + .../schemas/withJsonResponseBody.json | 35 + .../schemas/withQueryParameter.json | 25 + .../src/schema/errors.ts | 23 + .../src/schema/parameter-validation.test.ts | 458 +++++++++++ .../src/schema/parameter-validation.ts | 420 +++++++++++ .../src/schema/request-body-validation.ts | 115 +++ .../src/schema/response-body-validation.ts | 102 +++ .../backend-openapi-utils/src/schema/types.ts | 50 ++ .../backend-openapi-utils/src/schema/utils.ts | 36 + .../src/schema/validation.test.ts | 711 ++++++++++++++++++ .../src/schema/validation.ts | 501 +----------- yarn.lock | 3 + 13 files changed, 2007 insertions(+), 475 deletions(-) create mode 100644 packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json create mode 100644 packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withQueryParameter.json create mode 100644 packages/backend-openapi-utils/src/schema/errors.ts create mode 100644 packages/backend-openapi-utils/src/schema/parameter-validation.test.ts create mode 100644 packages/backend-openapi-utils/src/schema/parameter-validation.ts create mode 100644 packages/backend-openapi-utils/src/schema/request-body-validation.ts create mode 100644 packages/backend-openapi-utils/src/schema/response-body-validation.ts create mode 100644 packages/backend-openapi-utils/src/schema/types.ts create mode 100644 packages/backend-openapi-utils/src/schema/utils.ts create mode 100644 packages/backend-openapi-utils/src/schema/validation.test.ts diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index a173cbf17b..5fb1f98e1b 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -35,7 +35,9 @@ "dependencies": { "@apidevtools/swagger-parser": "^10.1.0", "@backstage/backend-plugin-api": "workspace:^", + "@backstage/backend-test-utils": "workspace:^", "@backstage/errors": "workspace:^", + "@backstage/types": "workspace:^", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", "ajv": "^8.16.0", @@ -45,6 +47,7 @@ "json-schema-to-ts": "^3.0.0", "lodash": "^4.17.21", "mockttp": "^3.13.0", + "msw": "^1.0.0", "openapi-merge": "^1.3.2", "openapi3-ts": "^3.1.2" }, diff --git a/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json new file mode 100644 index 0000000000..378ea0a168 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json @@ -0,0 +1,35 @@ +{ + "openapi": "3.0.0", + "info": { "title": "Test", "version": "1.0.0" }, + "paths": { + "/api/search": { + "get": { + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "results": { + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { + "type": "string" + } + } + } + } + } + } + } + } + } + } + } + } + } +} diff --git a/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withQueryParameter.json b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withQueryParameter.json new file mode 100644 index 0000000000..6f0db99697 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withQueryParameter.json @@ -0,0 +1,25 @@ +{ + "openapi": "3.0.0", + "info": { "title": "Test", "version": "1.0.0" }, + "paths": { + "/api/search": { + "get": { + "responses": { + "200": { + "description": "OK" + } + }, + "parameters": [ + { + "name": "param", + "in": "query", + "required": false, + "schema": { + "type": "string" + } + } + ] + } + } + } +} diff --git a/packages/backend-openapi-utils/src/schema/errors.ts b/packages/backend-openapi-utils/src/schema/errors.ts new file mode 100644 index 0000000000..aab5210f46 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/errors.ts @@ -0,0 +1,23 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Operation } from './types'; + +export class OperationError extends Error { + constructor(operation: Operation, message: string) { + super(`[${operation.path} (${operation.method})]: ${message}`); + } +} diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts new file mode 100644 index 0000000000..2fb8f371b1 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts @@ -0,0 +1,458 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import _ from 'lodash'; +import withQueryParameter from './__fixtures__/schemas/withQueryParameter.json'; +import { QueryParameterParser } from './parameter-validation'; +import { OperationObject, ParameterObject } from 'openapi3-ts'; +import Ajv from 'ajv'; +import { Operation } from './types'; + +const ajv = new Ajv(); + +describe('query parameters', () => { + let operation: Operation; + let parser: QueryParameterParser; + let schema: (typeof withQueryParameter)['paths']['/api/search']['get']; + + beforeEach(() => { + schema = _.cloneDeep(withQueryParameter.paths['/api/search'].get); + operation = { + schema: schema as OperationObject, + path: '/api/search', + method: 'get', + }; + parser = new QueryParameterParser(operation, { ajv }); + }); + describe('primitives', () => { + describe('string', () => { + it('should parse a string', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello', + } as Request; + const result = await parser.parse(request); + expect(result.param).toBe('hello'); + }); + + it('should throw an error if there are extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + + it('should throw an error if the parameter is required but missing', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + }); + + describe('number', () => { + beforeEach(() => { + schema.parameters![0].schema.type = 'number'; + }); + it('should parse a number', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=42', + } as Request; + const result = await parser.parse(request); + expect(result.param).toBe(42); + }); + + it('should throw an error if the parameter is not a number', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Query parameter validation failed"`, + ); + }); + }); + }); + + describe('arrays', () => { + beforeEach(() => { + schema.parameters![0].schema.type = 'array'; + }); + describe('form', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).style = 'form'; + }); + describe('explode=true', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).explode = true; + }); + it('should parse a form array with a single element', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual(['hello']); + }); + + it('should parse a form array with multiple elements', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello¶m=world', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual(['hello', 'world']); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should throw for extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + }); + + describe('explode=false', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).explode = false; + }); + + it('should parse a form array with a single element', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual(['hello']); + }); + + it('should parse a form array with multiple elements', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello,world', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual(['hello', 'world']); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should throw for extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + }); + }); + + describe('spaceDelimited', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).style = 'spaceDelimited'; + }); + + it('should parse a space separated array', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello%20world', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual(['hello', 'world']); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should throw for extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello%20world&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + }); + + describe('pipeDelimited', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).style = 'pipeDelimited'; + }); + + it('should parse a pipe separated array', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello|world', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual(['hello', 'world']); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should throw for extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=hello|world&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + }); + }); + + describe('objects', () => { + describe('form', () => { + beforeEach(() => { + schema.parameters![0].schema.type = 'object'; + }); + + describe('explode=true', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).explode = true; + }); + + it('should parse a form object with a single key', async () => { + const request = { + url: 'http://localhost:8080/api/search?key=value', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key: 'value' }); + }); + + it('should parse a form object with multiple keys', async () => { + const request = { + url: 'http://localhost:8080/api/search?key1=value1&key2=value2', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key1: 'value1', key2: 'value2' }); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should respect other parameter encodings', async () => { + const parameter = { + name: 'extra', + in: 'query', + style: 'form', + explode: false, + schema: { type: 'array' }, + required: false, + } as ParameterObject; + schema.parameters!.push(parameter as any); + parser = new QueryParameterParser(operation, { ajv }); + const request = { + url: 'http://localhost:8080/api/search?key=value&otherkey=value2&extra=hello,world', + } as Request; + + const result = await parser.parse(request); + expect(result.param).toEqual({ + key: 'value', + otherkey: 'value2', + }); + expect(result.extra).toEqual(['hello', 'world']); + }); + }); + + describe('explode=false', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).explode = false; + }); + + it('should parse a form object with a single key', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=key,value', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key: 'value' }); + }); + + it('should parse a form object with multiple keys', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=key1,value1,key2,value2', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key1: 'value1', key2: 'value2' }); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should throw for extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param=key,value&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + }); + }); + + describe('deepObject', () => { + beforeEach(() => { + (schema.parameters![0] as ParameterObject).style = 'deepObject'; + (schema.parameters![0] as ParameterObject).explode = true; + (schema.parameters![0] as ParameterObject).schema = { + type: 'object', + properties: { + key: { + type: 'string', + }, + }, + }; + }); + + it('should parse a deep object', async () => { + const request = { + url: 'http://localhost:8080/api/search?param[key]=value', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key: 'value' }); + }); + + it('should parse a deep object with multiple keys', async () => { + const request = { + url: 'http://localhost:8080/api/search?param[key1]=value1¶m[key2]=value2', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key1: 'value1', key2: 'value2' }); + }); + + it('should throw for missing required parameters', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/search', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Required query parameter param not found"`, + ); + }); + + it('should throw for extra parameters', async () => { + const request = { + url: 'http://localhost:8080/api/search?param[key]=value&extra=world', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (get)]: Unexpected query parameters: extra"`, + ); + }); + + it('should handle nested objects', async () => { + (schema.parameters![0] as ParameterObject).schema = { + type: 'object', + properties: { + key: { + type: 'object', + properties: { + subkey: { + type: 'string', + }, + }, + required: ['subkey'], + }, + }, + }; + parser = new QueryParameterParser(operation, { ajv }); + const request = { + url: 'http://localhost:8080/api/search?param[key][subkey]=value', + } as Request; + const result = await parser.parse(request); + expect(result.param).toEqual({ key: { subkey: 'value' } }); + }); + }); + }); +}); diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts new file mode 100644 index 0000000000..15570e120c --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -0,0 +1,420 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { OpenAPIObject, ParameterObject, SchemaObject } from 'openapi3-ts'; +import { + Operation, + ParserOptions, + RequestParser, + Validator, + ValidatorParams, +} from './types'; +import Ajv from 'ajv'; +import { OperationError } from './errors'; +import { mockttpToFetchRequest } from './utils'; + +type ReferencelessSchemaObject = SchemaObject & { $ref?: never }; + +type ReferencelessParameterObject = Omit & { + schema: ReferencelessSchemaObject; +}; + +class BaseParameterParser { + ajv: Ajv; + operation: Operation; + parameters: Record = {}; + constructor(operation: Operation, options: ParserOptions) { + this.ajv = options.ajv; + this.operation = operation; + const { schema, path, method } = operation; + const parameters = schema.parameters || []; + for (const parameter of parameters) { + if ('$ref' in parameter) { + throw new Error( + `[(${method}) ${path}] Reference objects are not supported`, + ); + } + + if (!parameter.schema) { + throw new OperationError( + operation, + 'Schema not found for path parameter', + ); + } + if ('$ref' in parameter.schema) { + throw new OperationError( + this.operation, + 'Reference objects are not supported for parameters', + ); + } + if (parameter.in === 'query') { + this.parameters[parameter.name] = + parameter as ReferencelessParameterObject; + } + } + } +} + +export class QueryParameterParser + extends BaseParameterParser + implements RequestParser> +{ + async parse(request: Request) { + const { searchParams } = new URL(request.url); + const remainingQueryParameters = new Set(searchParams.keys()); + const queryParameters: Record = {}; + const parameterIterator = Object.entries(this.parameters).toSorted( + ([_, parameter]) => { + if (parameter.schema.type !== 'object') { + return -1; + } + if (parameter.style === 'form' || !parameter.style) { + if (parameter.explode || typeof parameter.explode === 'undefined') { + return 1; + } + return 0; + } + return 0; + }, + ); + for (const [name, parameter] of parameterIterator) { + if (!parameter.schema) { + throw new OperationError( + this.operation, + 'Schema not found for query parameter', + ); + } + if ('$ref' in parameter.schema) { + throw new OperationError( + this.operation, + 'Reference objects are not supported for parameters', + ); + } + // eslint-disable-next-line prefer-const + let [param, indices]: [any | null, string[]] = this.#findQueryParameters( + this.parameters, + queryParameters, + searchParams, + name, + ); + if (!!param) { + indices.forEach(index => remainingQueryParameters.delete(index)); + } + if (parameter.schema.type !== 'array' && Array.isArray(param)) { + param = param.length > 0 ? param[0] : undefined; + } + if ( + parameter.required && + !indices.some(index => searchParams.has(index)) + ) { + throw new OperationError( + this.operation, + `Required query parameter ${name} not found`, + ); + } else if (!param && !parameter.required) { + continue; + } + if (parameter.schema.type === 'integer') { + // Try to parse the integer as AJV won't do it for us. + param = parseInt(param, 10); + } + if (parameter.schema.type === 'number') { + // Try to parse the number as AJV won't do it for us. + param = parseFloat(param); + } + const validate = this.ajv.compile(parameter.schema); + const valid = validate(param); + if (!valid) { + throw new OperationError( + this.operation, + 'Query parameter validation failed', + ); + } + queryParameters[name] = param; + } + if (remainingQueryParameters.size > 0) { + throw new OperationError( + this.operation, + `Unexpected query parameters: ${Array.from( + remainingQueryParameters, + ).join(', ')}`, + ); + } + return queryParameters; + } + + #findQueryParameters( + parameters: Record, + currentQueryParameters: Record, + searchParams: URLSearchParams, + name: string, + ): [any | null, string[]] { + const parameter = parameters[name]; + const schema = parameter.schema as SchemaObject; + + const getIfExists = (key: string) => + searchParams.has(key) ? searchParams.getAll(key) : null; + + if (schema.type === 'array') { + if (parameter.style === 'form' || !parameter.style) { + if (parameter.explode || typeof parameter.explode === 'undefined') { + if (!searchParams.has(name) && searchParams.has(`${name}[0]`)) { + const values: string[] = []; + const indices: string[] = []; + let index = 0; + while (searchParams.has(`${name}[${index}]`)) { + values.push(searchParams.get(`${name}[${index}]`)!); + indices.push(`${name}[${index}]`); + index++; + } + return [values, indices]; + } + return [getIfExists(name), [name]]; + } + if (!searchParams.has(name) && searchParams.has(`${name}[]`)) { + return [searchParams.get(`${name}[]`)?.split(','), [`${name}[]`]]; + } + if (searchParams.has(name) && searchParams.getAll(name).length > 1) { + throw new OperationError( + this.operation, + 'Array parameter should not have multiple values', + ); + } + return [searchParams.get(name)?.split(','), [name]]; + } else if (parameter.style === 'spaceDelimited') { + return [searchParams.get(name)?.split(' '), [name]]; + } else if (parameter.style === 'pipeDelimited') { + return [searchParams.get(name)?.split('|'), [name]]; + } + throw new OperationError( + this.operation, + 'Unsupported style for array parameter', + ); + } + if (schema.type === 'object') { + if (parameter.style === 'form' || !parameter.style) { + if (parameter.explode) { + const obj: Record = {}; + const indices: string[] = []; + for (const [key, value] of searchParams.entries()) { + if ( + this.#matchesOtherQueryParameters(currentQueryParameters, key) + ) { + continue; + } + indices.push(key); + obj[key] = value; + } + return [obj, indices]; + } + const obj: Record = {}; + const value = searchParams.get(name); + if (value) { + const parts = value.split(','); + if (parts.length % 2 !== 0) { + throw new OperationError( + this.operation, + 'Invalid object parameter', + ); + } + for (let i = 0; i < parts.length; i += 2) { + obj[parts[i]] = parts[i + 1]; + } + } + return [obj, [name]]; + } else if (parameter.style === 'deepObject') { + const obj: Record = {}; + const indices: string[] = []; + for (const [key, value] of searchParams.entries()) { + if (key.startsWith(`${name}[`)) { + indices.push(key); + const parts = key.split('['); + let currentLayer = obj; + for (let partIndex = 1; partIndex < parts.length - 1; partIndex++) { + const part = parts[partIndex]; + if (!part.includes(']')) { + throw new OperationError( + this.operation, + 'Invalid object parameter', + ); + } + const objKey = part.split(']')[0]; + if (!currentLayer[objKey]) { + currentLayer[objKey] = {}; + } + currentLayer = currentLayer[objKey]; + } + const lastPart = parts[parts.length - 1]; + if (!lastPart.includes(']')) { + throw new OperationError( + this.operation, + 'Invalid object parameter', + ); + } + currentLayer[lastPart.split(']')[0]] = value; + } + } + return [obj, indices]; + } + throw new OperationError( + this.operation, + 'Unsupported style for object parameter', + ); + } + // For everything else, just return the value. + return [getIfExists(name), [name]]; + } + + #matchesOtherQueryParameters( + parameters: Record, + nameToMatch: string, + ) { + for (const [name] of Object.entries(parameters)) { + if (name === nameToMatch) { + return true; + } + } + return false; + } +} + +export class HeaderParameterParser + extends BaseParameterParser + implements RequestParser> +{ + async parse(request: Request) { + const headerParameters: Record = {}; + for (const [name, parameter] of Object.entries(this.parameters)) { + const header = request.headers.get(name); + if (!header) { + if (parameter.required) { + throw new OperationError( + this.operation, + `Header parameter ${name} not found`, + ); + } + continue; + } + if (!parameter.schema) { + throw new OperationError( + this.operation, + 'Schema not found for header parameter', + ); + } + if ('$ref' in parameter.schema) { + throw new OperationError( + this.operation, + 'Reference objects are not supported for parameters', + ); + } + const validate = this.ajv.compile(parameter.schema); + const valid = validate(header); + + if (!valid) { + throw new OperationError( + this.operation, + 'Header parameter validation failed', + ); + } + headerParameters[name] = header; + } + return headerParameters; + } +} + +export class PathParameterParser + extends BaseParameterParser + implements RequestParser> +{ + async parse(request: Request) { + const { pathname } = new URL(request.url); + const params = this.parsePath({ + path: pathname, + schema: this.operation.path, + }); + const pathParameters: Record = {}; + for (const [name, parameter] of Object.entries(this.parameters)) { + if (!params[name] && parameter.required) { + throw new OperationError( + this.operation, + `Path parameter ${name} not found`, + ); + } else if (!params[name] && !parameter.required) { + continue; + } + + const validate = this.ajv.compile(parameter.schema); + const valid = validate(params[name]); + + if (!valid) { + throw new OperationError( + this.operation, + 'Path parameter validation failed', + ); + } + pathParameters[name] = params[name]; + } + return pathParameters; + } + + parsePath({ schema, path }: { schema: string; path: string }) { + const parts = path.split('/'); + const pathParts = schema.split('/'); + if (parts.length !== pathParts.length) { + throw new OperationError(this.operation, 'Path parts do not match'); + } + const params: Record = {}; + for (let i = 0; i < parts.length; i++) { + if (pathParts[i] === parts[i]) { + continue; + } + if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { + params[pathParts[i].slice(1, -1)] = parts[i]; + continue; + } + break; + } + return params; + } +} + +export class ParameterValidator implements Validator { + schema: OpenAPIObject; + cache: Record = {}; + constructor(schema: OpenAPIObject) { + this.schema = schema; + } + + async validate({ pair: { request, response }, operation }: ValidatorParams) { + if (response.statusCode === 400) { + // If the response is a 400, then the request is invalid and we shouldn't validate the parameters + return; + } + + const ajv = new Ajv(); + const queryParser = new QueryParameterParser(operation, { ajv }); + const headerParser = new HeaderParameterParser(operation, { ajv }); + const pathParser = new PathParameterParser(operation, { ajv }); + + const fetchRequest = mockttpToFetchRequest(request); + + await Promise.all([ + queryParser.parse(fetchRequest), + headerParser.parse(fetchRequest), + pathParser.parse(fetchRequest), + ]); + } +} diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.ts new file mode 100644 index 0000000000..f35b8f07c7 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.ts @@ -0,0 +1,115 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonObject } from '@backstage/types'; +import { Operation, ParserOptions, RequestParser } from './types'; +import { ValidateFunction } from 'ajv'; +import { OperationError } from './errors'; + +export class RequestBodyParser + implements RequestParser +{ + operation: Operation; + validate: + | { fn: ValidateFunction; disabled: false } + | { + fn: undefined; + disabled: true; + }; + constructor(operation: Operation, options: ParserOptions) { + this.operation = operation; + const { schema: operationSchema } = this.operation; + const requestBody = operationSchema.requestBody; + if (!requestBody) { + this.validate = { disabled: true, fn: undefined }; + return; + } + + if ('$ref' in requestBody!) { + throw new OperationError( + this.operation, + 'Reference objects are not supported', + ); + } + if (!requestBody!.content) { + throw new OperationError( + this.operation, + 'No content found in request body', + ); + } + if (!requestBody!.content['application/json']) { + throw new OperationError( + this.operation, + 'No application/json content type found in request body', + ); + } + const schema = requestBody!.content['application/json'].schema; + if (!schema) { + throw new OperationError( + this.operation, + 'No JSON schema found in request body', + ); + } + if ('$ref' in schema) { + throw new OperationError( + this.operation, + 'Reference objects are not supported', + ); + } + this.validate = { + disabled: false, + fn: options.ajv.compile(operation.schema), + }; + } + async parse(request: Request): Promise { + const { disabled, fn } = this.validate; + const bodyText = await request.text(); + if (!disabled && bodyText?.length) { + throw new OperationError( + this.operation, + `No request body found for ${request.url}`, + ); + } else if (disabled && !bodyText?.length) { + // If there is no request body in the schema and no body in the request, then the request is valid + return undefined; + } else if (disabled && bodyText?.length) { + throw new OperationError( + this.operation, + 'Received a body but no schema was found', + ); + } + + const contentType = + request.headers.get('content-type') || 'application/json'; + if (contentType !== 'application/json') { + throw new OperationError( + this.operation, + 'Content type is not application/json', + ); + } + const body = (await request.json()) as JsonObject; + const valid = fn!(body); + if (!valid) { + console.log(body); + console.error(fn!.errors); + throw new OperationError( + this.operation, + `Request body validation failed.`, + ); + } + return body; + } +} diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.ts new file mode 100644 index 0000000000..d947a21970 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.ts @@ -0,0 +1,102 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonObject } from '@backstage/types'; +import { Operation, ParserOptions, ResponseParser } from './types'; +import { OperationError } from './errors'; +import Ajv from 'ajv'; +import { OperationObject, ResponseObject } from 'openapi3-ts'; + +export class ResponseBodyParser + implements ResponseParser +{ + operation: Operation; + ajv: Ajv; + constructor(operation: Operation, options: ParserOptions) { + this.operation = operation; + this.ajv = options.ajv; + const responseSchemas = operation.schema.responses; + if (!Object.keys(responseSchemas).length) { + throw new OperationError(this.operation, `No response schemas found`); + } + for (const [statusCode, schema] of Object.entries(responseSchemas)) { + if (!schema.content) { + continue; + } else if (!schema.content['application/json']) { + throw new OperationError( + this.operation, + `No application/json content type found in response for status code ${statusCode}`, + ); + } else if ('$ref' in schema.content['application/json'].schema) { + throw new OperationError( + this.operation, + 'Reference objects are not supported', + ); + } + } + } + + async parse(response: Response): Promise { + const body = await response.text(); + const responseSchema = this.findResponseSchema( + this.operation.schema, + response, + ); + if (!responseSchema?.content && body?.length) { + throw new OperationError(this.operation, 'No content found in response'); + } else if (!responseSchema?.content && !body?.length) { + // If there is no content in the response schema and no body in the response, then the response is valid + return undefined; + } + if (!responseSchema?.content!['application/json']) { + throw new OperationError( + this.operation, + 'No application/json content type found in response', + ); + } + const schema = responseSchema.content!['application/json'].schema; + if (!schema) { + throw new OperationError(this.operation, 'No schema found in response'); + } + if ('$ref' in schema) { + throw new OperationError( + this.operation, + 'Reference objects are not supported', + ); + } + + const validate = this.ajv.compile(schema); + const jsonBody = (await response.json()) as JsonObject; + const valid = validate(jsonBody); + if (!valid) { + throw new OperationError( + this.operation, + 'Response body validation failed', + ); + } + return jsonBody; + } + + private findResponseSchema( + operationSchema: OperationObject, + response: Response, + ): ResponseObject | undefined { + const { status } = response; + return ( + operationSchema.responses?.[status] ?? operationSchema.responses?.default + ); + } +} diff --git a/packages/backend-openapi-utils/src/schema/types.ts b/packages/backend-openapi-utils/src/schema/types.ts new file mode 100644 index 0000000000..7de1e1c57e --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/types.ts @@ -0,0 +1,50 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { CompletedRequest, CompletedResponse } from 'mockttp'; +import { OperationObject } from 'openapi3-ts'; +import Ajv from 'ajv'; + +export interface RequestParser { + parse(request: Request): Promise; +} +export interface ResponseParser { + parse(response: Response): Promise; +} + +export interface ParserOptions { + ajv: Ajv; +} + +export interface Operation { + schema: OperationObject; + path: string; + method: string; +} + +export interface RequestResponsePair { + request: CompletedRequest; + response: CompletedResponse; +} + +export interface ValidatorParams { + pair: RequestResponsePair; + operation: Operation; +} + +export interface Validator { + validate(pair: ValidatorParams): Promise; +} diff --git a/packages/backend-openapi-utils/src/schema/utils.ts b/packages/backend-openapi-utils/src/schema/utils.ts new file mode 100644 index 0000000000..cc00d00095 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/utils.ts @@ -0,0 +1,36 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { CompletedRequest, CompletedResponse } from 'mockttp'; + +export function mockttpToFetchRequest(request: CompletedRequest) { + const headers = new Headers(request.rawHeaders); + return { + url: request.url, + method: request.method, + headers, + json: () => request.body.getJson(), + text: () => request.body.getText(), + } as Request; +} +export function mockttpToFetchResponse(response: CompletedResponse) { + const headers = new Headers(response.rawHeaders); + return { + status: response.statusCode, + headers, + json: () => response.body?.getJson(), + text: () => response.body?.getText(), + } as Response; +} diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts new file mode 100644 index 0000000000..1c440fa7b5 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -0,0 +1,711 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { OpenApiProxyValidator } from './validation'; +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; +import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { CompletedBody, CompletedRequest, CompletedResponse } from 'mockttp'; +import withResponseBody from './__fixtures__/schemas/withJsonResponseBody.json'; +import withQueryParameter from './__fixtures__/schemas/withQueryParameter.json'; +import _ from 'lodash'; +import { ParameterObject } from 'openapi3-ts'; + +const server = setupServer(); + +function createMockttpRequest(request: { + method: string; + url: string; + headers?: Record; + body?: object; +}): CompletedRequest { + return { + method: request.method, + url: `http://localhost:8080${request.url}`, + headers: { 'content-type': 'application/json', ...request.headers }, + body: { + getText: async () => JSON.stringify(request.body), + getJson: async () => request.body, + } as CompletedBody, + } as CompletedRequest; +} + +function createMockttpResponse(response: { + statusCode: number; + headers?: Record; + body?: object; +}): CompletedResponse { + return { + statusCode: response.statusCode, + headers: response.headers, + body: response.body + ? ({ + getText: async () => JSON.stringify(response.body), + getJson: async () => response.body, + } as CompletedBody) + : undefined, + } as CompletedResponse; +} + +describe('OpenApiProxyValidator', () => { + setupRequestMockHandlers(server); + let validator: OpenApiProxyValidator; + + async function mockSchema(schema: any) { + server.use( + rest.get('http://localhost:7000/openapi.json', (_req, res, ctx) => + res(ctx.json(schema)), + ), + ); + await validator.initialize('http://localhost:7000/openapi.json'); + } + + beforeEach(async () => { + validator = new OpenApiProxyValidator(); + }); + + describe('request body', () => { + it('validates a JSON request body', async () => { + await mockSchema(withResponseBody); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + headers: { + 'content-type': 'application/json', + }, + body: { results: [] }, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for missing request body per schema', async () => { + await mockSchema(withResponseBody); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + body: { id: '123' }, + }); + const response = createMockttpResponse({ + statusCode: 200, + headers: { + 'content-type': 'application/json', + }, + body: { results: [] }, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Received a body but no schema was found]`, + ); + }); + }); + + describe('query parameters', () => { + describe('primitives', () => { + describe('string', () => { + it('accepts valid parameter', async () => { + await mockSchema(withQueryParameter); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=abc', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + }); + describe('number', () => { + const schema = _.cloneDeep(withQueryParameter); + schema.paths['/api/search'].get.parameters[0].schema.type = 'number'; + it('throws for a missing required parameter', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?test=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Unexpected query parameters: test]`, + ); + }); + + it('throws for invalid parameter', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=abc', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Query parameter validation failed]`, + ); + }); + + it('accepts valid parameter', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + }); + }); + + describe('object', () => { + describe('deepObject', () => { + const schema = _.cloneDeep(withQueryParameter); + schema.paths['/api/search'].get.parameters[0].schema.type = 'object'; + ( + schema.paths['/api/search'].get.parameters[0] as ParameterObject + ).style = 'deepObject'; + it('throws for invalid parameter (not an object)', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Unexpected query parameters: param]`, + ); + }); + + it('throws for missing required property', async () => { + const requiredSchema = _.cloneDeep(schema); + requiredSchema.paths['/api/search'].get.parameters[0].required = true; + await mockSchema(requiredSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ); + }); + + it('throws for invalid format', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param[t=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Invalid object parameter]`, + ); + }); + }); + + describe('form', () => { + const schema = _.cloneDeep(withQueryParameter); + schema.paths['/api/search'].get.parameters[0].schema.type = 'object'; + ( + schema.paths['/api/search'].get.parameters[0] as ParameterObject + ).style = 'form'; + it('throws for invalid parameter (not an object)', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param[t=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Unexpected query parameters: param[t]`, + ); + }); + + it('throws for missing required property', async () => { + const requiredSchema = _.cloneDeep(schema); + requiredSchema.paths['/api/search'].get.parameters[0].required = true; + await mockSchema(requiredSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ); + }); + + it('throws for invalid format', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Invalid object parameter]`, + ); + }); + describe('explode', () => { + const explodeSchema = _.cloneDeep(schema); + ( + explodeSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).explode = true; + it('accepts valid parameter', async () => { + await mockSchema(explodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123,test,456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('accepts multiple parameters', async () => { + await mockSchema(explodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?test=123&myparam=test&otherparam=456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + }); + + describe('no explode', () => { + const noExplodeSchema = _.cloneDeep(schema); + ( + noExplodeSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).explode = false; + it('accepts valid parameter', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123,test,456,param', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for invalid parameter', async () => { + await mockSchema(schema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123,test,456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"[/api/search (GET)]: Invalid object parameter"`, + ); + }); + }); + }); + }); + + describe('array', () => { + const arraySchema = _.cloneDeep(withQueryParameter); + arraySchema.paths['/api/search'].get.parameters[0].schema.type = 'array'; + describe('form', () => { + describe('explode', () => { + const explodeSchema = _.cloneDeep(arraySchema); + ( + explodeSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).explode = true; + + it('accepts single parameter', async () => { + await mockSchema(explodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('accepts multiple parameters', async () => { + await mockSchema(explodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123¶m=test¶m=456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for missing required parameter', async () => { + const requiredSchema = _.cloneDeep(explodeSchema); + requiredSchema.paths['/api/search'].get.parameters[0].required = + true; + await mockSchema(requiredSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ); + }); + + it('throws for invalid parameter', async () => { + await mockSchema(explodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param[]=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Unexpected query parameters: param[]]`, + ); + }); + }); + + describe('no explode', () => { + const noExplodeSchema = _.cloneDeep(arraySchema); + ( + noExplodeSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).explode = false; + + it('accepts single parameter', async () => { + await mockSchema(noExplodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('accepts multiple parameters', async () => { + await mockSchema(noExplodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123,456,789', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for missing required parameter', async () => { + const requiredSchema = _.cloneDeep(noExplodeSchema); + requiredSchema.paths['/api/search'].get.parameters[0].required = + true; + await mockSchema(requiredSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ); + }); + + it('throws for invalid parameter', async () => { + await mockSchema(noExplodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123¶m=456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Array parameter should not have multiple values]`, + ); + }); + }); + + describe('compatible with qs', () => { + const noExplodeSchema = _.cloneDeep(arraySchema); + ( + noExplodeSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).explode = false; + + const explodeSchema = _.cloneDeep(arraySchema); + ( + explodeSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).explode = true; + it('accepts the [] syntax', async () => { + await mockSchema(noExplodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param[]=123,456,789', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('accepts the array index syntax', async () => { + await mockSchema(explodeSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param[0]=123¶m[1]=456¶m[2]=789', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + }); + }); + + describe('spaceDelimited', () => { + const spaceDelimitedSchema = _.cloneDeep(arraySchema); + ( + spaceDelimitedSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).style = 'spaceDelimited'; + + it('accepts single parameter', async () => { + await mockSchema(spaceDelimitedSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('accepts multiple parameters', async () => { + await mockSchema(spaceDelimitedSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123 test 456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for missing required parameter', async () => { + const requiredSchema = _.cloneDeep(spaceDelimitedSchema); + requiredSchema.paths['/api/search'].get.parameters[0].required = true; + await mockSchema(requiredSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ); + }); + }); + + describe('pipeDelimited', () => { + const pipeDelimitedSchema = _.cloneDeep(arraySchema); + ( + pipeDelimitedSchema.paths['/api/search'].get + .parameters[0] as ParameterObject + ).style = 'pipeDelimited'; + + it('accepts single parameter', async () => { + await mockSchema(pipeDelimitedSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('accepts multiple parameters', async () => { + await mockSchema(pipeDelimitedSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search?param=123|test|456', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for missing required parameter', async () => { + const requiredSchema = _.cloneDeep(pipeDelimitedSchema); + requiredSchema.paths['/api/search'].get.parameters[0].required = true; + await mockSchema(requiredSchema); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ); + }); + }); + }); + }); + + describe('response body', () => { + it('validates a JSON response body', async () => { + await mockSchema(withResponseBody); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + headers: { + 'content-type': 'application/json', + }, + body: { results: [] }, + }); + + expect(await validator.validate(request, response)).toBeUndefined(); + }); + + it('throws for missing response body per schema', async () => { + await mockSchema(withResponseBody); + const request = createMockttpRequest({ + method: 'GET', + url: '/api/search', + }); + const response = createMockttpResponse({ + statusCode: 200, + headers: { + 'content-type': 'application/json', + }, + }); + + await expect( + async () => await validator.validate(request, response), + ).rejects.toMatchInlineSnapshot( + `[Error: [/api/search (GET)]: Response body validation failed]`, + ); + }); + }); +}); diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index 9f2f22b9ac..b5a96e050a 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -23,489 +23,40 @@ import { } from 'openapi3-ts'; import Ajv from 'ajv'; import Parser from '@apidevtools/swagger-parser'; +import { Operation, Validator, ValidatorParams } from './types'; +import { ParameterValidator } from './parameter-validation'; +import { OperationError } from './errors'; +import { RequestBodyParser } from './request-body-validation'; +import { mockttpToFetchRequest, mockttpToFetchResponse } from './utils'; +import { ResponseBodyParser } from './response-body-validation'; const ajv = new Ajv({ allErrors: true }); // options can be passed, e.g. {allErrors: true} -interface RequestResponsePair { - request: CompletedRequest; - response: CompletedResponse; -} - -interface ValidatorParams { - pair: RequestResponsePair; - operationSchema: OperationObject; - path: string; -} - -interface Validator { - validate(pair: ValidatorParams): Promise; -} - -class RequestErrorFactory { - static createRequestError(request: CompletedRequest, message: string): Error { - return new Error(`[${request.url} (${request.method})]: ${message}`); - } -} - -export class ParameterValidator implements Validator { - schema: OpenAPIObject; - cache: Record = {}; - constructor(schema: OpenAPIObject) { - this.schema = schema; - } - - async validate({ - pair: { request, response }, - operationSchema, - path, - }: ValidatorParams) { - if (response.statusCode === 400) { - // If the response is a 400, then the request is invalid and we shouldn't validate the parameters - return; - } - const parameters = operationSchema.parameters; - const queryParameters: Record = {}; - const headerParameters: Record = {}; - const pathParameters: Record = {}; - for (const parameter of parameters || []) { - if ('$ref' in parameter) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported', - ); - } - if (parameter.in === 'query') { - queryParameters[parameter.name] = parameter; - } - if (parameter.in === 'header') { - headerParameters[parameter.name] = parameter; - } - if (parameter.in === 'path') { - pathParameters[parameter.name] = parameter; - } - } - this.validateQueryParameters(queryParameters, request); - this.validateHeaderParameters(headerParameters, request); - this.validatePathParameters(pathParameters, request, path); - } - - validateQueryParameters( - queryParameters: Record, - request: CompletedRequest, - ) { - const { searchParams } = new URL(request.url); - for (const [name, parameter] of Object.entries(queryParameters)) { - if (!parameter.schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Schema not found for query parameter', - ); - } - if ('$ref' in parameter.schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported for parameters', - ); - } - let param: any | null = this.#findQueryParameters( - request, - queryParameters, - searchParams, - name, - ); - if (parameter.schema.type !== 'array' && Array.isArray(param)) { - param = param.length > 0 ? param[0] : undefined; - } - - if (!param && parameter.required) { - throw RequestErrorFactory.createRequestError( - request, - `Required query parameter ${name} not found`, - ); - } else if (!param && !parameter.required) { - continue; - } - if (parameter.schema.type === 'integer') { - // Try to parse the integer as AJV won't do it for us. - param = parseInt(param, 10); - } - const validate = ajv.compile(parameter.schema); - const valid = validate(param); - if (!valid) { - console.log(param); - console.error(validate.errors); - throw RequestErrorFactory.createRequestError( - request, - 'Query parameter validation failed', - ); - } - } - } - - #findQueryParameters( - request: CompletedRequest, - parameters: Record, - searchParams: URLSearchParams, - name: string, - ) { - const parameter = parameters[name]; - const schema = parameter.schema as SchemaObject; - if (schema.type === 'array') { - if (parameter.style === 'form' || !parameter.style) { - if (parameter.explode || typeof parameter.explode === 'undefined') { - if (!searchParams.has(name) && searchParams.has(`${name}[0]`)) { - const values: string[] = []; - let index = 0; - while (searchParams.has(`${name}[${index}]`)) { - values.push(searchParams.get(`${name}[${index}]`)!); - index++; - } - return values; - } - return searchParams.getAll(name); - } - if (!searchParams.has(name) && searchParams.has(`${name}[]`)) { - return searchParams.getAll(`${name}[]`); - } - return searchParams.get(name)?.split(','); - } else if (parameter.style === 'spaceDelimited') { - return searchParams.get(name)?.split(' '); - } else if (parameter.style === 'pipeDelimited') { - return searchParams.get(name)?.split('|'); - } - throw RequestErrorFactory.createRequestError( - request, - 'Unsupported style for array parameter', - ); - } - if (schema.type === 'object') { - if (parameter.style === 'form' || !parameter.style) { - if (parameter.explode) { - const obj: Record = {}; - for (const [key, value] of searchParams.entries()) { - if (this.#matchesOtherQueryParameters(parameters, key)) { - continue; - } - obj[key] = value; - } - console.log(obj); - return obj; - } - const obj: Record = {}; - const value = searchParams.get(name); - if (value) { - const parts = value.split(','); - if (parts.length % 2 !== 0) { - throw RequestErrorFactory.createRequestError( - request, - 'Invalid object parameter', - ); - } - for (let i = 0; i < parts.length; i += 2) { - obj[parts[i]] = parts[i + 1]; - } - } - return obj; - } else if (parameter.style === 'deepObject') { - const obj: Record = {}; - for (const [key, value] of searchParams.entries()) { - if (key.startsWith(`${name}[`)) { - const parts = key.split('['); - let currentLayer = obj; - for (let partIndex = 0; partIndex < parts.length - 1; partIndex++) { - const part = parts[partIndex]; - const objKey = part.split(']')[0]; - if (!currentLayer[objKey]) { - currentLayer[objKey] = {}; - } - currentLayer = currentLayer[objKey]; - } - currentLayer[parts[parts.length - 1].split(']')[0]] = value; - } - } - return obj; - } - throw RequestErrorFactory.createRequestError( - request, - 'Unsupported style for object parameter', - ); - } - // For everything else, just return the value. - return searchParams.getAll(name); - } - - #matchesOtherQueryParameters( - parameters: Record, - nameToMatch: string, - ) { - for (const [name] of Object.entries(parameters)) { - if (name === nameToMatch) { - return true; - } - } - return false; - } - - validateHeaderParameters( - headerParameters: Record, - request: CompletedRequest, - ) { - for (const [name, parameter] of Object.entries(headerParameters)) { - if (!request.headers[name]) { - throw RequestErrorFactory.createRequestError( - request, - `Header parameter ${name} not found`, - ); - } - if (!parameter.schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Schema not found for path parameter', - ); - } - if ('$ref' in parameter.schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported for parameters', - ); - } - const validate = ajv.compile(parameter.schema); - const valid = validate(request.headers[name]); - - if (!valid) { - console.log(request.headers[name]); - console.error(validate.errors); - throw RequestErrorFactory.createRequestError( - request, - 'Header parameter validation failed', - ); - } - } - } - - validatePathParameters( - pathParameters: Record, - request: CompletedRequest, - path: string, - ) { - const { pathname } = new URL(request.url); - const params = parsePath({ request, path: pathname, schema: path }); - for (const [name, parameter] of Object.entries(pathParameters)) { - if (!params[name] && parameter.required) { - throw RequestErrorFactory.createRequestError( - request, - `Path parameter ${name} not found`, - ); - } - if (!parameter.schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Schema not found for path parameter', - ); - } - if ('$ref' in parameter.schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported for parameters', - ); - } - - const validate = ajv.compile(parameter.schema); - const valid = validate(params[name]); - - if (!valid) { - console.log(params); - console.error(validate.errors); - throw RequestErrorFactory.createRequestError( - request, - 'Path parameter validation failed', - ); - } - } - } -} - -function parsePath({ - request, - schema, - path, -}: { - request: CompletedRequest; - schema: string; - path: string; -}) { - const parts = path.split('/'); - const pathParts = schema.split('/'); - if (parts.length !== pathParts.length) { - throw RequestErrorFactory.createRequestError( - request, - 'Path parts do not match', - ); - } - const params: Record = {}; - for (let i = 0; i < parts.length; i++) { - if (pathParts[i] === parts[i]) { - continue; - } - if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { - params[pathParts[i].slice(1, -1)] = parts[i]; - continue; - } - break; - } - return params; -} - -export class RequestBodyValidator implements Validator { +class RequestBodyValidator implements Validator { schema: OpenAPIObject; constructor(schema: OpenAPIObject) { this.schema = schema; } - async validate({ - pair: { request, response }, - operationSchema, - }: ValidatorParams) { - if (response.statusCode === 400) { - // If the response is a 400, then the request is invalid and we shouldn't validate the request body - return; - } - const requestBody = operationSchema.requestBody; - const bodyText = await request.body.getText(); - if (!requestBody && bodyText?.length) { - throw RequestErrorFactory.createRequestError( - request, - `No request body found for ${request.url}`, - ); - } else if (!requestBody && !bodyText?.length) { - // If there is no request body in the schema and no body in the request, then the request is valid - return; - } - if ('$ref' in requestBody!) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported', - ); - } - if (!requestBody!.content) { - throw RequestErrorFactory.createRequestError( - request, - 'No content found in request body', - ); - } - if (!requestBody!.content['application/json']) { - throw RequestErrorFactory.createRequestError( - request, - 'No application/json content type found in request body', - ); - } - const contentType = request.headers['content-type']; - if (!contentType) { - throw RequestErrorFactory.createRequestError( - request, - 'Content type not found in request', - ); - } - if (contentType !== 'application/json') { - throw RequestErrorFactory.createRequestError( - request, - 'Content type is not application/json', - ); - } - const schema = requestBody!.content['application/json'].schema; - if (!schema) { - throw RequestErrorFactory.createRequestError( - request, - 'No schema found in request body', - ); - } - if ('$ref' in schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported', - ); - } - - const validate = ajv.compile(schema); - const body = await request.body.getJson(); - const valid = validate(body); - if (!valid) { - console.log(body); - console.error(validate.errors); - throw RequestErrorFactory.createRequestError( - request, - `Request body validation failed.`, - ); - } + async validate({ pair, operation }: ValidatorParams) { + const { request } = pair; + const parser = new RequestBodyParser(operation, { ajv }); + const fetchRequest = mockttpToFetchRequest(request); + await parser.parse(fetchRequest); } } -export class ResponseBodyValidator implements Validator { +class ResponseBodyValidator implements Validator { schema: OpenAPIObject; constructor(schema: OpenAPIObject) { this.schema = schema; } - async validate(pair: ValidatorParams) { - const { - pair: { response, request }, - operationSchema, - } = pair; - const responseSchema = this.findResponseSchema(operationSchema, response); - if (!responseSchema) { - throw RequestErrorFactory.createRequestError( - request, - `No response schema found for ${response.statusCode}`, - ); - } - const body = await response.body.getText(); - if (!responseSchema.content && body?.length) { - throw RequestErrorFactory.createRequestError( - request, - 'No content found in response', - ); - } else if (!responseSchema.content && !body?.length) { - // If there is no content in the response schema and no body in the response, then the response is valid - return; - } - if (!responseSchema.content!['application/json']) { - throw RequestErrorFactory.createRequestError( - request, - 'No application/json content type found in response', - ); - } - const schema = responseSchema.content!['application/json'].schema; - if (!schema) { - throw RequestErrorFactory.createRequestError( - request, - 'No schema found in response', - ); - } - if ('$ref' in schema) { - throw RequestErrorFactory.createRequestError( - request, - 'Reference objects are not supported', - ); - } - - const validate = ajv.compile(schema); - const valid = validate(await response.body.getJson()); - if (!valid) { - console.log(await response.body.getJson()); - console.error(validate.errors); - throw RequestErrorFactory.createRequestError( - request, - 'Response body validation failed', - ); - } - } - - private findResponseSchema( - operationSchema: OperationObject, - response: CompletedResponse, - ): ResponseObject | undefined { - const { statusCode } = response; - return operationSchema.responses?.[statusCode]; + async validate({ pair, operation }: ValidatorParams) { + const { response } = pair; + const parser = new ResponseBodyParser(operation, { ajv }); + const fetchResponse = mockttpToFetchResponse(response); + await parser.parse(fetchResponse); } } @@ -518,28 +69,28 @@ export class OpenApiProxyValidator { this.validators = [ new ParameterValidator(this.schema), new RequestBodyValidator(this.schema), - // new ResponseBodyValidator(this.schema), + new ResponseBodyValidator(this.schema), ]; } async validate(request: CompletedRequest, response: CompletedResponse) { - const operation = this.findOperation(request); - if (!operation) { - throw RequestErrorFactory.createRequestError( - request, + const operationPathTuple = this.findOperation(request); + if (!operationPathTuple) { + throw new OperationError( + { path: request.path, method: request.method } as Operation, `No operation schema found for ${request.url}`, ); } - const [path, operationSchema] = operation; + const [path, operationSchema] = operationPathTuple; + const operation = { path, method: request.method, schema: operationSchema }; const validators = this.validators!; await Promise.all( validators.map(validator => validator.validate({ pair: { request, response }, - operationSchema, - path, + operation, }), ), ); diff --git a/yarn.lock b/yarn.lock index 036ac5c852..bcc6a951fa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3740,8 +3740,10 @@ __metadata: dependencies: "@apidevtools/swagger-parser": ^10.1.0 "@backstage/backend-plugin-api": "workspace:^" + "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/errors": "workspace:^" + "@backstage/types": "workspace:^" "@types/express": ^4.17.6 "@types/express-serve-static-core": ^4.17.5 ajv: ^8.16.0 @@ -3751,6 +3753,7 @@ __metadata: json-schema-to-ts: ^3.0.0 lodash: ^4.17.21 mockttp: ^3.13.0 + msw: ^1.0.0 openapi-merge: ^1.3.2 openapi3-ts: ^3.1.2 supertest: ^7.0.0 From ea0b6b42628c71a5b5eb4321006d4966635432bd Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 14:50:12 -0400 Subject: [PATCH 04/23] adding test for request and response validation and adjusting error messages Signed-off-by: aramissennyeydd --- .../schemas/withJsonRequestBody.json | 29 +++++ .../schemas/withJsonResponseBody.json | 3 +- .../schemas/withPathParameter.json | 25 +++++ .../src/schema/errors.ts | 16 ++- .../src/schema/parameter-validation.test.ts | 101 +++++++++++++++--- .../src/schema/parameter-validation.ts | 49 ++++++--- .../schema/request-body-validation.test.ts | 82 ++++++++++++++ .../src/schema/request-body-validation.ts | 65 ++++++----- .../schema/response-body-validation.test.ts | 81 ++++++++++++++ .../src/schema/response-body-validation.ts | 76 ++++++++++--- .../src/schema/validation.test.ts | 66 ++++++------ .../src/schema/validation.ts | 4 +- 12 files changed, 493 insertions(+), 104 deletions(-) create mode 100644 packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonRequestBody.json create mode 100644 packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withPathParameter.json create mode 100644 packages/backend-openapi-utils/src/schema/request-body-validation.test.ts create mode 100644 packages/backend-openapi-utils/src/schema/response-body-validation.test.ts diff --git a/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonRequestBody.json b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonRequestBody.json new file mode 100644 index 0000000000..c5eda7ebb4 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonRequestBody.json @@ -0,0 +1,29 @@ +{ + "openapi": "3.0.0", + "info": { "title": "Test", "version": "1.0.0" }, + "paths": { + "/api/search": { + "post": { + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "query": { + "type": "string" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "OK" + } + } + } + } + } +} diff --git a/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json index 378ea0a168..6243a640b9 100644 --- a/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json +++ b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withJsonResponseBody.json @@ -23,7 +23,8 @@ } } } - } + }, + "additionalProperties": false } } } diff --git a/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withPathParameter.json b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withPathParameter.json new file mode 100644 index 0000000000..98e25d0975 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/__fixtures__/schemas/withPathParameter.json @@ -0,0 +1,25 @@ +{ + "openapi": "3.0.0", + "info": { "title": "Test", "version": "1.0.0" }, + "paths": { + "/api/item/{id}": { + "get": { + "responses": { + "200": { + "description": "OK" + } + }, + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ] + } + } + } +} diff --git a/packages/backend-openapi-utils/src/schema/errors.ts b/packages/backend-openapi-utils/src/schema/errors.ts index aab5210f46..44de054c53 100644 --- a/packages/backend-openapi-utils/src/schema/errors.ts +++ b/packages/backend-openapi-utils/src/schema/errors.ts @@ -18,6 +18,20 @@ import { Operation } from './types'; export class OperationError extends Error { constructor(operation: Operation, message: string) { - super(`[${operation.path} (${operation.method})]: ${message}`); + super( + `["${operation.method.toLocaleUpperCase('en-US')} ${ + operation.path + }"] ${message}`, + ); + } +} + +export class OperationResponseError extends Error { + constructor(operation: Operation, response: Response, message: string) { + super( + `["${operation.method.toLocaleUpperCase('en-US')} ${operation.path}" (${ + response.status + })]: ${message}`, + ); } } diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts index 2fb8f371b1..673f22f99b 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts @@ -16,7 +16,11 @@ import _ from 'lodash'; import withQueryParameter from './__fixtures__/schemas/withQueryParameter.json'; -import { QueryParameterParser } from './parameter-validation'; +import withPathParameter from './__fixtures__/schemas/withPathParameter.json'; +import { + PathParameterParser, + QueryParameterParser, +} from './parameter-validation'; import { OperationObject, ParameterObject } from 'openapi3-ts'; import Ajv from 'ajv'; import { Operation } from './types'; @@ -54,7 +58,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); @@ -66,7 +70,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); }); @@ -90,7 +94,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Query parameter validation failed"`, + `"["GET /api/search"] Query parameter validation failed"`, ); }); }); @@ -132,7 +136,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -143,7 +147,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); }); @@ -177,7 +181,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -188,7 +192,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); }); @@ -215,7 +219,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -226,7 +230,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); }); @@ -252,7 +256,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -263,7 +267,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); }); @@ -304,7 +308,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -361,7 +365,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -372,7 +376,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); }); @@ -416,7 +420,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Required query parameter param not found"`, + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -427,7 +431,7 @@ describe('query parameters', () => { await expect( parser.parse(request), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (get)]: Unexpected query parameters: extra"`, + `"["GET /api/search"] Unexpected query parameters: extra"`, ); }); @@ -456,3 +460,66 @@ describe('query parameters', () => { }); }); }); + +describe('path parameters', () => { + let operation: Operation; + let parser: PathParameterParser; + let schema: (typeof withPathParameter)['paths']['/api/item/{id}']['get']; + + beforeEach(() => { + schema = _.cloneDeep(withPathParameter.paths['/api/item/{id}'].get); + operation = { + schema: schema as OperationObject, + path: '/api/item/{id}', + method: 'get', + }; + parser = new PathParameterParser(operation, { ajv }); + }); + describe('primitives', () => { + describe('string', () => { + it('should parse a string', async () => { + const request = { + url: 'http://localhost:8080/api/item/test', + } as Request; + const result = await parser.parse(request); + expect(result.id).toBe('test'); + }); + + it('should throw an error if the parameter is required but missing', async () => { + (schema.parameters![0] as ParameterObject).required = true; + const request = { + url: 'http://localhost:8080/api/item', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/item/{id}"] Path parts do not match"`, + ); + }); + }); + + describe('number', () => { + beforeEach(() => { + schema.parameters![0].schema.type = 'number'; + }); + it('should parse a number', async () => { + const request = { + url: 'http://localhost:8080/api/item/42', + } as Request; + const result = await parser.parse(request); + expect(result.id).toBe(42); + }); + + it('should throw an error if the parameter is not a number', async () => { + const request = { + url: 'http://localhost:8080/api/item/hello', + } as Request; + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/item/{id}"] Path parameter validation failed"`, + ); + }); + }); + }); +}); diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 15570e120c..ddc0efbe45 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -36,7 +36,11 @@ class BaseParameterParser { ajv: Ajv; operation: Operation; parameters: Record = {}; - constructor(operation: Operation, options: ParserOptions) { + constructor( + parameterIn: string, + operation: Operation, + options: ParserOptions, + ) { this.ajv = options.ajv; this.operation = operation; const { schema, path, method } = operation; @@ -60,18 +64,31 @@ class BaseParameterParser { 'Reference objects are not supported for parameters', ); } - if (parameter.in === 'query') { + if (parameter.in === parameterIn) { this.parameters[parameter.name] = parameter as ReferencelessParameterObject; } } } + + optimisticallyParseValue(value: string, schema: SchemaObject) { + if (schema.type === 'integer') { + return parseInt(value, 10); + } + if (schema.type === 'number') { + return parseFloat(value); + } + return value; + } } export class QueryParameterParser extends BaseParameterParser implements RequestParser> { + constructor(operation: Operation, options: ParserOptions) { + super('query', operation, options); + } async parse(request: Request) { const { searchParams } = new URL(request.url); const remainingQueryParameters = new Set(searchParams.keys()); @@ -127,13 +144,8 @@ export class QueryParameterParser } else if (!param && !parameter.required) { continue; } - if (parameter.schema.type === 'integer') { - // Try to parse the integer as AJV won't do it for us. - param = parseInt(param, 10); - } - if (parameter.schema.type === 'number') { - // Try to parse the number as AJV won't do it for us. - param = parseFloat(param); + if (param) { + param = this.optimisticallyParseValue(param, parameter.schema); } const validate = this.ajv.compile(parameter.schema); const valid = validate(param); @@ -295,6 +307,9 @@ export class HeaderParameterParser extends BaseParameterParser implements RequestParser> { + constructor(operation: Operation, options: ParserOptions) { + super('header', operation, options); + } async parse(request: Request) { const headerParameters: Record = {}; for (const [name, parameter] of Object.entries(this.parameters)) { @@ -339,15 +354,19 @@ export class PathParameterParser extends BaseParameterParser implements RequestParser> { + constructor(operation: Operation, options: ParserOptions) { + super('path', operation, options); + } async parse(request: Request) { const { pathname } = new URL(request.url); const params = this.parsePath({ path: pathname, schema: this.operation.path, }); - const pathParameters: Record = {}; + const pathParameters: Record = {}; for (const [name, parameter] of Object.entries(this.parameters)) { - if (!params[name] && parameter.required) { + let param: string | number = params[name]; + if (!param && parameter.required) { throw new OperationError( this.operation, `Path parameter ${name} not found`, @@ -356,8 +375,12 @@ export class PathParameterParser continue; } + if (param) { + param = this.optimisticallyParseValue(param, parameter.schema); + } + const validate = this.ajv.compile(parameter.schema); - const valid = validate(params[name]); + const valid = validate(param); if (!valid) { throw new OperationError( @@ -365,7 +388,7 @@ export class PathParameterParser 'Path parameter validation failed', ); } - pathParameters[name] = params[name]; + pathParameters[name] = param; } return pathParameters; } diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts new file mode 100644 index 0000000000..a4452763fc --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import withJsonRequestBody from './__fixtures__/schemas/withJsonRequestBody.json'; +import { RequestBodyParser } from './request-body-validation'; +import Ajv from 'ajv'; +import { Operation, RequestParser } from './types'; +import _ from 'lodash'; +import { OperationObject, RequestBodyObject } from 'openapi3-ts'; +import { JsonObject } from '@backstage/types'; + +const ajv = new Ajv(); + +function toRequest(body?: object, headers?: Record): Request { + return { + text: async () => JSON.stringify(body), + json: async () => body, + url: '/api/search', + method: 'post', + headers: new Headers({ 'content-type': 'application/json', ...headers }), + } as Request; +} + +describe('request body', () => { + let operation: Operation; + let parser: RequestParser; + let schema: (typeof withJsonRequestBody)['paths']['/api/search']['post']; + beforeEach(() => { + schema = _.cloneDeep(withJsonRequestBody.paths['/api/search'].post); + operation = { + method: 'post', + schema: schema as OperationObject, + path: '/api/search', + }; + parser = new RequestBodyParser(operation, { + ajv, + }); + }); + it('should validate request body', async () => { + const requestBody = { + query: 'test', + }; + const result = await parser.parse(toRequest(requestBody)); + expect(result).toEqual(requestBody); + }); + + it('should throw error if request body is not valid', async () => { + const requestBody = { + query: 1, + }; + await expect( + parser.parse(toRequest(requestBody)), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["POST /api/search"] Request body validation failed."`, + ); + }); + + it('should throw error if request body is required but missing', async () => { + (schema.requestBody as RequestBodyObject).required = true; + parser = RequestBodyParser.fromOperation(operation, { + ajv, + }); + await expect( + parser.parse(toRequest()), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["POST /api/search"] No request body found for /api/search"`, + ); + }); +}); diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.ts index f35b8f07c7..22873c8100 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.ts @@ -18,24 +18,51 @@ import { JsonObject } from '@backstage/types'; import { Operation, ParserOptions, RequestParser } from './types'; import { ValidateFunction } from 'ajv'; import { OperationError } from './errors'; +import { RequestBodyObject, SchemaObject } from 'openapi3-ts'; +class DisabledRequestBodyParser + implements RequestParser +{ + operation: Operation; + constructor(operation: Operation) { + this.operation = operation; + } + async parse(request: Request): Promise { + const bodyText = await request.text(); + if (bodyText?.length) { + throw new OperationError( + this.operation, + 'Received a body but no schema was found', + ); + } + return undefined; + } +} export class RequestBodyParser implements RequestParser { operation: Operation; - validate: - | { fn: ValidateFunction; disabled: false } - | { - fn: undefined; - disabled: true; - }; + disabled: boolean = false; + validate!: ValidateFunction; + schema!: SchemaObject; + requestBodySchema!: RequestBodyObject; + + static fromOperation(operation: Operation, options: ParserOptions) { + return operation.schema.requestBody + ? new RequestBodyParser(operation, options) + : new DisabledRequestBodyParser(operation); + } + constructor(operation: Operation, options: ParserOptions) { this.operation = operation; const { schema: operationSchema } = this.operation; const requestBody = operationSchema.requestBody; + if (!requestBody) { - this.validate = { disabled: true, fn: undefined }; - return; + throw new OperationError( + this.operation, + 'No request body found in operation', + ); } if ('$ref' in requestBody!) { @@ -69,27 +96,17 @@ export class RequestBodyParser 'Reference objects are not supported', ); } - this.validate = { - disabled: false, - fn: options.ajv.compile(operation.schema), - }; + this.validate = options.ajv.compile(schema); + this.schema = schema; + this.requestBodySchema = requestBody; } async parse(request: Request): Promise { - const { disabled, fn } = this.validate; const bodyText = await request.text(); - if (!disabled && bodyText?.length) { + if (this.requestBodySchema.required && !bodyText?.length) { throw new OperationError( this.operation, `No request body found for ${request.url}`, ); - } else if (disabled && !bodyText?.length) { - // If there is no request body in the schema and no body in the request, then the request is valid - return undefined; - } else if (disabled && bodyText?.length) { - throw new OperationError( - this.operation, - 'Received a body but no schema was found', - ); } const contentType = @@ -101,10 +118,8 @@ export class RequestBodyParser ); } const body = (await request.json()) as JsonObject; - const valid = fn!(body); + const valid = this.validate(body); if (!valid) { - console.log(body); - console.error(fn!.errors); throw new OperationError( this.operation, `Request body validation failed.`, diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts new file mode 100644 index 0000000000..9ad47b8a19 --- /dev/null +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonObject } from '@backstage/types'; +import withJsonResponseBody from './__fixtures__/schemas/withJsonResponseBody.json'; +import { Operation, ResponseParser } from './types'; +import { ResponseBodyParser } from './response-body-validation'; +import Ajv from 'ajv'; +import { OperationObject, ResponsesObject } from 'openapi3-ts'; +import _ from 'lodash'; + +const ajv = new Ajv(); + +function toResponse(body?: object): Response { + return { + json: async () => body, + text: async () => JSON.stringify(body), + status: 200, + } as Response; +} + +describe('response body', () => { + let operation: Operation; + let parser: ResponseParser; + let schema: (typeof withJsonResponseBody)['paths']['/api/search']['get']; + beforeEach(() => { + schema = _.cloneDeep(withJsonResponseBody.paths['/api/search'].get); + operation = { + path: '/api/search', + method: 'get', + schema: schema as OperationObject, + }; + parser = ResponseBodyParser.fromOperation(operation, { + ajv, + }); + }); + + it('should validate response body', async () => { + const responseBody = { + results: [{ id: 'test' }], + }; + const result = await parser.parse(toResponse(responseBody)); + expect(result).toEqual(responseBody); + }); + + it('should throw error if response body is not valid', async () => { + const responseBody = { + result: 1, + }; + await expect( + parser.parse(toResponse(responseBody)), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search" (200)]: Response body validation failed"`, + ); + }); + + it('should throw error if response body is required but missing', async () => { + (schema.responses as ResponsesObject)['200'].required = true; + parser = ResponseBodyParser.fromOperation(operation, { + ajv, + }); + await expect( + parser.parse(toResponse()), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search" (200)]: Response body is required but missing"`, + ); + }); +}); diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.ts index d947a21970..116fc888e8 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.ts @@ -16,22 +16,46 @@ import { JsonObject } from '@backstage/types'; import { Operation, ParserOptions, ResponseParser } from './types'; -import { OperationError } from './errors'; +import { OperationError, OperationResponseError } from './errors'; import Ajv from 'ajv'; import { OperationObject, ResponseObject } from 'openapi3-ts'; +class DisabledResponseBodyParser + implements ResponseParser +{ + operation: Operation; + constructor(operation: Operation) { + this.operation = operation; + } + async parse(response: Response): Promise { + const body = await response.text(); + if (body?.length) { + throw new OperationError( + this.operation, + 'Received a body but no schema was found', + ); + } + return undefined; + } +} + export class ResponseBodyParser implements ResponseParser { operation: Operation; ajv: Ajv; + + static fromOperation(operation: Operation, options: ParserOptions) { + return operation.schema.responses && + Object.keys(operation.schema.responses).length + ? new ResponseBodyParser(operation, options) + : new DisabledResponseBodyParser(operation); + } + constructor(operation: Operation, options: ParserOptions) { this.operation = operation; this.ajv = options.ajv; const responseSchemas = operation.schema.responses; - if (!Object.keys(responseSchemas).length) { - throw new OperationError(this.operation, `No response schemas found`); - } for (const [statusCode, schema] of Object.entries(responseSchemas)) { if (!schema.content) { continue; @@ -55,15 +79,30 @@ export class ResponseBodyParser this.operation.schema, response, ); - if (!responseSchema?.content && body?.length) { - throw new OperationError(this.operation, 'No content found in response'); - } else if (!responseSchema?.content && !body?.length) { - // If there is no content in the response schema and no body in the response, then the response is valid + if (!responseSchema?.content && !body?.length) { + // If there is no content in the response schema and no body in the response, then the response is valid. + // eg 204 No Content return undefined; } - if (!responseSchema?.content!['application/json']) { - throw new OperationError( + if (!responseSchema) { + throw new OperationResponseError( this.operation, + response, + `No schema found.`, + ); + } + + if (!responseSchema?.content && body?.length) { + throw new OperationResponseError( + this.operation, + response, + 'Received a body but no schema was found', + ); + } + if (!responseSchema?.content!['application/json']) { + throw new OperationResponseError( + this.operation, + response, 'No application/json content type found in response', ); } @@ -72,18 +111,31 @@ export class ResponseBodyParser throw new OperationError(this.operation, 'No schema found in response'); } if ('$ref' in schema) { - throw new OperationError( + throw new OperationResponseError( this.operation, + response, 'Reference objects are not supported', ); } + if (!schema.required && !body?.length) { + throw new OperationResponseError( + this.operation, + response, + 'Response body is required but missing', + ); + } else if (!schema.required && !body?.length) { + // If there is no content in the response schema and no body in the response, then the response is valid + return undefined; + } + const validate = this.ajv.compile(schema); const jsonBody = (await response.json()) as JsonObject; const valid = validate(jsonBody); if (!valid) { - throw new OperationError( + throw new OperationResponseError( this.operation, + response, 'Response body validation failed', ); } diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index 1c440fa7b5..0fab287f1e 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -112,8 +112,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Received a body but no schema was found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Received a body but no schema was found"`, ); }); }); @@ -149,8 +149,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Unexpected query parameters: test]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Unexpected query parameters: test"`, ); }); @@ -166,8 +166,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Query parameter validation failed]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Query parameter validation failed"`, ); }); @@ -205,8 +205,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Unexpected query parameters: param]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Unexpected query parameters: param"`, ); }); @@ -224,8 +224,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -241,8 +241,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Invalid object parameter]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Invalid object parameter"`, ); }); }); @@ -265,8 +265,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Unexpected query parameters: param[t]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Unexpected query parameters: param[t"`, ); }); @@ -284,8 +284,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -300,8 +300,8 @@ describe('OpenApiProxyValidator', () => { }); await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Invalid object parameter]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Invalid object parameter"`, ); }); describe('explode', () => { @@ -369,7 +369,7 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"[/api/search (GET)]: Invalid object parameter"`, + `"["GET /api/search"] Invalid object parameter"`, ); }); }); @@ -428,8 +428,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -445,8 +445,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Unexpected query parameters: param[]]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Unexpected query parameters: param[]"`, ); }); }); @@ -499,8 +499,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Required query parameter param not found"`, ); }); @@ -516,8 +516,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Array parameter should not have multiple values]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Array parameter should not have multiple values"`, ); }); }); @@ -609,8 +609,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Required query parameter param not found"`, ); }); }); @@ -662,8 +662,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Required query parameter param not found]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Required query parameter param not found"`, ); }); }); @@ -703,8 +703,8 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), - ).rejects.toMatchInlineSnapshot( - `[Error: [/api/search (GET)]: Response body validation failed]`, + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search" (200)]: Response body is required but missing"`, ); }); }); diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index b5a96e050a..45255a2ea5 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -40,7 +40,7 @@ class RequestBodyValidator implements Validator { async validate({ pair, operation }: ValidatorParams) { const { request } = pair; - const parser = new RequestBodyParser(operation, { ajv }); + const parser = RequestBodyParser.fromOperation(operation, { ajv }); const fetchRequest = mockttpToFetchRequest(request); await parser.parse(fetchRequest); } @@ -54,7 +54,7 @@ class ResponseBodyValidator implements Validator { async validate({ pair, operation }: ValidatorParams) { const { response } = pair; - const parser = new ResponseBodyParser(operation, { ajv }); + const parser = ResponseBodyParser.fromOperation(operation, { ajv }); const fetchResponse = mockttpToFetchResponse(response); await parser.parse(fetchResponse); } From 71ee97ddba4fd1154a8ed7ed24a5086f6ea3603a Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 15:01:07 -0400 Subject: [PATCH 05/23] add tests for path calculations Signed-off-by: aramissennyeydd --- .../src/schema/parameter-validation.test.ts | 44 +++++++ .../src/schema/parameter-validation.ts | 15 ++- .../src/schema/validation.test.ts | 116 +++++++++++++++++- .../src/schema/validation.ts | 87 ++++++------- 4 files changed, 214 insertions(+), 48 deletions(-) diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts index 673f22f99b..717256e266 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts @@ -522,4 +522,48 @@ describe('path parameters', () => { }); }); }); + + describe('path parsing', () => { + it('should parse a path with a single parameters', async () => { + const parsedPath = PathParameterParser.parsePath({ + operation, + schema: '/api/item/{id}', + path: '/api/item/test123', + }); + expect(parsedPath).toEqual({ id: 'test123' }); + }); + it('should parse a path with multiple parameters', async () => { + const parsedPath = PathParameterParser.parsePath({ + operation, + schema: '/api/item/{id}/{name}', + path: '/api/item/42/test', + }); + // the string is expected here, but will be optimistically parsed as a number where it makes sense. + expect(parsedPath).toEqual({ id: '42', name: 'test' }); + }); + + it('should throw an error if the path does not have enough parts', async () => { + expect(() => + PathParameterParser.parsePath({ + operation, + schema: '/api/item/{id}', + path: '/api/item', + }), + ).toThrowErrorMatchingInlineSnapshot( + `"["GET /api/item/{id}"] Path parts do not match"`, + ); + }); + + it('should throw an error if the path has too many parts', async () => { + expect(() => + PathParameterParser.parsePath({ + operation, + schema: '/api/item/{id}', + path: '/api/item/test/123', + }), + ).toThrowErrorMatchingInlineSnapshot( + `"["GET /api/item/{id}"] Path parts do not match"`, + ); + }); + }); }); diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index ddc0efbe45..65e6439b7e 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -359,7 +359,8 @@ export class PathParameterParser } async parse(request: Request) { const { pathname } = new URL(request.url); - const params = this.parsePath({ + const params = PathParameterParser.parsePath({ + operation: this.operation, path: pathname, schema: this.operation.path, }); @@ -393,11 +394,19 @@ export class PathParameterParser return pathParameters; } - parsePath({ schema, path }: { schema: string; path: string }) { + static parsePath({ + operation, + schema, + path, + }: { + operation: Operation; + schema: string; + path: string; + }) { const parts = path.split('/'); const pathParts = schema.split('/'); if (parts.length !== pathParts.length) { - throw new OperationError(this.operation, 'Path parts do not match'); + throw new OperationError(operation, 'Path parts do not match'); } const params: Record = {}; for (let i = 0; i < parts.length; i++) { diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index 0fab287f1e..3f172c16fe 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { OpenApiProxyValidator } from './validation'; +import { findOperationByRequest, OpenApiProxyValidator } from './validation'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; @@ -22,7 +22,7 @@ import { CompletedBody, CompletedRequest, CompletedResponse } from 'mockttp'; import withResponseBody from './__fixtures__/schemas/withJsonResponseBody.json'; import withQueryParameter from './__fixtures__/schemas/withQueryParameter.json'; import _ from 'lodash'; -import { ParameterObject } from 'openapi3-ts'; +import { OpenAPIObject, ParameterObject } from 'openapi3-ts'; const server = setupServer(); @@ -709,3 +709,115 @@ describe('OpenApiProxyValidator', () => { }); }); }); + +describe('findOperationByRequest', () => { + it('finds an operation by request', () => { + const schema = { + openapi: '3.0.0', + info: { + title: 'Test', + version: '1.0.0', + }, + paths: { + '/api/search': { + get: { + parameters: [], + }, + }, + }, + }; + const request = { + method: 'GET', + url: 'http://localhost:8080/api/search', + } as CompletedRequest; + expect(findOperationByRequest(schema as OpenAPIObject, request)).toEqual([ + '/api/search', + schema.paths['/api/search'].get, + ]); + }); + + it('finds an operation by request when there are multiple other paths', () => { + const schema = { + openapi: '3.0.0', + info: { + title: 'Test', + version: '1.0.0', + }, + paths: { + '/api/search': { + get: { + parameters: [], + }, + }, + '/api/catalog/by-ref': { + get: { + parameters: [], + }, + }, + }, + }; + const request = { + method: 'GET', + url: 'http://localhost:8080/api/search', + } as CompletedRequest; + expect(findOperationByRequest(schema as OpenAPIObject, request)).toEqual([ + '/api/search', + schema.paths['/api/search'].get, + ]); + }); + + it('finds an operation by request when there are path parameters', () => { + const schema = { + openapi: '3.0.0', + info: { + title: 'Test', + version: '1.0.0', + }, + paths: { + '/api/catalog/by-id/{id}': { + get: { + parameters: [], + }, + }, + }, + }; + const request = { + method: 'GET', + url: 'http://localhost:8080/api/catalog/by-id/123', + } as CompletedRequest; + expect(findOperationByRequest(schema as OpenAPIObject, request)).toEqual([ + '/api/catalog/by-id/{id}', + schema.paths['/api/catalog/by-id/{id}'].get, + ]); + }); + + it('finds an operation by request when there are somewhat overlapping path parameters', () => { + const schema = { + openapi: '3.0.0', + info: { + title: 'Test', + version: '1.0.0', + }, + paths: { + '/api/catalog/by-id/{id}': { + get: { + parameters: [], + }, + }, + '/api/catalog/by-id': { + get: { + parameters: [], + }, + }, + }, + }; + const request = { + method: 'GET', + url: 'http://localhost:8080/api/catalog/by-id/123', + } as CompletedRequest; + expect(findOperationByRequest(schema as OpenAPIObject, request)).toEqual([ + '/api/catalog/by-id/{id}', + schema.paths['/api/catalog/by-id/{id}'].get, + ]); + }); +}); diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index 45255a2ea5..51de5d574b 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -60,6 +60,49 @@ class ResponseBodyValidator implements Validator { } } +export function findOperationByRequest( + openApiSchema: OpenAPIObject, + request: CompletedRequest, +): [string, OperationObject] | undefined { + const { url } = request; + const { pathname } = new URL(url); + + const parts = pathname.split('/'); + for (const [path, schema] of Object.entries(openApiSchema.paths)) { + const pathParts = path.split('/'); + if (parts.length !== pathParts.length) { + continue; + } + let found = true; + for (let i = 0; i < parts.length; i++) { + if (pathParts[i] === parts[i]) { + continue; + } + if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { + continue; + } + found = false; + break; + } + if (!found) { + continue; + } + let matchingOperationType: OperationObject | undefined = undefined; + for (const [operationType, operation] of Object.entries(schema)) { + if (operationType === request.method.toLowerCase()) { + matchingOperationType = operation as OperationObject; + break; + } + } + if (!matchingOperationType) { + continue; + } + return [path, matchingOperationType]; + } + + return undefined; +} + export class OpenApiProxyValidator { schema: OpenAPIObject | undefined; validators: Validator[] | undefined; @@ -74,7 +117,7 @@ export class OpenApiProxyValidator { } async validate(request: CompletedRequest, response: CompletedResponse) { - const operationPathTuple = this.findOperation(request); + const operationPathTuple = findOperationByRequest(this.schema!, request); if (!operationPathTuple) { throw new OperationError( { path: request.path, method: request.method } as Operation, @@ -95,46 +138,4 @@ export class OpenApiProxyValidator { ), ); } - - private findOperation( - request: CompletedRequest, - ): [string, OperationObject] | undefined { - const { url } = request; - const { pathname } = new URL(url); - - const parts = pathname.split('/'); - for (const [path, schema] of Object.entries(this.schema!.paths)) { - const pathParts = path.split('/'); - if (parts.length !== pathParts.length) { - continue; - } - let found = true; - for (let i = 0; i < parts.length; i++) { - if (pathParts[i] === parts[i]) { - continue; - } - if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { - continue; - } - found = false; - break; - } - if (!found) { - continue; - } - let matchingOperationType: OperationObject | undefined = undefined; - for (const [operationType, operation] of Object.entries(schema)) { - if (operationType === request.method.toLowerCase()) { - matchingOperationType = operation as OperationObject; - break; - } - } - if (!matchingOperationType) { - continue; - } - return [path, matchingOperationType]; - } - - return undefined; - } } From 1b05177ef616648c7268a4207db2787a02d02e0b Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 15:01:19 -0400 Subject: [PATCH 06/23] linting things Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/src/schema/validation.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index 51de5d574b..fda09b2297 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -14,13 +14,7 @@ * limitations under the License. */ import { CompletedRequest, CompletedResponse } from 'mockttp'; -import { - OpenAPIObject, - OperationObject, - ParameterObject, - ResponseObject, - SchemaObject, -} from 'openapi3-ts'; +import { OpenAPIObject, OperationObject } from 'openapi3-ts'; import Ajv from 'ajv'; import Parser from '@apidevtools/swagger-parser'; import { Operation, Validator, ValidatorParams } from './types'; From 2c7750ca57ca826c426e5dddd13855f816157e46 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 15:27:08 -0400 Subject: [PATCH 07/23] better error messages Signed-off-by: aramissennyeydd --- .../src/schema/errors.ts | 30 +++++++++++++++++++ .../src/schema/parameter-validation.test.ts | 20 ++++++------- .../src/schema/parameter-validation.ts | 17 ++++++----- .../schema/request-body-validation.test.ts | 10 +++---- .../src/schema/request-body-validation.ts | 7 +++-- .../schema/response-body-validation.test.ts | 10 +++---- .../src/schema/response-body-validation.ts | 16 ++++++---- .../backend-openapi-utils/src/schema/utils.ts | 17 +++++++++++ .../src/schema/validation.test.ts | 10 +++---- .../src/schema/validation.ts | 11 ++++++- 10 files changed, 107 insertions(+), 41 deletions(-) diff --git a/packages/backend-openapi-utils/src/schema/errors.ts b/packages/backend-openapi-utils/src/schema/errors.ts index 44de054c53..e1950bcce9 100644 --- a/packages/backend-openapi-utils/src/schema/errors.ts +++ b/packages/backend-openapi-utils/src/schema/errors.ts @@ -15,6 +15,8 @@ */ import { Operation } from './types'; +import { ErrorObject } from 'ajv'; +import { humanifyAjvError } from './utils'; export class OperationError extends Error { constructor(operation: Operation, message: string) { @@ -35,3 +37,31 @@ export class OperationResponseError extends Error { ); } } + +export class OperationParsingError extends OperationError { + constructor(operation: Operation, type: string, errors: ErrorObject[]) { + super( + operation, + `${type} validation failed.\n - ${errors + .map(humanifyAjvError) + .join('\n - ')}`, + ); + } +} + +export class OperationParsingResponseError extends OperationResponseError { + constructor( + operation: Operation, + response: Response, + type: string, + errors: ErrorObject[], + ) { + super( + operation, + response, + `${type} validation failed.\n - ${errors + .map(humanifyAjvError) + .join('\n - ')}`, + ); + } +} diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts index 717256e266..6ae2f4f8a0 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts @@ -91,11 +91,11 @@ describe('query parameters', () => { const request = { url: 'http://localhost:8080/api/search?param=hello', } as Request; - await expect( - parser.parse(request), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Query parameter validation failed"`, - ); + await expect(parser.parse(request)).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["GET /api/search"] Query parameter validation failed. + - Value should be of type number" + `); }); }); }); @@ -514,11 +514,11 @@ describe('path parameters', () => { const request = { url: 'http://localhost:8080/api/item/hello', } as Request; - await expect( - parser.parse(request), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/item/{id}"] Path parameter validation failed"`, - ); + await expect(parser.parse(request)).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["GET /api/item/{id}"] Path parameter validation failed. + - Value should be of type number" + `); }); }); }); diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 65e6439b7e..19d49ef6ef 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -23,7 +23,7 @@ import { ValidatorParams, } from './types'; import Ajv from 'ajv'; -import { OperationError } from './errors'; +import { OperationError, OperationParsingError } from './errors'; import { mockttpToFetchRequest } from './utils'; type ReferencelessSchemaObject = SchemaObject & { $ref?: never }; @@ -150,9 +150,10 @@ export class QueryParameterParser const validate = this.ajv.compile(parameter.schema); const valid = validate(param); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - 'Query parameter validation failed', + 'Query parameter', + validate.errors!, ); } queryParameters[name] = param; @@ -339,9 +340,10 @@ export class HeaderParameterParser const valid = validate(header); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - 'Header parameter validation failed', + 'Header parameter', + validate.errors!, ); } headerParameters[name] = header; @@ -384,9 +386,10 @@ export class PathParameterParser const valid = validate(param); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - 'Path parameter validation failed', + 'Path parameter', + validate.errors!, ); } pathParameters[name] = param; diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts index a4452763fc..df8b33402c 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts @@ -61,11 +61,11 @@ describe('request body', () => { const requestBody = { query: 1, }; - await expect( - parser.parse(toRequest(requestBody)), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["POST /api/search"] Request body validation failed."`, - ); + await expect(parser.parse(toRequest(requestBody))).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["POST /api/search"] Request body validation failed. + - "/query" should be of type string" + `); }); it('should throw error if request body is required but missing', async () => { diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.ts index 22873c8100..f7681f01c2 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.ts @@ -17,7 +17,7 @@ import { JsonObject } from '@backstage/types'; import { Operation, ParserOptions, RequestParser } from './types'; import { ValidateFunction } from 'ajv'; -import { OperationError } from './errors'; +import { OperationError, OperationParsingError } from './errors'; import { RequestBodyObject, SchemaObject } from 'openapi3-ts'; class DisabledRequestBodyParser @@ -120,9 +120,10 @@ export class RequestBodyParser const body = (await request.json()) as JsonObject; const valid = this.validate(body); if (!valid) { - throw new OperationError( + throw new OperationParsingError( this.operation, - `Request body validation failed.`, + `Request body`, + this.validate.errors!, ); } return body; diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts index 9ad47b8a19..6cd098b2e7 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.test.ts @@ -60,11 +60,11 @@ describe('response body', () => { const responseBody = { result: 1, }; - await expect( - parser.parse(toResponse(responseBody)), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search" (200)]: Response body validation failed"`, - ); + await expect(parser.parse(toResponse(responseBody))).rejects + .toThrowErrorMatchingInlineSnapshot(` + "["GET /api/search" (200)]: Response body validation failed. + - The "result" property is not allowed" + `); }); it('should throw error if response body is required but missing', async () => { diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.ts index 116fc888e8..1a2a475afc 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.ts @@ -16,7 +16,11 @@ import { JsonObject } from '@backstage/types'; import { Operation, ParserOptions, ResponseParser } from './types'; -import { OperationError, OperationResponseError } from './errors'; +import { + OperationError, + OperationParsingResponseError, + OperationResponseError, +} from './errors'; import Ajv from 'ajv'; import { OperationObject, ResponseObject } from 'openapi3-ts'; @@ -58,6 +62,7 @@ export class ResponseBodyParser const responseSchemas = operation.schema.responses; for (const [statusCode, schema] of Object.entries(responseSchemas)) { if (!schema.content) { + // Skip responses without content, eg 204 No Content. continue; } else if (!schema.content['application/json']) { throw new OperationError( @@ -107,6 +112,7 @@ export class ResponseBodyParser ); } const schema = responseSchema.content!['application/json'].schema; + // This is a bit of type laziness. Ideally, this would be a type-narrowing function, but I wasn't able to get the types to work. if (!schema) { throw new OperationError(this.operation, 'No schema found in response'); } @@ -133,10 +139,11 @@ export class ResponseBodyParser const jsonBody = (await response.json()) as JsonObject; const valid = validate(jsonBody); if (!valid) { - throw new OperationResponseError( + throw new OperationParsingResponseError( this.operation, response, - 'Response body validation failed', + 'Response body', + validate.errors!, ); } return jsonBody; @@ -144,9 +151,8 @@ export class ResponseBodyParser private findResponseSchema( operationSchema: OperationObject, - response: Response, + { status }: Response, ): ResponseObject | undefined { - const { status } = response; return ( operationSchema.responses?.[status] ?? operationSchema.responses?.default ); diff --git a/packages/backend-openapi-utils/src/schema/utils.ts b/packages/backend-openapi-utils/src/schema/utils.ts index cc00d00095..7f9385a404 100644 --- a/packages/backend-openapi-utils/src/schema/utils.ts +++ b/packages/backend-openapi-utils/src/schema/utils.ts @@ -14,6 +14,7 @@ * limitations under the License. */ import { CompletedRequest, CompletedResponse } from 'mockttp'; +import { ErrorObject } from 'ajv'; export function mockttpToFetchRequest(request: CompletedRequest) { const headers = new Headers(request.rawHeaders); @@ -34,3 +35,19 @@ export function mockttpToFetchResponse(response: CompletedResponse) { text: () => response.body?.getText(), } as Response; } + +export function humanifyAjvError(error: ErrorObject) { + switch (error.keyword) { + case 'required': + return `The ${error.params.missingProperty} property is required`; + case 'type': + console.log(error); + return `${ + error.instancePath ? `"${error.instancePath}"` : 'Value' + } should be of type ${error.params.type}`; + case 'additionalProperties': + return `The "${error.params.additionalProperty}" property is not allowed`; + default: + return error.message; + } +} diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index 3f172c16fe..3b73250cd9 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -164,11 +164,11 @@ describe('OpenApiProxyValidator', () => { statusCode: 200, }); - await expect( - async () => await validator.validate(request, response), - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Query parameter validation failed"`, - ); + await expect(async () => await validator.validate(request, response)) + .rejects.toThrowErrorMatchingInlineSnapshot(` + "["GET /api/search"] Query parameter validation failed. + - Value should be of type number" + `); }); it('accepts valid parameter', async () => { diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index fda09b2297..d9d80ddcf5 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -24,7 +24,7 @@ import { RequestBodyParser } from './request-body-validation'; import { mockttpToFetchRequest, mockttpToFetchResponse } from './utils'; import { ResponseBodyParser } from './response-body-validation'; -const ajv = new Ajv({ allErrors: true }); // options can be passed, e.g. {allErrors: true} +const ajv = new Ajv({ allErrors: true }); class RequestBodyValidator implements Validator { schema: OpenAPIObject; @@ -34,6 +34,7 @@ class RequestBodyValidator implements Validator { async validate({ pair, operation }: ValidatorParams) { const { request } = pair; + // NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal. const parser = RequestBodyParser.fromOperation(operation, { ajv }); const fetchRequest = mockttpToFetchRequest(request); await parser.parse(fetchRequest); @@ -48,12 +49,19 @@ class ResponseBodyValidator implements Validator { async validate({ pair, operation }: ValidatorParams) { const { response } = pair; + // NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal. const parser = ResponseBodyParser.fromOperation(operation, { ajv }); const fetchResponse = mockttpToFetchResponse(response); await parser.parse(fetchResponse); } } +/** + * Find an operation in an OpenAPI schema that matches a request. This is done by comparing the request URL to the paths in the schema. + * @param openApiSchema - The OpenAPI schema to search for the operation in. + * @param request - The request to find the operation for. + * @returns A tuple of the path and the operation object that matches the request. + */ export function findOperationByRequest( openApiSchema: OpenAPIObject, request: CompletedRequest, @@ -72,6 +80,7 @@ export function findOperationByRequest( if (pathParts[i] === parts[i]) { continue; } + // If the path part is a parameter, we can count it as a match. eg /api/{id} will match /api/1 if (pathParts[i].startsWith('{') && pathParts[i].endsWith('}')) { continue; } From 1e952426c1bbf1a7f70ab4325fc44e5c4bb92e79 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 16:03:45 -0400 Subject: [PATCH 08/23] add comments and improve form/explode checks Signed-off-by: aramissennyeydd --- .../src/schema/parameter-validation.test.ts | 45 ++++++ .../src/schema/parameter-validation.ts | 128 +++++++++++++----- .../backend-openapi-utils/src/schema/utils.ts | 1 - .../src/schema/validation.test.ts | 8 +- 4 files changed, 143 insertions(+), 39 deletions(-) diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts index 6ae2f4f8a0..7a34c2c309 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.test.ts @@ -334,6 +334,51 @@ describe('query parameters', () => { }); expect(result.extra).toEqual(['hello', 'world']); }); + + it('should respect other object encodings', async () => { + const parameter = { + name: 'extra', + in: 'query', + style: 'deepObject', + explode: true, + schema: { type: 'object' }, + required: false, + } as ParameterObject; + schema.parameters!.push(parameter as any); + parser = new QueryParameterParser(operation, { ajv }); + const request = { + url: 'http://localhost:8080/api/search?key=value&otherkey=value2&extra[hello]=world', + } as Request; + + const result = await parser.parse(request); + expect(result.param).toEqual({ + key: 'value', + otherkey: 'value2', + }); + expect(result.extra).toEqual({ hello: 'world' }); + }); + + it('should throw if there are 2 form explode parameters', async () => { + const parameter = { + name: 'extra', + in: 'query', + style: 'form', + explode: true, + schema: { type: 'object' }, + required: false, + } as ParameterObject; + schema.parameters!.push(parameter as any); + parser = new QueryParameterParser(operation, { ajv }); + const request = { + url: 'http://localhost:8080/api/search?key=value&otherkey=value2&extra[hello]=world', + } as Request; + + await expect(() => + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["GET /api/search"] Ambiguous query parameters, you cannot have 2 form explode parameters"`, + ); + }); }); describe('explode=false', () => { diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 19d49ef6ef..5cdb3dbd6b 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -71,6 +71,13 @@ class BaseParameterParser { } } + /** + * Attempt to transform a string value to its expected type, this allows Ajv to perform validation. As these are parameters, + * support for edge cases like nested type casting is not currently supported. + * @param value + * @param schema + * @returns + */ optimisticallyParseValue(value: string, schema: SchemaObject) { if (schema.type === 'integer') { return parseInt(value, 10); @@ -78,10 +85,20 @@ class BaseParameterParser { if (schema.type === 'number') { return parseFloat(value); } + if (schema.type === 'boolean') { + if (['true', 'false'].includes(value)) { + return value === 'true'; + } + throw new Error('Invalid boolean value must be either "true" or "false"'); + } return value; } } +const PLACE_A_BEFORE_B = -1; +const PLACE_A_AFTER_B = 1; +const EQUAL = 0; + export class QueryParameterParser extends BaseParameterParser implements RequestParser> @@ -93,18 +110,52 @@ export class QueryParameterParser const { searchParams } = new URL(request.url); const remainingQueryParameters = new Set(searchParams.keys()); const queryParameters: Record = {}; + + // object parameters with form/explode style should be processed last as they collect all remaining parameters. const parameterIterator = Object.entries(this.parameters).toSorted( - ([_, parameter]) => { - if (parameter.schema.type !== 'object') { - return -1; + ([_, parameterA], [_B, parameterB]) => { + if ( + parameterA.schema.type !== 'object' && + parameterB.schema.type !== 'object' + ) { + return EQUAL; } - if (parameter.style === 'form' || !parameter.style) { - if (parameter.explode || typeof parameter.explode === 'undefined') { - return 1; - } - return 0; + if ( + parameterA.schema.type === 'object' && + parameterB.schema.type !== 'object' + ) { + return PLACE_A_AFTER_B; } - return 0; + if ( + parameterA.schema.type !== 'object' && + parameterB.schema.type === 'object' + ) { + return PLACE_A_BEFORE_B; + } + const isParameterAForm = + parameterA.style === 'form' || !parameterA.style; + const isParameterAFormExplode = + isParameterAForm && + (parameterA.explode || typeof parameterA.explode === 'undefined'); + const isParameterBForm = + parameterB.style === 'form' || !parameterB.style; + const isParameterBFormExplode = + isParameterBForm && + (parameterB.explode || typeof parameterB.explode === 'undefined'); + // Sort the form explode to the bottom of the array. + if (isParameterAFormExplode && isParameterBFormExplode) { + throw new OperationError( + this.operation, + 'Ambiguous query parameters, you cannot have 2 form explode parameters', + ); + } + if (isParameterAFormExplode) { + return PLACE_A_AFTER_B; + } + if (isParameterBFormExplode) { + return PLACE_A_BEFORE_B; + } + return EQUAL; }, ); for (const [name, parameter] of parameterIterator) { @@ -123,13 +174,15 @@ export class QueryParameterParser // eslint-disable-next-line prefer-const let [param, indices]: [any | null, string[]] = this.#findQueryParameters( this.parameters, - queryParameters, + remainingQueryParameters, searchParams, name, ); if (!!param) { indices.forEach(index => remainingQueryParameters.delete(index)); } + + // The query parameters can be either a single value or an array of values, try to wrangle them into the expected format if they're not explicitly an array. if (parameter.schema.type !== 'array' && Array.isArray(param)) { param = param.length > 0 ? param[0] : undefined; } @@ -145,6 +198,7 @@ export class QueryParameterParser continue; } if (param) { + // We do this here because all query parameters are strings but the schema will expect the real value. param = this.optimisticallyParseValue(param, parameter.schema); } const validate = this.ajv.compile(parameter.schema); @@ -171,19 +225,26 @@ export class QueryParameterParser #findQueryParameters( parameters: Record, - currentQueryParameters: Record, + remainingQueryParameters: Set, searchParams: URLSearchParams, name: string, ): [any | null, string[]] { const parameter = parameters[name]; const schema = parameter.schema as SchemaObject; + // Since getAll will return an empty array if the key is not found, we need to check if the key exists first. const getIfExists = (key: string) => searchParams.has(key) ? searchParams.getAll(key) : null; if (schema.type === 'array') { - if (parameter.style === 'form' || !parameter.style) { + // Form is the default array format. + if ( + parameter.style === 'form' || + typeof parameter.style === 'undefined' + ) { + // As is explode = true. if (parameter.explode || typeof parameter.explode === 'undefined') { + // Support for qs explode format. Every value is stored as a separate query parameter. if (!searchParams.has(name) && searchParams.has(`${name}[0]`)) { const values: string[] = []; const indices: string[] = []; @@ -195,15 +256,18 @@ export class QueryParameterParser } return [values, indices]; } + // If not qs format, grab all values with the same name from search params. return [getIfExists(name), [name]]; } + // Add support for qs non-standard array format. This is helpful for search-backend, since that uses qs still. if (!searchParams.has(name) && searchParams.has(`${name}[]`)) { return [searchParams.get(`${name}[]`)?.split(','), [`${name}[]`]]; } + // Non-explode arrays should be comma separated. if (searchParams.has(name) && searchParams.getAll(name).length > 1) { throw new OperationError( this.operation, - 'Array parameter should not have multiple values', + 'Arrays must be comma separated in non-explode mode', ); } return [searchParams.get(name)?.split(','), [name]]; @@ -218,14 +282,19 @@ export class QueryParameterParser ); } if (schema.type === 'object') { - if (parameter.style === 'form' || !parameter.style) { + // Form is the default object format. + if ( + parameter.style === 'form' || + typeof parameter.style === 'undefined' + ) { if (parameter.explode) { + // Object form/explode is a collection of disjoint keys, there's no mapping for what they are so we collect all of them. + // This means we need to run this as the last query parameter that is processed. const obj: Record = {}; const indices: string[] = []; for (const [key, value] of searchParams.entries()) { - if ( - this.#matchesOtherQueryParameters(currentQueryParameters, key) - ) { + // Have we processed this query parameter as part of another parameter parsing? If not, consider it to be a part of this object. + if (!remainingQueryParameters.has(key)) { continue; } indices.push(key); @@ -233,6 +302,7 @@ export class QueryParameterParser } return [obj, indices]; } + // For non-explode, the schema is comma separated key,value "pairs", so filter=key1,value1,key2,value2 would parse to {key1: value1, key2: value2}. const obj: Record = {}; const value = searchParams.get(name); if (value) { @@ -240,7 +310,7 @@ export class QueryParameterParser if (parts.length % 2 !== 0) { throw new OperationError( this.operation, - 'Invalid object parameter', + 'Invalid object query parameter, must have an even number of key-value pairs', ); } for (let i = 0; i < parts.length; i += 2) { @@ -249,6 +319,8 @@ export class QueryParameterParser } return [obj, [name]]; } else if (parameter.style === 'deepObject') { + // Deep object is a nested object structure, so we need to parse the keys to build the object. + // example: ?filter[key1]=value1&filter[key2]=value2 => { key1: value1, key2: value2 } const obj: Record = {}; const indices: string[] = []; for (const [key, value] of searchParams.entries()) { @@ -261,7 +333,7 @@ export class QueryParameterParser if (!part.includes(']')) { throw new OperationError( this.operation, - 'Invalid object parameter', + `Invalid object parameter, missing closing bracket for key "${key}"`, ); } const objKey = part.split(']')[0]; @@ -274,7 +346,7 @@ export class QueryParameterParser if (!lastPart.includes(']')) { throw new OperationError( this.operation, - 'Invalid object parameter', + `Invalid object parameter, missing closing bracket for key "${key}"`, ); } currentLayer[lastPart.split(']')[0]] = value; @@ -284,24 +356,12 @@ export class QueryParameterParser } throw new OperationError( this.operation, - 'Unsupported style for object parameter', + `Unsupported style for object parameter, "${parameter.style}"`, ); } // For everything else, just return the value. return [getIfExists(name), [name]]; } - - #matchesOtherQueryParameters( - parameters: Record, - nameToMatch: string, - ) { - for (const [name] of Object.entries(parameters)) { - if (name === nameToMatch) { - return true; - } - } - return false; - } } export class HeaderParameterParser @@ -368,7 +428,7 @@ export class PathParameterParser }); const pathParameters: Record = {}; for (const [name, parameter] of Object.entries(this.parameters)) { - let param: string | number = params[name]; + let param: string | number | boolean = params[name]; if (!param && parameter.required) { throw new OperationError( this.operation, diff --git a/packages/backend-openapi-utils/src/schema/utils.ts b/packages/backend-openapi-utils/src/schema/utils.ts index 7f9385a404..9c5a400ec9 100644 --- a/packages/backend-openapi-utils/src/schema/utils.ts +++ b/packages/backend-openapi-utils/src/schema/utils.ts @@ -41,7 +41,6 @@ export function humanifyAjvError(error: ErrorObject) { case 'required': return `The ${error.params.missingProperty} property is required`; case 'type': - console.log(error); return `${ error.instancePath ? `"${error.instancePath}"` : 'Value' } should be of type ${error.params.type}`; diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index 3b73250cd9..fdb1350625 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -242,7 +242,7 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Invalid object parameter"`, + `"["GET /api/search"] Invalid object parameter, missing closing bracket for key "param[t""`, ); }); }); @@ -301,7 +301,7 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Invalid object parameter"`, + `"["GET /api/search"] Invalid object query parameter, must have an even number of key-value pairs"`, ); }); describe('explode', () => { @@ -369,7 +369,7 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Invalid object parameter"`, + `"["GET /api/search"] Invalid object query parameter, must have an even number of key-value pairs"`, ); }); }); @@ -517,7 +517,7 @@ describe('OpenApiProxyValidator', () => { await expect( async () => await validator.validate(request, response), ).rejects.toThrowErrorMatchingInlineSnapshot( - `"["GET /api/search"] Array parameter should not have multiple values"`, + `"["GET /api/search"] Arrays must be comma separated in non-explode mode"`, ); }); }); From f63ad78082a6157d3cc9b09882547c2bc6715b61 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 16:37:12 -0400 Subject: [PATCH 09/23] more fixes and validating with search-backend Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 1 + .../backend-openapi-utils/src/proxy/setup.ts | 17 +++++++---- .../schema/request-body-validation.test.ts | 28 ++++++++++++++++++- .../src/schema/request-body-validation.ts | 10 +++++-- .../src/schema/response-body-validation.ts | 21 ++++++++++---- .../backend-openapi-utils/src/testUtils.ts | 4 +-- yarn.lock | 8 ++++++ 7 files changed, 72 insertions(+), 17 deletions(-) diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 5fb1f98e1b..04653c5b32 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -44,6 +44,7 @@ "express": "^4.17.1", "express-openapi-validator": "^5.0.4", "express-promise-router": "^4.1.0", + "get-port": "^7.1.0", "json-schema-to-ts": "^3.0.0", "lodash": "^4.17.21", "mockttp": "^3.13.0", diff --git a/packages/backend-openapi-utils/src/proxy/setup.ts b/packages/backend-openapi-utils/src/proxy/setup.ts index 8f24aa2512..da84d44afc 100644 --- a/packages/backend-openapi-utils/src/proxy/setup.ts +++ b/packages/backend-openapi-utils/src/proxy/setup.ts @@ -16,6 +16,8 @@ import * as mockttp from 'mockttp'; import { OpenApiProxyValidator } from '../schema/validation'; +import getPort from 'get-port'; +import { Server } from 'http'; export class Proxy { server: mockttp.Mockttp; @@ -25,6 +27,8 @@ export class Proxy { mockttp.CompletedResponse >(); validator: OpenApiProxyValidator; + public forwardTo: { port: number } = { port: 0 }; + express: { server: Server | undefined } = { server: undefined }; constructor() { this.server = mockttp.getLocal(); this.validator = new OpenApiProxyValidator(); @@ -32,9 +36,10 @@ export class Proxy { async setup() { await this.server.start(); + this.forwardTo.port = await getPort(); this.server .forAnyRequest() - .thenForwardTo(`http://localhost:${process.env.PORT}`); + .thenForwardTo(`http://localhost:${this.forwardTo.port}`); await this.server.on('request', request => { this.#openRequests[request.id] = request; }); @@ -53,10 +58,9 @@ export class Proxy { }); } - async initialize() { - await this.validator.initialize( - `http://localhost:${process.env.PORT}/openapi.json`, - ); + async initialize(url: string, server: Server) { + await this.validator.initialize(`${url}/openapi.json`); + this.express.server = server; } stop() { @@ -64,6 +68,9 @@ export class Proxy { throw new Error('There are still open requests'); } this.server.stop(); + + // If this isn't expressly closed, it will cause a jest memory leak warning. + this.express.server?.close(); } get url() { diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts index df8b33402c..807eaa4569 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts @@ -19,7 +19,12 @@ import { RequestBodyParser } from './request-body-validation'; import Ajv from 'ajv'; import { Operation, RequestParser } from './types'; import _ from 'lodash'; -import { OperationObject, RequestBodyObject } from 'openapi3-ts'; +import { + ContentObject, + MediaTypeObject, + OperationObject, + RequestBodyObject, +} from 'openapi3-ts'; import { JsonObject } from '@backstage/types'; const ajv = new Ajv(); @@ -79,4 +84,25 @@ describe('request body', () => { `"["POST /api/search"] No request body found for /api/search"`, ); }); + + it('should throw error if request body is not application/json', async () => { + const request = toRequest({}, { 'content-type': 'text/plain' }); + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["POST /api/search"] Content type is not application/json"`, + ); + }); + + it('should NOT throw error if request body is not just application/json', async () => { + (schema.requestBody.content as ContentObject)[ + 'application/json; charset=utf-8' + ] = schema.requestBody.content['application/json'] as MediaTypeObject; + delete (schema.requestBody.content as ContentObject)['application/json']; + parser = new RequestBodyParser(operation, { + ajv, + }); + const request = toRequest({}); + expect(await parser.parse(request)).toEqual({}); + }); }); diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.ts index f7681f01c2..0075bae641 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.ts @@ -77,13 +77,17 @@ export class RequestBodyParser 'No content found in request body', ); } - if (!requestBody!.content['application/json']) { + const contentTypes = requestBody!.content; + const jsonContentType = Object.keys(contentTypes).find(contentType => + contentType.split(';').includes('application/json'), + ); + if (!jsonContentType) { throw new OperationError( this.operation, 'No application/json content type found in request body', ); } - const schema = requestBody!.content['application/json'].schema; + const schema = requestBody!.content[jsonContentType].schema; if (!schema) { throw new OperationError( this.operation, @@ -111,7 +115,7 @@ export class RequestBodyParser const contentType = request.headers.get('content-type') || 'application/json'; - if (contentType !== 'application/json') { + if (!contentType.split(';').includes('application/json')) { throw new OperationError( this.operation, 'Content type is not application/json', diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.ts index 1a2a475afc..71a4512e75 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.ts @@ -61,15 +61,20 @@ export class ResponseBodyParser this.ajv = options.ajv; const responseSchemas = operation.schema.responses; for (const [statusCode, schema] of Object.entries(responseSchemas)) { - if (!schema.content) { + const contentTypes = schema.content; + if (!contentTypes) { // Skip responses without content, eg 204 No Content. continue; - } else if (!schema.content['application/json']) { + } + const jsonContentType = Object.keys(contentTypes).find(contentType => + contentType.split(';').includes('application/json'), + ); + if (!jsonContentType) { throw new OperationError( this.operation, `No application/json content type found in response for status code ${statusCode}`, ); - } else if ('$ref' in schema.content['application/json'].schema) { + } else if ('$ref' in contentTypes[jsonContentType].schema) { throw new OperationError( this.operation, 'Reference objects are not supported', @@ -97,21 +102,25 @@ export class ResponseBodyParser ); } - if (!responseSchema?.content && body?.length) { + const contentTypes = responseSchema.content; + if (!contentTypes && body?.length) { throw new OperationResponseError( this.operation, response, 'Received a body but no schema was found', ); } - if (!responseSchema?.content!['application/json']) { + const jsonContentType = Object.keys(contentTypes ?? {}).find(contentType => + contentType.split(';').includes('application/json'), + ); + if (!jsonContentType) { throw new OperationResponseError( this.operation, response, 'No application/json content type found in response', ); } - const schema = responseSchema.content!['application/json'].schema; + const schema = responseSchema.content![jsonContentType].schema; // This is a bit of type laziness. Ideally, this would be a type-narrowing function, but I wasn't able to get the types to work. if (!schema) { throw new OperationError(this.operation, 'No schema found in response'); diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index f96a0b1b76..d93029fa8e 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -28,8 +28,8 @@ afterAll(() => { }); export async function wrapServer(app: Express): Promise { - const server = app.listen(+process.env.PORT!); - await proxy.initialize(); + const server = app.listen(proxy.forwardTo.port); + await proxy.initialize(`http://localhost:${proxy.forwardTo.port}`, server); return { ...server, address: () => new URL(proxy.url) } as any; } diff --git a/yarn.lock b/yarn.lock index bcc6a951fa..bf267d84fd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3750,6 +3750,7 @@ __metadata: express: ^4.17.1 express-openapi-validator: ^5.0.4 express-promise-router: ^4.1.0 + get-port: ^7.1.0 json-schema-to-ts: ^3.0.0 lodash: ^4.17.21 mockttp: ^3.13.0 @@ -28304,6 +28305,13 @@ __metadata: languageName: node linkType: hard +"get-port@npm:^7.1.0": + version: 7.1.0 + resolution: "get-port@npm:7.1.0" + checksum: f4d23b43026124007663a899578cc87ff37bfcf645c5c72651e9810ebafc759857784e409fb8e0ada9b90e5c5db089b0ae2f5f6b49fba1ce2e0aff86094ab17d + languageName: node + linkType: hard + "get-stdin@npm:^9.0.0": version: 9.0.0 resolution: "get-stdin@npm:9.0.0" From ea8ffdb9dc8c7e207b1af818f7e0eefb0f85c97f Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 16:39:10 -0400 Subject: [PATCH 10/23] don't validate 400 request body or query params Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/src/schema/utils.ts | 2 +- packages/backend-openapi-utils/src/schema/validation.ts | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/backend-openapi-utils/src/schema/utils.ts b/packages/backend-openapi-utils/src/schema/utils.ts index 9c5a400ec9..aaf3fc6c41 100644 --- a/packages/backend-openapi-utils/src/schema/utils.ts +++ b/packages/backend-openapi-utils/src/schema/utils.ts @@ -39,7 +39,7 @@ export function mockttpToFetchResponse(response: CompletedResponse) { export function humanifyAjvError(error: ErrorObject) { switch (error.keyword) { case 'required': - return `The ${error.params.missingProperty} property is required`; + return `The "${error.params.missingProperty}" property is required`; case 'type': return `${ error.instancePath ? `"${error.instancePath}"` : 'Value' diff --git a/packages/backend-openapi-utils/src/schema/validation.ts b/packages/backend-openapi-utils/src/schema/validation.ts index d9d80ddcf5..5f7c6fe353 100644 --- a/packages/backend-openapi-utils/src/schema/validation.ts +++ b/packages/backend-openapi-utils/src/schema/validation.ts @@ -33,7 +33,12 @@ class RequestBodyValidator implements Validator { } async validate({ pair, operation }: ValidatorParams) { - const { request } = pair; + const { request, response } = pair; + if (response.statusCode === 400) { + // If the response is a 400, then the request is invalid and we shouldn't validate the parameters + return; + } + // NOTE: There may be a worthwhile optimization here to cache these results to avoid re-parsing the schema for every request. As is, I don't think this is a big deal. const parser = RequestBodyParser.fromOperation(operation, { ajv }); const fetchRequest = mockttpToFetchRequest(request); From 66af016ab7fcdb75bbacbb8cbfda07bd4dd8ea02 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 16:49:52 -0400 Subject: [PATCH 11/23] add changesets Signed-off-by: aramissennyeydd --- .changeset/eleven-beds-play.md | 6 ++++++ .changeset/smart-jobs-sit.md | 5 +++++ 2 files changed, 11 insertions(+) create mode 100644 .changeset/eleven-beds-play.md create mode 100644 .changeset/smart-jobs-sit.md diff --git a/.changeset/eleven-beds-play.md b/.changeset/eleven-beds-play.md new file mode 100644 index 0000000000..32ae11ff49 --- /dev/null +++ b/.changeset/eleven-beds-play.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-search-backend': patch +'@backstage/plugin-catalog-backend': patch +--- + +Updated to use the improved OpenAPI Jest validation. diff --git a/.changeset/smart-jobs-sit.md b/.changeset/smart-jobs-sit.md new file mode 100644 index 0000000000..edd21e67b5 --- /dev/null +++ b/.changeset/smart-jobs-sit.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-openapi-utils': minor +--- + +Improved support for OpenAPI validation during Jest tests. Now, OpenAPI validation can happen as you are writing your Jest tests - you no longer have to run `repo schema openapi test`. From 28744791cb44365eff09f0b8f03bbf8f53847179 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 16:51:40 -0400 Subject: [PATCH 12/23] remove configuration around throwing Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/src/proxy/setup.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/backend-openapi-utils/src/proxy/setup.ts b/packages/backend-openapi-utils/src/proxy/setup.ts index da84d44afc..c49f0d87de 100644 --- a/packages/backend-openapi-utils/src/proxy/setup.ts +++ b/packages/backend-openapi-utils/src/proxy/setup.ts @@ -49,12 +49,7 @@ export class Proxy { this.requestResponsePairs.set(request, response); } delete this.#openRequests[response.id]; - this.validator.validate(request, response).catch(err => { - if (process.env.THROW) { - throw err; - } - console.error(err); - }); + this.validator.validate(request, response); }); } From 85b4e92f5ca5a96561422f331d4d0cf2bff8fd05 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 17:44:17 -0400 Subject: [PATCH 13/23] small fixes for CI checks Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/report.api.md | 8 ++ packages/backend-openapi-utils/src/index.ts | 6 +- .../src/schema/parameter-validation.ts | 88 +++++++++---------- .../src/schema/validation.test.ts | 4 +- .../backend-openapi-utils/src/testUtils.ts | 26 ++++-- .../src/service/createRouter.test.ts | 4 +- .../search-backend/src/service/router.test.ts | 4 +- 7 files changed, 84 insertions(+), 56 deletions(-) diff --git a/packages/backend-openapi-utils/report.api.md b/packages/backend-openapi-utils/report.api.md index a0495f39b5..5a2c89ee81 100644 --- a/packages/backend-openapi-utils/report.api.md +++ b/packages/backend-openapi-utils/report.api.md @@ -3,6 +3,8 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +/// + import type { ContentObject } from 'openapi3-ts'; import type core from 'express-serve-static-core'; import { Express as Express_2 } from 'express'; @@ -682,6 +684,9 @@ type SchemaRef = Schema extends { [Key in keyof Schema]: SchemaRef; }; +// @public +export function setupProxyHooks(): void; + // @public type TemplateToDocPath< Doc extends PathDoc, @@ -718,6 +723,9 @@ type ValueOf = T[keyof T]; // @public export const wrapInOpenApiTestServer: (app: Express_2) => Server | Express_2; +// @public +export function wrapServer(app: Express_2): Promise; + // Warnings were encountered during analysis: // // src/router.d.ts:8:5 - (ae-undocumented) Missing documentation for "get". diff --git a/packages/backend-openapi-utils/src/index.ts b/packages/backend-openapi-utils/src/index.ts index 57d9f755c2..24f4183311 100644 --- a/packages/backend-openapi-utils/src/index.ts +++ b/packages/backend-openapi-utils/src/index.ts @@ -32,4 +32,8 @@ export type { } from './utility'; export type { ApiRouter } from './router'; export { createValidatedOpenApiRouter, getOpenApiSpecRoute } from './stub'; -export { wrapInOpenApiTestServer, wrapServer } from './testUtils'; +export { + wrapInOpenApiTestServer, + wrapServer, + setupProxyHooks, +} from './testUtils'; diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 5cdb3dbd6b..90953e6e2a 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -111,53 +111,51 @@ export class QueryParameterParser const remainingQueryParameters = new Set(searchParams.keys()); const queryParameters: Record = {}; + const parameterIterator = Object.entries(this.parameters); + // object parameters with form/explode style should be processed last as they collect all remaining parameters. - const parameterIterator = Object.entries(this.parameters).toSorted( - ([_, parameterA], [_B, parameterB]) => { - if ( - parameterA.schema.type !== 'object' && - parameterB.schema.type !== 'object' - ) { - return EQUAL; - } - if ( - parameterA.schema.type === 'object' && - parameterB.schema.type !== 'object' - ) { - return PLACE_A_AFTER_B; - } - if ( - parameterA.schema.type !== 'object' && - parameterB.schema.type === 'object' - ) { - return PLACE_A_BEFORE_B; - } - const isParameterAForm = - parameterA.style === 'form' || !parameterA.style; - const isParameterAFormExplode = - isParameterAForm && - (parameterA.explode || typeof parameterA.explode === 'undefined'); - const isParameterBForm = - parameterB.style === 'form' || !parameterB.style; - const isParameterBFormExplode = - isParameterBForm && - (parameterB.explode || typeof parameterB.explode === 'undefined'); - // Sort the form explode to the bottom of the array. - if (isParameterAFormExplode && isParameterBFormExplode) { - throw new OperationError( - this.operation, - 'Ambiguous query parameters, you cannot have 2 form explode parameters', - ); - } - if (isParameterAFormExplode) { - return PLACE_A_AFTER_B; - } - if (isParameterBFormExplode) { - return PLACE_A_BEFORE_B; - } + parameterIterator.sort(([_, parameterA], [_B, parameterB]) => { + if ( + parameterA.schema.type !== 'object' && + parameterB.schema.type !== 'object' + ) { return EQUAL; - }, - ); + } + if ( + parameterA.schema.type === 'object' && + parameterB.schema.type !== 'object' + ) { + return PLACE_A_AFTER_B; + } + if ( + parameterA.schema.type !== 'object' && + parameterB.schema.type === 'object' + ) { + return PLACE_A_BEFORE_B; + } + const isParameterAForm = parameterA.style === 'form' || !parameterA.style; + const isParameterAFormExplode = + isParameterAForm && + (parameterA.explode || typeof parameterA.explode === 'undefined'); + const isParameterBForm = parameterB.style === 'form' || !parameterB.style; + const isParameterBFormExplode = + isParameterBForm && + (parameterB.explode || typeof parameterB.explode === 'undefined'); + // Sort the form explode to the bottom of the array. + if (isParameterAFormExplode && isParameterBFormExplode) { + throw new OperationError( + this.operation, + 'Ambiguous query parameters, you cannot have 2 form explode parameters', + ); + } + if (isParameterAFormExplode) { + return PLACE_A_AFTER_B; + } + if (isParameterBFormExplode) { + return PLACE_A_BEFORE_B; + } + return EQUAL; + }); for (const [name, parameter] of parameterIterator) { if (!parameter.schema) { throw new OperationError( diff --git a/packages/backend-openapi-utils/src/schema/validation.test.ts b/packages/backend-openapi-utils/src/schema/validation.test.ts index fdb1350625..9c8c4589a3 100644 --- a/packages/backend-openapi-utils/src/schema/validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/validation.test.ts @@ -17,7 +17,7 @@ import { findOperationByRequest, OpenApiProxyValidator } from './validation'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; +import { registerMswTestHooks } from '@backstage/test-utils'; import { CompletedBody, CompletedRequest, CompletedResponse } from 'mockttp'; import withResponseBody from './__fixtures__/schemas/withJsonResponseBody.json'; import withQueryParameter from './__fixtures__/schemas/withQueryParameter.json'; @@ -61,7 +61,7 @@ function createMockttpResponse(response: { } describe('OpenApiProxyValidator', () => { - setupRequestMockHandlers(server); + registerMswTestHooks(server); let validator: OpenApiProxyValidator; async function mockSchema(schema: any) { diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index d93029fa8e..ea329332bd 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -19,14 +19,28 @@ import { Proxy } from './proxy/setup'; const proxy = new Proxy(); -beforeAll(async () => { - await proxy.setup(); -}); +/** + * Setup the proxy hooks for the test suite. This will start the proxy before all tests and stop it after all tests. + * @public + */ +export function setupProxyHooks() { + beforeAll(async () => { + await proxy.setup(); + }); -afterAll(() => { - proxy.stop(); -}); + afterAll(() => { + proxy.stop(); + }); +} +/** + * !!! THIS CURRENTLY ONLY SUPPORTS SUPERTEST !!! + * Setup a server with a custom OpenAPI proxy. This proxy will capture all requests and responses and make sure they + * conform to the spec. + * @param app - express server, needed to ensure we have the correct ports for the proxy. + * @returns - a configured HTTP server that should be used with supertest. + * @public + */ export async function wrapServer(app: Express): Promise { const server = app.listen(proxy.forwardTo.port); await proxy.initialize(`http://localhost:${proxy.forwardTo.port}`, server); diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 6654187526..3b478a2a84 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -38,7 +38,7 @@ import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common/a import { CatalogProcessingOrchestrator } from '../processing/types'; import { z } from 'zod'; import { decodeCursor, encodeCursor } from './util'; -import { wrapServer } from '@backstage/backend-openapi-utils'; +import { setupProxyHooks, wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; @@ -53,6 +53,8 @@ describe('createRouter readonly disabled', () => { let locationAnalyzer: jest.Mocked; let permissionsService: jest.Mocked; + setupProxyHooks(); + beforeAll(async () => { entitiesCatalog = { entities: jest.fn(), diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 5b6900ede1..41b03501a3 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -23,7 +23,7 @@ import { import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; -import { wrapServer } from '@backstage/backend-openapi-utils'; +import { setupProxyHooks, wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, @@ -55,6 +55,8 @@ describe('createRouter', () => { }, }; + setupProxyHooks(); + beforeAll(async () => { const logger = mockServices.logger.mock(); mockSearchEngine = { From 79655cb580b68ceea81f7c35023d9fe39fd8295c Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 18:02:00 -0400 Subject: [PATCH 14/23] use a cjs compatible get-port Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 2 +- yarn.lock | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 04653c5b32..0a302ac27f 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -44,7 +44,7 @@ "express": "^4.17.1", "express-openapi-validator": "^5.0.4", "express-promise-router": "^4.1.0", - "get-port": "^7.1.0", + "get-port": "^5.1.1", "json-schema-to-ts": "^3.0.0", "lodash": "^4.17.21", "mockttp": "^3.13.0", diff --git a/yarn.lock b/yarn.lock index bf267d84fd..a326566b38 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3750,7 +3750,7 @@ __metadata: express: ^4.17.1 express-openapi-validator: ^5.0.4 express-promise-router: ^4.1.0 - get-port: ^7.1.0 + get-port: ^5.1.1 json-schema-to-ts: ^3.0.0 lodash: ^4.17.21 mockttp: ^3.13.0 @@ -28305,13 +28305,6 @@ __metadata: languageName: node linkType: hard -"get-port@npm:^7.1.0": - version: 7.1.0 - resolution: "get-port@npm:7.1.0" - checksum: f4d23b43026124007663a899578cc87ff37bfcf645c5c72651e9810ebafc759857784e409fb8e0ada9b90e5c5db089b0ae2f5f6b49fba1ce2e0aff86094ab17d - languageName: node - linkType: hard - "get-stdin@npm:^9.0.0": version: 9.0.0 resolution: "get-stdin@npm:9.0.0" From 94018ea5a8ef0912a23c166c8b755f7cfdbf082d Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 18:03:29 -0400 Subject: [PATCH 15/23] remove optic files for test cases that we're moving over to new jest plugin Signed-off-by: aramissennyeydd --- plugins/catalog-backend/optic.yml | 15 --------------- plugins/search-backend/optic.yml | 15 --------------- 2 files changed, 30 deletions(-) delete mode 100644 plugins/catalog-backend/optic.yml delete mode 100644 plugins/search-backend/optic.yml diff --git a/plugins/catalog-backend/optic.yml b/plugins/catalog-backend/optic.yml deleted file mode 100644 index dbbdefcf68..0000000000 --- a/plugins/catalog-backend/optic.yml +++ /dev/null @@ -1,15 +0,0 @@ -ruleset: - - breaking-changes -capture: - src/schema/openapi.yaml: - # 🔧 Runnable example with simple get requests. - # Run with "PORT=3000 optic capture src/schema/openapi.yaml --update interactive" in 'plugins/catalog-backend' - # You can change the server and the 'requests' section to experiment - server: - # This will not be used by 'backstage-repo-tools schema openapi test', but may be useful for interactive updates. - url: http://localhost:3000 - requests: - # ℹī¸ Requests should be sent to the Optic proxy, the address of which is injected into 'run.command's env as OPTIC_PROXY (or the value of 'run.proxy_variable'). - run: - # 🔧 Specify a command that will generate traffic - command: yarn backstage-cli package test --no-watch "src/service/router.test.ts" "src/service/createRouter.test.ts" diff --git a/plugins/search-backend/optic.yml b/plugins/search-backend/optic.yml deleted file mode 100644 index 75f20f1bb8..0000000000 --- a/plugins/search-backend/optic.yml +++ /dev/null @@ -1,15 +0,0 @@ -ruleset: - - breaking-changes -capture: - src/schema/openapi.yaml: - # 🔧 Runnable example with simple get requests. - # Run with "PORT=3000 optic capture src/schema/openapi.yaml --update interactive" in 'plugins/search-backend' - # You can change the server and the 'requests' section to experiment - server: - # This will not be used by 'backstage-repo-tools schema openapi test', but may be useful for interactive updates. - url: http://localhost:3000 - requests: - # ℹī¸ Requests should be sent to the Optic proxy, the address of which is injected into 'run.command's env as OPTIC_PROXY (or the value of 'run.proxy_variable'). - run: - # 🔧 Specify a command that will generate traffic - command: yarn backstage-cli package test --no-watch "src/service/router.test.ts" "src/service/createRouter.test.ts" From 395e6b9c3c83c16f2adb4c9260aee7c7741e39de Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 19:05:30 -0400 Subject: [PATCH 16/23] only mention schema changes in the changeset Signed-off-by: aramissennyeydd --- .changeset/eleven-beds-play.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/eleven-beds-play.md b/.changeset/eleven-beds-play.md index 32ae11ff49..fd51922cb4 100644 --- a/.changeset/eleven-beds-play.md +++ b/.changeset/eleven-beds-play.md @@ -1,6 +1,5 @@ --- '@backstage/plugin-search-backend': patch -'@backstage/plugin-catalog-backend': patch --- -Updated to use the improved OpenAPI Jest validation. +Fix to schema to allow arbitrary query parameters. From 0af3a4998b0bc7becb88cd162e352b5a9a507d8c Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 19:05:46 -0400 Subject: [PATCH 17/23] address feedback and double fix schema Signed-off-by: aramissennyeydd --- .../backend-openapi-utils/src/testUtils.ts | 38 +++++++++++-------- .../src/service/createRouter.test.ts | 4 +- .../src/schema/openapi.generated.ts | 15 +++++++- .../search-backend/src/schema/openapi.yaml | 13 ++++++- .../search-backend/src/service/router.test.ts | 4 +- 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index ea329332bd..ebe22fce26 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -17,21 +17,7 @@ import { Express } from 'express'; import { Server } from 'http'; import { Proxy } from './proxy/setup'; -const proxy = new Proxy(); - -/** - * Setup the proxy hooks for the test suite. This will start the proxy before all tests and stop it after all tests. - * @public - */ -export function setupProxyHooks() { - beforeAll(async () => { - await proxy.setup(); - }); - - afterAll(() => { - proxy.stop(); - }); -} +const proxiesToCleanup: Proxy[] = []; /** * !!! THIS CURRENTLY ONLY SUPPORTS SUPERTEST !!! @@ -42,11 +28,33 @@ export function setupProxyHooks() { * @public */ export async function wrapServer(app: Express): Promise { + const proxy = new Proxy(); + await proxy.setup(); const server = app.listen(proxy.forwardTo.port); await proxy.initialize(`http://localhost:${proxy.forwardTo.port}`, server); + return { ...server, address: () => new URL(proxy.url) } as any; } +let registered = false; +function registerHooks() { + if (typeof afterAll !== 'function' || typeof beforeAll !== 'function') { + return; + } + if (registered) { + return; + } + registered = true; + + afterAll(() => { + for (const proxy of proxiesToCleanup) { + proxy.stop(); + } + }); +} + +registerHooks(); + /** * !!! THIS CURRENTLY ONLY SUPPORTS SUPERTEST !!! * Running against supertest, we need some way to hit the optic proxy. This ensures that diff --git a/plugins/catalog-backend/src/service/createRouter.test.ts b/plugins/catalog-backend/src/service/createRouter.test.ts index 3b478a2a84..6654187526 100644 --- a/plugins/catalog-backend/src/service/createRouter.test.ts +++ b/plugins/catalog-backend/src/service/createRouter.test.ts @@ -38,7 +38,7 @@ import { RESOURCE_TYPE_CATALOG_ENTITY } from '@backstage/plugin-catalog-common/a import { CatalogProcessingOrchestrator } from '../processing/types'; import { z } from 'zod'; import { decodeCursor, encodeCursor } from './util'; -import { setupProxyHooks, wrapServer } from '@backstage/backend-openapi-utils'; +import { wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; import { LocationAnalyzer } from '@backstage/plugin-catalog-node'; @@ -53,8 +53,6 @@ describe('createRouter readonly disabled', () => { let locationAnalyzer: jest.Mocked; let permissionsService: jest.Mocked; - setupProxyHooks(); - beforeAll(async () => { entitiesCatalog = { entities: jest.fn(), diff --git a/plugins/search-backend/src/schema/openapi.generated.ts b/plugins/search-backend/src/schema/openapi.generated.ts index 56aaeb2bbe..1e6adfb08f 100644 --- a/plugins/search-backend/src/schema/openapi.generated.ts +++ b/plugins/search-backend/src/schema/openapi.generated.ts @@ -207,8 +207,7 @@ export const spec = { name: 'filters', in: 'query', required: false, - style: 'form', - explode: true, + style: 'deepObject', allowReserved: true, schema: { $ref: '#/components/schemas/JsonObject', @@ -244,6 +243,18 @@ export const spec = { type: 'integer', }, }, + { + name: 'unknown', + in: 'query', + required: false, + style: 'form', + explode: true, + allowReserved: true, + schema: { + type: 'object', + additionalProperties: true, + }, + }, ], }, }, diff --git a/plugins/search-backend/src/schema/openapi.yaml b/plugins/search-backend/src/schema/openapi.yaml index a49b783b64..cbf3db273a 100644 --- a/plugins/search-backend/src/schema/openapi.yaml +++ b/plugins/search-backend/src/schema/openapi.yaml @@ -139,8 +139,7 @@ paths: - name: filters in: query required: false - style: form - explode: true + style: deepObject allowReserved: true schema: # JsonObject is used here instead of the full ZOD schema definition as @@ -167,3 +166,13 @@ paths: allowReserved: true schema: type: integer + - name: unknown + in: query + required: false + # explode form is the equivalent to allow any extra query parameters + style: form + explode: true + allowReserved: true + schema: + type: object + additionalProperties: true diff --git a/plugins/search-backend/src/service/router.test.ts b/plugins/search-backend/src/service/router.test.ts index 41b03501a3..5b6900ede1 100644 --- a/plugins/search-backend/src/service/router.test.ts +++ b/plugins/search-backend/src/service/router.test.ts @@ -23,7 +23,7 @@ import { import express from 'express'; import request from 'supertest'; import { createRouter } from './router'; -import { setupProxyHooks, wrapServer } from '@backstage/backend-openapi-utils'; +import { wrapServer } from '@backstage/backend-openapi-utils'; import { Server } from 'http'; import { mockCredentials, @@ -55,8 +55,6 @@ describe('createRouter', () => { }, }; - setupProxyHooks(); - beforeAll(async () => { const logger = mockServices.logger.mock(); mockSearchEngine = { From 74aa8f8b49bdf3f9e56d179acf868022bfd9b9ad Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 19:16:27 -0400 Subject: [PATCH 18/23] fix api report Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/report.api.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/backend-openapi-utils/report.api.md b/packages/backend-openapi-utils/report.api.md index 5a2c89ee81..f543a9cfb7 100644 --- a/packages/backend-openapi-utils/report.api.md +++ b/packages/backend-openapi-utils/report.api.md @@ -684,9 +684,6 @@ type SchemaRef = Schema extends { [Key in keyof Schema]: SchemaRef; }; -// @public -export function setupProxyHooks(): void; - // @public type TemplateToDocPath< Doc extends PathDoc, From 7869607165df158236c0f436e541c7902f0c0a5e Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 19:16:45 -0400 Subject: [PATCH 19/23] simplify parameter sorting Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/src/index.ts | 6 +- .../src/schema/parameter-validation.ts | 77 +++++++------------ 2 files changed, 30 insertions(+), 53 deletions(-) diff --git a/packages/backend-openapi-utils/src/index.ts b/packages/backend-openapi-utils/src/index.ts index 24f4183311..57d9f755c2 100644 --- a/packages/backend-openapi-utils/src/index.ts +++ b/packages/backend-openapi-utils/src/index.ts @@ -32,8 +32,4 @@ export type { } from './utility'; export type { ApiRouter } from './router'; export { createValidatedOpenApiRouter, getOpenApiSpecRoute } from './stub'; -export { - wrapInOpenApiTestServer, - wrapServer, - setupProxyHooks, -} from './testUtils'; +export { wrapInOpenApiTestServer, wrapServer } from './testUtils'; diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index 90953e6e2a..cbcb098166 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -95,10 +95,6 @@ class BaseParameterParser { } } -const PLACE_A_BEFORE_B = -1; -const PLACE_A_AFTER_B = 1; -const EQUAL = 0; - export class QueryParameterParser extends BaseParameterParser implements RequestParser> @@ -111,51 +107,36 @@ export class QueryParameterParser const remainingQueryParameters = new Set(searchParams.keys()); const queryParameters: Record = {}; - const parameterIterator = Object.entries(this.parameters); + let parameterIterator = Object.entries(this.parameters); + + const isFormExplode = (parameter: ReferencelessParameterObject) => { + return ( + parameter.schema?.type === 'object' && + (parameter.style === 'form' || !parameter.style) && + parameter.explode + ); + }; + + const regularParameters = parameterIterator.filter( + ([_, parameter]) => !isFormExplode(parameter), + ); + + const formExplodeParameters = parameterIterator.filter(([_, parameter]) => + isFormExplode(parameter), + ); + + if (formExplodeParameters.length > 1) { + throw new OperationError( + this.operation, + 'Ambiguous query parameters, you cannot have 2 form explode parameters', + ); + } + + // Sort the parameters so that form explode parameters are processed last. + parameterIterator = [...regularParameters, ...formExplodeParameters]; + + console.log(parameterIterator); - // object parameters with form/explode style should be processed last as they collect all remaining parameters. - parameterIterator.sort(([_, parameterA], [_B, parameterB]) => { - if ( - parameterA.schema.type !== 'object' && - parameterB.schema.type !== 'object' - ) { - return EQUAL; - } - if ( - parameterA.schema.type === 'object' && - parameterB.schema.type !== 'object' - ) { - return PLACE_A_AFTER_B; - } - if ( - parameterA.schema.type !== 'object' && - parameterB.schema.type === 'object' - ) { - return PLACE_A_BEFORE_B; - } - const isParameterAForm = parameterA.style === 'form' || !parameterA.style; - const isParameterAFormExplode = - isParameterAForm && - (parameterA.explode || typeof parameterA.explode === 'undefined'); - const isParameterBForm = parameterB.style === 'form' || !parameterB.style; - const isParameterBFormExplode = - isParameterBForm && - (parameterB.explode || typeof parameterB.explode === 'undefined'); - // Sort the form explode to the bottom of the array. - if (isParameterAFormExplode && isParameterBFormExplode) { - throw new OperationError( - this.operation, - 'Ambiguous query parameters, you cannot have 2 form explode parameters', - ); - } - if (isParameterAFormExplode) { - return PLACE_A_AFTER_B; - } - if (isParameterBFormExplode) { - return PLACE_A_BEFORE_B; - } - return EQUAL; - }); for (const [name, parameter] of parameterIterator) { if (!parameter.schema) { throw new OperationError( From bc92f808c6aeb0a94a5e5a71db1fce8d51eb4727 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 19:19:24 -0400 Subject: [PATCH 20/23] cleanup new proxies Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/src/testUtils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index ebe22fce26..11716050db 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -29,7 +29,9 @@ const proxiesToCleanup: Proxy[] = []; */ export async function wrapServer(app: Express): Promise { const proxy = new Proxy(); + proxiesToCleanup.push(proxy); await proxy.setup(); + const server = app.listen(proxy.forwardTo.port); await proxy.initialize(`http://localhost:${proxy.forwardTo.port}`, server); From 4fe128466155ef29ccc995c5f584f92f92a590a7 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 20:28:47 -0400 Subject: [PATCH 21/23] add backend/test-utils Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 3 +- yarn.lock | 126 ++------------------ 2 files changed, 9 insertions(+), 120 deletions(-) diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 0a302ac27f..2a4cc4372c 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -35,8 +35,8 @@ "dependencies": { "@apidevtools/swagger-parser": "^10.1.0", "@backstage/backend-plugin-api": "workspace:^", - "@backstage/backend-test-utils": "workspace:^", "@backstage/errors": "workspace:^", + "@backstage/test-utils": "workspace:^", "@backstage/types": "workspace:^", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", @@ -53,6 +53,7 @@ "openapi3-ts": "^3.1.2" }, "devDependencies": { + "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", "supertest": "^7.0.0" } diff --git a/yarn.lock b/yarn.lock index a326566b38..5982e6a9bd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3743,6 +3743,7 @@ __metadata: "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/errors": "workspace:^" + "@backstage/test-utils": "workspace:^" "@backstage/types": "workspace:^" "@types/express": ^4.17.6 "@types/express-serve-static-core": ^4.17.5 @@ -10311,16 +10312,7 @@ __metadata: languageName: node linkType: hard -"@httptoolkit/httpolyglot@npm:^2.0.1, @httptoolkit/httpolyglot@npm:^2.1.1": - version: 2.1.1 - resolution: "@httptoolkit/httpolyglot@npm:2.1.1" - dependencies: - "@types/node": ^16.7.10 - checksum: 138ccd61355de334c509e2fc4ac9ade9e1aa6aa770ed2271e0bd1d883ed815eb742d0a4de37837edd03a9a243c05d6da32c5febe970f4518c46e2d76e6ff10d5 - languageName: node - linkType: hard - -"@httptoolkit/httpolyglot@npm:^2.2.1": +"@httptoolkit/httpolyglot@npm:^2.0.1, @httptoolkit/httpolyglot@npm:^2.2.1": version: 2.2.1 resolution: "@httptoolkit/httpolyglot@npm:2.2.1" dependencies: @@ -18283,7 +18275,7 @@ __metadata: languageName: node linkType: hard -"@types/node@npm:^16.11.26, @types/node@npm:^16.7.10": +"@types/node@npm:^16.11.26": version: 16.18.112 resolution: "@types/node@npm:16.18.112" checksum: d634729e60d2e7bd951843fddf5fb59ae786ca707f384e8f90881b011076962b4e3fee3393e3be4be4fb7d86943893e0bda9650aea24ac692bdf407bf1a5d84d @@ -21373,7 +21365,7 @@ __metadata: languageName: node linkType: hard -"async@npm:^2.6.2, async@npm:^2.6.4": +"async@npm:^2.6.4": version: 2.6.4 resolution: "async@npm:2.6.4" dependencies: @@ -22129,13 +22121,6 @@ __metadata: languageName: node linkType: hard -"brotli-wasm@npm:^1.1.0": - version: 1.3.1 - resolution: "brotli-wasm@npm:1.3.1" - checksum: ec2931a989ee6f0bb52c2aabf23a0d230232d3bd69fb68ee3dab9542fc9ae2d4085d0e5338f71520c25a4a26cf1cfc991ce02910c24d63d42c7915c5722a3713 - languageName: node - linkType: hard - "brotli-wasm@npm:^3.0.0": version: 3.0.1 resolution: "brotli-wasm@npm:3.0.1" @@ -24703,7 +24688,7 @@ __metadata: languageName: node linkType: hard -"debug@npm:^3.1.1, debug@npm:^3.2.7": +"debug@npm:^3.2.7": version: 3.2.7 resolution: "debug@npm:3.2.7" dependencies: @@ -25018,15 +25003,6 @@ __metadata: languageName: node linkType: hard -"destroyable-server@npm:^1.0.0": - version: 1.0.0 - resolution: "destroyable-server@npm:1.0.0" - dependencies: - "@types/node": "*" - checksum: ac81b26f616a9d0aaa9cb759fa5a5a186f887025362329f7ddc909f53090f4aea0d1b75c4dda23e210faee536e2d7352de017261e1625b7b18108f0e630efa1f - languageName: node - linkType: hard - "destroyable-server@npm:^1.0.2": version: 1.0.2 resolution: "destroyable-server@npm:1.0.2" @@ -29331,17 +29307,6 @@ __metadata: languageName: node linkType: hard -"http-encoding@npm:^1.5.1": - version: 1.5.1 - resolution: "http-encoding@npm:1.5.1" - dependencies: - brotli-wasm: ^1.1.0 - pify: ^5.0.0 - zstd-codec: ^0.1.4 - checksum: 534aa2facb0ae529fa88b9778867472247711626b90030fd4351572c6147fb5e895d9d2e305e7dc5cc993345f2fbdb17ca99345651bf76dbac39a07f552af2ac - languageName: node - linkType: hard - "http-encoding@npm:^2.0.1": version: 2.0.1 resolution: "http-encoding@npm:2.0.1" @@ -29501,16 +29466,6 @@ __metadata: languageName: node linkType: hard -"http2-wrapper@npm:^2.2.0": - version: 2.2.0 - resolution: "http2-wrapper@npm:2.2.0" - dependencies: - quick-lru: ^5.1.1 - resolve-alpn: ^1.2.0 - checksum: 6fd20e5cb6a58151715b3581e06a62a47df943187d2d1f69e538a50cccb7175dd334ecfde7900a37d18f3e13a1a199518a2c211f39860e81e9a16210c199cfaa - languageName: node - linkType: hard - "http2-wrapper@npm:^2.2.1": version: 2.2.1 resolution: "http2-wrapper@npm:2.2.1" @@ -34561,7 +34516,7 @@ __metadata: languageName: node linkType: hard -"mkdirp@npm:^0.5.1, mkdirp@npm:^0.5.4, mkdirp@npm:^0.5.5, mkdirp@npm:^0.5.6": +"mkdirp@npm:^0.5.1, mkdirp@npm:^0.5.4, mkdirp@npm:^0.5.6": version: 0.5.6 resolution: "mkdirp@npm:0.5.6" dependencies: @@ -34597,7 +34552,7 @@ __metadata: languageName: node linkType: hard -"mockttp@npm:^3.13.0": +"mockttp@npm:^3.13.0, mockttp@npm:^3.9.1": version: 3.15.2 resolution: "mockttp@npm:3.15.2" dependencies: @@ -34649,55 +34604,6 @@ __metadata: languageName: node linkType: hard -"mockttp@npm:^3.9.1": - version: 3.9.4 - resolution: "mockttp@npm:3.9.4" - dependencies: - "@graphql-tools/schema": ^8.5.0 - "@graphql-tools/utils": ^8.8.0 - "@httptoolkit/httpolyglot": ^2.1.1 - "@httptoolkit/subscriptions-transport-ws": ^0.11.2 - "@httptoolkit/websocket-stream": ^6.0.1 - "@types/cors": ^2.8.6 - "@types/node": "*" - base64-arraybuffer: ^0.1.5 - body-parser: ^1.15.2 - cacheable-lookup: ^6.0.0 - common-tags: ^1.8.0 - connect: ^3.7.0 - cors: ^2.8.4 - cors-gate: ^1.1.3 - cross-fetch: ^3.1.5 - destroyable-server: ^1.0.0 - express: ^4.14.0 - graphql: ^14.0.2 || ^15.5 - graphql-http: ^1.22.0 - graphql-subscriptions: ^1.1.0 - graphql-tag: ^2.12.6 - http-encoding: ^1.5.1 - http2-wrapper: ^2.2.0 - https-proxy-agent: ^5.0.1 - isomorphic-ws: ^4.0.1 - lodash: ^4.16.4 - lru-cache: ^7.14.0 - native-duplexpair: ^1.0.0 - node-forge: ^1.2.1 - pac-proxy-agent: ^7.0.0 - parse-multipart-data: ^1.4.0 - performance-now: ^2.1.0 - portfinder: 1.0.28 - read-tls-client-hello: ^1.0.0 - semver: ^7.5.3 - socks-proxy-agent: ^7.0.0 - typed-error: ^3.0.2 - uuid: ^8.3.2 - ws: ^8.8.0 - bin: - mockttp: dist/admin/admin-bin.js - checksum: 2e0b984d77a94e6a754e44c85a7ff2ded13ba42fd6cabf125b677a8a57eff543c896bf3ecb522799d3efbe18733bf019fbe707044f098fdd5e8e4bc2c0b1df4f - languageName: node - linkType: hard - "module-details-from-path@npm:^1.0.3": version: 1.0.3 resolution: "module-details-from-path@npm:1.0.3" @@ -37203,17 +37109,6 @@ __metadata: languageName: node linkType: hard -"portfinder@npm:1.0.28": - version: 1.0.28 - resolution: "portfinder@npm:1.0.28" - dependencies: - async: ^2.6.2 - debug: ^3.1.1 - mkdirp: ^0.5.5 - checksum: 91fef602f13f8f4c64385d0ad2a36cc9dc6be0b8d10a2628ee2c3c7b9917ab4fefb458815b82cea2abf4b785cd11c9b4e2d917ac6fa06f14b6fa880ca8f8928c - languageName: node - linkType: hard - "portfinder@npm:^1.0.28, portfinder@npm:^1.0.32": version: 1.0.32 resolution: "portfinder@npm:1.0.32" @@ -45373,13 +45268,6 @@ __metadata: languageName: node linkType: hard -"zstd-codec@npm:^0.1.4": - version: 0.1.4 - resolution: "zstd-codec@npm:0.1.4" - checksum: 8689bc0defc4f387d1be990b8b8ca8ca56690d17dfc8dd4703db798465b92a21e64e54e886acfaa376147d9d07d879a68627b09fddc34a0c93f0dc5c610a790c - languageName: node - linkType: hard - "zstd-codec@npm:^0.1.5": version: 0.1.5 resolution: "zstd-codec@npm:0.1.5" From 7263f925b4c803a25e7779ae75ac3121801bf880 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Tue, 24 Sep 2024 20:29:18 -0400 Subject: [PATCH 22/23] remove unused import Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 3 +-- yarn.lock | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 2a4cc4372c..6c391e8f70 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -36,7 +36,6 @@ "@apidevtools/swagger-parser": "^10.1.0", "@backstage/backend-plugin-api": "workspace:^", "@backstage/errors": "workspace:^", - "@backstage/test-utils": "workspace:^", "@backstage/types": "workspace:^", "@types/express": "^4.17.6", "@types/express-serve-static-core": "^4.17.5", @@ -53,8 +52,8 @@ "openapi3-ts": "^3.1.2" }, "devDependencies": { - "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", + "@backstage/test-utils": "workspace:^", "supertest": "^7.0.0" } } diff --git a/yarn.lock b/yarn.lock index 5982e6a9bd..880145f6bc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3740,7 +3740,6 @@ __metadata: dependencies: "@apidevtools/swagger-parser": ^10.1.0 "@backstage/backend-plugin-api": "workspace:^" - "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/test-utils": "workspace:^" From 06490f8c1e57157a47bb1480391c94524e4f7f87 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 8 Oct 2024 10:17:51 +0200 Subject: [PATCH 23/23] Update packages/backend-openapi-utils/src/schema/parameter-validation.ts Signed-off-by: Patrik Oldsberg --- .../backend-openapi-utils/src/schema/parameter-validation.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/backend-openapi-utils/src/schema/parameter-validation.ts b/packages/backend-openapi-utils/src/schema/parameter-validation.ts index cbcb098166..7a61578850 100644 --- a/packages/backend-openapi-utils/src/schema/parameter-validation.ts +++ b/packages/backend-openapi-utils/src/schema/parameter-validation.ts @@ -135,8 +135,6 @@ export class QueryParameterParser // Sort the parameters so that form explode parameters are processed last. parameterIterator = [...regularParameters, ...formExplodeParameters]; - console.log(parameterIterator); - for (const [name, parameter] of parameterIterator) { if (!parameter.schema) { throw new OperationError(