diff --git a/.changeset/tasty-drinks-teach.md b/.changeset/tasty-drinks-teach.md new file mode 100644 index 0000000000..4b4a5a748d --- /dev/null +++ b/.changeset/tasty-drinks-teach.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-common': patch +'@backstage/plugin-auth-backend': patch +'@backstage/plugin-auth-node': patch +--- + +Update to `jose` 4.6.0 diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 8a15c5ee8b..4fd9ede43c 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -60,7 +60,7 @@ "git-url-parse": "^11.6.0", "helmet": "^5.0.2", "isomorphic-git": "^1.8.0", - "jose": "^1.27.1", + "jose": "^4.6.0", "keyv": "^4.0.3", "keyv-memcache": "^1.2.5", "@keyv/redis": "^2.2.3", diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 1799291544..0ff7bfb316 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.test.ts @@ -17,7 +17,7 @@ import { getVoidLogger } from '../logging/voidLogger'; import { ConfigReader } from '@backstage/config'; import { ServerTokenManager } from './ServerTokenManager'; import { Logger } from 'winston'; -import { JWK } from 'jose'; +import * as jose from 'jose'; import { TokenManager } from './types'; const emptyConfig = new ConfigReader({}); @@ -204,7 +204,7 @@ describe('ServerTokenManager', () => { }); describe('NODE_ENV === development', () => { - const generateSyncSpy = jest.spyOn(JWK, 'generateSync'); + const generateSecretSpy = jest.spyOn(jose, 'generateSecret'); beforeEach(() => { (process.env as any).NODE_ENV = 'development'; @@ -214,26 +214,30 @@ describe('ServerTokenManager', () => { jest.clearAllMocks(); }); - it('should generate a key if no config is provided', () => { - ServerTokenManager.fromConfig(emptyConfig, { logger }); - - expect(generateSyncSpy).toHaveBeenCalledWith('oct', 192); + it('should generate a key if no config is provided', async () => { + const tokenManager = ServerTokenManager.fromConfig(emptyConfig, { + logger, + }); + const token = await tokenManager.getToken(); + expect(token).toBeDefined(); + expect(generateSecretSpy).toHaveBeenCalledWith('HS256'); }); - it('should generate a key if no keys are provided in the configuration', () => { - ServerTokenManager.fromConfig( + it('should generate a key if no keys are provided in the configuration', async () => { + const tokenManager = ServerTokenManager.fromConfig( new ConfigReader({ backend: { auth: { keys: [] } }, }), { logger }, ); - - expect(generateSyncSpy).toHaveBeenCalledWith('oct', 192); + const token = await tokenManager.getToken(); + expect(token).toBeDefined(); + expect(generateSecretSpy).toHaveBeenCalledWith('HS256'); }); it('should use provided secrets if config is provided', () => { ServerTokenManager.fromConfig(configWithSecret, { logger }); - expect(generateSyncSpy).not.toHaveBeenCalled(); + expect(generateSecretSpy).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index d6693edba1..484b99ca4d 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { JWKS, JWK, JWT } from 'jose'; +import { base64url, generateSecret, SignJWT, jwtVerify, exportJWK } from 'jose'; import { Config } from '@backstage/config'; import { AuthenticationError } from '@backstage/errors'; import { TokenManager } from './types'; @@ -37,8 +37,10 @@ class NoopTokenManager implements TokenManager { * @public */ export class ServerTokenManager implements TokenManager { - private readonly verificationKeys: JWKS.KeyStore; - private readonly signingKey: JWK.Key; + private verificationKeys: Uint8Array[]; + private signingKey: Uint8Array; + private privateKeyPromise?: Promise; + private logger: Logger; static noop(): TokenManager { return new NoopTokenManager(); @@ -49,51 +51,87 @@ export class ServerTokenManager implements TokenManager { const keys = config.getOptionalConfigArray('backend.auth.keys'); if (keys?.length) { - return new ServerTokenManager(keys.map(key => key.getString('secret'))); + return new ServerTokenManager( + keys.map(key => key.getString('secret')), + logger, + ); } if (process.env.NODE_ENV !== 'development') { throw new Error( 'You must configure at least one key in backend.auth.keys for production.', ); } - // For development, if a secret has not been configured, we auto generate a secret instead of throwing. - const generatedDevOnlyKey = JWK.generateSync('oct', 24 * 8); - if (generatedDevOnlyKey.k === undefined) { - throw new Error('Internal error, JWK key generation returned no data'); - } logger.warn( 'Generated a secret for backend-to-backend authentication: DEVELOPMENT USE ONLY.', ); - return new ServerTokenManager([generatedDevOnlyKey.k]); + return new ServerTokenManager([], logger); } - private constructor(secrets: string[]) { - if (!secrets.length) { + private constructor(secrets: string[], logger: Logger) { + if (!secrets.length && process.env.NODE_ENV !== 'development') { throw new Error( 'No secrets provided when constructing ServerTokenManager', ); } + this.logger = logger; + this.verificationKeys = secrets.map(s => base64url.decode(s)); + this.signingKey = this.verificationKeys[0]; + } - this.verificationKeys = new JWKS.KeyStore( - secrets.map(k => JWK.asKey({ kty: 'oct', k })), - ); - this.signingKey = this.verificationKeys.all()[0]; + // Called when no keys have been generated yet in the dev environment + private async generateKeys(): Promise { + if (process.env.NODE_ENV !== 'development') { + throw new Error( + 'Key generation is not supported outside of the dev environment', + ); + } + if (this.privateKeyPromise) { + return this.privateKeyPromise; + } + const promise = (async () => { + const secret = await generateSecret('HS256'); + const jwk = await exportJWK(secret); + this.verificationKeys.push(base64url.decode(jwk.k ?? '')); + this.signingKey = this.verificationKeys[0]; + return; + })(); + + try { + // If we fail to generate a new key, we need to clear the state so that + // the next caller will try to generate another key. + await promise; + } catch (error) { + this.logger.error(`Failed to generate new key, ${error}`); + delete this.privateKeyPromise; + } + return promise; } async getToken(): Promise<{ token: string }> { - const jwt = JWT.sign({ sub: 'backstage-server' }, this.signingKey, { - algorithm: 'HS256', - }); - + if (!this.verificationKeys.length) { + await this.generateKeys(); + } + const sub = 'backstage-server'; + const jwt = await new SignJWT({ alg: 'HS256' }) + .setProtectedHeader({ alg: 'HS256', sub: sub }) + .setSubject('backstage-server') + .sign(this.signingKey); return { token: jwt }; } async authenticate(token: string): Promise { - try { - JWT.verify(token, this.verificationKeys); - } catch (e) { - throw new AuthenticationError('Invalid server token'); + let verifyError = undefined; + for (const key of this.verificationKeys) { + try { + await jwtVerify(token, key); + // If the verify succeeded, return + return; + } catch (e) { + // Catch the verify exception and continue + verifyError = e; + } } + throw new AuthenticationError(`Invalid server token: ${verifyError}`); } } diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index ce6d65f276..c7d64a72af 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -51,7 +51,7 @@ "express-session": "^1.17.1", "fs-extra": "10.0.1", "google-auth-library": "^7.6.1", - "jose": "^1.27.1", + "jose": "^4.6.0", "jwt-decode": "^3.1.0", "knex": "^1.0.2", "lodash": "^4.17.21", diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index 4b1c1804df..14b3a00a7f 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -17,15 +17,16 @@ import { MemoryKeyStore } from './MemoryKeyStore'; import { TokenFactory } from './TokenFactory'; import { getVoidLogger } from '@backstage/backend-common'; -import { JWKS, JSONWebKey, JWT } from 'jose'; +import { createLocalJWKSet, decodeProtectedHeader, jwtVerify } from 'jose'; import { stringifyEntityRef } from '@backstage/catalog-model'; const logger = getVoidLogger(); function jwtKid(jwt: string): string { - const { header } = JWT.decode(jwt, { complete: true }) as { - header: { kid: string }; - }; + const header = decodeProtectedHeader(jwt); + if (!header.kid) { + throw new Error('JWT Header did not contain a key ID (kid)'); + } return header.kid; } @@ -49,22 +50,19 @@ describe('TokenFactory', () => { const token = await factory.issueToken({ claims: { sub: entityRef } }); const { keys } = await factory.listPublicKeys(); - const keyStore = JWKS.asKeyStore({ - keys: keys.map(key => key as JSONWebKey), - }); + const keyStore = createLocalJWKSet({ keys: keys }); - const payload = JWT.verify(token, keyStore) as object & { - iat: number; - exp: number; - }; - expect(payload).toEqual({ + const verifyResult = await jwtVerify(token, keyStore); + expect(verifyResult.payload).toEqual({ iss: 'my-issuer', aud: 'backstage', sub: entityRef, iat: expect.any(Number), exp: expect.any(Number), }); - expect(payload.exp).toBe(payload.iat + keyDurationSeconds); + expect(verifyResult.payload.exp).toBe( + verifyResult.payload.iat! + keyDurationSeconds, + ); }); it('should generate new signing keys when the current one expires', async () => { diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index cb2a8b453d..b72c3ef06d 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -15,11 +15,12 @@ */ import { TokenIssuer, TokenParams, KeyStore, AnyJWK } from './types'; -import { JSONWebKey, JWK, JWS } from 'jose'; +import { exportJWK, generateKeyPair, importJWK, JWK, SignJWT } from 'jose'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; import { DateTime } from 'luxon'; import { parseEntityRef } from '@backstage/catalog-model'; +import { AuthenticationError } from '@backstage/errors'; const MS_IN_S = 1000; @@ -54,7 +55,7 @@ export class TokenFactory implements TokenIssuer { private readonly keyDurationSeconds: number; private keyExpiry?: Date; - private privateKeyPromise?: Promise; + private privateKeyPromise?: Promise; constructor(options: Options) { this.issuer = options.issuer; @@ -84,10 +85,18 @@ export class TokenFactory implements TokenIssuer { this.logger.info(`Issuing token for ${sub}, with entities ${ent ?? []}`); - return JWS.sign({ iss, sub, aud, iat, exp, ent }, key, { - alg: key.alg, - kid: key.kid, - }); + if (!key.alg) { + throw new AuthenticationError('No algorithm was provided in the key'); + } + + return new SignJWT({ iss, sub, aud, iat, exp }) + .setProtectedHeader({ alg: key.alg, kid: key.kid }) + .setIssuer(iss) + .setAudience(aud) + .setSubject(sub) + .setIssuedAt(iat) + .setExpirationTime(exp) + .sign(await importJWK(key)); } // This will be called by other services that want to verify ID tokens. @@ -127,7 +136,7 @@ export class TokenFactory implements TokenIssuer { return { keys: validKeys.map(({ key }) => key) }; } - private async getKey(): Promise { + private async getKey(): Promise { // Make sure that we only generate one key at a time if (this.privateKeyPromise) { if ( @@ -147,11 +156,11 @@ export class TokenFactory implements TokenIssuer { .toJSDate(); const promise = (async () => { // This generates a new signing key to be used to sign tokens until the next key rotation - const key = await JWK.generate('EC', 'P-256', { - use: 'sig', - kid: uuid(), - alg: 'ES256', - }); + const key = await generateKeyPair('ES256'); + const publicKey = await exportJWK(key.publicKey); + const privateKey = await exportJWK(key.privateKey); + publicKey.kid = privateKey.kid = uuid(); + publicKey.alg = privateKey.alg = 'ES256'; // We're not allowed to use the key until it has been successfully stored // TODO: some token verification implementations aggressively cache the list of keys, and @@ -159,11 +168,11 @@ export class TokenFactory implements TokenIssuer { // may want to keep using the existing key for some period of time until we switch to // the new one. This also needs to be implemented cross-service though, meaning new services // that boot up need to be able to grab an existing key to use for signing. - this.logger.info(`Created new signing key ${key.kid}`); - await this.keyStore.addKey(key.toJWK(false) as unknown as AnyJWK); + this.logger.info(`Created new signing key ${publicKey.kid}`); + await this.keyStore.addKey(publicKey as AnyJWK); // At this point we are allowed to start using the new key - return key as JSONWebKey; + return privateKey; })(); this.privateKeyPromise = promise; diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts index a917858743..6ba545191b 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -15,7 +15,7 @@ */ import express from 'express'; -import { JWT } from 'jose'; +import { jwtVerify } from 'jose'; import { ALB_ACCESS_TOKEN_HEADER, ALB_JWT_HEADER, @@ -25,7 +25,7 @@ import { makeProfileInfo } from '../../lib/passport'; import { AuthResolverContext } from '../types'; import { AuthenticationError } from '@backstage/errors'; -const jwtMock = JWT as jest.Mocked; +const jwtMock = jwtVerify as jest.Mocked; const mockKey = async () => { return `-----BEGIN PUBLIC KEY----- @@ -116,7 +116,7 @@ describe('AwsAlbAuthProvider', () => { }, }); - jwtMock.verify.mockReturnValueOnce(mockClaims); + jwtMock.mockReturnValueOnce(Promise.resolve({ payload: mockClaims })); await provider.refresh(mockRequest, mockResponse); @@ -193,7 +193,7 @@ describe('AwsAlbAuthProvider', () => { }, }); - jwtMock.verify.mockImplementationOnce(() => { + jwtMock.mockImplementationOnce(() => { throw new Error('bad JWT'); }); @@ -215,7 +215,7 @@ describe('AwsAlbAuthProvider', () => { }, }); - jwtMock.verify.mockReturnValueOnce({}); + jwtMock.mockReturnValueOnce({}); await expect(provider.refresh(mockRequest, mockResponse)).rejects.toThrow( AuthenticationError, @@ -235,7 +235,7 @@ describe('AwsAlbAuthProvider', () => { }, }); - jwtMock.verify.mockReturnValueOnce({ + jwtMock.mockReturnValueOnce({ iss: 'INVALID_ISSUE_URL', }); @@ -257,7 +257,7 @@ describe('AwsAlbAuthProvider', () => { }, }); - jwtMock.verify.mockReturnValueOnce(mockClaims); + jwtMock.mockReturnValueOnce(mockClaims); await expect(provider.refresh(mockRequest, mockResponse)).rejects.toThrow( AuthenticationError, @@ -277,7 +277,7 @@ describe('AwsAlbAuthProvider', () => { }, }); - jwtMock.verify.mockReturnValueOnce(mockClaims); + jwtMock.mockReturnValueOnce(mockClaims); await expect(provider.refresh(mockRequest, mockResponse)).rejects.toThrow( AuthenticationError, diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index 8e021e8817..78bfb9f0ec 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -26,7 +26,7 @@ import fetch from 'node-fetch'; import * as crypto from 'crypto'; import { KeyObject } from 'crypto'; import NodeCache from 'node-cache'; -import { JWT } from 'jose'; +import { JWTHeaderParameters, jwtVerify } from 'jose'; import { Profile as PassportProfile } from 'passport'; import { makeProfileInfo } from '../../lib/passport'; import { AuthenticationError } from '@backstage/errors'; @@ -44,11 +44,6 @@ type Options = { resolverContext: AuthResolverContext; }; -export const getJWTHeaders = (input: string): AwsAlbHeaders => { - const encoded = input.split('.')[0]; - return JSON.parse(Buffer.from(encoded, 'base64').toString('utf8')); -}; - export type AwsAlbHeaders = { alg: string; kid: string; @@ -144,9 +139,8 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { } try { - const headers = getJWTHeaders(jwt); - const key = await this.getKey(headers.kid); - const claims = JWT.verify(jwt, key) as AwsAlbClaims; + const verifyResult = await jwtVerify(jwt, this.getKey); + const claims = verifyResult.payload as AwsAlbClaims; if (this.issuer && claims.iss !== this.issuer) { throw new AuthenticationError('Issuer mismatch on JWT token'); @@ -195,18 +189,24 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { }; } - async getKey(keyId: string): Promise { - const optionalCacheKey = this.keyCache.get(keyId); + async getKey(header: JWTHeaderParameters): Promise { + if (!header.kid) { + throw new AuthenticationError('No key id was specified in header'); + } + const optionalCacheKey = this.keyCache.get(header.kid); if (optionalCacheKey) { return crypto.createPublicKey(optionalCacheKey); } - const keyText = await fetch( + const keyText: string = await fetch( `https://public-keys.auth.elb.${encodeURIComponent( this.region, - )}.amazonaws.com/${encodeURIComponent(keyId)}`, + )}.amazonaws.com/${encodeURIComponent(header.kid)}`, ).then(response => response.text()); const keyValue = crypto.createPublicKey(keyText); - this.keyCache.set(keyId, keyValue.export({ format: 'pem', type: 'spki' })); + this.keyCache.set( + header.kid, + keyValue.export({ format: 'pem', type: 'spki' }), + ); return keyValue; } } diff --git a/plugins/auth-backend/src/providers/oauth2-proxy/provider.test.ts b/plugins/auth-backend/src/providers/oauth2-proxy/provider.test.ts index e27713acb3..d258c17e61 100644 --- a/plugins/auth-backend/src/providers/oauth2-proxy/provider.test.ts +++ b/plugins/auth-backend/src/providers/oauth2-proxy/provider.test.ts @@ -15,15 +15,13 @@ */ jest.mock('jose', () => ({ - JWT: { - decode: jest.fn(), - }, + decodeJwt: jest.fn(), })); jest.mock('@backstage/catalog-client'); import { AuthenticationError } from '@backstage/errors'; import express from 'express'; -import { JWT } from 'jose'; +import * as jose from 'jose'; import { Logger } from 'winston'; import { AuthHandler, AuthResolverContext, SignInResolver } from '../types'; import { @@ -45,12 +43,14 @@ describe('Oauth2ProxyAuthProvider', () => { let authHandler: jest.MockedFunction>>; let mockResponse: jest.Mocked; let mockRequest: jest.Mocked; - let JWTMock: jest.Mocked; + let mockJwtDecode: jest.MockedFunction; beforeEach(() => { jest.resetAllMocks(); - JWTMock = JWT as jest.Mocked; + mockJwtDecode = jose.decodeJwt as jest.MockedFunction< + typeof jose.decodeJwt + >; authHandler = jest.fn(); signInResolver = jest.fn(); logger = { error: jest.fn() } as unknown as jest.Mocked; @@ -120,7 +120,7 @@ describe('Oauth2ProxyAuthProvider', () => { await provider.refresh(mockRequest, mockResponse); expect(mockRequest.header).toBeCalledWith(OAUTH2_PROXY_JWT_HEADER); - expect(JWTMock.decode).toHaveBeenCalledWith('token'); + expect(mockJwtDecode).toHaveBeenCalledWith('token'); expect(mockResponse.json).toHaveBeenCalled(); }); @@ -136,7 +136,7 @@ describe('Oauth2ProxyAuthProvider', () => { token: mockToken, }); authHandler.mockResolvedValue({ profile: profile }); - JWTMock.decode.mockReturnValue(decodedToken as any); + mockJwtDecode.mockReturnValue(decodedToken as any); await provider.refresh(mockRequest, mockResponse); @@ -191,7 +191,7 @@ describe('Oauth2ProxyAuthProvider', () => { await handler.refresh!(mockRequest, mockResponse); expect(mockRequest.header).toBeCalledWith(OAUTH2_PROXY_JWT_HEADER); - expect(JWTMock.decode).toHaveBeenCalledWith('token'); + expect(mockJwtDecode).toHaveBeenCalledWith('token'); expect(mockResponse.json).toHaveBeenCalled(); }); }); diff --git a/plugins/auth-backend/src/providers/oauth2-proxy/provider.ts b/plugins/auth-backend/src/providers/oauth2-proxy/provider.ts index 374811198f..acef4a74ef 100644 --- a/plugins/auth-backend/src/providers/oauth2-proxy/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2-proxy/provider.ts @@ -24,7 +24,7 @@ import { AuthResponse, AuthResolverContext, } from '../types'; -import { JWT } from 'jose'; +import { decodeJwt } from 'jose'; import { prepareBackstageIdentityResponse } from '../prepareBackstageIdentityResponse'; import { createAuthProviderIntegration } from '../createAuthProviderIntegration'; @@ -142,7 +142,7 @@ export class Oauth2ProxyAuthProvider ); } - const decodedJWT = JWT.decode(jwt) as unknown as JWTPayload; + const decodedJWT = decodeJwt(jwt) as unknown as JWTPayload; return { fullProfile: decodedJWT, diff --git a/plugins/auth-backend/src/providers/oidc/provider.test.ts b/plugins/auth-backend/src/providers/oidc/provider.test.ts index 5312789a28..59a717f132 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.test.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.test.ts @@ -18,7 +18,7 @@ import { Config, ConfigReader } from '@backstage/config'; import { setupRequestMockHandlers } from '@backstage/backend-test-utils'; import express from 'express'; import { Session } from 'express-session'; -import { JWK, JWT } from 'jose'; +import { UnsecuredJWT } from 'jose'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { ClientMetadata, IssuerMetadata } from 'openid-client'; @@ -90,13 +90,18 @@ describe('OidcAuthProvider', () => { }); it('OidcAuthProvider#handler successfully invokes the oidc endpoints', async () => { - const jwt = { - sub: 'alice', - iss: 'https://oidc.test', - iat: Date.now(), - aud: clientMetadata.clientId, - exp: Date.now() + 10000, - }; + const sub = 'alice'; + const iss = 'https://oidc.test'; + const iat = Date.now(); + const aud = clientMetadata.clientId; + const exp = Date.now() + 10000; + const jwt = await new UnsecuredJWT({ iss, sub, aud, iat, exp }) + .setIssuer(iss) + .setAudience(aud) + .setSubject(sub) + .setIssuedAt(iat) + .setExpirationTime(exp) + .encode(); const requestSequence: Array = []; // The array of expected requests executed by the provider handler @@ -114,7 +119,7 @@ describe('OidcAuthProvider', () => { method: 'post', url: 'https://oidc.test/as/token.oauth2', payload: { - id_token: JWT.sign(jwt, JWK.None), + id_token: jwt, access_token: 'test', authorization_signed_response_alg: 'HS256', }, diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index b12af2b7f8..33d59ef4b8 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -26,12 +26,13 @@ "@backstage/backend-common": "^0.13.2", "@backstage/config": "^1.0.0", "@backstage/errors": "^1.0.0", - "jose": "^1.27.1", + "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" }, "devDependencies": { "@backstage/cli": "^0.17.0", + "lodash": "^4.17.21", "msw": "^0.35.0", "uuid": "^8.0.0" }, diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index 88d4021dc3..5efc85cb21 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -15,11 +15,17 @@ */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { JSONWebKey, JWK, JWS, JWT } from 'jose'; +import { + SignJWT, + generateKeyPair, + decodeProtectedHeader, + exportJWK, +} from 'jose'; +import { cloneDeep } from 'lodash'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; -import { v4 as uuid } from 'uuid'; import { IdentityClient } from './IdentityClient'; +import { v4 as uuid } from 'uuid'; interface AnyJWK extends Record { use: 'sig'; @@ -45,12 +51,11 @@ class FakeTokenFactory { ent?: string[]; }; }): Promise { - const key = await JWK.generate('EC', 'P-256', { - use: 'sig', - kid: uuid(), - alg: 'ES256', - }); - this.keys.push(key.toJWK(false) as unknown as AnyJWK); + const pair = await generateKeyPair('ES256'); + const publicKey = await exportJWK(pair.publicKey); + const kid = uuid(); + publicKey.kid = kid; + this.keys.push(publicKey as AnyJWK); const iss = this.options.issuer; const sub = params.claims.sub; @@ -59,10 +64,14 @@ class FakeTokenFactory { const iat = Math.floor(Date.now() / 1000); const exp = iat + this.options.keyDurationSeconds; - return JWS.sign({ iss, sub, aud, iat, exp, ent }, key, { - alg: key.alg, - kid: key.kid, - }); + return new SignJWT({ iss, sub, aud, iat, exp, ent, kid }) + .setProtectedHeader({ alg: 'ES256', ent: ent, kid: kid }) + .setIssuer(iss) + .setAudience(aud) + .setSubject(sub) + .setIssuedAt(iat) + .setExpirationTime(exp) + .sign(pair.privateKey); } async listPublicKeys(): Promise<{ keys: AnyJWK[] }> { @@ -71,10 +80,8 @@ class FakeTokenFactory { } function jwtKid(jwt: string): string { - const { header } = JWT.decode(jwt, { complete: true }) as { - header: { kid: string }; - }; - return header.kid; + const header = decodeProtectedHeader(jwt); + return header.kid ?? ''; } const server = setupServer(); @@ -118,13 +125,6 @@ describe('IdentityClient', () => { ); }); - it('should use the correct endpoint', async () => { - await factory.issueToken({ claims: { sub: 'foo' } }); - const keys = await factory.listPublicKeys(); - const response = await (client as any).listPublicKeys(); - expect(response).toEqual(keys); - }); - it('should throw on undefined header', async () => { return expect(async () => { await client.authenticate(undefined); @@ -144,6 +144,21 @@ describe('IdentityClient', () => { }); }); + it('should decode claims correctly', async () => { + const token = await factory.issueToken({ + claims: { sub: 'foo', ent: ['entity1', 'entity2'] }, + }); + const response = await client.authenticate(token); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo', + ownershipEntityRefs: ['entity1', 'entity2'], + }, + }); + }); + it('should throw on incorrect issuer', async () => { const hackerFactory = new FakeTokenFactory({ issuer: 'hacker', @@ -196,8 +211,13 @@ describe('IdentityClient', () => { } catch (_err) { // Ignore thrown error } - // Move forward in time where the signing key has been rotated - jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); + // Move forward in time where the signing key has been rotated and the + // cooldown period to look up a new public key has elapsed. + jest + .spyOn(Date, 'now') + .mockImplementation( + () => fixedTime + 30 * keyDurationSeconds * 1000 + 2, + ); const token = await factory.issueToken({ claims: { sub: 'foo' } }); const response = await client.authenticate(token); expect(response).toEqual({ @@ -229,36 +249,60 @@ describe('IdentityClient', () => { return await client.authenticate(fakeToken); }).rejects.toThrow(); }); - }); - describe('listPublicKeys', () => { - const defaultServiceResponse: { - keys: JSONWebKey[]; - } = { - keys: [ - { - crv: 'P-256', - x: 'JWy80Goa-8C3oaeDLnk0ANVPPMfI9T3u_T5T7W2b_ls', - y: 'Ge6jAhCDW1PFBfme2RA5ZsXN0cESiCwW29LMRPX5wkw', - kty: 'EC', - kid: 'kid-a', - alg: 'ES256', - use: 'sig', - }, - ], - }; - - beforeEach(() => { + it('should use an updated endpoint when the key is not found', async () => { + const updatedURL = 'http://backstage:9191/an-updated-base'; + const getBaseUrl = discovery.getBaseUrl; + const getExternalBaseUrl = discovery.getExternalBaseUrl; + // Generate a key and sign a token with it + await factory.issueToken({ claims: { sub: 'foo' } }); + // Only return the key from a single token + const singleKey = cloneDeep(await factory.listPublicKeys()); server.use( - rest.get(`${mockBaseUrl}/.well-known/jwks.json`, (_, res, ctx) => { - return res(ctx.json(defaultServiceResponse)); + rest.get( + `${mockBaseUrl}/.well-known/jwks.json`, + async (_, res, ctx) => { + return res(ctx.json(singleKey)); + }, + ), + ); + // Update the discovery endpoint to point to a new URL + discovery.getBaseUrl = async () => { + return updatedURL; + }; + discovery.getExternalBaseUrl = async () => { + return updatedURL; + }; + let calledUpdatedEndpoint = false; + server.use( + rest.get(`${updatedURL}/.well-known/jwks.json`, async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + calledUpdatedEndpoint = true; + return res(ctx.json(keys)); }), ); - }); - - it('should use the correct endpoint', async () => { - const response = await (client as any).listPublicKeys(); - expect(response).toEqual(defaultServiceResponse); + // Advance time + const future_11s = Date.now() + 11 * 1000; + const dateSpy = jest + .spyOn(Date, 'now') + .mockImplementation(() => future_11s); + // Issue a new token + const token = await factory.issueToken({ claims: { sub: 'foo2' } }); + const response = await client.authenticate(token); + // Verify that the endpoint was updated. + expect(calledUpdatedEndpoint).toBeTruthy(); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo2', + ownershipEntityRefs: [], + }, + }); + // Restore the discovery endpoint and time + discovery.getBaseUrl = getBaseUrl; + discovery.getExternalBaseUrl = getExternalBaseUrl; + dateSpy.mockClear(); }); }); }); diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index 54c2670ae8..605c434431 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -16,8 +16,15 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { AuthenticationError } from '@backstage/errors'; -import { JSONWebKey, JWK, JWKS, JWT } from 'jose'; -import fetch from 'node-fetch'; +import { + createRemoteJWKSet, + decodeJwt, + jwtVerify, + FlattenedJWSInput, + JWSHeaderParameters, + decodeProtectedHeader, +} from 'jose'; +import { GetKeyFunction } from 'jose/dist/types/types'; import { BackstageIdentityResponse } from './types'; const CLOCK_MARGIN_S = 10; @@ -32,8 +39,8 @@ const CLOCK_MARGIN_S = 10; export class IdentityClient { private readonly discovery: PluginEndpointDiscovery; private readonly issuer: string; - private keyStore: JWKS.KeyStore; - private keyStoreUpdated: number; + private keyStore?: GetKeyFunction; + private keyStoreUpdated: number = 0; /** * Create a new {@link IdentityClient} instance. @@ -51,8 +58,6 @@ export class IdentityClient { }) { this.discovery = options.discovery; this.issuer = options.issuer; - this.keyStore = new JWKS.KeyStore(); - this.keyStoreUpdated = 0; } /** @@ -67,22 +72,23 @@ export class IdentityClient { if (!token) { throw new AuthenticationError('No token specified'); } - // Get signing key matching token - const key = await this.getKey(token); - if (!key) { - throw new AuthenticationError('No signing key matching token found'); - } + // Verify token claims and signature // Note: Claims must match those set by TokenFactory when issuing tokens // Note: verify throws if verification fails - const decoded = JWT.IdToken.verify(token, key, { + // Check if the keystore needs to be updated + await this.refreshKeyStore(token); + if (!this.keyStore) { + throw new AuthenticationError('No keystore exists'); + } + const decoded = await jwtVerify(token, this.keyStore, { algorithms: ['ES256'], audience: 'backstage', issuer: this.issuer, - }) as { sub: string; ent: string[] }; + }); // Verified, return the matching user as BackstageIdentity // TODO: Settle internal user format/properties - if (!decoded.sub) { + if (!decoded.payload.sub) { throw new AuthenticationError('No user sub found in token'); } @@ -90,68 +96,45 @@ export class IdentityClient { token, identity: { type: 'user', - userEntityRef: decoded.sub, - ownershipEntityRefs: decoded.ent ?? [], + userEntityRef: decoded.payload.sub, + ownershipEntityRefs: decoded.payload.ent + ? (decoded.payload.ent as string[]) + : [], }, }; return user; } /** - * Returns the public signing key matching the given jwt token, - * or null if no matching key was found + * If the last keystore refresh is stale, update the keystore URL to the latest */ - private async getKey(rawJwtToken: string): Promise { - const { header, payload } = JWT.decode(rawJwtToken, { - complete: true, - }) as { - header: { kid: string }; - payload: { iat: number }; - }; + private async refreshKeyStore(rawJwtToken: string): Promise { + const payload = await decodeJwt(rawJwtToken); + const header = await decodeProtectedHeader(rawJwtToken); // Refresh public keys if needed + let keyStoreHasKey; + try { + if (this.keyStore) { + // Check if the key is present in the keystore + const [_, rawPayload, rawSignature] = rawJwtToken.split('.'); + keyStoreHasKey = await this.keyStore(header, { + payload: rawPayload, + signature: rawSignature, + }); + } + } catch (error) { + keyStoreHasKey = false; + } + // Refresh public key URL if needed // Add a small margin in case clocks are out of sync - const keyStoreHasKey = !!this.keyStore.get({ kid: header.kid }); const issuedAfterLastRefresh = payload?.iat && payload.iat > this.keyStoreUpdated - CLOCK_MARGIN_S; if (!keyStoreHasKey && issuedAfterLastRefresh) { - await this.refreshKeyStore(); + const url = await this.discovery.getBaseUrl('auth'); + const endpoint = new URL(`${url}/.well-known/jwks.json`); + this.keyStore = createRemoteJWKSet(endpoint); + this.keyStoreUpdated = Date.now() / 1000; } - - return this.keyStore.get({ kid: header.kid }); - } - - /** - * Lists public part of keys used to sign Backstage Identity tokens - */ - private async listPublicKeys(): Promise<{ - keys: JSONWebKey[]; - }> { - const url = `${await this.discovery.getBaseUrl( - 'auth', - )}/.well-known/jwks.json`; - const response = await fetch(url); - - if (!response.ok) { - const payload = await response.text(); - const message = `Request failed with ${response.status} ${response.statusText}, ${payload}`; - throw new Error(message); - } - - const publicKeys: { keys: JSONWebKey[] } = await response.json(); - - return publicKeys; - } - - /** - * Fetches public keys and caches them locally - */ - private async refreshKeyStore(): Promise { - const now = Date.now() / 1000; - const publicKeys = await this.listPublicKeys(); - this.keyStore = JWKS.asKeyStore({ - keys: publicKeys.keys.map(key => key as JSONWebKey), - }); - this.keyStoreUpdated = now; } } diff --git a/yarn.lock b/yarn.lock index 9a9b53a964..664459739d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15586,13 +15586,6 @@ joi@^17.4.0: "@sideway/formula" "^3.0.0" "@sideway/pinpoint" "^2.0.0" -jose@^1.27.1: - version "1.28.1" - resolved "https://registry.npmjs.org/jose/-/jose-1.28.1.tgz#34a0f851a534be59ffab82a6e8845f6874e8c128" - integrity sha512-6JK28rFu5ENp/yxMwM+iN7YeaInnY9B9Bggjkz5fuwLiJhbVrl2O4SJr65bdNBPl9y27fdC3Mymh+FVCvozLIg== - dependencies: - "@panva/asn1.js" "^1.0.0" - jose@^2.0.5: version "2.0.5" resolved "https://registry.npmjs.org/jose/-/jose-2.0.5.tgz#29746a18d9fff7dcf9d5d2a6f62cb0c7cd27abd3" @@ -15605,6 +15598,11 @@ jose@^4.1.4: resolved "https://registry.npmjs.org/jose/-/jose-4.5.0.tgz#92829d8cf846351eb55aaaf94f252fb1d191f2d5" integrity sha512-GFcVFQwYQKbQTUOo2JlpFGXTkgBw26uzDsRMD2q1WgSKNSnpKS9Ug7bdQ8dS+p4sZHNH6iRPu6WK2jLIjspaMA== +jose@^4.6.0: + version "4.6.0" + resolved "https://registry.npmjs.org/jose/-/jose-4.6.0.tgz#f3ff007ddcbce462c091d3d41b7af2e35dec348c" + integrity sha512-0hNAkhMBNi4soKSAX4zYOFV+aqJlEz/4j4fregvasJzEVtjDChvWqRjPvHwLqr5hx28Ayr6bsOs1Kuj87V0O8w== + joycon@^3.0.1: version "3.1.0" resolved "https://registry.npmjs.org/joycon/-/joycon-3.1.0.tgz#33bb2b6b5a6849a1e251bed623bdf610f477d49f"