Merge branch 'master' of github.com:spotify/backstage into migrate-to-msw

* 'master' of github.com:spotify/backstage: (110 commits)
  chore(catalog-backend): removing redudant classes and some functions
  chore(deps-dev): bump @types/webpack from 4.41.21 to 4.41.22 (#2765)
  move codecov.yml to .github
  feat(catalog-backend): add batch concurrency
  create-app: remove build step
  cli: simplify jest transform ignore regex
  feat(catalog-backend): introduce batching, speed up reading and writing of large datasets
  Techdocs: add Azure DevOps prepare support (#2748)
  feat(techdocs-header): Show breadcrumbs on docs page (#2786)
  changesets: add entry for create-app template location fix
  create-app: revert to github location type for example templates
  fix: make catalog filter work again
  Use new url scheme for techdocs
  feat: remove LocationProcessor.processEntity
  Add Dockerfile for helm chart
  feat: use the new UrlReader in the CodeOwnersProcessor
  feat: use new UrlReader in PlaceholderProcessor
  feat: remove the backstage.io/definition-at-location annotation
  Update loud-lamps-visit.md
  feat(proxy-backend): limit the forwarded http headers to a safe set
  ...

plugins/proxy-backend/package.json

@@ -22,7 +22,6 @@
     "@backstage/backend-common": "^0.1.1-alpha.24",
     "@backstage/config": "^0.1.1-alpha.24",
     "@types/express": "^4.17.6",
-    "@types/http-proxy-middleware": "^0.19.3",
     "express": "^4.17.1",
     "express-promise-router": "^3.0.3",
     "http-proxy-middleware": "^0.19.1",
@@ -35,6 +34,8 @@
   },
   "devDependencies": {
     "@backstage/cli": "^0.1.1-alpha.24",
+    "@types/http-proxy-middleware": "^0.19.3",
+    "@types/supertest": "^2.0.8",
     "@types/uuid": "^8.0.0",
     "@types/yup": "^0.28.2",
     "supertest": "^4.0.2"

plugins/proxy-backend/src/index.ts

@@ -14,4 +14,4 @@
  * limitations under the License.
  */
 
-export * from './service/router';
+export * from './service';

plugins/proxy-backend/src/service/index.ts

@@ -0,0 +1,17 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * 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 { createRouter } from './router';

plugins/proxy-backend/src/service/router.test.ts

@@ -14,13 +14,30 @@
  * limitations under the License.
  */
 
-import { createRouter } from './router';
+import { buildMiddleware, createRouter } from './router';
 import * as winston from 'winston';
 import { ConfigReader } from '@backstage/config';
 import {
   loadBackendConfig,
   SingleHostDiscovery,
 } from '@backstage/backend-common';
+import createProxyMiddleware, {
+  Config as ProxyMiddlewareConfig,
+  Proxy,
+} from 'http-proxy-middleware';
+import * as http from 'http';
+
+jest.mock('http-proxy-middleware', () => {
+  return jest.fn().mockImplementation(
+    (): Proxy => {
+      return () => undefined;
+    },
+  );
+});
+
+const mockCreateProxyMiddleware = createProxyMiddleware as jest.MockedFunction<
+  typeof createProxyMiddleware
+>;
 
 describe('createRouter', () => {
   it('works', async () => {
@@ -35,3 +52,151 @@ describe('createRouter', () => {
     expect(router).toBeDefined();
   });
 });
+
+describe('buildMiddleware', () => {
+  const logger = winston.createLogger();
+
+  beforeEach(() => {
+    mockCreateProxyMiddleware.mockClear();
+  });
+
+  it('accepts strings', async () => {
+    buildMiddleware('/api/', logger, 'test', 'http://mocked');
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const [filter, fullConfig] = mockCreateProxyMiddleware.mock.calls[0] as [
+      (pathname: string, req: Partial<http.IncomingMessage>) => boolean,
+      ProxyMiddlewareConfig,
+    ];
+    expect(filter('', { method: 'GET' })).toBe(true);
+    expect(filter('', { method: 'POST' })).toBe(true);
+    expect(filter('', { method: 'PUT' })).toBe(true);
+    expect(filter('', { method: 'PATCH' })).toBe(true);
+    expect(filter('', { method: 'DELETE' })).toBe(true);
+
+    expect(fullConfig.pathRewrite).toEqual({ '^/api/test/': '/' });
+    expect(fullConfig.changeOrigin).toBe(true);
+    expect(fullConfig.logProvider!(logger)).toBe(logger);
+  });
+
+  it('limits allowedMethods', async () => {
+    buildMiddleware('/api/', logger, 'test', {
+      target: 'http://mocked',
+      allowedMethods: ['GET', 'DELETE'],
+    });
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const [filter, fullConfig] = mockCreateProxyMiddleware.mock.calls[0] as [
+      (pathname: string, req: Partial<http.IncomingMessage>) => boolean,
+      ProxyMiddlewareConfig,
+    ];
+    expect(filter('', { method: 'GET' })).toBe(true);
+    expect(filter('', { method: 'POST' })).toBe(false);
+    expect(filter('', { method: 'PUT' })).toBe(false);
+    expect(filter('', { method: 'PATCH' })).toBe(false);
+    expect(filter('', { method: 'DELETE' })).toBe(true);
+
+    expect(fullConfig.pathRewrite).toEqual({ '^/api/test/': '/' });
+    expect(fullConfig.changeOrigin).toBe(true);
+    expect(fullConfig.logProvider!(logger)).toBe(logger);
+  });
+
+  it('permits default headers', async () => {
+    buildMiddleware('/api/', logger, 'test', {
+      target: 'http://mocked',
+    });
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const config = mockCreateProxyMiddleware.mock
+      .calls[0][1] as ProxyMiddlewareConfig;
+
+    const testClientRequest = {
+      getHeaderNames: () => [
+        'cache-control',
+        'content-language',
+        'content-length',
+        'content-type',
+        'expires',
+        'last-modified',
+        'pragma',
+        'host',
+        'accept',
+        'accept-language',
+        'user-agent',
+        'cookie',
+      ],
+      removeHeader: jest.fn(),
+    } as Partial<http.ClientRequest>;
+
+    expect(config).toBeDefined();
+    expect(config.onProxyReq).toBeDefined();
+
+    config.onProxyReq!(
+      testClientRequest as http.ClientRequest,
+      {} as http.IncomingMessage,
+      {} as http.ServerResponse,
+    );
+
+    expect(testClientRequest.removeHeader).toHaveBeenCalledTimes(1);
+    expect(testClientRequest.removeHeader).toHaveBeenCalledWith('cookie');
+  });
+
+  it('permits default and configured headers', async () => {
+    buildMiddleware('/api/', logger, 'test', {
+      target: 'http://mocked',
+      headers: {
+        Authorization: 'my-token',
+      },
+    });
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const config = mockCreateProxyMiddleware.mock
+      .calls[0][1] as ProxyMiddlewareConfig;
+
+    const testClientRequest = {
+      getHeaderNames: () => ['authorization', 'Cookie'],
+      removeHeader: jest.fn(),
+    } as Partial<http.ClientRequest>;
+
+    config.onProxyReq!(
+      testClientRequest as http.ClientRequest,
+      {} as http.IncomingMessage,
+      {} as http.ServerResponse,
+    );
+
+    expect(testClientRequest.removeHeader).toHaveBeenCalledTimes(1);
+    expect(testClientRequest.removeHeader).toHaveBeenCalledWith('Cookie');
+  });
+
+  it('permits configured headers', async () => {
+    buildMiddleware('/api/', logger, 'test', {
+      target: 'http://mocked',
+      allowedHeaders: ['authorization', 'cookie'],
+    });
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const config = mockCreateProxyMiddleware.mock
+      .calls[0][1] as ProxyMiddlewareConfig;
+
+    const testClientRequest = {
+      getHeaderNames: () => ['authorization', 'Cookie', 'X-Auth-Request-User'],
+      removeHeader: jest.fn(),
+    } as Partial<http.ClientRequest>;
+
+    config.onProxyReq!(
+      testClientRequest as http.ClientRequest,
+      {} as http.IncomingMessage,
+      {} as http.ServerResponse,
+    );
+
+    expect(testClientRequest.removeHeader).toHaveBeenCalledTimes(1);
+    expect(testClientRequest.removeHeader).toHaveBeenCalledWith(
+      'X-Auth-Request-User',
+    );
+  });
+});

plugins/proxy-backend/src/service/router.ts

@@ -25,6 +25,27 @@ import { Logger } from 'winston';
 import http from 'http';
 import { PluginEndpointDiscovery } from '@backstage/backend-common';
 
+// A list of headers that are always forwarded to the proxy targets.
+const safeForwardHeaders = [
+  // https://fetch.spec.whatwg.org/#cors-safelisted-request-header
+  'cache-control',
+  'content-language',
+  'content-length',
+  'content-type',
+  'expires',
+  'last-modified',
+  'pragma',
+
+  // host is overridden by default. if changeOrigin is configured to false,
+  // we assume this is a intentional and should also be forwarded.
+  'host',
+
+  // other headers that we assume to be ok
+  'accept',
+  'accept-language',
+  'user-agent',
+];
+
 export interface RouterOptions {
   logger: Logger;
   config: Config;
@@ -33,11 +54,12 @@ export interface RouterOptions {
 
 export interface ProxyConfig extends ProxyMiddlewareConfig {
   allowedMethods?: string[];
+  allowedHeaders?: string[];
 }
 
 // Creates a proxy middleware, possibly with defaults added on top of the
 // given config.
-function buildMiddleware(
+export function buildMiddleware(
   pathPrefix: string,
   logger: Logger,
   route: string,
@@ -68,6 +90,30 @@ function buildMiddleware(
     return fullConfig?.allowedMethods?.includes(req.method!) ?? true;
   };
 
+  // Only forward the allowed HTTP headers to not forward unwanted secret headers
+  const headerAllowList = new Set<string>(
+    [
+      // allow all safe headers
+      ...safeForwardHeaders,
+
+      // allow all headers that are set by the proxy
+      ...((fullConfig.headers && Object.keys(fullConfig.headers)) || []),
+
+      // allow all configured headers
+      ...(fullConfig.allowedHeaders || []),
+    ].map(h => h.toLocaleLowerCase()),
+  );
+
+  fullConfig.onProxyReq = (proxyReq: http.ClientRequest) => {
+    const headerNames = proxyReq.getHeaderNames();
+
+    headerNames.forEach(h => {
+      if (!headerAllowList.has(h.toLocaleLowerCase())) {
+        proxyReq.removeHeader(h);
+      }
+    });
+  };
+
   return createProxyMiddleware(filter, fullConfig);
 }