From 79d24a9665f6829f8fad65d3f2fe694aa881bff1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 20 Aug 2021 10:36:33 +0200 Subject: [PATCH] auth-backend: fix origin verification Signed-off-by: Patrik Oldsberg --- .changeset/thirty-turkeys-perform.md | 5 +++++ plugins/auth-backend/src/providers/types.ts | 2 +- .../auth-backend/src/service/router.test.ts | 20 +++++++++++++------ plugins/auth-backend/src/service/router.ts | 16 +++++++++------ 4 files changed, 30 insertions(+), 13 deletions(-) create mode 100644 .changeset/thirty-turkeys-perform.md diff --git a/.changeset/thirty-turkeys-perform.md b/.changeset/thirty-turkeys-perform.md new file mode 100644 index 0000000000..3eb58877af --- /dev/null +++ b/.changeset/thirty-turkeys-perform.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Fix an issue where the default app origin was not allowed to authenticate users. diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index 66928ab4a7..6f6df5d904 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -36,7 +36,7 @@ export type AuthProviderConfig = { appUrl: string; /** - * A function that is called to check whether an origin other than the apps default one is allowed. + * A function that is called to check whether an origin is allowed to receive the authentication result. */ isOriginAllowed: (origin: string) => boolean; }; diff --git a/plugins/auth-backend/src/service/router.test.ts b/plugins/auth-backend/src/service/router.test.ts index f0f5134848..3a1fde881e 100644 --- a/plugins/auth-backend/src/service/router.test.ts +++ b/plugins/auth-backend/src/service/router.test.ts @@ -18,27 +18,35 @@ import { ConfigReader } from '@backstage/config'; import { createOriginFilter } from './router'; describe('Auth origin filtering', () => { - const defaultConfigOptions = { + const config = new ConfigReader({ + app: { + baseUrl: 'http://example.com/extra-path', + }, auth: { experimentalExtraAllowedOrigins: ['https://test-*.example.net'], }, - }; - const defaultConfig = () => new ConfigReader(defaultConfigOptions); - const getOptionalString = jest.fn(); - const config = defaultConfig(); - config.getOptionalString = getOptionalString; + }); + it('Will explode, invalid origin', () => { const origin = 'https://test.example.net'; expect(createOriginFilter(config)(origin)).toBeFalsy(); }); + it('Will explode, invalid origin domain', () => { const origin = 'https://test-1234.examplee.net'; expect(createOriginFilter(config)(origin)).toBeFalsy(); }); + + it("Won't explode, uses app origin", () => { + const origin = 'http://example.com'; + expect(createOriginFilter(config)(origin)).toBeTruthy(); + }); + it("Won't explode, valid origin with numbers", () => { const origin = 'https://test-1234.example.net'; expect(createOriginFilter(config)(origin)).toBeTruthy(); }); + it("Won't explode, valid origin with chars and numbers", () => { const origin = 'https://test-test1234.example.net'; expect(createOriginFilter(config)(origin)).toBeTruthy(); diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index de6c1ffe03..423975aa75 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -165,18 +165,22 @@ export async function createRouter({ export function createOriginFilter( config: Config, ): (origin: string) => boolean { + const appUrl = config.getString('app.baseUrl'); + const { origin: appOrigin } = new URL(appUrl); + const allowedOrigins = config.getOptionalStringArray( 'auth.experimentalExtraAllowedOrigins', ); - if (!allowedOrigins || allowedOrigins.length === 0) { - return () => false; - } - const allowedOriginPatterns = allowedOrigins.map( - pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }), - ); + const allowedOriginPatterns = + allowedOrigins?.map( + pattern => new Minimatch(pattern, { nocase: true, noglobstar: true }), + ) ?? []; return origin => { + if (origin === appOrigin) { + return true; + } return allowedOriginPatterns.some(pattern => pattern.match(origin)); }; }