From 023ff80f48d551b8bf3eaa50524772889e5f4d7b Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Sun, 29 Nov 2020 23:24:29 +0100 Subject: [PATCH] refactor: Separate middleware from Express. --- config/config-default.json | 1 + config/config-file.json | 1 + config/config-path-routing.json | 1 + config/config-rdf-to-sparql-endpoint.json | 1 + config/config-sparql-endpoint.json | 1 + config/presets/http.json | 12 ++++ config/presets/ldp.json | 2 +- config/presets/middleware.json | 17 +++++ index.ts | 4 ++ src/server/ExpressHttpServerFactory.ts | 24 +------ src/server/middleware/CorsHandler.ts | 26 +++++++ src/server/middleware/HeaderHandler.ts | 22 ++++++ test/configs/middleware.json | 13 ++++ test/configs/websockets.json | 3 +- test/integration/Middleware.test.ts | 69 +++++++++++++++++++ .../server/ExpressHttpServerFactory.test.ts | 31 --------- .../server/middleware/CorsHandler.test.ts | 33 +++++++++ .../server/middleware/HeaderHandler.test.ts | 20 ++++++ 18 files changed, 227 insertions(+), 54 deletions(-) create mode 100644 config/presets/middleware.json create mode 100644 src/server/middleware/CorsHandler.ts create mode 100644 src/server/middleware/HeaderHandler.ts create mode 100644 test/configs/middleware.json create mode 100644 test/integration/Middleware.test.ts create mode 100644 test/unit/server/middleware/CorsHandler.test.ts create mode 100644 test/unit/server/middleware/HeaderHandler.test.ts diff --git a/config/config-default.json b/config/config-default.json index 7041c27ae..0940ce089 100644 --- a/config/config-default.json +++ b/config/config-default.json @@ -12,6 +12,7 @@ "files-scs:config/presets/ldp/request-parser.json", "files-scs:config/presets/ldp/websockets.json", "files-scs:config/presets/logging.json", + "files-scs:config/presets/middleware.json", "files-scs:config/presets/representation-conversion.json", "files-scs:config/presets/setup.json", "files-scs:config/presets/storage/backend/storage-memory.json", diff --git a/config/config-file.json b/config/config-file.json index fc34411bc..fe93fb6c2 100644 --- a/config/config-file.json +++ b/config/config-file.json @@ -12,6 +12,7 @@ "files-scs:config/presets/ldp/request-parser.json", "files-scs:config/presets/ldp/websockets.json", "files-scs:config/presets/logging.json", + "files-scs:config/presets/middleware.json", "files-scs:config/presets/representation-conversion.json", "files-scs:config/presets/setup.json", "files-scs:config/presets/storage/backend/storage-filesystem.json", diff --git a/config/config-path-routing.json b/config/config-path-routing.json index 931df203b..7d9d4423a 100644 --- a/config/config-path-routing.json +++ b/config/config-path-routing.json @@ -12,6 +12,7 @@ "files-scs:config/presets/ldp/request-parser.json", "files-scs:config/presets/ldp/websockets.json", "files-scs:config/presets/logging.json", + "files-scs:config/presets/middleware.json", "files-scs:config/presets/representation-conversion.json", "files-scs:config/presets/setup.json", "files-scs:config/presets/storage/backend/storage-filesystem.json", diff --git a/config/config-rdf-to-sparql-endpoint.json b/config/config-rdf-to-sparql-endpoint.json index c055e3787..8671d54fa 100644 --- a/config/config-rdf-to-sparql-endpoint.json +++ b/config/config-rdf-to-sparql-endpoint.json @@ -12,6 +12,7 @@ "files-scs:config/presets/ldp/request-parser.json", "files-scs:config/presets/ldp/websockets.json", "files-scs:config/presets/logging.json", + "files-scs:config/presets/middleware.json", "files-scs:config/presets/representation-conversion.json", "files-scs:config/presets/setup.json", "files-scs:config/presets/storage/backend/storage-filesystem.json", diff --git a/config/config-sparql-endpoint.json b/config/config-sparql-endpoint.json index 9e5b8aea3..945f5b254 100644 --- a/config/config-sparql-endpoint.json +++ b/config/config-sparql-endpoint.json @@ -12,6 +12,7 @@ "files-scs:config/presets/ldp/request-parser.json", "files-scs:config/presets/ldp/websockets.json", "files-scs:config/presets/logging.json", + "files-scs:config/presets/middleware.json", "files-scs:config/presets/representation-conversion.json", "files-scs:config/presets/setup.json", "files-scs:config/presets/storage/backend/storage-sparql-endpoint.json", diff --git a/config/presets/http.json b/config/presets/http.json index 5d3483a34..d960bbe1b 100644 --- a/config/presets/http.json +++ b/config/presets/http.json @@ -17,6 +17,18 @@ "ExpressHttpServerFactory:_handler": { "@id": "urn:solid-server:default:HttpHandler" } + }, + { + "@id": "urn:solid-server:default:HttpHandler", + "@type": "AllVoidCompositeHandler", + "AllVoidCompositeHandler:_handlers": [ + { + "@id": "urn:solid-server:default:Middleware" + }, + { + "@id": "urn:solid-server:default:LdpHandler" + } + ] } ] } diff --git a/config/presets/ldp.json b/config/presets/ldp.json index e5ca8f538..f0957d262 100644 --- a/config/presets/ldp.json +++ b/config/presets/ldp.json @@ -2,7 +2,7 @@ "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld", "@graph": [ { - "@id": "urn:solid-server:default:HttpHandler", + "@id": "urn:solid-server:default:LdpHandler", "@type": "AuthenticatedLdpHandler", "AuthenticatedLdpHandler:_args_requestParser": { "@id": "urn:solid-server:default:RequestParser" diff --git a/config/presets/middleware.json b/config/presets/middleware.json new file mode 100644 index 000000000..8fd11d958 --- /dev/null +++ b/config/presets/middleware.json @@ -0,0 +1,17 @@ +{ + "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld", + "@graph": [ + { + "@id": "urn:solid-server:default:Middleware", + "@type": "AllVoidCompositeHandler", + "AllVoidCompositeHandler:_handlers": [ + { + "@type": "CorsHandler" + }, + { + "@type": "HeaderHandler" + } + ] + } + ] +} diff --git a/index.ts b/index.ts index 717720e72..c0b52d5c1 100644 --- a/index.ts +++ b/index.ts @@ -95,6 +95,10 @@ export * from './src/server/HttpResponse'; export * from './src/server/WebSocketServerFactory'; export * from './src/server/WebSocketHandler'; +// Server/Middleware +export * from './src/server/middleware/CorsHandler'; +export * from './src/server/middleware/HeaderHandler'; + // Storage/Accessors export * from './src/storage/accessors/DataAccessor'; export * from './src/storage/accessors/FileDataAccessor'; diff --git a/src/server/ExpressHttpServerFactory.ts b/src/server/ExpressHttpServerFactory.ts index cf4d3a1c7..3c0224189 100644 --- a/src/server/ExpressHttpServerFactory.ts +++ b/src/server/ExpressHttpServerFactory.ts @@ -1,5 +1,4 @@ import type { Server } from 'http'; -import cors from 'cors'; import type { Express } from 'express'; import express from 'express'; import { getLoggerFor } from '../logging/LogUtil'; @@ -17,28 +16,11 @@ export class ExpressHttpServerFactory implements HttpServerFactory { } public startServer(port: number): Server { - const app = express(); - this.setup(app); - return app.listen(port); + return this.createApp().listen(port); } - protected setup(app: Express): void { - // Set up server identification - app.use((request, response, done): void => { - response.setHeader('X-Powered-By', 'Community Solid Server'); - done(); - }); - - // Set up Cross-Origin Resource Sharing (CORS) - app.use(cors({ - // Based on https://github.com/solid/solid-spec/blob/master/recommendations-server.md#cors---cross-origin-resource-sharing - // By default origin is always '*', this forces it to be the origin header if there is one - origin: (origin, callback): void => callback(null, (origin ?? '*') as any), - methods: [ 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'PATCH', 'DELETE' ], - })); - - // Delegate to the main handler - app.use(async(request, response, done): Promise => { + protected createApp(): Express { + return express().use(async(request, response, done): Promise => { try { this.logger.info(`Received request for ${request.url}`); await this.handler.handleSafe({ request: guardStream(request), response }); diff --git a/src/server/middleware/CorsHandler.ts b/src/server/middleware/CorsHandler.ts new file mode 100644 index 000000000..0a502d8c6 --- /dev/null +++ b/src/server/middleware/CorsHandler.ts @@ -0,0 +1,26 @@ +import cors from 'cors'; +import type { RequestHandler } from 'express'; +import { HttpHandler } from '../HttpHandler'; +import type { HttpRequest } from '../HttpRequest'; +import type { HttpResponse } from '../HttpResponse'; + +/** + * Handler that sets CORS options on the response. + */ +export class CorsHandler extends HttpHandler { + private readonly corsHandler: RequestHandler; + + public constructor() { + super(); + this.corsHandler = cors({ + origin: (origin, callback): void => callback(null, (origin ?? '*') as any), + methods: [ 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'PATCH', 'DELETE' ], + }); + } + + public async handle(input: { request: HttpRequest; response: HttpResponse }): Promise { + return new Promise((resolve): void => { + this.corsHandler(input.request as any, input.response as any, (): void => resolve()); + }); + } +} diff --git a/src/server/middleware/HeaderHandler.ts b/src/server/middleware/HeaderHandler.ts new file mode 100644 index 000000000..33ce334b8 --- /dev/null +++ b/src/server/middleware/HeaderHandler.ts @@ -0,0 +1,22 @@ +import { HttpHandler } from '../HttpHandler'; +import type { HttpResponse } from '../HttpResponse'; + +/** + * Handler that sets custom headers on the response. + */ +export class HeaderHandler extends HttpHandler { + private readonly headers: Record; + + public constructor() { + super(); + this.headers = { + 'x-powered-by': 'Community Solid Server', + }; + } + + public async handle({ response }: { response: HttpResponse }): Promise { + for (const header of Object.keys(this.headers)) { + response.setHeader(header, this.headers[header]); + } + } +} diff --git a/test/configs/middleware.json b/test/configs/middleware.json new file mode 100644 index 000000000..129a9f4b7 --- /dev/null +++ b/test/configs/middleware.json @@ -0,0 +1,13 @@ +{ + "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld", + "import": [ + "files-scs:config/presets/http.json", + "files-scs:config/presets/middleware.json" + ], + "@graph": [ + { + "@id": "urn:solid-server:default:LdpHandler", + "@type": "Variable" + } + ] +} diff --git a/test/configs/websockets.json b/test/configs/websockets.json index 0d7f09c31..651a6cf1a 100644 --- a/test/configs/websockets.json +++ b/test/configs/websockets.json @@ -9,6 +9,7 @@ "files-scs:config/presets/ldp/response-writer.json", "files-scs:config/presets/ldp/request-parser.json", "files-scs:config/presets/ldp/websockets.json", + "files-scs:config/presets/middleware.json", "files-scs:config/presets/representation-conversion.json", "files-scs:config/presets/storage/backend/storage-memory.json", "files-scs:config/presets/storage/routing/no-routing.json", @@ -17,7 +18,7 @@ ], "@graph": [ { - "@id": "urn:solid-server:default:HttpHandler", + "@id": "urn:solid-server:default:LdpHandler", "@type": "AuthenticatedLdpHandler", "AuthenticatedLdpHandler:_args_requestParser": { "@id": "urn:solid-server:default:RequestParser" diff --git a/test/integration/Middleware.test.ts b/test/integration/Middleware.test.ts new file mode 100644 index 000000000..d3fec7c5b --- /dev/null +++ b/test/integration/Middleware.test.ts @@ -0,0 +1,69 @@ +import type { Server } from 'http'; +import request from 'supertest'; +import type { ExpressHttpServerFactory } from '../../src/server/ExpressHttpServerFactory'; +import { HttpHandler } from '../../src/server/HttpHandler'; +import type { HttpRequest } from '../../src/server/HttpRequest'; +import type { HttpResponse } from '../../src/server/HttpResponse'; +import { instantiateFromConfig } from '../configs/Util'; + +const port = 6002; + +class SimpleHttpHandler extends HttpHandler { + public async handle(input: { request: HttpRequest; response: HttpResponse }): Promise { + input.response.writeHead(200); + input.response.end('Hello World'); + } +} + +describe('An Express server with middleware', (): void => { + let server: Server; + + beforeAll(async(): Promise => { + const factory = await instantiateFromConfig( + 'urn:solid-server:default:ExpressHttpServerFactory', 'middleware.json', { + 'urn:solid-server:default:LdpHandler': new SimpleHttpHandler(), + }, + ) as ExpressHttpServerFactory; + server = factory.startServer(port); + }); + + afterEach(async(): Promise => { + server.close(); + }); + + it('sends server identification in the X-Powered-By header.', async(): Promise => { + const res = await request(server).get('/'); + expect(res.header).toEqual(expect.objectContaining({ + 'x-powered-by': 'Community Solid Server', + })); + }); + + it('returns CORS headers for an OPTIONS request.', async(): Promise => { + const res = await request(server) + .options('/') + .set('Access-Control-Request-Headers', 'content-type') + .set('Access-Control-Request-Method', 'POST') + .set('Host', 'test.com') + .expect(204); + expect(res.header).toEqual(expect.objectContaining({ + 'access-control-allow-origin': '*', + 'access-control-allow-headers': 'content-type', + })); + const corsMethods = res.header['access-control-allow-methods'].split(',') + .map((method: string): string => method.trim()); + const allowedMethods = [ 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'PATCH', 'DELETE' ]; + expect(corsMethods).toEqual(expect.arrayContaining(allowedMethods)); + expect(corsMethods).toHaveLength(allowedMethods.length); + }); + + it('specifies CORS origin header if an origin was supplied.', async(): Promise => { + const res = await request(server).get('/').set('origin', 'test.com').expect(200); + expect(res.header).toEqual(expect.objectContaining({ 'access-control-allow-origin': 'test.com' })); + }); + + it('sends incoming requests to the handler.', async(): Promise => { + const response = request(server).get('/').set('Host', 'test.com'); + expect(response).toBeDefined(); + await response.expect(200).expect('Hello World'); + }); +}); diff --git a/test/unit/server/ExpressHttpServerFactory.test.ts b/test/unit/server/ExpressHttpServerFactory.test.ts index eff416d19..335f41d11 100644 --- a/test/unit/server/ExpressHttpServerFactory.test.ts +++ b/test/unit/server/ExpressHttpServerFactory.test.ts @@ -42,7 +42,6 @@ describe('ExpressHttpServerFactory', (): void => { }); afterEach(async(): Promise => { - // Close server server.close(); }); @@ -50,36 +49,6 @@ describe('ExpressHttpServerFactory', (): void => { mock.mockReset(); }); - it('sends server identification in the X-Powered-By header.', async(): Promise => { - const res = await request(server).get('/'); - expect(res.header).toEqual(expect.objectContaining({ - 'x-powered-by': 'Community Solid Server', - })); - }); - - it('returns CORS headers for an OPTIONS request.', async(): Promise => { - const res = await request(server) - .options('/') - .set('Access-Control-Request-Headers', 'content-type') - .set('Access-Control-Request-Method', 'POST') - .set('Host', 'test.com') - .expect(204); - expect(res.header).toEqual(expect.objectContaining({ - 'access-control-allow-origin': '*', - 'access-control-allow-headers': 'content-type', - })); - const corsMethods = res.header['access-control-allow-methods'].split(',') - .map((method: string): string => method.trim()); - const allowedMethods = [ 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'PATCH', 'DELETE' ]; - expect(corsMethods).toEqual(expect.arrayContaining(allowedMethods)); - expect(corsMethods).toHaveLength(allowedMethods.length); - }); - - it('specifies CORS origin header if an origin was supplied.', async(): Promise => { - const res = await request(server).get('/').set('origin', 'test.com').expect(200); - expect(res.header).toEqual(expect.objectContaining({ 'access-control-allow-origin': 'test.com' })); - }); - it('sends incoming requests to the handler.', async(): Promise => { await request(server).get('/').set('Host', 'test.com').expect(200); expect(canHandleJest).toHaveBeenCalledTimes(1); diff --git a/test/unit/server/middleware/CorsHandler.test.ts b/test/unit/server/middleware/CorsHandler.test.ts new file mode 100644 index 000000000..aec83c983 --- /dev/null +++ b/test/unit/server/middleware/CorsHandler.test.ts @@ -0,0 +1,33 @@ +import { createRequest, createResponse } from 'node-mocks-http'; +import { CorsHandler } from '../../../../src/server/middleware/CorsHandler'; +import { guardStream } from '../../../../src/util/GuardedStream'; + +describe('a CorsHandler', (): void => { + let handler: CorsHandler; + + beforeAll(async(): Promise => { + handler = new CorsHandler(); + }); + + it('returns CORS headers.', async(): Promise => { + const request = guardStream(createRequest()); + const response = createResponse(); + await handler.handleSafe({ request, response }); + expect(response.getHeaders()).toEqual(expect.objectContaining({ + 'access-control-allow-origin': '*', + })); + }); + + it('echoes the origin when specified.', async(): Promise => { + const request = guardStream(createRequest({ + headers: { + origin: 'example.org', + }, + })); + const response = createResponse(); + await handler.handleSafe({ request, response }); + expect(response.getHeaders()).toEqual(expect.objectContaining({ + 'access-control-allow-origin': 'example.org', + })); + }); +}); diff --git a/test/unit/server/middleware/HeaderHandler.test.ts b/test/unit/server/middleware/HeaderHandler.test.ts new file mode 100644 index 000000000..6ab777aab --- /dev/null +++ b/test/unit/server/middleware/HeaderHandler.test.ts @@ -0,0 +1,20 @@ +import { createRequest, createResponse } from 'node-mocks-http'; +import { HeaderHandler } from '../../../../src/server/middleware/HeaderHandler'; +import { guardStream } from '../../../../src/util/GuardedStream'; + +describe('a HeaderHandler', (): void => { + let handler: HeaderHandler; + + beforeAll(async(): Promise => { + handler = new HeaderHandler(); + }); + + it('returns an X-Powered-By header.', async(): Promise => { + const request = guardStream(createRequest()); + const response = createResponse(); + await handler.handleSafe({ request, response }); + expect(response.getHeaders()).toEqual(expect.objectContaining({ + 'x-powered-by': 'Community Solid Server', + })); + }); +});