From 09f89885bbdb45a82569096153a91ee745f8e9c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 24 Apr 2024 18:36:24 +0200 Subject: [PATCH 1/2] Remove explicit alg check for user tokens in verifyToken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/loud-timers-flow.md | 5 +++++ .../implementations/auth/user/UserTokenHandler.ts | 11 ++--------- 2 files changed, 7 insertions(+), 9 deletions(-) create mode 100644 .changeset/loud-timers-flow.md diff --git a/.changeset/loud-timers-flow.md b/.changeset/loud-timers-flow.md new file mode 100644 index 0000000000..d7d0d96970 --- /dev/null +++ b/.changeset/loud-timers-flow.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Remove explicit `alg` check for user tokens in `verifyToken` diff --git a/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.ts b/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.ts index af3faae22d..bd73d8701f 100644 --- a/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.ts +++ b/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.ts @@ -34,18 +34,14 @@ import { JwksClient } from '../JwksClient'; */ export class UserTokenHandler { static create(options: { discovery: DiscoveryService }): UserTokenHandler { - const algorithms = ['ES256']; // TODO: configurable? const jwksClient = new JwksClient(async () => { const url = await options.discovery.getBaseUrl('auth'); return new URL(`${url}/.well-known/jwks.json`); }); - return new UserTokenHandler(algorithms, jwksClient); + return new UserTokenHandler(jwksClient); } - constructor( - private readonly algorithms: string[], - private readonly jwksClient: JwksClient, - ) {} + constructor(private readonly jwksClient: JwksClient) {} async verifyToken(token: string) { const verifyOpts = this.#getTokenVerificationOptions(token); @@ -79,7 +75,6 @@ export class UserTokenHandler { if (typ === tokenTypes.user.typParam) { return { - algorithms: this.algorithms, requiredClaims: ['iat', 'exp', 'sub'], typ: tokenTypes.user.typParam, }; @@ -87,7 +82,6 @@ export class UserTokenHandler { if (typ === tokenTypes.limitedUser.typParam) { return { - algorithms: this.algorithms, requiredClaims: ['iat', 'exp', 'sub'], typ: tokenTypes.limitedUser.typParam, }; @@ -96,7 +90,6 @@ export class UserTokenHandler { const { aud } = decodeJwt(token); if (aud === tokenTypes.user.audClaim) { return { - algorithms: this.algorithms, audience: tokenTypes.user.audClaim, }; } From 0329b2514b3790413f53bb0f5012f816718f094f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 25 Apr 2024 09:57:53 +0200 Subject: [PATCH 2/2] add test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../auth/user/UserTokenHandler.test.ts | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.test.ts b/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.test.ts index f2a86e4a20..b0e8418655 100644 --- a/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.test.ts +++ b/packages/backend-app-api/src/services/implementations/auth/user/UserTokenHandler.test.ts @@ -133,6 +133,34 @@ describe('UserTokenHandler', () => { ).rejects.toThrow('signature verification failed'); }); + it('should fail to verify tokens that have a bad alg', async () => { + const expectedIssuedAt = 1712071714; + const expectedExpiresAt = 1712075314; + + jest.useFakeTimers({ + now: expectedIssuedAt * 1000 + 600_000, + }); + + const header = encodeData({ + typ: 'vnd.backstage.user', + alg: 'none', + }); + const payload = encodeData({ + iss: 'http://localhost:7007/api/auth', + sub: 'user:development/guest', + ent: ['user:development/guest', 'group:default/team-a'], + aud: 'backstage', + iat: expectedIssuedAt, + exp: expectedExpiresAt, + uip: 'proof', + }); + const token = `${header}.${payload}.`; + + await expect(userTokenHandler.verifyToken(token)).rejects.toThrow( + /Unsupported "alg" value/, + ); + }); + it('should verify a valid legacy backstage token', async () => { const expectedIssuedAt = 1712071714; const expectedExpiresAt = 1712075314; @@ -151,7 +179,7 @@ describe('UserTokenHandler', () => { sub: 'user:development/guest', ent: ['user:development/guest', 'group:default/team-a'], aud: 'backstage', - iat: 1712071714, + iat: expectedIssuedAt, exp: expectedExpiresAt, }, }; @@ -179,7 +207,7 @@ describe('UserTokenHandler', () => { iss: 'http://localhost:7007/api/auth', ent: ['user:development/guest', 'group:default/team-a'], aud: 'backstage', - iat: 1712071714, + iat: expectedIssuedAt, exp: expectedExpiresAt, }, }; @@ -209,7 +237,7 @@ describe('UserTokenHandler', () => { sub: 'user:development/guest', ent: ['user:development/guest', 'group:default/team-a'], aud: 'backstage', - iat: 1712071714, + iat: expectedIssuedAt, exp: expectedExpiresAt, uip: 'proof', }, @@ -239,7 +267,7 @@ describe('UserTokenHandler', () => { payload: { sub: 'user:development/guest', ent: ['user:development/guest', 'group:default/team-a'], - iat: 1712071714, + iat: expectedIssuedAt, exp: expectedExpiresAt, }, };