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