From 1624b52168f32978a5d87ad5c0f931fc1e70c6fa Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 14 Mar 2022 09:56:34 -0700 Subject: [PATCH 01/17] Update jose to 4.6.0 Signed-off-by: Andy Caruso --- packages/backend-common/package.json | 5 +- .../src/tokens/ServerTokenManager.test.ts | 28 ++-- .../src/tokens/ServerTokenManager.ts | 72 +++++++--- plugins/auth-backend/package.json | 5 +- .../src/identity/TokenFactory.test.ts | 23 ++-- .../auth-backend/src/identity/TokenFactory.ts | 35 ++--- .../src/providers/aws-alb/provider.test.ts | 16 +-- .../src/providers/aws-alb/provider.ts | 23 ++-- .../providers/oauth2-proxy/provider.test.ts | 18 +-- .../src/providers/oauth2-proxy/provider.ts | 4 +- .../src/providers/oidc/provider.test.ts | 23 ++-- plugins/auth-node/package.json | 6 +- plugins/auth-node/src/IdentityClient.test.ts | 79 ++++------- plugins/auth-node/src/IdentityClient.ts | 126 ++++++------------ plugins/permission-node/package.json | 3 + .../src/service/standaloneServer.ts | 2 +- yarn.lock | 47 ++++++- 17 files changed, 267 insertions(+), 248 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 9e621ba92d..86cdc04359 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", @@ -115,6 +115,9 @@ "recursive-readdir": "^2.2.2", "supertest": "^6.1.3" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist", "config.d.ts", diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 1799291544..7512e87661 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({}); @@ -134,7 +134,7 @@ describe('ServerTokenManager', () => { it('should throw for server tokens created using a noop TokenManager', async () => { const noopTokenManager = ServerTokenManager.noop(); - const tokenManager = ServerTokenManager.fromConfig( + const tokenManager = await ServerTokenManager.fromConfig( new ConfigReader({ backend: { auth: { keys: [{ secret: 'a1b2c3' }] } }, }), @@ -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..959c771c72 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -14,7 +14,15 @@ * limitations under the License. */ -import { JWKS, JWK, JWT } from 'jose'; +import { + base64url, + JWSHeaderParameters, + generateSecret, + SignJWT, + jwtVerify, + exportJWK, + decodeProtectedHeader, +} from 'jose'; import { Config } from '@backstage/config'; import { AuthenticationError } from '@backstage/errors'; import { TokenManager } from './types'; @@ -37,8 +45,8 @@ 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; static noop(): TokenManager { return new NoopTokenManager(); @@ -56,44 +64,64 @@ export class ServerTokenManager implements TokenManager { '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([]); } private constructor(secrets: string[]) { - if (!secrets.length) { + if (!secrets.length && process.env.NODE_ENV !== 'development') { throw new Error( 'No secrets provided when constructing ServerTokenManager', ); } + this.verificationKeys = new Array(); + secrets.map(k => this.verificationKeys.push(base64url.decode(k))); + 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', + ); + } + const secret = await generateSecret('HS256'); + const jwk = await exportJWK(secret); + this.verificationKeys.push(base64url.decode(jwk.k ?? '')); + this.signingKey = this.verificationKeys[0]; } 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) { + const lookup = (_protectedHeader: JWSHeaderParameters): Uint8Array => { + return key; + }; + try { + await jwtVerify(token, lookup); + // 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 9ba0468f94..e667a96145 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", @@ -91,6 +91,9 @@ "msw": "^0.35.0", "supertest": "^6.1.3" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist", "migrations", diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index 4b1c1804df..c0bfb14dca 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -17,16 +17,14 @@ 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 }; - }; - return header.kid; + const header = decodeProtectedHeader(jwt); + return header.kid ?? ''; } const entityRef = stringifyEntityRef({ @@ -49,22 +47,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..3d49b30b6c 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -15,7 +15,7 @@ */ 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'; @@ -52,9 +52,10 @@ export class TokenFactory implements TokenIssuer { private readonly logger: Logger; private readonly keyStore: KeyStore; private readonly keyDurationSeconds: number; + private readonly alg = 'ES256'; private keyExpiry?: Date; - private privateKeyPromise?: Promise; + private privateKeyPromise?: Promise; constructor(options: Options) { this.issuer = options.issuer; @@ -84,10 +85,14 @@ 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, - }); + return new SignJWT({ iss, sub, aud, iat, exp }) + .setProtectedHeader({ alg: this.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 +132,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 +152,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(this.alg); + const publicKey = await exportJWK(key.publicKey); + const privateKey = await exportJWK(key.privateKey); + publicKey.kid = privateKey.kid = uuid(); + publicKey.alg = privateKey.alg = this.alg; // 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 +164,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..6eee5ecd89 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'; @@ -144,9 +144,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 +194,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 dac6c3a584..68870779fe 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -25,8 +25,9 @@ "dependencies": { "@backstage/backend-common": "^0.13.2-next.1", "@backstage/config": "^1.0.0", + "@backstage/catalog-model": "^0.13.0", "@backstage/errors": "^1.0.0", - "jose": "^1.27.1", + "jose": "^4.6.0", "node-fetch": "^2.6.7", "winston": "^3.2.1" }, @@ -35,6 +36,9 @@ "msw": "^0.35.0", "uuid": "^8.0.0" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist" ] diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index 88d4021dc3..cbc030dd1f 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -15,11 +15,16 @@ */ import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { JSONWebKey, JWK, JWS, JWT } from 'jose'; +import { + SignJWT, + generateKeyPair, + decodeProtectedHeader, + exportJWK, +} from 'jose'; 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 +50,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 +63,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 +79,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(); @@ -98,7 +104,7 @@ describe('IdentityClient', () => { afterEach(() => server.resetHandlers()); beforeEach(() => { - client = IdentityClient.create({ discovery, issuer: mockBaseUrl }); + client = IdentityClient.create({ discovery, issuer: mockBaseUrl }, 0); factory = new FakeTokenFactory({ issuer: mockBaseUrl, keyDurationSeconds, @@ -118,13 +124,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); @@ -186,7 +185,7 @@ describe('IdentityClient', () => { it('should accept token from new key', async () => { const fixedTime = Date.now(); - jest + const spy = jest .spyOn(Date, 'now') .mockImplementation(() => fixedTime - keyDurationSeconds * 1000 * 2); const token1 = await factory.issueToken({ claims: { sub: 'foo1' } }); @@ -197,7 +196,7 @@ describe('IdentityClient', () => { // Ignore thrown error } // Move forward in time where the signing key has been rotated - jest.spyOn(Date, 'now').mockImplementation(() => fixedTime); + spy.mockRestore(); const token = await factory.issueToken({ claims: { sub: 'foo' } }); const response = await client.authenticate(token); expect(response).toEqual({ @@ -232,33 +231,9 @@ describe('IdentityClient', () => { }); 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(() => { - server.use( - rest.get(`${mockBaseUrl}/.well-known/jwks.json`, (_, res, ctx) => { - return res(ctx.json(defaultServiceResponse)); - }), - ); - }); - it('should use the correct endpoint', async () => { - const response = await (client as any).listPublicKeys(); - expect(response).toEqual(defaultServiceResponse); + const url = (client as any).endpoint as URL; + expect(url.toString()).toMatch(`${mockBaseUrl}/.well-known/jwks.json`); }); }); }); diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index 54c2670ae8..560b3b6373 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -16,12 +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, + jwtVerify, + FlattenedJWSInput, + JWSHeaderParameters, +} from 'jose'; +import { GetKeyFunction } from 'jose/dist/types/types'; import { BackstageIdentityResponse } from './types'; -const CLOCK_MARGIN_S = 10; - /** * An identity client to interact with auth-backend and authenticate Backstage * tokens @@ -32,27 +35,37 @@ 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 endpoint?: URL; /** * Create a new {@link IdentityClient} instance. */ - static create(options: { - discovery: PluginEndpointDiscovery; - issuer: string; - }): IdentityClient { - return new IdentityClient(options); + static create( + options: { + discovery: PluginEndpointDiscovery; + issuer: string; + }, + cooldownMs = 30000, + ): IdentityClient { + return new IdentityClient(options, cooldownMs); } - private constructor(options: { - discovery: PluginEndpointDiscovery; - issuer: string; - }) { + private constructor( + options: { + discovery: PluginEndpointDiscovery; + issuer: string; + }, + cooldownMs: number, + ) { this.discovery = options.discovery; this.issuer = options.issuer; - this.keyStore = new JWKS.KeyStore(); - this.keyStoreUpdated = 0; + this.discovery.getBaseUrl('auth').then(url => { + this.endpoint = new URL(`${url}/.well-known/jwks.json`); + this.keyStore = createRemoteJWKSet(this.endpoint, { + cooldownDuration: cooldownMs, + }); + }); } /** @@ -67,91 +80,34 @@ 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, { + 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'); } const user: BackstageIdentityResponse = { + id: decoded.payload.sub, 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 - */ - private async getKey(rawJwtToken: string): Promise { - const { header, payload } = JWT.decode(rawJwtToken, { - complete: true, - }) as { - header: { kid: string }; - payload: { iat: number }; - }; - - // Refresh public keys 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(); - } - - 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/plugins/permission-node/package.json b/plugins/permission-node/package.json index 46c1640759..4b4d16af45 100644 --- a/plugins/permission-node/package.json +++ b/plugins/permission-node/package.json @@ -49,6 +49,9 @@ "msw": "^0.35.0", "supertest": "^6.1.3" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist" ] diff --git a/plugins/search-backend/src/service/standaloneServer.ts b/plugins/search-backend/src/service/standaloneServer.ts index 3f1fdcb34e..22b38ce94b 100644 --- a/plugins/search-backend/src/service/standaloneServer.ts +++ b/plugins/search-backend/src/service/standaloneServer.ts @@ -43,7 +43,7 @@ export async function startStandaloneServer( const searchEngine = new LunrSearchEngine({ logger }); const indexBuilder = new IndexBuilder({ logger, searchEngine }); const discovery = SingleHostDiscovery.fromConfig(config); - const tokenManager = ServerTokenManager.fromConfig(config, { + const tokenManager = await ServerTokenManager.fromConfig(config, { logger, }); const permissions = ServerPermissionClient.fromConfig(config, { diff --git a/yarn.lock b/yarn.lock index 35d1382b57..8d4e5aed4f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1465,6 +1465,19 @@ "@backstage/errors" "^1.0.0" cross-fetch "^3.1.5" +"@backstage/catalog-model@^0.13.0": + version "0.13.0" + resolved "https://registry.npmjs.org/@backstage/catalog-model/-/catalog-model-0.13.0.tgz#abeb91522ac7ef7907907ad5bc889803131db209" + integrity sha512-Q+0C0LJ8niNQ4TXduUNTycZ2PLMbhKmfgWywunJLO34DZAoFnTWJEAdEYF536YyLciHhLO0dHRUaHY29Sdr+Xg== + dependencies: + "@backstage/config" "^0.1.15" + "@backstage/errors" "^0.2.2" + "@backstage/types" "^0.1.3" + ajv "^7.0.3" + json-schema "^0.4.0" + lodash "^4.17.21" + uuid "^8.0.0" + "@backstage/catalog-model@^1.0.0": version "1.0.0" resolved "https://registry.npmjs.org/@backstage/catalog-model/-/catalog-model-1.0.0.tgz#0aa8694a3182aaf4232725842da751bf5f78bd68" @@ -1478,6 +1491,14 @@ lodash "^4.17.21" uuid "^8.0.0" +"@backstage/config@^0.1.15": + version "0.1.15" + resolved "https://registry.npmjs.org/@backstage/config/-/config-0.1.15.tgz#4bad122ad861be5bd61a60639f92d2494fa245c5" + integrity sha512-eNJEYYSEu9MkrkBYiMpUBWEc3Bu64YgB9pZZGCMW7/9350tV2wbylEdoBJHslilJlJhiUyTXBckn8Ua7DOH7rw== + dependencies: + "@backstage/types" "^0.1.3" + lodash "^4.17.21" + "@backstage/core-components@^0.9.0", "@backstage/core-components@^0.9.2": version "0.9.2" resolved "https://registry.npmjs.org/@backstage/core-components/-/core-components-0.9.2.tgz#9a3d79a15039256bbc007e5daa08c983050e0238" @@ -1535,6 +1556,15 @@ react-router-dom "6.0.0-beta.0" zen-observable "^0.8.15" +"@backstage/errors@^0.2.2": + version "0.2.2" + resolved "https://registry.npmjs.org/@backstage/errors/-/errors-0.2.2.tgz#2113e0bc859e645b8b59bfcb435f7535739b02f8" + integrity sha512-s6Ru3NL4oFiHwNsMY1uiwqNKuOu9KQWkpse3adWZsDu1fCQKOw2omQ2qBHZ4Uj1xJ6x9Ye0yNTJ4vfHfdwiaEg== + dependencies: + "@backstage/types" "^0.1.3" + cross-fetch "^3.1.5" + serialize-error "^8.0.1" + "@backstage/integration-react@^1.0.0": version "1.0.0" resolved "https://registry.npmjs.org/@backstage/integration-react/-/integration-react-1.0.0.tgz#8075e65c6b5387631d27a9c242e7a4fff6f92417" @@ -1615,6 +1645,11 @@ react-use "^17.2.4" swr "^1.1.2" +"@backstage/types@^0.1.3": + version "0.1.3" + resolved "https://registry.npmjs.org/@backstage/types/-/types-0.1.3.tgz#6613d8cbdf97d42d31cd1e66a833df533e7ccf14" + integrity sha512-fJVi4oVrlO+G3PRv1fYSll9/X4pE11HLnkI//Geare9sP6wSfp/2zXpLYfKVsG0e24jOl7Swkc8lwLkQ90zMaQ== + "@balena/dockerignore@^1.0.2": version "1.0.2" resolved "https://registry.npmjs.org/@balena/dockerignore/-/dockerignore-1.0.2.tgz#9ffe4726915251e8eb69f44ef3547e0da2c03e0d" @@ -15721,13 +15756,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" @@ -15740,6 +15768,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.1" + resolved "https://registry.npmjs.org/jose/-/jose-4.6.1.tgz#241472ff928c79b2e2e2fe4c1056b3085384ec42" + integrity sha512-EFnufEivlIB6j7+JwaenYQzdUDs/McajDr9WnhT6EI0WxbexnfuZimpWX1GnobF6OnQsUFmWFXUXdWyZHWdQow== + joycon@^3.0.1: version "3.1.0" resolved "https://registry.npmjs.org/joycon/-/joycon-3.1.0.tgz#33bb2b6b5a6849a1e251bed623bdf610f477d49f" From 9ec4e0613e447fe59b891d9887d8b7dfffee7a89 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 14 Mar 2022 10:26:57 -0700 Subject: [PATCH 02/17] Add changeset Signed-off-by: Andy Caruso --- .changeset/tasty-drinks-teach.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/tasty-drinks-teach.md diff --git a/.changeset/tasty-drinks-teach.md b/.changeset/tasty-drinks-teach.md new file mode 100644 index 0000000000..96dc2bf3b5 --- /dev/null +++ b/.changeset/tasty-drinks-teach.md @@ -0,0 +1,8 @@ +--- +'@backstage/backend-common': patch +'@backstage/plugin-auth-backend': patch +'@backstage/plugin-auth-node': patch +'@backstage/plugin-permission-node': patch +--- + +Update to jose 4.6.0 From a1167e41b189c48b0cdcbd874425840ed40191a4 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 14 Mar 2022 11:17:17 -0700 Subject: [PATCH 03/17] Update a few test environments, remove unused import. Signed-off-by: Andy Caruso --- .changeset/tasty-drinks-teach.md | 2 +- packages/backend-common/src/tokens/ServerTokenManager.ts | 1 - packages/backend-test-utils/package.json | 3 +++ plugins/airbrake-backend/package.json | 3 +++ 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.changeset/tasty-drinks-teach.md b/.changeset/tasty-drinks-teach.md index 96dc2bf3b5..8c1faaf594 100644 --- a/.changeset/tasty-drinks-teach.md +++ b/.changeset/tasty-drinks-teach.md @@ -5,4 +5,4 @@ '@backstage/plugin-permission-node': patch --- -Update to jose 4.6.0 +Update to `jose` 4.6.0 diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index 959c771c72..83181cca4b 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -21,7 +21,6 @@ import { SignJWT, jwtVerify, exportJWK, - decodeProtectedHeader, } from 'jose'; import { Config } from '@backstage/config'; import { AuthenticationError } from '@backstage/errors'; diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index cb487f3e3f..7e11b13b1c 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -48,6 +48,9 @@ "devDependencies": { "@backstage/cli": "^0.17.0-next.1" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist" ] diff --git a/plugins/airbrake-backend/package.json b/plugins/airbrake-backend/package.json index 021db97be8..d2a519a828 100644 --- a/plugins/airbrake-backend/package.json +++ b/plugins/airbrake-backend/package.json @@ -39,6 +39,9 @@ "supertest": "^6.1.6", "msw": "^0.35.0" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist", "config.d.ts" From a1d373170bc376d16d13f6a16d866a2d41fb44d9 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 14 Mar 2022 12:05:18 -0700 Subject: [PATCH 04/17] Add API report docs Signed-off-by: Andy Caruso --- plugins/auth-node/api-report.md | 11 +++++++---- plugins/auth-node/src/IdentityClient.ts | 1 - 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index f7fdd6369b..c577ccb353 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -30,9 +30,12 @@ export function getBearerTokenFromAuthorizationHeader( // @public export class IdentityClient { authenticate(token: string | undefined): Promise; - static create(options: { - discovery: PluginEndpointDiscovery; - issuer: string; - }): IdentityClient; + static create( + options: { + discovery: PluginEndpointDiscovery; + issuer: string; + }, + cooldownMs?: number, + ): IdentityClient; } ``` diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index 560b3b6373..a98330fc71 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -98,7 +98,6 @@ export class IdentityClient { } const user: BackstageIdentityResponse = { - id: decoded.payload.sub, token, identity: { type: 'user', From 6fe50b8f4d7569b0b0415222c3a0fcc7b49f744e Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 14 Mar 2022 13:28:38 -0700 Subject: [PATCH 05/17] Set nodeEnvironment for app-backend plugin Signed-off-by: Andy Caruso --- plugins/app-backend/package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/app-backend/package.json b/plugins/app-backend/package.json index d787b7ffb1..f8d7c312b5 100644 --- a/plugins/app-backend/package.json +++ b/plugins/app-backend/package.json @@ -58,6 +58,9 @@ "msw": "^0.35.0", "supertest": "^6.1.3" }, + "jest": { + "testEnvironment": "node" + }, "files": [ "dist", "migrations/**/*.{js,d.ts}", From ef8df1948c9ffb5b69ec5113f3ff6c6def44d5f9 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 21 Mar 2022 09:20:36 -0700 Subject: [PATCH 06/17] Remove cooldown parameter option Signed-off-by: Andy Caruso --- plugins/auth-node/src/IdentityClient.test.ts | 11 +++++--- plugins/auth-node/src/IdentityClient.ts | 28 +++++++------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index cbc030dd1f..3386d4e414 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -185,7 +185,7 @@ describe('IdentityClient', () => { it('should accept token from new key', async () => { const fixedTime = Date.now(); - const spy = jest + jest .spyOn(Date, 'now') .mockImplementation(() => fixedTime - keyDurationSeconds * 1000 * 2); const token1 = await factory.issueToken({ claims: { sub: 'foo1' } }); @@ -195,8 +195,13 @@ describe('IdentityClient', () => { } catch (_err) { // Ignore thrown error } - // Move forward in time where the signing key has been rotated - spy.mockRestore(); + // 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({ diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index a98330fc71..2fdb876dd6 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -41,30 +41,22 @@ export class IdentityClient { /** * Create a new {@link IdentityClient} instance. */ - static create( - options: { - discovery: PluginEndpointDiscovery; - issuer: string; - }, - cooldownMs = 30000, - ): IdentityClient { - return new IdentityClient(options, cooldownMs); + static create(options: { + discovery: PluginEndpointDiscovery; + issuer: string; + }): IdentityClient { + return new IdentityClient(options); } - private constructor( - options: { - discovery: PluginEndpointDiscovery; - issuer: string; - }, - cooldownMs: number, - ) { + private constructor(options: { + discovery: PluginEndpointDiscovery; + issuer: string; + }) { this.discovery = options.discovery; this.issuer = options.issuer; this.discovery.getBaseUrl('auth').then(url => { this.endpoint = new URL(`${url}/.well-known/jwks.json`); - this.keyStore = createRemoteJWKSet(this.endpoint, { - cooldownDuration: cooldownMs, - }); + this.keyStore = createRemoteJWKSet(this.endpoint); }); } From 504f5939edac697661d636c5be073841670ca1b3 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Tue, 22 Mar 2022 08:20:07 -0700 Subject: [PATCH 07/17] Correct test constructor Signed-off-by: Andy Caruso --- plugins/auth-node/src/IdentityClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index 3386d4e414..9cdd9a81a6 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -104,7 +104,7 @@ describe('IdentityClient', () => { afterEach(() => server.resetHandlers()); beforeEach(() => { - client = IdentityClient.create({ discovery, issuer: mockBaseUrl }, 0); + client = IdentityClient.create({ discovery, issuer: mockBaseUrl }); factory = new FakeTokenFactory({ issuer: mockBaseUrl, keyDurationSeconds, From df7ea8e22fc88cfc7005a7e9cfe5e8f2ded63fe5 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Tue, 22 Mar 2022 09:15:46 -0700 Subject: [PATCH 08/17] Update API docs Signed-off-by: Andy Caruso --- plugins/auth-node/api-report.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/plugins/auth-node/api-report.md b/plugins/auth-node/api-report.md index c577ccb353..f7fdd6369b 100644 --- a/plugins/auth-node/api-report.md +++ b/plugins/auth-node/api-report.md @@ -30,12 +30,9 @@ export function getBearerTokenFromAuthorizationHeader( // @public export class IdentityClient { authenticate(token: string | undefined): Promise; - static create( - options: { - discovery: PluginEndpointDiscovery; - issuer: string; - }, - cooldownMs?: number, - ): IdentityClient; + static create(options: { + discovery: PluginEndpointDiscovery; + issuer: string; + }): IdentityClient; } ``` From 4dc6aafca7ff745f40add8cee589a9c3cd9db5a3 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Fri, 25 Mar 2022 09:05:37 -0700 Subject: [PATCH 09/17] Remove node jest changes and unneeded awaits Signed-off-by: Andy Caruso --- packages/backend-common/package.json | 3 --- packages/backend-test-utils/package.json | 3 --- plugins/airbrake-backend/package.json | 3 --- plugins/app-backend/package.json | 3 --- plugins/auth-backend/package.json | 3 --- plugins/auth-node/package.json | 3 --- plugins/permission-node/package.json | 3 --- plugins/search-backend/src/service/standaloneServer.ts | 2 +- 8 files changed, 1 insertion(+), 22 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 86cdc04359..a8eafe9cf2 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -115,9 +115,6 @@ "recursive-readdir": "^2.2.2", "supertest": "^6.1.3" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist", "config.d.ts", diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index 7e11b13b1c..cb487f3e3f 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -48,9 +48,6 @@ "devDependencies": { "@backstage/cli": "^0.17.0-next.1" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist" ] diff --git a/plugins/airbrake-backend/package.json b/plugins/airbrake-backend/package.json index d2a519a828..021db97be8 100644 --- a/plugins/airbrake-backend/package.json +++ b/plugins/airbrake-backend/package.json @@ -39,9 +39,6 @@ "supertest": "^6.1.6", "msw": "^0.35.0" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist", "config.d.ts" diff --git a/plugins/app-backend/package.json b/plugins/app-backend/package.json index f8d7c312b5..d787b7ffb1 100644 --- a/plugins/app-backend/package.json +++ b/plugins/app-backend/package.json @@ -58,9 +58,6 @@ "msw": "^0.35.0", "supertest": "^6.1.3" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist", "migrations/**/*.{js,d.ts}", diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index e667a96145..82c03b99fa 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -91,9 +91,6 @@ "msw": "^0.35.0", "supertest": "^6.1.3" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist", "migrations", diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 68870779fe..9a99506702 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -36,9 +36,6 @@ "msw": "^0.35.0", "uuid": "^8.0.0" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist" ] diff --git a/plugins/permission-node/package.json b/plugins/permission-node/package.json index 4b4d16af45..46c1640759 100644 --- a/plugins/permission-node/package.json +++ b/plugins/permission-node/package.json @@ -49,9 +49,6 @@ "msw": "^0.35.0", "supertest": "^6.1.3" }, - "jest": { - "testEnvironment": "node" - }, "files": [ "dist" ] diff --git a/plugins/search-backend/src/service/standaloneServer.ts b/plugins/search-backend/src/service/standaloneServer.ts index 22b38ce94b..3f1fdcb34e 100644 --- a/plugins/search-backend/src/service/standaloneServer.ts +++ b/plugins/search-backend/src/service/standaloneServer.ts @@ -43,7 +43,7 @@ export async function startStandaloneServer( const searchEngine = new LunrSearchEngine({ logger }); const indexBuilder = new IndexBuilder({ logger, searchEngine }); const discovery = SingleHostDiscovery.fromConfig(config); - const tokenManager = await ServerTokenManager.fromConfig(config, { + const tokenManager = ServerTokenManager.fromConfig(config, { logger, }); const permissions = ServerPermissionClient.fromConfig(config, { From ab93c04c67e21455edbf80bc75448c54f6f2d5a7 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Fri, 25 Mar 2022 09:08:37 -0700 Subject: [PATCH 10/17] Correct changeset and remove unused await Signed-off-by: Andy Caruso --- .changeset/tasty-drinks-teach.md | 1 - packages/backend-common/src/tokens/ServerTokenManager.test.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/tasty-drinks-teach.md b/.changeset/tasty-drinks-teach.md index 8c1faaf594..4b4a5a748d 100644 --- a/.changeset/tasty-drinks-teach.md +++ b/.changeset/tasty-drinks-teach.md @@ -2,7 +2,6 @@ '@backstage/backend-common': patch '@backstage/plugin-auth-backend': patch '@backstage/plugin-auth-node': patch -'@backstage/plugin-permission-node': patch --- Update to `jose` 4.6.0 diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 7512e87661..0ff7bfb316 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.test.ts @@ -134,7 +134,7 @@ describe('ServerTokenManager', () => { it('should throw for server tokens created using a noop TokenManager', async () => { const noopTokenManager = ServerTokenManager.noop(); - const tokenManager = await ServerTokenManager.fromConfig( + const tokenManager = ServerTokenManager.fromConfig( new ConfigReader({ backend: { auth: { keys: [{ secret: 'a1b2c3' }] } }, }), From cbe119efa69533ab9385e4bac43cd9ddc53e37c1 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Tue, 29 Mar 2022 09:12:22 -0700 Subject: [PATCH 11/17] Correct handling of claim entities Signed-off-by: Andy Caruso --- plugins/auth-node/src/IdentityClient.test.ts | 15 +++++++++++++++ plugins/auth-node/src/IdentityClient.ts | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index 9cdd9a81a6..b452c3f81d 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -143,6 +143,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', diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index 2fdb876dd6..c0eba6a069 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -95,7 +95,7 @@ export class IdentityClient { type: 'user', userEntityRef: decoded.payload.sub, ownershipEntityRefs: decoded.payload.ent - ? [decoded.payload.ent as string] + ? (decoded.payload.ent as string[]) : [], }, }; From d253d46ba996a2ff1c41735ca456ce7a33018533 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Tue, 29 Mar 2022 10:08:10 -0700 Subject: [PATCH 12/17] Clean up as per review comments Signed-off-by: Andy Caruso --- .../src/tokens/ServerTokenManager.ts | 17 ++-------- .../src/identity/TokenFactory.test.ts | 5 ++- .../src/providers/aws-alb/provider.ts | 5 --- plugins/auth-node/src/IdentityClient.test.ts | 32 +++++++++++++++++++ plugins/auth-node/src/IdentityClient.ts | 8 +++++ 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index 83181cca4b..b0512aed3a 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -14,14 +14,7 @@ * limitations under the License. */ -import { - base64url, - JWSHeaderParameters, - generateSecret, - SignJWT, - jwtVerify, - exportJWK, -} from 'jose'; +import { base64url, generateSecret, SignJWT, jwtVerify, exportJWK } from 'jose'; import { Config } from '@backstage/config'; import { AuthenticationError } from '@backstage/errors'; import { TokenManager } from './types'; @@ -76,8 +69,7 @@ export class ServerTokenManager implements TokenManager { 'No secrets provided when constructing ServerTokenManager', ); } - this.verificationKeys = new Array(); - secrets.map(k => this.verificationKeys.push(base64url.decode(k))); + this.verificationKeys = secrets.map(s => base64url.decode(s)); this.signingKey = this.verificationKeys[0]; } @@ -109,11 +101,8 @@ export class ServerTokenManager implements TokenManager { async authenticate(token: string): Promise { let verifyError = undefined; for (const key of this.verificationKeys) { - const lookup = (_protectedHeader: JWSHeaderParameters): Uint8Array => { - return key; - }; try { - await jwtVerify(token, lookup); + await jwtVerify(token, key); // If the verify succeeded, return return; } catch (e) { diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index c0bfb14dca..14b3a00a7f 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -24,7 +24,10 @@ const logger = getVoidLogger(); function jwtKid(jwt: string): string { const header = decodeProtectedHeader(jwt); - return header.kid ?? ''; + if (!header.kid) { + throw new Error('JWT Header did not contain a key ID (kid)'); + } + return header.kid; } const entityRef = stringifyEntityRef({ diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index 6eee5ecd89..78bfb9f0ec 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -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; diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index b452c3f81d..c4802d57c9 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -248,6 +248,38 @@ describe('IdentityClient', () => { return await client.authenticate(fakeToken); }).rejects.toThrow(); }); + + it('should use an updated endpoint', async () => { + const updatedURL = 'http://backstage:9191/an-updated-base'; + const getBaseUrl = discovery.getBaseUrl; + const getExternalBaseUrl = discovery.getExternalBaseUrl; + discovery.getBaseUrl = async () => { + return updatedURL; + }; + discovery.getExternalBaseUrl = async () => { + return updatedURL; + }; + server.use( + rest.get(`${updatedURL}/.well-known/jwks.json`, async (_, res, ctx) => { + const keys = await factory.listPublicKeys(); + return res(ctx.json(keys)); + }), + ); + const token = await factory.issueToken({ claims: { sub: 'foo' } }); + const response = await client.authenticate(token); + const url = (client as any).endpoint as URL; + expect(url.toString()).toMatch(`${updatedURL}/.well-known/jwks.json`); + expect(response).toEqual({ + token: token, + identity: { + type: 'user', + userEntityRef: 'foo', + ownershipEntityRefs: [], + }, + }); + discovery.getBaseUrl = getBaseUrl; + discovery.getExternalBaseUrl = getExternalBaseUrl; + }); }); describe('listPublicKeys', () => { diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index c0eba6a069..3498fb463e 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -72,6 +72,14 @@ export class IdentityClient { if (!token) { throw new AuthenticationError('No token specified'); } + // Check if the keystore needs to be updated + const url = await this.discovery.getBaseUrl('auth'); + const endpoint = new URL(`${url}/.well-known/jwks.json`); + if (endpoint !== this.endpoint) { + this.endpoint = endpoint; + this.keyStore = createRemoteJWKSet(this.endpoint); + } + // Verify token claims and signature // Note: Claims must match those set by TokenFactory when issuing tokens // Note: verify throws if verification fails From d8063ed8e20348569f6e97938ef037942aecc97e Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Tue, 29 Mar 2022 10:33:22 -0700 Subject: [PATCH 13/17] Use the algorithm from the key object Signed-off-by: Andy Caruso --- plugins/auth-backend/src/identity/TokenFactory.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/plugins/auth-backend/src/identity/TokenFactory.ts b/plugins/auth-backend/src/identity/TokenFactory.ts index 3d49b30b6c..b72c3ef06d 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -20,6 +20,7 @@ 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; @@ -52,7 +53,6 @@ export class TokenFactory implements TokenIssuer { private readonly logger: Logger; private readonly keyStore: KeyStore; private readonly keyDurationSeconds: number; - private readonly alg = 'ES256'; private keyExpiry?: Date; private privateKeyPromise?: Promise; @@ -85,8 +85,12 @@ export class TokenFactory implements TokenIssuer { this.logger.info(`Issuing token for ${sub}, with entities ${ent ?? []}`); + if (!key.alg) { + throw new AuthenticationError('No algorithm was provided in the key'); + } + return new SignJWT({ iss, sub, aud, iat, exp }) - .setProtectedHeader({ alg: this.alg, kid: key.kid }) + .setProtectedHeader({ alg: key.alg, kid: key.kid }) .setIssuer(iss) .setAudience(aud) .setSubject(sub) @@ -152,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 generateKeyPair(this.alg); + 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 = this.alg; + 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 From 1e0a61da9d2a17a787cc51ff0b1174c781327275 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Tue, 29 Mar 2022 10:34:35 -0700 Subject: [PATCH 14/17] Update auth-node dependency on catalog-model Signed-off-by: Andy Caruso --- plugins/auth-node/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 9a99506702..446e20ec9a 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -25,7 +25,7 @@ "dependencies": { "@backstage/backend-common": "^0.13.2-next.1", "@backstage/config": "^1.0.0", - "@backstage/catalog-model": "^0.13.0", + "@backstage/catalog-model": "^1.0.1-next.0", "@backstage/errors": "^1.0.0", "jose": "^4.6.0", "node-fetch": "^2.6.7", From 999eb151f12f4152dd7b6b1672e09494622d8244 Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Wed, 30 Mar 2022 19:55:30 -0700 Subject: [PATCH 15/17] Update public key URL when key lookup fails and mutex dev key Signed-off-by: Andy Caruso --- .../src/tokens/ServerTokenManager.ts | 36 +++++++++++--- plugins/auth-node/package.json | 1 + plugins/auth-node/src/IdentityClient.test.ts | 29 +++++++++-- plugins/auth-node/src/IdentityClient.ts | 49 ++++++++++++++++--- 4 files changed, 98 insertions(+), 17 deletions(-) diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index b0512aed3a..484b99ca4d 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -39,6 +39,8 @@ class NoopTokenManager implements TokenManager { export class ServerTokenManager implements TokenManager { private verificationKeys: Uint8Array[]; private signingKey: Uint8Array; + private privateKeyPromise?: Promise; + private logger: Logger; static noop(): TokenManager { return new NoopTokenManager(); @@ -49,7 +51,10 @@ 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( @@ -60,15 +65,16 @@ export class ServerTokenManager implements TokenManager { logger.warn( 'Generated a secret for backend-to-backend authentication: DEVELOPMENT USE ONLY.', ); - return new ServerTokenManager([]); + return new ServerTokenManager([], logger); } - private constructor(secrets: string[]) { + 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]; } @@ -80,10 +86,26 @@ export class ServerTokenManager implements TokenManager { 'Key generation is not supported outside of the dev environment', ); } - const secret = await generateSecret('HS256'); - const jwk = await exportJWK(secret); - this.verificationKeys.push(base64url.decode(jwk.k ?? '')); - this.signingKey = this.verificationKeys[0]; + 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 }> { diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 446e20ec9a..419ac8674c 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -33,6 +33,7 @@ }, "devDependencies": { "@backstage/cli": "^0.17.0-next.1", + "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 c4802d57c9..7ef0b0c2e2 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -21,6 +21,7 @@ import { decodeProtectedHeader, exportJWK, } from 'jose'; +import { cloneDeep } from 'lodash'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { IdentityClient } from './IdentityClient'; @@ -249,10 +250,23 @@ describe('IdentityClient', () => { }).rejects.toThrow(); }); - it('should use an updated endpoint', async () => { + 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`, + async (_, res, ctx) => { + return res(ctx.json(singleKey)); + }, + ), + ); + // Update the discovery endpoint to point to a new URL discovery.getBaseUrl = async () => { return updatedURL; }; @@ -265,20 +279,29 @@ describe('IdentityClient', () => { return res(ctx.json(keys)); }), ); - const token = await factory.issueToken({ claims: { sub: 'foo' } }); + // 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. const url = (client as any).endpoint as URL; expect(url.toString()).toMatch(`${updatedURL}/.well-known/jwks.json`); expect(response).toEqual({ token: token, identity: { type: 'user', - userEntityRef: 'foo', + 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 3498fb463e..cfe802d184 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -18,13 +18,17 @@ import { PluginEndpointDiscovery } from '@backstage/backend-common'; import { AuthenticationError } from '@backstage/errors'; 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; + /** * An identity client to interact with auth-backend and authenticate Backstage * tokens @@ -37,6 +41,7 @@ export class IdentityClient { private readonly issuer: string; private keyStore?: GetKeyFunction; private endpoint?: URL; + private keyStoreUpdated: number = 0; /** * Create a new {@link IdentityClient} instance. @@ -57,6 +62,7 @@ export class IdentityClient { this.discovery.getBaseUrl('auth').then(url => { this.endpoint = new URL(`${url}/.well-known/jwks.json`); this.keyStore = createRemoteJWKSet(this.endpoint); + this.keyStoreUpdated = Date.now() / 1000; }); } @@ -72,13 +78,6 @@ export class IdentityClient { if (!token) { throw new AuthenticationError('No token specified'); } - // Check if the keystore needs to be updated - const url = await this.discovery.getBaseUrl('auth'); - const endpoint = new URL(`${url}/.well-known/jwks.json`); - if (endpoint !== this.endpoint) { - this.endpoint = endpoint; - this.keyStore = createRemoteJWKSet(this.endpoint); - } // Verify token claims and signature // Note: Claims must match those set by TokenFactory when issuing tokens @@ -86,6 +85,8 @@ export class IdentityClient { if (!this.keyStore) { throw new AuthenticationError('No keystore exists'); } + // Check if the keystore needs to be updated + await this.refreshKeyStore(token); const decoded = await jwtVerify(token, this.keyStore, { algorithms: ['ES256'], audience: 'backstage', @@ -109,4 +110,38 @@ export class IdentityClient { }; return user; } + + /** + * If the last keystore refresh is stale, update the keystore URL to the latest + */ + 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 issuedAfterLastRefresh = + payload?.iat && payload.iat > this.keyStoreUpdated - CLOCK_MARGIN_S; + if (!keyStoreHasKey && issuedAfterLastRefresh) { + const url = await this.discovery.getBaseUrl('auth'); + const endpoint = new URL(`${url}/.well-known/jwks.json`); + this.endpoint = endpoint; + this.keyStore = createRemoteJWKSet(this.endpoint); + this.keyStoreUpdated = Date.now() / 1000; + } + } } From a7bc5b09f9a769f97a31d04c7d6b3faca34cb00c Mon Sep 17 00:00:00 2001 From: Andy Caruso Date: Mon, 4 Apr 2022 09:06:38 -0700 Subject: [PATCH 16/17] Remove keystore creation from constructor Signed-off-by: Andy Caruso --- plugins/auth-node/src/IdentityClient.test.ts | 12 +++--------- plugins/auth-node/src/IdentityClient.ts | 13 +++---------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/plugins/auth-node/src/IdentityClient.test.ts b/plugins/auth-node/src/IdentityClient.test.ts index 7ef0b0c2e2..5efc85cb21 100644 --- a/plugins/auth-node/src/IdentityClient.test.ts +++ b/plugins/auth-node/src/IdentityClient.test.ts @@ -273,9 +273,11 @@ describe('IdentityClient', () => { 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)); }), ); @@ -288,8 +290,7 @@ describe('IdentityClient', () => { const token = await factory.issueToken({ claims: { sub: 'foo2' } }); const response = await client.authenticate(token); // Verify that the endpoint was updated. - const url = (client as any).endpoint as URL; - expect(url.toString()).toMatch(`${updatedURL}/.well-known/jwks.json`); + expect(calledUpdatedEndpoint).toBeTruthy(); expect(response).toEqual({ token: token, identity: { @@ -304,11 +305,4 @@ describe('IdentityClient', () => { dateSpy.mockClear(); }); }); - - describe('listPublicKeys', () => { - it('should use the correct endpoint', async () => { - const url = (client as any).endpoint as URL; - expect(url.toString()).toMatch(`${mockBaseUrl}/.well-known/jwks.json`); - }); - }); }); diff --git a/plugins/auth-node/src/IdentityClient.ts b/plugins/auth-node/src/IdentityClient.ts index cfe802d184..605c434431 100644 --- a/plugins/auth-node/src/IdentityClient.ts +++ b/plugins/auth-node/src/IdentityClient.ts @@ -40,7 +40,6 @@ export class IdentityClient { private readonly discovery: PluginEndpointDiscovery; private readonly issuer: string; private keyStore?: GetKeyFunction; - private endpoint?: URL; private keyStoreUpdated: number = 0; /** @@ -59,11 +58,6 @@ export class IdentityClient { }) { this.discovery = options.discovery; this.issuer = options.issuer; - this.discovery.getBaseUrl('auth').then(url => { - this.endpoint = new URL(`${url}/.well-known/jwks.json`); - this.keyStore = createRemoteJWKSet(this.endpoint); - this.keyStoreUpdated = Date.now() / 1000; - }); } /** @@ -82,11 +76,11 @@ export class IdentityClient { // Verify token claims and signature // Note: Claims must match those set by TokenFactory when issuing tokens // Note: verify throws if verification fails + // Check if the keystore needs to be updated + await this.refreshKeyStore(token); if (!this.keyStore) { throw new AuthenticationError('No keystore exists'); } - // Check if the keystore needs to be updated - await this.refreshKeyStore(token); const decoded = await jwtVerify(token, this.keyStore, { algorithms: ['ES256'], audience: 'backstage', @@ -139,8 +133,7 @@ export class IdentityClient { if (!keyStoreHasKey && issuedAfterLastRefresh) { const url = await this.discovery.getBaseUrl('auth'); const endpoint = new URL(`${url}/.well-known/jwks.json`); - this.endpoint = endpoint; - this.keyStore = createRemoteJWKSet(this.endpoint); + this.keyStore = createRemoteJWKSet(endpoint); this.keyStoreUpdated = Date.now() / 1000; } } From 047cff37d2b8109a7c22a6ab3aab8753baeb8c6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 9 Apr 2022 15:11:35 +0200 Subject: [PATCH 17/17] update yarn.lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/auth-node/package.json | 1 - yarn.lock | 41 +++------------------------------- 2 files changed, 3 insertions(+), 39 deletions(-) diff --git a/plugins/auth-node/package.json b/plugins/auth-node/package.json index 419ac8674c..f253e33171 100644 --- a/plugins/auth-node/package.json +++ b/plugins/auth-node/package.json @@ -25,7 +25,6 @@ "dependencies": { "@backstage/backend-common": "^0.13.2-next.1", "@backstage/config": "^1.0.0", - "@backstage/catalog-model": "^1.0.1-next.0", "@backstage/errors": "^1.0.0", "jose": "^4.6.0", "node-fetch": "^2.6.7", diff --git a/yarn.lock b/yarn.lock index 8d4e5aed4f..e362d30a07 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1465,19 +1465,6 @@ "@backstage/errors" "^1.0.0" cross-fetch "^3.1.5" -"@backstage/catalog-model@^0.13.0": - version "0.13.0" - resolved "https://registry.npmjs.org/@backstage/catalog-model/-/catalog-model-0.13.0.tgz#abeb91522ac7ef7907907ad5bc889803131db209" - integrity sha512-Q+0C0LJ8niNQ4TXduUNTycZ2PLMbhKmfgWywunJLO34DZAoFnTWJEAdEYF536YyLciHhLO0dHRUaHY29Sdr+Xg== - dependencies: - "@backstage/config" "^0.1.15" - "@backstage/errors" "^0.2.2" - "@backstage/types" "^0.1.3" - ajv "^7.0.3" - json-schema "^0.4.0" - lodash "^4.17.21" - uuid "^8.0.0" - "@backstage/catalog-model@^1.0.0": version "1.0.0" resolved "https://registry.npmjs.org/@backstage/catalog-model/-/catalog-model-1.0.0.tgz#0aa8694a3182aaf4232725842da751bf5f78bd68" @@ -1491,14 +1478,6 @@ lodash "^4.17.21" uuid "^8.0.0" -"@backstage/config@^0.1.15": - version "0.1.15" - resolved "https://registry.npmjs.org/@backstage/config/-/config-0.1.15.tgz#4bad122ad861be5bd61a60639f92d2494fa245c5" - integrity sha512-eNJEYYSEu9MkrkBYiMpUBWEc3Bu64YgB9pZZGCMW7/9350tV2wbylEdoBJHslilJlJhiUyTXBckn8Ua7DOH7rw== - dependencies: - "@backstage/types" "^0.1.3" - lodash "^4.17.21" - "@backstage/core-components@^0.9.0", "@backstage/core-components@^0.9.2": version "0.9.2" resolved "https://registry.npmjs.org/@backstage/core-components/-/core-components-0.9.2.tgz#9a3d79a15039256bbc007e5daa08c983050e0238" @@ -1556,15 +1535,6 @@ react-router-dom "6.0.0-beta.0" zen-observable "^0.8.15" -"@backstage/errors@^0.2.2": - version "0.2.2" - resolved "https://registry.npmjs.org/@backstage/errors/-/errors-0.2.2.tgz#2113e0bc859e645b8b59bfcb435f7535739b02f8" - integrity sha512-s6Ru3NL4oFiHwNsMY1uiwqNKuOu9KQWkpse3adWZsDu1fCQKOw2omQ2qBHZ4Uj1xJ6x9Ye0yNTJ4vfHfdwiaEg== - dependencies: - "@backstage/types" "^0.1.3" - cross-fetch "^3.1.5" - serialize-error "^8.0.1" - "@backstage/integration-react@^1.0.0": version "1.0.0" resolved "https://registry.npmjs.org/@backstage/integration-react/-/integration-react-1.0.0.tgz#8075e65c6b5387631d27a9c242e7a4fff6f92417" @@ -1645,11 +1615,6 @@ react-use "^17.2.4" swr "^1.1.2" -"@backstage/types@^0.1.3": - version "0.1.3" - resolved "https://registry.npmjs.org/@backstage/types/-/types-0.1.3.tgz#6613d8cbdf97d42d31cd1e66a833df533e7ccf14" - integrity sha512-fJVi4oVrlO+G3PRv1fYSll9/X4pE11HLnkI//Geare9sP6wSfp/2zXpLYfKVsG0e24jOl7Swkc8lwLkQ90zMaQ== - "@balena/dockerignore@^1.0.2": version "1.0.2" resolved "https://registry.npmjs.org/@balena/dockerignore/-/dockerignore-1.0.2.tgz#9ffe4726915251e8eb69f44ef3547e0da2c03e0d" @@ -15769,9 +15734,9 @@ jose@^4.1.4: integrity sha512-GFcVFQwYQKbQTUOo2JlpFGXTkgBw26uzDsRMD2q1WgSKNSnpKS9Ug7bdQ8dS+p4sZHNH6iRPu6WK2jLIjspaMA== jose@^4.6.0: - version "4.6.1" - resolved "https://registry.npmjs.org/jose/-/jose-4.6.1.tgz#241472ff928c79b2e2e2fe4c1056b3085384ec42" - integrity sha512-EFnufEivlIB6j7+JwaenYQzdUDs/McajDr9WnhT6EI0WxbexnfuZimpWX1GnobF6OnQsUFmWFXUXdWyZHWdQow== + 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"