Merge pull request #16265 from dhoizner/revive-proxy-req-bodies

[Proxy-Backend] Feature: Revive Request Bodies

.changeset/ten-tigers-marry.md

@@ -0,0 +1,16 @@
+---
+'@backstage/plugin-proxy-backend': patch
+---
+
+The proxy-backend plugin now supports reviving request bodies that have previously been consumed by an express middleware (e.g. `express.json()`). This is done by setting `reviveConsumedRequestBodies: true` on the proxy `RouterOptions`. In order to preserve the current behavior, the proxy will **not** revive request bodies by default.
+
+The following is an example of a proxy `createRouter` invocation that revives request bodies:
+
+```diff
+const router = await createRouter({
+  config,
+  logger,
+  discovery,
++ reviveConsumedRequestBodies: true,
+});
+```

plugins/proxy-backend/api-report.md

@@ -17,6 +17,7 @@ export const proxyPlugin: (
   options?:
     | {
         skipInvalidProxies?: boolean | undefined;
+        reviveConsumedRequestBodies?: boolean | undefined;
       }
     | undefined,
 ) => BackendFeature;
@@ -30,6 +31,8 @@ export interface RouterOptions {
   // (undocumented)
   logger: Logger;
   // (undocumented)
+  reviveConsumedRequestBodies?: boolean;
+  // (undocumented)
   skipInvalidProxies?: boolean;
 }
 ```

plugins/proxy-backend/src/plugin.ts

@@ -27,7 +27,10 @@ import { createRouter } from './service/router';
  * @alpha
  */
 export const proxyPlugin = createBackendPlugin(
-  (options?: { skipInvalidProxies?: boolean }) => ({
+  (options?: {
+    skipInvalidProxies?: boolean;
+    reviveConsumedRequestBodies?: boolean;
+  }) => ({
     pluginId: 'proxy',
     register(env) {
       env.registerInit({
@@ -44,6 +47,7 @@ export const proxyPlugin = createBackendPlugin(
               discovery,
               logger: loggerToWinstonLogger(logger),
               skipInvalidProxies: options?.skipInvalidProxies,
+              reviveConsumedRequestBodies: options?.reviveConsumedRequestBodies,
             }),
           );
         },

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

@@ -18,11 +18,16 @@ import { getVoidLogger, SingleHostDiscovery } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
 import { Request, Response } from 'express';
 import * as http from 'http';
-import { createProxyMiddleware, Options } from 'http-proxy-middleware';
+import {
+  createProxyMiddleware,
+  fixRequestBody,
+  Options,
+} from 'http-proxy-middleware';
 import { buildMiddleware, createRouter } from './router';
 
 jest.mock('http-proxy-middleware', () => ({
   createProxyMiddleware: jest.fn(() => () => undefined),
+  fixRequestBody: jest.fn(),
 }));
 
 const mockCreateProxyMiddleware = createProxyMiddleware as jest.MockedFunction<
@@ -31,25 +36,30 @@ const mockCreateProxyMiddleware = createProxyMiddleware as jest.MockedFunction<
 
 describe('createRouter', () => {
   describe('where all proxy config are valid', () => {
+    const logger = getVoidLogger();
+    const config = new ConfigReader({
+      backend: {
+        baseUrl: 'https://example.com:7007',
+        listen: {
+          port: 7007,
+        },
+      },
+      proxy: {
+        '/test': {
+          target: 'https://example.com',
+          headers: {
+            Authorization: 'Bearer supersecret',
+          },
+        },
+      },
+    });
+    const discovery = SingleHostDiscovery.fromConfig(config);
+
+    beforeEach(() => {
+      mockCreateProxyMiddleware.mockClear();
+    });
+
     it('works', async () => {
-      const logger = getVoidLogger();
-      const config = new ConfigReader({
-        backend: {
-          baseUrl: 'https://example.com:7007',
-          listen: {
-            port: 7007,
-          },
-        },
-        proxy: {
-          '/test': {
-            target: 'https://example.com',
-            headers: {
-              Authorization: 'Bearer supersecret',
-            },
-          },
-        },
-      });
-      const discovery = SingleHostDiscovery.fromConfig(config);
       const router = await createRouter({
         config,
         logger,
@@ -57,6 +67,36 @@ describe('createRouter', () => {
       });
       expect(router).toBeDefined();
     });
+
+    it('revives request bodies when set', async () => {
+      const router = await createRouter({
+        config,
+        logger,
+        discovery,
+        reviveConsumedRequestBodies: true,
+      });
+      expect(router).toBeDefined();
+
+      expect(
+        mockCreateProxyMiddleware.mock.calls[0][1]?.onProxyReq,
+      ).toBeDefined();
+      expect(mockCreateProxyMiddleware.mock.calls[0][1]?.onProxyReq).toEqual(
+        fixRequestBody,
+      );
+    });
+
+    it('does not revive request bodies when not set', async () => {
+      const router = await createRouter({
+        config,
+        logger,
+        discovery,
+      });
+      expect(router).toBeDefined();
+
+      expect(
+        mockCreateProxyMiddleware.mock.calls[0][1]?.onProxyReq,
+      ).not.toBeDefined();
+    });
   });
 
   describe('where buildMiddleware would fail', () => {
@@ -389,6 +429,46 @@ describe('buildMiddleware', () => {
     expect(Object.keys(testClientResponse.headers!)).toEqual(['set-cookie']);
   });
 
+  it('revives request body when configured', async () => {
+    buildMiddleware(
+      '/proxy',
+      logger,
+      '/test',
+      {
+        target: 'http://mocked',
+      },
+      true,
+    );
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const config = mockCreateProxyMiddleware.mock.calls[0][1] as Options;
+
+    expect(config).toBeDefined();
+    expect(config.onProxyReq).toBeDefined();
+
+    config.onProxyReq!(
+      {} as http.ClientRequest,
+      {} as Request,
+      {} as Response,
+      {},
+    );
+    expect(fixRequestBody).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not revive request body when not configured', async () => {
+    buildMiddleware('/proxy', logger, '/test', {
+      target: 'http://mocked',
+    });
+
+    expect(createProxyMiddleware).toHaveBeenCalledTimes(1);
+
+    const config = mockCreateProxyMiddleware.mock.calls[0][1] as Options;
+
+    expect(config).toBeDefined();
+    expect(config.onProxyReq).not.toBeDefined();
+  });
+
   it('rejects malformed target URLs', async () => {
     expect(() =>
       buildMiddleware('/proxy', logger, '/test', 'backstage.io'),

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

@@ -19,6 +19,7 @@ import express from 'express';
 import Router from 'express-promise-router';
 import {
   createProxyMiddleware,
+  fixRequestBody,
   Options,
   RequestHandler,
 } from 'http-proxy-middleware';
@@ -53,11 +54,13 @@ export interface RouterOptions {
   config: Config;
   discovery: PluginEndpointDiscovery;
   skipInvalidProxies?: boolean;
+  reviveConsumedRequestBodies?: boolean;
 }
 
 export interface ProxyConfig extends Options {
   allowedMethods?: string[];
   allowedHeaders?: string[];
+  reviveRequestBody?: boolean;
 }
 
 // Creates a proxy middleware, possibly with defaults added on top of the
@@ -67,6 +70,7 @@ export function buildMiddleware(
   logger: Logger,
   route: string,
   config: string | ProxyConfig,
+  reviveConsumedRequestBodies?: boolean,
 ): RequestHandler {
   const fullConfig =
     typeof config === 'string' ? { target: config } : { ...config };
@@ -175,6 +179,10 @@ export function buildMiddleware(
     });
   };
 
+  if (reviveConsumedRequestBodies) {
+    fullConfig.onProxyReq = fixRequestBody;
+  }
+
   return createProxyMiddleware(filter, fullConfig);
 }
 
@@ -242,7 +250,13 @@ function configureMiddlewares(
     try {
       router.use(
         route,
-        buildMiddleware(pathPrefix, options.logger, route, proxyRouteConfig),
+        buildMiddleware(
+          pathPrefix,
+          options.logger,
+          route,
+          proxyRouteConfig,
+          options.reviveConsumedRequestBodies,
+        ),
       );
     } catch (e) {
       if (options.skipInvalidProxies) {