From 7d616d6add3ab3f2ae7c33b643c4c4e67f25448f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 12 Mar 2019 10:28:11 +0100 Subject: [PATCH 1/3] feat(testlab): add dummy HTTPS config Simplify setup of HTTPS servers in tests by enhancing the helper `givenHttpServerConfig` to automatically add dummy TLS key & cert data when the protocol is set to `https`. --- .../integration/rest.server.integration.ts | 2 ++ packages/testlab/fixtures/cert.pem | 21 ++++++++++++ packages/testlab/fixtures/key.pem | 28 +++++++++++++++ packages/testlab/fixtures/pfx.pfx | Bin 0 -> 2477 bytes packages/testlab/src/http-server-config.ts | 32 ++++++++++++++++-- 5 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 packages/testlab/fixtures/cert.pem create mode 100644 packages/testlab/fixtures/key.pem create mode 100644 packages/testlab/fixtures/pfx.pfx diff --git a/packages/rest/src/__tests__/integration/rest.server.integration.ts b/packages/rest/src/__tests__/integration/rest.server.integration.ts index a1beb44302c5..34d507cfdb74 100644 --- a/packages/rest/src/__tests__/integration/rest.server.integration.ts +++ b/packages/rest/src/__tests__/integration/rest.server.integration.ts @@ -683,6 +683,8 @@ paths: const options = { port: 0, protocol: 'https', + key: undefined, + cert: undefined, }; const serverOptions = givenHttpServerConfig(options); const server = await givenAServer({rest: serverOptions}); diff --git a/packages/testlab/fixtures/cert.pem b/packages/testlab/fixtures/cert.pem new file mode 100644 index 000000000000..ca9226e3c3b7 --- /dev/null +++ b/packages/testlab/fixtures/cert.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDaDCCAlACCQDSmMwp5CM+CzANBgkqhkiG9w0BAQUFADB2MQswCQYDVQQGEwJV +UzELMAkGA1UECAwCQ0ExCzAJBgNVBAcMAlNGMQswCQYDVQQKDAJMQjEMMAoGA1UE +CwwDTEI0MRIwEAYDVQQDDAlsb2NhbGhvc3QxHjAcBgkqhkiG9w0BCQEWD2xiNEBl +eGFtcGxlLmNvbTAeFw0xODA2MjgxNDMwNTdaFw0xOTA2MjgxNDMwNTdaMHYxCzAJ +BgNVBAYTAlVTMQswCQYDVQQIDAJDQTELMAkGA1UEBwwCU0YxCzAJBgNVBAoMAkxC +MQwwCgYDVQQLDANMQjQxEjAQBgNVBAMMCWxvY2FsaG9zdDEeMBwGCSqGSIb3DQEJ +ARYPbGI0QGV4YW1wbGUuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC +AQEA3mV25nB7LprWwnw2esZbzuS9vG68Eqcjiu9K0ZO9Ym8al70Wz1Q7ytqfuP4c +DEjEAngvbkrT3W1ZaXUOQz5vxAa5OaLpB7moOZ3cldVyDTwlExBvFrXB5Qqrh/7Y +c7OVvtb3Dah1wzvRHEt8I0EXPnojjae2uxmTu3ThZqACcpZS5SQC3hA3VOmcRpMS +xKd8tvbbYYb37aaldOJkxcKge0C5adpOB8MsDvWZagBDCWaN41Wc/mER71Q1UMrz +BrGB0Let4IibvUcW5nlUlfzu9qjY6ZXdb4cTDA7q6xTHmaIwhLklsI/K2Mda1YC5 +aIu558Kxaq1e3RWb0Hl/RpEQSQIDAQABMA0GCSqGSIb3DQEBBQUAA4IBAQCSdHKL +juogGyAvUH0cAOahhvuUOnfpjcOWscRa1VLpI8lR9hWX5CLt3IIqT3gVFTl8bQbq +joOfUB+ArusERMtay8l/dI83l6BxOkjhz8IcKT89W5emsHPDk6l0DAMmcrZAgMM5 +Ow9Rt3M5dEJ7tY3xWS9WpM3WCSpou+4AZt9PLE/sCqSjkDCO0/+ao1Pr9HORP40n +NOPjSqMjlesUVlfJQTi0Rscal3BQG4+2cNG+p8KzR6KLEJruORuHzRqLWh3jkUKU +snB9FTDkj9kSq287SidEcF2tfi2X6ptAoxv/jdFx6unZ1q3wI0qSDZYaEAbwlO84 +q3Y/oEQkYu19Wzta +-----END CERTIFICATE----- diff --git a/packages/testlab/fixtures/key.pem b/packages/testlab/fixtures/key.pem new file mode 100644 index 000000000000..dbbf628e7c16 --- /dev/null +++ b/packages/testlab/fixtures/key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDeZXbmcHsumtbC +fDZ6xlvO5L28brwSpyOK70rRk71ibxqXvRbPVDvK2p+4/hwMSMQCeC9uStPdbVlp +dQ5DPm/EBrk5oukHuag5ndyV1XINPCUTEG8WtcHlCquH/thzs5W+1vcNqHXDO9Ec +S3wjQRc+eiONp7a7GZO7dOFmoAJyllLlJALeEDdU6ZxGkxLEp3y29tthhvftpqV0 +4mTFwqB7QLlp2k4HwywO9ZlqAEMJZo3jVZz+YRHvVDVQyvMGsYHQt63giJu9Rxbm +eVSV/O72qNjpld1vhxMMDurrFMeZojCEuSWwj8rYx1rVgLloi7nnwrFqrV7dFZvQ +eX9GkRBJAgMBAAECggEABHSh8jH0tdVSUiks6j7JHhcFGh5Z1EHW+3SZ2iMMm0lA +jiOyrkqwu/qvUoR8yV431xjTUnFbV0dWkD9RHtXEZXgBA/+YjZgRn73i6nmRRGSd +FYmxwBG6Jb2V/C6F5aOGb4FdB8AFQ/kR0nBMt2QZdB38Ul020v7LL+lCjszL38HL +qPuZLbvQi4Vs4J8JpO8k2ST3gQhaYWb36XOACaD20mL2xHNpOO5vyBJSnNVb6ueg +jpy1QV/1JOEumYLXFGOudk2QRm/yN2ym4gwpqD3sWIM9iYZsc84K61oibhLRZFtO +luUnuRLrNrzpZfZga2EqEPUEy0IHLij1S9+H2MQTFQKBgQD+A9fwVDnY//PdSDBp ++xlggP2N3RdINi1EStPiLsy//Ok7xSCTXgE09iouZsjaP9r6nSKlG3SO18Hvy4CI +whRzu95Z2vZQLYHuCCwLnqIhpM7xnFL93ueud7ATiE3fGFhJNUMGTYTQ+ZmwFuFQ +7eddWrqVOEqezy2btpnsIVkINwKBgQDgIl4sZ7fl98S64+K0fcB0rCnrciL7OBap +aucHuzmjydaVWW5WkzUOMxh+er2Zeqt1+0cTjnV6J7DJ96d/R8eWkNjTVtULJezf +z91titYbB3O6TwYLx4IzXoweHuC/uLhE27Jxnvgz2IZacK1fKvql1lM5MaP7GDZ8 +VPvmiUFrfwKBgEABs+4JKzJ0/Hwr7pcmALUCi+GtbmpxzGJDALUj2dAe6J55A8Ze +j6tKxEJBACeOo5027z3vdyVvVJ0aCF9tmD25f0PhGuQFM5JJWN/sryoPH15eZ8M0 +4ehinGmvlP+8YLLBywvRiMAnxQRMH6aG7B/n9tAXCSaPSgzMrGiF1qttAoGBAJ51 +Dbk9FpFZ6tbqF7PdF7wkn3padgq/q53Y+z7HCcgXAUMTN+OzLRY933pD0ll4lVHS +9XwJAlr7RoxzLxLYL23uN6yqPfIkvOO6dGRmfFodmZ7FEZQwV4dzt4Hv+JrywCvG +WtDjP7x/vvSfpqKaoxute6b6xmDVzGd4OaLRtNOHAoGAUyockJhGQEkUG9A21DXv +hqR343WeUne1tqwfxkg0DQIBAaaFgGkeL1DjdHhE5ZNz+F/t5LRcvMkZDShRK0u3 +Ytnw2XtSJYtCrPlnDrt1/59dBr7S1nbhStI5xfPojctd0DbVvhC5UfQMKSNHOLCs +tUWwM07FtltvXMoC0xXf5sI= +-----END PRIVATE KEY----- diff --git a/packages/testlab/fixtures/pfx.pfx b/packages/testlab/fixtures/pfx.pfx new file mode 100644 index 0000000000000000000000000000000000000000..afe83cea4150ad2380dcaf6a82bd9c1c561192f7 GIT binary patch literal 2477 zcmY+^c{CJ?9tZH5F*9byE+bn)A>-26iNTe9mwg#)wjs>evLww|lh7Smq9#(v$d;kv z;tEsQuWW^=u@the!_ztMz5CuDzwH1pL*u{z-sn5h&u{8`yzRG$$C=eWpEsOlGtS z1cCt)i7Y*Tb>TXf8@E?HoMb_=NEMA0qlUiyhMh3@-EKJ@>feNHFowIpE=wTVCM|mU zi6`pTt)D%&p^fED(LPRDN6vP5C>`)-OTp|pgInAb=*vl$>3XNG`J+^#W1ya%UzT0b zR+9bfMlAE{?pDW4eo9tH9xQZZH-m9|JZzkN-hgPBUN`k+md*3-K9l)egeLJ5HZz>c zCg{X8Jq@gS*$CX!0yVtQZ8=`_uzuW_T>#`3bndlIcuoJn@8lc(o`9($eR?<&v>A;% z)C#WX&`r%5#6S|%=v|#5#Y!S#^Aj3kf}-5eEHyD|cKSx!+bIiox`vHE-ou!{r|f>* zN=WJy?vG@r#f#rmt5Jf*9xP7-;+3j7hg_ab?{}_iN-_8_Gn6 zo3-|lj=Yb?>v!-wjJhDu?@KR4Ur}(EN^qrV1ycz z$2VvDU?Xbut{y^O(kC~Iojl{#ET?n>{uH0LXz`6<4X$8`xBo;o%;FH*^Q3vvdN#H5P)eoTN zL91Wwi<v(~I;>ut`cDw?#^>Kj{{K9PiiLp+>Qb zINJ{Os-S5_e^#=xI&Ym1v3QBRo+uh&AmC;(>6v&nGX65jcSLS2R>v^;O$QijFh9>6 z&Ny&|wFR*8+cTtw;_l-$Zj!Ej$*dvNE3%omr)Kqt!!?Ei9VPO!&7}WdVoJ9%^O@*_}Z^KBoEW$XnQ*E z&#RTO`cR}k;%qkQ?iZ>Wu4Y17@=o7ZG~AE}xn z>51`(R}CykXLICmlUqIMND;|`$;kM^h|AiI(5>268dI`^6&Sw9X7_XtiE^2HN!ARL zZF8?x&gWy~8v56%=Co{IYaY|52zV@q6>j{NpJbg9-l&lD1wO)`I9l~ zq@kV^PsH@e zhue%0$)op=vSbv`>cz5MiAKloPAXPbiAqQ**GG-Ri!kMKgaW@+;-Ty6%NMI-Ay0!3 zM4E%br!?cUoU}+0o*q!FNYY@7j8iPi#K6VWvIFG{;xF`sH9o$?cW9cvILI!=D&Mvx zwAmrRU*K14IV+d8SG&28(MF+adfeXTrcX;SfmM}lV|`I1e6rT=_2Plrf||+Z5B7d| z%s|{~o=%~e1z)1|gVp|kz;mnJ+^KJC3l`g>$&rXZeEu%T|M<62?v6fEc3&lv5}j<5 zK5~PxWJKVyuUk;pE*mFYesp2q<)~!_YT3Dh4IDMjV4p)RdB+eQxqlACPNfgE2BD%< z0)9JI?`pEbx++$P357RlZG1h&T&)BSAjO9hGHh-tWV$>{1%OK=6YjKY>KIalh!bA# zvBYHC+`zuXv%_0QtSO1bT4mGr{qr^2Erv@0VGf^jl8foybeJk?`fwqKowgChen4ee1$@ zoj0v84wGEHRLQ`Zs|F5ozpHoS{XRF5*P}xl9F!9IKi#joDU01t>buS_XlAja3 z#zd1Z#HY;nS+zJTsZMZe42^k2>xdOnyrfKNgeBIj!1QBq;z9IBzsTW}3#YOKFYr*! zAk(fo-5Fl%BT>SYP_BIA62<|TDvjfsyV2wIRORQwJyC)7gQXVs(E}|0F@prNm_yoa zkY{8roE3=``Rn@{NMo&F3EmY{fLJ@7^5V<#cn|eS*p1UsN*I{`dme&H;K+eAlCAG~ z)V)IgmnpaQ+O;8%N)_)7)um5w-I?UEpTxCoM!fh&boQoU414}qCd<0&dIm*Z&W_%| zqlK0UGV4g^(hTr->GJMCGxYwJ7mD1-zJm+2T21QM5)5kB3_WQ=o ze_Sh-NYBqCCB6kzaWJU89%W}>b^58O%Io%q^$9!XVu-f*4Asf9}KJDm? zy{s@Vnlv_)pJ|jnXU%L%6><7z%dV2mV^n9BZpH&e=OHE}$&gCPL3V)wi* = T & { + host: string; + port: number; +} & HttpsServerOptions; + /** * Create an HTTP-server configuration that works well in test environments. * - Ask the operating system to assign a free (ephemeral) port. * - Use IPv4 localhost `127.0.0.1` to avoid known IPv6 issues in Docker-based * environments like Travis-CI. + * - Provide default TLS key & cert when `protocol` is set to `https`. * * @param customConfig Additional configuration options to apply. */ export function givenHttpServerConfig( - customConfig?: T, -): T & {host: string; port: number} { + customConfig?: T & {protocol?: string}, +): ConfigRetval { const defaults = { host: '127.0.0.1', port: 0, + protocol: undefined, }; - const config = Object.assign({}, defaults, customConfig); + const config: ConfigRetval = Object.assign({}, defaults, customConfig); if (config.host === undefined) config.host = defaults.host; if (config.port === undefined) config.port = defaults.port; + if (customConfig && customConfig.protocol === 'https') { + setupTlsConfig(config); + } return config; } + +function setupTlsConfig(config: HttpsServerOptions) { + if ('key' in config && 'cert' in config) return; + if ('pfx' in config) return; + Object.assign(config, DUMMY_TLS_CONFIG); +} From 7c11d0e5e490bcb35aa7bbc4e8ba68613aca4b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 7 Mar 2019 10:11:00 +0100 Subject: [PATCH 2/3] refactor(rest): improve handling of RestServer config defaults Provide more type information to allow the compiler to understand when we are expecting configuration with optional properties and when consumers are relying on the optional config properties to have been already resolved with defaults. This change allows us to remove several usages of `!` operator. --- .../integration/rest.server.integration.ts | 17 +++ packages/rest/src/rest.server.ts | 119 +++++++++++------- 2 files changed, 91 insertions(+), 45 deletions(-) diff --git a/packages/rest/src/__tests__/integration/rest.server.integration.ts b/packages/rest/src/__tests__/integration/rest.server.integration.ts index 34d507cfdb74..c5ebdca63072 100644 --- a/packages/rest/src/__tests__/integration/rest.server.integration.ts +++ b/packages/rest/src/__tests__/integration/rest.server.integration.ts @@ -11,6 +11,7 @@ import { httpsGetAsync, itSkippedOnTravis, supertest, + createRestAppClient, } from '@loopback/testlab'; import * as fs from 'fs'; import {IncomingMessage, ServerResponse} from 'http'; @@ -29,6 +30,7 @@ import { RestComponent, RestServer, RestServerConfig, + RestApplication, } from '../..'; const readFileAsync = util.promisify(fs.readFile); @@ -548,6 +550,21 @@ paths: expect(response.get('Location')).match(expectedUrl); }); + it('handles requests with missing Host header', async () => { + const app = new RestApplication({ + rest: {port: 0, host: '127.0.0.1'}, + }); + await app.start(); + const port = await app.restServer.get(RestBindings.PORT); + + const response = await createRestAppClient(app) + .get('/explorer') + .set('host', ''); + await app.stop(); + const expectedUrl = new RegExp(`\\?url=http://127.0.0.1:${port}`); + expect(response.get('Location')).match(expectedUrl); + }); + it('exposes "GET /explorer" endpoint with apiExplorer.url', async () => { const server = await givenAServer({ rest: { diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index caf81e9ef44c..f7277fbec278 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -135,7 +135,7 @@ export class RestServer extends Context implements Server, HttpServerLike { return this._requestHandler; } - public readonly config: RestServerConfig; + public readonly config: RestServerResolvedConfig; private _basePath: string; protected _httpHandler: HttpHandler; @@ -172,27 +172,9 @@ export class RestServer extends Context implements Server, HttpServerLike { ) { super(app); - // Can't check falsiness, 0 is a valid port. - if (config.port == null) { - config.port = 3000; - } - if (config.host == null) { - // Set it to '' so that the http server will listen on all interfaces - config.host = undefined; - } - - config.openApiSpec = config.openApiSpec || {}; - config.openApiSpec.endpointMapping = - config.openApiSpec.endpointMapping || OPENAPI_SPEC_MAPPING; - - config.apiExplorer = normalizeApiExplorerConfig(config.apiExplorer); - if (config.openApiSpec.disabled) { - // Disable apiExplorer if the OpenAPI spec endpoint is disabled - config.apiExplorer.disabled = true; - } + this.config = resolveRestServerConfig(config); - this.config = config; - this.bind(RestBindings.PORT).to(config.port); + this.bind(RestBindings.PORT).to(this.config.port); this.bind(RestBindings.HOST).to(config.host); this.bind(RestBindings.PROTOCOL).to(config.protocol || 'http'); this.bind(RestBindings.HTTPS_OPTIONS).to(config as ServerOptions); @@ -225,15 +207,7 @@ export class RestServer extends Context implements Server, HttpServerLike { // Allow CORS support for all endpoints so that users // can test with online SwaggerUI instance - const corsOptions = this.config.cors || { - origin: '*', - methods: 'GET,HEAD,PUT,PATCH,POST,DELETE', - preflightContinue: false, - optionsSuccessStatus: 204, - maxAge: 86400, - credentials: true, - }; - this._expressApp.use(cors(corsOptions)); + this._expressApp.use(cors(this.config.cors)); // Set up endpoints for OpenAPI spec/ui this._setupOpenApiSpecEndpoints(); @@ -255,7 +229,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * Apply express settings. */ protected _applyExpressSettings() { - const settings = this.config.expressSettings || {}; + const settings = this.config.expressSettings; for (const key in settings) { this._expressApp.set(key, settings[key]); } @@ -269,7 +243,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * to redirect to externally hosted API explorer */ protected _setupOpenApiSpecEndpoints() { - if (this.config.openApiSpec!.disabled) return; + if (this.config.openApiSpec.disabled) return; // NOTE(bajtos) Regular routes are handled through Sequence. // IMO, this built-in endpoint should not run through a Sequence, // because it's not part of the application API itself. @@ -277,7 +251,7 @@ export class RestServer extends Context implements Server, HttpServerLike { // this endpoint to trigger a log entry. If the server implements // content-negotiation to support XML clients, I don't want the OpenAPI // spec to be converted into an XML response. - const mapping = this.config.openApiSpec!.endpointMapping!; + const mapping = this.config.openApiSpec.endpointMapping!; // Serving OpenAPI spec for (const p in mapping) { this._expressApp.get(p, (req, res) => @@ -402,7 +376,7 @@ export class RestServer extends Context implements Server, HttpServerLike { ) { specForm = specForm || {version: '3.0.0', format: 'json'}; let specObj = this.getApiSpec(); - if (this.config.openApiSpec!.setServersFromRequest) { + if (this.config.openApiSpec.setServersFromRequest) { specObj = Object.assign({}, specObj); specObj.servers = [{url: this._getUrlForClient(request)}]; } @@ -473,9 +447,10 @@ export class RestServer extends Context implements Server, HttpServerLike { port = forwardedPort || port; if (!host) { - // No host detected from http headers. Use the configured values - host = this.config.host!; - port = this.config.port == null ? '' : this.config.port.toString(); + // No host detected from http headers + // Use the configured values or the local network address + host = this.config.host || request.socket.localAddress; + port = (this.config.port || request.socket.localPort).toString(); } // clear default ports @@ -507,7 +482,7 @@ export class RestServer extends Context implements Server, HttpServerLike { response: Response, next: express.NextFunction, ) { - const config = this.config.apiExplorer!; + const config = this.config.apiExplorer; if (config.disabled) { debug('Redirect to swagger-ui was disabled by configuration.'); @@ -987,21 +962,25 @@ export interface ApiExplorerOptions { } /** - * Options for RestServer configuration + * RestServer options */ -export interface RestServerOptions { +export type RestServerOptions = Partial; + +export interface RestServerResolvedOptions { + port: number; + /** * Base path for API/static routes */ basePath?: string; - cors?: cors.CorsOptions; - openApiSpec?: OpenApiSpecOptions; - apiExplorer?: ApiExplorerOptions; + cors: cors.CorsOptions; + openApiSpec: OpenApiSpecOptions; + apiExplorer: ApiExplorerOptions; requestBodyParser?: RequestBodyParserOptions; sequence?: Constructor; // tslint:disable-next-line:no-any - expressSettings?: {[name: string]: any}; - router?: RestRouterOptions; + expressSettings: {[name: string]: any}; + router: RestRouterOptions; } /** @@ -1012,6 +991,56 @@ export interface RestServerOptions { */ export type RestServerConfig = RestServerOptions & HttpServerOptions; +type RestServerResolvedConfig = RestServerResolvedOptions & HttpServerOptions; + +const DEFAULT_CONFIG: RestServerResolvedConfig = { + port: 3000, + openApiSpec: {}, + apiExplorer: {}, + cors: { + origin: '*', + methods: 'GET,HEAD,PUT,PATCH,POST,DELETE', + preflightContinue: false, + optionsSuccessStatus: 204, + maxAge: 86400, + credentials: true, + }, + expressSettings: {}, + router: {}, +}; + +function resolveRestServerConfig( + config: RestServerConfig, +): RestServerResolvedConfig { + const result: RestServerResolvedConfig = Object.assign( + {}, + DEFAULT_CONFIG, + config, + ); + + // Can't check falsiness, 0 is a valid port. + if (result.port == null) { + result.port = 3000; + } + + if (result.host == null) { + // Set it to '' so that the http server will listen on all interfaces + result.host = undefined; + } + + if (!result.openApiSpec.endpointMapping) + result.openApiSpec.endpointMapping = OPENAPI_SPEC_MAPPING; + + result.apiExplorer = normalizeApiExplorerConfig(config.apiExplorer); + + if (result.openApiSpec.disabled) { + // Disable apiExplorer if the OpenAPI spec endpoint is disabled + result.apiExplorer.disabled = true; + } + + return result; +} + function normalizeApiExplorerConfig( input: ApiExplorerOptions | undefined, ): ApiExplorerOptions { From 0bb9b5aae8c667c80bbd14ab35ef980942cbca83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 7 Mar 2019 10:43:59 +0100 Subject: [PATCH 3/3] feat(rest): add `requestedBaseUrl` API to RequestContext Move the code calculating the requested protocol, base path and base URL from RestServer private methods into RequestContext's public API. --- packages/rest/src/__tests__/helpers.ts | 17 +++ .../integration/http-handler.integration.ts | 4 +- .../request-context.integration.ts | 130 ++++++++++++++++++ .../src/__tests__/unit/rest.component.unit.ts | 5 +- packages/rest/src/http-handler.ts | 7 +- packages/rest/src/request-context.ts | 82 +++++++++++ packages/rest/src/rest.server.ts | 102 +++----------- 7 files changed, 260 insertions(+), 87 deletions(-) create mode 100644 packages/rest/src/__tests__/integration/request-context.integration.ts diff --git a/packages/rest/src/__tests__/helpers.ts b/packages/rest/src/__tests__/helpers.ts index d17ca9dff3cc..fef411f78ce4 100644 --- a/packages/rest/src/__tests__/helpers.ts +++ b/packages/rest/src/__tests__/helpers.ts @@ -10,6 +10,7 @@ import { } from '@loopback/openapi-v3-types'; import {IncomingMessage} from 'http'; import {LogError} from '..'; +import {RestServerConfig, RestServerResolvedConfig} from '../rest.server'; export function createUnexpectedHttpErrorLogger( expectedStatusCode: number = 0, @@ -51,3 +52,19 @@ export function aBodySpec( }); return spec as RequestBodyObject; } + +export function aRestServerConfig( + customConfig?: RestServerConfig, +): RestServerResolvedConfig { + return Object.assign( + { + port: 3000, + openApiSpec: {disabled: true}, + apiExplorer: {disabled: true}, + cors: {}, + expressSettings: {}, + router: {}, + }, + customConfig, + ); +} diff --git a/packages/rest/src/__tests__/integration/http-handler.integration.ts b/packages/rest/src/__tests__/integration/http-handler.integration.ts index 71576bf2af05..319034807f12 100644 --- a/packages/rest/src/__tests__/integration/http-handler.integration.ts +++ b/packages/rest/src/__tests__/integration/http-handler.integration.ts @@ -30,7 +30,7 @@ import { UrlEncodedBodyParser, writeResultToResponse, } from '../..'; -import {createUnexpectedHttpErrorLogger} from '../helpers'; +import {createUnexpectedHttpErrorLogger, aRestServerConfig} from '../helpers'; const SequenceActions = RestBindings.SequenceActions; @@ -652,7 +652,7 @@ describe('HttpHandler', () => { .bind(RestBindings.REQUEST_BODY_PARSER) .toClass(RequestBodyParser); - handler = new HttpHandler(rootContext); + handler = new HttpHandler(rootContext, aRestServerConfig()); rootContext.bind(RestBindings.HANDLER).to(handler); } diff --git a/packages/rest/src/__tests__/integration/request-context.integration.ts b/packages/rest/src/__tests__/integration/request-context.integration.ts new file mode 100644 index 000000000000..96777c0977b7 --- /dev/null +++ b/packages/rest/src/__tests__/integration/request-context.integration.ts @@ -0,0 +1,130 @@ +// Copyright IBM Corp. 2018. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {ApplicationConfig} from '@loopback/core'; +import { + Client, + createRestAppClient, + expect, + givenHttpServerConfig, + supertest, + httpsGetAsync, +} from '@loopback/testlab'; +import * as express from 'express'; +import {RequestContext} from '../../request-context'; +import {RestApplication} from '../../rest.application'; +import {RestServerConfig} from '../../rest.server'; +import {DefaultSequence} from '../../sequence'; + +let app: RestApplication; +let client: Client; +let observedCtx: RequestContext; + +describe('RequestContext', () => { + beforeEach(setup); + afterEach(teardown); + + describe('requestedProtocol', () => { + it('defaults to "http"', async () => { + await givenRunningAppWithClient(); + await client.get('/products').expect(200); + expect(observedCtx.requestedProtocol).to.equal('http'); + }); + + it('honors "x-forwarded-proto" header', async () => { + await givenRunningAppWithClient(); + await client + .get('/products') + .set('x-forwarded-proto', 'https') + .expect(200); + expect(observedCtx.requestedProtocol).to.equal('https'); + }); + + it('honors protocol provided by Express request', async () => { + await givenRunningAppWithClient({protocol: 'https'}); + expect(app.restServer.url).to.startWith('https:'); + // supertest@3 fails with Error: self signed certificate + // FIXME(bajtos) rework this code once we upgrade to supertest@4 + // await client.get('/products').trustLocalhost().expect(200); + await httpsGetAsync(app.restServer.url + '/products'); + expect(observedCtx.requestedProtocol).to.equal('https'); + }); + }); + + describe('basePath', () => { + it('defaults to an empty string', async () => { + await givenRunningAppWithClient(); + await client.get('/products').expect(200); + expect(observedCtx.basePath).to.equal(''); + }); + + it('honors baseUrl when mounted on a sub-path', async () => { + const lbApp = new RestApplication(); + lbApp.handler(contextObservingHandler); + + const expressApp = express(); + expressApp.use('/api', lbApp.requestHandler); + + await supertest(expressApp) + .get('/api/products') + .expect(200); + + expect(observedCtx.basePath).to.equal('/api'); + }); + }); + + describe('requestedBaseUrl', () => { + it('defaults to data from the HTTP connection', async () => { + await givenRunningAppWithClient({ + host: undefined, + port: 0, + }); + const serverUrl = app.restServer.url; + + await client.get('/products').expect(200); + + expect(observedCtx.requestedBaseUrl).to.equal(serverUrl); + }); + + it('honors "x-forwarded-*" headers', async () => { + await givenRunningAppWithClient(); + await client + .get('/products') + .set('x-forwarded-proto', 'https') + .set('x-forwarded-host', 'example.com') + .set('x-forwarded-port', '8080') + .expect(200); + expect(observedCtx.requestedBaseUrl).to.equal('https://example.com:8080'); + }); + }); +}); + +function setup() { + (app as unknown) = undefined; + (client as unknown) = undefined; + (observedCtx as unknown) = undefined; +} + +async function teardown() { + if (app) await app.stop(); +} + +async function givenRunningAppWithClient(restOptions?: RestServerConfig) { + const options: ApplicationConfig = { + rest: givenHttpServerConfig(restOptions), + }; + app = new RestApplication(options); + app.handler(contextObservingHandler); + await app.start(); + client = createRestAppClient(app); +} + +function contextObservingHandler( + ctx: RequestContext, + _sequence: DefaultSequence, +) { + observedCtx = ctx; + ctx.response.end('ok'); +} diff --git a/packages/rest/src/__tests__/unit/rest.component.unit.ts b/packages/rest/src/__tests__/unit/rest.component.unit.ts index 8220b6a83c85..5c3df5728a8a 100644 --- a/packages/rest/src/__tests__/unit/rest.component.unit.ts +++ b/packages/rest/src/__tests__/unit/rest.component.unit.ts @@ -20,6 +20,7 @@ import { RestComponentConfig, RestServer, } from '../..'; +import {aRestServerConfig} from '../helpers'; const SequenceActions = RestBindings.SequenceActions; describe('RestComponent', () => { @@ -34,7 +35,9 @@ describe('RestComponent', () => { // Stub constructor requirements for some providers. app.bind(RestBindings.Http.CONTEXT).to(new Context()); - app.bind(RestBindings.HANDLER).to(new HttpHandler(app)); + app + .bind(RestBindings.HANDLER) + .to(new HttpHandler(app, aRestServerConfig())); comp = await app.get('components.RestComponent'); }); diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index bf00f017da67..fe532bd5eaaa 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -19,6 +19,7 @@ import {Request, Response} from './types'; import {RestBindings} from './keys'; import {RequestContext} from './request-context'; +import {RestServerResolvedConfig} from './rest.server'; export class HttpHandler { protected _apiDefinitions: SchemasObject; @@ -26,8 +27,9 @@ export class HttpHandler { public handleRequest: (request: Request, response: Response) => Promise; constructor( - protected _rootContext: Context, - protected _routes = new RoutingTable(), + protected readonly _rootContext: Context, + protected readonly _serverConfig: RestServerResolvedConfig, + protected readonly _routes = new RoutingTable(), ) { this.handleRequest = (req, res) => this._handleRequest(req, res); } @@ -70,6 +72,7 @@ export class HttpHandler { request, response, this._rootContext, + this._serverConfig, ); const sequence = await requestContext.get( diff --git a/packages/rest/src/request-context.ts b/packages/rest/src/request-context.ts index 340eadab55a0..1fb4a4c545e2 100644 --- a/packages/rest/src/request-context.ts +++ b/packages/rest/src/request-context.ts @@ -6,6 +6,7 @@ import {Context} from '@loopback/context'; import * as onFinished from 'on-finished'; import {RestBindings} from './keys'; +import {RestServerResolvedConfig} from './rest.server'; import {HandlerContext, Request, Response} from './types'; /** @@ -13,10 +14,82 @@ import {HandlerContext, Request, Response} from './types'; * (request, response, etc.). */ export class RequestContext extends Context implements HandlerContext { + /** + * Get the protocol used by the client to make the request. + * Please note this protocol may be different from what we are observing + * at HTTP/TCP level, because reverse proxies like nginx or sidecars like + * Envoy are switching between protocols. + */ + get requestedProtocol(): string { + return ( + (this.request.get('x-forwarded-proto') || '').split(',')[0] || + this.request.protocol || + this.serverConfig.protocol || + 'http' + ); + } + + /** + * Get the effective base path of the incoming request. This base path + * combines `baseUrl` provided by Express when LB4 handler is mounted on + * a non-root path, with the `basePath` value configured at LB4 side. + */ + get basePath(): string { + const request = this.request; + let basePath = this.serverConfig.basePath || ''; + if (request.baseUrl && request.baseUrl !== '/') { + basePath = request.baseUrl + basePath; + } + return basePath; + } + + /** + * Get the base URL used by the client to make the request. + * This URL contains the protocol, hostname, port and base path. + * The path of the invoked route and query string is not included. + * + * Please note these values may be different from what we are observing + * at HTTP/TCP level, because reverse proxies like nginx are rewriting them. + */ + get requestedBaseUrl(): string { + const request = this.request; + const config = this.serverConfig; + + const protocol = this.requestedProtocol; + // The host can be in one of the forms + // [::1]:3000 + // [::1] + // 127.0.0.1:3000 + // 127.0.0.1 + let {host, port} = parseHostAndPort( + request.get('x-forwarded-host') || request.headers.host, + ); + + const forwardedPort = (request.get('x-forwarded-port') || '').split(',')[0]; + port = forwardedPort || port; + + if (!host) { + // No host detected from http headers + // Use the configured values or the local network address + host = config.host || request.socket.localAddress; + port = (config.port || request.socket.localPort).toString(); + } + + // clear default ports + port = protocol === 'https' && port === '443' ? '' : port; + port = protocol === 'http' && port === '80' ? '' : port; + + // add port number of present + host += port !== '' ? ':' + port : ''; + + return protocol + '://' + host + this.basePath; + } + constructor( public readonly request: Request, public readonly response: Response, parent: Context, + public readonly serverConfig: RestServerResolvedConfig, name?: string, ) { super(parent, name); @@ -40,3 +113,12 @@ export class RequestContext extends Context implements HandlerContext { .lock(); } } + +function parseHostAndPort(host: string | undefined) { + host = host || ''; + host = host.split(',')[0]; + const portPattern = /:([0-9]+)$/; + const port = (host.match(portPattern) || [])[1] || ''; + host = host.replace(portPattern, ''); + return {host, port}; +} diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index f7277fbec278..26660f6c36cd 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -282,7 +282,7 @@ export class RestServer extends Context implements Server, HttpServerLike { const router = this.getSync(RestBindings.ROUTER, {optional: true}); const routingTable = new RoutingTable(router, this._externalRoutes); - this._httpHandler = new HttpHandler(this, routingTable); + this._httpHandler = new HttpHandler(this, this.config, routingTable); for (const b of this.find('controllers.*')) { const controllerName = b.key.replace(/^controllers\./, ''); const ctor = b.valueConstructor; @@ -374,14 +374,21 @@ export class RestServer extends Context implements Server, HttpServerLike { response: Response, specForm?: OpenApiSpecForm, ) { + const requestContext = new RequestContext( + request, + response, + this, + this.config, + ); + specForm = specForm || {version: '3.0.0', format: 'json'}; let specObj = this.getApiSpec(); if (this.config.openApiSpec.setServersFromRequest) { specObj = Object.assign({}, specObj); - specObj.servers = [{url: this._getUrlForClient(request)}]; + specObj.servers = [{url: requestContext.requestedBaseUrl}]; } - const basePath = this.getBasePathFor(request); + const basePath = requestContext.basePath; if (specObj.servers && basePath) { for (const s of specObj.servers) { // Update the default server url to honor `basePath` @@ -401,82 +408,6 @@ export class RestServer extends Context implements Server, HttpServerLike { response.end(yaml, 'utf-8'); } } - - /** - * Get the protocol for a request - * @param request Http request - */ - private _getProtocolForRequest(request: Request) { - return ( - (request.get('x-forwarded-proto') || '').split(',')[0] || - request.protocol || - this.config.protocol || - 'http' - ); - } - - /** - * Parse the host:port string into an object for host and port - * @param host The host string - */ - private _parseHostAndPort(host: string | undefined) { - host = host || ''; - host = host.split(',')[0]; - const portPattern = /:([0-9]+)$/; - const port = (host.match(portPattern) || [])[1] || ''; - host = host.replace(portPattern, ''); - return {host, port}; - } - - /** - * Get the URL of the request sent by the client - * @param request Http request - */ - private _getUrlForClient(request: Request) { - const protocol = this._getProtocolForRequest(request); - // The host can be in one of the forms - // [::1]:3000 - // [::1] - // 127.0.0.1:3000 - // 127.0.0.1 - let {host, port} = this._parseHostAndPort( - request.get('x-forwarded-host') || request.headers.host, - ); - - const forwardedPort = (request.get('x-forwarded-port') || '').split(',')[0]; - port = forwardedPort || port; - - if (!host) { - // No host detected from http headers - // Use the configured values or the local network address - host = this.config.host || request.socket.localAddress; - port = (this.config.port || request.socket.localPort).toString(); - } - - // clear default ports - port = protocol === 'https' && port === '443' ? '' : port; - port = protocol === 'http' && port === '80' ? '' : port; - - // add port number of present - host += port !== '' ? ':' + port : ''; - - return protocol + '://' + host + this.getBasePathFor(request); - } - - /** - * Get the base for the request. It honors `baseUrl` sets by express if the - * application is mounted to an express app, such as: - * expressApp.use('/api', app.requestHandler); - * @param request Http request - */ - private getBasePathFor(request: Request) { - let basePath = this._basePath; - if (request.baseUrl && request.baseUrl !== '/') { - basePath = request.baseUrl + basePath; - } - return basePath; - } - private async _redirectToSwaggerUI( request: Request, response: Response, @@ -491,9 +422,15 @@ export class RestServer extends Context implements Server, HttpServerLike { } debug('Redirecting to swagger-ui from %j.', request.originalUrl); - const protocol = this._getProtocolForRequest(request); + const requestContext = new RequestContext( + request, + response, + this, + this.config, + ); + const protocol = requestContext.requestedProtocol; const baseUrl = protocol === 'http' ? config.httpUrl : config.url; - const openApiUrl = `${this._getUrlForClient(request)}/openapi.json`; + const openApiUrl = `${requestContext.requestedBaseUrl}/openapi.json`; const fullUrl = `${baseUrl}?url=${openApiUrl}`; response.redirect(308, fullUrl); } @@ -991,7 +928,8 @@ export interface RestServerResolvedOptions { */ export type RestServerConfig = RestServerOptions & HttpServerOptions; -type RestServerResolvedConfig = RestServerResolvedOptions & HttpServerOptions; +export type RestServerResolvedConfig = RestServerResolvedOptions & + HttpServerOptions; const DEFAULT_CONFIG: RestServerResolvedConfig = { port: 3000,