From 42f91cf01eb40777010d3226366c62db6b990847 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Tue, 5 Oct 2021 23:58:45 +0200 Subject: [PATCH 1/7] add glob option to cors origin config Signed-off-by: Juan Pablo Garcia Ripa --- packages/backend-common/package.json | 2 ++ .../src/service/lib/config.test.ts | 34 ++++++++++++++++++- .../backend-common/src/service/lib/config.ts | 20 ++++++++++- yarn.lock | 7 ++++ 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 29018ef8c8..3c426d780a 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -57,6 +57,7 @@ "knex": "^0.95.1", "lodash": "^4.17.21", "logform": "^2.1.1", + "micromatch": "^4.0.4", "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", @@ -84,6 +85,7 @@ "@types/concat-stream": "^1.6.0", "@types/fs-extra": "^9.0.3", "@types/http-errors": "^1.6.3", + "@types/micromatch": "^4.0.2", "@types/minimist": "^1.2.0", "@types/mock-fs": "^4.13.0", "@types/morgan": "^1.9.0", diff --git a/packages/backend-common/src/service/lib/config.test.ts b/packages/backend-common/src/service/lib/config.test.ts index f2f0c88cfb..10c2d705ca 100644 --- a/packages/backend-common/src/service/lib/config.test.ts +++ b/packages/backend-common/src/service/lib/config.test.ts @@ -15,7 +15,7 @@ */ import { ConfigReader } from '@backstage/config'; -import { readCspOptions } from './config'; +import { readCorsOptions, readCspOptions } from './config'; describe('config', () => { describe('readCspOptions', () => { @@ -42,4 +42,36 @@ describe('config', () => { expect(() => readCspOptions(config)).toThrow(/wanted string-array/); }); }); + + describe('readCorsOptions', () => { + it('reads single string', () => { + const config = new ConfigReader({ cors: { origin: 'https://*.value*' } }); + const cors = readCorsOptions(config); + expect(cors).toEqual( + expect.objectContaining({ + origin: expect.any(RegExp), + }), + ); + + const origin = cors?.origin as RegExp; + expect(origin.test('https://a.value')).toBe(true); + expect(origin.test('http://a.value')).toBe(false); + }); + + it('reads string array', () => { + const config = new ConfigReader({ + cors: { origin: ['https://*.value*', 'http(s|)://*.value*'] }, + }); + const cors = readCorsOptions(config); + expect(cors).toEqual( + expect.objectContaining({ + origin: expect.any(Array), + }), + ); + 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); + }); + }); }); diff --git a/packages/backend-common/src/service/lib/config.ts b/packages/backend-common/src/service/lib/config.ts index 77ef925403..5d46e2fccf 100644 --- a/packages/backend-common/src/service/lib/config.ts +++ b/packages/backend-common/src/service/lib/config.ts @@ -16,6 +16,7 @@ import { Config } from '@backstage/config'; import { CorsOptions } from 'cors'; +import { makeRe } from 'micromatch'; export type BaseOptions = { listenPort?: string | number; @@ -112,7 +113,7 @@ export function readCorsOptions(config: Config): CorsOptions | undefined { } return removeUnknown({ - origin: getOptionalStringOrStrings(cc, 'origin'), + origin: getOptionalGlobOrGlobs(cc, 'origin'), methods: getOptionalStringOrStrings(cc, 'methods'), allowedHeaders: getOptionalStringOrStrings(cc, 'allowedHeaders'), exposedHeaders: getOptionalStringOrStrings(cc, 'exposedHeaders'), @@ -217,6 +218,23 @@ function getOptionalStringOrStrings( throw new Error(`Expected string or array of strings, got ${typeof value}`); } +function getOptionalGlobOrGlobs( + config: Config, + key: string, +): RegExp | RegExp[] | undefined { + const value = config.getOptional(key); + 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}`); +} + function isStringArray(value: any): value is string[] { if (!Array.isArray(value)) { return false; diff --git a/yarn.lock b/yarn.lock index c961b6045b..ca1483daf5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7088,6 +7088,13 @@ dependencies: "@types/braces" "*" +"@types/micromatch@^4.0.2": + version "4.0.2" + resolved "https://registry.npmjs.org/@types/micromatch/-/micromatch-4.0.2.tgz#ce29c8b166a73bf980a5727b1e4a4d099965151d" + integrity sha512-oqXqVb0ci19GtH0vOA/U2TmHTcRY9kuZl4mqUxe0QmJAlIW13kzhuK5pi1i9+ngav8FjpSb9FVS/GE00GLX1VA== + dependencies: + "@types/braces" "*" + "@types/mime-types@^2.1.0": version "2.1.0" resolved "https://registry.npmjs.org/@types/mime-types/-/mime-types-2.1.0.tgz#9ca52cda363f699c69466c2a6ccdaad913ea7a73" From 0dcdf5a0065bc51d6c8cb340130cd210656d79cb Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 6 Oct 2021 12:27:57 +0200 Subject: [PATCH 2/7] use minimatch pattern match to validate origin Signed-off-by: Juan Pablo Garcia Ripa --- .../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; +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[] { From 10ef0ffbd7a3cc4d4ffa7e3bc69fb04bbb2ce847 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 6 Oct 2021 12:36:43 +0200 Subject: [PATCH 3/7] remove micromatch dependency Signed-off-by: Juan Pablo Garcia Ripa --- packages/backend-common/package.json | 2 -- yarn.lock | 7 ------- 2 files changed, 9 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 3c426d780a..29018ef8c8 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -57,7 +57,6 @@ "knex": "^0.95.1", "lodash": "^4.17.21", "logform": "^2.1.1", - "micromatch": "^4.0.4", "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", @@ -85,7 +84,6 @@ "@types/concat-stream": "^1.6.0", "@types/fs-extra": "^9.0.3", "@types/http-errors": "^1.6.3", - "@types/micromatch": "^4.0.2", "@types/minimist": "^1.2.0", "@types/mock-fs": "^4.13.0", "@types/morgan": "^1.9.0", diff --git a/yarn.lock b/yarn.lock index ca1483daf5..c961b6045b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7088,13 +7088,6 @@ dependencies: "@types/braces" "*" -"@types/micromatch@^4.0.2": - version "4.0.2" - resolved "https://registry.npmjs.org/@types/micromatch/-/micromatch-4.0.2.tgz#ce29c8b166a73bf980a5727b1e4a4d099965151d" - integrity sha512-oqXqVb0ci19GtH0vOA/U2TmHTcRY9kuZl4mqUxe0QmJAlIW13kzhuK5pi1i9+ngav8FjpSb9FVS/GE00GLX1VA== - dependencies: - "@types/braces" "*" - "@types/mime-types@^2.1.0": version "2.1.0" resolved "https://registry.npmjs.org/@types/mime-types/-/mime-types-2.1.0.tgz#9ca52cda363f699c69466c2a6ccdaad913ea7a73" From d7055285de636efa4a5e0edd2e8588069409225b Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 6 Oct 2021 14:01:07 +0200 Subject: [PATCH 4/7] add changeset Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/rich-deers-run.md | 16 ++++++++++++++++ .../src/service/lib/config.test.ts | 8 +++++--- 2 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 .changeset/rich-deers-run.md diff --git a/.changeset/rich-deers-run.md b/.changeset/rich-deers-run.md new file mode 100644 index 0000000000..23d81ef30d --- /dev/null +++ b/.changeset/rich-deers-run.md @@ -0,0 +1,16 @@ +--- +'@backstage/backend-common': patch +--- + +Add glob patterns support to config CORS options. It's possible to send patterns like: + +```yaml +backend: + cors: + origin: + [ + https://*.my-domain.com, + http://localhost:700?, + 'https://sub-domain-+([0-9]).my-domain.com', + ] +``` diff --git a/packages/backend-common/src/service/lib/config.test.ts b/packages/backend-common/src/service/lib/config.test.ts index 59a11d3ab5..29693a8fcd 100644 --- a/packages/backend-common/src/service/lib/config.test.ts +++ b/packages/backend-common/src/service/lib/config.test.ts @@ -69,7 +69,9 @@ describe('config', () => { it('reads string array', () => { const mockCallback = jest.fn(); const config = new ConfigReader({ - cors: { origin: ['https://*.value*', 'http://*.value'] }, + cors: { + origin: ['http?(s)://*.value?(-+([0-9])).com', 'http://*.value'], + }, }); const cors = readCorsOptions(config); expect(cors).toEqual( @@ -78,8 +80,8 @@ describe('config', () => { }), ); const origin = cors?.origin as Function; - origin('https://a.value', mockCallback); - origin('https://a.valuex', mockCallback); + origin('https://a.b.c.value-9.com', mockCallback); + origin('http://a.value-999.com', mockCallback); origin('http://a.value', mockCallback); origin('http://a.valuex', mockCallback); From a44f4c742092254219e55bebf85d8ad3f37e9d91 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 6 Oct 2021 14:47:46 +0200 Subject: [PATCH 5/7] better changeset example Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/rich-deers-run.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.changeset/rich-deers-run.md b/.changeset/rich-deers-run.md index 23d81ef30d..e8a15c4464 100644 --- a/.changeset/rich-deers-run.md +++ b/.changeset/rich-deers-run.md @@ -8,9 +8,7 @@ Add glob patterns support to config CORS options. It's possible to send patterns backend: cors: origin: - [ - https://*.my-domain.com, - http://localhost:700?, - 'https://sub-domain-+([0-9]).my-domain.com', - ] + - https://*.my-domain.com + - http://localhost:700[0-9] + - https://sub-domain-+([0-9]).my-domain.com ``` From 8ebec8486524b2257703b8c7e4e97a557157ef9b Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 6 Oct 2021 15:44:34 +0200 Subject: [PATCH 6/7] add test for undefined values Signed-off-by: Juan Pablo Garcia Ripa --- packages/backend-common/src/service/lib/config.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/backend-common/src/service/lib/config.test.ts b/packages/backend-common/src/service/lib/config.test.ts index 29693a8fcd..b80d475517 100644 --- a/packages/backend-common/src/service/lib/config.test.ts +++ b/packages/backend-common/src/service/lib/config.test.ts @@ -95,5 +95,14 @@ describe('config', () => { expect(mockCallback.mock.calls[2][1]).toBe(true); expect(mockCallback.mock.calls[3][1]).toBe(false); }); + + it('reads undefined origin', () => { + const config = new ConfigReader({ + cors: {}, + }); + const cors = readCorsOptions(config); + expect(cors).toEqual(expect.objectContaining({})); + expect(cors?.origin).toBeUndefined(); + }); }); }); From a745ae275746cc2270e17eff90db5ee83498b753 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 6 Oct 2021 15:49:56 +0200 Subject: [PATCH 7/7] better name function Signed-off-by: Juan Pablo Garcia Ripa --- .../backend-common/src/service/lib/config.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/backend-common/src/service/lib/config.ts b/packages/backend-common/src/service/lib/config.ts index ad20d99b0e..25820dde91 100644 --- a/packages/backend-common/src/service/lib/config.ts +++ b/packages/backend-common/src/service/lib/config.ts @@ -120,7 +120,7 @@ export function readCorsOptions(config: Config): CorsOptions | undefined { } return removeUnknown({ - origin: getOptionalGlobOrGlobs(cc, 'origin'), + origin: createCorsOriginMatcher(getOptionalStringOrStrings(cc, 'origin')), methods: getOptionalStringOrStrings(cc, 'methods'), allowedHeaders: getOptionalStringOrStrings(cc, 'allowedHeaders'), exposedHeaders: getOptionalStringOrStrings(cc, 'exposedHeaders'), @@ -221,23 +221,24 @@ function getOptionalStringOrStrings( throw new Error(`Expected string or array of strings, got ${typeof value}`); } -function getOptionalGlobOrGlobs( - config: Config, - key: string, +function createCorsOriginMatcher( + originValue: string | string[] | undefined, ): CustomOrigin | undefined { - const value = config.getOptional(key); - if (!isStringOrStrings(value)) { - throw new Error(`Expected string or array of strings, got ${typeof value}`); + if (originValue === undefined) { + return originValue; } - if (value === undefined) { - return value; + if (!isStringOrStrings(originValue)) { + throw new Error( + `Expected string or array of strings, got ${typeof originValue}`, + ); } - const valueArr = typeof value === 'string' ? [value] : value; + const allowedOrigin = + typeof originValue === 'string' ? [originValue] : originValue; const allowedOriginPatterns = - valueArr?.map( + allowedOrigin?.map( pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }), ) ?? [];