From 7ae7fc615c197839dcb68ca085bc3040d4a6e598 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 21 Jan 2021 12:00:49 +0100 Subject: [PATCH] auth-backend: revert jose to v3 --- plugins/auth-backend/package.json | 7 +-- .../src/identity/TokenFactory.test.ts | 37 ++++++---------- .../auth-backend/src/identity/TokenFactory.ts | 44 ++++++++----------- .../src/providers/aws-alb/provider.test.ts | 33 +++++--------- .../src/providers/aws-alb/provider.ts | 6 +-- .../src/providers/oidc/provider.test.ts | 8 +--- yarn.lock | 12 ++--- 7 files changed, 55 insertions(+), 92 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 880b9a33e8..4445e02ad5 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -16,11 +16,6 @@ "url": "https://github.com/backstage/backstage", "directory": "plugins/auth-backend" }, - "jest": { - "moduleNameMapper": { - "^jose/(.*)$": "/../../../node_modules/jose/dist/node/cjs/$1" - } - }, "keywords": [ "backstage" ], @@ -49,7 +44,7 @@ "fs-extra": "^9.0.0", "got": "^11.5.2", "helmet": "^4.0.0", - "jose": "^3.5.1", + "jose": "^1.27.1", "jwt-decode": "^3.1.0", "knex": "^0.21.6", "moment": "^2.26.0", diff --git a/plugins/auth-backend/src/identity/TokenFactory.test.ts b/plugins/auth-backend/src/identity/TokenFactory.test.ts index e28ded2bf2..8b719799b1 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.test.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.test.ts @@ -13,23 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { TextDecoder, TextEncoder } from 'util'; - -// These two statements are structured like this because Jest doesn't include these in the default -// test environment, even though they exist in Node. -global.TextEncoder = TextEncoder; -// @ts-ignore -global.TextDecoder = TextDecoder; import { utc } from 'moment'; import { TokenFactory } from './TokenFactory'; import { getVoidLogger } from '@backstage/backend-common'; -import { AnyJWK, KeyStore, StoredKey } from './types'; -import jwtVerify from 'jose/jwt/verify'; -import parseJwk from 'jose/jwk/parse'; -import decodeProtectedHeader, { - ProtectedHeaderParameters, -} from 'jose/util/decode_protected_header'; +import { KeyStore, AnyJWK, StoredKey } from './types'; +import { JWKS, JSONWebKey, JWT } from 'jose'; const logger = getVoidLogger(); @@ -63,8 +52,10 @@ class MemoryKeyStore implements KeyStore { } function jwtKid(jwt: string): string { - const header = decodeProtectedHeader(jwt) as ProtectedHeaderParameters; - return header.kid as string; + const { header } = JWT.decode(jwt, { complete: true }) as { + header: { kid: string }; + }; + return header.kid; } describe('TokenFactory', () => { @@ -81,14 +72,14 @@ describe('TokenFactory', () => { const token = await factory.issueToken({ claims: { sub: 'foo' } }); const { keys } = await factory.listPublicKeys(); - const keyMap = Object.fromEntries(keys.map(key => [key.kid, key])); + const keyStore = JWKS.asKeyStore({ + keys: keys.map(key => key as JSONWebKey), + }); - const payload = ( - await jwtVerify(token, async header => { - const kid = header.kid as string; - return await parseJwk(keyMap[kid]); - }) - ).payload; + const payload = JWT.verify(token, keyStore) as object & { + iat: number; + exp: number; + }; expect(payload).toEqual({ iss: 'my-issuer', aud: 'backstage', @@ -96,7 +87,7 @@ describe('TokenFactory', () => { iat: expect.any(Number), exp: expect.any(Number), }); - expect(payload.exp).toBe(Number(payload.iat) + keyDurationSeconds); + expect(payload.exp).toBe(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 a1d846d5d7..36622332e0 100644 --- a/plugins/auth-backend/src/identity/TokenFactory.ts +++ b/plugins/auth-backend/src/identity/TokenFactory.ts @@ -16,10 +16,7 @@ import moment from 'moment'; import { TokenIssuer, TokenParams, KeyStore, AnyJWK } from './types'; -import parseJwk, { JWK } from 'jose/jwk/parse'; -import SignJWT from 'jose/jwt/sign'; -import generateKeyPair from 'jose/util/generate_key_pair'; -import fromKeyLike from 'jose/jwk/from_key_like'; +import { JSONWebKey, JWK, JWS } from 'jose'; import { Logger } from 'winston'; import { v4 as uuid } from 'uuid'; @@ -56,7 +53,7 @@ export class TokenFactory implements TokenIssuer { private readonly keyDurationSeconds: number; private keyExpiry?: moment.Moment; - private privateKeyPromise?: Promise; + private privateKeyPromise?: Promise; constructor(options: Options) { this.issuer = options.issuer; @@ -67,7 +64,7 @@ export class TokenFactory implements TokenIssuer { async issueToken(params: TokenParams): Promise { const key = await this.getKey(); - const keyLike = await parseJwk(key); + const iss = this.issuer; const sub = params.claims.sub; const aud = 'backstage'; @@ -75,9 +72,11 @@ export class TokenFactory implements TokenIssuer { const exp = iat + this.keyDurationSeconds; this.logger.info(`Issuing token for ${sub}`); - return new SignJWT({ iss, sub, aud, iat, exp }) - .setProtectedHeader({ alg: key.alg, typ: 'JWT', kid: key.kid }) - .sign(keyLike); + + return JWS.sign({ iss, sub, aud, iat, exp }, key, { + alg: key.alg, + kid: key.kid, + }); } // This will be called by other services that want to verify ID tokens. @@ -115,7 +114,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 (this.keyExpiry?.isAfter()) { @@ -128,30 +127,23 @@ export class TokenFactory implements TokenIssuer { this.keyExpiry = moment().add(this.keyDurationSeconds, 'seconds'); const promise = (async () => { // This generates a new signing key to be used to sign tokens until the next key rotation - - const key = await generateKeyPair('ES256'); - const kid = uuid(); - const jwk = await fromKeyLike(key.privateKey); - - // JOSE Library provides optional for most fields - and TS does not distinguish between missing/undefined. - // Because AnyJWK requires keys to have type "string", this throws a TypeError - though in practice, if the field - // is undefined, JOSE will not send it back as key. - const storedJwk: AnyJWK = { - ...jwk, - alg: 'ES256', - kid: kid, + const key = await JWK.generate('EC', 'P-256', { use: 'sig', - } as AnyJWK; + kid: uuid(), + 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 // don't attempt to fetch new ones even if they encounter an unknown kid. Therefore we // 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 ${jwk.kid}`); - await this.keyStore.addKey(storedJwk); + this.logger.info(`Created new signing key ${key.kid}`); + await this.keyStore.addKey((key.toJWK(false) as unknown) as AnyJWK); + // At this point we are allowed to start using the new key - return storedJwk; + return key as JSONWebKey; })(); 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 3d27539314..284dca1a6e 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.test.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.test.ts @@ -15,12 +15,12 @@ */ import { getVoidLogger } from '@backstage/backend-common'; import express from 'express'; -import * as jwtVerify from 'jose/jwt/verify'; +import { JWT } from 'jose'; import { AwsAlbAuthProvider } from './provider'; import { AuthResponse } from '../types'; -const mockedJwtVerify = jwtVerify as jest.Mocked; +const jwtMock = JWT as jest.Mocked; const mockKey = async () => { return `-----BEGIN PUBLIC KEY----- @@ -30,6 +30,8 @@ yOlxJ2VW88mLAQGJ7HPAvOdylxZsItMnzCuqNzZvie8m/NJsOjhDncVkrw== `; }; +jest.mock('jose'); + jest.mock('cross-fetch', () => ({ __esModule: true, default: async () => { @@ -41,13 +43,6 @@ jest.mock('cross-fetch', () => ({ }, })); -jest.mock('jose/jwt/verify', () => { - return { - __esModule: true, - default: jest.fn(), - }; -}); - const identityResolutionCallbackMock = async (): Promise> => { return { backstageIdentity: { @@ -106,13 +101,9 @@ describe('AwsALBAuthProvider', () => { issuer: 'foo', }); - mockedJwtVerify.default.mockImplementationOnce(async () => { - return { - payload: { - sub: 'foo', - }, - }; - }); + jwtMock.verify.mockImplementationOnce(() => ({ + sub: 'foo', + })); await provider.refresh(mockRequest, mockResponse); @@ -148,7 +139,7 @@ describe('AwsALBAuthProvider', () => { issuer: 'foo', }); - mockedJwtVerify.default.mockImplementationOnce(async () => { + jwtMock.verify.mockImplementationOnce(() => { throw new Error('bad JWT'); }); @@ -164,9 +155,7 @@ describe('AwsALBAuthProvider', () => { issuer: 'foobar', }); - mockedJwtVerify.default.mockImplementationOnce(async () => { - return {}; - }); + jwtMock.verify.mockReturnValueOnce({}); await provider.refresh(mockRequest, mockResponse); @@ -180,9 +169,7 @@ describe('AwsALBAuthProvider', () => { issuer: 'foo', }); - mockedJwtVerify.default.mockImplementationOnce(async () => { - return {}; - }); + jwtMock.verify.mockReturnValueOnce({}); await provider.refresh(mockRequest, mockResponse); diff --git a/plugins/auth-backend/src/providers/aws-alb/provider.ts b/plugins/auth-backend/src/providers/aws-alb/provider.ts index d8ec0cb525..7003354077 100644 --- a/plugins/auth-backend/src/providers/aws-alb/provider.ts +++ b/plugins/auth-backend/src/providers/aws-alb/provider.ts @@ -24,7 +24,7 @@ import * as crypto from 'crypto'; import { KeyObject } from 'crypto'; import { Logger } from 'winston'; import NodeCache from 'node-cache'; -import jwtVerify from 'jose/jwt/verify'; +import { JWT } from 'jose'; import { CatalogApi } from '@backstage/catalog-client'; const ALB_JWT_HEADER = 'x-amzn-oidc-data'; @@ -68,7 +68,7 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { try { const headers = getJWTHeaders(jwt); const key = await this.getKey(headers.kid); - const verifiedToken = await jwtVerify(jwt, key, {}); + const payload = JWT.verify(jwt, key); if ( this.options.issuer !== '' && @@ -78,7 +78,7 @@ export class AwsAlbAuthProvider implements AuthProviderRouteHandlers { } const resolvedEntity = await this.options.identityResolutionCallback( - verifiedToken.payload, + payload, this.catalogClient, ); res.send(resolvedEntity); diff --git a/plugins/auth-backend/src/providers/oidc/provider.test.ts b/plugins/auth-backend/src/providers/oidc/provider.test.ts index ae0ca2da2b..fcbe9d7abc 100644 --- a/plugins/auth-backend/src/providers/oidc/provider.test.ts +++ b/plugins/auth-backend/src/providers/oidc/provider.test.ts @@ -13,17 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { TextDecoder, TextEncoder } from 'util'; -// @ts-ignore -global.TextDecoder = TextDecoder; -global.TextEncoder = TextEncoder; import express from 'express'; import { Session } from 'express-session'; import nock from 'nock'; import { ClientMetadata, IssuerMetadata } from 'openid-client'; import { createOidcProvider, OidcAuthProvider } from './provider'; -import UnsecuredJWT from 'jose/jwt/unsecured'; +import { JWT, JWK } from 'jose'; import { AuthProviderFactoryOptions } from '../types'; import { Config } from '@backstage/config'; import { OAuthAdapter } from '../../lib/oauth'; @@ -84,7 +80,7 @@ describe('OidcAuthProvider', () => { .reply(200, issuerMetadata) .post('/as/token.oauth2') .reply(200, { - id_token: new UnsecuredJWT(jwt).encode(), + id_token: JWT.sign(jwt, JWK.None), access_token: 'test', authorization_signed_response_alg: 'HS256', }) diff --git a/yarn.lock b/yarn.lock index 514c9a4ff9..f6762a63f0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16714,6 +16714,13 @@ jest@^26.0.1: import-local "^3.0.2" jest-cli "^26.6.3" +jose@^1.27.1: + version "1.28.0" + resolved "https://registry.npmjs.org/jose/-/jose-1.28.0.tgz#0803f8c71f43cd293a9d931c555c30531f5ca5dc" + integrity sha512-JmfDRzt/HSj8ipd9TsDtEHoLUnLYavG+7e8F6s1mx2jfVSfXOTaFQsJUydbjJpTnTDHP1+yKL9Ke7ktS/a0Eiw== + dependencies: + "@panva/asn1.js" "^1.0.0" + jose@^2.0.2: version "2.0.3" resolved "https://registry.npmjs.org/jose/-/jose-2.0.3.tgz#9c931ab3e13e2d16a5b9e6183e60b2fc40a8e1b8" @@ -16721,11 +16728,6 @@ jose@^2.0.2: dependencies: "@panva/asn1.js" "^1.0.0" -jose@^3.5.1: - version "3.5.1" - resolved "https://registry.npmjs.org/jose/-/jose-3.5.1.tgz#adc0d5000dbf64a7f4db171d449dbcf6b556b6f0" - integrity sha512-BQQJafCDvsmtc/LaK57cjfhU/1AKBhJSbJhKPq+uhrjMoV/sh3/dbk9cm08nAeSGS6j+sjhWoY+LZPUXiKzYzw== - joycon@^2.2.5: version "2.2.5" resolved "https://registry.npmjs.org/joycon/-/joycon-2.2.5.tgz#8d4cf4cbb2544d7b7583c216fcdfec19f6be1615"