From 0939f56fd17f81e6a90fd57c1d0ea0a0576ed610 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Jan 2023 17:17:03 +0100 Subject: [PATCH 01/25] backend-app-api: move http router factory implementation into subfolder Signed-off-by: Patrik Oldsberg --- .../httpRouterFactory.test.ts} | 2 +- .../httpRouterFactory.ts} | 0 .../implementations/httpRouter/index.ts | 18 ++++++++++++++++++ .../src/services/implementations/index.ts | 4 ++-- 4 files changed, 21 insertions(+), 3 deletions(-) rename packages/backend-app-api/src/services/implementations/{httpRouterService.test.ts => httpRouter/httpRouterFactory.test.ts} (97%) rename packages/backend-app-api/src/services/implementations/{httpRouterService.ts => httpRouter/httpRouterFactory.ts} (100%) create mode 100644 packages/backend-app-api/src/services/implementations/httpRouter/index.ts diff --git a/packages/backend-app-api/src/services/implementations/httpRouterService.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterFactory.test.ts similarity index 97% rename from packages/backend-app-api/src/services/implementations/httpRouterService.test.ts rename to packages/backend-app-api/src/services/implementations/httpRouter/httpRouterFactory.test.ts index 89f6db2ac8..7717f4116e 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouterService.test.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterFactory.test.ts @@ -18,7 +18,7 @@ import { HttpRouterService, ServiceFactory, } from '@backstage/backend-plugin-api'; -import { httpRouterFactory } from './httpRouterService'; +import { httpRouterFactory } from './httpRouterFactory'; describe('httpRouterFactory', () => { it('should register plugin paths', async () => { diff --git a/packages/backend-app-api/src/services/implementations/httpRouterService.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterFactory.ts similarity index 100% rename from packages/backend-app-api/src/services/implementations/httpRouterService.ts rename to packages/backend-app-api/src/services/implementations/httpRouter/httpRouterFactory.ts diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/index.ts b/packages/backend-app-api/src/services/implementations/httpRouter/index.ts new file mode 100644 index 0000000000..f2db660414 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/httpRouter/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2023 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. + */ + +export { httpRouterFactory } from './httpRouterFactory'; +export type { HttpRouterFactoryOptions } from './httpRouterFactory'; diff --git a/packages/backend-app-api/src/services/implementations/index.ts b/packages/backend-app-api/src/services/implementations/index.ts index 2365ff36f4..de5350e72d 100644 --- a/packages/backend-app-api/src/services/implementations/index.ts +++ b/packages/backend-app-api/src/services/implementations/index.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +export * from './httpRouter'; + export { cacheFactory } from './cacheService'; export { configFactory } from './configService'; export { databaseFactory } from './databaseService'; @@ -24,9 +26,7 @@ export { permissionsFactory } from './permissionsService'; export { schedulerFactory } from './schedulerService'; export { tokenManagerFactory } from './tokenManagerService'; export { urlReaderFactory } from './urlReaderService'; -export { httpRouterFactory } from './httpRouterService'; export { rootHttpRouterFactory } from './rootHttpRouterService'; export { lifecycleFactory } from './lifecycleService'; export { rootLifecycleFactory } from './rootLifecycleService'; -export type { HttpRouterFactoryOptions } from './httpRouterService'; export type { RootHttpRouterFactoryOptions } from './rootHttpRouterService'; From 6eb86a13af4a32245714e77c7e19842aafcf0f6b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Jan 2023 17:18:42 +0100 Subject: [PATCH 02/25] backend-app-api: move rootHttpRouter into subfolder Signed-off-by: Patrik Oldsberg --- .../src/services/implementations/index.ts | 3 +-- .../implementations/rootHttpRouter/index.ts | 18 ++++++++++++++++++ .../rootHttpRouterFactory.test.ts} | 2 +- .../rootHttpRouterFactory.ts} | 0 4 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts rename packages/backend-app-api/src/services/implementations/{rootHttpRouterService.test.ts => rootHttpRouter/rootHttpRouterFactory.test.ts} (95%) rename packages/backend-app-api/src/services/implementations/{rootHttpRouterService.ts => rootHttpRouter/rootHttpRouterFactory.ts} (100%) diff --git a/packages/backend-app-api/src/services/implementations/index.ts b/packages/backend-app-api/src/services/implementations/index.ts index de5350e72d..73c3418090 100644 --- a/packages/backend-app-api/src/services/implementations/index.ts +++ b/packages/backend-app-api/src/services/implementations/index.ts @@ -15,6 +15,7 @@ */ export * from './httpRouter'; +export * from './rootHttpRouter'; export { cacheFactory } from './cacheService'; export { configFactory } from './configService'; @@ -26,7 +27,5 @@ export { permissionsFactory } from './permissionsService'; export { schedulerFactory } from './schedulerService'; export { tokenManagerFactory } from './tokenManagerService'; export { urlReaderFactory } from './urlReaderService'; -export { rootHttpRouterFactory } from './rootHttpRouterService'; export { lifecycleFactory } from './lifecycleService'; export { rootLifecycleFactory } from './rootLifecycleService'; -export type { RootHttpRouterFactoryOptions } from './rootHttpRouterService'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts new file mode 100644 index 0000000000..9b72f17060 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2023 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. + */ + +export { rootHttpRouterFactory } from './rootHttpRouterFactory'; +export type { RootHttpRouterFactoryOptions } from './rootHttpRouterFactory'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts similarity index 95% rename from packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts rename to packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts index fea99b0df9..67b5066fb1 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { findConflictingPath } from './rootHttpRouterService'; +import { findConflictingPath } from './rootHttpRouterFactory'; describe('findConflictingPath', () => { it('finds conflicts when present', () => { diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts similarity index 100% rename from packages/backend-app-api/src/services/implementations/rootHttpRouterService.ts rename to packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts From b77786901197004a7bb3431be0ba6773730ebda4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Jan 2023 17:40:45 +0100 Subject: [PATCH 03/25] backend-app-api: extract RestrictedIndexedRouter out of root router Signed-off-by: Patrik Oldsberg --- .../RestrictedIndexedRouter.test.ts | 51 +++++++++++++ .../rootHttpRouter/RestrictedIndexedRouter.ts | 73 +++++++++++++++++++ .../rootHttpRouterFactory.test.ts | 31 -------- .../rootHttpRouter/rootHttpRouterFactory.ts | 54 ++------------ 4 files changed, 130 insertions(+), 79 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.test.ts create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.ts delete mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.test.ts new file mode 100644 index 0000000000..b72e87f77b --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.test.ts @@ -0,0 +1,51 @@ +/* + * Copyright 2022 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 { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; + +describe('RestrictedIndexedRouter', () => { + it.each([ + [['/b'], '/a'], + [['/a'], '/aa/b'], + [['/aa'], '/a/b'], + [['/a/b'], '/aa'], + [['/b/a'], '/a'], + [['/a'], '/aa'], + ])(`with existing paths %s, adds %s without conflict`, (existing, added) => { + const router = new RestrictedIndexedRouter(false); + for (const path of existing) { + router.use(path, () => {}); + } + expect(() => router.use(added, () => {})).not.toThrow(); + }); + + it.each([ + [['/a'], '/a', '/a'], + [['/a'], '/a/b', '/a'], + [['/a/b'], '/a', '/a/b'], + ])( + `find conflict when existing paths %s, adds %s`, + (existing, added, conflict) => { + const router = new RestrictedIndexedRouter(false); + for (const path of existing) { + router.use(path, () => {}); + } + expect(() => router.use(added, () => {})).toThrow( + `Path ${added} conflicts with the existing path ${conflict}`, + ); + }, + ); +}); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.ts new file mode 100644 index 0000000000..961277f34d --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/RestrictedIndexedRouter.ts @@ -0,0 +1,73 @@ +/* + * Copyright 2023 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 { RootHttpRouterService } from '@backstage/backend-plugin-api'; +import { Handler, Router } from 'express'; + +function normalizePath(path: string): string { + return path.replace(/\/*$/, '/'); +} + +export class RestrictedIndexedRouter implements RootHttpRouterService { + #indexPath?: false | string; + + #router = Router(); + #namedRoutes = Router(); + #indexRouter = Router(); + #existingPaths = new Array(); + + constructor(indexPath?: false | string) { + this.#indexPath = indexPath; + this.#router.use(this.#namedRoutes); + this.#router.use(this.#indexRouter); + } + + use(path: string, handler: Handler) { + if (path.match(/^[/\s]*$/)) { + throw new Error(`Root router path may not be empty`); + } + const conflictingPath = this.#findConflictingPath(path); + if (conflictingPath) { + throw new Error( + `Path ${path} conflicts with the existing path ${conflictingPath}`, + ); + } + this.#existingPaths.push(path); + this.#namedRoutes.use(path, handler); + + if (this.#indexPath === path) { + this.#indexRouter.use(handler); + } + } + + handler(): Handler { + return this.#router; + } + + #findConflictingPath(newPath: string): string | undefined { + const normalizedNewPath = normalizePath(newPath); + for (const path of this.#existingPaths) { + const normalizedPath = normalizePath(path); + if (normalizedPath.startsWith(normalizedNewPath)) { + return path; + } + if (normalizedNewPath.startsWith(normalizedPath)) { + return path; + } + } + return undefined; + } +} diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts deleted file mode 100644 index 67b5066fb1..0000000000 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2022 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 { findConflictingPath } from './rootHttpRouterFactory'; - -describe('findConflictingPath', () => { - it('finds conflicts when present', () => { - expect(findConflictingPath(['/a'], '/a')).toBe('/a'); - expect(findConflictingPath(['/b'], '/a')).toBe(undefined); - expect(findConflictingPath(['/a'], '/a/b')).toBe('/a'); - expect(findConflictingPath(['/a'], '/aa/b')).toBe(undefined); - expect(findConflictingPath(['/aa'], '/a/b')).toBe(undefined); - expect(findConflictingPath(['/a/b'], '/a')).toBe('/a/b'); - expect(findConflictingPath(['/a/b'], '/aa')).toBe(undefined); - expect(findConflictingPath(['/b/a'], '/a')).toBe(undefined); - expect(findConflictingPath(['/a'], '/aa')).toBe(undefined); - }); -}); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index dae7474793..ffe345f0a0 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -18,9 +18,9 @@ import { createServiceFactory, coreServices, } from '@backstage/backend-plugin-api'; -import Router from 'express-promise-router'; import { Handler } from 'express'; import { createServiceBuilder } from '@backstage/backend-common'; +import { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; /** * @public @@ -45,10 +45,9 @@ export const rootHttpRouterFactory = createServiceFactory({ lifecycle: coreServices.rootLifecycle, }, async factory({ config, lifecycle }, options?: RootHttpRouterFactoryOptions) { - const indexPath = options?.indexPath ?? '/api/app'; - - const namedRouter = Router(); - const indexRouter = Router(); + const router = new RestrictedIndexedRouter( + options?.indexPath ?? '/api/app', + ); const service = createServiceBuilder(module).loadConfig(config); @@ -56,7 +55,7 @@ export const rootHttpRouterFactory = createServiceFactory({ service.addRouter('', middleware); } - service.addRouter('', namedRouter).addRouter('', indexRouter); + service.addRouter('', router.handler()); const server = await service.start(); // Stop method isn't part of the public API, let's fix that once we move the implementation here. @@ -79,47 +78,6 @@ export const rootHttpRouterFactory = createServiceFactory({ labels: { service: 'rootHttpRouter' }, }); - const existingPaths = new Array(); - - return { - use: (path: string, handler: Handler) => { - if (path.match(/^[/\s]*$/)) { - throw new Error(`Root router path may not be empty`); - } - const conflictingPath = findConflictingPath(existingPaths, path); - if (conflictingPath) { - throw new Error( - `Path ${path} conflicts with the existing path ${conflictingPath}`, - ); - } - existingPaths.push(path); - namedRouter.use(path, handler); - - if (indexPath === path) { - indexRouter.use(handler); - } - }, - }; + return router; }, }); - -function normalizePath(path: string): string { - return path.replace(/\/*$/, '/'); -} - -export function findConflictingPath( - paths: string[], - newPath: string, -): string | undefined { - const normalizedNewPath = normalizePath(newPath); - for (const path of paths) { - const normalizedPath = normalizePath(path); - if (normalizedPath.startsWith(normalizedNewPath)) { - return path; - } - if (normalizedNewPath.startsWith(normalizedPath)) { - return path; - } - } - return undefined; -} From 7695c5a44c35619e14353b8cb2cf489ff8a837dd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Jan 2023 15:09:30 +0100 Subject: [PATCH 04/25] backend-app-api: forklift sevrice factory config from backend-common Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 11 ++++++++++- .../implementations/rootHttpRouter}/config.test.ts | 0 .../implementations/rootHttpRouter}/config.ts | 0 .../implementations/rootHttpRouter}/hostFactory.ts | 0 yarn.lock | 9 +++++++++ 5 files changed, 19 insertions(+), 1 deletion(-) rename packages/{backend-common/src/service/lib => backend-app-api/src/services/implementations/rootHttpRouter}/config.test.ts (100%) rename packages/{backend-common/src/service/lib => backend-app-api/src/services/implementations/rootHttpRouter}/config.ts (100%) rename packages/{backend-common/src/service/lib => backend-app-api/src/services/implementations/rootHttpRouter}/hostFactory.ts (100%) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 80598ec93d..20e196c4b6 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -36,15 +36,24 @@ "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/backend-tasks": "workspace:^", + "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/plugin-permission-node": "workspace:^", + "@types/cors": "^2.8.6", "@types/express": "^4.17.6", + "cors": "^2.8.5", "express": "^4.17.1", "express-promise-router": "^4.1.0", + "fs-extra": "10.1.0", + "minimatch": "^5.0.0", + "node-forge": "^1.3.1", + "selfsigned": "^2.0.0", "winston": "^3.2.1" }, "devDependencies": { - "@backstage/cli": "workspace:^" + "@backstage/cli": "workspace:^", + "@types/fs-extra": "^9.0.3", + "@types/node-forge": "^1.3.0" }, "files": [ "dist", diff --git a/packages/backend-common/src/service/lib/config.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.test.ts similarity index 100% rename from packages/backend-common/src/service/lib/config.test.ts rename to packages/backend-app-api/src/services/implementations/rootHttpRouter/config.test.ts diff --git a/packages/backend-common/src/service/lib/config.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts similarity index 100% rename from packages/backend-common/src/service/lib/config.ts rename to packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts diff --git a/packages/backend-common/src/service/lib/hostFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/hostFactory.ts similarity index 100% rename from packages/backend-common/src/service/lib/hostFactory.ts rename to packages/backend-app-api/src/services/implementations/rootHttpRouter/hostFactory.ts diff --git a/yarn.lock b/yarn.lock index 588d41c776..ff351c8957 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3381,11 +3381,20 @@ __metadata: "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-tasks": "workspace:^" "@backstage/cli": "workspace:^" + "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/plugin-permission-node": "workspace:^" + "@types/cors": ^2.8.6 "@types/express": ^4.17.6 + "@types/fs-extra": ^9.0.3 + "@types/node-forge": ^1.3.0 + cors: ^2.8.5 express: ^4.17.1 express-promise-router: ^4.1.0 + fs-extra: 10.1.0 + minimatch: ^5.0.0 + node-forge: ^1.3.1 + selfsigned: ^2.0.0 winston: ^3.2.1 languageName: unknown linkType: soft From 31f20f50297c3834da67542c19ee0c5106aaad2c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Jan 2023 15:10:47 +0100 Subject: [PATCH 05/25] backend-app-api: move http options reading to lib + refactor Signed-off-by: Patrik Oldsberg --- .../backend-app-api/src/lib/http/config.ts | 94 ++++++++++++++++++ .../backend-app-api/src/lib/http/index.ts | 18 ++++ .../backend-app-api/src/lib/http/types.ts | 46 +++++++++ .../implementations/rootHttpRouter/config.ts | 99 ------------------- 4 files changed, 158 insertions(+), 99 deletions(-) create mode 100644 packages/backend-app-api/src/lib/http/config.ts create mode 100644 packages/backend-app-api/src/lib/http/index.ts create mode 100644 packages/backend-app-api/src/lib/http/types.ts diff --git a/packages/backend-app-api/src/lib/http/config.ts b/packages/backend-app-api/src/lib/http/config.ts new file mode 100644 index 0000000000..933bab42a0 --- /dev/null +++ b/packages/backend-app-api/src/lib/http/config.ts @@ -0,0 +1,94 @@ +/* + * Copyright 2023 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 { Config } from '@backstage/config'; +import { HttpServerOptions } from './types'; + +const DEFAULT_PORT = 7007; +const DEFAULT_HOST = ''; + +/** + * Reads {@link HttpServerOptions} from a {@link @backstage/config#Config} object. + * + * @public + * @remarks + * + * The provided configuration object should contain the `listen` and + * additional keys directly. + * + * @example + * ```ts + * const opts = readHttpServerOptions(config.getConfig('backend')); + * ``` + */ +export function readHttpServerOptions(config: Config): HttpServerOptions { + return { + listen: readHttpListenOptions(config), + https: readHttpsOptions(config), + }; +} + +function readHttpListenOptions(config: Config): HttpServerOptions['listen'] { + const listen = config.get('listen'); + if (typeof listen === 'string') { + const parts = listen.split(':'); + const port = parseInt(parts[parts.length - 1], 10); + if (!isNaN(port)) { + if (parts.length === 1) { + return { port, host: DEFAULT_HOST }; + } + if (parts.length === 2) { + return { host: parts[0], port }; + } + } + throw new Error( + `Unable to parse listen address ${listen}, expected or :`, + ); + } + + return { + port: config.getOptionalNumber('listen.port') ?? DEFAULT_PORT, + host: config.getOptionalString('listen.host') ?? DEFAULT_HOST, + }; +} + +function readHttpsOptions(config: Config): HttpServerOptions['https'] { + const https = config.getOptional('https'); + if (https === true) { + const baseUrl = config.getString('baseUrl'); + let hostname; + try { + hostname = new URL(baseUrl).hostname; + } catch (error) { + throw new Error(`Invalid baseUrl "${baseUrl}"`); + } + + return { certificate: { type: 'generated', hostname } }; + } + + const cc = config.getOptionalConfig('https'); + if (!cc) { + return undefined; + } + + return { + certificate: { + type: 'plain', + cert: cc.getString('certificate.cert'), + key: cc.getString('certificate.key'), + }, + }; +} diff --git a/packages/backend-app-api/src/lib/http/index.ts b/packages/backend-app-api/src/lib/http/index.ts new file mode 100644 index 0000000000..0717194f75 --- /dev/null +++ b/packages/backend-app-api/src/lib/http/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2023 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. + */ + +export { readHttpServerOptions } from './config'; +export type { HttpServerOptions, HttpServerCertificateOptions } from './types'; diff --git a/packages/backend-app-api/src/lib/http/types.ts b/packages/backend-app-api/src/lib/http/types.ts new file mode 100644 index 0000000000..a33415ec83 --- /dev/null +++ b/packages/backend-app-api/src/lib/http/types.ts @@ -0,0 +1,46 @@ +/* + * Copyright 2023 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. + */ + +/** + * Options for starting up an HTTP server. + * + * @public + */ +export type HttpServerOptions = { + listen: { + port: number; + host: string; + }; + https?: { + certificate: HttpServerCertificateOptions; + }; +}; + +/** + * Options for configuring HTTPS for an HTTP server. + * + * @public + */ +export type HttpServerCertificateOptions = + | { + type: 'plain'; + key: string; + cert: string; + } + | { + type: 'generated'; + hostname: string; + }; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts index 93cc12b7d4..a147b67ea1 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts @@ -52,49 +52,6 @@ type CustomOrigin = ( callback: (err: Error | null, origin?: StaticOrigin) => void, ) => void; -/** - * Reads some base options out of a config object. - * - * @param config - The root of a backend config object - * @returns A base options object - * - * @example - * ```json - * { - * baseUrl: "http://localhost:7007", - * listen: "0.0.0.0:7007" - * } - * ``` - */ -export function readBaseOptions(config: Config): BaseOptions { - if (typeof config.get('listen') === 'string') { - // TODO(freben): Expand this to support more addresses and perhaps optional - const { host, port } = parseListenAddress(config.getString('listen')); - - return removeUnknown({ - listenPort: port, - listenHost: host, - }); - } - - const port = config.getOptional('listen.port'); - if ( - typeof port !== 'undefined' && - typeof port !== 'number' && - typeof port !== 'string' - ) { - throw new Error( - `Invalid type in config for key 'backend.listen.port', got ${typeof port}, wanted string or number`, - ); - } - - return removeUnknown({ - listenPort: port, - listenHost: config.getOptionalString('listen.host'), - baseUrl: config.getOptionalString('baseUrl'), - }); -} - /** * Attempts to read a CORS options object from the root of a config object. * @@ -165,49 +122,6 @@ export function readCspOptions( return result; } -/** - * Attempts to read a https settings object from the root of a config object. - * - * @param config - The root of a backend config object - * @returns A https settings object, or undefined if not specified - * - * @example - * ```json - * { - * https: { - * certificate: ... - * } - * } - * ``` - */ -export function readHttpsSettings(config: Config): HttpsSettings | undefined { - const https = config.getOptional('https'); - if (https === true) { - const baseUrl = config.getString('baseUrl'); - let hostname; - try { - hostname = new URL(baseUrl).hostname; - } catch (error) { - throw new Error(`Invalid backend.baseUrl "${baseUrl}"`); - } - - return { certificate: { hostname } }; - } - - const cc = config.getOptionalConfig('https'); - if (!cc) { - return undefined; - } - - const certificateConfig = cc.get('certificate'); - - const cfg = { - certificate: certificateConfig, - }; - - return removeUnknown(cfg as HttpsSettings); -} - function getOptionalStringOrStrings( config: Config, key: string, @@ -269,16 +183,3 @@ function removeUnknown(obj: T): T { Object.entries(obj).filter(([, v]) => v !== undefined), ) as T; } - -function parseListenAddress(value: string): { host?: string; port?: number } { - const parts = value.split(':'); - if (parts.length === 1) { - return { port: parseInt(parts[0], 10) }; - } - if (parts.length === 2) { - return { host: parts[0], port: parseInt(parts[1], 10) }; - } - throw new Error( - `Unable to parse listen address ${value}, expected or :`, - ); -} From 64f40724d5e9a9caf084d846da8236eeab226b56 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Jan 2023 16:14:59 +0100 Subject: [PATCH 06/25] backend-app-api: tests and fixes for readHttpServerOptions Signed-off-by: Patrik Oldsberg --- .../src/lib/http/config.test.ts | 81 +++++++++++++++++++ .../backend-app-api/src/lib/http/config.ts | 13 ++- 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 packages/backend-app-api/src/lib/http/config.test.ts diff --git a/packages/backend-app-api/src/lib/http/config.test.ts b/packages/backend-app-api/src/lib/http/config.test.ts new file mode 100644 index 0000000000..3ab5eda5e9 --- /dev/null +++ b/packages/backend-app-api/src/lib/http/config.test.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2020 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 { ConfigReader } from '@backstage/config'; +import { readHttpServerOptions } from './config'; + +describe('readHttpServerOptions', () => { + it.each([ + [{}, { listen: { host: '', port: 7007 } }], + [{ listen: ':80' }, { listen: { host: '', port: 80 } }], + [{ listen: '80' }, { listen: { host: '', port: 80 } }], + [{ listen: '1.2.3.4:80' }, { listen: { host: '1.2.3.4', port: 80 } }], + [{ listen: { host: '' } }, { listen: { host: '', port: 7007 } }], + [ + { listen: { host: '0.0.0.0' } }, + { listen: { host: '0.0.0.0', port: 7007 } }, + ], + [ + { listen: { host: '0.0.0.0', port: '80' } }, + { listen: { host: '0.0.0.0', port: 80 } }, + ], + [{ listen: { port: '80' } }, { listen: { host: '', port: 80 } }], + [{ listen: { port: 80 } }, { listen: { host: '', port: 80 } }], + [{ listen: { port: 80 } }, { listen: { host: '', port: 80 } }], + [ + { baseUrl: 'http://example.com:8080', https: true }, + { + listen: { host: '', port: 7007 }, + https: { certificate: { type: 'generated', hostname: 'example.com' } }, + }, + ], + [ + { https: { certificate: { cert: 'my-cert', key: 'my-key' } } }, + { + listen: { host: '', port: 7007 }, + https: { + certificate: { type: 'plain', cert: 'my-cert', key: 'my-key' }, + }, + }, + ], + ])('should read http server options %#', (input, output) => { + expect(readHttpServerOptions(new ConfigReader(input))).toEqual(output); + }); + + it.each([ + [ + { listen: { port: 'not-a-number' } }, + "Unable to convert config value for key 'listen.port' in 'mock-config' to a number", + ], + [ + { listen: { port: {} } }, + "Invalid type in config for key 'listen.port' in 'mock-config', got object, wanted number", + ], + [ + { listen: { host: false } }, + "Invalid type in config for key 'listen.host' in 'mock-config', got boolean, wanted string", + ], + [{ https: {} }, "Missing required config value at 'https.certificate.cert"], + [ + { https: { certificate: { cert: 'x' } } }, + "Missing required config value at 'https.certificate.key", + ], + ])('should throw on bad options %#', (input, message) => { + expect(() => readHttpServerOptions(new ConfigReader(input))).toThrow( + message, + ); + }); +}); diff --git a/packages/backend-app-api/src/lib/http/config.ts b/packages/backend-app-api/src/lib/http/config.ts index 933bab42a0..3c5acf0ffd 100644 --- a/packages/backend-app-api/src/lib/http/config.ts +++ b/packages/backend-app-api/src/lib/http/config.ts @@ -42,9 +42,9 @@ export function readHttpServerOptions(config: Config): HttpServerOptions { } function readHttpListenOptions(config: Config): HttpServerOptions['listen'] { - const listen = config.get('listen'); + const listen = config.getOptional('listen'); if (typeof listen === 'string') { - const parts = listen.split(':'); + const parts = String(listen).split(':'); const port = parseInt(parts[parts.length - 1], 10); if (!isNaN(port)) { if (parts.length === 1) { @@ -59,9 +59,16 @@ function readHttpListenOptions(config: Config): HttpServerOptions['listen'] { ); } + // Workaround to allow empty string + const host = config.getOptional('listen.host') ?? DEFAULT_HOST; + if (typeof host !== 'string') { + config.getOptionalString('listen.host'); // will throw + throw new Error('unreachable'); + } + return { port: config.getOptionalNumber('listen.port') ?? DEFAULT_PORT, - host: config.getOptionalString('listen.host') ?? DEFAULT_HOST, + host, }; } From 950e0392aac27b5d4a31317cea99fe2157b5c8ca Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 4 Jan 2023 17:52:38 +0100 Subject: [PATCH 07/25] backend-app-api: refactor hostFactory into createHttpServer Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 4 +- .../src/lib/http/createHttpServer.ts | 98 +++++++++++++++++ .../http/getGeneratedCertificate.ts} | 102 ++++-------------- .../backend-app-api/src/lib/http/types.ts | 15 +++ yarn.lock | 2 + 5 files changed, 137 insertions(+), 84 deletions(-) create mode 100644 packages/backend-app-api/src/lib/http/createHttpServer.ts rename packages/backend-app-api/src/{services/implementations/rootHttpRouter/hostFactory.ts => lib/http/getGeneratedCertificate.ts} (55%) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 20e196c4b6..20c723bd51 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -48,12 +48,14 @@ "minimatch": "^5.0.0", "node-forge": "^1.3.1", "selfsigned": "^2.0.0", + "stoppable": "^1.1.0", "winston": "^3.2.1" }, "devDependencies": { "@backstage/cli": "workspace:^", "@types/fs-extra": "^9.0.3", - "@types/node-forge": "^1.3.0" + "@types/node-forge": "^1.3.0", + "@types/stoppable": "^1.1.0" }, "files": [ "dist", diff --git a/packages/backend-app-api/src/lib/http/createHttpServer.ts b/packages/backend-app-api/src/lib/http/createHttpServer.ts new file mode 100644 index 0000000000..f72269c079 --- /dev/null +++ b/packages/backend-app-api/src/lib/http/createHttpServer.ts @@ -0,0 +1,98 @@ +/* + * Copyright 2020 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 http from 'http'; +import * as https from 'https'; +import stoppableServer from 'stoppable'; +import { RequestListener } from 'http'; +import { LoggerService } from '@backstage/backend-plugin-api'; +import { HttpServerOptions, ExtendedHttpServer } from './types'; +import { getGeneratedCertificate } from './getGeneratedCertificate'; + +/** + * Creates a Node.js HTTP or HTTPS server instance. + * + * @public + */ +export async function createHttpServer( + listener: RequestListener, + options: HttpServerOptions, + deps: { logger: LoggerService }, +): Promise { + const server = await createServer(listener, options, deps); + + const stopper = stoppableServer(server, 0); + + return Object.assign(server, { + start() { + return new Promise((resolve, reject) => { + const handleStartupError = (error: Error) => { + server.close(); + reject(error); + }; + + server.on('error', handleStartupError); + + const { host, port } = options.listen; + server.listen(port, host, () => { + server.off('error', handleStartupError); + deps.logger.info(`Listening on ${host}:${port}`); + resolve(); + }); + }); + }, + + stop() { + return new Promise((resolve, reject) => { + stopper.stop((error?: Error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); + }, + + port() { + const address = server.address(); + if (typeof address === 'string' || address === null) { + throw new Error(`Unexpected server address '${address}'`); + } + return address.port; + }, + }); +} + +async function createServer( + listener: RequestListener, + options: HttpServerOptions, + deps: { logger: LoggerService }, +): Promise { + if (options.https) { + const { certificate } = options.https; + if (certificate.type === 'generated') { + const credentials = await getGeneratedCertificate( + certificate.hostname, + deps.logger, + ); + return https.createServer(credentials, listener); + } + return https.createServer(certificate, listener); + } + + return http.createServer(listener); +} diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/hostFactory.ts b/packages/backend-app-api/src/lib/http/getGeneratedCertificate.ts similarity index 55% rename from packages/backend-app-api/src/services/implementations/rootHttpRouter/hostFactory.ts rename to packages/backend-app-api/src/lib/http/getGeneratedCertificate.ts index db4343cae5..b5cd420aff 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/hostFactory.ts +++ b/packages/backend-app-api/src/lib/http/getGeneratedCertificate.ts @@ -16,86 +16,16 @@ import fs from 'fs-extra'; import { resolve as resolvePath, dirname } from 'path'; -import express from 'express'; -import * as http from 'http'; -import * as https from 'https'; import { LoggerService } from '@backstage/backend-plugin-api'; -import { HttpsSettings } from './config'; import forge from 'node-forge'; const FIVE_DAYS_IN_MS = 5 * 24 * 60 * 60 * 1000; const IP_HOSTNAME_REGEX = /:|^\d+\.\d+\.\d+\.\d+$/; -/** - * Creates a Http server instance based on an Express application. - * - * @param app - The Express application object - * @param logger - Optional Winston logger object - * @returns A Http server instance - * - */ -export function createHttpServer( - app: express.Express, - logger?: LoggerService, -): http.Server { - logger?.info('Initializing http server'); - - return http.createServer(app); -} - -/** - * Creates a Https server instance based on an Express application. - * - * @param app - The Express application object - * @param httpsSettings - HttpsSettings for self-signed certificate generation - * @param logger - Optional Winston logger object - * @returns A Https server instance - * - */ -export async function createHttpsServer( - app: express.Express, - httpsSettings: HttpsSettings, - logger?: LoggerService, -): Promise { - logger?.info('Initializing https server'); - - let credentials: { key: string | Buffer; cert: string | Buffer }; - - if ('hostname' in httpsSettings?.certificate) { - credentials = await getGeneratedCertificate( - httpsSettings.certificate.hostname, - logger, - ); - } else { - logger?.info('Loading certificate from config'); - - credentials = { - key: httpsSettings?.certificate?.key, - cert: httpsSettings?.certificate?.cert, - }; - } - - if (!credentials.key || !credentials.cert) { - throw new Error('Invalid HTTPS credentials'); - } - - return https.createServer(credentials, app) as http.Server; -} - -function getCertificateExpiration(cert: string, logger?: LoggerService) { - try { - const crt = forge.pki.certificateFromPem(cert); - return crt.validity.notAfter.getTime() - Date.now(); - } catch (error) { - logger?.warn(`Unable to parse self-signed certificate. ${error}`); - return 0; - } -} - -async function getGeneratedCertificate( +export async function getGeneratedCertificate( hostname: string, - logger?: LoggerService, + logger: LoggerService, ) { const hasModules = await fs.pathExists('node_modules'); let certPath; @@ -109,24 +39,30 @@ async function getGeneratedCertificate( } if (await fs.pathExists(certPath)) { - const cert = await fs.readFile(certPath); - const remainingMs = getCertificateExpiration(cert.toString(), logger); - if (remainingMs > FIVE_DAYS_IN_MS) { - logger?.info('Using existing self-signed certificate'); - return { - key: cert, - cert, - }; + try { + const cert = await fs.readFile(certPath); + + const crt = forge.pki.certificateFromPem(cert.toString()); + const remainingMs = crt.validity.notAfter.getTime() - Date.now(); + if (remainingMs > FIVE_DAYS_IN_MS) { + logger.info('Using existing self-signed certificate'); + return { + key: cert, + cert, + }; + } + } catch (error) { + logger.warn(`Unable to use existing self-signed certificate, ${error}`); } } - logger?.info('Generating new self-signed certificate'); - const newCert = await createCertificate(hostname); + logger.info('Generating new self-signed certificate'); + const newCert = await generateCertificate(hostname); await fs.writeFile(certPath, newCert.cert + newCert.key, 'utf8'); return newCert; } -async function createCertificate(hostname: string) { +async function generateCertificate(hostname: string) { const attributes = [ { name: 'commonName', diff --git a/packages/backend-app-api/src/lib/http/types.ts b/packages/backend-app-api/src/lib/http/types.ts index a33415ec83..ff96f69bff 100644 --- a/packages/backend-app-api/src/lib/http/types.ts +++ b/packages/backend-app-api/src/lib/http/types.ts @@ -14,6 +14,21 @@ * limitations under the License. */ +import * as http from 'http'; + +/** + * An HTTP server extended with utility methods. + * + * @public + */ +export interface ExtendedHttpServer extends http.Server { + start(): Promise; + + stop(): Promise; + + port(): number; +} + /** * Options for starting up an HTTP server. * diff --git a/yarn.lock b/yarn.lock index ff351c8957..53e68b28f8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3388,6 +3388,7 @@ __metadata: "@types/express": ^4.17.6 "@types/fs-extra": ^9.0.3 "@types/node-forge": ^1.3.0 + "@types/stoppable": ^1.1.0 cors: ^2.8.5 express: ^4.17.1 express-promise-router: ^4.1.0 @@ -3395,6 +3396,7 @@ __metadata: minimatch: ^5.0.0 node-forge: ^1.3.1 selfsigned: ^2.0.0 + stoppable: ^1.1.0 winston: ^3.2.1 languageName: unknown linkType: soft From 1fed5327c37827106eab88ee76855113b8d329e7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Jan 2023 01:08:34 +0100 Subject: [PATCH 08/25] backend-app-api: split out readCorsOptions Signed-off-by: Patrik Oldsberg --- .../implementations/rootHttpRouter/config.ts | 105 ------------------ .../rootHttpRouter/readCorsOptions.test.ts | 81 ++++++++++++++ .../rootHttpRouter/readCorsOptions.ts | 81 ++++++++++++++ 3 files changed, 162 insertions(+), 105 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts index a147b67ea1..931fb48a63 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts @@ -15,8 +15,6 @@ */ import { Config } from '@backstage/config'; -import { CorsOptions } from 'cors'; -import { Minimatch } from 'minimatch'; export type BaseOptions = { listenPort?: string | number; @@ -45,47 +43,6 @@ export type CertificateAttributes = { */ export type CspOptions = Record; -type StaticOrigin = boolean | string | RegExp | (boolean | string | RegExp)[]; - -type CustomOrigin = ( - requestOrigin: string | undefined, - callback: (err: Error | null, origin?: StaticOrigin) => void, -) => void; - -/** - * Attempts to read a CORS options object from the root of a config object. - * - * @param config - The root of a backend config object - * @returns A CORS options object, or undefined if not specified - * - * @example - * ```json - * { - * cors: { - * origin: "http://localhost:3000", - * credentials: true - * } - * } - * ``` - */ -export function readCorsOptions(config: Config): CorsOptions | undefined { - const cc = config.getOptionalConfig('cors'); - if (!cc) { - return undefined; - } - - return removeUnknown({ - origin: createCorsOriginMatcher(getOptionalStringOrStrings(cc, 'origin')), - methods: getOptionalStringOrStrings(cc, 'methods'), - allowedHeaders: getOptionalStringOrStrings(cc, 'allowedHeaders'), - exposedHeaders: getOptionalStringOrStrings(cc, 'exposedHeaders'), - credentials: cc.getOptionalBoolean('credentials'), - maxAge: cc.getOptionalNumber('maxAge'), - preflightContinue: cc.getOptionalBoolean('preflightContinue'), - optionsSuccessStatus: cc.getOptionalNumber('optionsSuccessStatus'), - }); -} - /** * Attempts to read a CSP options object from the root of a config object. * @@ -121,65 +78,3 @@ export function readCspOptions( return result; } - -function getOptionalStringOrStrings( - config: Config, - key: string, -): string | string[] | undefined { - const value = config.getOptional(key); - if (value === undefined || isStringOrStrings(value)) { - return value; - } - throw new Error(`Expected string or array of strings, got ${typeof value}`); -} - -function createCorsOriginMatcher( - originValue: string | string[] | undefined, -): CustomOrigin | undefined { - if (originValue === undefined) { - return originValue; - } - - if (!isStringOrStrings(originValue)) { - throw new Error( - `Expected string or array of strings, got ${typeof originValue}`, - ); - } - - const allowedOrigin = - typeof originValue === 'string' ? [originValue] : originValue; - - const allowedOriginPatterns = - allowedOrigin?.map( - pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }), - ) ?? []; - - return (origin, callback) => { - return callback( - null, - allowedOriginPatterns.some(pattern => pattern.match(origin ?? '')), - ); - }; -} - -function isStringOrStrings(value: any): value is string | string[] { - return typeof value === 'string' || isStringArray(value); -} - -function isStringArray(value: any): value is string[] { - if (!Array.isArray(value)) { - return false; - } - for (const v of value) { - if (typeof v !== 'string') { - return false; - } - } - return true; -} - -function removeUnknown(obj: T): T { - return Object.fromEntries( - Object.entries(obj).filter(([, v]) => v !== undefined), - ) as T; -} diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts new file mode 100644 index 0000000000..1c70d020c4 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2020 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 { ConfigReader } from '@backstage/config'; +import { readCorsOptions } from './readCorsOptions'; + +describe('readCorsOptions', () => { + it('reads single string', () => { + const mockCallback = jest.fn(); + const config = new ConfigReader({ cors: { origin: 'https://*.value*' } }); + const cors = readCorsOptions(config); + expect(cors).toEqual( + expect.objectContaining({ + origin: expect.any(Function), + }), + ); + const origin = cors?.origin as Function; + origin('https://a.value', mockCallback); // valid origin + origin('http://a.value', mockCallback); // invalid origin + origin(undefined, mockCallback); // when not origin needs to reject the call + + expect(mockCallback.mock.calls[0][0]).toBe(null); + expect(mockCallback.mock.calls[1][0]).toBe(null); + + expect(mockCallback.mock.calls[0][1]).toBe(true); + expect(mockCallback.mock.calls[1][1]).toBe(false); + expect(mockCallback.mock.calls[2][1]).toBe(false); + }); + + it('reads string array', () => { + const mockCallback = jest.fn(); + const config = new ConfigReader({ + cors: { + origin: ['http?(s)://*.value?(-+([0-9])).com', 'http://*.value'], + }, + }); + const cors = readCorsOptions(config); + expect(cors).toEqual( + expect.objectContaining({ + origin: expect.any(Function), + }), + ); + const origin = cors?.origin as Function; + origin('https://a.b.c.value-9.com', mockCallback); + origin('http://a.value-999.com', mockCallback); + origin('http://a.value', mockCallback); + origin('http://a.valuex', mockCallback); + + expect(mockCallback.mock.calls[0][0]).toBe(null); + expect(mockCallback.mock.calls[1][0]).toBe(null); + expect(mockCallback.mock.calls[2][0]).toBe(null); + expect(mockCallback.mock.calls[3][0]).toBe(null); + + expect(mockCallback.mock.calls[0][1]).toBe(true); + expect(mockCallback.mock.calls[1][1]).toBe(true); + expect(mockCallback.mock.calls[2][1]).toBe(true); + expect(mockCallback.mock.calls[3][1]).toBe(false); + }); + + it('reads undefined origin', () => { + const config = new ConfigReader({ + cors: {}, + }); + const cors = readCorsOptions(config); + expect(cors).toEqual(expect.objectContaining({})); + expect(cors?.origin).toBeUndefined(); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts new file mode 100644 index 0000000000..17c3157ebd --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2023 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 { Config } from '@backstage/config'; +import { CorsOptions } from 'cors'; +import { Minimatch } from 'minimatch'; + +/** + * Attempts to read a CORS options object from the backend configuration object. + * + * @param config - The backend configuration object. + * @returns A CORS options object, or undefined if no cors configuration is present. + * + * @example + * ```ts + * const corsOptions = readCorsOptions(config.getConfig('backend')); + * ``` + */ +export function readCorsOptions(config: Config): CorsOptions | undefined { + const cc = config.getOptionalConfig('cors'); + if (!cc) { + return undefined; + } + + return { + origin: createCorsOriginMatcher(readStringArray(cc, 'origin')), + methods: readStringArray(cc, 'methods'), + allowedHeaders: readStringArray(cc, 'allowedHeaders'), + exposedHeaders: readStringArray(cc, 'exposedHeaders'), + credentials: cc.getOptionalBoolean('credentials'), + maxAge: cc.getOptionalNumber('maxAge'), + preflightContinue: cc.getOptionalBoolean('preflightContinue'), + optionsSuccessStatus: cc.getOptionalNumber('optionsSuccessStatus'), + }; +} + +function readStringArray(config: Config, key: string): string[] | undefined { + const value = config.getOptional(key); + if (typeof value === 'string') { + return [value]; + } else if (!value) { + return undefined; + } + return config.getStringArray(key); +} + +function createCorsOriginMatcher(allowedOriginPatterns: string[] | undefined) { + if (!allowedOriginPatterns) { + return undefined; + } + + const allowedOriginMatchers = allowedOriginPatterns.map( + pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }), + ); + + return ( + origin: string | undefined, + callback: ( + err: Error | null, + origin: boolean | string | RegExp | (boolean | string | RegExp)[], + ) => void, + ) => { + return callback( + null, + allowedOriginMatchers.some(pattern => pattern.match(origin ?? '')), + ); + }; +} From 4097435eeb483e379c347bba4c29b6298f158c24 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Jan 2023 11:22:54 +0100 Subject: [PATCH 09/25] backend-app-api: split out readHelmetOptions Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 1 + .../rootHttpRouter/config.test.ts | 108 ------------------ .../implementations/rootHttpRouter/config.ts | 80 ------------- .../rootHttpRouter/readHelmetOptions.test.ts | 57 +++++++++ .../rootHttpRouter/readHelmetOptions.ts | 108 ++++++++++++++++++ yarn.lock | 1 + 6 files changed, 167 insertions(+), 188 deletions(-) delete mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/config.test.ts delete mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts create mode 100644 packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 20c723bd51..7d56da10f1 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -45,6 +45,7 @@ "express": "^4.17.1", "express-promise-router": "^4.1.0", "fs-extra": "10.1.0", + "helmet": "^6.0.0", "minimatch": "^5.0.0", "node-forge": "^1.3.1", "selfsigned": "^2.0.0", diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.test.ts deleted file mode 100644 index b80d475517..0000000000 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.test.ts +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2020 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 { ConfigReader } from '@backstage/config'; -import { readCorsOptions, readCspOptions } from './config'; - -describe('config', () => { - describe('readCspOptions', () => { - it('reads valid values', () => { - const config = new ConfigReader({ csp: { key: ['value'] } }); - expect(readCspOptions(config)).toEqual( - expect.objectContaining({ - key: ['value'], - }), - ); - }); - - it('accepts false', () => { - const config = new ConfigReader({ csp: { key: false } }); - expect(readCspOptions(config)).toEqual( - expect.objectContaining({ - key: false, - }), - ); - }); - - it('rejects invalid value types', () => { - const config = new ConfigReader({ csp: { key: [4] } }); - expect(() => readCspOptions(config)).toThrow(/wanted string-array/); - }); - }); - - describe('readCorsOptions', () => { - it('reads single string', () => { - const mockCallback = jest.fn(); - const config = new ConfigReader({ cors: { origin: 'https://*.value*' } }); - const cors = readCorsOptions(config); - expect(cors).toEqual( - expect.objectContaining({ - origin: expect.any(Function), - }), - ); - const origin = cors?.origin as Function; - origin('https://a.value', mockCallback); // valid origin - origin('http://a.value', mockCallback); // invalid origin - origin(undefined, mockCallback); // when not origin needs to reject the call - - expect(mockCallback.mock.calls[0][0]).toBe(null); - expect(mockCallback.mock.calls[1][0]).toBe(null); - - expect(mockCallback.mock.calls[0][1]).toBe(true); - expect(mockCallback.mock.calls[1][1]).toBe(false); - expect(mockCallback.mock.calls[2][1]).toBe(false); - }); - - it('reads string array', () => { - const mockCallback = jest.fn(); - const config = new ConfigReader({ - cors: { - origin: ['http?(s)://*.value?(-+([0-9])).com', 'http://*.value'], - }, - }); - const cors = readCorsOptions(config); - expect(cors).toEqual( - expect.objectContaining({ - origin: expect.any(Function), - }), - ); - const origin = cors?.origin as Function; - origin('https://a.b.c.value-9.com', mockCallback); - origin('http://a.value-999.com', mockCallback); - origin('http://a.value', mockCallback); - origin('http://a.valuex', mockCallback); - - expect(mockCallback.mock.calls[0][0]).toBe(null); - expect(mockCallback.mock.calls[1][0]).toBe(null); - expect(mockCallback.mock.calls[2][0]).toBe(null); - expect(mockCallback.mock.calls[3][0]).toBe(null); - - expect(mockCallback.mock.calls[0][1]).toBe(true); - expect(mockCallback.mock.calls[1][1]).toBe(true); - expect(mockCallback.mock.calls[2][1]).toBe(true); - expect(mockCallback.mock.calls[3][1]).toBe(false); - }); - - it('reads undefined origin', () => { - const config = new ConfigReader({ - cors: {}, - }); - const cors = readCorsOptions(config); - expect(cors).toEqual(expect.objectContaining({})); - expect(cors?.origin).toBeUndefined(); - }); - }); -}); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts deleted file mode 100644 index 931fb48a63..0000000000 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/config.ts +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright 2020 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 { Config } from '@backstage/config'; - -export type BaseOptions = { - listenPort?: string | number; - listenHost?: string; -}; - -export type HttpsSettings = { - certificate: CertificateGenerationOptions | CertificateReferenceOptions; -}; - -export type CertificateReferenceOptions = { - key: string; - cert: string; -}; - -export type CertificateGenerationOptions = { - hostname: string; -}; - -export type CertificateAttributes = { - commonName: string; -}; - -/** - * A map from CSP directive names to their values. - */ -export type CspOptions = Record; - -/** - * Attempts to read a CSP options object from the root of a config object. - * - * @param config - The root of a backend config object - * @returns A CSP options object, or undefined if not specified. Values can be - * false as well, which means to remove the default behavior for that - * key. - * - * @example - * ```yaml - * backend: - * csp: - * connect-src: ["'self'", 'http:', 'https:'] - * upgrade-insecure-requests: false - * ``` - */ -export function readCspOptions( - config: Config, -): Record | undefined { - const cc = config.getOptionalConfig('csp'); - if (!cc) { - return undefined; - } - - const result: Record = {}; - for (const key of cc.keys()) { - if (cc.get(key) === false) { - result[key] = false; - } else { - result[key] = cc.getStringArray(key); - } - } - - return result; -} diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts new file mode 100644 index 0000000000..d5e0e29264 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts @@ -0,0 +1,57 @@ +/* + * Copyright 2020 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 { ConfigReader } from '@backstage/config'; +import { readHelmetOptions } from './readHelmetOptions'; + +describe('readHelmetOptions', () => { + it('should add additional directives', () => { + const config = new ConfigReader({ + csp: { + key: ['value'], + 'img-src': false, + 'script-src-attr': ['custom'], + }, + }); + expect(readHelmetOptions(config)).toEqual({ + contentSecurityPolicy: { + useDefaults: false, + directives: { + 'default-src': ["'self'"], + 'base-uri': ["'self'"], + 'font-src': ["'self'", 'https:', 'data:'], + 'frame-ancestors': ["'self'"], + // 'img-src': ["'self'", 'data:'], + 'object-src': ["'none'"], + 'script-src': ["'self'", "'unsafe-eval'"], + 'style-src': ["'self'", 'https:', "'unsafe-inline'"], + 'script-src-attr': ['custom'], + 'upgrade-insecure-requests': [], + key: ['value'], + }, + }, + crossOriginEmbedderPolicy: false, + crossOriginOpenerPolicy: false, + crossOriginResourcePolicy: false, + originAgentCluster: false, + }); + }); + + it('rejects invalid value types', () => { + const config = new ConfigReader({ csp: { key: [4] } }); + expect(() => readHelmetOptions(config)).toThrow(/wanted string-array/); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts new file mode 100644 index 0000000000..e86997d685 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts @@ -0,0 +1,108 @@ +/* + * Copyright 2020 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 { Config } from '@backstage/config'; +import helmet from 'helmet'; +import { HelmetOptions } from 'helmet'; +import { ContentSecurityPolicyOptions } from 'helmet/dist/types/middlewares/content-security-policy'; + +/** + * Attempts to read Helmet options from the backend configuration object. + * + * @param config - The backend configuration object. + * @returns A Helmet options object, or undefined if no Helmet configuration is present. + * + * @example + * ```ts + * const helmetOptions = readHelmetOptions(config.getConfig('backend')); + * ``` + */ +export function readHelmetOptions(config: Config): HelmetOptions { + const cspOptions = readCspDirectives(config); + return { + contentSecurityPolicy: { + useDefaults: false, + directives: applyCspDirectives(cspOptions), + }, + // These are all disabled in order to maintain backwards compatibility + // when bumping helmet v5. We can't enable these by default because + // there is no way for users to configure them. + // TODO(Rugvip): We should give control of this setup to consumers + crossOriginEmbedderPolicy: false, + crossOriginOpenerPolicy: false, + crossOriginResourcePolicy: false, + originAgentCluster: false, + }; +} + +type CspDirectives = Record | undefined; + +/** + * Attempts to read a CSP directives from the backend configuration object. + * + * @example + * ```yaml + * backend: + * csp: + * connect-src: ["'self'", 'http:', 'https:'] + * upgrade-insecure-requests: false + * ``` + */ +function readCspDirectives(config: Config): CspDirectives { + const cc = config.getOptionalConfig('csp'); + if (!cc) { + return undefined; + } + + const result: Record = {}; + for (const key of cc.keys()) { + if (cc.get(key) === false) { + result[key] = false; + } else { + result[key] = cc.getStringArray(key); + } + } + + return result; +} + +export function applyCspDirectives( + directives: CspDirectives, +): ContentSecurityPolicyOptions['directives'] { + const result: ContentSecurityPolicyOptions['directives'] = + helmet.contentSecurityPolicy.getDefaultDirectives(); + + // TODO(Rugvip): We currently use non-precompiled AJV for validation in the frontend, which uses eval. + // It should be replaced by any other solution that doesn't require unsafe-eval. + result['script-src'] = ["'self'", "'unsafe-eval'"]; + + // TODO(Rugvip): This is removed so that we maintained backwards compatibility + // when bumping to helmet v5, we could remove this as well as + // skip setting `useDefaults: false` in the future. + delete result['form-action']; + + if (directives) { + for (const [key, value] of Object.entries(directives)) { + if (value === false) { + delete result[key]; + } else { + result[key] = value; + } + } + } + + return result; +} diff --git a/yarn.lock b/yarn.lock index 53e68b28f8..0179e179e2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3393,6 +3393,7 @@ __metadata: express: ^4.17.1 express-promise-router: ^4.1.0 fs-extra: 10.1.0 + helmet: ^6.0.0 minimatch: ^5.0.0 node-forge: ^1.3.1 selfsigned: ^2.0.0 From e8d2de592dd933195be0508c3f3fab87d035c833 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Jan 2023 13:33:13 +0100 Subject: [PATCH 10/25] backend-app-api: readCorsOptions now disables cors by default Signed-off-by: Patrik Oldsberg --- .../implementations/rootHttpRouter/readCorsOptions.test.ts | 6 ++++++ .../implementations/rootHttpRouter/readCorsOptions.ts | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts index 1c70d020c4..113b2a8ee6 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts @@ -18,6 +18,12 @@ import { ConfigReader } from '@backstage/config'; import { readCorsOptions } from './readCorsOptions'; describe('readCorsOptions', () => { + it('should be disabled by default', () => { + expect(readCorsOptions(new ConfigReader({}))).toEqual({ + origin: false, + }); + }); + it('reads single string', () => { const mockCallback = jest.fn(); const config = new ConfigReader({ cors: { origin: 'https://*.value*' } }); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts index 17c3157ebd..809ee9b527 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts @@ -29,10 +29,10 @@ import { Minimatch } from 'minimatch'; * const corsOptions = readCorsOptions(config.getConfig('backend')); * ``` */ -export function readCorsOptions(config: Config): CorsOptions | undefined { +export function readCorsOptions(config: Config): CorsOptions { const cc = config.getOptionalConfig('cors'); if (!cc) { - return undefined; + return { origin: false }; // Disable CORS } return { From f56886014e551577ed0d3e8b99ebe8437e1572ea Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Jan 2023 13:39:42 +0100 Subject: [PATCH 11/25] backend-app-api: make config optional for all config readers Signed-off-by: Patrik Oldsberg --- .../src/lib/http/config.test.ts | 6 +++++ .../backend-app-api/src/lib/http/config.ts | 20 +++++++-------- .../rootHttpRouter/readCorsOptions.test.ts | 2 +- .../rootHttpRouter/readCorsOptions.ts | 4 +-- .../rootHttpRouter/readHelmetOptions.test.ts | 25 ++++++++++++++++++- .../rootHttpRouter/readHelmetOptions.ts | 6 ++--- 6 files changed, 46 insertions(+), 17 deletions(-) diff --git a/packages/backend-app-api/src/lib/http/config.test.ts b/packages/backend-app-api/src/lib/http/config.test.ts index 3ab5eda5e9..f54ba927ca 100644 --- a/packages/backend-app-api/src/lib/http/config.test.ts +++ b/packages/backend-app-api/src/lib/http/config.test.ts @@ -18,6 +18,12 @@ import { ConfigReader } from '@backstage/config'; import { readHttpServerOptions } from './config'; describe('readHttpServerOptions', () => { + it('should return defaults', () => { + expect(readHttpServerOptions()).toEqual({ + listen: { host: '', port: 7007 }, + }); + }); + it.each([ [{}, { listen: { host: '', port: 7007 } }], [{ listen: ':80' }, { listen: { host: '', port: 80 } }], diff --git a/packages/backend-app-api/src/lib/http/config.ts b/packages/backend-app-api/src/lib/http/config.ts index 3c5acf0ffd..e3b41c63de 100644 --- a/packages/backend-app-api/src/lib/http/config.ts +++ b/packages/backend-app-api/src/lib/http/config.ts @@ -34,15 +34,15 @@ const DEFAULT_HOST = ''; * const opts = readHttpServerOptions(config.getConfig('backend')); * ``` */ -export function readHttpServerOptions(config: Config): HttpServerOptions { +export function readHttpServerOptions(config?: Config): HttpServerOptions { return { listen: readHttpListenOptions(config), https: readHttpsOptions(config), }; } -function readHttpListenOptions(config: Config): HttpServerOptions['listen'] { - const listen = config.getOptional('listen'); +function readHttpListenOptions(config?: Config): HttpServerOptions['listen'] { + const listen = config?.getOptional('listen'); if (typeof listen === 'string') { const parts = String(listen).split(':'); const port = parseInt(parts[parts.length - 1], 10); @@ -60,22 +60,22 @@ function readHttpListenOptions(config: Config): HttpServerOptions['listen'] { } // Workaround to allow empty string - const host = config.getOptional('listen.host') ?? DEFAULT_HOST; + const host = config?.getOptional('listen.host') ?? DEFAULT_HOST; if (typeof host !== 'string') { - config.getOptionalString('listen.host'); // will throw + config?.getOptionalString('listen.host'); // will throw throw new Error('unreachable'); } return { - port: config.getOptionalNumber('listen.port') ?? DEFAULT_PORT, + port: config?.getOptionalNumber('listen.port') ?? DEFAULT_PORT, host, }; } -function readHttpsOptions(config: Config): HttpServerOptions['https'] { - const https = config.getOptional('https'); +function readHttpsOptions(config?: Config): HttpServerOptions['https'] { + const https = config?.getOptional('https'); if (https === true) { - const baseUrl = config.getString('baseUrl'); + const baseUrl = config!.getString('baseUrl'); let hostname; try { hostname = new URL(baseUrl).hostname; @@ -86,7 +86,7 @@ function readHttpsOptions(config: Config): HttpServerOptions['https'] { return { certificate: { type: 'generated', hostname } }; } - const cc = config.getOptionalConfig('https'); + const cc = config?.getOptionalConfig('https'); if (!cc) { return undefined; } diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts index 113b2a8ee6..af490a26f9 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts @@ -19,7 +19,7 @@ import { readCorsOptions } from './readCorsOptions'; describe('readCorsOptions', () => { it('should be disabled by default', () => { - expect(readCorsOptions(new ConfigReader({}))).toEqual({ + expect(readCorsOptions()).toEqual({ origin: false, }); }); diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts index 809ee9b527..98b548d02e 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts @@ -29,8 +29,8 @@ import { Minimatch } from 'minimatch'; * const corsOptions = readCorsOptions(config.getConfig('backend')); * ``` */ -export function readCorsOptions(config: Config): CorsOptions { - const cc = config.getOptionalConfig('cors'); +export function readCorsOptions(config?: Config): CorsOptions { + const cc = config?.getOptionalConfig('cors'); if (!cc) { return { origin: false }; // Disable CORS } diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts index d5e0e29264..bc31404634 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts @@ -18,6 +18,30 @@ import { ConfigReader } from '@backstage/config'; import { readHelmetOptions } from './readHelmetOptions'; describe('readHelmetOptions', () => { + it('should return defaults', () => { + expect(readHelmetOptions()).toEqual({ + contentSecurityPolicy: { + useDefaults: false, + directives: { + 'default-src': ["'self'"], + 'base-uri': ["'self'"], + 'font-src': ["'self'", 'https:', 'data:'], + 'frame-ancestors': ["'self'"], + 'img-src': ["'self'", 'data:'], + 'object-src': ["'none'"], + 'script-src': ["'self'", "'unsafe-eval'"], + 'style-src': ["'self'", 'https:', "'unsafe-inline'"], + 'script-src-attr': ["'none'"], + 'upgrade-insecure-requests': [], + }, + }, + crossOriginEmbedderPolicy: false, + crossOriginOpenerPolicy: false, + crossOriginResourcePolicy: false, + originAgentCluster: false, + }); + }); + it('should add additional directives', () => { const config = new ConfigReader({ csp: { @@ -34,7 +58,6 @@ describe('readHelmetOptions', () => { 'base-uri': ["'self'"], 'font-src': ["'self'", 'https:', 'data:'], 'frame-ancestors': ["'self'"], - // 'img-src': ["'self'", 'data:'], 'object-src': ["'none'"], 'script-src': ["'self'", "'unsafe-eval'"], 'style-src': ["'self'", 'https:', "'unsafe-inline'"], diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts index e86997d685..9cdf339844 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts @@ -30,7 +30,7 @@ import { ContentSecurityPolicyOptions } from 'helmet/dist/types/middlewares/cont * const helmetOptions = readHelmetOptions(config.getConfig('backend')); * ``` */ -export function readHelmetOptions(config: Config): HelmetOptions { +export function readHelmetOptions(config?: Config): HelmetOptions { const cspOptions = readCspDirectives(config); return { contentSecurityPolicy: { @@ -61,8 +61,8 @@ type CspDirectives = Record | undefined; * upgrade-insecure-requests: false * ``` */ -function readCspDirectives(config: Config): CspDirectives { - const cc = config.getOptionalConfig('csp'); +function readCspDirectives(config?: Config): CspDirectives { + const cc = config?.getOptionalConfig('csp'); if (!cc) { return undefined; } From d56bd5dfef0985f363027b157b36fb300f73ea47 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Jan 2023 14:32:21 +0100 Subject: [PATCH 12/25] backend-app-api: add startHttpServer helper Signed-off-by: Patrik Oldsberg --- .../backend-app-api/src/lib/http/index.ts | 9 ++- .../src/lib/http/startHttpServer.ts | 62 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 packages/backend-app-api/src/lib/http/startHttpServer.ts diff --git a/packages/backend-app-api/src/lib/http/index.ts b/packages/backend-app-api/src/lib/http/index.ts index 0717194f75..94ab61f6c8 100644 --- a/packages/backend-app-api/src/lib/http/index.ts +++ b/packages/backend-app-api/src/lib/http/index.ts @@ -14,5 +14,12 @@ * limitations under the License. */ +export { createHttpServer } from './createHttpServer'; +export { startHttpServer } from './startHttpServer'; +export type { StartHttpServerOptions } from './startHttpServer'; export { readHttpServerOptions } from './config'; -export type { HttpServerOptions, HttpServerCertificateOptions } from './types'; +export type { + HttpServerOptions, + HttpServerCertificateOptions, + ExtendedHttpServer, +} from './types'; diff --git a/packages/backend-app-api/src/lib/http/startHttpServer.ts b/packages/backend-app-api/src/lib/http/startHttpServer.ts new file mode 100644 index 0000000000..64d0be3f1e --- /dev/null +++ b/packages/backend-app-api/src/lib/http/startHttpServer.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2023 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 { + ConfigService, + RootLifecycleService, + RootLoggerService, +} from '@backstage/backend-plugin-api'; +import { RequestListener } from 'http'; +import { readHttpServerOptions } from './config'; +import { createHttpServer } from './createHttpServer'; + +/** + * Options for {@link startHttpServer}. + * + * @public + */ +export interface StartHttpServerOptions { + config: ConfigService; + logger: RootLoggerService; + lifecycle: RootLifecycleService; +} + +/** + * Starts up an HTTP server, as well as registers a shutdown handler that stops the server. + * + * @public + */ +export async function startHttpServer( + listener: RequestListener, + options: StartHttpServerOptions, +) { + const { config, logger, lifecycle } = options; + + const server = await createHttpServer( + listener, + readHttpServerOptions(config.getOptionalConfig('backend')), + { logger }, + ); + + lifecycle.addShutdownHook({ + async fn() { + await server.stop(); + }, + labels: { type: 'httpServer' }, + }); + + await server.start(); +} From 15f15e4a7121d7eff8d8ae18ec3ef7ac26c0b5f5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 5 Jan 2023 14:32:58 +0100 Subject: [PATCH 13/25] backend-app-api: reimplement rootHttpRouterFactory with internal API Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 2 + .../rootHttpRouter/rootHttpRouterFactory.ts | 64 ++++++++----------- yarn.lock | 2 + 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 7d56da10f1..8c29b08c9e 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -41,6 +41,7 @@ "@backstage/plugin-permission-node": "workspace:^", "@types/cors": "^2.8.6", "@types/express": "^4.17.6", + "compression": "^1.7.4", "cors": "^2.8.5", "express": "^4.17.1", "express-promise-router": "^4.1.0", @@ -54,6 +55,7 @@ }, "devDependencies": { "@backstage/cli": "workspace:^", + "@types/compression": "^1.7.0", "@types/fs-extra": "^9.0.3", "@types/node-forge": "^1.3.0", "@types/stoppable": "^1.1.0" diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index ffe345f0a0..d491663ff8 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -18,9 +18,19 @@ import { createServiceFactory, coreServices, } from '@backstage/backend-plugin-api'; -import { Handler } from 'express'; -import { createServiceBuilder } from '@backstage/backend-common'; +import express from 'express'; +import compression from 'compression'; +import cors from 'cors'; +import helmet from 'helmet'; import { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; +import { readCorsOptions } from './readCorsOptions'; +import { startHttpServer } from '../../../lib/http'; +import { readHelmetOptions } from './readHelmetOptions'; +import { + errorHandler, + notFoundHandler, + requestLoggingHandler, +} from '@backstage/backend-common'; /** * @public @@ -30,11 +40,6 @@ export type RootHttpRouterFactoryOptions = { * The path to forward all unmatched requests to. Defaults to '/api/app' */ indexPath?: string | false; - - /** - * Middlewares that are added before all other routes. - */ - middleware?: Handler[]; }; /** @public */ @@ -42,41 +47,26 @@ export const rootHttpRouterFactory = createServiceFactory({ service: coreServices.rootHttpRouter, deps: { config: coreServices.config, + logger: coreServices.rootLogger, lifecycle: coreServices.rootLifecycle, }, - async factory({ config, lifecycle }, options?: RootHttpRouterFactoryOptions) { - const router = new RestrictedIndexedRouter( - options?.indexPath ?? '/api/app', - ); + async factory( + { config, logger, lifecycle }, + { indexPath }: RootHttpRouterFactoryOptions = {}, + ) { + const router = new RestrictedIndexedRouter(indexPath ?? '/api/app'); - const service = createServiceBuilder(module).loadConfig(config); + const app = express(); - for (const middleware of options?.middleware ?? []) { - service.addRouter('', middleware); - } + app.use(helmet(readHelmetOptions(config.getOptionalConfig('backend')))); + app.use(cors(readCorsOptions(config.getOptionalConfig('backend')))); + app.use(compression()); + app.use(requestLoggingHandler(logger)); + app.use(router.handler()); + app.use(notFoundHandler()); + app.use(errorHandler({ logger })); - service.addRouter('', router.handler()); - - const server = await service.start(); - // Stop method isn't part of the public API, let's fix that once we move the implementation here. - const stoppableServer = server as typeof server & { - stop: (cb: (error?: Error) => void) => void; - }; - - lifecycle.addShutdownHook({ - async fn() { - await new Promise((resolve, reject) => { - stoppableServer.stop((error?: Error) => { - if (error) { - reject(error); - } else { - resolve(); - } - }); - }); - }, - labels: { service: 'rootHttpRouter' }, - }); + await startHttpServer(app, { config, logger, lifecycle }); return router; }, diff --git a/yarn.lock b/yarn.lock index 0179e179e2..6409539d39 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3384,11 +3384,13 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/plugin-permission-node": "workspace:^" + "@types/compression": ^1.7.0 "@types/cors": ^2.8.6 "@types/express": ^4.17.6 "@types/fs-extra": ^9.0.3 "@types/node-forge": ^1.3.0 "@types/stoppable": ^1.1.0 + compression: ^1.7.4 cors: ^2.8.5 express: ^4.17.1 express-promise-router: ^4.1.0 From eb62b5be7481697b172a3696d51db45e0da80abb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 15:51:32 +0100 Subject: [PATCH 14/25] backend-app-api: add MiddlewareFactory + configure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: blam Co-authored-by: Fredrik Adelöw Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 7 +- .../src/lib/http/MiddlewareFactory.test.ts | 213 ++++++++++++++ .../src/lib/http/MiddlewareFactory.ts | 260 ++++++++++++++++++ .../backend-app-api/src/lib/http/index.ts | 2 + .../rootHttpRouter/rootHttpRouterFactory.ts | 66 +++-- yarn.lock | 5 + 6 files changed, 531 insertions(+), 22 deletions(-) create mode 100644 packages/backend-app-api/src/lib/http/MiddlewareFactory.test.ts create mode 100644 packages/backend-app-api/src/lib/http/MiddlewareFactory.ts diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 8c29b08c9e..ed1d2bca40 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -48,6 +48,7 @@ "fs-extra": "10.1.0", "helmet": "^6.0.0", "minimatch": "^5.0.0", + "morgan": "^1.10.0", "node-forge": "^1.3.1", "selfsigned": "^2.0.0", "stoppable": "^1.1.0", @@ -57,8 +58,12 @@ "@backstage/cli": "workspace:^", "@types/compression": "^1.7.0", "@types/fs-extra": "^9.0.3", + "@types/http-errors": "^2.0.0", + "@types/morgan": "^1.9.0", "@types/node-forge": "^1.3.0", - "@types/stoppable": "^1.1.0" + "@types/stoppable": "^1.1.0", + "http-errors": "^2.0.0", + "supertest": "^6.1.3" }, "files": [ "dist", diff --git a/packages/backend-app-api/src/lib/http/MiddlewareFactory.test.ts b/packages/backend-app-api/src/lib/http/MiddlewareFactory.test.ts new file mode 100644 index 0000000000..64812564ca --- /dev/null +++ b/packages/backend-app-api/src/lib/http/MiddlewareFactory.test.ts @@ -0,0 +1,213 @@ +/* + * Copyright 2020 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 { + AuthenticationError, + ConflictError, + InputError, + NotAllowedError, + NotFoundError, + NotModifiedError, +} from '@backstage/errors'; +import express from 'express'; +import createError from 'http-errors'; +import request from 'supertest'; +import { MiddlewareFactory } from './MiddlewareFactory'; +import { ConfigReader } from '@backstage/config'; + +describe('MiddlewareFactory', () => { + describe('middleware.error', () => { + const childLogger = { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + child: jest.fn(), + }; + + const logger = { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + debug: jest.fn(), + child: () => childLogger, + }; + + const middleware = MiddlewareFactory.create({ + logger, + config: new ConfigReader({}), + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('gives default code and message', async () => { + const app = express(); + app.use('/breaks', () => { + throw new Error('some message'); + }); + app.use(middleware.error()); + + const response = await request(app).get('/breaks'); + + expect(response.status).toBe(500); + expect(response.body).toEqual({ + error: expect.objectContaining({ + name: 'Error', + message: 'some message', + }), + request: { method: 'GET', url: '/breaks' }, + response: { statusCode: 500 }, + }); + }); + + it('does not try to send the response again if its already been sent', async () => { + const app = express(); + const mockSend = jest.fn(); + + app.use('/works_with_async_fail', (_, res) => { + res.status(200).send('hello'); + + // mutate the response object to test the middleware. + // it's hard to catch errors inside middleware from the outside. + res.send = mockSend; + throw new Error('some message'); + }); + + app.use(middleware.error()); + const response = await request(app).get('/works_with_async_fail'); + + expect(response.status).toBe(200); + expect(response.text).toBe('hello'); + + expect(mockSend).not.toHaveBeenCalled(); + }); + + it('takes code from http-errors library errors', async () => { + const app = express(); + app.use('/breaks', () => { + throw createError(432, 'Some Message'); + }); + app.use(middleware.error()); + + const response = await request(app).get('/breaks'); + + expect(response.status).toBe(432); + expect(response.body).toEqual({ + error: { + expose: true, + name: 'BadRequestError', + message: 'Some Message', + status: 432, + statusCode: 432, + }, + request: { + method: 'GET', + url: '/breaks', + }, + response: { statusCode: 432 }, + }); + }); + + it('handles well-known error classes', async () => { + const app = express(); + app.use('/NotModifiedError', () => { + throw new NotModifiedError(); + }); + app.use('/InputError', () => { + throw new InputError(); + }); + app.use('/AuthenticationError', () => { + throw new AuthenticationError(); + }); + app.use('/NotAllowedError', () => { + throw new NotAllowedError(); + }); + app.use('/NotFoundError', () => { + throw new NotFoundError(); + }); + app.use('/ConflictError', () => { + throw new ConflictError(); + }); + app.use(middleware.error()); + + const r = request(app); + expect((await r.get('/NotModifiedError')).status).toBe(304); + expect((await r.get('/InputError')).status).toBe(400); + expect((await r.get('/InputError')).body.error.name).toBe('InputError'); + expect((await r.get('/AuthenticationError')).status).toBe(401); + expect((await r.get('/AuthenticationError')).body.error.name).toBe( + 'AuthenticationError', + ); + expect((await r.get('/NotAllowedError')).status).toBe(403); + expect((await r.get('/NotAllowedError')).body.error.name).toBe( + 'NotAllowedError', + ); + expect((await r.get('/NotFoundError')).status).toBe(404); + expect((await r.get('/NotFoundError')).body.error.name).toBe( + 'NotFoundError', + ); + expect((await r.get('/ConflictError')).status).toBe(409); + expect((await r.get('/ConflictError')).body.error.name).toBe( + 'ConflictError', + ); + }); + + it('logs all 500 errors', async () => { + const app = express(); + const thrownError = new Error('some error'); + + app.use('/breaks', () => { + throw thrownError; + }); + app.use(middleware.error()); + + await request(app).get('/breaks'); + + expect(childLogger.error).toHaveBeenCalledWith( + 'Request failed with status 500', + thrownError, + ); + }); + + it('does not log 400 errors', async () => { + const app = express(); + + app.use('/NotFound', () => { + throw new NotFoundError(); + }); + app.use(middleware.error()); + + await request(app).get('/NotFound'); + + expect(childLogger.error).not.toHaveBeenCalled(); + }); + + it('log 400 errors when logAllErrors is true', async () => { + const app = express(); + + app.use('/NotFound', () => { + throw new NotFoundError(); + }); + app.use(middleware.error({ logAllErrors: true })); + + await request(app).get('/NotFound'); + + expect(childLogger.error).toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts b/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts new file mode 100644 index 0000000000..ccc94779b0 --- /dev/null +++ b/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts @@ -0,0 +1,260 @@ +/* + * Copyright 2023 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 { ConfigService, LoggerService } from '@backstage/backend-plugin-api'; +import { Request, Response, ErrorRequestHandler, NextFunction } from 'express'; +import cors from 'cors'; +import helmet from 'helmet'; +import morgan from 'morgan'; +import compression from 'compression'; +import { readHelmetOptions } from '../../services/implementations/rootHttpRouter/readHelmetOptions'; +import { readCorsOptions } from '../../services/implementations/rootHttpRouter/readCorsOptions'; +import { + AuthenticationError, + ConflictError, + ErrorResponseBody, + InputError, + NotAllowedError, + NotFoundError, + NotModifiedError, + serializeError, +} from '@backstage/errors'; + +/** + * Options used to create a {@link MiddlewareFactory}. + * + * @public + */ +export interface MiddlewareFactoryOptions { + config: ConfigService; + logger: LoggerService; +} + +/** + * Options passed to the {@link errorHandler} middleware. + * + * @public + */ +export interface MiddlewareFactoryErrorOptions { + /** + * Whether error response bodies should show error stack traces or not. + * + * If not specified, by default shows stack traces only in development mode. + */ + showStackTraces?: boolean; + + /** + * Whether any 4xx errors should be logged or not. + * + * If not specified, default to only logging 5xx errors. + */ + logAllErrors?: boolean; +} + +/** + * A utility to configure common middleware. + * + * @public + */ +export class MiddlewareFactory { + #config: ConfigService; + #logger: LoggerService; + + /** + * Creates a new {@link MiddlewareFactory}. + */ + static create(options: MiddlewareFactoryOptions) { + return new MiddlewareFactory(options); + } + + private constructor(options: MiddlewareFactoryOptions) { + this.#config = options.config; + this.#logger = options.logger; + } + + /** + * Returns a middleware that unconditionally produces a 404 error response. + * + * @remarks + * + * Typically you want to place this middleware at the end of the chain, such + * that it's the last one attempted after no other routes matched. + * + * @returns An Express request handler + */ + notFound() { + return (_req: Request, res: Response) => { + res.status(404).end(); + }; + } + + /** + * Returns the compression middleware. + * + * @remarks + * + * The middleware will attempt to compress response bodies for all requests + * that traverse through the middleware. + */ + compression() { + return compression(); + } + + /** + * Returns a request logging middleware. + * + * @remarks + * + * Typically you want to place this middleware at the start of the chain, such + * that it always logs requests whether they are "caught" by handlers farther + * down or not. + * + * @returns An Express request handler + */ + logging() { + const logger = this.#logger.child({ + type: 'incomingRequest', + }); + + return morgan('combined', { + stream: { + write(message: string) { + logger.info(message.trimEnd()); + }, + }, + }); + } + + /** + * Returns a middleware that implements the helmet library. + * + * @remarks + * + * This middleware applies security policies to incoming requests and outgoing + * responses. It is configured using config keys such as `backend.csp`. + * + * @see {@link https://helmetjs.github.io/} + * + * @returns An Express request handler + */ + helmet() { + return helmet(readHelmetOptions(this.#config.getOptionalConfig('backend'))); + } + + /** + * Returns a middleware that implements the cors library. + * + * @remarks + * + * This middleware handles CORS. It is configured using the config key + * `backend.cors`. + * + * @see {@link https://github.com/expressjs/cors} + * + * @returns An Express request handler + */ + cors() { + return cors(readCorsOptions(this.#config.getOptionalConfig('backend'))); + } + + /** + * Express middleware to handle errors during request processing. + * + * @remarks + * + * This is commonly the very last middleware in the chain. + * + * Its primary purpose is not to do translation of business logic exceptions, + * but rather to be a global catch-all for uncaught "fatal" errors that are + * expected to result in a 500 error. However, it also does handle some common + * error types (such as http-error exceptions, and the well-known error types + * in the `@backstage/errors` package) and returns the enclosed status code + * accordingly. + * + * It will also produce a response body with a serialized form of the error, + * unless a previous handler already did send a body. See + * {@link @backstage/errors#ErrorResponseBody} for the response shape used. + * + * @returns An Express error request handler + */ + error(options: MiddlewareFactoryErrorOptions = {}): ErrorRequestHandler { + const showStackTraces = + options.showStackTraces ?? process.env.NODE_ENV === 'development'; + + const logger = this.#logger.child({ + type: 'errorHandler', + }); + + return (error: Error, req: Request, res: Response, next: NextFunction) => { + const statusCode = getStatusCode(error); + if (options.logAllErrors || statusCode >= 500) { + logger.error(`Request failed with status ${statusCode}`, error); + } + + if (res.headersSent) { + // If the headers have already been sent, do not send the response again + // as this will throw an error in the backend. + next(error); + return; + } + + const body: ErrorResponseBody = { + error: serializeError(error, { includeStack: showStackTraces }), + request: { method: req.method, url: req.url }, + response: { statusCode }, + }; + + res.status(statusCode).json(body); + }; + } +} + +function getStatusCode(error: Error): number { + // Look for common http library status codes + const knownStatusCodeFields = ['statusCode', 'status']; + for (const field of knownStatusCodeFields) { + const statusCode = (error as any)[field]; + if ( + typeof statusCode === 'number' && + (statusCode | 0) === statusCode && // is whole integer + statusCode >= 100 && + statusCode <= 599 + ) { + return statusCode; + } + } + + // Handle well-known error types + switch (error.name) { + case NotModifiedError.name: + return 304; + case InputError.name: + return 400; + case AuthenticationError.name: + return 401; + case NotAllowedError.name: + return 403; + case NotFoundError.name: + return 404; + case ConflictError.name: + return 409; + default: + break; + } + + // Fall back to internal server error + return 500; +} diff --git a/packages/backend-app-api/src/lib/http/index.ts b/packages/backend-app-api/src/lib/http/index.ts index 94ab61f6c8..db579d1b44 100644 --- a/packages/backend-app-api/src/lib/http/index.ts +++ b/packages/backend-app-api/src/lib/http/index.ts @@ -23,3 +23,5 @@ export type { HttpServerCertificateOptions, ExtendedHttpServer, } from './types'; +export { MiddlewareFactory } from './MiddlewareFactory'; +export type { MiddlewareFactoryOptions } from './MiddlewareFactory'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index d491663ff8..f82cfe4a69 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -15,22 +15,24 @@ */ import { - createServiceFactory, + ConfigService, coreServices, + createServiceFactory, + LifecycleService, + LoggerService, } from '@backstage/backend-plugin-api'; -import express from 'express'; -import compression from 'compression'; -import cors from 'cors'; -import helmet from 'helmet'; +import express, { RequestHandler, Express } from 'express'; +import { MiddlewareFactory, startHttpServer } from '../../../lib/http'; import { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; -import { readCorsOptions } from './readCorsOptions'; -import { startHttpServer } from '../../../lib/http'; -import { readHelmetOptions } from './readHelmetOptions'; -import { - errorHandler, - notFoundHandler, - requestLoggingHandler, -} from '@backstage/backend-common'; + +interface RootHttpRouterConfigureOptions { + app: Express; + middleware: MiddlewareFactory; + routes: RequestHandler; + config: ConfigService; + logger: LoggerService; + lifecycle: LifecycleService; +} /** * @public @@ -40,8 +42,24 @@ export type RootHttpRouterFactoryOptions = { * The path to forward all unmatched requests to. Defaults to '/api/app' */ indexPath?: string | false; + + configure?(options: RootHttpRouterConfigureOptions): void; }; +function defaultConfigure({ + app, + routes, + middleware, +}: RootHttpRouterConfigureOptions) { + app.use(middleware.helmet()); + app.use(middleware.cors()); + app.use(middleware.compression()); + app.use(middleware.logging()); + app.use(routes); + app.use(middleware.notFound()); + app.use(middleware.error()); +} + /** @public */ export const rootHttpRouterFactory = createServiceFactory({ service: coreServices.rootHttpRouter, @@ -52,19 +70,25 @@ export const rootHttpRouterFactory = createServiceFactory({ }, async factory( { config, logger, lifecycle }, - { indexPath }: RootHttpRouterFactoryOptions = {}, + { + indexPath, + configure = defaultConfigure, + }: RootHttpRouterFactoryOptions = {}, ) { const router = new RestrictedIndexedRouter(indexPath ?? '/api/app'); const app = express(); - app.use(helmet(readHelmetOptions(config.getOptionalConfig('backend')))); - app.use(cors(readCorsOptions(config.getOptionalConfig('backend')))); - app.use(compression()); - app.use(requestLoggingHandler(logger)); - app.use(router.handler()); - app.use(notFoundHandler()); - app.use(errorHandler({ logger })); + const middleware = MiddlewareFactory.create({ config, logger }); + + configure({ + app, + routes: router.handler(), + middleware, + config, + logger, + lifecycle, + }); await startHttpServer(app, { config, logger, lifecycle }); diff --git a/yarn.lock b/yarn.lock index 6409539d39..ed60523406 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3388,6 +3388,8 @@ __metadata: "@types/cors": ^2.8.6 "@types/express": ^4.17.6 "@types/fs-extra": ^9.0.3 + "@types/http-errors": ^2.0.0 + "@types/morgan": ^1.9.0 "@types/node-forge": ^1.3.0 "@types/stoppable": ^1.1.0 compression: ^1.7.4 @@ -3396,10 +3398,13 @@ __metadata: express-promise-router: ^4.1.0 fs-extra: 10.1.0 helmet: ^6.0.0 + http-errors: ^2.0.0 minimatch: ^5.0.0 + morgan: ^1.10.0 node-forge: ^1.3.1 selfsigned: ^2.0.0 stoppable: ^1.1.0 + supertest: ^6.1.3 winston: ^3.2.1 languageName: unknown linkType: soft From c44dc6225ffb0eefa55571e3a5c7f7bc0bd2228d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 16:04:25 +0100 Subject: [PATCH 15/25] backend-common: reimplement middleware using MiddlewareFactory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/src/index.ts | 1 + packages/backend-common/package.json | 1 + .../src/middleware/errorHandler.ts | 84 ++----------------- .../src/middleware/notFoundHandler.ts | 13 +-- .../src/middleware/requestLoggingHandler.ts | 18 ++-- yarn.lock | 1 + 6 files changed, 26 insertions(+), 92 deletions(-) diff --git a/packages/backend-app-api/src/index.ts b/packages/backend-app-api/src/index.ts index 02633f3732..a8cc079d47 100644 --- a/packages/backend-app-api/src/index.ts +++ b/packages/backend-app-api/src/index.ts @@ -20,5 +20,6 @@ * @packageDocumentation */ +export * from './lib/http'; export * from './wiring'; export * from './services/implementations'; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index aff5e56abb..9c16472263 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -34,6 +34,7 @@ "test:kubernetes": "backstage-cli package test -t KubernetesContainerRunner --no-watch" }, "dependencies": { + "@backstage/backend-app-api": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/cli-common": "workspace:^", "@backstage/config": "workspace:^", diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts index 04b3f92e26..99ede75ab0 100644 --- a/packages/backend-common/src/middleware/errorHandler.ts +++ b/packages/backend-common/src/middleware/errorHandler.ts @@ -14,19 +14,11 @@ * limitations under the License. */ -import { - AuthenticationError, - ConflictError, - ErrorResponseBody, - InputError, - NotAllowedError, - NotFoundError, - NotModifiedError, - serializeError, -} from '@backstage/errors'; -import { ErrorRequestHandler, NextFunction, Request, Response } from 'express'; +import { ErrorRequestHandler } from 'express'; import { LoggerService } from '@backstage/backend-plugin-api'; import { getRootLogger } from '../logging'; +import { ConfigReader } from '@backstage/config'; +import { MiddlewareFactory } from '@backstage/backend-app-api'; /** * Options passed to the {@link errorHandler} middleware. @@ -73,69 +65,11 @@ export type ErrorHandlerOptions = { export function errorHandler( options: ErrorHandlerOptions = {}, ): ErrorRequestHandler { - const showStackTraces = - options.showStackTraces ?? process.env.NODE_ENV === 'development'; - - const logger = (options.logger || getRootLogger()).child({ - type: 'errorHandler', + return MiddlewareFactory.create({ + config: new ConfigReader({}), + logger: options.logger ?? getRootLogger(), + }).error({ + logAllErrors: options.logClientErrors, + showStackTraces: options.showStackTraces, }); - - return (error: Error, req: Request, res: Response, next: NextFunction) => { - const statusCode = getStatusCode(error); - if (options.logClientErrors || statusCode >= 500) { - logger.error(`Request failed with status ${statusCode}`, error); - } - - if (res.headersSent) { - // If the headers have already been sent, do not send the response again - // as this will throw an error in the backend. - next(error); - return; - } - - const body: ErrorResponseBody = { - error: serializeError(error, { includeStack: showStackTraces }), - request: { method: req.method, url: req.url }, - response: { statusCode }, - }; - - res.status(statusCode).json(body); - }; -} - -function getStatusCode(error: Error): number { - // Look for common http library status codes - const knownStatusCodeFields = ['statusCode', 'status']; - for (const field of knownStatusCodeFields) { - const statusCode = (error as any)[field]; - if ( - typeof statusCode === 'number' && - (statusCode | 0) === statusCode && // is whole integer - statusCode >= 100 && - statusCode <= 599 - ) { - return statusCode; - } - } - - // Handle well-known error types - switch (error.name) { - case NotModifiedError.name: - return 304; - case InputError.name: - return 400; - case AuthenticationError.name: - return 401; - case NotAllowedError.name: - return 403; - case NotFoundError.name: - return 404; - case ConflictError.name: - return 409; - default: - break; - } - - // Fall back to internal server error - return 500; } diff --git a/packages/backend-common/src/middleware/notFoundHandler.ts b/packages/backend-common/src/middleware/notFoundHandler.ts index 2d8b0ef3d0..4f29b4f824 100644 --- a/packages/backend-common/src/middleware/notFoundHandler.ts +++ b/packages/backend-common/src/middleware/notFoundHandler.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { NextFunction, Request, RequestHandler, Response } from 'express'; +import { MiddlewareFactory } from '@backstage/backend-app-api'; +import { ConfigReader } from '@backstage/config'; +import { RequestHandler } from 'express'; +import { getRootLogger } from '../logging'; /** * Express middleware to handle requests for missing routes. @@ -26,8 +29,8 @@ import { NextFunction, Request, RequestHandler, Response } from 'express'; * @returns An Express request handler */ export function notFoundHandler(): RequestHandler { - /* eslint-disable @typescript-eslint/no-unused-vars */ - return (_request: Request, response: Response, _next: NextFunction) => { - response.status(404).end(); - }; + return MiddlewareFactory.create({ + config: new ConfigReader({}), + logger: getRootLogger(), + }).notFound(); } diff --git a/packages/backend-common/src/middleware/requestLoggingHandler.ts b/packages/backend-common/src/middleware/requestLoggingHandler.ts index 1618f43ced..a03e3c31f6 100644 --- a/packages/backend-common/src/middleware/requestLoggingHandler.ts +++ b/packages/backend-common/src/middleware/requestLoggingHandler.ts @@ -14,10 +14,11 @@ * limitations under the License. */ +import { MiddlewareFactory } from '@backstage/backend-app-api'; import { RequestHandler } from 'express'; import { LoggerService } from '@backstage/backend-plugin-api'; -import morgan from 'morgan'; import { getRootLogger } from '../logging'; +import { ConfigReader } from '@backstage/config'; /** * Logs incoming requests. @@ -27,15 +28,8 @@ import { getRootLogger } from '../logging'; * @returns An Express request handler */ export function requestLoggingHandler(logger?: LoggerService): RequestHandler { - const actualLogger = (logger || getRootLogger()).child({ - type: 'incomingRequest', - }); - - return morgan('combined', { - stream: { - write(message: string) { - actualLogger.info(message.trimEnd()); - }, - }, - }); + return MiddlewareFactory.create({ + config: new ConfigReader({}), + logger: logger ?? getRootLogger(), + }).logging(); } diff --git a/yarn.lock b/yarn.lock index ed60523406..fe2bc0bcca 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3413,6 +3413,7 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/backend-common@workspace:packages/backend-common" dependencies: + "@backstage/backend-app-api": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-test-utils": "workspace:^" "@backstage/cli": "workspace:^" From 011c8a8ff16014b4e5bbdbd4b16d6d995e996efd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 16:04:57 +0100 Subject: [PATCH 16/25] backend-common: use readHttpServerOptions to read SingleHostDiscovery options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../backend-common/src/discovery/SingleHostDiscovery.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/backend-common/src/discovery/SingleHostDiscovery.ts b/packages/backend-common/src/discovery/SingleHostDiscovery.ts index cc23a2f744..ded23b3374 100644 --- a/packages/backend-common/src/discovery/SingleHostDiscovery.ts +++ b/packages/backend-common/src/discovery/SingleHostDiscovery.ts @@ -16,8 +16,7 @@ import { Config } from '@backstage/config'; import { PluginEndpointDiscovery } from './types'; -import { readBaseOptions } from '../service/lib/config'; -import { DEFAULT_PORT } from '../service/lib/ServiceBuilderImpl'; +import { readHttpServerOptions } from '@backstage/backend-app-api'; /** * SingleHostDiscovery is a basic PluginEndpointDiscovery implementation @@ -43,9 +42,9 @@ export class SingleHostDiscovery implements PluginEndpointDiscovery { const basePath = options?.basePath ?? '/api'; const externalBaseUrl = config.getString('backend.baseUrl'); - const { listenHost = '::', listenPort = DEFAULT_PORT } = readBaseOptions( - config.getConfig('backend'), - ); + const { + listen: { host: listenHost = '::', port: listenPort }, + } = readHttpServerOptions(config.getConfig('backend')); const protocol = config.has('backend.https') ? 'https' : 'http'; // Translate bind-all to localhost, and support IPv6 From a70b64b4889c66be46556386b5cb7d2c91f6882c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 17:22:14 +0100 Subject: [PATCH 17/25] backend-app-api: move cors and helmet options reader to http folder Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/src/lib/http/MiddlewareFactory.ts | 4 ++-- packages/backend-app-api/src/lib/http/index.ts | 2 ++ .../rootHttpRouter => lib/http}/readCorsOptions.test.ts | 0 .../rootHttpRouter => lib/http}/readCorsOptions.ts | 0 .../rootHttpRouter => lib/http}/readHelmetOptions.test.ts | 0 .../rootHttpRouter => lib/http}/readHelmetOptions.ts | 0 6 files changed, 4 insertions(+), 2 deletions(-) rename packages/backend-app-api/src/{services/implementations/rootHttpRouter => lib/http}/readCorsOptions.test.ts (100%) rename packages/backend-app-api/src/{services/implementations/rootHttpRouter => lib/http}/readCorsOptions.ts (100%) rename packages/backend-app-api/src/{services/implementations/rootHttpRouter => lib/http}/readHelmetOptions.test.ts (100%) rename packages/backend-app-api/src/{services/implementations/rootHttpRouter => lib/http}/readHelmetOptions.ts (100%) diff --git a/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts b/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts index ccc94779b0..4be8467db3 100644 --- a/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts +++ b/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts @@ -20,8 +20,8 @@ import cors from 'cors'; import helmet from 'helmet'; import morgan from 'morgan'; import compression from 'compression'; -import { readHelmetOptions } from '../../services/implementations/rootHttpRouter/readHelmetOptions'; -import { readCorsOptions } from '../../services/implementations/rootHttpRouter/readCorsOptions'; +import { readHelmetOptions } from './readHelmetOptions'; +import { readCorsOptions } from './readCorsOptions'; import { AuthenticationError, ConflictError, diff --git a/packages/backend-app-api/src/lib/http/index.ts b/packages/backend-app-api/src/lib/http/index.ts index db579d1b44..8f1a8c5d92 100644 --- a/packages/backend-app-api/src/lib/http/index.ts +++ b/packages/backend-app-api/src/lib/http/index.ts @@ -24,4 +24,6 @@ export type { ExtendedHttpServer, } from './types'; export { MiddlewareFactory } from './MiddlewareFactory'; +export { readHelmetOptions } from './readHelmetOptions'; +export { readCorsOptions } from './readCorsOptions'; export type { MiddlewareFactoryOptions } from './MiddlewareFactory'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts b/packages/backend-app-api/src/lib/http/readCorsOptions.test.ts similarity index 100% rename from packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.test.ts rename to packages/backend-app-api/src/lib/http/readCorsOptions.test.ts diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts b/packages/backend-app-api/src/lib/http/readCorsOptions.ts similarity index 100% rename from packages/backend-app-api/src/services/implementations/rootHttpRouter/readCorsOptions.ts rename to packages/backend-app-api/src/lib/http/readCorsOptions.ts diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts b/packages/backend-app-api/src/lib/http/readHelmetOptions.test.ts similarity index 100% rename from packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.test.ts rename to packages/backend-app-api/src/lib/http/readHelmetOptions.test.ts diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts b/packages/backend-app-api/src/lib/http/readHelmetOptions.ts similarity index 100% rename from packages/backend-app-api/src/services/implementations/rootHttpRouter/readHelmetOptions.ts rename to packages/backend-app-api/src/lib/http/readHelmetOptions.ts From 56d7066e8588d8b7ce3a9c3928dff6f0a0c74a45 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 17:23:53 +0100 Subject: [PATCH 18/25] backend-common: reimplement service builder using backend-app-api MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: blam Co-authored-by: Johan Haals Co-authored-by: Fredrik Adelöw Signed-off-by: Patrik Oldsberg --- .../src/service/lib/ServiceBuilderImpl.ts | 152 ++++++------------ 1 file changed, 53 insertions(+), 99 deletions(-) diff --git a/packages/backend-common/src/service/lib/ServiceBuilderImpl.ts b/packages/backend-common/src/service/lib/ServiceBuilderImpl.ts index a3551416dd..f74dcd9b64 100644 --- a/packages/backend-common/src/service/lib/ServiceBuilderImpl.ts +++ b/packages/backend-common/src/service/lib/ServiceBuilderImpl.ts @@ -18,10 +18,9 @@ import { Config } from '@backstage/config'; import compression from 'compression'; import cors from 'cors'; import express, { Router, ErrorRequestHandler } from 'express'; -import helmet from 'helmet'; +import helmet, { HelmetOptions } from 'helmet'; import { ContentSecurityPolicyOptions } from 'helmet/dist/types/middlewares/content-security-policy'; import * as http from 'http'; -import stoppable from 'stoppable'; import { LoggerService } from '@backstage/backend-plugin-api'; import { useHotCleanup } from '../../hot'; import { getRootLogger } from '../../logging'; @@ -32,26 +31,20 @@ import { } from '../../middleware'; import { RequestLoggingHandlerFactory, ServiceBuilder } from '../types'; import { - CspOptions, - HttpsSettings, - readBaseOptions, readCorsOptions, - readCspOptions, - readHttpsSettings, -} from './config'; -import { createHttpServer, createHttpsServer } from './hostFactory'; + readHelmetOptions, + readHttpServerOptions, + HttpServerOptions, + createHttpServer, +} from '@backstage/backend-app-api'; -export const DEFAULT_PORT = 7007; -// '' is express default, which listens to all interfaces -const DEFAULT_HOST = ''; +export type CspOptions = Record; export class ServiceBuilderImpl implements ServiceBuilder { - private port: number | undefined; - private host: string | undefined; private logger: LoggerService | undefined; - private corsOptions: cors.CorsOptions | undefined; - private cspOptions: Record | undefined; - private httpsSettings: HttpsSettings | undefined; + private serverOptions: HttpServerOptions; + private helmetOptions: HelmetOptions; + private corsOptions: cors.CorsOptions; private routers: [string, Router][]; private requestLoggingHandler: RequestLoggingHandlerFactory | undefined; private errorHandler: ErrorRequestHandler | undefined; @@ -64,50 +57,29 @@ export class ServiceBuilderImpl implements ServiceBuilder { this.routers = []; this.module = moduleRef; this.useDefaultErrorHandler = true; + + this.serverOptions = readHttpServerOptions(); + this.corsOptions = readCorsOptions(); + this.helmetOptions = readHelmetOptions(); } loadConfig(config: Config): ServiceBuilder { const backendConfig = config.getOptionalConfig('backend'); - if (!backendConfig) { - return this; - } - const baseOptions = readBaseOptions(backendConfig); - if (baseOptions.listenPort) { - this.port = - typeof baseOptions.listenPort === 'string' - ? parseInt(baseOptions.listenPort, 10) - : baseOptions.listenPort; - } - if (baseOptions.listenHost) { - this.host = baseOptions.listenHost; - } - - const corsOptions = readCorsOptions(backendConfig); - if (corsOptions) { - this.corsOptions = corsOptions; - } - - const cspOptions = readCspOptions(backendConfig); - if (cspOptions) { - this.cspOptions = cspOptions; - } - - const httpsSettings = readHttpsSettings(backendConfig); - if (httpsSettings) { - this.httpsSettings = httpsSettings; - } + this.serverOptions = readHttpServerOptions(backendConfig); + this.corsOptions = readCorsOptions(backendConfig); + this.helmetOptions = readHelmetOptions(backendConfig); return this; } setPort(port: number): ServiceBuilder { - this.port = port; + this.serverOptions.listen.port = port; return this; } setHost(host: string): ServiceBuilder { - this.host = host; + this.serverOptions.listen.host = host; return this; } @@ -116,8 +88,24 @@ export class ServiceBuilderImpl implements ServiceBuilder { return this; } - setHttpsSettings(settings: HttpsSettings): ServiceBuilder { - this.httpsSettings = settings; + setHttpsSettings(settings: { + certificate: { key: string; cert: string } | { hostname: string }; + }): ServiceBuilder { + if ('hostname' in settings.certificate) { + this.serverOptions.https = { + certificate: { + ...settings.certificate, + type: 'generated', + }, + }; + } else { + this.serverOptions.https = { + certificate: { + ...settings.certificate, + type: 'plain', + }, + }; + } return this; } @@ -127,7 +115,11 @@ export class ServiceBuilderImpl implements ServiceBuilder { } setCsp(options: CspOptions): ServiceBuilder { - this.cspOptions = options; + const csp = this.helmetOptions.contentSecurityPolicy; + this.helmetOptions.contentSecurityPolicy = { + ...(typeof csp === 'object' ? csp : {}), + directives: applyCspDirectives(options), + }; return this; } @@ -155,13 +147,10 @@ export class ServiceBuilderImpl implements ServiceBuilder { async start(): Promise { const app = express(); - const { port, host, logger, corsOptions, httpsSettings, helmetOptions } = - this.getOptions(); + const logger = this.logger ?? getRootLogger(); - app.use(helmet(helmetOptions)); - if (corsOptions) { - app.use(cors(corsOptions)); - } + app.use(helmet(this.helmetOptions)); + app.use(cors(this.corsOptions)); app.use(compression()); app.use( (this.requestLoggingHandler ?? defaultRequestLoggingHandler)(logger), @@ -179,58 +168,23 @@ export class ServiceBuilderImpl implements ServiceBuilder { app.use(defaultErrorHandler()); } - const server: http.Server = httpsSettings - ? await createHttpsServer(app, httpsSettings, logger) - : createHttpServer(app, logger); - const stoppableServer = stoppable(server, 0); + const server = await createHttpServer(app, this.serverOptions, { logger }); useHotCleanup(this.module, () => - stoppableServer.stop((e: any) => { - if (e) console.error(e); + server.stop().catch(error => { + console.error(error); }), ); - return new Promise((resolve, reject) => { - function handleStartupError(e: unknown) { - server.close(); - reject(e); - } + await server.start(); - server.on('error', handleStartupError); - - server.listen(port, host, () => { - server.off('error', handleStartupError); - logger.info(`Listening on ${host}:${port}`); - resolve(stoppableServer); - }); - }); - } - - private getOptions() { - return { - port: this.port ?? DEFAULT_PORT, - host: this.host ?? DEFAULT_HOST, - logger: this.logger ?? getRootLogger(), - corsOptions: this.corsOptions, - httpsSettings: this.httpsSettings, - helmetOptions: { - contentSecurityPolicy: { - useDefaults: false, - directives: applyCspDirectives(this.cspOptions), - }, - // These are all disabled in order to maintain backwards compatibility - // when bumping helmet v5. We can't enable these by default because - // there is no way for users to configure them. - // TODO(Rugvip): We should give control of this setup to consumers - crossOriginEmbedderPolicy: false, - crossOriginOpenerPolicy: false, - crossOriginResourcePolicy: false, - originAgentCluster: false, - }, - }; + return server; } } +// TODO(Rugvip): This is a duplicate of the same logic over in backend-app-api. +// It's needed as we don't want to export this helper from there, but need +// It to implement the setCsp method here. export function applyCspDirectives( directives: Record | undefined, ): ContentSecurityPolicyOptions['directives'] { From 243f38de5a14ca01f5d5b4d44e3bc242939d6cdb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 17:24:33 +0100 Subject: [PATCH 19/25] backend-app-api: move http dir out of lib Signed-off-by: Patrik Oldsberg --- .../src/{lib => }/http/MiddlewareFactory.test.ts | 0 .../backend-app-api/src/{lib => }/http/MiddlewareFactory.ts | 0 packages/backend-app-api/src/{lib => }/http/config.test.ts | 0 packages/backend-app-api/src/{lib => }/http/config.ts | 0 packages/backend-app-api/src/{lib => }/http/createHttpServer.ts | 0 .../src/{lib => }/http/getGeneratedCertificate.ts | 0 packages/backend-app-api/src/{lib => }/http/index.ts | 0 .../backend-app-api/src/{lib => }/http/readCorsOptions.test.ts | 0 packages/backend-app-api/src/{lib => }/http/readCorsOptions.ts | 0 .../src/{lib => }/http/readHelmetOptions.test.ts | 0 .../backend-app-api/src/{lib => }/http/readHelmetOptions.ts | 0 packages/backend-app-api/src/{lib => }/http/startHttpServer.ts | 0 packages/backend-app-api/src/{lib => }/http/types.ts | 0 packages/backend-app-api/src/index.ts | 2 +- .../implementations/rootHttpRouter/rootHttpRouterFactory.ts | 2 +- 15 files changed, 2 insertions(+), 2 deletions(-) rename packages/backend-app-api/src/{lib => }/http/MiddlewareFactory.test.ts (100%) rename packages/backend-app-api/src/{lib => }/http/MiddlewareFactory.ts (100%) rename packages/backend-app-api/src/{lib => }/http/config.test.ts (100%) rename packages/backend-app-api/src/{lib => }/http/config.ts (100%) rename packages/backend-app-api/src/{lib => }/http/createHttpServer.ts (100%) rename packages/backend-app-api/src/{lib => }/http/getGeneratedCertificate.ts (100%) rename packages/backend-app-api/src/{lib => }/http/index.ts (100%) rename packages/backend-app-api/src/{lib => }/http/readCorsOptions.test.ts (100%) rename packages/backend-app-api/src/{lib => }/http/readCorsOptions.ts (100%) rename packages/backend-app-api/src/{lib => }/http/readHelmetOptions.test.ts (100%) rename packages/backend-app-api/src/{lib => }/http/readHelmetOptions.ts (100%) rename packages/backend-app-api/src/{lib => }/http/startHttpServer.ts (100%) rename packages/backend-app-api/src/{lib => }/http/types.ts (100%) diff --git a/packages/backend-app-api/src/lib/http/MiddlewareFactory.test.ts b/packages/backend-app-api/src/http/MiddlewareFactory.test.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/MiddlewareFactory.test.ts rename to packages/backend-app-api/src/http/MiddlewareFactory.test.ts diff --git a/packages/backend-app-api/src/lib/http/MiddlewareFactory.ts b/packages/backend-app-api/src/http/MiddlewareFactory.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/MiddlewareFactory.ts rename to packages/backend-app-api/src/http/MiddlewareFactory.ts diff --git a/packages/backend-app-api/src/lib/http/config.test.ts b/packages/backend-app-api/src/http/config.test.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/config.test.ts rename to packages/backend-app-api/src/http/config.test.ts diff --git a/packages/backend-app-api/src/lib/http/config.ts b/packages/backend-app-api/src/http/config.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/config.ts rename to packages/backend-app-api/src/http/config.ts diff --git a/packages/backend-app-api/src/lib/http/createHttpServer.ts b/packages/backend-app-api/src/http/createHttpServer.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/createHttpServer.ts rename to packages/backend-app-api/src/http/createHttpServer.ts diff --git a/packages/backend-app-api/src/lib/http/getGeneratedCertificate.ts b/packages/backend-app-api/src/http/getGeneratedCertificate.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/getGeneratedCertificate.ts rename to packages/backend-app-api/src/http/getGeneratedCertificate.ts diff --git a/packages/backend-app-api/src/lib/http/index.ts b/packages/backend-app-api/src/http/index.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/index.ts rename to packages/backend-app-api/src/http/index.ts diff --git a/packages/backend-app-api/src/lib/http/readCorsOptions.test.ts b/packages/backend-app-api/src/http/readCorsOptions.test.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/readCorsOptions.test.ts rename to packages/backend-app-api/src/http/readCorsOptions.test.ts diff --git a/packages/backend-app-api/src/lib/http/readCorsOptions.ts b/packages/backend-app-api/src/http/readCorsOptions.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/readCorsOptions.ts rename to packages/backend-app-api/src/http/readCorsOptions.ts diff --git a/packages/backend-app-api/src/lib/http/readHelmetOptions.test.ts b/packages/backend-app-api/src/http/readHelmetOptions.test.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/readHelmetOptions.test.ts rename to packages/backend-app-api/src/http/readHelmetOptions.test.ts diff --git a/packages/backend-app-api/src/lib/http/readHelmetOptions.ts b/packages/backend-app-api/src/http/readHelmetOptions.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/readHelmetOptions.ts rename to packages/backend-app-api/src/http/readHelmetOptions.ts diff --git a/packages/backend-app-api/src/lib/http/startHttpServer.ts b/packages/backend-app-api/src/http/startHttpServer.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/startHttpServer.ts rename to packages/backend-app-api/src/http/startHttpServer.ts diff --git a/packages/backend-app-api/src/lib/http/types.ts b/packages/backend-app-api/src/http/types.ts similarity index 100% rename from packages/backend-app-api/src/lib/http/types.ts rename to packages/backend-app-api/src/http/types.ts diff --git a/packages/backend-app-api/src/index.ts b/packages/backend-app-api/src/index.ts index a8cc079d47..9527e35919 100644 --- a/packages/backend-app-api/src/index.ts +++ b/packages/backend-app-api/src/index.ts @@ -20,6 +20,6 @@ * @packageDocumentation */ -export * from './lib/http'; +export * from './http'; export * from './wiring'; export * from './services/implementations'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index f82cfe4a69..ebd2945545 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -22,7 +22,7 @@ import { LoggerService, } from '@backstage/backend-plugin-api'; import express, { RequestHandler, Express } from 'express'; -import { MiddlewareFactory, startHttpServer } from '../../../lib/http'; +import { MiddlewareFactory, startHttpServer } from '../../../http'; import { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; interface RootHttpRouterConfigureOptions { From d92cbd02050fcff15d45bcc053a8484f6c1f7cfd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 21:44:41 +0100 Subject: [PATCH 20/25] backend-app-api: update API report + fixes Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 151 +++++++++++++++++- .../src/http/MiddlewareFactory.ts | 2 +- packages/backend-app-api/src/http/index.ts | 17 +- .../src/http/readCorsOptions.ts | 1 + .../src/http/readHelmetOptions.ts | 1 + .../implementations/rootHttpRouter/index.ts | 5 +- .../rootHttpRouter/rootHttpRouterFactory.ts | 5 +- .../src/database/migrateBackendTasks.ts | 10 +- 8 files changed, 175 insertions(+), 17 deletions(-) diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index a402150755..24499f6c86 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -3,21 +3,38 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +/// +/// + import { BackendFeature } from '@backstage/backend-plugin-api'; +import { Config } from '@backstage/config'; import { ConfigService } from '@backstage/backend-plugin-api'; +import cors from 'cors'; +import { CorsOptions } from 'cors'; +import { ErrorRequestHandler } from 'express'; +import { Express as Express_2 } from 'express'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; -import { Handler } from 'express'; +import { HelmetOptions } from 'helmet'; +import * as http from 'http'; import { HttpRouterService } from '@backstage/backend-plugin-api'; +import { IncomingMessage } from 'http'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { ParamsDictionary } from 'express-serve-static-core'; +import { ParsedQs } from 'qs'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { PluginCacheManager } from '@backstage/backend-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { Request as Request_2 } from 'express'; +import { RequestHandler } from 'express'; +import { RequestListener } from 'http'; +import { Response as Response_2 } from 'express'; import { RootHttpRouterService } from '@backstage/backend-plugin-api'; import { RootLifecycleService } from '@backstage/backend-plugin-api'; import { RootLoggerService } from '@backstage/backend-plugin-api'; import { SchedulerService } from '@backstage/backend-plugin-api'; +import { ServerResponse } from 'http'; import { ServiceFactory } from '@backstage/backend-plugin-api'; import { ServiceFactoryOrFunction } from '@backstage/backend-plugin-api'; import { ServiceRef } from '@backstage/backend-plugin-api'; @@ -44,6 +61,15 @@ export const configFactory: ( options?: undefined, ) => ServiceFactory; +// @public +export function createHttpServer( + listener: RequestListener, + options: HttpServerOptions, + deps: { + logger: LoggerService; + }, +): Promise; + // @public (undocumented) export function createSpecializedBackend( options: CreateSpecializedBackendOptions, @@ -65,6 +91,16 @@ export const discoveryFactory: ( options?: undefined, ) => ServiceFactory; +// @public +export interface ExtendedHttpServer extends http.Server { + // (undocumented) + port(): number; + // (undocumented) + start(): Promise; + // (undocumented) + stop(): Promise; +} + // @public (undocumented) export const httpRouterFactory: ( options?: HttpRouterFactoryOptions | undefined, @@ -75,6 +111,29 @@ export type HttpRouterFactoryOptions = { getPath(pluginId: string): string; }; +// @public +export type HttpServerCertificateOptions = + | { + type: 'plain'; + key: string; + cert: string; + } + | { + type: 'generated'; + hostname: string; + }; + +// @public +export type HttpServerOptions = { + listen: { + port: number; + host: string; + }; + https?: { + certificate: HttpServerCertificateOptions; + }; +}; + // @public export const lifecycleFactory: ( options?: undefined, @@ -85,11 +144,83 @@ export const loggerFactory: ( options?: undefined, ) => ServiceFactory; +// @public +export class MiddlewareFactory { + compression(): RequestHandler< + ParamsDictionary, + any, + any, + ParsedQs, + Record + >; + cors(): ( + req: cors.CorsRequest, + res: { + statusCode?: number | undefined; + setHeader(key: string, value: string): any; + end(): any; + }, + next: (err?: any) => any, + ) => void; + static create(options: MiddlewareFactoryOptions): MiddlewareFactory; + error(options?: MiddlewareFactoryErrorOptions): ErrorRequestHandler; + helmet(): ( + req: IncomingMessage, + res: ServerResponse, + next: (err?: unknown) => void, + ) => void; + logging(): ( + req: IncomingMessage, + res: ServerResponse, + callback: (err?: Error | undefined) => void, + ) => void; + notFound(): (_req: Request_2, res: Response_2) => void; +} + +// @public +export interface MiddlewareFactoryErrorOptions { + logAllErrors?: boolean; + showStackTraces?: boolean; +} + +// @public +export interface MiddlewareFactoryOptions { + // (undocumented) + config: ConfigService; + // (undocumented) + logger: LoggerService; +} + // @public (undocumented) export const permissionsFactory: ( options?: undefined, ) => ServiceFactory; +// @public +export function readCorsOptions(config?: Config): CorsOptions; + +// @public +export function readHelmetOptions(config?: Config): HelmetOptions; + +// @public +export function readHttpServerOptions(config?: Config): HttpServerOptions; + +// @public (undocumented) +export interface RootHttpRouterConfigureOptions { + // (undocumented) + app: Express_2; + // (undocumented) + config: ConfigService; + // (undocumented) + lifecycle: LifecycleService; + // (undocumented) + logger: LoggerService; + // (undocumented) + middleware: MiddlewareFactory; + // (undocumented) + routes: RequestHandler; +} + // @public (undocumented) export const rootHttpRouterFactory: ( options?: RootHttpRouterFactoryOptions | undefined, @@ -98,7 +229,7 @@ export const rootHttpRouterFactory: ( // @public (undocumented) export type RootHttpRouterFactoryOptions = { indexPath?: string | false; - middleware?: Handler[]; + configure?(options: RootHttpRouterConfigureOptions): void; }; // @public @@ -121,6 +252,22 @@ export type ServiceOrExtensionPoint = | ExtensionPoint | ServiceRef; +// @public +export function startHttpServer( + listener: RequestListener, + options: StartHttpServerOptions, +): Promise; + +// @public +export interface StartHttpServerOptions { + // (undocumented) + config: ConfigService; + // (undocumented) + lifecycle: RootLifecycleService; + // (undocumented) + logger: RootLoggerService; +} + // @public (undocumented) export const tokenManagerFactory: ( options?: undefined, diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.ts b/packages/backend-app-api/src/http/MiddlewareFactory.ts index 4be8467db3..09e6709793 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.ts +++ b/packages/backend-app-api/src/http/MiddlewareFactory.ts @@ -44,7 +44,7 @@ export interface MiddlewareFactoryOptions { } /** - * Options passed to the {@link errorHandler} middleware. + * Options passed to the {@link MiddlewareFactory.error} middleware. * * @public */ diff --git a/packages/backend-app-api/src/http/index.ts b/packages/backend-app-api/src/http/index.ts index 8f1a8c5d92..5bd54ec4a5 100644 --- a/packages/backend-app-api/src/http/index.ts +++ b/packages/backend-app-api/src/http/index.ts @@ -14,16 +14,19 @@ * limitations under the License. */ +export { readHttpServerOptions } from './config'; export { createHttpServer } from './createHttpServer'; +export { MiddlewareFactory } from './MiddlewareFactory'; +export type { + MiddlewareFactoryErrorOptions, + MiddlewareFactoryOptions, +} from './MiddlewareFactory'; +export { readCorsOptions } from './readCorsOptions'; +export { readHelmetOptions } from './readHelmetOptions'; export { startHttpServer } from './startHttpServer'; export type { StartHttpServerOptions } from './startHttpServer'; -export { readHttpServerOptions } from './config'; export type { - HttpServerOptions, - HttpServerCertificateOptions, ExtendedHttpServer, + HttpServerCertificateOptions, + HttpServerOptions, } from './types'; -export { MiddlewareFactory } from './MiddlewareFactory'; -export { readHelmetOptions } from './readHelmetOptions'; -export { readCorsOptions } from './readCorsOptions'; -export type { MiddlewareFactoryOptions } from './MiddlewareFactory'; diff --git a/packages/backend-app-api/src/http/readCorsOptions.ts b/packages/backend-app-api/src/http/readCorsOptions.ts index 98b548d02e..b58cae5ded 100644 --- a/packages/backend-app-api/src/http/readCorsOptions.ts +++ b/packages/backend-app-api/src/http/readCorsOptions.ts @@ -21,6 +21,7 @@ import { Minimatch } from 'minimatch'; /** * Attempts to read a CORS options object from the backend configuration object. * + * @public * @param config - The backend configuration object. * @returns A CORS options object, or undefined if no cors configuration is present. * diff --git a/packages/backend-app-api/src/http/readHelmetOptions.ts b/packages/backend-app-api/src/http/readHelmetOptions.ts index 9cdf339844..0555bdef00 100644 --- a/packages/backend-app-api/src/http/readHelmetOptions.ts +++ b/packages/backend-app-api/src/http/readHelmetOptions.ts @@ -22,6 +22,7 @@ import { ContentSecurityPolicyOptions } from 'helmet/dist/types/middlewares/cont /** * Attempts to read Helmet options from the backend configuration object. * + * @public * @param config - The backend configuration object. * @returns A Helmet options object, or undefined if no Helmet configuration is present. * diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts index 9b72f17060..1dfd72273d 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/index.ts @@ -15,4 +15,7 @@ */ export { rootHttpRouterFactory } from './rootHttpRouterFactory'; -export type { RootHttpRouterFactoryOptions } from './rootHttpRouterFactory'; +export type { + RootHttpRouterFactoryOptions, + RootHttpRouterConfigureOptions, +} from './rootHttpRouterFactory'; diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index ebd2945545..284b019a2e 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -25,7 +25,10 @@ import express, { RequestHandler, Express } from 'express'; import { MiddlewareFactory, startHttpServer } from '../../../http'; import { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; -interface RootHttpRouterConfigureOptions { +/** + * @public + */ +export interface RootHttpRouterConfigureOptions { app: Express; middleware: MiddlewareFactory; routes: RequestHandler; diff --git a/packages/backend-tasks/src/database/migrateBackendTasks.ts b/packages/backend-tasks/src/database/migrateBackendTasks.ts index cc0f4349ac..9530668e3e 100644 --- a/packages/backend-tasks/src/database/migrateBackendTasks.ts +++ b/packages/backend-tasks/src/database/migrateBackendTasks.ts @@ -18,12 +18,12 @@ import { resolvePackagePath } from '@backstage/backend-common'; import { Knex } from 'knex'; import { DB_MIGRATIONS_TABLE } from './tables'; -const migrationsDir = resolvePackagePath( - '@backstage/backend-tasks', - 'migrations', -); - export async function migrateBackendTasks(knex: Knex): Promise { + const migrationsDir = resolvePackagePath( + '@backstage/backend-tasks', + 'migrations', + ); + await knex.migrate.latest({ directory: migrationsDir, tableName: DB_MIGRATIONS_TABLE, From b99c030f1b369f0b8fb7c66692f95f38c88fff20 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 21:48:21 +0100 Subject: [PATCH 21/25] added changesets for the root API refactor and move from backend-common Signed-off-by: Patrik Oldsberg --- .changeset/fair-seals-fry.md | 5 +++++ .changeset/kind-badgers-know.md | 5 +++++ .changeset/orange-spoons-approve.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/fair-seals-fry.md create mode 100644 .changeset/kind-badgers-know.md create mode 100644 .changeset/orange-spoons-approve.md diff --git a/.changeset/fair-seals-fry.md b/.changeset/fair-seals-fry.md new file mode 100644 index 0000000000..09b8970357 --- /dev/null +++ b/.changeset/fair-seals-fry.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Moved over implementation of the root HTTP service from `@backstage/backend-common`, and replaced the `middleware` option with a `configure` callback option. diff --git a/.changeset/kind-badgers-know.md b/.changeset/kind-badgers-know.md new file mode 100644 index 0000000000..e01351cb5e --- /dev/null +++ b/.changeset/kind-badgers-know.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-tasks': patch +--- + +Minor internal refactor to avoid import cycle issue. diff --git a/.changeset/orange-spoons-approve.md b/.changeset/orange-spoons-approve.md new file mode 100644 index 0000000000..96b1eda6fc --- /dev/null +++ b/.changeset/orange-spoons-approve.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Refactor to rely on `@backstage/backend-app-api` for the implementation of `createServiceBuilder`. From c028fa124d654a29b421fc83988b13bb4c401f3b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Jan 2023 22:09:39 +0100 Subject: [PATCH 22/25] backend-app-api: explicit request handler types for MiddlewareFactory Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 40 +++---------------- .../src/http/MiddlewareFactory.ts | 18 ++++++--- 2 files changed, 17 insertions(+), 41 deletions(-) diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 24499f6c86..18c2c52370 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -4,12 +4,10 @@ ```ts /// -/// import { BackendFeature } from '@backstage/backend-plugin-api'; import { Config } from '@backstage/config'; import { ConfigService } from '@backstage/backend-plugin-api'; -import cors from 'cors'; import { CorsOptions } from 'cors'; import { ErrorRequestHandler } from 'express'; import { Express as Express_2 } from 'express'; @@ -17,24 +15,18 @@ import { ExtensionPoint } from '@backstage/backend-plugin-api'; import { HelmetOptions } from 'helmet'; import * as http from 'http'; import { HttpRouterService } from '@backstage/backend-plugin-api'; -import { IncomingMessage } from 'http'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { LoggerService } from '@backstage/backend-plugin-api'; -import { ParamsDictionary } from 'express-serve-static-core'; -import { ParsedQs } from 'qs'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { PluginCacheManager } from '@backstage/backend-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { Request as Request_2 } from 'express'; import { RequestHandler } from 'express'; import { RequestListener } from 'http'; -import { Response as Response_2 } from 'express'; import { RootHttpRouterService } from '@backstage/backend-plugin-api'; import { RootLifecycleService } from '@backstage/backend-plugin-api'; import { RootLoggerService } from '@backstage/backend-plugin-api'; import { SchedulerService } from '@backstage/backend-plugin-api'; -import { ServerResponse } from 'http'; import { ServiceFactory } from '@backstage/backend-plugin-api'; import { ServiceFactoryOrFunction } from '@backstage/backend-plugin-api'; import { ServiceRef } from '@backstage/backend-plugin-api'; @@ -146,35 +138,13 @@ export const loggerFactory: ( // @public export class MiddlewareFactory { - compression(): RequestHandler< - ParamsDictionary, - any, - any, - ParsedQs, - Record - >; - cors(): ( - req: cors.CorsRequest, - res: { - statusCode?: number | undefined; - setHeader(key: string, value: string): any; - end(): any; - }, - next: (err?: any) => any, - ) => void; + compression(): RequestHandler; + cors(): RequestHandler; static create(options: MiddlewareFactoryOptions): MiddlewareFactory; error(options?: MiddlewareFactoryErrorOptions): ErrorRequestHandler; - helmet(): ( - req: IncomingMessage, - res: ServerResponse, - next: (err?: unknown) => void, - ) => void; - logging(): ( - req: IncomingMessage, - res: ServerResponse, - callback: (err?: Error | undefined) => void, - ) => void; - notFound(): (_req: Request_2, res: Response_2) => void; + helmet(): RequestHandler; + logging(): RequestHandler; + notFound(): RequestHandler; } // @public diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.ts b/packages/backend-app-api/src/http/MiddlewareFactory.ts index 09e6709793..dc9fe9aa5f 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.ts +++ b/packages/backend-app-api/src/http/MiddlewareFactory.ts @@ -15,7 +15,13 @@ */ import { ConfigService, LoggerService } from '@backstage/backend-plugin-api'; -import { Request, Response, ErrorRequestHandler, NextFunction } from 'express'; +import { + Request, + Response, + ErrorRequestHandler, + NextFunction, + RequestHandler, +} from 'express'; import cors from 'cors'; import helmet from 'helmet'; import morgan from 'morgan'; @@ -95,7 +101,7 @@ export class MiddlewareFactory { * * @returns An Express request handler */ - notFound() { + notFound(): RequestHandler { return (_req: Request, res: Response) => { res.status(404).end(); }; @@ -109,7 +115,7 @@ export class MiddlewareFactory { * The middleware will attempt to compress response bodies for all requests * that traverse through the middleware. */ - compression() { + compression(): RequestHandler { return compression(); } @@ -124,7 +130,7 @@ export class MiddlewareFactory { * * @returns An Express request handler */ - logging() { + logging(): RequestHandler { const logger = this.#logger.child({ type: 'incomingRequest', }); @@ -150,7 +156,7 @@ export class MiddlewareFactory { * * @returns An Express request handler */ - helmet() { + helmet(): RequestHandler { return helmet(readHelmetOptions(this.#config.getOptionalConfig('backend'))); } @@ -166,7 +172,7 @@ export class MiddlewareFactory { * * @returns An Express request handler */ - cors() { + cors(): RequestHandler { return cors(readCorsOptions(this.#config.getOptionalConfig('backend'))); } From bd20e8b925a190576bdd8ccb4441b47dd0804dfd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 10:16:52 +0100 Subject: [PATCH 23/25] backend-app-api: fix infinite loop in http server stop Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/src/http/createHttpServer.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/backend-app-api/src/http/createHttpServer.ts b/packages/backend-app-api/src/http/createHttpServer.ts index f72269c079..5c526ba1e2 100644 --- a/packages/backend-app-api/src/http/createHttpServer.ts +++ b/packages/backend-app-api/src/http/createHttpServer.ts @@ -35,6 +35,10 @@ export async function createHttpServer( const server = await createServer(listener, options, deps); const stopper = stoppableServer(server, 0); + // The stopper here is actually the server itself, so if we try + // to call stopper.stop() down in the stop implementation, we'll + // be calling ourselves. + const stopServer = stopper.stop.bind(stopper); return Object.assign(server, { start() { @@ -57,7 +61,7 @@ export async function createHttpServer( stop() { return new Promise((resolve, reject) => { - stopper.stop((error?: Error) => { + stopServer((error?: Error) => { if (error) { reject(error); } else { From bff790af9c4a8c0f96bd3ecd3150062c7e66e3d8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 12:19:06 +0100 Subject: [PATCH 24/25] backend-app-api: remove startHttpServer Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 16 ----- packages/backend-app-api/src/http/index.ts | 2 - .../src/http/startHttpServer.ts | 62 ------------------- .../rootHttpRouter/rootHttpRouterFactory.ts | 21 ++++++- 4 files changed, 19 insertions(+), 82 deletions(-) delete mode 100644 packages/backend-app-api/src/http/startHttpServer.ts diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 18c2c52370..cc84309a2f 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -222,22 +222,6 @@ export type ServiceOrExtensionPoint = | ExtensionPoint | ServiceRef; -// @public -export function startHttpServer( - listener: RequestListener, - options: StartHttpServerOptions, -): Promise; - -// @public -export interface StartHttpServerOptions { - // (undocumented) - config: ConfigService; - // (undocumented) - lifecycle: RootLifecycleService; - // (undocumented) - logger: RootLoggerService; -} - // @public (undocumented) export const tokenManagerFactory: ( options?: undefined, diff --git a/packages/backend-app-api/src/http/index.ts b/packages/backend-app-api/src/http/index.ts index 5bd54ec4a5..4a9ec14cf8 100644 --- a/packages/backend-app-api/src/http/index.ts +++ b/packages/backend-app-api/src/http/index.ts @@ -23,8 +23,6 @@ export type { } from './MiddlewareFactory'; export { readCorsOptions } from './readCorsOptions'; export { readHelmetOptions } from './readHelmetOptions'; -export { startHttpServer } from './startHttpServer'; -export type { StartHttpServerOptions } from './startHttpServer'; export type { ExtendedHttpServer, HttpServerCertificateOptions, diff --git a/packages/backend-app-api/src/http/startHttpServer.ts b/packages/backend-app-api/src/http/startHttpServer.ts deleted file mode 100644 index 64d0be3f1e..0000000000 --- a/packages/backend-app-api/src/http/startHttpServer.ts +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2023 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 { - ConfigService, - RootLifecycleService, - RootLoggerService, -} from '@backstage/backend-plugin-api'; -import { RequestListener } from 'http'; -import { readHttpServerOptions } from './config'; -import { createHttpServer } from './createHttpServer'; - -/** - * Options for {@link startHttpServer}. - * - * @public - */ -export interface StartHttpServerOptions { - config: ConfigService; - logger: RootLoggerService; - lifecycle: RootLifecycleService; -} - -/** - * Starts up an HTTP server, as well as registers a shutdown handler that stops the server. - * - * @public - */ -export async function startHttpServer( - listener: RequestListener, - options: StartHttpServerOptions, -) { - const { config, logger, lifecycle } = options; - - const server = await createHttpServer( - listener, - readHttpServerOptions(config.getOptionalConfig('backend')), - { logger }, - ); - - lifecycle.addShutdownHook({ - async fn() { - await server.stop(); - }, - labels: { type: 'httpServer' }, - }); - - await server.start(); -} diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index 284b019a2e..4b021b977c 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -22,7 +22,11 @@ import { LoggerService, } from '@backstage/backend-plugin-api'; import express, { RequestHandler, Express } from 'express'; -import { MiddlewareFactory, startHttpServer } from '../../../http'; +import { + createHttpServer, + MiddlewareFactory, + readHttpServerOptions, +} from '../../../http'; import { RestrictedIndexedRouter } from './RestrictedIndexedRouter'; /** @@ -93,7 +97,20 @@ export const rootHttpRouterFactory = createServiceFactory({ lifecycle, }); - await startHttpServer(app, { config, logger, lifecycle }); + const server = await createHttpServer( + app, + readHttpServerOptions(config.getOptionalConfig('backend')), + { logger }, + ); + + lifecycle.addShutdownHook({ + async fn() { + await server.stop(); + }, + labels: { service: 'rootHttpRouter' }, + }); + + await server.start(); return router; }, From c45be22cc031171511da4fd07d0f30ff15834ca9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 12:52:55 +0100 Subject: [PATCH 25/25] backend-app-api: create a labelled logger for rootHttpRouter Signed-off-by: Patrik Oldsberg --- .../implementations/rootHttpRouter/rootHttpRouterFactory.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts index 4b021b977c..d8cedda0d9 100644 --- a/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootHttpRouter/rootHttpRouterFactory.ts @@ -72,17 +72,18 @@ export const rootHttpRouterFactory = createServiceFactory({ service: coreServices.rootHttpRouter, deps: { config: coreServices.config, - logger: coreServices.rootLogger, + rootLogger: coreServices.rootLogger, lifecycle: coreServices.rootLifecycle, }, async factory( - { config, logger, lifecycle }, + { config, rootLogger, lifecycle }, { indexPath, configure = defaultConfigure, }: RootHttpRouterFactoryOptions = {}, ) { const router = new RestrictedIndexedRouter(indexPath ?? '/api/app'); + const logger = rootLogger.child({ service: 'rootHttpRouter' }); const app = express();