From 0c8ba31d72670d9ff5eda3ca726e061fff088115 Mon Sep 17 00:00:00 2001 From: Harry Hogg Date: Tue, 1 Mar 2022 17:46:39 +0000 Subject: [PATCH 1/2] plugin-auth-backend: Added validation to ensure any custom auth resolvers are using EntityRefs for subject claims Signed-off-by: Harry Hogg --- .changeset/tasty-poems-raise.md | 5 +++ .../src/identity/TokenFactory.test.ts | 39 ++++++++++++++++--- .../auth-backend/src/identity/TokenFactory.ts | 10 +++++ 3 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 .changeset/tasty-poems-raise.md diff --git a/.changeset/tasty-poems-raise.md b/.changeset/tasty-poems-raise.md new file mode 100644 index 0000000000..47556cb3ad --- /dev/null +++ b/.changeset/tasty-poems-raise.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': minor +--- + +Added validation to TokenFactory.issueToken that ensure any sub claim given is a valid entityRef. This will affect any custom resolver functions given to auth providers. diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index f5f06f1209..4b1c1804df 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -18,6 +18,7 @@ import { MemoryKeyStore } from './MemoryKeyStore'; import { TokenFactory } from './TokenFactory'; import { getVoidLogger } from '@backstage/backend-common'; import { JWKS, JSONWebKey, JWT } from 'jose'; +import { stringifyEntityRef } from '@backstage/catalog-model'; const logger = getVoidLogger(); @@ -28,6 +29,12 @@ function jwtKid(jwt: string): string { return header.kid; } +const entityRef = stringifyEntityRef({ + kind: 'User', + namespace: 'default', + name: 'JackFrost', +}); + describe('TokenFactory', () => { it('should issue valid tokens signed by a listed key', async () => { const keyDurationSeconds = 5; @@ -39,7 +46,7 @@ describe('TokenFactory', () => { }); await expect(factory.listPublicKeys()).resolves.toEqual({ keys: [] }); - const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const token = await factory.issueToken({ claims: { sub: entityRef } }); const { keys } = await factory.listPublicKeys(); const keyStore = JWKS.asKeyStore({ @@ -53,7 +60,7 @@ describe('TokenFactory', () => { expect(payload).toEqual({ iss: 'my-issuer', aud: 'backstage', - sub: 'foo', + sub: entityRef, iat: expect.any(Number), exp: expect.any(Number), }); @@ -71,8 +78,12 @@ describe('TokenFactory', () => { logger, }); - const token1 = await factory.issueToken({ claims: { sub: 'foo' } }); - const token2 = await factory.issueToken({ claims: { sub: 'foo' } }); + const token1 = await factory.issueToken({ + claims: { sub: entityRef }, + }); + const token2 = await factory.issueToken({ + claims: { sub: entityRef }, + }); expect(jwtKid(token1)).toBe(jwtKid(token2)); await expect(factory.listPublicKeys()).resolves.toEqual({ @@ -89,7 +100,9 @@ describe('TokenFactory', () => { keys: [], }); - const token3 = await factory.issueToken({ claims: { sub: 'foo' } }); + const token3 = await factory.issueToken({ + claims: { sub: entityRef }, + }); expect(jwtKid(token3)).not.toBe(jwtKid(token2)); await expect(factory.listPublicKeys()).resolves.toEqual({ @@ -100,4 +113,20 @@ describe('TokenFactory', () => { ], }); }); + + it('should throw an error with a non entityRef sub claim', async () => { + const keyDurationSeconds = 5; + const factory = new TokenFactory({ + issuer: 'my-issuer', + keyStore: new MemoryKeyStore(), + keyDurationSeconds, + logger, + }); + + await expect(() => { + return factory.issueToken({ + claims: { sub: 'UserId' }, + }); + }).rejects.toThrowError(); + }); }); diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 041a292d44..cb2a8b453d 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -19,6 +19,7 @@ import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; import { DateTime } from 'luxon'; +import { parseEntityRef } from '@backstage/catalog-model'; const MS_IN_S = 1000; @@ -72,6 +73,15 @@ export class TokenFactory implements TokenIssuer { const iat = Math.floor(Date.now() / MS_IN_S); const exp = iat + this.keyDurationSeconds; + // Validate that the subject claim is a valid EntityRef + try { + parseEntityRef(sub); + } catch (error) { + throw new Error( + '"sub" claim provided by the auth resolver is not a valid EntityRef.', + ); + } + this.logger.info(`Issuing token for ${sub}, with entities ${ent ?? []}`); return JWS.sign({ iss, sub, aud, iat, exp, ent }, key, { From 8a687faa7bfb2616345a55a4e06e4dd618298cb4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 3 Mar 2022 12:51:15 +0100 Subject: [PATCH 2/2] Update tasty-poems-raise.md Signed-off-by: Patrik Oldsberg --- .changeset/tasty-poems-raise.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tasty-poems-raise.md b/.changeset/tasty-poems-raise.md index 47556cb3ad..fbd81423b6 100644 --- a/.changeset/tasty-poems-raise.md +++ b/.changeset/tasty-poems-raise.md @@ -2,4 +2,4 @@ '@backstage/plugin-auth-backend': minor --- -Added validation to TokenFactory.issueToken that ensure any sub claim given is a valid entityRef. This will affect any custom resolver functions given to auth providers. +**BREAKING**: The `TokenFactory.issueToken` used by custom sign-in resolvers now ensures that the sub claim given is a full entity reference of the format `:/`. Any existing custom sign-in resolver functions that do not supply a full entity reference must be updated.