From 5635f31769390a6c355870f7f05fdbf3cc9a303a Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Wed, 25 Sep 2024 18:40:00 +0100 Subject: [PATCH 01/12] first version of alb cache --- src/alb.ts | 117 +++++++++++++++++++++++++++++++++++ src/jwk.ts | 8 ++- tests/unit/alb-jwks-test.pem | 4 ++ tests/unit/alb.test.ts | 89 ++++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 src/alb.ts create mode 100644 tests/unit/alb-jwks-test.pem create mode 100644 tests/unit/alb.test.ts diff --git a/src/alb.ts b/src/alb.ts new file mode 100644 index 0000000..c860c46 --- /dev/null +++ b/src/alb.ts @@ -0,0 +1,117 @@ +import { createPublicKey } from "crypto"; +import { + JwtBaseError, + JwtWithoutValidKidError, + KidNotFoundInJwksError, +} from "./error"; +import { + JwkWithKid, + Jwks, + JwksCache, + JwksParser, + PenaltyBox, + SimpleJwksCache, + assertIsJwks, +} from "./jwk"; +import { JwtHeader, JwtPayload } from "./jwt-model"; + +interface DecomposedJwt { + header: JwtHeader; + payload: JwtPayload; +} + +const albJwksUriRegex = + /https:\/\/public-keys.auth.elb.(?[a-z0-9-]+).amazonaws.com\/(?[a-z0-9-]+)/; + +export class AlbUriError extends JwtBaseError {} + +const parseJwks: JwksParser = function (jwksBin: ArrayBuffer, jwksUri: string) { + const match = jwksUri.match(albJwksUriRegex); + if (!match || !match.groups?.kid) { + throw new AlbUriError("Wrong URI for ALB public key"); + } + const jwks = { + keys: [ + { + kid: match.groups.kid, + use: "sig", + ...createPublicKey({ + key: Buffer.from(jwksBin), + format: "pem", + type: "spki", + }).export({ + format: "jwk", + }), + }, + ], + }; + assertIsJwks(jwks); + return jwks; +}; + +const KID_URI_VARIABLE = "{kid}"; + +export class AwsAlbJwksCache implements JwksCache { + simpleJwksCache: SimpleJwksCache; + + constructor(props?: { penaltyBox?: PenaltyBox }) { + this.simpleJwksCache = new SimpleJwksCache({ + penaltyBox: props?.penaltyBox, + jwksParser: parseJwks, + }); + } + + /** + * + * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} + * @param decomposedJwt + * @returns + */ + private expandWithKid(jwksUri: string, decomposedJwt: DecomposedJwt): string { + const kid = this.getKid(decomposedJwt); + if (jwksUri.indexOf(KID_URI_VARIABLE) < 0) { + throw new KidNotFoundInJwksError("kid not found in URI"); + } + return jwksUri.replace(KID_URI_VARIABLE, encodeURIComponent(kid)); + } + + private getKid(decomposedJwt: DecomposedJwt): string { + if (typeof decomposedJwt.header.kid !== "string") { + throw new JwtWithoutValidKidError( + "JWT header does not have valid kid claim" + ); + } + return decomposedJwt.header.kid; + } + + public addJwks(): void { + throw new Error("Method not implemented."); + } + + async getJwks(): Promise { + throw new Error("Method not implemented."); + } + + + /** + * + * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} + * @param decomposedJwt + * @returns + */ + public getCachedJwk( + jwksUri: string, + decomposedJwt: DecomposedJwt + ): JwkWithKid { + const jwksUriExpanded = this.expandWithKid(jwksUri, decomposedJwt); + return this.simpleJwksCache.getCachedJwk(jwksUriExpanded, decomposedJwt); + } + + public async getJwk( + jwksUri: string, + decomposedJwt: DecomposedJwt + ): Promise { + const jwksUriExpanded = this.expandWithKid(jwksUri, decomposedJwt); + return this.simpleJwksCache.getJwk(jwksUriExpanded, decomposedJwt); + } +} diff --git a/src/jwk.ts b/src/jwk.ts index 09318f6..67c735c 100644 --- a/src/jwk.ts +++ b/src/jwk.ts @@ -99,7 +99,7 @@ export interface JwksCache { * @param jwksBin * @returns Jwks */ -export type JwksParser = (jwksBin: ArrayBuffer) => Jwks; +export type JwksParser = (jwksBin: ArrayBuffer, jwksUri: string) => Jwks; /** * UTF-8 decode binary data and then JSON parse it @@ -122,7 +122,7 @@ const parseJwks: JwksParser = function (jwksBin: ArrayBuffer) { }; export async function fetchJwks(jwksUri: string): Promise { - return fetch(jwksUri).then(parseJwks); + return fetch(jwksUri).then((jwksBin) => parseJwks(jwksBin, jwksUri)); } export async function fetchJwk( @@ -323,7 +323,9 @@ export class SimpleJwksCache implements JwksCache { if (existingFetch) { return existingFetch; } - const jwksPromise = this.fetcher.fetch(jwksUri).then(this.jwksParser); + const jwksPromise = this.fetcher + .fetch(jwksUri) + .then((jwksBin) => this.jwksParser(jwksBin, jwksUri)); this.fetchingJwks.set(jwksUri, jwksPromise); let jwks: Jwks; try { diff --git a/tests/unit/alb-jwks-test.pem b/tests/unit/alb-jwks-test.pem new file mode 100644 index 0000000..7ca2e7f --- /dev/null +++ b/tests/unit/alb-jwks-test.pem @@ -0,0 +1,4 @@ +-----BEGIN PUBLIC KEY----- +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEGBJCbjNusVteS//606LS3fgYrhQy +vfAh+GbOfy2n7rWgG433Rtb4C/Gxyh6xVoTuvI8hKOqx4qCKjoflk7nGaQ== +-----END PUBLIC KEY----- \ No newline at end of file diff --git a/tests/unit/alb.test.ts b/tests/unit/alb.test.ts new file mode 100644 index 0000000..f331398 --- /dev/null +++ b/tests/unit/alb.test.ts @@ -0,0 +1,89 @@ +import { + JwtWithoutValidKidError, +} from "../../src/error"; +import { AwsAlbJwksCache } from "../../src/alb"; +import { + allowAllRealNetworkTraffic, + disallowAllRealNetworkTraffic, + mockHttpsUri, +} from "./test-util"; +import { readFileSync } from "fs"; +import { join } from "path"; + +describe("unit tests https", () => { + const kid = "abcdefgh-1234-ijkl-5678-mnopqrstuvwx"; + const jwksUriTemplate = "https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid}"; + const jwksUri = `https://public-keys.auth.elb.eu-west-1.amazonaws.com/${kid}`; + + const albResponse = readFileSync(join(__dirname, "alb-jwks-test.pem")); + const jwk = { + kid: kid, + use: "sig", + kty: "EC", + x: "GBJCbjNusVteS__606LS3fgYrhQyvfAh-GbOfy2n7rU", + y: "oBuN90bW-AvxscoesVaE7ryPISjqseKgio6H5ZO5xmk", + crv: "P-256", + }; + const jwks = { + keys: [jwk], + }; + const getDecomposedJwt = (kidParam?: string) => ({ + header: { + alg: "EC256", + kid: kidParam ?? kid, + }, + payload: {}, + }); + + beforeAll(() => { + disallowAllRealNetworkTraffic(); + }); + afterAll(() => { + allowAllRealNetworkTraffic(); + }); + + test("ALB JWKS cache error flow: kid empty", () => { + const jwksCache = new AwsAlbJwksCache(); + expect.assertions(1); + return expect( + jwksCache.getJwk(jwksUriTemplate, { header: { alg: "EC256" }, payload: {} }) + ).rejects.toThrow(JwtWithoutValidKidError); + }); + + test("ALB JWKS add cache return not implemented exception", () => { + const jwksCache = new AwsAlbJwksCache(); + return expect(jwksCache.addJwks).toThrow("Method not implemented."); + }); + + test("ALB JWKS get JWKS return not implemented exception", () => { + const jwksCache = new AwsAlbJwksCache(); + expect.assertions(1); + return expect(jwksCache.getJwks()).rejects.toThrow( + "Method not implemented." + ); + }); + + test("ALB JWKS cache fetches URI one attempt at a time", async () => { + /** + * Test what happens when the the JWKS URI is requested multiple times in parallel + * (e.g. in parallel promises). When this happens only 1 actual HTTPS request should + * be made to the JWKS URI. + */ + + mockHttpsUri(jwksUri, { + responsePayload: albResponse, + }); + + const jwksCache = new AwsAlbJwksCache(); + + const fetch = jest.spyOn(jwksCache.simpleJwksCache.fetcher,"fetch"); + + const promise1 = jwksCache.getJwk(jwksUriTemplate, getDecomposedJwt()); + const promise2 = jwksCache.getJwk(jwksUriTemplate, getDecomposedJwt()); + expect.assertions(2); + expect(promise1).toEqual(promise2); + await Promise.all([promise1, promise2]); + expect(fetch).toHaveBeenCalledTimes(1); + }); + +}); From 5d047c29ba23d1c5600a8a753b8f718b1f132877 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Tue, 8 Oct 2024 18:36:30 +0100 Subject: [PATCH 02/12] add LRU cache and ALB V2 --- src/{alb.ts => alb-v1.ts} | 12 +- src/alb-v2.ts | 151 ++++++ src/cache.ts | 153 ++++++ tests/unit/{alb.test.ts => alb-v1.test.ts} | 14 +- tests/unit/cache.test.ts | 542 +++++++++++++++++++++ 5 files changed, 856 insertions(+), 16 deletions(-) rename src/{alb.ts => alb-v1.ts} (83%) create mode 100644 src/alb-v2.ts create mode 100644 src/cache.ts rename tests/unit/{alb.test.ts => alb-v1.test.ts} (81%) create mode 100644 tests/unit/cache.test.ts diff --git a/src/alb.ts b/src/alb-v1.ts similarity index 83% rename from src/alb.ts rename to src/alb-v1.ts index c860c46..44bc782 100644 --- a/src/alb.ts +++ b/src/alb-v1.ts @@ -2,7 +2,6 @@ import { createPublicKey } from "crypto"; import { JwtBaseError, JwtWithoutValidKidError, - KidNotFoundInJwksError, } from "./error"; import { JwkWithKid, @@ -49,8 +48,6 @@ const parseJwks: JwksParser = function (jwksBin: ArrayBuffer, jwksUri: string) { return jwks; }; -const KID_URI_VARIABLE = "{kid}"; - export class AwsAlbJwksCache implements JwksCache { simpleJwksCache: SimpleJwksCache; @@ -63,16 +60,13 @@ export class AwsAlbJwksCache implements JwksCache { /** * - * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} + * @param Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com * @param decomposedJwt * @returns */ private expandWithKid(jwksUri: string, decomposedJwt: DecomposedJwt): string { const kid = this.getKid(decomposedJwt); - if (jwksUri.indexOf(KID_URI_VARIABLE) < 0) { - throw new KidNotFoundInJwksError("kid not found in URI"); - } - return jwksUri.replace(KID_URI_VARIABLE, encodeURIComponent(kid)); + return `${jwksUri}/${encodeURIComponent(kid)}`; } private getKid(decomposedJwt: DecomposedJwt): string { @@ -95,7 +89,7 @@ export class AwsAlbJwksCache implements JwksCache { /** * - * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} + * @param Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com * @param decomposedJwt * @returns */ diff --git a/src/alb-v2.ts b/src/alb-v2.ts new file mode 100644 index 0000000..0ee584c --- /dev/null +++ b/src/alb-v2.ts @@ -0,0 +1,151 @@ +import { createPublicKey } from "crypto"; +import { + JwksNotAvailableInCacheError, + JwtBaseError, + JwtWithoutValidKidError, +} from "./error"; +import { + JwkWithKid, + Jwks, + JwksCache, +} from "./jwk"; +import { JwtHeader, JwtPayload } from "./jwt-model"; +import { Fetcher, SimpleFetcher } from "./https"; +import { SimpleLruCache } from "./cache"; + +interface DecomposedJwt { + header: JwtHeader; + payload: JwtPayload; +} + +type JwksUri = string; + +type CacheValue = { + jwk?:JwkWithKid; + promise:Promise; +} + +export class AlbUriError extends JwtBaseError {} + +export class AwsAlbJwksCache implements JwksCache { + + fetcher: Fetcher; + + private jwkCache: SimpleLruCache = new SimpleLruCache(2); + + constructor(props?: { + fetcher?: Fetcher; + }) { + this.fetcher = props?.fetcher ?? new SimpleFetcher(); + } + + + /** + * + * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} + * @param decomposedJwt + * @returns + */ + private expandWithKid(jwksUri: string, kid: string): string { + return `${jwksUri}/${encodeURIComponent(kid)}`; + } + + private getKid(decomposedJwt: DecomposedJwt): string { + if (typeof decomposedJwt.header.kid !== "string") { + throw new JwtWithoutValidKidError( + "JWT header does not have valid kid claim" + ); + } + return decomposedJwt.header.kid; + } + + public async getJwk( + jwksUri: string, + decomposedJwt: DecomposedJwt + ): Promise { + const kid = this.getKid(decomposedJwt); + const jwksUriWithKid = this.expandWithKid(jwksUri, kid); + const cacheValue = this.jwkCache.get(jwksUriWithKid); + if(cacheValue){ + //cache hit + if(cacheValue.jwk){ + return cacheValue.jwk; + }else{ + return cacheValue.promise; + } + }else{ + //cache miss + const cacheValue:CacheValue = { + promise:this.fetcher + .fetch(jwksUri) + .then(pem =>this.pemToJwk(kid,pem)) + .then(jwk=>cacheValue.jwk = jwk) + } + + const jwkPromise = cacheValue.promise; + //TODO error and retry + this.jwkCache.set(jwksUriWithKid,cacheValue); + return jwkPromise; + } + } + + private pemToJwk(kid:string, pem:ArrayBuffer):JwkWithKid{ + const jwk = createPublicKey({ + key: Buffer.from(pem), + format: "pem", + type: "spki", + }).export({ + format: "jwk", + }); + + if(!jwk.kty){ + throw new Error("todo"); + } + + return { + kid: kid, + use: "sig", + ...jwk, + } as JwkWithKid + } + + public getCachedJwk( + jwksUri: string, + decomposedJwt: DecomposedJwt + ): JwkWithKid { + const kid = this.getKid(decomposedJwt); + const jwksUriWithKid = this.expandWithKid(jwksUri, kid); + const cacheValue = this.jwkCache.get(jwksUriWithKid); + if(cacheValue?.jwk){ + return cacheValue.jwk; + }else{ + throw new JwksNotAvailableInCacheError( + `JWKS for uri ${jwksUri} not yet available in cache` + ); + } + } + + public addJwks(jwksUri: string, jwks: Jwks): void { + if(jwks.keys.length===1){ + const jwk = jwks.keys[0]; + if(jwk.kid){ + const jwkWithKid = jwk as JwkWithKid; + const kid = jwk.kid; + const jwksUriWithKid = this.expandWithKid(jwksUri, kid); + this.jwkCache.set(jwksUriWithKid,{ + jwk:jwkWithKid, + promise:Promise.resolve(jwkWithKid) + }); + }else{ + throw new Error("TODO"); + } + }else{ + throw new Error("TODO"); + } + } + + async getJwks(): Promise { + throw new Error("Method not implemented."); + } + +} diff --git a/src/cache.ts b/src/cache.ts new file mode 100644 index 0000000..52704fc --- /dev/null +++ b/src/cache.ts @@ -0,0 +1,153 @@ +import assert from "assert"; + +interface LinkedListNode { + t:T + prev?:LinkedListNode; + next?:LinkedListNode; +} + +export class SimpleLinkedList { + + first?:LinkedListNode; + last?:LinkedListNode; + size: number = 0; + + public moveFirst(node:LinkedListNode){ + if(node !== this.first){ + this.moveNodeAfter(node,'first'); + } + } + + private removeNode(node:LinkedListNode){ + if(node === this.last){ + this.last = this.last.next; + } + + if(node === this.first){ + this.first = this.first.prev; + } + + if(node.prev){ + node.prev.next = node.next; + } + if(node.next){ + node.next.prev = node.prev; + } + + node.next = undefined; + node.prev = undefined; + + this.size--; + } + + private addAfter(t:T, after:LinkedListNode | 'first'):LinkedListNode{ + const newNode: LinkedListNode = { t }; + this.moveNodeAfter(newNode,after); + this.size++; + return newNode; + } + + private moveNodeAfter(node:LinkedListNode,after:LinkedListNode | 'first'):void{ + if(after==='first'){ + if(this.first){ + return this.moveNodeAfter(node,this.first); + }else{ + //When empty LinkedList + this.first = node; + this.last = node; + } + }else{ + + if(node === this.last){ + this.last = this.last.next; + } + + if(after === this.first){ + this.first = node; + } + + // nodePrev -- node -- nodeNext -- afterPrev --- after -- afterNext + // => + // nodePrev -- nodeNext -- afterPrev --- after -- node -- afterNext + + const afterNext = after.next; + const nodePrev = node.prev; + const nodeNext = node.next; + + if(afterNext){ + afterNext.prev = node; + } + if(nodePrev){ + nodePrev.next = node.next; + } + if(nodeNext){ + nodeNext.prev = node.prev; + } + + after.next = node; + node.next = afterNext; + node.prev = after; + } + } + + public addFirst(t:T):LinkedListNode{ + return this.addAfter(t,'first'); + } + + public removeLast():T|undefined{ + const last = this.last; + if(last){ + this.removeNode(last); + return last.t; + }else{ + return undefined; + } + } + +} + +export class SimpleLruCache { + + index:Map>; + list:SimpleLinkedList<[Key,Value]>; + + constructor(public readonly capacity:number){ + assert(capacity>0); + this.index = new Map>(); + this.list = new SimpleLinkedList<[Key,Value]>(); + } + + public get size(){ + return this.index.size; + } + + public get(key:Key):Value|undefined{ + const node = this.index.get(key); + if(node){ + this.list.moveFirst(node); + return node.t[1]; + }else{ + return undefined; + } + } + + public set(key:Key, value:Value):this{ + const node = this.index.get(key); + if(node){ + this.list.moveFirst(node); + node.t[1] = value; + }else{ + if(this.size>=this.capacity){ + const last = this.list.removeLast(); + assert(last); + assert(this.index.delete(last[0])); + } + + const newNode = this.list.addFirst([key,value]); + this.index.set(key,newNode); + } + return this; + } + +} + diff --git a/tests/unit/alb.test.ts b/tests/unit/alb-v1.test.ts similarity index 81% rename from tests/unit/alb.test.ts rename to tests/unit/alb-v1.test.ts index f331398..f5b32f3 100644 --- a/tests/unit/alb.test.ts +++ b/tests/unit/alb-v1.test.ts @@ -1,7 +1,7 @@ import { JwtWithoutValidKidError, } from "../../src/error"; -import { AwsAlbJwksCache } from "../../src/alb"; +import { AwsAlbJwksCache } from "../../src/alb-v1"; import { allowAllRealNetworkTraffic, disallowAllRealNetworkTraffic, @@ -12,8 +12,8 @@ import { join } from "path"; describe("unit tests https", () => { const kid = "abcdefgh-1234-ijkl-5678-mnopqrstuvwx"; - const jwksUriTemplate = "https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid}"; - const jwksUri = `https://public-keys.auth.elb.eu-west-1.amazonaws.com/${kid}`; + const jwksUri = "https://public-keys.auth.elb.eu-west-1.amazonaws.com"; + const jwksUriWithKid = `https://public-keys.auth.elb.eu-west-1.amazonaws.com/${kid}`; const albResponse = readFileSync(join(__dirname, "alb-jwks-test.pem")); const jwk = { @@ -46,7 +46,7 @@ describe("unit tests https", () => { const jwksCache = new AwsAlbJwksCache(); expect.assertions(1); return expect( - jwksCache.getJwk(jwksUriTemplate, { header: { alg: "EC256" }, payload: {} }) + jwksCache.getJwk(jwksUri, { header: { alg: "EC256" }, payload: {} }) ).rejects.toThrow(JwtWithoutValidKidError); }); @@ -70,7 +70,7 @@ describe("unit tests https", () => { * be made to the JWKS URI. */ - mockHttpsUri(jwksUri, { + mockHttpsUri(jwksUriWithKid, { responsePayload: albResponse, }); @@ -78,8 +78,8 @@ describe("unit tests https", () => { const fetch = jest.spyOn(jwksCache.simpleJwksCache.fetcher,"fetch"); - const promise1 = jwksCache.getJwk(jwksUriTemplate, getDecomposedJwt()); - const promise2 = jwksCache.getJwk(jwksUriTemplate, getDecomposedJwt()); + const promise1 = jwksCache.getJwk(jwksUri, getDecomposedJwt()); + const promise2 = jwksCache.getJwk(jwksUri, getDecomposedJwt()); expect.assertions(2); expect(promise1).toEqual(promise2); await Promise.all([promise1, promise2]); diff --git a/tests/unit/cache.test.ts b/tests/unit/cache.test.ts new file mode 100644 index 0000000..d126e97 --- /dev/null +++ b/tests/unit/cache.test.ts @@ -0,0 +1,542 @@ +import { + SimpleLinkedList, + SimpleLruCache, +} from "../../src/cache"; + +describe("unit tests cache", () => { + + test("SimpleLinkedList with 0 element", () => { + const list = new SimpleLinkedList(); + expect(list.size).toBe(0); + expect(list.first).toBeUndefined(); + expect(list.last).toBeUndefined(); + }); + + test("SimpleLinkedList addFirst 1 element", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + + expect(list.size).toBe(1); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBeUndefined(); + + expect(list.first).toBe(node1); + expect(list.last).toBe(node1); + + }); + + test("SimpleLinkedList addFirst 2 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + + expect(list.size).toBe(2); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBe(node2); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBe(node1); + expect(node2.next).toBeUndefined(); + + expect(list.first).toBe(node2); + expect(list.last).toBe(node1); + }); + + test("SimpleLinkedList addFirst 3 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + const node3 = list.addFirst("value3"); + + expect(list.size).toBe(3); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBe(node2); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBe(node1); + expect(node2.next).toBe(node3); + + expect(node3.t).toBe("value3"); + expect(node3.prev).toBe(node2); + expect(node3.next).toBeUndefined(); + + expect(list.first).toBe(node3); + expect(list.last).toBe(node1); + + }); + + test("SimpleLinkedList removeLast with 0 elements", () => { + const list = new SimpleLinkedList(); + + const lastRemoved = list.removeLast(); + + expect(list.size).toBe(0); + expect(list.first).toBeUndefined(); + expect(list.last).toBeUndefined(); + + return expect(lastRemoved).toBeUndefined(); + + }); + + test("SimpleLinkedList removeLast with 1 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + + const lastRemoved = list.removeLast(); + + expect(list.size).toBe(0); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBeUndefined(); + + expect(list.first).toBeUndefined(); + expect(list.last).toBeUndefined(); + + return expect(lastRemoved).toBe("value1"); + + }); + + test("SimpleLinkedList removeLast with 2 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + + const lastRemoved = list.removeLast(); + + expect(list.size).toBe(1); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBeUndefined(); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBeUndefined(); + expect(node2.next).toBeUndefined(); + + expect(list.first).toBe(node2); + expect(list.last).toBe(node2); + + return expect(lastRemoved).toBe("value1"); + + }); + + test("SimpleLinkedList removeLast with 3 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + const node3 = list.addFirst("value3"); + + const lastRemoved = list.removeLast(); + + expect(list.size).toBe(2); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBeUndefined(); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBeUndefined(); + expect(node2.next).toBe(node3); + + expect(node3.t).toBe("value3"); + expect(node3.prev).toBe(node2); + expect(node3.next).toBeUndefined(); + + expect(list.first).toBe(node3); + expect(list.last).toBe(node2); + + return expect(lastRemoved).toBe("value1"); + + }); + + test("SimpleLinkedList moveFirst with 1 element", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + list.moveFirst(node1); + + expect(list.size).toBe(1); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBeUndefined(); + + expect(list.first).toBe(node1); + expect(list.last).toBe(node1); + + }); + + test("SimpleLinkedList moveFirst with 2 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + list.moveFirst(node1); + + expect(list.size).toBe(2); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBe(node2); + expect(node1.next).toBeUndefined(); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBeUndefined(); + expect(node2.next).toBe(node1); + + expect(list.first).toBe(node1); + expect(list.last).toBe(node2); + }); + + test("SimpleLinkedList moveFirst the last element with 3 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + const node3 = list.addFirst("value3"); + list.moveFirst(node1); + + expect(list.size).toBe(3); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBe(node3); + expect(node1.next).toBeUndefined(); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBeUndefined(); + expect(node2.next).toBe(node3); + + expect(node3.t).toBe("value3"); + expect(node3.prev).toBe(node2); + expect(node3.next).toBe(node1); + + expect(list.first).toBe(node1); + expect(list.last).toBe(node2); + + }); + + test("SimpleLinkedList moveFirst the second element with 3 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + const node3 = list.addFirst("value3"); + list.moveFirst(node2); + + expect(list.size).toBe(3); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBe(node3); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBe(node3); + expect(node2.next).toBeUndefined(); + + expect(node3.t).toBe("value3"); + expect(node3.prev).toBe(node1); + expect(node3.next).toBe(node2); + + expect(list.first).toBe(node2); + expect(list.last).toBe(node1); + + }); + + test("SimpleLinkedList moveFirst the first element with 3 elements", () => { + const list = new SimpleLinkedList(); + const node1 = list.addFirst("value1"); + const node2 = list.addFirst("value2"); + const node3 = list.addFirst("value3"); + list.moveFirst(node3); + + expect(list.size).toBe(3); + + expect(node1.t).toBe("value1"); + expect(node1.prev).toBeUndefined(); + expect(node1.next).toBe(node2); + + expect(node2.t).toBe("value2"); + expect(node2.prev).toBe(node1); + expect(node2.next).toBe(node3); + + expect(node3.t).toBe("value3"); + expect(node3.prev).toBe(node2); + expect(node3.next).toBeUndefined(); + + expect(list.first).toBe(node3); + expect(list.last).toBe(node1); + + }); + + + test("CacheLru get undefined", () => { + const cache = new SimpleLruCache(2); + return expect(cache.get("key1")).toBeUndefined(); + }); + + test("CacheLru get value1 with 1 element", () => { + const cache = new SimpleLruCache(3); + + const addFirst = jest.spyOn(cache.list,'addFirst'); + + cache.set("key1","value1"); + + expect(cache.size).toBe(1); + + expect(addFirst).toHaveBeenCalledWith(["key1","value1"]); + + return expect(cache.get("key1")).toBe("value1"); + }); + + test("CacheLru get value1 with 2 elements", () => { + const cache = new SimpleLruCache(3); + + const set = jest.spyOn(cache.index,'set'); + const addFirst = jest.spyOn(cache.list,'addFirst'); + + cache.set("key1","value1"); + cache.set("key2","value2"); + + expect(cache.capacity).toBe(3); + expect(cache.size).toBe(2); + + const node1 = { + t:["key1", "value1"], + } as any; + const node2 = { + t:["key2", "value2"], + } as any; + node1.next = node2; + node2.prev = node1; + + expect(set).toHaveBeenNthCalledWith(1,"key1", node1); + expect(set).toHaveBeenNthCalledWith(2,"key2", node2); + + expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); + expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + + return expect(cache.get("key1")).toBe("value1"); + }); + + + test("CacheLru get value1 with 3 elements", () => { + const cache = new SimpleLruCache(3); + + const set = jest.spyOn(cache.index,'set'); + const addFirst = jest.spyOn(cache.list,'addFirst'); + + cache.set("key1","value1"); + cache.set("key2","value2"); + cache.set("key3","value3"); + + expect(cache.capacity).toBe(3); + expect(cache.size).toBe(3); + + const node1 = { + t:["key1", "value1"], + } as any; + const node2 = { + t:["key2", "value2"], + } as any; + const node3 = { + t:["key3", "value3"], + } as any; + node1.next = node2; + node2.prev = node1; + node2.next = node3; + node3.prev = node2; + + expect(set).toHaveBeenNthCalledWith(1,"key1", node1); + expect(set).toHaveBeenNthCalledWith(2,"key2", node2); + expect(set).toHaveBeenNthCalledWith(3,"key3", node3); + + expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); + expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); + + return expect(cache.get("key1")).toBe("value1"); + }); + + + test("CacheLru get value 1 with 4 elements", () => { + const cache = new SimpleLruCache(3); + + const set = jest.spyOn(cache.index,'set'); + const addFirst = jest.spyOn(cache.list,'addFirst'); + const removeLast = jest.spyOn(cache.list,'removeLast'); + + cache.set("key1","value1"); + cache.set("key2","value2"); + cache.set("key3","value3"); + cache.set("key4","value4"); + + expect(cache.capacity).toBe(3); + expect(cache.size).toBe(3); + + const node1 = { + t:["key1", "value1"], + } as any; + const node2 = { + t:["key2", "value2"], + } as any; + const node3 = { + t:["key3", "value3"], + } as any; + const node4 = { + t:["key4", "value4"], + } as any; + node2.next = node3; + node3.prev = node2; + node3.next = node4; + node4.prev = node3; + + expect(set).toHaveBeenNthCalledWith(1,"key1", node1); + expect(set).toHaveBeenNthCalledWith(2,"key2", node2); + expect(set).toHaveBeenNthCalledWith(3,"key3", node3); + expect(set).toHaveBeenNthCalledWith(4,"key4", node4); + + expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); + expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); + expect(addFirst).toHaveBeenNthCalledWith(4, ["key4","value4"]); + + expect(removeLast).toHaveReturnedWith(["key1","value1"]); + + return expect(cache.get("key1")).toBeUndefined(); + }); + + + test("CacheLru change priority value1 with 2 elements", () => { + const cache = new SimpleLruCache(3); + + const addFirst = jest.spyOn(cache.list,'addFirst'); + const moveFirst = jest.spyOn(cache.list,'moveFirst'); + + cache.set("key1","value1"); + cache.set("key2","value2"); + + const value = cache.get("key1"); + + expect(cache.capacity).toBe(3); + expect(cache.size).toBe(2); + + const node1 = { + t:["key1", "value1"], + } as any; + const node2 = { + t:["key2", "value2"], + } as any; + node2.next = node1; + node1.prev = node2; + + expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); + expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + + expect(moveFirst).toHaveBeenCalledWith(node1); + + return expect(value).toBe("value1"); + }); + + test("CacheLru change priority value2 with 3 elements", () => { + const cache = new SimpleLruCache(3); + + const addFirst = jest.spyOn(cache.list,'addFirst'); + const moveFirst = jest.spyOn(cache.list,'moveFirst'); + + cache.set("key1","value1"); + cache.set("key2","value2"); + cache.set("key3","value3"); + + const value = cache.get("key2"); + + expect(cache.capacity).toBe(3); + expect(cache.size).toBe(3); + + const node1 = { + t:["key1", "value1"], + } as any; + const node2 = { + t:["key2", "value2"], + } as any; + const node3 = { + t:["key3", "value3"], + } as any; + node2.prev = node3; + node3.next = node2; + node3.prev = node1 + node1.next = node3; + + expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); + expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); + + expect(moveFirst).toHaveBeenCalledWith(node2); + + return expect(value).toBe("value2"); + + }); + + test("CacheLru change priority value3 with 4 elements", () => { + const cache = new SimpleLruCache(3); + + const addFirst = jest.spyOn(cache.list,'addFirst'); + const moveFirst = jest.spyOn(cache.list,'moveFirst'); + const removeLast = jest.spyOn(cache.list,'removeLast'); + + cache.set("key1","value1"); + cache.set("key2","value2"); + cache.set("key3","value3"); + cache.set("key4","value4"); + + const value = cache.get("key3"); + + expect(cache.capacity).toBe(3); + expect(cache.size).toBe(3); + + const node2 = { + t:["key2", "value2"], + } as any; + const node3 = { + t:["key3", "value3"], + } as any; + const node4 = { + t:["key4", "value4"], + } as any; + node3.prev = node4; + node4.next = node3; + node4.prev = node2; + node2.next = node4; + + expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); + expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); + expect(addFirst).toHaveBeenNthCalledWith(4, ["key4","value4"]); + + expect(moveFirst).toHaveBeenCalledWith(node3); + + expect(removeLast).toHaveReturnedWith(["key1","value1"]); + + return expect(value).toBe("value3"); + + }); + + test("CacheLru update key1", () => { + const cache = new SimpleLruCache(3); + + const addFirst = jest.spyOn(cache.list,'addFirst'); + const moveFirst = jest.spyOn(cache.list,'moveFirst'); + + cache.set("key1","value1"); + cache.set("key1","value2"); + + expect(cache.size).toBe(1); + + expect(addFirst).toHaveBeenCalledTimes(1); + expect(moveFirst).toHaveBeenCalledTimes(1); + + return expect(cache.get("key1")).toBe("value2"); + }); + +}); From 85a2a6446a52a9d45b08757dddc15a1be2a16364 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sat, 12 Oct 2024 21:29:01 +0100 Subject: [PATCH 03/12] simplify SimpleLruCache --- src/alb-v2.ts | 48 ++++- src/cache.ts | 161 +++------------ tests/unit/cache.test.ts | 431 +-------------------------------------- 3 files changed, 79 insertions(+), 561 deletions(-) diff --git a/src/alb-v2.ts b/src/alb-v2.ts index 0ee584c..941fe87 100644 --- a/src/alb-v2.ts +++ b/src/alb-v2.ts @@ -8,6 +8,8 @@ import { JwkWithKid, Jwks, JwksCache, + PenaltyBox, + SimplePenaltyBox, } from "./jwk"; import { JwtHeader, JwtPayload } from "./jwt-model"; import { Fetcher, SimpleFetcher } from "./https"; @@ -30,13 +32,16 @@ export class AlbUriError extends JwtBaseError {} export class AwsAlbJwksCache implements JwksCache { fetcher: Fetcher; + penaltyBox:PenaltyBox; private jwkCache: SimpleLruCache = new SimpleLruCache(2); constructor(props?: { fetcher?: Fetcher; + penaltyBox?: PenaltyBox; }) { this.fetcher = props?.fetcher ?? new SimpleFetcher(); + this.penaltyBox = props?.penaltyBox ?? new SimplePenaltyBox(); } @@ -51,14 +56,27 @@ export class AwsAlbJwksCache implements JwksCache { } private getKid(decomposedJwt: DecomposedJwt): string { - if (typeof decomposedJwt.header.kid !== "string") { + const kid = decomposedJwt.header.kid; + if (typeof kid !== "string" || !this.isValidAlbKid(kid)) { throw new JwtWithoutValidKidError( "JWT header does not have valid kid claim" ); } - return decomposedJwt.header.kid; + return kid; } + private isValidAlbKid(kid:string) { + // for (let i = 0; i < kid.length; i++) { + // const code = kid.charCodeAt(i); + // if (!(code > 47 && code < 58) && // 0-9 + // !(code > 64 && code < 91) && // A-Z + // !(code > 96 && code < 123)) { // a-z + // return false; + // } + // } + return true; + }; + public async getJwk( jwksUri: string, decomposedJwt: DecomposedJwt @@ -68,24 +86,32 @@ export class AwsAlbJwksCache implements JwksCache { const cacheValue = this.jwkCache.get(jwksUriWithKid); if(cacheValue){ //cache hit - if(cacheValue.jwk){ - return cacheValue.jwk; - }else{ - return cacheValue.promise; - } + return cacheValue.promise; }else{ //cache miss + + //TODO fetching with error will rotate cache + + await this.penaltyBox.wait(jwksUriWithKid, kid); + const cacheValue:CacheValue = { promise:this.fetcher .fetch(jwksUri) .then(pem =>this.pemToJwk(kid,pem)) - .then(jwk=>cacheValue.jwk = jwk) + .then(jwk=>{ + cacheValue.jwk = jwk; + this.penaltyBox.registerSuccessfulAttempt(jwksUriWithKid, kid); + return jwk; + }) + .catch(error=>{ + this.penaltyBox.registerFailedAttempt(jwksUriWithKid, kid); + throw error; + }) } - const jwkPromise = cacheValue.promise; - //TODO error and retry this.jwkCache.set(jwksUriWithKid,cacheValue); - return jwkPromise; + + return cacheValue.promise; } } diff --git a/src/cache.ts b/src/cache.ts index 52704fc..cb5afa1 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -1,120 +1,12 @@ import assert from "assert"; -interface LinkedListNode { - t:T - prev?:LinkedListNode; - next?:LinkedListNode; -} - -export class SimpleLinkedList { - - first?:LinkedListNode; - last?:LinkedListNode; - size: number = 0; - - public moveFirst(node:LinkedListNode){ - if(node !== this.first){ - this.moveNodeAfter(node,'first'); - } - } - - private removeNode(node:LinkedListNode){ - if(node === this.last){ - this.last = this.last.next; - } - - if(node === this.first){ - this.first = this.first.prev; - } - - if(node.prev){ - node.prev.next = node.next; - } - if(node.next){ - node.next.prev = node.prev; - } - - node.next = undefined; - node.prev = undefined; - - this.size--; - } - - private addAfter(t:T, after:LinkedListNode | 'first'):LinkedListNode{ - const newNode: LinkedListNode = { t }; - this.moveNodeAfter(newNode,after); - this.size++; - return newNode; - } - - private moveNodeAfter(node:LinkedListNode,after:LinkedListNode | 'first'):void{ - if(after==='first'){ - if(this.first){ - return this.moveNodeAfter(node,this.first); - }else{ - //When empty LinkedList - this.first = node; - this.last = node; - } - }else{ - - if(node === this.last){ - this.last = this.last.next; - } - - if(after === this.first){ - this.first = node; - } - - // nodePrev -- node -- nodeNext -- afterPrev --- after -- afterNext - // => - // nodePrev -- nodeNext -- afterPrev --- after -- node -- afterNext - - const afterNext = after.next; - const nodePrev = node.prev; - const nodeNext = node.next; - - if(afterNext){ - afterNext.prev = node; - } - if(nodePrev){ - nodePrev.next = node.next; - } - if(nodeNext){ - nodeNext.prev = node.prev; - } - - after.next = node; - node.next = afterNext; - node.prev = after; - } - } - - public addFirst(t:T):LinkedListNode{ - return this.addAfter(t,'first'); - } - - public removeLast():T|undefined{ - const last = this.last; - if(last){ - this.removeNode(last); - return last.t; - }else{ - return undefined; - } - } - -} - export class SimpleLruCache { - index:Map>; - list:SimpleLinkedList<[Key,Value]>; - + private index:Map; + constructor(public readonly capacity:number){ assert(capacity>0); - this.index = new Map>(); - this.list = new SimpleLinkedList<[Key,Value]>(); + this.index = new Map(); } public get size(){ @@ -122,32 +14,45 @@ export class SimpleLruCache { } public get(key:Key):Value|undefined{ - const node = this.index.get(key); - if(node){ - this.list.moveFirst(node); - return node.t[1]; + const value = this.index.get(key); + if(value){ + this.moveFirst(key,value); + + return value; }else{ return undefined; } } public set(key:Key, value:Value):this{ - const node = this.index.get(key); - if(node){ - this.list.moveFirst(node); - node.t[1] = value; - }else{ - if(this.size>=this.capacity){ - const last = this.list.removeLast(); - assert(last); - assert(this.index.delete(last[0])); - } - - const newNode = this.list.addFirst([key,value]); - this.index.set(key,newNode); + if(this.size>=this.capacity){ + this.removeLast(); } + + this.moveFirst(key,value); + return this; } + + private moveFirst(key:Key, value:Value){ + this.index.delete(key); + this.index.set(key,value); + } + + private removeLast(){ + const last = this.index.keys().next().value; + if(last){ + this.index.delete(last) + } + } + + /** + * + * @returns array ordered from the least recent to the most recent + */ + public toArray():Array<[Key,Value]>{ + return Array.from(this.index); + } } diff --git a/tests/unit/cache.test.ts b/tests/unit/cache.test.ts index d126e97..6b6f491 100644 --- a/tests/unit/cache.test.ts +++ b/tests/unit/cache.test.ts @@ -1,276 +1,9 @@ import { - SimpleLinkedList, SimpleLruCache, } from "../../src/cache"; describe("unit tests cache", () => { - test("SimpleLinkedList with 0 element", () => { - const list = new SimpleLinkedList(); - expect(list.size).toBe(0); - expect(list.first).toBeUndefined(); - expect(list.last).toBeUndefined(); - }); - - test("SimpleLinkedList addFirst 1 element", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - - expect(list.size).toBe(1); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBeUndefined(); - - expect(list.first).toBe(node1); - expect(list.last).toBe(node1); - - }); - - test("SimpleLinkedList addFirst 2 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - - expect(list.size).toBe(2); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBe(node2); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBe(node1); - expect(node2.next).toBeUndefined(); - - expect(list.first).toBe(node2); - expect(list.last).toBe(node1); - }); - - test("SimpleLinkedList addFirst 3 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - const node3 = list.addFirst("value3"); - - expect(list.size).toBe(3); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBe(node2); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBe(node1); - expect(node2.next).toBe(node3); - - expect(node3.t).toBe("value3"); - expect(node3.prev).toBe(node2); - expect(node3.next).toBeUndefined(); - - expect(list.first).toBe(node3); - expect(list.last).toBe(node1); - - }); - - test("SimpleLinkedList removeLast with 0 elements", () => { - const list = new SimpleLinkedList(); - - const lastRemoved = list.removeLast(); - - expect(list.size).toBe(0); - expect(list.first).toBeUndefined(); - expect(list.last).toBeUndefined(); - - return expect(lastRemoved).toBeUndefined(); - - }); - - test("SimpleLinkedList removeLast with 1 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - - const lastRemoved = list.removeLast(); - - expect(list.size).toBe(0); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBeUndefined(); - - expect(list.first).toBeUndefined(); - expect(list.last).toBeUndefined(); - - return expect(lastRemoved).toBe("value1"); - - }); - - test("SimpleLinkedList removeLast with 2 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - - const lastRemoved = list.removeLast(); - - expect(list.size).toBe(1); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBeUndefined(); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBeUndefined(); - expect(node2.next).toBeUndefined(); - - expect(list.first).toBe(node2); - expect(list.last).toBe(node2); - - return expect(lastRemoved).toBe("value1"); - - }); - - test("SimpleLinkedList removeLast with 3 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - const node3 = list.addFirst("value3"); - - const lastRemoved = list.removeLast(); - - expect(list.size).toBe(2); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBeUndefined(); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBeUndefined(); - expect(node2.next).toBe(node3); - - expect(node3.t).toBe("value3"); - expect(node3.prev).toBe(node2); - expect(node3.next).toBeUndefined(); - - expect(list.first).toBe(node3); - expect(list.last).toBe(node2); - - return expect(lastRemoved).toBe("value1"); - - }); - - test("SimpleLinkedList moveFirst with 1 element", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - list.moveFirst(node1); - - expect(list.size).toBe(1); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBeUndefined(); - - expect(list.first).toBe(node1); - expect(list.last).toBe(node1); - - }); - - test("SimpleLinkedList moveFirst with 2 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - list.moveFirst(node1); - - expect(list.size).toBe(2); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBe(node2); - expect(node1.next).toBeUndefined(); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBeUndefined(); - expect(node2.next).toBe(node1); - - expect(list.first).toBe(node1); - expect(list.last).toBe(node2); - }); - - test("SimpleLinkedList moveFirst the last element with 3 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - const node3 = list.addFirst("value3"); - list.moveFirst(node1); - - expect(list.size).toBe(3); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBe(node3); - expect(node1.next).toBeUndefined(); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBeUndefined(); - expect(node2.next).toBe(node3); - - expect(node3.t).toBe("value3"); - expect(node3.prev).toBe(node2); - expect(node3.next).toBe(node1); - - expect(list.first).toBe(node1); - expect(list.last).toBe(node2); - - }); - - test("SimpleLinkedList moveFirst the second element with 3 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - const node3 = list.addFirst("value3"); - list.moveFirst(node2); - - expect(list.size).toBe(3); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBe(node3); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBe(node3); - expect(node2.next).toBeUndefined(); - - expect(node3.t).toBe("value3"); - expect(node3.prev).toBe(node1); - expect(node3.next).toBe(node2); - - expect(list.first).toBe(node2); - expect(list.last).toBe(node1); - - }); - - test("SimpleLinkedList moveFirst the first element with 3 elements", () => { - const list = new SimpleLinkedList(); - const node1 = list.addFirst("value1"); - const node2 = list.addFirst("value2"); - const node3 = list.addFirst("value3"); - list.moveFirst(node3); - - expect(list.size).toBe(3); - - expect(node1.t).toBe("value1"); - expect(node1.prev).toBeUndefined(); - expect(node1.next).toBe(node2); - - expect(node2.t).toBe("value2"); - expect(node2.prev).toBe(node1); - expect(node2.next).toBe(node3); - - expect(node3.t).toBe("value3"); - expect(node3.prev).toBe(node2); - expect(node3.next).toBeUndefined(); - - expect(list.first).toBe(node3); - expect(list.last).toBe(node1); - - }); - - test("CacheLru get undefined", () => { const cache = new SimpleLruCache(2); return expect(cache.get("key1")).toBeUndefined(); @@ -278,14 +11,10 @@ describe("unit tests cache", () => { test("CacheLru get value1 with 1 element", () => { const cache = new SimpleLruCache(3); - - const addFirst = jest.spyOn(cache.list,'addFirst'); - cache.set("key1","value1"); expect(cache.size).toBe(1); - - expect(addFirst).toHaveBeenCalledWith(["key1","value1"]); + expect(cache.toArray()).toStrictEqual([["key1","value1"]]); return expect(cache.get("key1")).toBe("value1"); }); @@ -293,29 +22,12 @@ describe("unit tests cache", () => { test("CacheLru get value1 with 2 elements", () => { const cache = new SimpleLruCache(3); - const set = jest.spyOn(cache.index,'set'); - const addFirst = jest.spyOn(cache.list,'addFirst'); - cache.set("key1","value1"); cache.set("key2","value2"); expect(cache.capacity).toBe(3); expect(cache.size).toBe(2); - - const node1 = { - t:["key1", "value1"], - } as any; - const node2 = { - t:["key2", "value2"], - } as any; - node1.next = node2; - node2.prev = node1; - - expect(set).toHaveBeenNthCalledWith(1,"key1", node1); - expect(set).toHaveBeenNthCalledWith(2,"key2", node2); - - expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); - expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); + expect(cache.toArray()).toStrictEqual([["key1","value1"],["key2","value2"]]); return expect(cache.get("key1")).toBe("value1"); }); @@ -324,37 +36,13 @@ describe("unit tests cache", () => { test("CacheLru get value1 with 3 elements", () => { const cache = new SimpleLruCache(3); - const set = jest.spyOn(cache.index,'set'); - const addFirst = jest.spyOn(cache.list,'addFirst'); - cache.set("key1","value1"); cache.set("key2","value2"); cache.set("key3","value3"); expect(cache.capacity).toBe(3); expect(cache.size).toBe(3); - - const node1 = { - t:["key1", "value1"], - } as any; - const node2 = { - t:["key2", "value2"], - } as any; - const node3 = { - t:["key3", "value3"], - } as any; - node1.next = node2; - node2.prev = node1; - node2.next = node3; - node3.prev = node2; - - expect(set).toHaveBeenNthCalledWith(1,"key1", node1); - expect(set).toHaveBeenNthCalledWith(2,"key2", node2); - expect(set).toHaveBeenNthCalledWith(3,"key3", node3); - - expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); - expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); - expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); + expect(cache.toArray()).toStrictEqual([["key1","value1"],["key2","value2"],["key3","value3"]]); return expect(cache.get("key1")).toBe("value1"); }); @@ -363,10 +51,6 @@ describe("unit tests cache", () => { test("CacheLru get value 1 with 4 elements", () => { const cache = new SimpleLruCache(3); - const set = jest.spyOn(cache.index,'set'); - const addFirst = jest.spyOn(cache.list,'addFirst'); - const removeLast = jest.spyOn(cache.list,'removeLast'); - cache.set("key1","value1"); cache.set("key2","value2"); cache.set("key3","value3"); @@ -374,35 +58,7 @@ describe("unit tests cache", () => { expect(cache.capacity).toBe(3); expect(cache.size).toBe(3); - - const node1 = { - t:["key1", "value1"], - } as any; - const node2 = { - t:["key2", "value2"], - } as any; - const node3 = { - t:["key3", "value3"], - } as any; - const node4 = { - t:["key4", "value4"], - } as any; - node2.next = node3; - node3.prev = node2; - node3.next = node4; - node4.prev = node3; - - expect(set).toHaveBeenNthCalledWith(1,"key1", node1); - expect(set).toHaveBeenNthCalledWith(2,"key2", node2); - expect(set).toHaveBeenNthCalledWith(3,"key3", node3); - expect(set).toHaveBeenNthCalledWith(4,"key4", node4); - - expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); - expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); - expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); - expect(addFirst).toHaveBeenNthCalledWith(4, ["key4","value4"]); - - expect(removeLast).toHaveReturnedWith(["key1","value1"]); + expect(cache.toArray()).toStrictEqual([["key2","value2"],["key3","value3"],["key4","value4"]]); return expect(cache.get("key1")).toBeUndefined(); }); @@ -410,10 +66,7 @@ describe("unit tests cache", () => { test("CacheLru change priority value1 with 2 elements", () => { const cache = new SimpleLruCache(3); - - const addFirst = jest.spyOn(cache.list,'addFirst'); - const moveFirst = jest.spyOn(cache.list,'moveFirst'); - + cache.set("key1","value1"); cache.set("key2","value2"); @@ -421,20 +74,7 @@ describe("unit tests cache", () => { expect(cache.capacity).toBe(3); expect(cache.size).toBe(2); - - const node1 = { - t:["key1", "value1"], - } as any; - const node2 = { - t:["key2", "value2"], - } as any; - node2.next = node1; - node1.prev = node2; - - expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); - expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); - - expect(moveFirst).toHaveBeenCalledWith(node1); + expect(cache.toArray()).toStrictEqual([["key2","value2"],["key1","value1"]]); return expect(value).toBe("value1"); }); @@ -442,9 +82,6 @@ describe("unit tests cache", () => { test("CacheLru change priority value2 with 3 elements", () => { const cache = new SimpleLruCache(3); - const addFirst = jest.spyOn(cache.list,'addFirst'); - const moveFirst = jest.spyOn(cache.list,'moveFirst'); - cache.set("key1","value1"); cache.set("key2","value2"); cache.set("key3","value3"); @@ -453,26 +90,7 @@ describe("unit tests cache", () => { expect(cache.capacity).toBe(3); expect(cache.size).toBe(3); - - const node1 = { - t:["key1", "value1"], - } as any; - const node2 = { - t:["key2", "value2"], - } as any; - const node3 = { - t:["key3", "value3"], - } as any; - node2.prev = node3; - node3.next = node2; - node3.prev = node1 - node1.next = node3; - - expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); - expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); - expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); - - expect(moveFirst).toHaveBeenCalledWith(node2); + expect(cache.toArray()).toStrictEqual([["key1","value1"],["key3","value3"],["key2","value2"]]); return expect(value).toBe("value2"); @@ -481,10 +99,6 @@ describe("unit tests cache", () => { test("CacheLru change priority value3 with 4 elements", () => { const cache = new SimpleLruCache(3); - const addFirst = jest.spyOn(cache.list,'addFirst'); - const moveFirst = jest.spyOn(cache.list,'moveFirst'); - const removeLast = jest.spyOn(cache.list,'removeLast'); - cache.set("key1","value1"); cache.set("key2","value2"); cache.set("key3","value3"); @@ -494,29 +108,7 @@ describe("unit tests cache", () => { expect(cache.capacity).toBe(3); expect(cache.size).toBe(3); - - const node2 = { - t:["key2", "value2"], - } as any; - const node3 = { - t:["key3", "value3"], - } as any; - const node4 = { - t:["key4", "value4"], - } as any; - node3.prev = node4; - node4.next = node3; - node4.prev = node2; - node2.next = node4; - - expect(addFirst).toHaveBeenNthCalledWith(1, ["key1","value1"]); - expect(addFirst).toHaveBeenNthCalledWith(2, ["key2","value2"]); - expect(addFirst).toHaveBeenNthCalledWith(3, ["key3","value3"]); - expect(addFirst).toHaveBeenNthCalledWith(4, ["key4","value4"]); - - expect(moveFirst).toHaveBeenCalledWith(node3); - - expect(removeLast).toHaveReturnedWith(["key1","value1"]); + expect(cache.toArray()).toStrictEqual([["key2","value2"],["key4","value4"],["key3","value3"]]); return expect(value).toBe("value3"); @@ -525,16 +117,11 @@ describe("unit tests cache", () => { test("CacheLru update key1", () => { const cache = new SimpleLruCache(3); - const addFirst = jest.spyOn(cache.list,'addFirst'); - const moveFirst = jest.spyOn(cache.list,'moveFirst'); - cache.set("key1","value1"); cache.set("key1","value2"); expect(cache.size).toBe(1); - - expect(addFirst).toHaveBeenCalledTimes(1); - expect(moveFirst).toHaveBeenCalledTimes(1); + expect(cache.toArray()).toStrictEqual([["key1","value2"]]); return expect(cache.get("key1")).toBe("value2"); }); From 56c70695ec297efe98ca1c385ae31d05207e0cee Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sat, 12 Oct 2024 21:45:04 +0100 Subject: [PATCH 04/12] change alb-v2 cache logic=> Temporary Jwk fetcher cache + set Jwk in LRU cache only if fetch is OK, otherwise create penalty --- src/alb-v2.ts | 72 +++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/src/alb-v2.ts b/src/alb-v2.ts index 941fe87..6d699fd 100644 --- a/src/alb-v2.ts +++ b/src/alb-v2.ts @@ -22,11 +22,6 @@ interface DecomposedJwt { type JwksUri = string; -type CacheValue = { - jwk?:JwkWithKid; - promise:Promise; -} - export class AlbUriError extends JwtBaseError {} export class AwsAlbJwksCache implements JwksCache { @@ -34,7 +29,8 @@ export class AwsAlbJwksCache implements JwksCache { fetcher: Fetcher; penaltyBox:PenaltyBox; - private jwkCache: SimpleLruCache = new SimpleLruCache(2); + private jwkCache: SimpleLruCache = new SimpleLruCache(2); + private fetchingJwks: Map> = new Map(); constructor(props?: { fetcher?: Fetcher; @@ -44,7 +40,6 @@ export class AwsAlbJwksCache implements JwksCache { this.penaltyBox = props?.penaltyBox ?? new SimplePenaltyBox(); } - /** * * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} @@ -83,35 +78,37 @@ export class AwsAlbJwksCache implements JwksCache { ): Promise { const kid = this.getKid(decomposedJwt); const jwksUriWithKid = this.expandWithKid(jwksUri, kid); - const cacheValue = this.jwkCache.get(jwksUriWithKid); - if(cacheValue){ + const jwk = this.jwkCache.get(jwksUriWithKid); + if(jwk){ //cache hit - return cacheValue.promise; + return jwk; }else{ //cache miss - - //TODO fetching with error will rotate cache - - await this.penaltyBox.wait(jwksUriWithKid, kid); - - const cacheValue:CacheValue = { - promise:this.fetcher - .fetch(jwksUri) - .then(pem =>this.pemToJwk(kid,pem)) - .then(jwk=>{ - cacheValue.jwk = jwk; - this.penaltyBox.registerSuccessfulAttempt(jwksUriWithKid, kid); - return jwk; - }) - .catch(error=>{ - this.penaltyBox.registerFailedAttempt(jwksUriWithKid, kid); - throw error; - }) + const fetchPromise = this.fetchingJwks.get(jwksUriWithKid); + if(fetchPromise){ + return fetchPromise; + }else{ + await this.penaltyBox.wait(jwksUriWithKid, kid); + + const newFetchPromise = this.fetcher + .fetch(jwksUri) + .then(pem =>this.pemToJwk(kid,pem)) + .then(jwk=>{ + this.penaltyBox.registerSuccessfulAttempt(jwksUriWithKid, kid); + this.jwkCache.set(jwksUriWithKid,jwk); + return jwk; + }) + .catch(error=>{ + this.penaltyBox.registerFailedAttempt(jwksUriWithKid, kid); + throw error; + }).finally(()=>{ + this.fetchingJwks.delete(jwksUriWithKid); + }); + + this.fetchingJwks.set(jwksUriWithKid,newFetchPromise) + + return newFetchPromise; } - - this.jwkCache.set(jwksUriWithKid,cacheValue); - - return cacheValue.promise; } } @@ -141,9 +138,9 @@ export class AwsAlbJwksCache implements JwksCache { ): JwkWithKid { const kid = this.getKid(decomposedJwt); const jwksUriWithKid = this.expandWithKid(jwksUri, kid); - const cacheValue = this.jwkCache.get(jwksUriWithKid); - if(cacheValue?.jwk){ - return cacheValue.jwk; + const jwk = this.jwkCache.get(jwksUriWithKid); + if(jwk){ + return jwk; }else{ throw new JwksNotAvailableInCacheError( `JWKS for uri ${jwksUri} not yet available in cache` @@ -158,10 +155,7 @@ export class AwsAlbJwksCache implements JwksCache { const jwkWithKid = jwk as JwkWithKid; const kid = jwk.kid; const jwksUriWithKid = this.expandWithKid(jwksUri, kid); - this.jwkCache.set(jwksUriWithKid,{ - jwk:jwkWithKid, - promise:Promise.resolve(jwkWithKid) - }); + this.jwkCache.set(jwksUriWithKid,jwkWithKid); }else{ throw new Error("TODO"); } From f10e3e627d9896c25887eac20b5ff905d3794353 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sat, 19 Oct 2024 15:27:47 +0100 Subject: [PATCH 05/12] first version alb verifier --- src/alb-v2.ts | 18 +++-- src/alb-verifier.ts | 121 ++++++++++++++++++++++++++++++++ tests/unit/alb-v2.test.ts | 51 ++++++++++++++ tests/unit/alb-verifier.test.ts | 119 +++++++++++++++++++++++++++++++ 4 files changed, 299 insertions(+), 10 deletions(-) create mode 100644 src/alb-verifier.ts create mode 100644 tests/unit/alb-v2.test.ts create mode 100644 tests/unit/alb-verifier.test.ts diff --git a/src/alb-v2.ts b/src/alb-v2.ts index 6d699fd..f52c464 100644 --- a/src/alb-v2.ts +++ b/src/alb-v2.ts @@ -8,8 +8,6 @@ import { JwkWithKid, Jwks, JwksCache, - PenaltyBox, - SimplePenaltyBox, } from "./jwk"; import { JwtHeader, JwtPayload } from "./jwt-model"; import { Fetcher, SimpleFetcher } from "./https"; @@ -24,20 +22,21 @@ type JwksUri = string; export class AlbUriError extends JwtBaseError {} +//TODO comment importance of safe architecture export class AwsAlbJwksCache implements JwksCache { fetcher: Fetcher; - penaltyBox:PenaltyBox; + // penaltyBox:PenaltyBox; private jwkCache: SimpleLruCache = new SimpleLruCache(2); private fetchingJwks: Map> = new Map(); constructor(props?: { fetcher?: Fetcher; - penaltyBox?: PenaltyBox; + // penaltyBox?: PenaltyBox; }) { this.fetcher = props?.fetcher ?? new SimpleFetcher(); - this.penaltyBox = props?.penaltyBox ?? new SimplePenaltyBox(); + // this.penaltyBox = props?.penaltyBox ?? new SimplePenaltyBox(); } /** @@ -88,18 +87,17 @@ export class AwsAlbJwksCache implements JwksCache { if(fetchPromise){ return fetchPromise; }else{ - await this.penaltyBox.wait(jwksUriWithKid, kid); - + // await this.penaltyBox.wait(jwksUriWithKid, kid); const newFetchPromise = this.fetcher - .fetch(jwksUri) + .fetch(jwksUriWithKid) .then(pem =>this.pemToJwk(kid,pem)) .then(jwk=>{ - this.penaltyBox.registerSuccessfulAttempt(jwksUriWithKid, kid); + // this.penaltyBox.registerSuccessfulAttempt(jwksUriWithKid, kid); this.jwkCache.set(jwksUriWithKid,jwk); return jwk; }) .catch(error=>{ - this.penaltyBox.registerFailedAttempt(jwksUriWithKid, kid); + // this.penaltyBox.registerFailedAttempt(jwksUriWithKid, kid); throw error; }).finally(()=>{ this.fetchingJwks.delete(jwksUriWithKid); diff --git a/src/alb-verifier.ts b/src/alb-verifier.ts new file mode 100644 index 0000000..1cc764a --- /dev/null +++ b/src/alb-verifier.ts @@ -0,0 +1,121 @@ +import { AwsAlbJwksCache } from "./alb-v2"; +import { JwksCache } from "./jwk"; +import { JwtVerifierBase, JwtVerifierProperties } from "./jwt-verifier"; +import { Properties } from "./typing-util"; + +export interface AlbVerifyProperties { + + /** + * The client ID that you expect to be present on the JWT + * (In the ID token's aud claim, or the Access token's client_id claim). + * If you provide a string array, that means at least one of those client IDs + * must be present on the JWT. + * Pass null explicitly to not check the JWT's client ID--if you know what you're doing + */ + clientId: string | string[] | null; + + loadBalancerArn: string; + + jwksUri?: string; + issuer: string; + +} + + +/** Type for Cognito JWT verifier properties, for a single User Pool */ +export type AlbJwtVerifierProperties = { + +} & Partial; + +/** + * Type for Cognito JWT verifier properties, when multiple User Pools are used in the verifier. + * In this case, you should be explicit in mapping `clientId` to User Pool. + */ +export type AlbJwtVerifierMultiProperties = { + +} & AlbVerifyProperties; + + +/** + * TODO rename + */ +export type AlbJwtVerifierSingleUserPool< + T extends AlbJwtVerifierProperties, +> = AlbJwtVerifier< + Properties, + T & + JwtVerifierProperties & { + audience: null; + }, + false +>; + +/** + * TODO rename + */ +export type AlbJwtVerifierMultiUserPool< + T extends AlbJwtVerifierMultiProperties, +> = AlbJwtVerifier< + Properties, + T & + JwtVerifierProperties & { + audience: null; + }, + true +>; + + +export class AlbJwtVerifier< + SpecificVerifyProperties extends Partial, + IssuerConfig extends JwtVerifierProperties & { + audience: null; + }, + MultiIssuer extends boolean, +> extends JwtVerifierBase { + + private constructor( + props: AlbJwtVerifierProperties | AlbJwtVerifierMultiProperties[], + ) { + const issuerConfig = Array.isArray(props) + ? (props.map((p) => ({ + ...p, + audience: null, // checked instead by validateCognitoJwtFields + })) as IssuerConfig[]) + : ({ + ...props, + audience: null, // checked instead by validateCognitoJwtFields + } as IssuerConfig); + super(issuerConfig, new AwsAlbJwksCache()); + } + + + /** + * Create a Cognito JWT verifier for a single User Pool + * + * @param verifyProperties The verification properties for your User Pool + * @param additionalProperties Additional properties + * @param additionalProperties.jwksCache Overriding JWKS cache that you want to use + * @returns An Cognito JWT Verifier instance, that you can use to verify Cognito signed JWTs with + */ + static create( + verifyProperties: T & Partial, + additionalProperties?: { jwksCache: JwksCache } + ): AlbJwtVerifierSingleUserPool; + + + static create( + props: (T & Partial)[], + additionalProperties?: { jwksCache: JwksCache } + ): AlbJwtVerifierMultiUserPool; + + + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + static create( + verifyProperties: + | AlbJwtVerifierProperties + | AlbJwtVerifierMultiProperties[] + ) { + return new this(verifyProperties); + } + +} \ No newline at end of file diff --git a/tests/unit/alb-v2.test.ts b/tests/unit/alb-v2.test.ts new file mode 100644 index 0000000..601eb8e --- /dev/null +++ b/tests/unit/alb-v2.test.ts @@ -0,0 +1,51 @@ +import { AwsAlbJwksCache } from "../../src/alb-v2"; +import { + allowAllRealNetworkTraffic, + disallowAllRealNetworkTraffic, + mockHttpsUri, +} from "./test-util"; +import { readFileSync } from "fs"; +import { join } from "path"; + +describe("alb", () => { + const kid = "abcdefgh-1234-ijkl-5678-mnopqrstuvwx"; + const jwksUri = "https://public-keys.auth.elb.eu-west-1.amazonaws.com"; + + const albResponse = readFileSync(join(__dirname, "alb-jwks-test.pem")); + + const decomposedJwt ={ + header: { + alg: "EC256", + kid + }, + payload: {}, + }; + + beforeAll(() => { + disallowAllRealNetworkTraffic(); + }); + afterAll(() => { + allowAllRealNetworkTraffic(); + }); + + test("ALB JWKS cache fetches URI one attempt at a time", async () => { + /** + * Test what happens when the the JWKS URI is requested multiple times in parallel + * (e.g. in parallel promises). When this happens only 1 actual HTTPS request should + * be made to the JWKS URI. + */ + + mockHttpsUri(`${jwksUri}/${kid}`, { + responsePayload: albResponse, + }); + + const jwksCache = new AwsAlbJwksCache(); + + const promise1 = jwksCache.getJwk(jwksUri, decomposedJwt); + const promise2 = jwksCache.getJwk(jwksUri, decomposedJwt); + expect.assertions(2); + expect(promise1).toEqual(promise2); + await Promise.all([promise1, promise2]); + }); + +}); diff --git a/tests/unit/alb-verifier.test.ts b/tests/unit/alb-verifier.test.ts new file mode 100644 index 0000000..0e96760 --- /dev/null +++ b/tests/unit/alb-verifier.test.ts @@ -0,0 +1,119 @@ +import { + generateKeyPair, + signJwt, + allowAllRealNetworkTraffic, + disallowAllRealNetworkTraffic, + mockHttpsUri, +} from "./test-util"; +import { AlbJwtVerifier } from "../../src/alb-verifier"; +import { createPublicKey } from "crypto"; + +describe("unit tests alb verifier", () => { + let keypair: ReturnType; + beforeAll(() => { + keypair = generateKeyPair(); + disallowAllRealNetworkTraffic(); + }); + afterAll(() => { + allowAllRealNetworkTraffic(); + }); + + describe("AlbJwtVerifier", () => { + describe("verify", () => { + test("happy flow with cached public key", async () => { + + jest.useFakeTimers().setSystemTime(new Date(1727070000 * 1000)); + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const kid = keypair.jwk.kid; + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"RS256",//ES256. + iss:issuer, + client:clientId, + signer:loadBalancerArn, + exp:1727080000 + }, + { + hello: "world", + exp:1727080000, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbJwtVerifier.create({ + issuer, + clientId, + loadBalancerArn, + jwksUri + }); + albVerifier.cacheJwks(keypair.jwks); + expect.assertions(1); + expect( + await albVerifier.verify(signedJwt) + ).toMatchObject({ hello: "world" }); + }); + + test("happy flow with public key fetching", async () => { + + jest.useFakeTimers().setSystemTime(new Date(1727070000 * 1000)); + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const jwk = keypair.jwk; + const kid = jwk.kid; + + const pem = createPublicKey({ + key: jwk, + format: "jwk", + }).export({ + format: "pem", + type: "spki", + }); + + mockHttpsUri(`${jwksUri}/${kid}`, { + responsePayload: pem, + }); + + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"RS256",//ES256. + iss:issuer, + client:clientId, + signer:loadBalancerArn, + exp:1727080000 + }, + { + hello: "world", + exp:1727080000, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbJwtVerifier.create({ + issuer, + clientId, + loadBalancerArn, + jwksUri + }); + expect.assertions(1); + expect( + await albVerifier.verify(signedJwt) + ).toMatchObject({ hello: "world" }); + }); + }); + }); +}); From bfef4a68c070502f05f22fdf7001fa2032630da9 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sat, 19 Oct 2024 16:37:11 +0100 Subject: [PATCH 06/12] Fix unit tests. nock and jest.useFakeTimer not compatible --- src/alb-v2.ts | 12 ++++++------ tests/unit/alb-v1.test.ts | 7 ++----- tests/unit/alb-v2.test.ts | 4 ++-- tests/unit/alb-verifier.test.ts | 19 +++++++++---------- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/alb-v2.ts b/src/alb-v2.ts index f52c464..88c3849 100644 --- a/src/alb-v2.ts +++ b/src/alb-v2.ts @@ -39,12 +39,6 @@ export class AwsAlbJwksCache implements JwksCache { // this.penaltyBox = props?.penaltyBox ?? new SimplePenaltyBox(); } - /** - * - * @param jwksUri should be a template URI with the kid expression. Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com/{kid} - * @param decomposedJwt - * @returns - */ private expandWithKid(jwksUri: string, kid: string): string { return `${jwksUri}/${encodeURIComponent(kid)}`; } @@ -130,6 +124,12 @@ export class AwsAlbJwksCache implements JwksCache { } as JwkWithKid } + /** + * + * @param Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com + * @param decomposedJwt + * @returns + */ public getCachedJwk( jwksUri: string, decomposedJwt: DecomposedJwt diff --git a/tests/unit/alb-v1.test.ts b/tests/unit/alb-v1.test.ts index f5b32f3..d168ed0 100644 --- a/tests/unit/alb-v1.test.ts +++ b/tests/unit/alb-v1.test.ts @@ -24,12 +24,9 @@ describe("unit tests https", () => { y: "oBuN90bW-AvxscoesVaE7ryPISjqseKgio6H5ZO5xmk", crv: "P-256", }; - const jwks = { - keys: [jwk], - }; const getDecomposedJwt = (kidParam?: string) => ({ header: { - alg: "EC256", + alg: "ES256", kid: kidParam ?? kid, }, payload: {}, @@ -46,7 +43,7 @@ describe("unit tests https", () => { const jwksCache = new AwsAlbJwksCache(); expect.assertions(1); return expect( - jwksCache.getJwk(jwksUri, { header: { alg: "EC256" }, payload: {} }) + jwksCache.getJwk(jwksUri, { header: { alg: "ES256" }, payload: {} }) ).rejects.toThrow(JwtWithoutValidKidError); }); diff --git a/tests/unit/alb-v2.test.ts b/tests/unit/alb-v2.test.ts index 601eb8e..2a1e14e 100644 --- a/tests/unit/alb-v2.test.ts +++ b/tests/unit/alb-v2.test.ts @@ -15,7 +15,7 @@ describe("alb", () => { const decomposedJwt ={ header: { - alg: "EC256", + alg: "ES256", kid }, payload: {}, @@ -43,7 +43,7 @@ describe("alb", () => { const promise1 = jwksCache.getJwk(jwksUri, decomposedJwt); const promise2 = jwksCache.getJwk(jwksUri, decomposedJwt); - expect.assertions(2); + expect.assertions(1); expect(promise1).toEqual(promise2); await Promise.all([promise1, promise2]); }); diff --git a/tests/unit/alb-verifier.test.ts b/tests/unit/alb-verifier.test.ts index 0e96760..3f21123 100644 --- a/tests/unit/alb-verifier.test.ts +++ b/tests/unit/alb-verifier.test.ts @@ -22,8 +22,6 @@ describe("unit tests alb verifier", () => { describe("verify", () => { test("happy flow with cached public key", async () => { - jest.useFakeTimers().setSystemTime(new Date(1727070000 * 1000)); - const region = "us-east-1"; const userPoolId = "us-east-1_123456"; const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; @@ -31,6 +29,8 @@ describe("unit tests alb verifier", () => { const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; const kid = keypair.jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + const signedJwt = signJwt( { typ:"JWT", @@ -39,11 +39,11 @@ describe("unit tests alb verifier", () => { iss:issuer, client:clientId, signer:loadBalancerArn, - exp:1727080000 + exp }, { hello: "world", - exp:1727080000, + exp, iss:issuer, }, keypair.privateKey @@ -63,8 +63,6 @@ describe("unit tests alb verifier", () => { test("happy flow with public key fetching", async () => { - jest.useFakeTimers().setSystemTime(new Date(1727070000 * 1000)); - const region = "us-east-1"; const userPoolId = "us-east-1_123456"; const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; @@ -73,14 +71,15 @@ describe("unit tests alb verifier", () => { const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; const jwk = keypair.jwk; const kid = jwk.kid; - + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead const pem = createPublicKey({ key: jwk, format: "jwk", }).export({ format: "pem", type: "spki", - }); + });//pem with -----BEGIN PUBLIC KEY----- and -----END PUBLIC KEY-----. + mockHttpsUri(`${jwksUri}/${kid}`, { responsePayload: pem, @@ -94,11 +93,11 @@ describe("unit tests alb verifier", () => { iss:issuer, client:clientId, signer:loadBalancerArn, - exp:1727080000 + exp }, { hello: "world", - exp:1727080000, + exp, iss:issuer, }, keypair.privateKey From a61f8196d1a384fef3a19708881b4d7619bd051a Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sun, 27 Oct 2024 08:31:26 +0000 Subject: [PATCH 07/12] add alb verifier v1 (dev from scratch) et v2 (dev from JwtVerifier) --- src/alb-verifier-v1.ts | 295 ++++++++++++++++++ src/alb-verifier-v2.ts | 210 +++++++++++++ src/alb-verifier.ts | 121 ------- src/jwt-verifier.ts | 4 +- ...rifier.test.ts => alb-verifier-v1.test.ts} | 4 +- 5 files changed, 509 insertions(+), 125 deletions(-) create mode 100644 src/alb-verifier-v1.ts create mode 100644 src/alb-verifier-v2.ts delete mode 100644 src/alb-verifier.ts rename tests/unit/{alb-verifier.test.ts => alb-verifier-v1.test.ts} (97%) diff --git a/src/alb-verifier-v1.ts b/src/alb-verifier-v1.ts new file mode 100644 index 0000000..92cfcfb --- /dev/null +++ b/src/alb-verifier-v1.ts @@ -0,0 +1,295 @@ +import { AwsAlbJwksCache } from "./alb-v2"; +import { assertStringArrayContainsString } from "./assert"; +import { JwtInvalidClaimError, ParameterValidationError } from "./error"; +import { Jwk, Jwks, JwksCache } from "./jwk"; +import { DecomposedJwt, decomposeUnverifiedJwt } from "./jwt"; +import { JwtHeader, JwtPayload } from "./jwt-model"; +import { KeyObjectCache, verifyDecomposedJwt, verifyDecomposedJwtSync } from "./jwt-verifier"; +import { JsonObject } from "./safe-json-parse"; +import { Properties } from "./typing-util"; + +type LoadBalancerArn = string; + +export class JwtInvalidSignerError extends JwtInvalidClaimError {} + +export interface AlbVerifyProperties { + + /** + * The client ID that you expect to be present on the JWT + * (In the ID token's aud claim, or the Access token's client_id claim). + * If you provide a string array, that means at least one of those client IDs + * must be present on the JWT. + * Pass null explicitly to not check the JWT's client ID--if you know what you're doing + */ + clientId: string | string[] | null; + + loadBalancerArn: string; + + jwksUri?: string; + issuer: string; + + /** + * If you want to peek inside the invalid JWT when verification fails, set `includeRawJwtInErrors` to true. + * Then, if an error is thrown during verification of the invalid JWT (e.g. the JWT is invalid because it is expired), + * the Error object will include a property `rawJwt`, with the raw decoded contents of the **invalid** JWT. + * The `rawJwt` will only be included in the Error object, if the JWT's signature can at least be verified. + */ + includeRawJwtInErrors?: boolean; + +} + +/** Type for ALB JWT verifier properties, for a single ALB */ +export type AlbJwtVerifierProperties = { + + loadBalancerArn: string; + +} & Partial; + +/** + * Type for ALB JWT verifier properties, when multiple ALB are used in the verifier. + */ +export type AlbJwtVerifierMultiProperties = { + + loadBalancerArn: string; + +} & AlbVerifyProperties; + +export type AlbVerifierProperties = { + + loadBalancerArn: string; + + +} & Partial; + +export type AlbJwtVerifierSingleAlb< +T extends AlbJwtVerifierProperties, +> = AlbJwtVerifier< + Properties, + false +>; + +export type AlbJwtVerifierMultiAlb< +T extends AlbJwtVerifierProperties, +> = AlbJwtVerifier< + Properties, + true +>; + +type AlbVerifyParameters = { + [key: string]: never; +} extends SpecificVerifyProperties + ? [jwt: string, props?: SpecificVerifyProperties] + : [jwt: string, props: SpecificVerifyProperties]; + + +type DataTokenPayload = { + exp:number + iss:string, +} & JsonObject; + + +/** + * TODO rename JWT by Data + * Differences between this and the Cognito JWT verifier: + * 1. The issuer is the load balancer arn/Signer + * 2. The JWT validation is different + * 3. Hydrate is not supported + * + * Pro: No hydratemethod + * Con: Code duplicated + * + * Con v2 dont inforce loadBalancerArn not null + */ +export class AlbJwtVerifier< + SpecificVerifyProperties extends Partial, + MultiAlb extends boolean, +> { + + private readonly albConfigMap: Map = new Map(); + private readonly publicKeyCache = new KeyObjectCache(); + private readonly jwksCache: JwksCache = new AwsAlbJwksCache(); + private readonly defaultJwksUri; + + private constructor( + props: AlbJwtVerifierProperties | AlbJwtVerifierMultiProperties[], + ) { + if(Array.isArray(props)){ + if (!props.length) { + throw new ParameterValidationError( + "Provide at least one alb configuration" + ); + } + for (const albProps of props) { + if (this.albConfigMap.has(albProps.loadBalancerArn)) { + throw new ParameterValidationError( + `loadBalancerArn ${albProps.loadBalancerArn} supplied multiple times` + ); + } + this.albConfigMap.set(albProps.loadBalancerArn, albProps); + } + }else { + this.albConfigMap.set(props.loadBalancerArn, props); + } + const region = "us-east-1";//TODO extract region + this.defaultJwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + } + + static create( + verifyProperties: T & Partial + ): AlbJwtVerifierSingleAlb; + + + static create( + props: (T & Partial)[] + ): AlbJwtVerifierMultiAlb; + + static create( + verifyProperties: + | AlbJwtVerifierProperties + | AlbJwtVerifierMultiProperties[] + ) { + return new this(verifyProperties); + } + + public async verify( + ...[jwt, properties]: AlbVerifyParameters): Promise{ + const { decomposedJwt, jwksUri, verifyProperties } = this.getVerifyParameters(jwt, properties); + await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties); + try { + this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); + } catch (err) { + if ( + verifyProperties.includeRawJwtInErrors && + err instanceof JwtInvalidClaimError + ) { + throw err.withRawJwt(decomposedJwt); + } + throw err; + } + return decomposedJwt.payload as DataTokenPayload; + } + + public verifySync( ...[jwt, properties]: AlbVerifyParameters): DataTokenPayload { + const { decomposedJwt, jwksUri, verifyProperties } = this.getVerifyParameters(jwt, properties); + this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties); + try { + this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); + } catch (err) { + if ( + verifyProperties.includeRawJwtInErrors && + err instanceof JwtInvalidClaimError + ) { + throw err.withRawJwt(decomposedJwt); + } + throw err; + } + return decomposedJwt.payload as DataTokenPayload; + } + + protected getVerifyParameters( + jwt: string, + verifyProperties?: Partial + ): { + decomposedJwt: DecomposedJwt; + jwksUri: string, + verifyProperties: AlbJwtVerifierProperties; + } { + const decomposedJwt = decomposeUnverifiedJwt(jwt); + assertStringArrayContainsString( + "Signer", + decomposedJwt.header.signer, + this.expectedLoadBalancerArn, + JwtInvalidSignerError + ); + const albConfig = this.getAlbConfig(decomposedJwt.header.signer); + return { + decomposedJwt, + jwksUri: verifyProperties?.jwksUri ?? this.defaultJwksUri, + verifyProperties: { + ...albConfig, + ...verifyProperties, + } as unknown as AlbJwtVerifierProperties, + }; + } + + private validateDataJwtFields( + header:JwtHeader, + payload: JwtPayload, + options: { + clientId?: string | string[] | null; + } + ): void { + //TODO check client header, signer header, iss payload + } + + public cacheJwks( + ...[jwks, loadBalancerArn]: MultiAlb extends false + ? [jwks: Jwks, loadBalancerArn?: string] + : [jwks: Jwks, loadBalancerArn: string] + ): void { + // const albConfig = this.getAlbConfig(loadBalancerArn); + // this.jwksCache.addJwks(albConfig.jwksUri, jwks); + // this.publicKeyCache.clearCache(albConfig.issuer); + //TODO + } + + //Duplicate from JwtVerifier + protected getAlbConfig( + loadBalancerArn?: string + ): AlbJwtVerifierProperties { + if (!loadBalancerArn) { + if (this.albConfigMap.size !== 1) { + throw new ParameterValidationError("loadBalancerArn must be provided"); + } + loadBalancerArn = this.albConfigMap.keys().next().value; + } + const config = this.albConfigMap.get(loadBalancerArn!); + if (!config) { + throw new ParameterValidationError(`loadBalancerArn not configured: ${loadBalancerArn}`); + } + return config; + } + + //Duplicate from JwtVerifier + protected get expectedLoadBalancerArn(): string[] { + return Array.from(this.albConfigMap.keys()); + } + + //Duplicate from JwtVerifier + protected verifyDecomposedJwt( + decomposedJwt: DecomposedJwt, + jwksUri: string, + verifyProperties: AlbJwtVerifierProperties + ): Promise { + return verifyDecomposedJwt( + decomposedJwt, + jwksUri, + { + includeRawJwtInErrors: verifyProperties.includeRawJwtInErrors, + issuer: verifyProperties.issuer, + audience:null + }, + this.jwksCache.getJwk.bind(this.jwksCache), + this.publicKeyCache.transformJwkToKeyObjectAsync.bind(this.publicKeyCache) + ); + } + + //Duplicate from JwtVerifier + protected verifyDecomposedJwtSync( + decomposedJwt: DecomposedJwt, + jwksUri: string, + verifyProperties: AlbJwtVerifierProperties + ): JwtPayload { + const jwk = this.jwksCache.getCachedJwk(jwksUri, decomposedJwt); + return verifyDecomposedJwtSync( + decomposedJwt, + jwk, + { + includeRawJwtInErrors: verifyProperties.includeRawJwtInErrors, + issuer: verifyProperties.issuer, + audience:null + }, + this.publicKeyCache.transformJwkToKeyObjectSync.bind(this.publicKeyCache) + ); + } +} diff --git a/src/alb-verifier-v2.ts b/src/alb-verifier-v2.ts new file mode 100644 index 0000000..deaec0a --- /dev/null +++ b/src/alb-verifier-v2.ts @@ -0,0 +1,210 @@ +import { AwsAlbJwksCache } from "./alb-v2"; +import { assertStringArrayContainsString } from "./assert"; +import { JwtInvalidClaimError } from "./error"; +import { JwksCache } from "./jwk"; +import { DecomposedJwt, decomposeUnverifiedJwt } from "./jwt"; +import { JwtHeader, JwtPayload } from "./jwt-model"; +import { JwtVerifierBase, JwtVerifierProperties } from "./jwt-verifier"; +import { JsonObject } from "./safe-json-parse"; +import { Properties } from "./typing-util"; + +export class JwtInvalidSignerError extends JwtInvalidClaimError {} + +export interface AlbVerifyProperties { + + /** + * The client ID that you expect to be present on the JWT + * (In the ID token's aud claim, or the Access token's client_id claim). + * If you provide a string array, that means at least one of those client IDs + * must be present on the JWT. + * Pass null explicitly to not check the JWT's client ID--if you know what you're doing + */ + clientId: string | string[] | null; + + loadBalancerArn: string; + + jwksUri?: string; + issuer: string; + + /** + * If you want to peek inside the invalid JWT when verification fails, set `includeRawJwtInErrors` to true. + * Then, if an error is thrown during verification of the invalid JWT (e.g. the JWT is invalid because it is expired), + * the Error object will include a property `rawJwt`, with the raw decoded contents of the **invalid** JWT. + * The `rawJwt` will only be included in the Error object, if the JWT's signature can at least be verified. + */ + includeRawJwtInErrors?: boolean; + +} + + +/** Type for Alb JWT verifier properties, for a single Alb */ +export type AlbJwtVerifierProperties = { + +} & Partial; + +/** + * Type for Alb JWT verifier properties, when multiple Alb are used in the verifier. + */ +export type AlbJwtVerifierMultiProperties = { + +} & AlbVerifyProperties; + +export type AlbJwtVerifierSingleAlb< + T extends AlbJwtVerifierProperties, +> = AlbJwtVerifier< + Properties, + T & + JwtVerifierProperties & { + userPoolId: string; + audience: null; + }, + false +>; + +export type AlbJwtVerifierMultiAlb< + T extends AlbJwtVerifierMultiProperties, +> = AlbJwtVerifier< + Properties, + T & + JwtVerifierProperties & { + userPoolId: string; + audience: null; + }, + true +>; + +type AlbVerifyParameters = { + [key: string]: never; +} extends SpecificVerifyProperties + ? [jwt: string, props?: SpecificVerifyProperties] + : [jwt: string, props: SpecificVerifyProperties]; + +type DataTokenPayload = { + exp:number + iss:string, +} & JsonObject; + +export class AlbJwtVerifier< + SpecificVerifyProperties extends Partial, + IssuerConfig extends JwtVerifierProperties & { + audience: null; + }, + MultiIssuer extends boolean, +> extends JwtVerifierBase { + + private readonly defaultJwksUri; + + private constructor( + props: AlbJwtVerifierProperties | AlbJwtVerifierMultiProperties[], + ) { + const issuerConfig = Array.isArray(props) + ? (props.map((p) => ({ + ...p, + audience: null, // checked instead by validateCognitoJwtFields + })) as IssuerConfig[]) + : ({ + ...props, + audience: null, // checked instead by validateCognitoJwtFields + } as IssuerConfig); + super(issuerConfig, new AwsAlbJwksCache()); + const region = "us-east-1";//TODO extract region + this.defaultJwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + } + + static create( + verifyProperties: T & Partial, + additionalProperties?: { jwksCache: JwksCache } + ): AlbJwtVerifierSingleAlb; + + + static create( + props: (T & Partial)[], + additionalProperties?: { jwksCache: JwksCache } + ): AlbJwtVerifierMultiAlb; + + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + static create( + verifyProperties: + | AlbJwtVerifierProperties + | AlbJwtVerifierMultiProperties[] + ) { + return new this(verifyProperties); + } + + public verifySync( + ...[jwt, properties]: AlbVerifyParameters + ): DataTokenPayload { + const { decomposedJwt, jwksUri, verifyProperties } = + this.getVerifyParameters(jwt, properties); + this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties); + try { + this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); + } catch (err) { + if ( + verifyProperties.includeRawJwtInErrors && + err instanceof JwtInvalidClaimError + ) { + throw err.withRawJwt(decomposedJwt); + } + throw err; + } + return decomposedJwt.payload as DataTokenPayload; + } + + public async verify( + ...[jwt, properties]: AlbVerifyParameters + ): Promise { + const { decomposedJwt, jwksUri, verifyProperties } = + this.getVerifyParameters(jwt, properties); + await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties); + try { + this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); + } catch (err) { + if ( + verifyProperties.includeRawJwtInErrors && + err instanceof JwtInvalidClaimError + ) { + throw err.withRawJwt(decomposedJwt); + } + throw err; + } + return decomposedJwt.payload as DataTokenPayload; + } + + protected getVerifyParameters( + jwt: string, + verifyProperties?: Partial + ): { + decomposedJwt: DecomposedJwt; + jwksUri: string; + verifyProperties: SpecificVerifyProperties; + } { + const decomposedJwt = decomposeUnverifiedJwt(jwt); + assertStringArrayContainsString( + "Signer", + decomposedJwt.header.signer, + this.expectedIssuers, + JwtInvalidSignerError + ); + const albConfig = this.getIssuerConfig(decomposedJwt.header.signer); + return { + decomposedJwt, + jwksUri: verifyProperties?.jwksUri ?? this.defaultJwksUri, + verifyProperties: { + ...albConfig, + ...verifyProperties, + } as unknown as SpecificVerifyProperties, + }; + } + + private validateDataJwtFields( + header:JwtHeader, + payload: JwtPayload, + options: { + clientId?: string | string[] | null; + } + ): void { + //TODO check client header, signer header, iss payload + } + +} \ No newline at end of file diff --git a/src/alb-verifier.ts b/src/alb-verifier.ts deleted file mode 100644 index 1cc764a..0000000 --- a/src/alb-verifier.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { AwsAlbJwksCache } from "./alb-v2"; -import { JwksCache } from "./jwk"; -import { JwtVerifierBase, JwtVerifierProperties } from "./jwt-verifier"; -import { Properties } from "./typing-util"; - -export interface AlbVerifyProperties { - - /** - * The client ID that you expect to be present on the JWT - * (In the ID token's aud claim, or the Access token's client_id claim). - * If you provide a string array, that means at least one of those client IDs - * must be present on the JWT. - * Pass null explicitly to not check the JWT's client ID--if you know what you're doing - */ - clientId: string | string[] | null; - - loadBalancerArn: string; - - jwksUri?: string; - issuer: string; - -} - - -/** Type for Cognito JWT verifier properties, for a single User Pool */ -export type AlbJwtVerifierProperties = { - -} & Partial; - -/** - * Type for Cognito JWT verifier properties, when multiple User Pools are used in the verifier. - * In this case, you should be explicit in mapping `clientId` to User Pool. - */ -export type AlbJwtVerifierMultiProperties = { - -} & AlbVerifyProperties; - - -/** - * TODO rename - */ -export type AlbJwtVerifierSingleUserPool< - T extends AlbJwtVerifierProperties, -> = AlbJwtVerifier< - Properties, - T & - JwtVerifierProperties & { - audience: null; - }, - false ->; - -/** - * TODO rename - */ -export type AlbJwtVerifierMultiUserPool< - T extends AlbJwtVerifierMultiProperties, -> = AlbJwtVerifier< - Properties, - T & - JwtVerifierProperties & { - audience: null; - }, - true ->; - - -export class AlbJwtVerifier< - SpecificVerifyProperties extends Partial, - IssuerConfig extends JwtVerifierProperties & { - audience: null; - }, - MultiIssuer extends boolean, -> extends JwtVerifierBase { - - private constructor( - props: AlbJwtVerifierProperties | AlbJwtVerifierMultiProperties[], - ) { - const issuerConfig = Array.isArray(props) - ? (props.map((p) => ({ - ...p, - audience: null, // checked instead by validateCognitoJwtFields - })) as IssuerConfig[]) - : ({ - ...props, - audience: null, // checked instead by validateCognitoJwtFields - } as IssuerConfig); - super(issuerConfig, new AwsAlbJwksCache()); - } - - - /** - * Create a Cognito JWT verifier for a single User Pool - * - * @param verifyProperties The verification properties for your User Pool - * @param additionalProperties Additional properties - * @param additionalProperties.jwksCache Overriding JWKS cache that you want to use - * @returns An Cognito JWT Verifier instance, that you can use to verify Cognito signed JWTs with - */ - static create( - verifyProperties: T & Partial, - additionalProperties?: { jwksCache: JwksCache } - ): AlbJwtVerifierSingleUserPool; - - - static create( - props: (T & Partial)[], - additionalProperties?: { jwksCache: JwksCache } - ): AlbJwtVerifierMultiUserPool; - - - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - static create( - verifyProperties: - | AlbJwtVerifierProperties - | AlbJwtVerifierMultiProperties[] - ) { - return new this(verifyProperties); - } - -} \ No newline at end of file diff --git a/src/jwt-verifier.ts b/src/jwt-verifier.ts index d3294b0..158df86 100644 --- a/src/jwt-verifier.ts +++ b/src/jwt-verifier.ts @@ -229,7 +229,7 @@ export async function verifyJwt( * @param transformJwkToKeyObjectFn A function that can transform a JWK into a crypto native key object * @returns Promise that resolves to the payload of the JWT––if the JWT is valid, otherwise the promise rejects */ -async function verifyDecomposedJwt( +export async function verifyDecomposedJwt( decomposedJwt: DecomposedJwt, jwksUri: string, options: { @@ -333,7 +333,7 @@ export function verifyJwtSync( * @param transformJwkToKeyObjectFn A function that can transform a JWK into a crypto native key object * @returns The (JSON parsed) payload of the JWT––if the JWT is valid, otherwise an error is thrown */ -function verifyDecomposedJwtSync( +export function verifyDecomposedJwtSync( decomposedJwt: DecomposedJwt, jwkOrJwks: JsonObject, options: { diff --git a/tests/unit/alb-verifier.test.ts b/tests/unit/alb-verifier-v1.test.ts similarity index 97% rename from tests/unit/alb-verifier.test.ts rename to tests/unit/alb-verifier-v1.test.ts index 3f21123..6381590 100644 --- a/tests/unit/alb-verifier.test.ts +++ b/tests/unit/alb-verifier-v1.test.ts @@ -5,7 +5,7 @@ import { disallowAllRealNetworkTraffic, mockHttpsUri, } from "./test-util"; -import { AlbJwtVerifier } from "../../src/alb-verifier"; +import { AlbJwtVerifier } from "../../src/alb-verifier-v1"; import { createPublicKey } from "crypto"; describe("unit tests alb verifier", () => { @@ -106,7 +106,7 @@ describe("unit tests alb verifier", () => { issuer, clientId, loadBalancerArn, - jwksUri + jwksUri, }); expect.assertions(1); expect( From 8be8cc25e3eaebac0b394fab34f56793a4f0a545 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Tue, 29 Oct 2024 21:38:01 +0000 Subject: [PATCH 08/12] alb-verifier-v1 first working version --- src/alb-verifier-v1.ts | 52 +++--- tests/unit/alb-verifier-v1.test.ts | 273 +++++++++++++++++++++++++++++ 2 files changed, 295 insertions(+), 30 deletions(-) diff --git a/src/alb-verifier-v1.ts b/src/alb-verifier-v1.ts index 92cfcfb..62cba15 100644 --- a/src/alb-verifier-v1.ts +++ b/src/alb-verifier-v1.ts @@ -54,13 +54,6 @@ export type AlbJwtVerifierMultiProperties = { } & AlbVerifyProperties; -export type AlbVerifierProperties = { - - loadBalancerArn: string; - - -} & Partial; - export type AlbJwtVerifierSingleAlb< T extends AlbJwtVerifierProperties, > = AlbJwtVerifier< @@ -80,32 +73,24 @@ type AlbVerifyParameters = { } extends SpecificVerifyProperties ? [jwt: string, props?: SpecificVerifyProperties] : [jwt: string, props: SpecificVerifyProperties]; + +export type AlbConfig = { + + loadBalancerArn: string; +} & Partial; type DataTokenPayload = { exp:number iss:string, } & JsonObject; - -/** - * TODO rename JWT by Data - * Differences between this and the Cognito JWT verifier: - * 1. The issuer is the load balancer arn/Signer - * 2. The JWT validation is different - * 3. Hydrate is not supported - * - * Pro: No hydratemethod - * Con: Code duplicated - * - * Con v2 dont inforce loadBalancerArn not null - */ export class AlbJwtVerifier< SpecificVerifyProperties extends Partial, MultiAlb extends boolean, > { - private readonly albConfigMap: Map = new Map(); + private readonly albConfigMap: Map = new Map(); private readonly publicKeyCache = new KeyObjectCache(); private readonly jwksCache: JwksCache = new AwsAlbJwksCache(); private readonly defaultJwksUri; @@ -151,7 +136,7 @@ export class AlbJwtVerifier< return new this(verifyProperties); } - public async verify( + public async verify( ...[jwt, properties]: AlbVerifyParameters): Promise{ const { decomposedJwt, jwksUri, verifyProperties } = this.getVerifyParameters(jwt, properties); await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties); @@ -169,7 +154,7 @@ export class AlbJwtVerifier< return decomposedJwt.payload as DataTokenPayload; } - public verifySync( ...[jwt, properties]: AlbVerifyParameters): DataTokenPayload { + public verifySync( ...[jwt, properties]: AlbVerifyParameters): DataTokenPayload { const { decomposedJwt, jwksUri, verifyProperties } = this.getVerifyParameters(jwt, properties); this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties); try { @@ -227,16 +212,15 @@ export class AlbJwtVerifier< ? [jwks: Jwks, loadBalancerArn?: string] : [jwks: Jwks, loadBalancerArn: string] ): void { - // const albConfig = this.getAlbConfig(loadBalancerArn); - // this.jwksCache.addJwks(albConfig.jwksUri, jwks); - // this.publicKeyCache.clearCache(albConfig.issuer); - //TODO + const albConfig = this.getAlbConfig(loadBalancerArn); + this.jwksCache.addJwks(albConfig.jwksUri ?? this.defaultJwksUri, jwks); + this.publicKeyCache.clearCache(albConfig.loadBalancerArn); } //Duplicate from JwtVerifier protected getAlbConfig( loadBalancerArn?: string - ): AlbJwtVerifierProperties { + ): AlbConfig { if (!loadBalancerArn) { if (this.albConfigMap.size !== 1) { throw new ParameterValidationError("loadBalancerArn must be provided"); @@ -270,7 +254,11 @@ export class AlbJwtVerifier< audience:null }, this.jwksCache.getJwk.bind(this.jwksCache), - this.publicKeyCache.transformJwkToKeyObjectAsync.bind(this.publicKeyCache) + (jwk, alg, _issuer) => { + // Use the load balancer ARN instead of the issuer for the public key cache + const loadBalancerArn = decomposedJwt.header.signer as string; + return this.publicKeyCache.transformJwkToKeyObjectAsync(jwk, alg, loadBalancerArn); + } ); } @@ -289,7 +277,11 @@ export class AlbJwtVerifier< issuer: verifyProperties.issuer, audience:null }, - this.publicKeyCache.transformJwkToKeyObjectSync.bind(this.publicKeyCache) + (jwk, alg, _issuer) => { + // Use the load balancer ARN instead of the issuer for the public key cache + const loadBalancerArn = decomposedJwt.header.signer as string; + return this.publicKeyCache.transformJwkToKeyObjectSync(jwk, alg, loadBalancerArn); + } ); } } diff --git a/tests/unit/alb-verifier-v1.test.ts b/tests/unit/alb-verifier-v1.test.ts index 6381590..b2cd27d 100644 --- a/tests/unit/alb-verifier-v1.test.ts +++ b/tests/unit/alb-verifier-v1.test.ts @@ -113,6 +113,279 @@ describe("unit tests alb verifier", () => { await albVerifier.verify(signedJwt) ).toMatchObject({ hello: "world" }); }); + + + test("happy flow with lazy property", async () => { + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const jwk = keypair.jwk; + const kid = jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + const pem = createPublicKey({ + key: jwk, + format: "jwk", + }).export({ + format: "pem", + type: "spki", + });//pem with -----BEGIN PUBLIC KEY----- and -----END PUBLIC KEY-----. + + + mockHttpsUri(`${jwksUri}/${kid}`, { + responsePayload: pem, + }); + + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"RS256",//ES256. + iss:issuer, + client:clientId, + signer:loadBalancerArn, + exp + }, + { + hello: "world", + exp, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbJwtVerifier.create({ + loadBalancerArn + }); + expect.assertions(1); + expect( + await albVerifier.verify(signedJwt,{ + issuer, + clientId, + jwksUri, + }) + ).toMatchObject({ hello: "world" }); + }); + + + test("flow with no jwksUri", async () => { + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const jwk = keypair.jwk; + const kid = jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + const pem = createPublicKey({ + key: jwk, + format: "jwk", + }).export({ + format: "pem", + type: "spki", + });//pem with -----BEGIN PUBLIC KEY----- and -----END PUBLIC KEY-----. + + + mockHttpsUri(`${jwksUri}/${kid}`, { + responsePayload: pem, + }); + + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"RS256",//ES256. + iss:issuer, + client:clientId, + signer:loadBalancerArn, + exp + }, + { + hello: "world", + exp, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbJwtVerifier.create({ + issuer, + clientId, + loadBalancerArn, + }); + expect.assertions(1); + expect( + await albVerifier.verify(signedJwt) + ).toMatchObject({ hello: "world" }); + }); + + test("happy flow with multi properties", async () => { + + const keypair1 = generateKeyPair({kty:"EC",kid:"kid1"}); + const keypair2 = generateKeyPair({kty:"EC",kid:"kid2"}); + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn1 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/AAAAAAAAAAAAAAAA"; + const loadBalancerArn2 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/BBBBBBBBBBBBBBBB"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + + mockHttpsUri(`${jwksUri}/${keypair1.jwk.kid}`, { + responsePayload: createPublicKey({ + key: keypair1.jwk, + format: "jwk", + }).export({ + format: "pem", + type: "spki", + }), + }); + + mockHttpsUri(`${jwksUri}/${keypair2.jwk.kid}`, { + responsePayload: createPublicKey({ + key: keypair2.jwk, + format: "jwk", + }).export({ + format: "pem", + type: "spki", + }), + }); + + const signedJwt1 = signJwt( + { + typ:"JWT", + kid:keypair1.jwk.kid, + alg:"ES256", + iss:issuer, + client:clientId, + signer:loadBalancerArn1, + exp + }, + { + hello: "world1", + exp, + iss:issuer, + }, + keypair1.privateKey + ); + + const signedJwt2 = signJwt( + { + typ:"JWT", + kid:keypair2.jwk.kid, + alg:"ES256", + iss:issuer, + client:clientId, + signer:loadBalancerArn2, + exp + }, + { + hello: "world2", + exp, + iss:issuer, + }, + keypair2.privateKey + ); + const albVerifier = AlbJwtVerifier.create([{ + issuer, + clientId, + loadBalancerArn:loadBalancerArn1, + jwksUri, + },{ + issuer, + clientId, + loadBalancerArn:loadBalancerArn2, + jwksUri, + }]); + + expect.assertions(2); + + expect( + await albVerifier.verify(signedJwt1) + ).toMatchObject({ hello: "world1" }); + + expect( + await albVerifier.verify(signedJwt2) + ).toMatchObject({ hello: "world2" }); + }); + + test("flow with multi properties and no jwksUri", async () => { + + const keypair1 = generateKeyPair({kty:"EC",kid:"kid1"}); + const keypair2 = generateKeyPair({kty:"EC",kid:"kid2"}); + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn1 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/AAAAAAAAAAAAAAAA"; + const loadBalancerArn2 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/BBBBBBBBBBBBBBBB"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + + const signedJwt1 = signJwt( + { + typ:"JWT", + kid:keypair1.jwk.kid, + alg:"ES256", + iss:issuer, + client:clientId, + signer:loadBalancerArn1, + exp + }, + { + hello: "world1", + exp, + iss:issuer, + }, + keypair1.privateKey + ); + + const signedJwt2 = signJwt( + { + typ:"JWT", + kid:keypair2.jwk.kid, + alg:"ES256", + iss:issuer, + client:clientId, + signer:loadBalancerArn2, + exp + }, + { + hello: "world2", + exp, + iss:issuer, + }, + keypair2.privateKey + ); + const albVerifier = AlbJwtVerifier.create([{ + issuer, + clientId, + loadBalancerArn:loadBalancerArn1, + },{ + issuer, + clientId, + loadBalancerArn:loadBalancerArn2, + }]); + + albVerifier.cacheJwks(keypair1.jwks,loadBalancerArn1); + albVerifier.cacheJwks(keypair2.jwks,loadBalancerArn2); + + expect.assertions(2); + + expect( + await albVerifier.verify(signedJwt1) + ).toMatchObject({ hello: "world1" }); + + expect( + await albVerifier.verify(signedJwt2) + ).toMatchObject({ hello: "world2" }); + }); }); }); }); From 2d0a3529bc9b4a0eb79b5aad9577ca7f2f9a3eaa Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sat, 2 Nov 2024 09:59:13 +0000 Subject: [PATCH 09/12] working version of the alb-verifier. Add unit tests. Rename AlbJwtVerifer by JwtDataVerifier because it verify only the data jwt token. --- src/alb-v1.ts | 4 + src/alb-v2.ts | 50 +++- src/alb-verifier-v2.ts | 210 ---------------- src/{alb-verifier-v1.ts => alb-verifier.ts} | 58 +++-- tests/unit/alb-v2.test.ts | 19 +- ...rifier-v1.test.ts => alb-verifier.test.ts} | 238 ++++++++++++++---- 6 files changed, 297 insertions(+), 282 deletions(-) delete mode 100644 src/alb-verifier-v2.ts rename src/{alb-verifier-v1.ts => alb-verifier.ts} (85%) rename tests/unit/{alb-verifier-v1.test.ts => alb-verifier.test.ts} (60%) diff --git a/src/alb-v1.ts b/src/alb-v1.ts index 44bc782..5261492 100644 --- a/src/alb-v1.ts +++ b/src/alb-v1.ts @@ -1,3 +1,7 @@ +/** + * NOT USED. NEED TO BE REMOVED + */ + import { createPublicKey } from "crypto"; import { JwtBaseError, diff --git a/src/alb-v2.ts b/src/alb-v2.ts index 88c3849..59c4874 100644 --- a/src/alb-v2.ts +++ b/src/alb-v2.ts @@ -1,5 +1,6 @@ import { createPublicKey } from "crypto"; import { + JwkInvalidKtyError, JwksNotAvailableInCacheError, JwtBaseError, JwtWithoutValidKidError, @@ -12,6 +13,7 @@ import { import { JwtHeader, JwtPayload } from "./jwt-model"; import { Fetcher, SimpleFetcher } from "./https"; import { SimpleLruCache } from "./cache"; +import { assertStringEquals } from "./assert"; interface DecomposedJwt { header: JwtHeader; @@ -22,7 +24,13 @@ type JwksUri = string; export class AlbUriError extends JwtBaseError {} -//TODO comment importance of safe architecture +/** + * + * Security considerations: + * It's important that the application protected by this library run in a secure environment. This application should be behind the ALB and deployed in a private subnet, or a public subnet but with no access from a untrusted network. + * This security requierement is essential to be respected otherwise the application is exposed to several security risks. This class can be subject to a DoS attack if the attacker can control the kid. + * + */ export class AwsAlbJwksCache implements JwksCache { fetcher: Fetcher; @@ -54,15 +62,33 @@ export class AwsAlbJwksCache implements JwksCache { } private isValidAlbKid(kid:string) { - // for (let i = 0; i < kid.length; i++) { - // const code = kid.charCodeAt(i); - // if (!(code > 47 && code < 58) && // 0-9 - // !(code > 64 && code < 91) && // A-Z - // !(code > 96 && code < 123)) { // a-z - // return false; - // } - // } - return true; + if (kid.length !== 36) { + return false; + } + + const part1 = kid.slice(0, 8); + const part2 = kid.slice(9, 13); + const part3 = kid.slice(14, 18); + const part4 = kid.slice(19, 23); + const part5 = kid.slice(24, 36); + + if (kid[8] !== '-' || kid[13] !== '-' || kid[18] !== '-' || kid[23] !== '-') { + return false; + } + + const isHex = (str: string) => { + for (let i = 0; i < str.length; i++) { + const code = str.charCodeAt(i); + if (!(code >= 48 && code <= 57) && // 0-9 + !(code >= 97 && code <= 102) && // a-f + !(code >= 65 && code <= 70)) { // A-F + return false; + } + } + return true; + }; + + return isHex(part1) && isHex(part2) && isHex(part3) && isHex(part4) && isHex(part5); }; public async getJwk( @@ -113,9 +139,7 @@ export class AwsAlbJwksCache implements JwksCache { format: "jwk", }); - if(!jwk.kty){ - throw new Error("todo"); - } + assertStringEquals("JWK kty", jwk.kty, "EC", JwkInvalidKtyError); return { kid: kid, diff --git a/src/alb-verifier-v2.ts b/src/alb-verifier-v2.ts deleted file mode 100644 index deaec0a..0000000 --- a/src/alb-verifier-v2.ts +++ /dev/null @@ -1,210 +0,0 @@ -import { AwsAlbJwksCache } from "./alb-v2"; -import { assertStringArrayContainsString } from "./assert"; -import { JwtInvalidClaimError } from "./error"; -import { JwksCache } from "./jwk"; -import { DecomposedJwt, decomposeUnverifiedJwt } from "./jwt"; -import { JwtHeader, JwtPayload } from "./jwt-model"; -import { JwtVerifierBase, JwtVerifierProperties } from "./jwt-verifier"; -import { JsonObject } from "./safe-json-parse"; -import { Properties } from "./typing-util"; - -export class JwtInvalidSignerError extends JwtInvalidClaimError {} - -export interface AlbVerifyProperties { - - /** - * The client ID that you expect to be present on the JWT - * (In the ID token's aud claim, or the Access token's client_id claim). - * If you provide a string array, that means at least one of those client IDs - * must be present on the JWT. - * Pass null explicitly to not check the JWT's client ID--if you know what you're doing - */ - clientId: string | string[] | null; - - loadBalancerArn: string; - - jwksUri?: string; - issuer: string; - - /** - * If you want to peek inside the invalid JWT when verification fails, set `includeRawJwtInErrors` to true. - * Then, if an error is thrown during verification of the invalid JWT (e.g. the JWT is invalid because it is expired), - * the Error object will include a property `rawJwt`, with the raw decoded contents of the **invalid** JWT. - * The `rawJwt` will only be included in the Error object, if the JWT's signature can at least be verified. - */ - includeRawJwtInErrors?: boolean; - -} - - -/** Type for Alb JWT verifier properties, for a single Alb */ -export type AlbJwtVerifierProperties = { - -} & Partial; - -/** - * Type for Alb JWT verifier properties, when multiple Alb are used in the verifier. - */ -export type AlbJwtVerifierMultiProperties = { - -} & AlbVerifyProperties; - -export type AlbJwtVerifierSingleAlb< - T extends AlbJwtVerifierProperties, -> = AlbJwtVerifier< - Properties, - T & - JwtVerifierProperties & { - userPoolId: string; - audience: null; - }, - false ->; - -export type AlbJwtVerifierMultiAlb< - T extends AlbJwtVerifierMultiProperties, -> = AlbJwtVerifier< - Properties, - T & - JwtVerifierProperties & { - userPoolId: string; - audience: null; - }, - true ->; - -type AlbVerifyParameters = { - [key: string]: never; -} extends SpecificVerifyProperties - ? [jwt: string, props?: SpecificVerifyProperties] - : [jwt: string, props: SpecificVerifyProperties]; - -type DataTokenPayload = { - exp:number - iss:string, -} & JsonObject; - -export class AlbJwtVerifier< - SpecificVerifyProperties extends Partial, - IssuerConfig extends JwtVerifierProperties & { - audience: null; - }, - MultiIssuer extends boolean, -> extends JwtVerifierBase { - - private readonly defaultJwksUri; - - private constructor( - props: AlbJwtVerifierProperties | AlbJwtVerifierMultiProperties[], - ) { - const issuerConfig = Array.isArray(props) - ? (props.map((p) => ({ - ...p, - audience: null, // checked instead by validateCognitoJwtFields - })) as IssuerConfig[]) - : ({ - ...props, - audience: null, // checked instead by validateCognitoJwtFields - } as IssuerConfig); - super(issuerConfig, new AwsAlbJwksCache()); - const region = "us-east-1";//TODO extract region - this.defaultJwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; - } - - static create( - verifyProperties: T & Partial, - additionalProperties?: { jwksCache: JwksCache } - ): AlbJwtVerifierSingleAlb; - - - static create( - props: (T & Partial)[], - additionalProperties?: { jwksCache: JwksCache } - ): AlbJwtVerifierMultiAlb; - - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - static create( - verifyProperties: - | AlbJwtVerifierProperties - | AlbJwtVerifierMultiProperties[] - ) { - return new this(verifyProperties); - } - - public verifySync( - ...[jwt, properties]: AlbVerifyParameters - ): DataTokenPayload { - const { decomposedJwt, jwksUri, verifyProperties } = - this.getVerifyParameters(jwt, properties); - this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties); - try { - this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); - } catch (err) { - if ( - verifyProperties.includeRawJwtInErrors && - err instanceof JwtInvalidClaimError - ) { - throw err.withRawJwt(decomposedJwt); - } - throw err; - } - return decomposedJwt.payload as DataTokenPayload; - } - - public async verify( - ...[jwt, properties]: AlbVerifyParameters - ): Promise { - const { decomposedJwt, jwksUri, verifyProperties } = - this.getVerifyParameters(jwt, properties); - await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties); - try { - this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); - } catch (err) { - if ( - verifyProperties.includeRawJwtInErrors && - err instanceof JwtInvalidClaimError - ) { - throw err.withRawJwt(decomposedJwt); - } - throw err; - } - return decomposedJwt.payload as DataTokenPayload; - } - - protected getVerifyParameters( - jwt: string, - verifyProperties?: Partial - ): { - decomposedJwt: DecomposedJwt; - jwksUri: string; - verifyProperties: SpecificVerifyProperties; - } { - const decomposedJwt = decomposeUnverifiedJwt(jwt); - assertStringArrayContainsString( - "Signer", - decomposedJwt.header.signer, - this.expectedIssuers, - JwtInvalidSignerError - ); - const albConfig = this.getIssuerConfig(decomposedJwt.header.signer); - return { - decomposedJwt, - jwksUri: verifyProperties?.jwksUri ?? this.defaultJwksUri, - verifyProperties: { - ...albConfig, - ...verifyProperties, - } as unknown as SpecificVerifyProperties, - }; - } - - private validateDataJwtFields( - header:JwtHeader, - payload: JwtPayload, - options: { - clientId?: string | string[] | null; - } - ): void { - //TODO check client header, signer header, iss payload - } - -} \ No newline at end of file diff --git a/src/alb-verifier-v1.ts b/src/alb-verifier.ts similarity index 85% rename from src/alb-verifier-v1.ts rename to src/alb-verifier.ts index 62cba15..7723f9e 100644 --- a/src/alb-verifier-v1.ts +++ b/src/alb-verifier.ts @@ -1,7 +1,7 @@ import { AwsAlbJwksCache } from "./alb-v2"; import { assertStringArrayContainsString } from "./assert"; -import { JwtInvalidClaimError, ParameterValidationError } from "./error"; -import { Jwk, Jwks, JwksCache } from "./jwk"; +import { JwtInvalidClaimError, JwtInvalidSignatureAlgorithmError, ParameterValidationError } from "./error"; +import { Jwks, JwksCache } from "./jwk"; import { DecomposedJwt, decomposeUnverifiedJwt } from "./jwt"; import { JwtHeader, JwtPayload } from "./jwt-model"; import { KeyObjectCache, verifyDecomposedJwt, verifyDecomposedJwtSync } from "./jwt-verifier"; @@ -10,7 +10,12 @@ import { Properties } from "./typing-util"; type LoadBalancerArn = string; -export class JwtInvalidSignerError extends JwtInvalidClaimError {} +export const supportedSignatureAlgorithms = [ + "ES256", +] as const; + +export class AlbJwtInvalidSignerError extends JwtInvalidClaimError {} +export class AlbJwtInvalidClientIdError extends JwtInvalidClaimError {} export interface AlbVerifyProperties { @@ -54,16 +59,16 @@ export type AlbJwtVerifierMultiProperties = { } & AlbVerifyProperties; -export type AlbJwtVerifierSingleAlb< +export type AlbDataVerifierSingleAlb< T extends AlbJwtVerifierProperties, -> = AlbJwtVerifier< +> = AlbDataVerifier< Properties, false >; -export type AlbJwtVerifierMultiAlb< +export type AlbDataVerifierMultiAlb< T extends AlbJwtVerifierProperties, -> = AlbJwtVerifier< +> = AlbDataVerifier< Properties, true >; @@ -85,7 +90,7 @@ type DataTokenPayload = { iss:string, } & JsonObject; -export class AlbJwtVerifier< +export class AlbDataVerifier< SpecificVerifyProperties extends Partial, MultiAlb extends boolean, > { @@ -121,12 +126,12 @@ export class AlbJwtVerifier< static create( verifyProperties: T & Partial - ): AlbJwtVerifierSingleAlb; + ): AlbDataVerifierSingleAlb; static create( props: (T & Partial)[] - ): AlbJwtVerifierMultiAlb; + ): AlbDataVerifierMultiAlb; static create( verifyProperties: @@ -141,7 +146,7 @@ export class AlbJwtVerifier< const { decomposedJwt, jwksUri, verifyProperties } = this.getVerifyParameters(jwt, properties); await this.verifyDecomposedJwt(decomposedJwt, jwksUri, verifyProperties); try { - this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); + this.validateDataJwtFields(decomposedJwt.header, verifyProperties); } catch (err) { if ( verifyProperties.includeRawJwtInErrors && @@ -158,7 +163,7 @@ export class AlbJwtVerifier< const { decomposedJwt, jwksUri, verifyProperties } = this.getVerifyParameters(jwt, properties); this.verifyDecomposedJwtSync(decomposedJwt, jwksUri, verifyProperties); try { - this.validateDataJwtFields(decomposedJwt.header, decomposedJwt.payload, verifyProperties); + this.validateDataJwtFields(decomposedJwt.header, verifyProperties); } catch (err) { if ( verifyProperties.includeRawJwtInErrors && @@ -184,7 +189,7 @@ export class AlbJwtVerifier< "Signer", decomposedJwt.header.signer, this.expectedLoadBalancerArn, - JwtInvalidSignerError + AlbJwtInvalidSignerError ); const albConfig = this.getAlbConfig(decomposedJwt.header.signer); return { @@ -199,12 +204,35 @@ export class AlbJwtVerifier< private validateDataJwtFields( header:JwtHeader, - payload: JwtPayload, options: { + loadBalancerArn: string; + issuer?: string; clientId?: string | string[] | null; } ): void { - //TODO check client header, signer header, iss payload + + // Check JWT signature algorithm is one of the supported signature algorithms + assertStringArrayContainsString( + "JWT signature algorithm", + header.alg, + supportedSignatureAlgorithms, + JwtInvalidSignatureAlgorithmError + ); + + // Check client ID header + if (options.clientId !== null) { + if (options.clientId === undefined) { + throw new ParameterValidationError( + "clientId must be provided or set to null explicitly" + ); + } + assertStringArrayContainsString( + "Client ID", + header.client, + options.clientId, + AlbJwtInvalidClientIdError + ); + } } public cacheJwks( diff --git a/tests/unit/alb-v2.test.ts b/tests/unit/alb-v2.test.ts index 2a1e14e..382ddaf 100644 --- a/tests/unit/alb-v2.test.ts +++ b/tests/unit/alb-v2.test.ts @@ -1,4 +1,5 @@ import { AwsAlbJwksCache } from "../../src/alb-v2"; +import { JwtWithoutValidKidError } from "../../src/error"; import { allowAllRealNetworkTraffic, disallowAllRealNetworkTraffic, @@ -8,7 +9,7 @@ import { readFileSync } from "fs"; import { join } from "path"; describe("alb", () => { - const kid = "abcdefgh-1234-ijkl-5678-mnopqrstuvwx"; + const kid = "00000000-0000-0000-0000-000000000000"; const jwksUri = "https://public-keys.auth.elb.eu-west-1.amazonaws.com"; const albResponse = readFileSync(join(__dirname, "alb-jwks-test.pem")); @@ -48,4 +49,20 @@ describe("alb", () => { await Promise.all([promise1, promise2]); }); + test("Invalid kid", () => { + + const decomposedJwt ={ + header: { + alg: "ES256", + kid: "invalid-kid" + }, + payload: {}, + }; + + const jwksCache = new AwsAlbJwksCache(); + + expect.assertions(1); + expect(()=>jwksCache.getJwk(jwksUri, decomposedJwt)).rejects.toThrow(JwtWithoutValidKidError); + }); + }); diff --git a/tests/unit/alb-verifier-v1.test.ts b/tests/unit/alb-verifier.test.ts similarity index 60% rename from tests/unit/alb-verifier-v1.test.ts rename to tests/unit/alb-verifier.test.ts index b2cd27d..f5c585d 100644 --- a/tests/unit/alb-verifier-v1.test.ts +++ b/tests/unit/alb-verifier.test.ts @@ -5,13 +5,18 @@ import { disallowAllRealNetworkTraffic, mockHttpsUri, } from "./test-util"; -import { AlbJwtVerifier } from "../../src/alb-verifier-v1"; +import { AlbJwtInvalidClientIdError, AlbJwtInvalidSignerError, AlbDataVerifier } from "../../src/alb-verifier-v1"; import { createPublicKey } from "crypto"; +import { JwtInvalidIssuerError } from "../../src/error"; describe("unit tests alb verifier", () => { let keypair: ReturnType; beforeAll(() => { - keypair = generateKeyPair(); + keypair = generateKeyPair({ + kid:"00000000-0000-0000-0000-000000000000", + kty:"EC", + alg:"ES256", + }); disallowAllRealNetworkTraffic(); }); afterAll(() => { @@ -35,7 +40,7 @@ describe("unit tests alb verifier", () => { { typ:"JWT", kid, - alg:"RS256",//ES256. + alg:"ES256", iss:issuer, client:clientId, signer:loadBalancerArn, @@ -48,7 +53,7 @@ describe("unit tests alb verifier", () => { }, keypair.privateKey ); - const albVerifier = AlbJwtVerifier.create({ + const albVerifier = AlbDataVerifier.create({ issuer, clientId, loadBalancerArn, @@ -89,7 +94,7 @@ describe("unit tests alb verifier", () => { { typ:"JWT", kid, - alg:"RS256",//ES256. + alg:"ES256", iss:issuer, client:clientId, signer:loadBalancerArn, @@ -102,7 +107,7 @@ describe("unit tests alb verifier", () => { }, keypair.privateKey ); - const albVerifier = AlbJwtVerifier.create({ + const albVerifier = AlbDataVerifier.create({ issuer, clientId, loadBalancerArn, @@ -143,7 +148,7 @@ describe("unit tests alb verifier", () => { { typ:"JWT", kid, - alg:"RS256",//ES256. + alg:"ES256", iss:issuer, client:clientId, signer:loadBalancerArn, @@ -156,7 +161,7 @@ describe("unit tests alb verifier", () => { }, keypair.privateKey ); - const albVerifier = AlbJwtVerifier.create({ + const albVerifier = AlbDataVerifier.create({ loadBalancerArn }); expect.assertions(1); @@ -198,7 +203,7 @@ describe("unit tests alb verifier", () => { { typ:"JWT", kid, - alg:"RS256",//ES256. + alg:"ES256", iss:issuer, client:clientId, signer:loadBalancerArn, @@ -211,7 +216,7 @@ describe("unit tests alb verifier", () => { }, keypair.privateKey ); - const albVerifier = AlbJwtVerifier.create({ + const albVerifier = AlbDataVerifier.create({ issuer, clientId, loadBalancerArn, @@ -224,8 +229,8 @@ describe("unit tests alb verifier", () => { test("happy flow with multi properties", async () => { - const keypair1 = generateKeyPair({kty:"EC",kid:"kid1"}); - const keypair2 = generateKeyPair({kty:"EC",kid:"kid2"}); + const keypair1 = generateKeyPair({kty:"EC",alg:"ES256",kid:"11111111-1111-1111-1111-111111111111"}); + const keypair2 = generateKeyPair({kty:"EC",alg:"ES256",kid:"22222222-2222-2222-2222-222222222222"}); const region = "us-east-1"; const userPoolId = "us-east-1_123456"; @@ -291,7 +296,7 @@ describe("unit tests alb verifier", () => { }, keypair2.privateKey ); - const albVerifier = AlbJwtVerifier.create([{ + const albVerifier = AlbDataVerifier.create([{ issuer, clientId, loadBalancerArn:loadBalancerArn1, @@ -314,28 +319,23 @@ describe("unit tests alb verifier", () => { ).toMatchObject({ hello: "world2" }); }); - test("flow with multi properties and no jwksUri", async () => { + test("happy flow with default jwksUri", async () => { - const keypair1 = generateKeyPair({kty:"EC",kid:"kid1"}); - const keypair2 = generateKeyPair({kty:"EC",kid:"kid2"}); - const region = "us-east-1"; const userPoolId = "us-east-1_123456"; - const loadBalancerArn1 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/AAAAAAAAAAAAAAAA"; - const loadBalancerArn2 = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/BBBBBBBBBBBBBBBB"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/AAAAAAAAAAAAAAAA"; const clientId = "my-client-id"; const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; - const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead - const signedJwt1 = signJwt( + const signedJwt = signJwt( { typ:"JWT", - kid:keypair1.jwk.kid, + kid:keypair.jwk.kid, alg:"ES256", iss:issuer, client:clientId, - signer:loadBalancerArn1, + signer:loadBalancerArn, exp }, { @@ -343,49 +343,201 @@ describe("unit tests alb verifier", () => { exp, iss:issuer, }, - keypair1.privateKey + keypair.privateKey ); - const signedJwt2 = signJwt( + const albVerifier = AlbDataVerifier.create({ + issuer, + clientId, + loadBalancerArn:loadBalancerArn, + }); + + albVerifier.cacheJwks(keypair.jwks,loadBalancerArn); + + expect.assertions(1); + + expect( + await albVerifier.verify(signedJwt) + ).toMatchObject({ hello: "world1" }); + + }); + + test("invalid issuer", () => { + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const badIssuer = `https://badissuer.amazonaws.com`; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const kid = keypair.jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + + const signedJwt = signJwt( { typ:"JWT", - kid:keypair2.jwk.kid, + kid, alg:"ES256", - iss:issuer, + iss:badIssuer, client:clientId, - signer:loadBalancerArn2, + signer:loadBalancerArn, exp }, { - hello: "world2", + hello: "world", exp, - iss:issuer, + iss:badIssuer, }, - keypair2.privateKey + keypair.privateKey ); - const albVerifier = AlbJwtVerifier.create([{ + const albVerifier = AlbDataVerifier.create({ issuer, clientId, - loadBalancerArn:loadBalancerArn1, - },{ + loadBalancerArn, + jwksUri + }); + + albVerifier.cacheJwks(keypair.jwks); + + expect.assertions(1); + expect( + albVerifier.verify(signedJwt) + ).rejects.toThrow(JwtInvalidIssuerError); + }); + + test("invalid signer", () => { + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const badSigner = "arn:aws:elasticloadbalancing:us-east-1:badaccount:loadbalancer/app/badloadbalancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const kid = keypair.jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"ES256", + iss:issuer, + client:clientId, + signer:badSigner, + exp + }, + { + hello: "world", + exp, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbDataVerifier.create({ issuer, clientId, - loadBalancerArn:loadBalancerArn2, - }]); + loadBalancerArn, + jwksUri + }); + + albVerifier.cacheJwks(keypair.jwks); - albVerifier.cacheJwks(keypair1.jwks,loadBalancerArn1); - albVerifier.cacheJwks(keypair2.jwks,loadBalancerArn2); + expect.assertions(1); + expect( + albVerifier.verify(signedJwt) + ).rejects.toThrow(AlbJwtInvalidSignerError); + }); - expect.assertions(2); + + test("invalid client id", () => { + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const badClientId = "bad-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const kid = keypair.jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"ES256", + iss:issuer, + client:badClientId, + signer:loadBalancerArn, + exp + }, + { + hello: "world", + exp, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbDataVerifier.create({ + issuer, + clientId, + loadBalancerArn, + jwksUri + }); + + albVerifier.cacheJwks(keypair.jwks); + expect.assertions(1); expect( - await albVerifier.verify(signedJwt1) - ).toMatchObject({ hello: "world1" }); + albVerifier.verify(signedJwt) + ).rejects.toThrow(AlbJwtInvalidClientIdError); + }); + + test("null client id", async () => { + + const region = "us-east-1"; + const userPoolId = "us-east-1_123456"; + const loadBalancerArn = "arn:aws:elasticloadbalancing:us-east-1:123456789012:loadbalancer/app/my-load-balancer/50dc6c495c0c9188"; + const clientId = "my-client-id"; + const issuer = `https://cognito-idp.${region}.amazonaws.com/${userPoolId}`; + const jwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; + const kid = keypair.jwk.kid; + const exp = 4000000000;// nock and jest.useFakeTimers do not work well together. Used of a long expired date instead + + const signedJwt = signJwt( + { + typ:"JWT", + kid, + alg:"ES256", + iss:issuer, + client:clientId, + signer:loadBalancerArn, + exp + }, + { + hello: "world", + exp, + iss:issuer, + }, + keypair.privateKey + ); + const albVerifier = AlbDataVerifier.create({ + issuer, + clientId:null, + loadBalancerArn, + jwksUri + }); + + albVerifier.cacheJwks(keypair.jwks); + expect.assertions(1); expect( - await albVerifier.verify(signedJwt2) - ).toMatchObject({ hello: "world2" }); + await albVerifier.verify(signedJwt) + ).toMatchObject({ hello: "world" }); }); + }); }); }); From ae9b364e87693691207f97969cb096b614a1e76a Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Sat, 2 Nov 2024 09:59:56 +0000 Subject: [PATCH 10/12] fix file rename --- tests/unit/alb-verifier.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/alb-verifier.test.ts b/tests/unit/alb-verifier.test.ts index f5c585d..7e6bab2 100644 --- a/tests/unit/alb-verifier.test.ts +++ b/tests/unit/alb-verifier.test.ts @@ -5,7 +5,7 @@ import { disallowAllRealNetworkTraffic, mockHttpsUri, } from "./test-util"; -import { AlbJwtInvalidClientIdError, AlbJwtInvalidSignerError, AlbDataVerifier } from "../../src/alb-verifier-v1"; +import { AlbJwtInvalidClientIdError, AlbJwtInvalidSignerError, AlbDataVerifier } from "../../src/alb-verifier"; import { createPublicKey } from "crypto"; import { JwtInvalidIssuerError } from "../../src/error"; From 589845b68bfc4e69b388d9718d2e564f3567c254 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Mon, 4 Nov 2024 08:05:25 +0000 Subject: [PATCH 11/12] version withouth the publicKeyCache + default region per AlbConfig --- src/alb-verifier.ts | 63 +++++++++++++++++++++++++++------------------ src/jwt-verifier.ts | 2 +- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/alb-verifier.ts b/src/alb-verifier.ts index 7723f9e..d92b754 100644 --- a/src/alb-verifier.ts +++ b/src/alb-verifier.ts @@ -82,6 +82,7 @@ type AlbVerifyParameters = { export type AlbConfig = { loadBalancerArn: string; + defaultJwksUri: string;//Managing multi region ALB even if not possible (ALB target group are single region) } & Partial; @@ -95,10 +96,9 @@ export class AlbDataVerifier< MultiAlb extends boolean, > { - private readonly albConfigMap: Map = new Map(); - private readonly publicKeyCache = new KeyObjectCache(); - private readonly jwksCache: JwksCache = new AwsAlbJwksCache(); - private readonly defaultJwksUri; + readonly albConfigMap: Map = new Map(); + // private readonly publicKeyCache = new KeyObjectCache(); + readonly jwksCache: JwksCache = new AwsAlbJwksCache(); private constructor( props: AlbJwtVerifierProperties | AlbJwtVerifierMultiProperties[], @@ -115,15 +115,32 @@ export class AlbDataVerifier< `loadBalancerArn ${albProps.loadBalancerArn} supplied multiple times` ); } - this.albConfigMap.set(albProps.loadBalancerArn, albProps); + this.albConfigMap.set(albProps.loadBalancerArn, { + ...albProps, + defaultJwksUri: this.defaultJwksUri(albProps), + }); } }else { - this.albConfigMap.set(props.loadBalancerArn, props); + this.albConfigMap.set(props.loadBalancerArn, { + ...props, + defaultJwksUri: this.defaultJwksUri(props), + }); } - const region = "us-east-1";//TODO extract region - this.defaultJwksUri = `https://public-keys.auth.elb.${region}.amazonaws.com`; } + private extractRegionFromLoadBalancerArn(loadBalancerArn: string): string { + const arnParts = loadBalancerArn.split(":"); + if (arnParts.length < 4) { + throw new ParameterValidationError(`Invalid load balancer ARN: ${loadBalancerArn}`); + } + return arnParts[3]; + } + + private defaultJwksUri(params:{loadBalancerArn: string}): string { + const region = this.extractRegionFromLoadBalancerArn(params.loadBalancerArn); + return `https://public-keys.auth.elb.${region}.amazonaws.com`; + } + static create( verifyProperties: T & Partial ): AlbDataVerifierSingleAlb; @@ -194,7 +211,7 @@ export class AlbDataVerifier< const albConfig = this.getAlbConfig(decomposedJwt.header.signer); return { decomposedJwt, - jwksUri: verifyProperties?.jwksUri ?? this.defaultJwksUri, + jwksUri: verifyProperties?.jwksUri ?? albConfig.defaultJwksUri, verifyProperties: { ...albConfig, ...verifyProperties, @@ -241,11 +258,10 @@ export class AlbDataVerifier< : [jwks: Jwks, loadBalancerArn: string] ): void { const albConfig = this.getAlbConfig(loadBalancerArn); - this.jwksCache.addJwks(albConfig.jwksUri ?? this.defaultJwksUri, jwks); - this.publicKeyCache.clearCache(albConfig.loadBalancerArn); + this.jwksCache.addJwks(albConfig.jwksUri ?? albConfig.defaultJwksUri, jwks); + // this.publicKeyCache.clearCache(albConfig.loadBalancerArn); } - //Duplicate from JwtVerifier protected getAlbConfig( loadBalancerArn?: string ): AlbConfig { @@ -262,12 +278,10 @@ export class AlbDataVerifier< return config; } - //Duplicate from JwtVerifier protected get expectedLoadBalancerArn(): string[] { return Array.from(this.albConfigMap.keys()); } - //Duplicate from JwtVerifier protected verifyDecomposedJwt( decomposedJwt: DecomposedJwt, jwksUri: string, @@ -282,15 +296,14 @@ export class AlbDataVerifier< audience:null }, this.jwksCache.getJwk.bind(this.jwksCache), - (jwk, alg, _issuer) => { - // Use the load balancer ARN instead of the issuer for the public key cache - const loadBalancerArn = decomposedJwt.header.signer as string; - return this.publicKeyCache.transformJwkToKeyObjectAsync(jwk, alg, loadBalancerArn); - } + // (jwk, alg, _issuer) => { + // // Use the load balancer ARN instead of the issuer for the public key cache + // const loadBalancerArn = decomposedJwt.header.signer as string; + // return this.publicKeyCache.transformJwkToKeyObjectAsync(jwk, alg, loadBalancerArn); + // } ); } - //Duplicate from JwtVerifier protected verifyDecomposedJwtSync( decomposedJwt: DecomposedJwt, jwksUri: string, @@ -305,11 +318,11 @@ export class AlbDataVerifier< issuer: verifyProperties.issuer, audience:null }, - (jwk, alg, _issuer) => { - // Use the load balancer ARN instead of the issuer for the public key cache - const loadBalancerArn = decomposedJwt.header.signer as string; - return this.publicKeyCache.transformJwkToKeyObjectSync(jwk, alg, loadBalancerArn); - } + // (jwk, alg, _issuer) => { + // // Use the load balancer ARN instead of the issuer for the public key cache + // const loadBalancerArn = decomposedJwt.header.signer as string; + // return this.publicKeyCache.transformJwkToKeyObjectSync(jwk, alg, loadBalancerArn); + // } ); } } diff --git a/src/jwt-verifier.ts b/src/jwt-verifier.ts index 158df86..de43b36 100644 --- a/src/jwt-verifier.ts +++ b/src/jwt-verifier.ts @@ -348,7 +348,7 @@ export function verifyDecomposedJwtSync( }) => void; includeRawJwtInErrors?: boolean; }, - transformJwkToKeyObjectFn: JwkToKeyObjectTransformerSync + transformJwkToKeyObjectFn: JwkToKeyObjectTransformerSync = nodeWebCompat.transformJwkToKeyObjectSync ) { const { header, headerB64, payload, payloadB64, signatureB64 } = decomposedJwt; From 40e0042620d5e465ef350a87ba3640dd51f02597 Mon Sep 17 00:00:00 2001 From: Nicolas Viaud Date: Fri, 13 Dec 2024 21:01:51 +0000 Subject: [PATCH 12/12] clean up: remove alb v1 --- src/alb-v1.ts | 115 --------------------- src/alb-verifier.ts | 4 +- src/{alb-v2.ts => alb.ts} | 0 tests/unit/alb-v1.test.ts | 86 --------------- tests/unit/{alb-v2.test.ts => alb.test.ts} | 2 +- 5 files changed, 3 insertions(+), 204 deletions(-) delete mode 100644 src/alb-v1.ts rename src/{alb-v2.ts => alb.ts} (100%) delete mode 100644 tests/unit/alb-v1.test.ts rename tests/unit/{alb-v2.test.ts => alb.test.ts} (97%) diff --git a/src/alb-v1.ts b/src/alb-v1.ts deleted file mode 100644 index 5261492..0000000 --- a/src/alb-v1.ts +++ /dev/null @@ -1,115 +0,0 @@ -/** - * NOT USED. NEED TO BE REMOVED - */ - -import { createPublicKey } from "crypto"; -import { - JwtBaseError, - JwtWithoutValidKidError, -} from "./error"; -import { - JwkWithKid, - Jwks, - JwksCache, - JwksParser, - PenaltyBox, - SimpleJwksCache, - assertIsJwks, -} from "./jwk"; -import { JwtHeader, JwtPayload } from "./jwt-model"; - -interface DecomposedJwt { - header: JwtHeader; - payload: JwtPayload; -} - -const albJwksUriRegex = - /https:\/\/public-keys.auth.elb.(?[a-z0-9-]+).amazonaws.com\/(?[a-z0-9-]+)/; - -export class AlbUriError extends JwtBaseError {} - -const parseJwks: JwksParser = function (jwksBin: ArrayBuffer, jwksUri: string) { - const match = jwksUri.match(albJwksUriRegex); - if (!match || !match.groups?.kid) { - throw new AlbUriError("Wrong URI for ALB public key"); - } - const jwks = { - keys: [ - { - kid: match.groups.kid, - use: "sig", - ...createPublicKey({ - key: Buffer.from(jwksBin), - format: "pem", - type: "spki", - }).export({ - format: "jwk", - }), - }, - ], - }; - assertIsJwks(jwks); - return jwks; -}; - -export class AwsAlbJwksCache implements JwksCache { - simpleJwksCache: SimpleJwksCache; - - constructor(props?: { penaltyBox?: PenaltyBox }) { - this.simpleJwksCache = new SimpleJwksCache({ - penaltyBox: props?.penaltyBox, - jwksParser: parseJwks, - }); - } - - /** - * - * @param Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com - * @param decomposedJwt - * @returns - */ - private expandWithKid(jwksUri: string, decomposedJwt: DecomposedJwt): string { - const kid = this.getKid(decomposedJwt); - return `${jwksUri}/${encodeURIComponent(kid)}`; - } - - private getKid(decomposedJwt: DecomposedJwt): string { - if (typeof decomposedJwt.header.kid !== "string") { - throw new JwtWithoutValidKidError( - "JWT header does not have valid kid claim" - ); - } - return decomposedJwt.header.kid; - } - - public addJwks(): void { - throw new Error("Method not implemented."); - } - - async getJwks(): Promise { - throw new Error("Method not implemented."); - } - - - /** - * - * @param Ex: https://public-keys.auth.elb.eu-west-1.amazonaws.com - * @param decomposedJwt - * @returns - */ - public getCachedJwk( - jwksUri: string, - decomposedJwt: DecomposedJwt - ): JwkWithKid { - const jwksUriExpanded = this.expandWithKid(jwksUri, decomposedJwt); - return this.simpleJwksCache.getCachedJwk(jwksUriExpanded, decomposedJwt); - } - - public async getJwk( - jwksUri: string, - decomposedJwt: DecomposedJwt - ): Promise { - const jwksUriExpanded = this.expandWithKid(jwksUri, decomposedJwt); - return this.simpleJwksCache.getJwk(jwksUriExpanded, decomposedJwt); - } -} diff --git a/src/alb-verifier.ts b/src/alb-verifier.ts index d92b754..4d0550a 100644 --- a/src/alb-verifier.ts +++ b/src/alb-verifier.ts @@ -1,10 +1,10 @@ -import { AwsAlbJwksCache } from "./alb-v2"; +import { AwsAlbJwksCache } from "./alb"; import { assertStringArrayContainsString } from "./assert"; import { JwtInvalidClaimError, JwtInvalidSignatureAlgorithmError, ParameterValidationError } from "./error"; import { Jwks, JwksCache } from "./jwk"; import { DecomposedJwt, decomposeUnverifiedJwt } from "./jwt"; import { JwtHeader, JwtPayload } from "./jwt-model"; -import { KeyObjectCache, verifyDecomposedJwt, verifyDecomposedJwtSync } from "./jwt-verifier"; +import { verifyDecomposedJwt, verifyDecomposedJwtSync } from "./jwt-verifier"; import { JsonObject } from "./safe-json-parse"; import { Properties } from "./typing-util"; diff --git a/src/alb-v2.ts b/src/alb.ts similarity index 100% rename from src/alb-v2.ts rename to src/alb.ts diff --git a/tests/unit/alb-v1.test.ts b/tests/unit/alb-v1.test.ts deleted file mode 100644 index d168ed0..0000000 --- a/tests/unit/alb-v1.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { - JwtWithoutValidKidError, -} from "../../src/error"; -import { AwsAlbJwksCache } from "../../src/alb-v1"; -import { - allowAllRealNetworkTraffic, - disallowAllRealNetworkTraffic, - mockHttpsUri, -} from "./test-util"; -import { readFileSync } from "fs"; -import { join } from "path"; - -describe("unit tests https", () => { - const kid = "abcdefgh-1234-ijkl-5678-mnopqrstuvwx"; - const jwksUri = "https://public-keys.auth.elb.eu-west-1.amazonaws.com"; - const jwksUriWithKid = `https://public-keys.auth.elb.eu-west-1.amazonaws.com/${kid}`; - - const albResponse = readFileSync(join(__dirname, "alb-jwks-test.pem")); - const jwk = { - kid: kid, - use: "sig", - kty: "EC", - x: "GBJCbjNusVteS__606LS3fgYrhQyvfAh-GbOfy2n7rU", - y: "oBuN90bW-AvxscoesVaE7ryPISjqseKgio6H5ZO5xmk", - crv: "P-256", - }; - const getDecomposedJwt = (kidParam?: string) => ({ - header: { - alg: "ES256", - kid: kidParam ?? kid, - }, - payload: {}, - }); - - beforeAll(() => { - disallowAllRealNetworkTraffic(); - }); - afterAll(() => { - allowAllRealNetworkTraffic(); - }); - - test("ALB JWKS cache error flow: kid empty", () => { - const jwksCache = new AwsAlbJwksCache(); - expect.assertions(1); - return expect( - jwksCache.getJwk(jwksUri, { header: { alg: "ES256" }, payload: {} }) - ).rejects.toThrow(JwtWithoutValidKidError); - }); - - test("ALB JWKS add cache return not implemented exception", () => { - const jwksCache = new AwsAlbJwksCache(); - return expect(jwksCache.addJwks).toThrow("Method not implemented."); - }); - - test("ALB JWKS get JWKS return not implemented exception", () => { - const jwksCache = new AwsAlbJwksCache(); - expect.assertions(1); - return expect(jwksCache.getJwks()).rejects.toThrow( - "Method not implemented." - ); - }); - - test("ALB JWKS cache fetches URI one attempt at a time", async () => { - /** - * Test what happens when the the JWKS URI is requested multiple times in parallel - * (e.g. in parallel promises). When this happens only 1 actual HTTPS request should - * be made to the JWKS URI. - */ - - mockHttpsUri(jwksUriWithKid, { - responsePayload: albResponse, - }); - - const jwksCache = new AwsAlbJwksCache(); - - const fetch = jest.spyOn(jwksCache.simpleJwksCache.fetcher,"fetch"); - - const promise1 = jwksCache.getJwk(jwksUri, getDecomposedJwt()); - const promise2 = jwksCache.getJwk(jwksUri, getDecomposedJwt()); - expect.assertions(2); - expect(promise1).toEqual(promise2); - await Promise.all([promise1, promise2]); - expect(fetch).toHaveBeenCalledTimes(1); - }); - -}); diff --git a/tests/unit/alb-v2.test.ts b/tests/unit/alb.test.ts similarity index 97% rename from tests/unit/alb-v2.test.ts rename to tests/unit/alb.test.ts index 382ddaf..3962037 100644 --- a/tests/unit/alb-v2.test.ts +++ b/tests/unit/alb.test.ts @@ -1,4 +1,4 @@ -import { AwsAlbJwksCache } from "../../src/alb-v2"; +import { AwsAlbJwksCache } from "../../src/alb"; import { JwtWithoutValidKidError } from "../../src/error"; import { allowAllRealNetworkTraffic,