From 0dcdf5a0065bc51d6c8cb340130cd210656d79cb Mon Sep 17 00:00:00 2001
From: Juan Pablo Garcia Ripa <sarabadu@gmail.com>
Date: Wed, 6 Oct 2021 12:27:57 +0200
Subject: [PATCH] use minimatch pattern match to validate origin

Signed-off-by: Juan Pablo Garcia Ripa <sarabadu@gmail.com>
---
 .../src/service/lib/config.test.ts            | 40 ++++++++++++----
 .../backend-common/src/service/lib/config.ts  | 46 +++++++++++++------
 2 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/packages/backend-common/src/service/lib/config.test.ts b/packages/backend-common/src/service/lib/config.test.ts
index 10c2d705ca..59a11d3ab5 100644
--- a/packages/backend-common/src/service/lib/config.test.ts
+++ b/packages/backend-common/src/service/lib/config.test.ts
@@ -45,33 +45,53 @@ describe('config', () => {
 
   describe('readCorsOptions', () => {
     it('reads single string', () => {
+      const mockCallback = jest.fn();
       const config = new ConfigReader({ cors: { origin: 'https://*.value*' } });
       const cors = readCorsOptions(config);
       expect(cors).toEqual(
         expect.objectContaining({
-          origin: expect.any(RegExp),
+          origin: expect.any(Function),
         }),
       );
+      const origin = cors?.origin as Function;
+      origin('https://a.value', mockCallback); // valid origin
+      origin('http://a.value', mockCallback); // invalid origin
+      origin(undefined, mockCallback); // when not origin needs to reject the call
 
-      const origin = cors?.origin as RegExp;
-      expect(origin.test('https://a.value')).toBe(true);
-      expect(origin.test('http://a.value')).toBe(false);
+      expect(mockCallback.mock.calls[0][0]).toBe(null);
+      expect(mockCallback.mock.calls[1][0]).toBe(null);
+
+      expect(mockCallback.mock.calls[0][1]).toBe(true);
+      expect(mockCallback.mock.calls[1][1]).toBe(false);
+      expect(mockCallback.mock.calls[2][1]).toBe(false);
     });
 
     it('reads string array', () => {
+      const mockCallback = jest.fn();
       const config = new ConfigReader({
-        cors: { origin: ['https://*.value*', 'http(s|)://*.value*'] },
+        cors: { origin: ['https://*.value*', 'http://*.value'] },
       });
       const cors = readCorsOptions(config);
       expect(cors).toEqual(
         expect.objectContaining({
-          origin: expect.any(Array),
+          origin: expect.any(Function),
         }),
       );
-      const origin = cors?.origin as RegExp[];
-      expect(origin[0].test('https://a.value')).toBe(true);
-      expect(origin[1].test('https://a.value')).toBe(true);
-      expect(origin[1].test('http://a.value')).toBe(true);
+      const origin = cors?.origin as Function;
+      origin('https://a.value', mockCallback);
+      origin('https://a.valuex', mockCallback);
+      origin('http://a.value', mockCallback);
+      origin('http://a.valuex', mockCallback);
+
+      expect(mockCallback.mock.calls[0][0]).toBe(null);
+      expect(mockCallback.mock.calls[1][0]).toBe(null);
+      expect(mockCallback.mock.calls[2][0]).toBe(null);
+      expect(mockCallback.mock.calls[3][0]).toBe(null);
+
+      expect(mockCallback.mock.calls[0][1]).toBe(true);
+      expect(mockCallback.mock.calls[1][1]).toBe(true);
+      expect(mockCallback.mock.calls[2][1]).toBe(true);
+      expect(mockCallback.mock.calls[3][1]).toBe(false);
     });
   });
 });
diff --git a/packages/backend-common/src/service/lib/config.ts b/packages/backend-common/src/service/lib/config.ts
index 5d46e2fccf..ad20d99b0e 100644
--- a/packages/backend-common/src/service/lib/config.ts
+++ b/packages/backend-common/src/service/lib/config.ts
@@ -16,7 +16,7 @@
 
 import { Config } from '@backstage/config';
 import { CorsOptions } from 'cors';
-import { makeRe } from 'micromatch';
+import { Minimatch } from 'minimatch';
 
 export type BaseOptions = {
   listenPort?: string | number;
@@ -47,6 +47,13 @@ export type CertificateAttributes = {
  */
 export type CspOptions = Record<string, string[]>;
 
+type StaticOrigin = boolean | string | RegExp | (boolean | string | RegExp)[];
+
+type CustomOrigin = (
+  requestOrigin: string | undefined,
+  callback: (err: Error | null, origin?: StaticOrigin) => void,
+) => void;
+
 /**
  * Reads some base options out of a config object.
  *
@@ -208,11 +215,7 @@ function getOptionalStringOrStrings(
   key: string,
 ): string | string[] | undefined {
   const value = config.getOptional(key);
-  if (
-    value === undefined ||
-    typeof value === 'string' ||
-    isStringArray(value)
-  ) {
+  if (value === undefined || isStringOrStrings(value)) {
     return value;
   }
   throw new Error(`Expected string or array of strings, got ${typeof value}`);
@@ -221,18 +224,33 @@ function getOptionalStringOrStrings(
 function getOptionalGlobOrGlobs(
   config: Config,
   key: string,
-): RegExp | RegExp[] | undefined {
+): CustomOrigin | undefined {
   const value = config.getOptional(key);
+  if (!isStringOrStrings(value)) {
+    throw new Error(`Expected string or array of strings, got ${typeof value}`);
+  }
+
   if (value === undefined) {
     return value;
   }
-  if (typeof value === 'string') {
-    return makeRe(value, { debug: true });
-  }
-  if (isStringArray(value)) {
-    return value.map(val => makeRe(val));
-  }
-  throw new Error(`Expected string or array of strings, got ${typeof value}`);
+
+  const valueArr = typeof value === 'string' ? [value] : value;
+
+  const allowedOriginPatterns =
+    valueArr?.map(
+      pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }),
+    ) ?? [];
+
+  return (origin, callback) => {
+    return callback(
+      null,
+      allowedOriginPatterns.some(pattern => pattern.match(origin ?? '')),
+    );
+  };
+}
+
+function isStringOrStrings(value: any): value is string | string[] {
+  return typeof value === 'string' || isStringArray(value);
 }
 
 function isStringArray(value: any): value is string[] {