From d2f144006d874824e39b2ac46123471add2d8483 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 11:06:15 +1300 Subject: [PATCH 01/33] setup test env to handle TextEncoder + IndexedDb --- .gitignore | 3 +++ jest-environment-jsdom.cjs | 28 ++++++++++++++++++++++++++++ jest.config.mjs | 5 ++++- test/setup.ts | 1 + 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 jest-environment-jsdom.cjs diff --git a/.gitignore b/.gitignore index 54d944eda..87670dff7 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ node_modules/ .vscode/ temp/ .history/ + +# Jetbrains IDEs +.idea/ diff --git a/jest-environment-jsdom.cjs b/jest-environment-jsdom.cjs new file mode 100644 index 000000000..e3f1d4e40 --- /dev/null +++ b/jest-environment-jsdom.cjs @@ -0,0 +1,28 @@ +'use strict'; + +const { TextEncoder, TextDecoder } = require('util'); +const { default: $JSDOMEnvironment, TestEnvironment } = require('jest-environment-jsdom'); +const crypto = require("crypto"); + +Object.defineProperty(exports, '__esModule', { + value: true +}); + +class JSDOMEnvironment extends $JSDOMEnvironment { + constructor(...args) { + const { global } = super(...args); + // see https://github.com/jsdom/jsdom/issues/2524 + global.TextEncoder = TextEncoder; + global.TextDecoder = TextDecoder; + // see https://github.com/jestjs/jest/issues/9983 + global.Uint8Array = Uint8Array; + global.crypto.subtle = crypto.subtle; + global.crypto.randomUUID = crypto.randomUUID; + // see https://github.com/dumbmatter/fakeIndexedDB#jsdom-often-used-with-jest + global.structuredClone = structuredClone; + } +} + +exports.default = JSDOMEnvironment; +exports.TestEnvironment = TestEnvironment === $JSDOMEnvironment ? + JSDOMEnvironment : TestEnvironment; diff --git a/jest.config.mjs b/jest.config.mjs index 1041416ec..4e5b05c54 100644 --- a/jest.config.mjs +++ b/jest.config.mjs @@ -7,9 +7,12 @@ export default { clearMocks: true, setupFilesAfterEnv: ["./test/setup.ts"], testMatch: ["**/{src,test}/**/*.test.ts"], - testEnvironment: "jsdom", + testEnvironment: "./jest-environment-jsdom.cjs", collectCoverage, coverageReporters: collectCoverage ? ["lcov"] : ["lcov", "text"], + moduleNameMapper: { + "^jose": "jose", // map to jose cjs module otherwise jest breaks + }, transform: { "^.+\\.tsx?$": [ "ts-jest", diff --git a/test/setup.ts b/test/setup.ts index a38681ba8..26c27ef13 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -1,4 +1,5 @@ import { Log } from "../src"; +import "fake-indexeddb/auto"; import { TextEncoder } from "util"; import { webcrypto } from "node:crypto"; From 1008c98d94d0b38415bcb6994d5085bf4b0095eb Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 11:06:37 +1300 Subject: [PATCH 02/33] add dev deps for testing --- package-lock.json | 20 ++++++++++++++++++++ package.json | 2 ++ 2 files changed, 22 insertions(+) diff --git a/package-lock.json b/package-lock.json index 4a0fb0f45..98b0e76dc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,11 +21,13 @@ "esbuild": "^0.20.0", "eslint": "^8.5.0", "eslint-plugin-testing-library": "^6.0.0", + "fake-indexeddb": "^5.0.1", "http-proxy-middleware": "^3.0.0", "husky": "^9.0.6", "jest": "^29.3.1", "jest-environment-jsdom": "^29.3.1", "jest-mock": "^29.3.1", + "jose": "^5.1.2", "lint-staged": "^15.0.1", "ts-jest": "^29.0.3", "typedoc": "^0.25.0", @@ -3788,6 +3790,15 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, + "node_modules/fake-indexeddb": { + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/fake-indexeddb/-/fake-indexeddb-5.0.2.tgz", + "integrity": "sha512-cB507r5T3D55DfclY01GLkninZLfU7HXV/mhVRTnTRm5k2u+fY7Fof2dBkr80p5t7G7dlA/G5dI87QiMdPpMCQ==", + "dev": true, + "engines": { + "node": ">=18" + } + }, "node_modules/fast-deep-equal": { "version": "3.1.3", "integrity": "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==", @@ -5094,6 +5105,15 @@ "integrity": "sha512-8wb9Yw966OSxApiCt0K3yNJL8pnNeIv+OEq2YMidz4FKP6nonSRoOXc80iXY4JaN2FC11B9qsNmDsm+ZOfMROA==", "dev": true }, + "node_modules/jose": { + "version": "5.2.3", + "resolved": "https://registry.npmjs.org/jose/-/jose-5.2.3.tgz", + "integrity": "sha512-KUXdbctm1uHVL8BYhnyHkgp3zDX5KW8ZhAKVFEfUbU2P8Alpzjb+48hHvjOdQIyPshoblhzsuqOwEEAbtHVirA==", + "dev": true, + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/js-tokens": { "version": "4.0.0", "integrity": "sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==", diff --git a/package.json b/package.json index 9180922be..c99da70a4 100644 --- a/package.json +++ b/package.json @@ -52,10 +52,12 @@ "eslint": "^8.5.0", "eslint-plugin-testing-library": "^6.0.0", "http-proxy-middleware": "^3.0.0", + "fake-indexeddb": "^5.0.1", "husky": "^9.0.6", "jest": "^29.3.1", "jest-environment-jsdom": "^29.3.1", "jest-mock": "^29.3.1", + "jose": "^5.1.2", "lint-staged": "^15.0.1", "ts-jest": "^29.0.3", "typedoc": "^0.25.0", From 2bd6ad083b70d742286fbde907bf7ab78b7f42fb Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 11:07:20 +1300 Subject: [PATCH 03/33] Add IndexedDb store to handle private crypto keys --- src/IndexedDbCryptoKeyPairStore.test.ts | 91 +++++++++++++++++++++++++ src/IndexedDbCryptoKeyPairStore.ts | 62 +++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 src/IndexedDbCryptoKeyPairStore.test.ts create mode 100644 src/IndexedDbCryptoKeyPairStore.ts diff --git a/src/IndexedDbCryptoKeyPairStore.test.ts b/src/IndexedDbCryptoKeyPairStore.test.ts new file mode 100644 index 000000000..5dc6dde5f --- /dev/null +++ b/src/IndexedDbCryptoKeyPairStore.test.ts @@ -0,0 +1,91 @@ +import { IndexedDbCryptoKeyPairStore as subject } from "./IndexedDbCryptoKeyPairStore"; + +describe("IndexedDBCryptoKeyPairStore", () => { + let data: CryptoKeyPair; + const createCryptoKeyPair = async () => { + return await window.crypto.subtle.generateKey( + { + name: "ECDSA", + namedCurve: "P-256", + }, + false, + ["sign", "verify"], + ); + }; + + beforeEach(async () => { + data = await createCryptoKeyPair(); + }); + + describe("set", () => { + it("should return a promise", async () => { + // act + const p = subject.set("key", data); + + // assert + expect(p).toBeInstanceOf(Promise); + // eslint-disable-next-line no-empty + try { await p; } catch {} + }); + + it("should store a key in IndexedDB", async () => { + await subject.set("foo", data); + const result = await subject.get("foo"); + + expect(result).toEqual(data); + }); + }); + + describe("remove", () => { + it("should return a promise", async () => { + // act + const p = subject.remove("key"); + + // assert + expect(p).toBeInstanceOf(Promise); + // eslint-disable-next-line no-empty + try { await p; } catch {} + }); + + it("should remove a key from IndexedDB", async () => { + await subject.set("foo", data); + let result = await subject.get("foo"); + + expect(result).toEqual(data); + + await subject.remove("foo"); + result = await subject.get("foo"); + expect(result).toBeUndefined(); + }); + + it("should return a value if key exists", async () => { + await subject.set("foo", data); + const result = await subject.remove("foo"); + + expect(result).toEqual(data); + }); + }); + + describe("getAllKeys", () => { + it("should return a promise", async () => { + // act + const p = subject.getAllKeys(); + + // assert + expect(p).toBeInstanceOf(Promise); + // eslint-disable-next-line no-empty + try { await p; } catch {} + }); + + it("should get all keys in IndexedDB", async () => { + await subject.set("foo", data); + const dataTwo = await createCryptoKeyPair(); + await subject.set("boo", dataTwo); + + const result = await subject.getAllKeys(); + expect(result.length).toEqual(2); + expect(result).toContain("foo"); + expect(result).toContain("boo"); + }); + }); +}); diff --git a/src/IndexedDbCryptoKeyPairStore.ts b/src/IndexedDbCryptoKeyPairStore.ts new file mode 100644 index 000000000..63c33e2dc --- /dev/null +++ b/src/IndexedDbCryptoKeyPairStore.ts @@ -0,0 +1,62 @@ +export class IndexedDbCryptoKeyPairStore { + static dbName = "oidc"; + static storeName = "dpop"; + + static async get(key: string): Promise { + const store = await this.createStore(this.dbName, this.storeName); + return await store("readonly", (str) => { + return this.promisifyRequest(str.get(key)); + }) as CryptoKeyPair; + } + + static async getAllKeys(): Promise { + const store = await this.createStore(this.dbName, this.storeName); + return await store("readonly", (str) => { + return this.promisifyRequest(str.getAllKeys()); + }) as string[]; + } + + static async remove(key: string): Promise { + const item = await this.get(key); + const store = await this.createStore(this.dbName, this.storeName); + await store("readwrite", (str) => { + return this.promisifyRequest(str.delete(key)); + }); + return item; + } + + static async set(key: string, value: CryptoKeyPair): Promise { + const store = await this.createStore(this.dbName, this.storeName); + await store("readwrite", (str: IDBObjectStore) => { + str.put(value, key); + return this.promisifyRequest(str.transaction); + }); + } + + static promisifyRequest( + request: IDBRequest | IDBTransaction): Promise { + return new Promise((resolve, reject) => { + (request as IDBTransaction).oncomplete = (request as IDBRequest).onsuccess = () => resolve((request as IDBRequest).result); + (request as IDBTransaction).onabort = (request as IDBRequest).onerror = () => reject((request as IDBRequest).error); + }); + } + + static async createStore( + dbName: string, + storeName: string, + ): Promise<(txMode: IDBTransactionMode, callback: (store: IDBObjectStore) => T | PromiseLike) => Promise> { + const request = indexedDB.open(dbName); + request.onupgradeneeded = () => request.result.createObjectStore(storeName); + const db = await this.promisifyRequest(request); + + return async ( + txMode: IDBTransactionMode, + callback: (store: IDBObjectStore) => T | PromiseLike, + ) => { + const tx = db.transaction(storeName, txMode); + const store = tx.objectStore(storeName); + return await callback(store); + }; + } + +} From 489fbdc24f89b90141242de5bd67555021191c36 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 11:07:56 +1300 Subject: [PATCH 04/33] Add DPoPService and associated cypto + jwt helper functions --- src/DPoPService.test.ts | 44 ++++++++++++++ src/DPoPService.ts | 104 ++++++++++++++++++++++++++++++++++ src/utils/CryptoUtils.test.ts | 13 +++++ src/utils/CryptoUtils.ts | 60 ++++++++++++++++++++ src/utils/JwtUtils.test.ts | 39 +++++++++++++ src/utils/JwtUtils.ts | 19 +++++++ 6 files changed, 279 insertions(+) create mode 100644 src/DPoPService.test.ts create mode 100644 src/DPoPService.ts diff --git a/src/DPoPService.test.ts b/src/DPoPService.test.ts new file mode 100644 index 000000000..b1a336f87 --- /dev/null +++ b/src/DPoPService.test.ts @@ -0,0 +1,44 @@ +import { DPoPService } from "./DPoPService"; +import { jwtVerify, decodeProtectedHeader, importJWK, type JWK } from "jose"; +import { IndexedDbCryptoKeyPairStore as idb } from "./IndexedDbCryptoKeyPairStore"; + +describe("DPoPService", () => { + + beforeEach(async () => { + await idb.remove("oidc.dpop"); + }); + + describe("generateDPoPProof", () => { + it("should generate a valid proof without an access token", async () => { + const proof = await DPoPService.generateDPoPProof({ url: "http://example.com" }); + const protectedHeader = decodeProtectedHeader(proof); + const publicKey = await importJWK(protectedHeader.jwk); + const verifiedResult = await jwtVerify(proof, publicKey); + + expect(verifiedResult.payload).toHaveProperty("htu"); + expect(verifiedResult.payload).toHaveProperty("htm"); + }); + + it("should generate a valid proof with an access token", async () => { + await DPoPService.generateDPoPProof({ url: "http://example.com" }); + const proof = await DPoPService.generateDPoPProof({ url: "http://example.com", accessToken: "some_access_token" }); + + const protectedHeader = decodeProtectedHeader(proof); + const publicKey = await importJWK(protectedHeader.jwk); + const verifiedResult = await jwtVerify(proof, publicKey); + + expect(verifiedResult.payload).toHaveProperty("htu"); + expect(verifiedResult.payload).toHaveProperty("htm"); + expect(verifiedResult.payload).toHaveProperty("ath"); + expect(verifiedResult.payload["htu"]).toEqual("http://example.com"); + }); + }); + + describe("dpopJkt", () => { + it("should generate crypto keys when generating a dpop thumbprint if no keys exists in the store", async () => { + const setMock = jest.spyOn(idb, "set"); + await DPoPService.generateDPoPJkt(); + expect(setMock).toHaveBeenCalled(); + }); + }); +}); diff --git a/src/DPoPService.ts b/src/DPoPService.ts new file mode 100644 index 000000000..b01e427fa --- /dev/null +++ b/src/DPoPService.ts @@ -0,0 +1,104 @@ +import { CryptoUtils, JwtUtils } from "./utils"; +import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; + +/** + * Provides an implementation of Demonstrating Proof of Possession (DPoP) as defined in the + * OAuth2 spec https://datatracker.ietf.org/doc/html/rfc9449. + */ + +export interface GenerateDPoPProofOpts { + url: string; + accessToken?: string; + httpMethod?: string; +} +export class DPoPService { + public static async generateDPoPProof({ + url, + accessToken, + httpMethod, + }: GenerateDPoPProofOpts): Promise { + let hashedToken: Uint8Array; + let encodedHash: string; + + const payload: Record = { + "jti": window.crypto.randomUUID(), + "htm": httpMethod ?? "GET", + "htu": url, + "iat": Math.floor(Date.now() / 1000), + }; + + const keyPair = await this.loadKeyPair(); + + if (accessToken) { + hashedToken = await CryptoUtils.hash("SHA-256", accessToken); + encodedHash = CryptoUtils.encodeBase64Url(hashedToken); + payload.ath = encodedHash; + } + + try { + const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); + const header = { + "alg": "ES256", + "typ": "dpop+jwt", + "jwk": { + "crv": publicJwk.crv, + "kty": publicJwk.kty, + "x": publicJwk.x, + "y": publicJwk.y, + }, + }; + return await JwtUtils.generateSignedJwt(header, payload, keyPair.privateKey); + } catch (err) { + if (err instanceof TypeError) { + throw new Error(`Error exporting dpop public key: ${err.message}`); + } else { + throw err; + } + } + } + + public static async generateDPoPJkt() : Promise { + try { + const keyPair = await this.loadKeyPair(); + const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); + return await CryptoUtils.customCalculateJwkThumbprint(publicJwk); + } catch (err) { + if (err instanceof TypeError) { + throw new Error(`Could not retrieve dpop keys from storage: ${err.message}`); + } else { + throw err; + } + } + } + + protected static async loadKeyPair() : Promise { + try { + const allKeys = await IndexedDbCryptoKeyPairStore.getAllKeys(); + let keyPair: CryptoKeyPair; + if (!allKeys.includes("oidc.dpop")) { + keyPair = await this.generateKeys(); + await IndexedDbCryptoKeyPairStore.set("oidc.dpop", keyPair); + } else { + keyPair = await IndexedDbCryptoKeyPairStore.get("oidc.dpop") as CryptoKeyPair; + } + return keyPair; + } catch (err) { + if (err instanceof TypeError) { + throw new Error(`Could not retrieve dpop keys from storage: ${err.message}`); + } else { + throw err; + } + } + } + + protected static async generateKeys() : Promise { + return await window.crypto.subtle.generateKey( + { + name: "ECDSA", + namedCurve: "P-256", + }, + false, + ["sign", "verify"], + ); + } +} diff --git a/src/utils/CryptoUtils.test.ts b/src/utils/CryptoUtils.test.ts index d78f747d2..847abebef 100644 --- a/src/utils/CryptoUtils.test.ts +++ b/src/utils/CryptoUtils.test.ts @@ -12,4 +12,17 @@ describe("CryptoUtils", () => { expect(rnd).toMatch(pattern); }); }); + + describe("customCalculateJwkThumbprint", () => { + it("should return a valid rfc7638 jwk thumbprint", async () => { + const jwk = { + "kty": "EC", + "x": "zSau12OpG01OkWtiU8yG1ppv06v1uDrG66cNeqMWk_8", + "y": "Mjr6rkLy4chKd7f8m0ctUFEA2DtZuk_F09FU3h98xyo", + "crv": "P-256", + } as JsonWebKey; + const jwkThumbprint = await CryptoUtils.customCalculateJwkThumbprint(jwk); + expect(jwkThumbprint).toEqual("fvRy8PxXeUhrCgW4r0hAFroUAqSnmyiCncJmlCamt9g"); + }); + }); }); diff --git a/src/utils/CryptoUtils.ts b/src/utils/CryptoUtils.ts index 5f3fdd895..bbb80da2a 100644 --- a/src/utils/CryptoUtils.ts +++ b/src/utils/CryptoUtils.ts @@ -62,4 +62,64 @@ export class CryptoUtils { const data = encoder.encode([client_id, client_secret].join(":")); return toBase64(data); } + + /** + * Generates a hash of a string using a given algorithm + * @param alg + * @param message + */ + public static async hash(alg: string, message: string) : Promise { + const msgUint8 = new TextEncoder().encode(message); + const hashBuffer = await crypto.subtle.digest(alg, msgUint8); + return new Uint8Array(hashBuffer); + } + + /** + * Generates a base64url encoded string + */ + public static encodeBase64Url = (input: Uint8Array) => { + return toBase64(input).replace(/=/g, "").replace(/\+/g, "-").replace(/\//g, "_"); + }; + + /** + * Generates a rfc7638 compliant jwk thumbprint + * @param jwk + */ + public static async customCalculateJwkThumbprint(jwk: JsonWebKey): Promise { + let jsonObject: object; + switch (jwk.kty) { + case "RSA": + jsonObject = { + "e": jwk.e, + "kty": jwk.kty, + "n": jwk.n, + }; + break; + case "EC": + jsonObject = { + "crv": jwk.crv, + "kty": jwk.kty, + "x": jwk.x, + "y": jwk.y, + }; + break; + case "OKP": + jsonObject = { + "crv": jwk.crv, + "kty": jwk.kty, + "x": jwk.x, + }; + break; + case "oct": + jsonObject = { + "crv": jwk.k, + "kty": jwk.kty, + }; + break; + default: + throw new Error("Unknown jwk type"); + } + const utf8encodedAndHashed = await CryptoUtils.hash("SHA-256", JSON.stringify(jsonObject)); + return CryptoUtils.encodeBase64Url(utf8encodedAndHashed); + } } diff --git a/src/utils/JwtUtils.test.ts b/src/utils/JwtUtils.test.ts index f7548cc5f..d9263c265 100644 --- a/src/utils/JwtUtils.test.ts +++ b/src/utils/JwtUtils.test.ts @@ -48,4 +48,43 @@ describe("JwtUtils", () => { } }); }); + + describe("createJwt", () => { + it("should be able to create identical jwts two different ways", async () => { + const keyPair = await window.crypto.subtle.generateKey( + { + name: "ECDSA", + namedCurve: "P-256", + }, + false, + ["sign", "verify"]); + + const jti = window.crypto.randomUUID(); + + const payload: Record = { + "jti": jti, + "htm": "GET", + "htu": "http://test.com", + }; + + const iat = Date.now(); + + const header = { + "alg": "ES256", + "typ": "dpop+jwt", + }; + payload.iat = iat; + const jwt = await JwtUtils.generateSignedJwt(header, payload, keyPair.privateKey); + + const result = JwtUtils.decode(jwt); + + expect(result).toEqual( + { + "jti": jti, + "htm": "GET", + "htu": "http://test.com", + "iat": iat, + }); + }); + }); }); diff --git a/src/utils/JwtUtils.ts b/src/utils/JwtUtils.ts index 9546b79c8..ef75adc3b 100644 --- a/src/utils/JwtUtils.ts +++ b/src/utils/JwtUtils.ts @@ -2,6 +2,7 @@ import { jwtDecode } from "jwt-decode"; import { Logger } from "./Logger"; import type { JwtClaims } from "../Claims"; +import { CryptoUtils } from "./CryptoUtils"; /** * @internal @@ -17,4 +18,22 @@ export class JwtUtils { throw err; } } + + public static async generateSignedJwt(header: object, payload: object, privateKey: CryptoKey) : Promise { + const encodedHeader = CryptoUtils.encodeBase64Url(new TextEncoder().encode(JSON.stringify(header))); + const encodedPayload = CryptoUtils.encodeBase64Url(new TextEncoder().encode(JSON.stringify(payload))); + const encodedToken = `${encodedHeader}.${encodedPayload}`; + + const signature = await window.crypto.subtle.sign( + { + name: "ECDSA", + hash: { name: "SHA-256" }, + }, + privateKey, + new TextEncoder().encode(encodedToken), + ); + + const encodedSignature = CryptoUtils.encodeBase64Url(new Uint8Array(signature)); + return `${encodedToken}.${encodedSignature}`; + } } From 749fa5c099cfa9d67fcea74f7171d619e23e12f6 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 11:12:33 +1300 Subject: [PATCH 05/33] Add DPoP settings to UserManagerSettings --- docs/oidc-client-ts.api.md | 4 ++++ src/UserManagerSettings.test.ts | 16 ++++++++++++++++ src/UserManagerSettings.ts | 13 ++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 17a02f001..92897dbe7 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -1050,6 +1050,8 @@ export interface UserManagerSettings extends OidcClientSettings { accessTokenExpiringNotificationTimeInSeconds?: number; automaticSilentRenew?: boolean; checkSessionIntervalInSeconds?: number; + // Warning: (ae-forgotten-export) The symbol "DPoPSettings" needs to be exported by the entry point index.d.ts + dpopSettings?: DPoPSettings; iframeNotifyParentOrigin?: string; iframeScriptOrigin?: string; includeIdTokenInSilentRenew?: boolean; @@ -1086,6 +1088,8 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { // (undocumented) readonly checkSessionIntervalInSeconds: number; // (undocumented) + readonly dpopSettings: DPoPSettings; + // (undocumented) readonly iframeNotifyParentOrigin: string | undefined; // (undocumented) readonly iframeScriptOrigin: string | undefined; diff --git a/src/UserManagerSettings.test.ts b/src/UserManagerSettings.test.ts index e1e3cb397..0c610876e 100644 --- a/src/UserManagerSettings.test.ts +++ b/src/UserManagerSettings.test.ts @@ -376,4 +376,20 @@ describe("UserManagerSettings", () => { expect(subject.stopCheckSessionOnError).toEqual(true); }); }); + + describe("dpop", () => { + it("should return value from initial settings", () => { + // act + const subject = new UserManagerSettingsStore({ + authority: "authority", + client_id: "client", + redirect_uri: "redirect", + stopCheckSessionOnError : false, + dpopSettings: { enabled: true }, + }); + + // assert + expect(subject.dpopSettings.enabled).toEqual(true); + }); + }); }); diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index cf6ae3662..855708485 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -17,6 +17,11 @@ const DefaultAccessTokenExpiringNotificationTimeInSeconds = 60; const DefaultCheckSessionIntervalInSeconds = 2; export const DefaultSilentRequestTimeoutInSeconds = 10; +export interface DPoPSettings { + enabled: boolean; + bind_authorization_code?: boolean; +} + /** * The settings used to configure the {@link UserManager}. * @@ -55,7 +60,10 @@ export interface UserManagerSettings extends OidcClientSettings { validateSubOnSilentRenew?: boolean; /** Flag to control if id_token is included as id_token_hint in silent renew calls (default: false) */ includeIdTokenInSilentRenew?: boolean; - + /** Indicates whether to apply Dynamic Proof Of Possession when requesting an access token + * See https://datatracker.ietf.org/doc/html/rfc9449 + */ + dpopSettings?: DPoPSettings; /** Will raise events for when user has performed a signout at the OP (default: false) */ monitorSession?: boolean; monitorAnonymousSession?: boolean; @@ -110,6 +118,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { public readonly monitorSession: boolean; public readonly monitorAnonymousSession: boolean; + public readonly dpopSettings: DPoPSettings; public readonly checkSessionIntervalInSeconds: number; public readonly query_status_response_type: string; public readonly stopCheckSessionOnError: boolean; @@ -142,6 +151,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { monitorSession = false, monitorAnonymousSession = false, + dpopSettings = { enabled: false, bind_authorization_code: false }, checkSessionIntervalInSeconds = DefaultCheckSessionIntervalInSeconds, query_status_response_type = "code", stopCheckSessionOnError = true, @@ -175,6 +185,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { this.monitorSession = monitorSession; this.monitorAnonymousSession = monitorAnonymousSession; + this.dpopSettings = dpopSettings; this.checkSessionIntervalInSeconds = checkSessionIntervalInSeconds; this.stopCheckSessionOnError = stopCheckSessionOnError; this.query_status_response_type = query_status_response_type; From dec3112114a00aed8e5afd710964464d530d8f78 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 11:36:53 +1300 Subject: [PATCH 06/33] Wire up userManager to DPoP functionality for code exchange and refresh token --- docs/oidc-client-ts.api.md | 10 ++- src/OidcClient.ts | 1 + src/SigninRequest.ts | 6 ++ src/User.ts | 5 ++ src/UserManager.test.ts | 149 +++++++++++++++++++++++++++++++++++++ src/UserManager.ts | 47 +++++++++++- 6 files changed, 215 insertions(+), 3 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 92897dbe7..ce5186f35 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -47,6 +47,8 @@ export class CheckSessionIFrame { // @public (undocumented) export interface CreateSigninRequestArgs extends Omit { + // (undocumented) + dpopJkt?: string; // (undocumented) redirect_uri?: string; // (undocumented) @@ -623,7 +625,7 @@ export type SigninRedirectArgs = RedirectParams & ExtraSigninRequestArgs; // @public (undocumented) export class SigninRequest { // (undocumented) - static create({ url, authority, client_id, redirect_uri, response_type, scope, state_data, response_mode, request_type, client_secret, nonce, url_state, resource, skipUserInfo, extraQueryParams, extraTokenParams, disablePKCE, ...optionalParams }: SigninRequestCreateArgs): Promise; + static create({ url, authority, client_id, redirect_uri, response_type, scope, state_data, response_mode, request_type, client_secret, nonce, url_state, resource, skipUserInfo, extraQueryParams, extraTokenParams, disablePKCE, dpopJkt, ...optionalParams }: SigninRequestCreateArgs): Promise; // (undocumented) readonly state: SigninState; // (undocumented) @@ -645,6 +647,8 @@ export interface SigninRequestCreateArgs { // (undocumented) display?: string; // (undocumented) + dpopJkt?: string; + // (undocumented) extraQueryParams?: Record; // (undocumented) extraTokenParams?: Record; @@ -902,6 +906,8 @@ export class User { url_state?: string; }); access_token: string; + // (undocumented) + dpopProof(url: string, httpMethod?: string): Promise; get expired(): boolean | undefined; expires_at?: number; get expires_in(): number | undefined; @@ -949,6 +955,8 @@ export class UserManager { clearStaleState(): Promise; // (undocumented) protected readonly _client: OidcClient; + // (undocumented) + protected readonly _dpopNonceStore: WebStorageStateStore | null; get events(): UserManagerEvents; // (undocumented) protected readonly _events: UserManagerEvents; diff --git a/src/OidcClient.ts b/src/OidcClient.ts index 1793ec2b4..b4e5dd55c 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -24,6 +24,7 @@ export interface CreateSigninRequestArgs redirect_uri?: string; response_type?: string; scope?: string; + dpopJkt?: string; /** custom "state", which can be used by a caller to have "data" round tripped */ state?: unknown; diff --git a/src/SigninRequest.ts b/src/SigninRequest.ts index b44bfb534..d8d6367cc 100644 --- a/src/SigninRequest.ts +++ b/src/SigninRequest.ts @@ -21,6 +21,7 @@ export interface SigninRequestCreateArgs { response_mode?: "query" | "fragment"; nonce?: string; display?: string; + dpopJkt?: string; prompt?: string; max_age?: number; ui_locales?: string; @@ -72,6 +73,7 @@ export class SigninRequest { extraQueryParams, extraTokenParams, disablePKCE, + dpopJkt, ...optionalParams }: SigninRequestCreateArgs): Promise { if (!url) { @@ -119,6 +121,10 @@ export class SigninRequest { parsedUrl.searchParams.append("nonce", nonce); } + if (dpopJkt) { + parsedUrl.searchParams.append("dpop_jkt", dpopJkt); + } + let stateParam = state.id; if (url_state) { stateParam = `${stateParam}${URL_STATE_DELIMITER}${url_state}`; diff --git a/src/User.ts b/src/User.ts index 67a07c9cf..209420d74 100644 --- a/src/User.ts +++ b/src/User.ts @@ -3,6 +3,7 @@ import { Logger, Timer } from "./utils"; import type { IdTokenClaims } from "./Claims"; +import { DPoPService } from "./DPoPService"; /** * Holds claims represented by a combination of the `id_token` and the user info endpoint. @@ -124,4 +125,8 @@ export class User { Logger.createStatic("User", "fromStorageString"); return new User(JSON.parse(storageString)); } + + public async dpopProof(url: string, httpMethod?: string): Promise { + return await DPoPService.generateDPoPProof({ url, accessToken: this.access_token, httpMethod: httpMethod } ); + } } diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 890aab104..7a5d5cbc1 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -18,6 +18,7 @@ import type { UserProfile } from "./User"; import { WebStorageStateStore } from "./WebStorageStateStore"; import type { SigninState } from "./SigninState"; import type { State } from "./State"; +import { IndexedDbCryptoKeyPairStore as idb } from "./IndexedDbCryptoKeyPairStore"; import { mocked } from "jest-mock"; @@ -169,6 +170,25 @@ describe("UserManager", () => { }); }); + describe("removeUser", () => { + it("should remove user from store and remove dpop keys from indexedDB if dpop setting is true", async () => { + // arrange + Object.assign(subject.settings.dpopSettings, { + enabled: true, + }); + const storeUserMock = jest.spyOn(subject, "storeUser"); + const unloadMock = jest.spyOn(subject["_events"], "unload"); + const delMock = jest.spyOn(idb, "remove"); + // act + await subject.removeUser(); + + // assert + expect(storeUserMock).toBeCalledWith(null); + expect(unloadMock).toBeCalled(); + expect(delMock).toBeCalled(); + }); + }); + describe("revokeTokens", () => { it("should revoke the token types specified", async () => { // arrange @@ -302,6 +322,21 @@ describe("UserManager", () => { }); }); + it("should return a user if DPoP enabled", async () => { + // arrange + subject.settings.dpopSettings.enabled = true; + const spy = jest.spyOn(subject["_client"], "processSigninResponse") + .mockResolvedValue({} as SigninResponse); + + // act + const user = await subject.signinRedirectCallback("http://app/cb?state=test&code=code"); + + // assert + expect(spy).toHaveBeenCalledWith("http://app/cb?state=test&code=code", { "DPoP": expect.any(String) }); + expect(user).toBeInstanceOf(User); + spy.mockRestore(); + }); + describe("signinResourceOwnerCredentials", () => { it("should fail on wrong credentials", async () => { // arrange @@ -401,6 +436,43 @@ describe("UserManager", () => { handle, ); }); + + it("should pass dpopJkt arg if dpop is enabled and bind_authorization_code is true", async () => { + // arrange + const user = new User({ + access_token: "access_token", + token_type: "token_type", + profile: {} as UserProfile, + }); + const handle = { } as PopupWindow; + jest.spyOn(subject["_popupNavigator"], "prepare") + .mockImplementation(() => Promise.resolve(handle)); + subject["_signin"] = jest.fn().mockResolvedValue(user); + const extraArgs: SigninPopupArgs = { + extraQueryParams: { q : "q" }, + extraTokenParams: { t: "t" }, + state: "state", + nonce: "random_nonce", + redirect_uri: "http://app/extra_callback", + prompt: "login", + }; + subject.settings.dpopSettings.enabled = true; + subject.settings.dpopSettings.bind_authorization_code = true; + + // act + await subject.signinPopup(extraArgs); + + // assert + expect(subject["_signin"]).toHaveBeenCalledWith( + { + request_type: "si:p", + display: "popup", + dpopJkt: expect.any(String), + ...extraArgs, + }, + handle, + ); + }); }); describe("signinPopupCallback", () => { @@ -493,6 +565,45 @@ describe("UserManager", () => { ); }); + it("should pass dpopJkt to _signIn if dpop enabled and bind_authorization_code is true", async () => { + // arrange + const user = new User({ + access_token: "access_token", + token_type: "token_type", + profile: {} as UserProfile, + }); + jest.spyOn(subject["_popupNavigator"], "prepare"); + subject["_signin"] = jest.fn().mockResolvedValue(user); + const extraArgs: SigninSilentArgs = { + extraQueryParams: { q : "q" }, + extraTokenParams: { t: "t" }, + state: "state", + nonce: "random_nonce", + redirect_uri: "http://app/extra_callback", + }; + subject.settings.dpopSettings.enabled = true; + subject.settings.dpopSettings.bind_authorization_code = true; + + // act + await subject.signinSilent(extraArgs); + + // assert + expect(subject["_signin"]).toHaveBeenCalledWith( + { + request_type: "si:s", + prompt: "none", + id_token_hint: undefined, + dpopJkt: expect.any(String), + ...extraArgs, + }, + expect.objectContaining({ + close: expect.any(Function), + navigate: expect.any(Function), + }), + undefined, + ); + }); + it("should work when having no user present", async () => { // arrange const user = new User({ @@ -545,6 +656,44 @@ describe("UserManager", () => { ); }); + it("should use DPoP when enabled when using a refresh token", async () => { + // arrange + const user = new User({ + access_token: "access_token", + token_type: "token_type", + refresh_token: "refresh_token", + profile: { + sub: "sub", + nickname: "Nick", + } as UserProfile, + }); + subject.settings.dpopSettings.enabled = true; + + const useRefreshTokenSpy = jest.spyOn(subject["_client"], "useRefreshToken").mockResolvedValue({ + access_token: "new_access_token", + profile: { + sub: "sub", + nickname: "Nicholas", + }, + } as unknown as SigninResponse); + subject["_loadUser"] = jest.fn().mockResolvedValue(user); + + // act + const refreshedUser = await subject.signinSilent(); + expect(refreshedUser).toHaveProperty("access_token", "new_access_token"); + expect(refreshedUser!.profile).toHaveProperty("nickname", "Nicholas"); + expect(useRefreshTokenSpy).toHaveBeenCalledWith( + expect.objectContaining({ + state: { + refresh_token: user.refresh_token, + session_state: null, + "profile": { "nickname": "Nick", "sub": "sub" }, + }, + extraHeaders: { "DPoP": expect.any(String) }, + }), + ); + }); + it("should use the resource from settings when a refresh token is present", async () => { // arrange const user = new User({ diff --git a/src/UserManager.ts b/src/UserManager.ts index be5b33ef0..3719d4959 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -15,6 +15,11 @@ import type { SignoutResponse } from "./SignoutResponse"; import type { MetadataService } from "./MetadataService"; import { RefreshState } from "./RefreshState"; import type { SigninResponse } from "./SigninResponse"; +import type { ExtraHeader } from "./OidcClientSettings"; +import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; +import { InMemoryWebStorage } from "./InMemoryWebStorage"; +import { WebStorageStateStore } from "./WebStorageStateStore"; +import { DPoPService } from "./DPoPService"; /** * @public @@ -88,6 +93,7 @@ export class UserManager { protected readonly _events: UserManagerEvents; protected readonly _silentRenewService: SilentRenewService; protected readonly _sessionMonitor: SessionMonitor | null; + protected readonly _dpopNonceStore: WebStorageStateStore | null; public constructor(settings: UserManagerSettings, redirectNavigator?: INavigator, popupNavigator?: INavigator, iframeNavigator?: INavigator) { this.settings = new UserManagerSettingsStore(settings); @@ -111,6 +117,11 @@ export class UserManager { this._sessionMonitor = new SessionMonitor(this); } + this._dpopNonceStore = null; + if (this.settings.dpopSettings.enabled) { + const store = typeof window !== "undefined" ? window.sessionStorage : new InMemoryWebStorage(); + this._dpopNonceStore = new WebStorageStateStore( { store }); + } } /** @@ -154,6 +165,11 @@ export class UserManager { const logger = this._logger.create("removeUser"); await this.storeUser(null); logger.info("user removed from storage"); + if (this.settings.dpopSettings.enabled) { + await this._dpopNonceStore?.remove("dpop_nonce"); + await IndexedDbCryptoKeyPairStore.remove("oidc.dpop"); + logger.debug("removed dpop cyptokeys from storage"); + } await this._events.unload(); } @@ -231,6 +247,7 @@ export class UserManager { */ public async signinPopup(args: SigninPopupArgs = {}): Promise { const logger = this._logger.create("signinPopup"); + let dpopJkt: string | undefined; const { popupWindowFeatures, popupWindowTarget, @@ -241,11 +258,16 @@ export class UserManager { logger.throw(new Error("No popup_redirect_uri configured")); } + if (this.settings.dpopSettings.enabled && this.settings.dpopSettings.bind_authorization_code) { + dpopJkt = await DPoPService.generateDPoPJkt(); + } + const handle = await this._popupNavigator.prepare({ popupWindowFeatures, popupWindowTarget }); const user = await this._signin({ request_type: "si:p", redirect_uri: url, display: "popup", + dpopJkt, ...requestArgs, }, handle); if (user) { @@ -289,14 +311,21 @@ export class UserManager { if (user?.refresh_token) { logger.debug("using refresh token"); const state = new RefreshState(user as Required); + const extraHeaders: Record = {}; + if (this.settings.dpopSettings.enabled) { + const url = await this.metadataService.getTokenEndpoint(false); + extraHeaders["DPoP"] = await DPoPService.generateDPoPProof({ url, httpMethod: "POST" }); + } return await this._useRefreshToken({ state, redirect_uri: requestArgs.redirect_uri, resource: requestArgs.resource, extraTokenParams: requestArgs.extraTokenParams, timeoutInSeconds: silentRequestTimeoutInSeconds, + extraHeaders: extraHeaders, }); } + let dpopJkt: string | undefined; const url = this.settings.silent_redirect_uri; if (!url) { @@ -310,11 +339,15 @@ export class UserManager { } const handle = await this._iframeNavigator.prepare({ silentRequestTimeoutInSeconds }); + if (this.settings.dpopSettings.enabled && this.settings.dpopSettings.bind_authorization_code) { + dpopJkt = await DPoPService.generateDPoPJkt(); + } user = await this._signin({ request_type: "si:s", redirect_uri: url, prompt: "none", id_token_hint: this.settings.includeIdTokenInSilentRenew ? user?.id_token : undefined, + dpopJkt, ...requestArgs, }, handle, verifySub); if (user) { @@ -438,7 +471,12 @@ export class UserManager { ...requestArgs, }, handle); try { - const signinResponse = await this._client.processSigninResponse(navResponse.url); + const extraHeaders: Record = {}; + if (this.settings.dpopSettings.enabled) { + const tokenUrl = await this.metadataService.getTokenEndpoint(false); + extraHeaders["DPoP"] = await DPoPService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); + } + const signinResponse = await this._client.processSigninResponse(navResponse.url, extraHeaders); logger.debug("got signin response"); if (signinResponse.session_state && signinResponse.profile.sub) { @@ -496,7 +534,12 @@ export class UserManager { } protected async _signinEnd(url: string, verifySub?: string): Promise { const logger = this._logger.create("_signinEnd"); - const signinResponse = await this._client.processSigninResponse(url); + const extraHeaders: Record = {}; + if (this.settings.dpopSettings.enabled) { + const tokenUrl = await this.metadataService.getTokenEndpoint(false); + extraHeaders["DPoP"] = await DPoPService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); + } + const signinResponse = await this._client.processSigninResponse(url, extraHeaders); logger.debug("got signin response"); const user = await this._buildUser(signinResponse, verifySub); From b0973a0687af118dbc47ff83990240a6925ac61d Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 19:40:10 +1300 Subject: [PATCH 07/33] Add tests for DPoPService exception handling --- src/DPoPService.test.ts | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/DPoPService.test.ts b/src/DPoPService.test.ts index b1a336f87..ce29b80df 100644 --- a/src/DPoPService.test.ts +++ b/src/DPoPService.test.ts @@ -32,6 +32,20 @@ describe("DPoPService", () => { expect(verifiedResult.payload).toHaveProperty("ath"); expect(verifiedResult.payload["htu"]).toEqual("http://example.com"); }); + + it("should throw an exception if there is an error exporting the public key", async () => { + const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockRejectedValue(new TypeError("Export key error")); + await expect(DPoPService.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Error exporting dpop public key: Export key error"); + exportKeyMock.mockRestore(); + }); + + it("should throw an exception if there is an error generating the signed JWT", async () => { + const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockResolvedValue({} as JsonWebKey); + const generateSignedJwtMock = jest.spyOn(crypto.subtle, "sign").mockRejectedValue(new Error("Generate signed JWT error")); + await expect(DPoPService.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Generate signed JWT error"); + exportKeyMock.mockRestore(); + generateSignedJwtMock.mockRestore(); + }); }); describe("dpopJkt", () => { @@ -40,5 +54,33 @@ describe("DPoPService", () => { await DPoPService.generateDPoPJkt(); expect(setMock).toHaveBeenCalled(); }); + + it("should throw a TypeError exception if the keys cannot be retrieved from the store", async () => { + const getMock = jest.spyOn(idb, "get").mockRejectedValue(new TypeError("Export key error")); + await DPoPService.generateDPoPProof({ url: "http://example.com" }); + await expect(DPoPService.generateDPoPJkt()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); + getMock.mockRestore(); + }); + + it("should throw an exception for any other reason", async () => { + const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockRejectedValue(new Error("Export key error")); + await expect(DPoPService.generateDPoPJkt()).rejects.toThrow("Export key error"); + exportKeyMock.mockRestore(); + }); + }); + + describe("loadKeyPair", () => { + it("should throw an exception if the keys cannot be retrieved from the store", async () => { + const getMock = jest.spyOn(idb, "get").mockRejectedValue(new TypeError("Export key error")); + await DPoPService.generateDPoPProof({ url: "http://example.com" }); + await expect(DPoPService["loadKeyPair"]()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); + getMock.mockRestore(); + }); + + it("should throw an exception for any other reason", async () => { + const setMock = jest.spyOn(idb, "set").mockRejectedValue(new Error("Set error")); + await expect(DPoPService["loadKeyPair"]()).rejects.toThrow("Set error"); + setMock.mockRestore(); + }); }); }); From 3c64e6a7337692bd7a3f886c2ea0391f515afb4c Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 19:51:59 +1300 Subject: [PATCH 08/33] Add test to gover retreiving a proof from the User class --- src/User.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/User.test.ts b/src/User.test.ts index 202fe33a1..0acde958e 100644 --- a/src/User.test.ts +++ b/src/User.test.ts @@ -45,4 +45,12 @@ describe("User", () => { expect(subject.scopes).toEqual(["foo", "bar", "baz"]); }); }); + + describe("dpopProof", () => { + it("should return a DPoP proof", async () => { + const subject = new User({ scope: "foo" } as never); + const dpopProof = await subject.dpopProof("http://example.com", "GET"); + expect(dpopProof).toEqual(expect.any(String)); + }); + }); }); From ec37bf9023efdd966c74cf4c9619e56be2d7d089 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 4 Apr 2024 19:56:56 +1300 Subject: [PATCH 09/33] Remove unneccesary nonce code --- docs/oidc-client-ts.api.md | 2 -- src/UserManager.ts | 10 ---------- 2 files changed, 12 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index ce5186f35..510327f03 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -955,8 +955,6 @@ export class UserManager { clearStaleState(): Promise; // (undocumented) protected readonly _client: OidcClient; - // (undocumented) - protected readonly _dpopNonceStore: WebStorageStateStore | null; get events(): UserManagerEvents; // (undocumented) protected readonly _events: UserManagerEvents; diff --git a/src/UserManager.ts b/src/UserManager.ts index 3719d4959..7505b8d0d 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -17,8 +17,6 @@ import { RefreshState } from "./RefreshState"; import type { SigninResponse } from "./SigninResponse"; import type { ExtraHeader } from "./OidcClientSettings"; import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; -import { InMemoryWebStorage } from "./InMemoryWebStorage"; -import { WebStorageStateStore } from "./WebStorageStateStore"; import { DPoPService } from "./DPoPService"; /** @@ -93,7 +91,6 @@ export class UserManager { protected readonly _events: UserManagerEvents; protected readonly _silentRenewService: SilentRenewService; protected readonly _sessionMonitor: SessionMonitor | null; - protected readonly _dpopNonceStore: WebStorageStateStore | null; public constructor(settings: UserManagerSettings, redirectNavigator?: INavigator, popupNavigator?: INavigator, iframeNavigator?: INavigator) { this.settings = new UserManagerSettingsStore(settings); @@ -116,12 +113,6 @@ export class UserManager { if (this.settings.monitorSession) { this._sessionMonitor = new SessionMonitor(this); } - - this._dpopNonceStore = null; - if (this.settings.dpopSettings.enabled) { - const store = typeof window !== "undefined" ? window.sessionStorage : new InMemoryWebStorage(); - this._dpopNonceStore = new WebStorageStateStore( { store }); - } } /** @@ -166,7 +157,6 @@ export class UserManager { await this.storeUser(null); logger.info("user removed from storage"); if (this.settings.dpopSettings.enabled) { - await this._dpopNonceStore?.remove("dpop_nonce"); await IndexedDbCryptoKeyPairStore.remove("oidc.dpop"); logger.debug("removed dpop cyptokeys from storage"); } From c9961999d8c235c29750aa29fec8dd35d3304fe1 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Fri, 5 Apr 2024 14:26:08 +1300 Subject: [PATCH 10/33] Add test to cover dpopJkt in SignInRequest settings --- src/SigninRequest.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/SigninRequest.test.ts b/src/SigninRequest.test.ts index a2e779daf..daa0c95cb 100644 --- a/src/SigninRequest.test.ts +++ b/src/SigninRequest.test.ts @@ -280,5 +280,16 @@ describe("SigninRequest", () => { // assert expect(subject.url).toContain("state=" + subject.state.id + encodeURIComponent(URL_STATE_DELIMITER + "foo")); }); + + it("should include dpop_jkt", async () => { + // arrange + settings.dpopJkt = "foo"; + + // act + subject = await SigninRequest.create(settings); + + // assert + expect(subject.url).toContain("dpop_jkt=foo"); + }); }); }); From 3e32c76c4415b6df819deda7ea687aecbe81f528 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Fri, 5 Apr 2024 14:41:06 +1300 Subject: [PATCH 11/33] Move UserManager dpop test into describe block --- src/UserManager.test.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 7a5d5cbc1..aa7139606 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -320,21 +320,21 @@ describe("UserManager", () => { expect(user).toBeInstanceOf(User); spy.mockRestore(); }); - }); - it("should return a user if DPoP enabled", async () => { - // arrange - subject.settings.dpopSettings.enabled = true; - const spy = jest.spyOn(subject["_client"], "processSigninResponse") - .mockResolvedValue({} as SigninResponse); + it("should return a user if DPoP enabled", async () => { + // arrange + subject.settings.dpopSettings.enabled = true; + const spy = jest.spyOn(subject["_client"], "processSigninResponse") + .mockResolvedValue({} as SigninResponse); - // act - const user = await subject.signinRedirectCallback("http://app/cb?state=test&code=code"); + // act + const user = await subject.signinRedirectCallback("http://app/cb?state=test&code=code"); - // assert - expect(spy).toHaveBeenCalledWith("http://app/cb?state=test&code=code", { "DPoP": expect.any(String) }); - expect(user).toBeInstanceOf(User); - spy.mockRestore(); + // assert + expect(spy).toHaveBeenCalledWith("http://app/cb?state=test&code=code", { "DPoP": expect.any(String) }); + expect(user).toBeInstanceOf(User); + spy.mockRestore(); + }); }); describe("signinResourceOwnerCredentials", () => { From e571f7f681aab33375f8674b9f44704362c7c245 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Fri, 5 Apr 2024 14:49:30 +1300 Subject: [PATCH 12/33] Add test coverage to CryptoUtils.customCalculateJwkThumbprint --- src/utils/CryptoUtils.test.ts | 38 ++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/utils/CryptoUtils.test.ts b/src/utils/CryptoUtils.test.ts index 847abebef..65a54e805 100644 --- a/src/utils/CryptoUtils.test.ts +++ b/src/utils/CryptoUtils.test.ts @@ -14,7 +14,7 @@ describe("CryptoUtils", () => { }); describe("customCalculateJwkThumbprint", () => { - it("should return a valid rfc7638 jwk thumbprint", async () => { + it("should return a valid rfc7638 jwk thumbprint for EC", async () => { const jwk = { "kty": "EC", "x": "zSau12OpG01OkWtiU8yG1ppv06v1uDrG66cNeqMWk_8", @@ -24,5 +24,41 @@ describe("CryptoUtils", () => { const jwkThumbprint = await CryptoUtils.customCalculateJwkThumbprint(jwk); expect(jwkThumbprint).toEqual("fvRy8PxXeUhrCgW4r0hAFroUAqSnmyiCncJmlCamt9g"); }); + + it("should return a valid rfc7638 jwk thumbprint for RSA", async () => { + const jwk = { + "kty": "RSA", + "e": "AQAB", + "n": "qPfgaTEWEP3S9w0tgsicURfo-nLW09_0KfOPinhYZ4ouzU-3xC4pSlEp8Ut9FgL0AgqNslNaK34Kq-NZjO9DAQ==", + } as JsonWebKey; + const jwkThumbprint = await CryptoUtils.customCalculateJwkThumbprint(jwk); + expect(jwkThumbprint).toEqual("PY09-sc60ozeKeWZNizkBrT_081gcm9YHwIRyZ__3_0"); + }); + + it("should return a valid rfc7638 jwk thumbprint for OKP", async () => { + const jwk = { + "kty": "OKP", + "x": "zSau12OpG01OkWtiU8yG1ppv06v1uDrG66cNeqMWk_8", + "crv": "P-256", + } as JsonWebKey; + const jwkThumbprint = await CryptoUtils.customCalculateJwkThumbprint(jwk); + expect(jwkThumbprint).toEqual("2UtIWugaDqeNZtm87tNRSAnJ6XiFqhRLf_BRFNFs9T8"); + }); + + it("should return a valid rfc7638 jwk thumbprint for oct", async () => { + const jwk = { + "kty": "OKP", + "crv": "P-256", + } as JsonWebKey; + const jwkThumbprint = await CryptoUtils.customCalculateJwkThumbprint(jwk); + expect(jwkThumbprint).toEqual("b0HfX8_AYRZlfiRj51oOHHL9BrTz0Q4z6AqSk-2nyJM"); + }); + + it("should throw an error for an unknown jwk type", async () => { + const jwk = { + "kty": "unknown", + } as JsonWebKey; + await expect(CryptoUtils.customCalculateJwkThumbprint(jwk)).rejects.toThrow("Unknown jwk type"); + }); }); }); From 2a8e78b0d50a9cf25e8be0e581aa9cce91dc411f Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Wed, 10 Apr 2024 19:28:45 +1200 Subject: [PATCH 13/33] Refactor DPoPService to non static class --- docs/oidc-client-ts.api.md | 7 +++++-- src/DPoPService.test.ts | 26 ++++++++++++++------------ src/DPoPService.ts | 11 +++++++---- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 510327f03..e45839180 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -906,8 +906,6 @@ export class User { url_state?: string; }); access_token: string; - // (undocumented) - dpopProof(url: string, httpMethod?: string): Promise; get expired(): boolean | undefined; expires_at?: number; get expires_in(): number | undefined; @@ -955,6 +953,11 @@ export class UserManager { clearStaleState(): Promise; // (undocumented) protected readonly _client: OidcClient; + dpopProof(url: string, user: User, httpMethod?: string): Promise; + // Warning: (ae-forgotten-export) The symbol "DPoPService" needs to be exported by the entry point index.d.ts + // + // (undocumented) + protected readonly _dpopService: DPoPService | null; get events(): UserManagerEvents; // (undocumented) protected readonly _events: UserManagerEvents; diff --git a/src/DPoPService.test.ts b/src/DPoPService.test.ts index ce29b80df..76b023b2b 100644 --- a/src/DPoPService.test.ts +++ b/src/DPoPService.test.ts @@ -3,14 +3,16 @@ import { jwtVerify, decodeProtectedHeader, importJWK, type JWK } from "jose"; import { IndexedDbCryptoKeyPairStore as idb } from "./IndexedDbCryptoKeyPairStore"; describe("DPoPService", () => { + let subject: DPoPService; beforeEach(async () => { await idb.remove("oidc.dpop"); + subject = new DPoPService(); }); describe("generateDPoPProof", () => { it("should generate a valid proof without an access token", async () => { - const proof = await DPoPService.generateDPoPProof({ url: "http://example.com" }); + const proof = await subject.generateDPoPProof({ url: "http://example.com" }); const protectedHeader = decodeProtectedHeader(proof); const publicKey = await importJWK(protectedHeader.jwk); const verifiedResult = await jwtVerify(proof, publicKey); @@ -20,8 +22,8 @@ describe("DPoPService", () => { }); it("should generate a valid proof with an access token", async () => { - await DPoPService.generateDPoPProof({ url: "http://example.com" }); - const proof = await DPoPService.generateDPoPProof({ url: "http://example.com", accessToken: "some_access_token" }); + await subject.generateDPoPProof({ url: "http://example.com" }); + const proof = await subject.generateDPoPProof({ url: "http://example.com", accessToken: "some_access_token" }); const protectedHeader = decodeProtectedHeader(proof); const publicKey = await importJWK(protectedHeader.jwk); @@ -35,14 +37,14 @@ describe("DPoPService", () => { it("should throw an exception if there is an error exporting the public key", async () => { const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockRejectedValue(new TypeError("Export key error")); - await expect(DPoPService.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Error exporting dpop public key: Export key error"); + await expect(subject.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Error exporting dpop public key: Export key error"); exportKeyMock.mockRestore(); }); it("should throw an exception if there is an error generating the signed JWT", async () => { const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockResolvedValue({} as JsonWebKey); const generateSignedJwtMock = jest.spyOn(crypto.subtle, "sign").mockRejectedValue(new Error("Generate signed JWT error")); - await expect(DPoPService.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Generate signed JWT error"); + await expect(subject.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Generate signed JWT error"); exportKeyMock.mockRestore(); generateSignedJwtMock.mockRestore(); }); @@ -51,20 +53,20 @@ describe("DPoPService", () => { describe("dpopJkt", () => { it("should generate crypto keys when generating a dpop thumbprint if no keys exists in the store", async () => { const setMock = jest.spyOn(idb, "set"); - await DPoPService.generateDPoPJkt(); + await subject.generateDPoPJkt(); expect(setMock).toHaveBeenCalled(); }); it("should throw a TypeError exception if the keys cannot be retrieved from the store", async () => { const getMock = jest.spyOn(idb, "get").mockRejectedValue(new TypeError("Export key error")); - await DPoPService.generateDPoPProof({ url: "http://example.com" }); - await expect(DPoPService.generateDPoPJkt()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); + await subject.generateDPoPProof({ url: "http://example.com" }); + await expect(subject.generateDPoPJkt()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); getMock.mockRestore(); }); it("should throw an exception for any other reason", async () => { const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockRejectedValue(new Error("Export key error")); - await expect(DPoPService.generateDPoPJkt()).rejects.toThrow("Export key error"); + await expect(subject.generateDPoPJkt()).rejects.toThrow("Export key error"); exportKeyMock.mockRestore(); }); }); @@ -72,14 +74,14 @@ describe("DPoPService", () => { describe("loadKeyPair", () => { it("should throw an exception if the keys cannot be retrieved from the store", async () => { const getMock = jest.spyOn(idb, "get").mockRejectedValue(new TypeError("Export key error")); - await DPoPService.generateDPoPProof({ url: "http://example.com" }); - await expect(DPoPService["loadKeyPair"]()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); + await subject.generateDPoPProof({ url: "http://example.com" }); + await expect(subject["loadKeyPair"]()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); getMock.mockRestore(); }); it("should throw an exception for any other reason", async () => { const setMock = jest.spyOn(idb, "set").mockRejectedValue(new Error("Set error")); - await expect(DPoPService["loadKeyPair"]()).rejects.toThrow("Set error"); + await expect(subject["loadKeyPair"]()).rejects.toThrow("Set error"); setMock.mockRestore(); }); }); diff --git a/src/DPoPService.ts b/src/DPoPService.ts index b01e427fa..a9e934baa 100644 --- a/src/DPoPService.ts +++ b/src/DPoPService.ts @@ -12,7 +12,10 @@ export interface GenerateDPoPProofOpts { httpMethod?: string; } export class DPoPService { - public static async generateDPoPProof({ + + public constructor() {} + + public async generateDPoPProof({ url, accessToken, httpMethod, @@ -57,7 +60,7 @@ export class DPoPService { } } - public static async generateDPoPJkt() : Promise { + public async generateDPoPJkt() : Promise { try { const keyPair = await this.loadKeyPair(); const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); @@ -71,7 +74,7 @@ export class DPoPService { } } - protected static async loadKeyPair() : Promise { + protected async loadKeyPair() : Promise { try { const allKeys = await IndexedDbCryptoKeyPairStore.getAllKeys(); let keyPair: CryptoKeyPair; @@ -91,7 +94,7 @@ export class DPoPService { } } - protected static async generateKeys() : Promise { + protected async generateKeys() : Promise { return await window.crypto.subtle.generateKey( { name: "ECDSA", From 101871ade1073fd72e0ba630a83ca664c8a7fd22 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Wed, 10 Apr 2024 19:29:55 +1200 Subject: [PATCH 14/33] Remove dpopProof method --- src/User.test.ts | 8 -------- src/User.ts | 5 ----- 2 files changed, 13 deletions(-) diff --git a/src/User.test.ts b/src/User.test.ts index 0acde958e..202fe33a1 100644 --- a/src/User.test.ts +++ b/src/User.test.ts @@ -45,12 +45,4 @@ describe("User", () => { expect(subject.scopes).toEqual(["foo", "bar", "baz"]); }); }); - - describe("dpopProof", () => { - it("should return a DPoP proof", async () => { - const subject = new User({ scope: "foo" } as never); - const dpopProof = await subject.dpopProof("http://example.com", "GET"); - expect(dpopProof).toEqual(expect.any(String)); - }); - }); }); diff --git a/src/User.ts b/src/User.ts index 209420d74..67a07c9cf 100644 --- a/src/User.ts +++ b/src/User.ts @@ -3,7 +3,6 @@ import { Logger, Timer } from "./utils"; import type { IdTokenClaims } from "./Claims"; -import { DPoPService } from "./DPoPService"; /** * Holds claims represented by a combination of the `id_token` and the user info endpoint. @@ -125,8 +124,4 @@ export class User { Logger.createStatic("User", "fromStorageString"); return new User(JSON.parse(storageString)); } - - public async dpopProof(url: string, httpMethod?: string): Promise { - return await DPoPService.generateDPoPProof({ url, accessToken: this.access_token, httpMethod: httpMethod } ); - } } From b998075d2bc8ef9212788d0e2e78c8c8644da9c6 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Wed, 10 Apr 2024 19:30:17 +1200 Subject: [PATCH 15/33] Add dpopProof method for external consumers --- src/UserManager.test.ts | 16 ++++++++++++++++ src/UserManager.ts | 40 ++++++++++++++++++++++++++++++---------- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index aa7139606..c23b064e4 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -44,6 +44,9 @@ describe("UserManager", () => { token_endpoint: "http://sts/oidc/token", revocation_endpoint: "http://sts/oidc/revoke", }, + dpopSettings: { + enabled: true, + }, }); }); @@ -1148,4 +1151,17 @@ describe("UserManager", () => { expect(storageString).toBeNull(); }); }); + + describe("dpopProof", () => { + it("should return a DPoP proof", async () => { + // arrange + const user = new User({ + access_token: "access_token", + token_type: "token_type", + profile: {} as UserProfile, + }); + const dpopProof = await subject.dpopProof("http://example.com", user); + expect(dpopProof).toEqual(expect.any(String)); + }); + }); }); diff --git a/src/UserManager.ts b/src/UserManager.ts index 7505b8d0d..570a47be1 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -91,6 +91,7 @@ export class UserManager { protected readonly _events: UserManagerEvents; protected readonly _silentRenewService: SilentRenewService; protected readonly _sessionMonitor: SessionMonitor | null; + protected readonly _dpopService: DPoPService | null; public constructor(settings: UserManagerSettings, redirectNavigator?: INavigator, popupNavigator?: INavigator, iframeNavigator?: INavigator) { this.settings = new UserManagerSettingsStore(settings); @@ -113,6 +114,11 @@ export class UserManager { if (this.settings.monitorSession) { this._sessionMonitor = new SessionMonitor(this); } + + this._dpopService = null; + if (this.settings.dpopSettings.enabled) { + this._dpopService = new DPoPService(); + } } /** @@ -248,8 +254,8 @@ export class UserManager { logger.throw(new Error("No popup_redirect_uri configured")); } - if (this.settings.dpopSettings.enabled && this.settings.dpopSettings.bind_authorization_code) { - dpopJkt = await DPoPService.generateDPoPJkt(); + if (this._dpopService && this.settings.dpopSettings.bind_authorization_code) { + dpopJkt = await this._dpopService.generateDPoPJkt(); } const handle = await this._popupNavigator.prepare({ popupWindowFeatures, popupWindowTarget }); @@ -302,9 +308,9 @@ export class UserManager { logger.debug("using refresh token"); const state = new RefreshState(user as Required); const extraHeaders: Record = {}; - if (this.settings.dpopSettings.enabled) { + if (this._dpopService) { const url = await this.metadataService.getTokenEndpoint(false); - extraHeaders["DPoP"] = await DPoPService.generateDPoPProof({ url, httpMethod: "POST" }); + extraHeaders["DPoP"] = await this._dpopService.generateDPoPProof({ url, httpMethod: "POST" }); } return await this._useRefreshToken({ state, @@ -329,8 +335,8 @@ export class UserManager { } const handle = await this._iframeNavigator.prepare({ silentRequestTimeoutInSeconds }); - if (this.settings.dpopSettings.enabled && this.settings.dpopSettings.bind_authorization_code) { - dpopJkt = await DPoPService.generateDPoPJkt(); + if (this._dpopService && this.settings.dpopSettings.bind_authorization_code) { + dpopJkt = await this._dpopService.generateDPoPJkt(); } user = await this._signin({ request_type: "si:s", @@ -432,6 +438,20 @@ export class UserManager { } } + /** + * Dynamically generates a DPoP proof for a given user, URL and optional Http method. + * This method is useful when you need to make a request to a resource server + * with fetch or similar, and you need to include a DPoP proof in a DPoP header. + * @param url - The URL to generate the DPoP proof for + * @param user - The user object to generate the DPoP proof for + * @param httpMethod - Optional, defaults to "GET" + * + * @returns A promise containing the DPoP proof + */ + public async dpopProof(url: string, user: User, httpMethod?: string): Promise { + return await this._dpopService?.generateDPoPProof({ url, accessToken: user.access_token, httpMethod: httpMethod }); + } + /** * Query OP for user's current signin status. * @@ -462,9 +482,9 @@ export class UserManager { }, handle); try { const extraHeaders: Record = {}; - if (this.settings.dpopSettings.enabled) { + if (this._dpopService) { const tokenUrl = await this.metadataService.getTokenEndpoint(false); - extraHeaders["DPoP"] = await DPoPService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); + extraHeaders["DPoP"] = await this._dpopService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); } const signinResponse = await this._client.processSigninResponse(navResponse.url, extraHeaders); logger.debug("got signin response"); @@ -525,9 +545,9 @@ export class UserManager { protected async _signinEnd(url: string, verifySub?: string): Promise { const logger = this._logger.create("_signinEnd"); const extraHeaders: Record = {}; - if (this.settings.dpopSettings.enabled) { + if (this._dpopService) { const tokenUrl = await this.metadataService.getTokenEndpoint(false); - extraHeaders["DPoP"] = await DPoPService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); + extraHeaders["DPoP"] = await this._dpopService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); } const signinResponse = await this._client.processSigninResponse(url, extraHeaders); logger.debug("got signin response"); From a15232801059fc9c57f9f08de3e9574608da2510 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Mon, 15 Apr 2024 10:08:04 +1200 Subject: [PATCH 16/33] Create DPoPStorageStateStore that extends WebStorageStateStore and implements StateStore interface --- docs/oidc-client-ts.api.md | 18 +++++++------- src/DPoPService.test.ts | 3 ++- src/DPoPService.ts | 13 ++++++---- src/DPoPStorageStateStore.ts | 43 ++++++++++++++++++++++++++++++++++ src/OidcClientSettings.test.ts | 4 ++-- src/OidcClientSettings.ts | 4 ++-- src/State.ts | 2 +- src/StateStore.ts | 8 +++---- src/UserManager.ts | 9 +++---- src/UserManagerSettings.ts | 2 +- src/WebStorageStateStore.ts | 2 +- 11 files changed, 76 insertions(+), 32 deletions(-) create mode 100644 src/DPoPStorageStateStore.ts diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index e45839180..a8db127e6 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -378,7 +378,7 @@ export interface OidcClientSettings { scope?: string; signingKeys?: SigningKey[]; staleStateAgeInSeconds?: number; - stateStore?: StateStore; + stateStore?: StateStore; ui_locales?: string; } @@ -446,7 +446,7 @@ export class OidcClientSettingsStore { // (undocumented) readonly staleStateAgeInSeconds: number; // (undocumented) - readonly stateStore: StateStore; + readonly stateStore: StateStore; // (undocumented) readonly ui_locales: string | undefined; } @@ -863,7 +863,7 @@ export class State { url_state?: string; }); // (undocumented) - static clearStaleState(storage: StateStore, age: number): Promise; + static clearStaleState(storage: StateStore, age: number): Promise; // (undocumented) readonly created: number; readonly data?: unknown; @@ -880,15 +880,15 @@ export class State { } // @public (undocumented) -export interface StateStore { +export interface StateStore { // (undocumented) - get(key: string): Promise; + get(key: string): Promise; // (undocumented) getAllKeys(): Promise; // (undocumented) - remove(key: string): Promise; + remove(key: string): Promise; // (undocumented) - set(key: string, value: string): Promise; + set(key: string, value: T): Promise; } // @public (undocumented) @@ -1135,7 +1135,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { // (undocumented) readonly stopCheckSessionOnError: boolean; // (undocumented) - readonly userStore: WebStorageStateStore; + userStore: WebStorageStateStore; // (undocumented) readonly validateSubOnSilentRenew: boolean; } @@ -1159,7 +1159,7 @@ export type UserUnloadedCallback = () => Promise | void; export const Version: string; // @public (undocumented) -export class WebStorageStateStore implements StateStore { +export class WebStorageStateStore implements StateStore { constructor({ prefix, store, }?: { prefix?: string; store?: AsyncStorage | Storage; diff --git a/src/DPoPService.test.ts b/src/DPoPService.test.ts index 76b023b2b..13c1171b9 100644 --- a/src/DPoPService.test.ts +++ b/src/DPoPService.test.ts @@ -1,13 +1,14 @@ import { DPoPService } from "./DPoPService"; import { jwtVerify, decodeProtectedHeader, importJWK, type JWK } from "jose"; import { IndexedDbCryptoKeyPairStore as idb } from "./IndexedDbCryptoKeyPairStore"; +import { DPoPStorageStateStore } from "./DPoPStorageStateStore"; describe("DPoPService", () => { let subject: DPoPService; beforeEach(async () => { await idb.remove("oidc.dpop"); - subject = new DPoPService(); + subject = new DPoPService(new DPoPStorageStateStore()); }); describe("generateDPoPProof", () => { diff --git a/src/DPoPService.ts b/src/DPoPService.ts index a9e934baa..7b1728b4e 100644 --- a/src/DPoPService.ts +++ b/src/DPoPService.ts @@ -1,5 +1,5 @@ import { CryptoUtils, JwtUtils } from "./utils"; -import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; +import { DPoPStorageStateStore } from "./DPoPStorageStateStore"; /** * Provides an implementation of Demonstrating Proof of Possession (DPoP) as defined in the @@ -12,8 +12,11 @@ export interface GenerateDPoPProofOpts { httpMethod?: string; } export class DPoPService { + readonly _dpopStorageStateStore; - public constructor() {} + public constructor(dpopStorageStateStore: DPoPStorageStateStore) { + this._dpopStorageStateStore = dpopStorageStateStore; + } public async generateDPoPProof({ url, @@ -76,13 +79,13 @@ export class DPoPService { protected async loadKeyPair() : Promise { try { - const allKeys = await IndexedDbCryptoKeyPairStore.getAllKeys(); + const allKeys = await this._dpopStorageStateStore.getAllKeys(); let keyPair: CryptoKeyPair; if (!allKeys.includes("oidc.dpop")) { keyPair = await this.generateKeys(); - await IndexedDbCryptoKeyPairStore.set("oidc.dpop", keyPair); + await this._dpopStorageStateStore.set("oidc.dpop", keyPair); } else { - keyPair = await IndexedDbCryptoKeyPairStore.get("oidc.dpop") as CryptoKeyPair; + keyPair = await this._dpopStorageStateStore.getDpop("oidc.dpop") as CryptoKeyPair; } return keyPair; } catch (err) { diff --git a/src/DPoPStorageStateStore.ts b/src/DPoPStorageStateStore.ts new file mode 100644 index 000000000..4da846581 --- /dev/null +++ b/src/DPoPStorageStateStore.ts @@ -0,0 +1,43 @@ +import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; +import { WebStorageStateStore } from "./WebStorageStateStore"; +import type { AsyncStorage } from "./AsyncStorage"; +import type { StateStore } from "./StateStore"; + +export class DPoPStorageStateStore extends WebStorageStateStore implements StateStore { + readonly dpop_key: string = "oidc.dpop"; + + public constructor({ + prefix = "oidc.", + store = localStorage, + }: { prefix?: string; store?: AsyncStorage | Storage } = {}) { + super({ prefix, store }); + } + + public async set(key: string, value: string | CryptoKeyPair): Promise { + if (key === this.dpop_key) { + await IndexedDbCryptoKeyPairStore.set(key, value as CryptoKeyPair); + } else { + await super.set(key, value as string); + } + } + + public async get(key: string): Promise { + return await super.get(key); + } + + public async getDpop(key: string): Promise { + return await IndexedDbCryptoKeyPairStore.get(key); + } + + public async remove(key: string): Promise { + await IndexedDbCryptoKeyPairStore.remove(this.dpop_key); + return await super.remove(key); + } + + public async getAllKeys(): Promise { + const keys = await super.getAllKeys(); + const dpopKeys = await IndexedDbCryptoKeyPairStore.getAllKeys(); + keys.push(...dpopKeys); + return keys; + } +} diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts index 9bd1adb2c..efca4f0b2 100644 --- a/src/OidcClientSettings.test.ts +++ b/src/OidcClientSettings.test.ts @@ -405,7 +405,7 @@ describe("OidcClientSettings", () => { it("should return value from initial settings", () => { // arrange - const temp = {} as StateStore; + const temp = {} as StateStore; // act const subject = new OidcClientSettingsStore({ @@ -424,7 +424,7 @@ describe("OidcClientSettings", () => { it("should return value from initial settings", () => { // arrange - const temp = {} as StateStore; + const temp = {} as StateStore; // act const subject = new OidcClientSettingsStore({ diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 5e915ef1a..0315b6aa6 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -104,7 +104,7 @@ export interface OidcClientSettings { * Storage object used to persist interaction state (default: window.localStorage, InMemoryWebStorage iff no window). * E.g. `stateStore: new WebStorageStateStore({ store: window.localStorage })` */ - stateStore?: StateStore; + stateStore?: StateStore; /** * An object containing additional query string parameters to be including in the authorization request. @@ -177,7 +177,7 @@ export class OidcClientSettingsStore { public readonly staleStateAgeInSeconds: number; public readonly mergeClaimsStrategy: { array: "replace" | "merge" }; - public readonly stateStore: StateStore; + public readonly stateStore: StateStore; // extra public readonly extraQueryParams: Record; diff --git a/src/State.ts b/src/State.ts index b9294b1cd..0c0f61116 100644 --- a/src/State.ts +++ b/src/State.ts @@ -52,7 +52,7 @@ export class State { return Promise.resolve(new State(JSON.parse(storageString))); } - public static async clearStaleState(storage: StateStore, age: number): Promise { + public static async clearStaleState(storage: StateStore, age: number): Promise { const logger = Logger.createStatic("State", "clearStaleState"); const cutoff = Timer.getEpochTime() - age; diff --git a/src/StateStore.ts b/src/StateStore.ts index 6271499a7..073e50b69 100644 --- a/src/StateStore.ts +++ b/src/StateStore.ts @@ -1,9 +1,9 @@ /** * @public */ -export interface StateStore { - set(key: string, value: string): Promise; - get(key: string): Promise; - remove(key: string): Promise; +export interface StateStore { + set(key: string, value: T): Promise; + get(key: string): Promise; + remove(key: string): Promise; getAllKeys(): Promise; } diff --git a/src/UserManager.ts b/src/UserManager.ts index 570a47be1..5811e817b 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -16,8 +16,8 @@ import type { MetadataService } from "./MetadataService"; import { RefreshState } from "./RefreshState"; import type { SigninResponse } from "./SigninResponse"; import type { ExtraHeader } from "./OidcClientSettings"; -import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; import { DPoPService } from "./DPoPService"; +import { DPoPStorageStateStore } from "./DPoPStorageStateStore"; /** * @public @@ -117,7 +117,8 @@ export class UserManager { this._dpopService = null; if (this.settings.dpopSettings.enabled) { - this._dpopService = new DPoPService(); + this.settings.userStore = new DPoPStorageStateStore(); + this._dpopService = new DPoPService(this.settings.userStore as DPoPStorageStateStore); } } @@ -162,10 +163,6 @@ export class UserManager { const logger = this._logger.create("removeUser"); await this.storeUser(null); logger.info("user removed from storage"); - if (this.settings.dpopSettings.enabled) { - await IndexedDbCryptoKeyPairStore.remove("oidc.dpop"); - logger.debug("removed dpop cyptokeys from storage"); - } await this._events.unload(); } diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index 855708485..f65b34fa7 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -129,7 +129,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { public readonly accessTokenExpiringNotificationTimeInSeconds: number; - public readonly userStore: WebStorageStateStore; + public userStore: WebStorageStateStore; public constructor(args: UserManagerSettings) { const { diff --git a/src/WebStorageStateStore.ts b/src/WebStorageStateStore.ts index 112408942..13ec9aa52 100644 --- a/src/WebStorageStateStore.ts +++ b/src/WebStorageStateStore.ts @@ -8,7 +8,7 @@ import type { AsyncStorage } from "./AsyncStorage"; /** * @public */ -export class WebStorageStateStore implements StateStore { +export class WebStorageStateStore implements StateStore { private readonly _logger = new Logger("WebStorageStateStore"); private readonly _store: AsyncStorage | Storage; From 0a730788a2bf35bc8a526e961cda8355d1942fa7 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 13:47:40 +1200 Subject: [PATCH 17/33] Re-organise dpop code into CyrptoUtils, DPoPStore and OidcClient --- docs/oidc-client-ts.api.md | 19 ++- src/DPoPService.test.ts | 89 ---------- src/DPoPService.ts | 110 ------------ src/DPoPStorageStateStore.ts | 43 ----- ...KeyPairStore.test.ts => DPoPStore.test.ts} | 7 +- ...edDbCryptoKeyPairStore.ts => DPoPStore.ts} | 45 +++-- src/OidcClient.ts | 35 +++- src/OidcClientSettings.ts | 8 + src/StateStore.ts | 2 +- src/UserManager.test.ts | 156 +----------------- src/UserManager.ts | 105 ++++++------ src/UserManagerSettings.test.ts | 16 -- src/UserManagerSettings.ts | 3 - src/utils/CryptoUtils.ts | 76 +++++++++ 14 files changed, 211 insertions(+), 503 deletions(-) delete mode 100644 src/DPoPService.test.ts delete mode 100644 src/DPoPService.ts delete mode 100644 src/DPoPStorageStateStore.ts rename src/{IndexedDbCryptoKeyPairStore.test.ts => DPoPStore.test.ts} (94%) rename src/{IndexedDbCryptoKeyPairStore.ts => DPoPStore.ts} (70%) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index a8db127e6..3cb181d44 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -306,6 +306,12 @@ export class OidcClient { createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; + // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts + // + // (undocumented) + protected readonly _dpopStore: DPoPStore | undefined; + // (undocumented) + getDpopProof(dpopStore: DPoPStore): Promise; // (undocumented) protected readonly _logger: Logger; // (undocumented) @@ -352,6 +358,7 @@ export interface OidcClientSettings { client_secret?: string; disablePKCE?: boolean; display?: string; + dpop?: boolean; extraHeaders?: Record; extraQueryParams?: Record; // (undocumented) @@ -384,7 +391,7 @@ export interface OidcClientSettings { // @public export class OidcClientSettingsStore { - constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, }: OidcClientSettings); + constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, dpop, }: OidcClientSettings); // (undocumented) readonly acr_values: string | undefined; // (undocumented) @@ -400,6 +407,8 @@ export class OidcClientSettingsStore { // (undocumented) readonly display: string | undefined; // (undocumented) + readonly dpop: boolean | undefined; + // (undocumented) readonly extraHeaders: Record; // (undocumented) readonly extraQueryParams: Record; @@ -880,7 +889,7 @@ export class State { } // @public (undocumented) -export interface StateStore { +export interface StateStore { // (undocumented) get(key: string): Promise; // (undocumented) @@ -954,10 +963,8 @@ export class UserManager { // (undocumented) protected readonly _client: OidcClient; dpopProof(url: string, user: User, httpMethod?: string): Promise; - // Warning: (ae-forgotten-export) The symbol "DPoPService" needs to be exported by the entry point index.d.ts - // // (undocumented) - protected readonly _dpopService: DPoPService | null; + protected readonly _dpopStore: DPoPStore | undefined; get events(): UserManagerEvents; // (undocumented) protected readonly _events: UserManagerEvents; @@ -1097,8 +1104,6 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { // (undocumented) readonly checkSessionIntervalInSeconds: number; // (undocumented) - readonly dpopSettings: DPoPSettings; - // (undocumented) readonly iframeNotifyParentOrigin: string | undefined; // (undocumented) readonly iframeScriptOrigin: string | undefined; diff --git a/src/DPoPService.test.ts b/src/DPoPService.test.ts deleted file mode 100644 index 13c1171b9..000000000 --- a/src/DPoPService.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { DPoPService } from "./DPoPService"; -import { jwtVerify, decodeProtectedHeader, importJWK, type JWK } from "jose"; -import { IndexedDbCryptoKeyPairStore as idb } from "./IndexedDbCryptoKeyPairStore"; -import { DPoPStorageStateStore } from "./DPoPStorageStateStore"; - -describe("DPoPService", () => { - let subject: DPoPService; - - beforeEach(async () => { - await idb.remove("oidc.dpop"); - subject = new DPoPService(new DPoPStorageStateStore()); - }); - - describe("generateDPoPProof", () => { - it("should generate a valid proof without an access token", async () => { - const proof = await subject.generateDPoPProof({ url: "http://example.com" }); - const protectedHeader = decodeProtectedHeader(proof); - const publicKey = await importJWK(protectedHeader.jwk); - const verifiedResult = await jwtVerify(proof, publicKey); - - expect(verifiedResult.payload).toHaveProperty("htu"); - expect(verifiedResult.payload).toHaveProperty("htm"); - }); - - it("should generate a valid proof with an access token", async () => { - await subject.generateDPoPProof({ url: "http://example.com" }); - const proof = await subject.generateDPoPProof({ url: "http://example.com", accessToken: "some_access_token" }); - - const protectedHeader = decodeProtectedHeader(proof); - const publicKey = await importJWK(protectedHeader.jwk); - const verifiedResult = await jwtVerify(proof, publicKey); - - expect(verifiedResult.payload).toHaveProperty("htu"); - expect(verifiedResult.payload).toHaveProperty("htm"); - expect(verifiedResult.payload).toHaveProperty("ath"); - expect(verifiedResult.payload["htu"]).toEqual("http://example.com"); - }); - - it("should throw an exception if there is an error exporting the public key", async () => { - const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockRejectedValue(new TypeError("Export key error")); - await expect(subject.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Error exporting dpop public key: Export key error"); - exportKeyMock.mockRestore(); - }); - - it("should throw an exception if there is an error generating the signed JWT", async () => { - const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockResolvedValue({} as JsonWebKey); - const generateSignedJwtMock = jest.spyOn(crypto.subtle, "sign").mockRejectedValue(new Error("Generate signed JWT error")); - await expect(subject.generateDPoPProof({ url: "http://example.com" })).rejects.toThrow("Generate signed JWT error"); - exportKeyMock.mockRestore(); - generateSignedJwtMock.mockRestore(); - }); - }); - - describe("dpopJkt", () => { - it("should generate crypto keys when generating a dpop thumbprint if no keys exists in the store", async () => { - const setMock = jest.spyOn(idb, "set"); - await subject.generateDPoPJkt(); - expect(setMock).toHaveBeenCalled(); - }); - - it("should throw a TypeError exception if the keys cannot be retrieved from the store", async () => { - const getMock = jest.spyOn(idb, "get").mockRejectedValue(new TypeError("Export key error")); - await subject.generateDPoPProof({ url: "http://example.com" }); - await expect(subject.generateDPoPJkt()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); - getMock.mockRestore(); - }); - - it("should throw an exception for any other reason", async () => { - const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockRejectedValue(new Error("Export key error")); - await expect(subject.generateDPoPJkt()).rejects.toThrow("Export key error"); - exportKeyMock.mockRestore(); - }); - }); - - describe("loadKeyPair", () => { - it("should throw an exception if the keys cannot be retrieved from the store", async () => { - const getMock = jest.spyOn(idb, "get").mockRejectedValue(new TypeError("Export key error")); - await subject.generateDPoPProof({ url: "http://example.com" }); - await expect(subject["loadKeyPair"]()).rejects.toThrow("Could not retrieve dpop keys from storage: Export key error"); - getMock.mockRestore(); - }); - - it("should throw an exception for any other reason", async () => { - const setMock = jest.spyOn(idb, "set").mockRejectedValue(new Error("Set error")); - await expect(subject["loadKeyPair"]()).rejects.toThrow("Set error"); - setMock.mockRestore(); - }); - }); -}); diff --git a/src/DPoPService.ts b/src/DPoPService.ts deleted file mode 100644 index 7b1728b4e..000000000 --- a/src/DPoPService.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { CryptoUtils, JwtUtils } from "./utils"; -import { DPoPStorageStateStore } from "./DPoPStorageStateStore"; - -/** - * Provides an implementation of Demonstrating Proof of Possession (DPoP) as defined in the - * OAuth2 spec https://datatracker.ietf.org/doc/html/rfc9449. - */ - -export interface GenerateDPoPProofOpts { - url: string; - accessToken?: string; - httpMethod?: string; -} -export class DPoPService { - readonly _dpopStorageStateStore; - - public constructor(dpopStorageStateStore: DPoPStorageStateStore) { - this._dpopStorageStateStore = dpopStorageStateStore; - } - - public async generateDPoPProof({ - url, - accessToken, - httpMethod, - }: GenerateDPoPProofOpts): Promise { - let hashedToken: Uint8Array; - let encodedHash: string; - - const payload: Record = { - "jti": window.crypto.randomUUID(), - "htm": httpMethod ?? "GET", - "htu": url, - "iat": Math.floor(Date.now() / 1000), - }; - - const keyPair = await this.loadKeyPair(); - - if (accessToken) { - hashedToken = await CryptoUtils.hash("SHA-256", accessToken); - encodedHash = CryptoUtils.encodeBase64Url(hashedToken); - payload.ath = encodedHash; - } - - try { - const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); - const header = { - "alg": "ES256", - "typ": "dpop+jwt", - "jwk": { - "crv": publicJwk.crv, - "kty": publicJwk.kty, - "x": publicJwk.x, - "y": publicJwk.y, - }, - }; - return await JwtUtils.generateSignedJwt(header, payload, keyPair.privateKey); - } catch (err) { - if (err instanceof TypeError) { - throw new Error(`Error exporting dpop public key: ${err.message}`); - } else { - throw err; - } - } - } - - public async generateDPoPJkt() : Promise { - try { - const keyPair = await this.loadKeyPair(); - const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); - return await CryptoUtils.customCalculateJwkThumbprint(publicJwk); - } catch (err) { - if (err instanceof TypeError) { - throw new Error(`Could not retrieve dpop keys from storage: ${err.message}`); - } else { - throw err; - } - } - } - - protected async loadKeyPair() : Promise { - try { - const allKeys = await this._dpopStorageStateStore.getAllKeys(); - let keyPair: CryptoKeyPair; - if (!allKeys.includes("oidc.dpop")) { - keyPair = await this.generateKeys(); - await this._dpopStorageStateStore.set("oidc.dpop", keyPair); - } else { - keyPair = await this._dpopStorageStateStore.getDpop("oidc.dpop") as CryptoKeyPair; - } - return keyPair; - } catch (err) { - if (err instanceof TypeError) { - throw new Error(`Could not retrieve dpop keys from storage: ${err.message}`); - } else { - throw err; - } - } - } - - protected async generateKeys() : Promise { - return await window.crypto.subtle.generateKey( - { - name: "ECDSA", - namedCurve: "P-256", - }, - false, - ["sign", "verify"], - ); - } -} diff --git a/src/DPoPStorageStateStore.ts b/src/DPoPStorageStateStore.ts deleted file mode 100644 index 4da846581..000000000 --- a/src/DPoPStorageStateStore.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { IndexedDbCryptoKeyPairStore } from "./IndexedDbCryptoKeyPairStore"; -import { WebStorageStateStore } from "./WebStorageStateStore"; -import type { AsyncStorage } from "./AsyncStorage"; -import type { StateStore } from "./StateStore"; - -export class DPoPStorageStateStore extends WebStorageStateStore implements StateStore { - readonly dpop_key: string = "oidc.dpop"; - - public constructor({ - prefix = "oidc.", - store = localStorage, - }: { prefix?: string; store?: AsyncStorage | Storage } = {}) { - super({ prefix, store }); - } - - public async set(key: string, value: string | CryptoKeyPair): Promise { - if (key === this.dpop_key) { - await IndexedDbCryptoKeyPairStore.set(key, value as CryptoKeyPair); - } else { - await super.set(key, value as string); - } - } - - public async get(key: string): Promise { - return await super.get(key); - } - - public async getDpop(key: string): Promise { - return await IndexedDbCryptoKeyPairStore.get(key); - } - - public async remove(key: string): Promise { - await IndexedDbCryptoKeyPairStore.remove(this.dpop_key); - return await super.remove(key); - } - - public async getAllKeys(): Promise { - const keys = await super.getAllKeys(); - const dpopKeys = await IndexedDbCryptoKeyPairStore.getAllKeys(); - keys.push(...dpopKeys); - return keys; - } -} diff --git a/src/IndexedDbCryptoKeyPairStore.test.ts b/src/DPoPStore.test.ts similarity index 94% rename from src/IndexedDbCryptoKeyPairStore.test.ts rename to src/DPoPStore.test.ts index 5dc6dde5f..6adda03bf 100644 --- a/src/IndexedDbCryptoKeyPairStore.test.ts +++ b/src/DPoPStore.test.ts @@ -1,7 +1,10 @@ -import { IndexedDbCryptoKeyPairStore as subject } from "./IndexedDbCryptoKeyPairStore"; +import { DPoPStore } from "./DPoPStore"; + +describe("DPoPStore", () => { + const subject = new DPoPStore(); -describe("IndexedDBCryptoKeyPairStore", () => { let data: CryptoKeyPair; + const createCryptoKeyPair = async () => { return await window.crypto.subtle.generateKey( { diff --git a/src/IndexedDbCryptoKeyPairStore.ts b/src/DPoPStore.ts similarity index 70% rename from src/IndexedDbCryptoKeyPairStore.ts rename to src/DPoPStore.ts index 63c33e2dc..1ae5ea0bd 100644 --- a/src/IndexedDbCryptoKeyPairStore.ts +++ b/src/DPoPStore.ts @@ -1,39 +1,39 @@ -export class IndexedDbCryptoKeyPairStore { - static dbName = "oidc"; - static storeName = "dpop"; +export class DPoPStore { + readonly _dbName: string = "oidc"; + readonly _storeName: string = "dpop"; - static async get(key: string): Promise { - const store = await this.createStore(this.dbName, this.storeName); - return await store("readonly", (str) => { - return this.promisifyRequest(str.get(key)); - }) as CryptoKeyPair; + public async set(key: string, value: CryptoKeyPair): Promise { + const store = await this.createStore(this._dbName, this._storeName); + await store("readwrite", (str: IDBObjectStore) => { + str.put(value, key); + return this.promisifyRequest(str.transaction); + }); } - static async getAllKeys(): Promise { - const store = await this.createStore(this.dbName, this.storeName); + public async get(key: string): Promise { + const store = await this.createStore(this._dbName, this._storeName); return await store("readonly", (str) => { - return this.promisifyRequest(str.getAllKeys()); - }) as string[]; + return this.promisifyRequest(str.get(key)); + }) as CryptoKeyPair; } - static async remove(key: string): Promise { + public async remove(key: string): Promise { const item = await this.get(key); - const store = await this.createStore(this.dbName, this.storeName); + const store = await this.createStore(this._dbName, this._storeName); await store("readwrite", (str) => { return this.promisifyRequest(str.delete(key)); }); return item; } - static async set(key: string, value: CryptoKeyPair): Promise { - const store = await this.createStore(this.dbName, this.storeName); - await store("readwrite", (str: IDBObjectStore) => { - str.put(value, key); - return this.promisifyRequest(str.transaction); - }); + public async getAllKeys(): Promise { + const store = await this.createStore(this._dbName, this._storeName); + return await store("readonly", (str) => { + return this.promisifyRequest(str.getAllKeys()); + }) as string[]; } - static promisifyRequest( + promisifyRequest( request: IDBRequest | IDBTransaction): Promise { return new Promise((resolve, reject) => { (request as IDBTransaction).oncomplete = (request as IDBRequest).onsuccess = () => resolve((request as IDBRequest).result); @@ -41,7 +41,7 @@ export class IndexedDbCryptoKeyPairStore { }); } - static async createStore( + async createStore( dbName: string, storeName: string, ): Promise<(txMode: IDBTransactionMode, callback: (store: IDBObjectStore) => T | PromiseLike) => Promise> { @@ -58,5 +58,4 @@ export class IndexedDbCryptoKeyPairStore { return await callback(store); }; } - } diff --git a/src/OidcClient.ts b/src/OidcClient.ts index b4e5dd55c..c2ef59214 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -1,7 +1,7 @@ // Copyright (c) Brock Allen & Dominick Baier. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -import { Logger, UrlUtils } from "./utils"; +import { CryptoUtils, Logger, UrlUtils } from "./utils"; import { ErrorResponse } from "./errors"; import { type ExtraHeader, type OidcClientSettings, OidcClientSettingsStore } from "./OidcClientSettings"; import { ResponseValidator } from "./ResponseValidator"; @@ -15,6 +15,7 @@ import { SigninState } from "./SigninState"; import { State } from "./State"; import { TokenClient } from "./TokenClient"; import { ClaimsService } from "./ClaimsService"; +import { DPoPStore } from "./DPoPStore"; /** * @public @@ -78,6 +79,7 @@ export class OidcClient { protected readonly _claimsService: ClaimsService; protected readonly _validator: ResponseValidator; protected readonly _tokenClient: TokenClient; + protected readonly _dpopStore: DPoPStore | undefined; public constructor(settings: OidcClientSettings); public constructor(settings: OidcClientSettingsStore, metadataService: MetadataService); @@ -88,6 +90,10 @@ export class OidcClient { this._claimsService = new ClaimsService(this.settings); this._validator = new ResponseValidator(this.settings, this.metadataService, this._claimsService); this._tokenClient = new TokenClient(this.settings, this.metadataService); + + if (this.settings.dpop) { + this._dpopStore = new DPoPStore(); + } } public async createSigninRequest({ @@ -172,10 +178,32 @@ export class OidcClient { const { state, response } = await this.readSigninResponseState(url, true); logger.debug("received state from storage; validating response"); + + if (this.settings.dpop && this._dpopStore) { + const dpopProof = await this.getDpopProof(this._dpopStore); + extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; + } await this._validator.validateSigninResponse(response, state, extraHeaders); return response; } + async getDpopProof(dpopStore: DPoPStore): Promise { + let keyPair: CryptoKeyPair; + + if (!(await dpopStore.getAllKeys()).includes(this.settings.client_id)) { + keyPair = await CryptoUtils.generateDPoPKeys(); + await dpopStore.set(this.settings.client_id, keyPair); + } else { + keyPair = await dpopStore.get(this.settings.client_id) as CryptoKeyPair; + } + + return await CryptoUtils.generateDPoPProof({ + url: await this.metadataService.getTokenEndpoint(false), + httpMethod: "POST", + keyPair: keyPair, + }); + } + public async processResourceOwnerPasswordCredentials({ username, password, @@ -212,6 +240,11 @@ export class OidcClient { scope = providedScopes.filter(s => allowableScopes.includes(s)).join(" "); } + if (this.settings.dpop && this._dpopStore) { + const dpopProof = await this.getDpopProof(this._dpopStore); + extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; + } + const result = await this._tokenClient.exchangeRefreshToken({ refresh_token: state.refresh_token, // provide the (possible filtered) scope list diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 0315b6aa6..69168df67 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -119,6 +119,11 @@ export interface OidcClientSettings { */ extraHeaders?: Record; + /** + * DPoP enabled or disabled + */ + dpop?: boolean; + /** * Will check the content type header of the response of the revocation endpoint to match these passed values (default: []) */ @@ -182,6 +187,7 @@ export class OidcClientSettingsStore { // extra public readonly extraQueryParams: Record; public readonly extraTokenParams: Record; + public readonly dpop: boolean | undefined; public readonly extraHeaders: Record; public readonly revokeTokenAdditionalContentTypes?: string[]; @@ -213,6 +219,7 @@ export class OidcClientSettingsStore { extraQueryParams = {}, extraTokenParams = {}, extraHeaders = {}, + dpop = false, }: OidcClientSettings) { this.authority = authority; @@ -271,5 +278,6 @@ export class OidcClientSettingsStore { this.extraQueryParams = extraQueryParams; this.extraTokenParams = extraTokenParams; this.extraHeaders = extraHeaders; + this.dpop = dpop; } } diff --git a/src/StateStore.ts b/src/StateStore.ts index 073e50b69..413361ae5 100644 --- a/src/StateStore.ts +++ b/src/StateStore.ts @@ -1,7 +1,7 @@ /** * @public */ -export interface StateStore { +export interface StateStore { set(key: string, value: T): Promise; get(key: string): Promise; remove(key: string): Promise; diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index c23b064e4..357e40255 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -18,7 +18,6 @@ import type { UserProfile } from "./User"; import { WebStorageStateStore } from "./WebStorageStateStore"; import type { SigninState } from "./SigninState"; import type { State } from "./State"; -import { IndexedDbCryptoKeyPairStore as idb } from "./IndexedDbCryptoKeyPairStore"; import { mocked } from "jest-mock"; @@ -174,21 +173,16 @@ describe("UserManager", () => { }); describe("removeUser", () => { - it("should remove user from store and remove dpop keys from indexedDB if dpop setting is true", async () => { + it("should remove user from store", async () => { // arrange - Object.assign(subject.settings.dpopSettings, { - enabled: true, - }); const storeUserMock = jest.spyOn(subject, "storeUser"); const unloadMock = jest.spyOn(subject["_events"], "unload"); - const delMock = jest.spyOn(idb, "remove"); // act await subject.removeUser(); // assert expect(storeUserMock).toBeCalledWith(null); expect(unloadMock).toBeCalled(); - expect(delMock).toBeCalled(); }); }); @@ -323,21 +317,6 @@ describe("UserManager", () => { expect(user).toBeInstanceOf(User); spy.mockRestore(); }); - - it("should return a user if DPoP enabled", async () => { - // arrange - subject.settings.dpopSettings.enabled = true; - const spy = jest.spyOn(subject["_client"], "processSigninResponse") - .mockResolvedValue({} as SigninResponse); - - // act - const user = await subject.signinRedirectCallback("http://app/cb?state=test&code=code"); - - // assert - expect(spy).toHaveBeenCalledWith("http://app/cb?state=test&code=code", { "DPoP": expect.any(String) }); - expect(user).toBeInstanceOf(User); - spy.mockRestore(); - }); }); describe("signinResourceOwnerCredentials", () => { @@ -386,7 +365,7 @@ describe("UserManager", () => { describe("signinPopup", () => { it("should pass navigator params to navigator", async () => { // arrange - const handle = { } as PopupWindow; + const handle = {} as PopupWindow; const prepareMock = jest.spyOn(subject["_popupNavigator"], "prepare") .mockImplementation(() => Promise.resolve(handle)); subject["_signin"] = jest.fn(); @@ -413,12 +392,12 @@ describe("UserManager", () => { token_type: "token_type", profile: {} as UserProfile, }); - const handle = { } as PopupWindow; + const handle = {} as PopupWindow; jest.spyOn(subject["_popupNavigator"], "prepare") .mockImplementation(() => Promise.resolve(handle)); subject["_signin"] = jest.fn().mockResolvedValue(user); const extraArgs: SigninPopupArgs = { - extraQueryParams: { q : "q" }, + extraQueryParams: { q: "q" }, extraTokenParams: { t: "t" }, state: "state", nonce: "random_nonce", @@ -439,43 +418,6 @@ describe("UserManager", () => { handle, ); }); - - it("should pass dpopJkt arg if dpop is enabled and bind_authorization_code is true", async () => { - // arrange - const user = new User({ - access_token: "access_token", - token_type: "token_type", - profile: {} as UserProfile, - }); - const handle = { } as PopupWindow; - jest.spyOn(subject["_popupNavigator"], "prepare") - .mockImplementation(() => Promise.resolve(handle)); - subject["_signin"] = jest.fn().mockResolvedValue(user); - const extraArgs: SigninPopupArgs = { - extraQueryParams: { q : "q" }, - extraTokenParams: { t: "t" }, - state: "state", - nonce: "random_nonce", - redirect_uri: "http://app/extra_callback", - prompt: "login", - }; - subject.settings.dpopSettings.enabled = true; - subject.settings.dpopSettings.bind_authorization_code = true; - - // act - await subject.signinPopup(extraArgs); - - // assert - expect(subject["_signin"]).toHaveBeenCalledWith( - { - request_type: "si:p", - display: "popup", - dpopJkt: expect.any(String), - ...extraArgs, - }, - handle, - ); - }); }); describe("signinPopupCallback", () => { @@ -568,45 +510,6 @@ describe("UserManager", () => { ); }); - it("should pass dpopJkt to _signIn if dpop enabled and bind_authorization_code is true", async () => { - // arrange - const user = new User({ - access_token: "access_token", - token_type: "token_type", - profile: {} as UserProfile, - }); - jest.spyOn(subject["_popupNavigator"], "prepare"); - subject["_signin"] = jest.fn().mockResolvedValue(user); - const extraArgs: SigninSilentArgs = { - extraQueryParams: { q : "q" }, - extraTokenParams: { t: "t" }, - state: "state", - nonce: "random_nonce", - redirect_uri: "http://app/extra_callback", - }; - subject.settings.dpopSettings.enabled = true; - subject.settings.dpopSettings.bind_authorization_code = true; - - // act - await subject.signinSilent(extraArgs); - - // assert - expect(subject["_signin"]).toHaveBeenCalledWith( - { - request_type: "si:s", - prompt: "none", - id_token_hint: undefined, - dpopJkt: expect.any(String), - ...extraArgs, - }, - expect.objectContaining({ - close: expect.any(Function), - navigate: expect.any(Function), - }), - undefined, - ); - }); - it("should work when having no user present", async () => { // arrange const user = new User({ @@ -659,44 +562,6 @@ describe("UserManager", () => { ); }); - it("should use DPoP when enabled when using a refresh token", async () => { - // arrange - const user = new User({ - access_token: "access_token", - token_type: "token_type", - refresh_token: "refresh_token", - profile: { - sub: "sub", - nickname: "Nick", - } as UserProfile, - }); - subject.settings.dpopSettings.enabled = true; - - const useRefreshTokenSpy = jest.spyOn(subject["_client"], "useRefreshToken").mockResolvedValue({ - access_token: "new_access_token", - profile: { - sub: "sub", - nickname: "Nicholas", - }, - } as unknown as SigninResponse); - subject["_loadUser"] = jest.fn().mockResolvedValue(user); - - // act - const refreshedUser = await subject.signinSilent(); - expect(refreshedUser).toHaveProperty("access_token", "new_access_token"); - expect(refreshedUser!.profile).toHaveProperty("nickname", "Nicholas"); - expect(useRefreshTokenSpy).toHaveBeenCalledWith( - expect.objectContaining({ - state: { - refresh_token: user.refresh_token, - session_state: null, - "profile": { "nickname": "Nick", "sub": "sub" }, - }, - extraHeaders: { "DPoP": expect.any(String) }, - }), - ); - }); - it("should use the resource from settings when a refresh token is present", async () => { // arrange const user = new User({ @@ -1151,17 +1016,4 @@ describe("UserManager", () => { expect(storageString).toBeNull(); }); }); - - describe("dpopProof", () => { - it("should return a DPoP proof", async () => { - // arrange - const user = new User({ - access_token: "access_token", - token_type: "token_type", - profile: {} as UserProfile, - }); - const dpopProof = await subject.dpopProof("http://example.com", user); - expect(dpopProof).toEqual(expect.any(String)); - }); - }); }); diff --git a/src/UserManager.ts b/src/UserManager.ts index 5811e817b..bd115f52f 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -1,7 +1,7 @@ // Copyright (c) Brock Allen & Dominick Baier. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -import { Logger } from "./utils"; +import { CryptoUtils, Logger } from "./utils"; import { ErrorResponse } from "./errors"; import { type NavigateResponse, type PopupWindowParams, type IWindow, type IFrameWindowParams, type RedirectParams, RedirectNavigator, PopupNavigator, IFrameNavigator, type INavigator } from "./navigators"; import { OidcClient, type CreateSigninRequestArgs, type CreateSignoutRequestArgs, type ProcessResourceOwnerPasswordCredentialsArgs, type UseRefreshTokenArgs } from "./OidcClient"; @@ -16,8 +16,7 @@ import type { MetadataService } from "./MetadataService"; import { RefreshState } from "./RefreshState"; import type { SigninResponse } from "./SigninResponse"; import type { ExtraHeader } from "./OidcClientSettings"; -import { DPoPService } from "./DPoPService"; -import { DPoPStorageStateStore } from "./DPoPStorageStateStore"; +import { DPoPStore } from "./DPoPStore"; /** * @public @@ -91,7 +90,7 @@ export class UserManager { protected readonly _events: UserManagerEvents; protected readonly _silentRenewService: SilentRenewService; protected readonly _sessionMonitor: SessionMonitor | null; - protected readonly _dpopService: DPoPService | null; + protected readonly _dpopStore: DPoPStore | undefined; public constructor(settings: UserManagerSettings, redirectNavigator?: INavigator, popupNavigator?: INavigator, iframeNavigator?: INavigator) { this.settings = new UserManagerSettingsStore(settings); @@ -115,10 +114,8 @@ export class UserManager { this._sessionMonitor = new SessionMonitor(this); } - this._dpopService = null; - if (this.settings.dpopSettings.enabled) { - this.settings.userStore = new DPoPStorageStateStore(); - this._dpopService = new DPoPService(this.settings.userStore as DPoPStorageStateStore); + if (this.settings.dpop) { + this._dpopStore = new DPoPStore(); } } @@ -162,6 +159,9 @@ export class UserManager { public async removeUser(): Promise { const logger = this._logger.create("removeUser"); await this.storeUser(null); + if (this.settings.dpop) { + await this._dpopStore?.remove(this.settings.client_id); + } logger.info("user removed from storage"); await this._events.unload(); } @@ -199,8 +199,7 @@ export class UserManager { const user = await this._signinEnd(url); if (user.profile && user.profile.sub) { logger.info("success, signed in subject", user.profile.sub); - } - else { + } else { logger.info("no subject"); } @@ -220,7 +219,12 @@ export class UserManager { }: SigninResourceOwnerCredentialsArgs): Promise { const logger = this._logger.create("signinResourceOwnerCredential"); - const signinResponse = await this._client.processResourceOwnerPasswordCredentials({ username, password, skipUserInfo, extraTokenParams: this.settings.extraTokenParams }); + const signinResponse = await this._client.processResourceOwnerPasswordCredentials({ + username, + password, + skipUserInfo, + extraTokenParams: this.settings.extraTokenParams, + }); logger.debug("got signin response"); const user = await this._buildUser(signinResponse); @@ -251,10 +255,6 @@ export class UserManager { logger.throw(new Error("No popup_redirect_uri configured")); } - if (this._dpopService && this.settings.dpopSettings.bind_authorization_code) { - dpopJkt = await this._dpopService.generateDPoPJkt(); - } - const handle = await this._popupNavigator.prepare({ popupWindowFeatures, popupWindowTarget }); const user = await this._signin({ request_type: "si:p", @@ -266,14 +266,14 @@ export class UserManager { if (user) { if (user.profile && user.profile.sub) { logger.info("success, signed in subject", user.profile.sub); - } - else { + } else { logger.info("no subject"); } } return user; } + /** * Notify the opening window of response (callback) from the authorization endpoint. * It is recommended to use {@link UserManager.signinCallback} instead. @@ -305,10 +305,6 @@ export class UserManager { logger.debug("using refresh token"); const state = new RefreshState(user as Required); const extraHeaders: Record = {}; - if (this._dpopService) { - const url = await this.metadataService.getTokenEndpoint(false); - extraHeaders["DPoP"] = await this._dpopService.generateDPoPProof({ url, httpMethod: "POST" }); - } return await this._useRefreshToken({ state, redirect_uri: requestArgs.redirect_uri, @@ -332,9 +328,6 @@ export class UserManager { } const handle = await this._iframeNavigator.prepare({ silentRequestTimeoutInSeconds }); - if (this._dpopService && this.settings.dpopSettings.bind_authorization_code) { - dpopJkt = await this._dpopService.generateDPoPJkt(); - } user = await this._signin({ request_type: "si:s", redirect_uri: url, @@ -346,8 +339,7 @@ export class UserManager { if (user) { if (user.profile?.sub) { logger.info("success, signed in subject", user.profile.sub); - } - else { + } else { logger.info("no subject"); } } @@ -435,20 +427,6 @@ export class UserManager { } } - /** - * Dynamically generates a DPoP proof for a given user, URL and optional Http method. - * This method is useful when you need to make a request to a resource server - * with fetch or similar, and you need to include a DPoP proof in a DPoP header. - * @param url - The URL to generate the DPoP proof for - * @param user - The user object to generate the DPoP proof for - * @param httpMethod - Optional, defaults to "GET" - * - * @returns A promise containing the DPoP proof - */ - public async dpopProof(url: string, user: User, httpMethod?: string): Promise { - return await this._dpopService?.generateDPoPProof({ url, accessToken: user.access_token, httpMethod: httpMethod }); - } - /** * Query OP for user's current signin status. * @@ -479,10 +457,6 @@ export class UserManager { }, handle); try { const extraHeaders: Record = {}; - if (this._dpopService) { - const tokenUrl = await this.metadataService.getTokenEndpoint(false); - extraHeaders["DPoP"] = await this._dpopService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); - } const signinResponse = await this._client.processSigninResponse(navResponse.url, extraHeaders); logger.debug("got signin response"); @@ -496,8 +470,7 @@ export class UserManager { logger.info("success, user not authenticated"); return null; - } - catch (err) { + } catch (err) { if (this.settings.monitorAnonymousSession && err instanceof ErrorResponse) { switch (err.error) { case "login_required": @@ -519,6 +492,7 @@ export class UserManager { const navResponse = await this._signinStart(args, handle); return await this._signinEnd(navResponse.url, verifySub); } + protected async _signinStart(args: CreateSigninRequestArgs, handle: IWindow): Promise { const logger = this._logger.create("_signinStart"); @@ -532,20 +506,16 @@ export class UserManager { response_mode: signinRequest.state.response_mode, scriptOrigin: this.settings.iframeScriptOrigin, }); - } - catch (err) { + } catch (err) { logger.debug("error after preparing navigator, closing navigator window"); handle.close(); throw err; } } + protected async _signinEnd(url: string, verifySub?: string): Promise { const logger = this._logger.create("_signinEnd"); const extraHeaders: Record = {}; - if (this._dpopService) { - const tokenUrl = await this.metadataService.getTokenEndpoint(false); - extraHeaders["DPoP"] = await this._dpopService.generateDPoPProof({ url: tokenUrl, httpMethod: "POST" }); - } const signinResponse = await this._client.processSigninResponse(url, extraHeaders); logger.debug("got signin response"); @@ -653,6 +623,7 @@ export class UserManager { const navResponse = await this._signoutStart(args, handle); return await this._signoutEnd(navResponse.url); } + protected async _signoutStart(args: CreateSignoutRequestArgs = {}, handle: IWindow): Promise { const logger = this._logger.create("_signoutStart"); @@ -681,13 +652,13 @@ export class UserManager { state: signoutRequest.state?.id, scriptOrigin: this.settings.iframeScriptOrigin, }); - } - catch (err) { + } catch (err) { logger.debug("error after preparing navigator, closing navigator window"); handle.close(); throw err; } } + protected async _signoutEnd(url: string): Promise { const logger = this._logger.create("_signoutEnd"); const signoutResponse = await this._client.processSignoutResponse(url); @@ -808,8 +779,7 @@ export class UserManager { logger.debug("storing user"); const storageString = user.toStorageString(); await this.settings.userStore.set(this._userStoreKey, storageString); - } - else { + } else { this._logger.debug("removing user"); await this.settings.userStore.remove(this._userStoreKey); } @@ -821,4 +791,27 @@ export class UserManager { public async clearStaleState(): Promise { await this._client.clearStaleState(); } + + /** + * Dynamically generates a DPoP proof for a given user, URL and optional Http method. + * This method is useful when you need to make a request to a resource server + * with fetch or similar, and you need to include a DPoP proof in a DPoP header. + * @param url - The URL to generate the DPoP proof for + * @param user - The user to generate the DPoP proof for + * @param httpMethod - Optional, defaults to "GET" + * + * @returns A promise containing the DPoP proof or undefined if DPoP is not enabled/no user is found. + */ + public async dpopProof(url: string, user: User, httpMethod?: string): Promise { + const dpopKey = await this._dpopStore?.get(this.settings.client_id); + if (dpopKey) { + return await CryptoUtils.generateDPoPProof({ + url, + accessToken: user?.access_token, + httpMethod: httpMethod, + keyPair: dpopKey, + }); + } + return undefined; + } } diff --git a/src/UserManagerSettings.test.ts b/src/UserManagerSettings.test.ts index 0c610876e..e1e3cb397 100644 --- a/src/UserManagerSettings.test.ts +++ b/src/UserManagerSettings.test.ts @@ -376,20 +376,4 @@ describe("UserManagerSettings", () => { expect(subject.stopCheckSessionOnError).toEqual(true); }); }); - - describe("dpop", () => { - it("should return value from initial settings", () => { - // act - const subject = new UserManagerSettingsStore({ - authority: "authority", - client_id: "client", - redirect_uri: "redirect", - stopCheckSessionOnError : false, - dpopSettings: { enabled: true }, - }); - - // assert - expect(subject.dpopSettings.enabled).toEqual(true); - }); - }); }); diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index f65b34fa7..6564ae88f 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -118,7 +118,6 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { public readonly monitorSession: boolean; public readonly monitorAnonymousSession: boolean; - public readonly dpopSettings: DPoPSettings; public readonly checkSessionIntervalInSeconds: number; public readonly query_status_response_type: string; public readonly stopCheckSessionOnError: boolean; @@ -151,7 +150,6 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { monitorSession = false, monitorAnonymousSession = false, - dpopSettings = { enabled: false, bind_authorization_code: false }, checkSessionIntervalInSeconds = DefaultCheckSessionIntervalInSeconds, query_status_response_type = "code", stopCheckSessionOnError = true, @@ -185,7 +183,6 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { this.monitorSession = monitorSession; this.monitorAnonymousSession = monitorAnonymousSession; - this.dpopSettings = dpopSettings; this.checkSessionIntervalInSeconds = checkSessionIntervalInSeconds; this.stopCheckSessionOnError = stopCheckSessionOnError; this.query_status_response_type = query_status_response_type; diff --git a/src/utils/CryptoUtils.ts b/src/utils/CryptoUtils.ts index bbb80da2a..39af3f54e 100644 --- a/src/utils/CryptoUtils.ts +++ b/src/utils/CryptoUtils.ts @@ -1,4 +1,12 @@ import { Logger } from "./Logger"; +import { JwtUtils } from "./JwtUtils"; + +export interface GenerateDPoPProofOpts { + url: string; + accessToken?: string; + httpMethod?: string; + keyPair: CryptoKeyPair; +} const UUID_V4_TEMPLATE = "10000000-1000-4000-8000-100000000000"; @@ -122,4 +130,72 @@ export class CryptoUtils { const utf8encodedAndHashed = await CryptoUtils.hash("SHA-256", JSON.stringify(jsonObject)); return CryptoUtils.encodeBase64Url(utf8encodedAndHashed); } + + public static async generateDPoPProof({ + url, + accessToken, + httpMethod, + keyPair, + }: GenerateDPoPProofOpts): Promise { + let hashedToken: Uint8Array; + let encodedHash: string; + + const payload: Record = { + "jti": window.crypto.randomUUID(), + "htm": httpMethod ?? "GET", + "htu": url, + "iat": Math.floor(Date.now() / 1000), + }; + + if (accessToken) { + hashedToken = await CryptoUtils.hash("SHA-256", accessToken); + encodedHash = CryptoUtils.encodeBase64Url(hashedToken); + payload.ath = encodedHash; + } + + try { + const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); + const header = { + "alg": "ES256", + "typ": "dpop+jwt", + "jwk": { + "crv": publicJwk.crv, + "kty": publicJwk.kty, + "x": publicJwk.x, + "y": publicJwk.y, + }, + }; + return await JwtUtils.generateSignedJwt(header, payload, keyPair.privateKey); + } catch (err) { + if (err instanceof TypeError) { + throw new Error(`Error exporting dpop public key: ${err.message}`); + } else { + throw err; + } + } + } + + public static async generateDPoPJkt(keyPair: CryptoKeyPair) : Promise { + try { + const publicJwk = await crypto.subtle.exportKey("jwk", keyPair.publicKey); + return await CryptoUtils.customCalculateJwkThumbprint(publicJwk); + } catch (err) { + if (err instanceof TypeError) { + throw new Error(`Could not retrieve dpop keys from storage: ${err.message}`); + } else { + throw err; + } + } + } + + public static async generateDPoPKeys() : Promise { + return await window.crypto.subtle.generateKey( + { + name: "ECDSA", + namedCurve: "P-256", + }, + false, + ["sign", "verify"], + ); + } } From 52124896694f615482ecf7beedd11273d060f0e4 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 14:51:54 +1200 Subject: [PATCH 18/33] Add tests to cover reorganisation of code --- src/OidcClient.test.ts | 79 +++++++++++++++++++++++++++++++++++ src/UserManager.test.ts | 72 +++++++++++++++++++++++++++++-- src/utils/CryptoUtils.test.ts | 49 ++++++++++++++++++++++ 3 files changed, 197 insertions(+), 3 deletions(-) diff --git a/src/OidcClient.test.ts b/src/OidcClient.test.ts index 8999e7d44..cc026ad30 100644 --- a/src/OidcClient.test.ts +++ b/src/OidcClient.test.ts @@ -362,6 +362,38 @@ describe("OidcClient", () => { // assert expect(validateSigninResponseMock).toHaveBeenCalledWith(response, item, extraHeaders); }); + + it("should pass DPoP extraHeader if enabled", async () => { + subject = new OidcClient({ + authority: "authority", + client_id: "client", + redirect_uri: "redirect", + post_logout_redirect_uri: "http://app", + dpop: true, + }); + + // arrange + const item = await SigninState.create({ + id: "1", + authority: "authority", + client_id: "client", + redirect_uri: "http://app/cb", + scope: "scope", + request_type: "type", + }); + jest.spyOn(subject.settings.stateStore, "remove") + .mockImplementation(async () => item.toStorageString()); + const validateSigninResponseMock = jest.spyOn(subject["_validator"], "validateSigninResponse") + .mockResolvedValue(); + const metadataServiceMock = jest.spyOn(subject["metadataService"], "getTokenEndpoint").mockResolvedValue("http://sts/token"); + + // act + const response = await subject.processSigninResponse("http://app/cb?state=1"); + + // assert + expect(metadataServiceMock).toHaveBeenCalled(); + expect(validateSigninResponseMock).toHaveBeenCalledWith(response, item, { "DPoP": expect.any(String) }); + }); }); describe("processResourceOwnerPasswordCredentials", () => { @@ -540,6 +572,53 @@ describe("OidcClient", () => { .rejects.toThrow("sub in id_token does not match current sub"); }); + it("should pass DPoP extraHeader to tokenClient.exchangeRefreshToken if DPoP enabled", async () => { + // arrange + const settings: OidcClientSettings = { + authority: "authority", + client_id: "client", + redirect_uri: "redirect", + post_logout_redirect_uri: "http://app", + dpop: true, + }; + + subject = new OidcClient(settings); + + const tokenResponse = { + access_token: "new_access_token", + }; + const exchangeRefreshTokenMock = + jest.spyOn(subject["_tokenClient"], "exchangeRefreshToken") + .mockResolvedValue(tokenResponse); + jest.spyOn(JwtUtils, "decode").mockReturnValue({ sub: "sub" }); + const mockMetaDataService = jest.spyOn(subject["metadataService"], "getTokenEndpoint").mockResolvedValue("http://sts/token"); + + const state = new RefreshState({ + refresh_token: "refresh_token", + id_token: "id_token", + session_state: "session_state", + scope: "openid", + profile: {} as UserProfile, + }); + + // act + const response = await subject.useRefreshToken({ state, resource: "resource" }); + + // assert + expect(mockMetaDataService).toHaveBeenCalled(); + expect(exchangeRefreshTokenMock).toHaveBeenCalledWith( { + refresh_token: "refresh_token", + scope: "openid", + timeoutInSeconds: undefined, + resource: "resource", + extraHeaders: { "DPoP": expect.any(String) }, + }); + expect(response).toBeInstanceOf(SigninResponse); + expect(response).toMatchObject(tokenResponse); + expect(response).toHaveProperty("session_state", state.session_state); + expect(response).toHaveProperty("scope", state.scope); + }); + it("should pass extraHeaders to tokenClient.exchangeRefreshToken if supplied", async () => { // arrange const tokenResponse = { diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 357e40255..4870a4ecd 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -20,6 +20,7 @@ import type { SigninState } from "./SigninState"; import type { State } from "./State"; import { mocked } from "jest-mock"; +import { CryptoUtils } from "./utils"; describe("UserManager", () => { let userStoreMock: WebStorageStateStore; @@ -43,9 +44,6 @@ describe("UserManager", () => { token_endpoint: "http://sts/oidc/token", revocation_endpoint: "http://sts/oidc/revoke", }, - dpopSettings: { - enabled: true, - }, }); }); @@ -184,6 +182,39 @@ describe("UserManager", () => { expect(storeUserMock).toBeCalledWith(null); expect(unloadMock).toBeCalled(); }); + + it("should remove dpop key from store if DPoP enabled", async () => { + // arrange + subject = new UserManager({ + authority: "http://sts/oidc", + client_id: "client", + redirect_uri: "http://app/cb", + monitorSession : false, + userStore: userStoreMock, + metadata: { + authorization_endpoint: "http://sts/oidc/authorize", + end_session_endpoint: "http://sts/oidc/logout", + token_endpoint: "http://sts/oidc/token", + revocation_endpoint: "http://sts/oidc/revoke", + }, + dpop: true, + }); + + const storeUserMock = jest.spyOn(subject, "storeUser"); + const unloadMock = jest.spyOn(subject["_events"], "unload"); + let dpopStoreMock; + if (subject["_dpopStore"]) { + dpopStoreMock = jest.spyOn(subject["_dpopStore"], "remove"); + } + + // act + await subject.removeUser(); + + // assert + expect(storeUserMock).toHaveBeenCalledWith(null); + expect(unloadMock).toHaveBeenCalled(); + expect(dpopStoreMock).toHaveBeenCalled(); + }); }); describe("revokeTokens", () => { @@ -1016,4 +1047,39 @@ describe("UserManager", () => { expect(storageString).toBeNull(); }); }); + + describe("getDPoPProof",() => { + it("should return a DPoP proof", async () => { + // arrange + subject = new UserManager({ + authority: "http://sts/oidc", + client_id: "client", + redirect_uri: "http://app/cb", + monitorSession : false, + userStore: userStoreMock, + metadata: { + authorization_endpoint: "http://sts/oidc/authorize", + end_session_endpoint: "http://sts/oidc/logout", + token_endpoint: "http://sts/oidc/token", + revocation_endpoint: "http://sts/oidc/revoke", + }, + dpop: true, + }); + + const user = await subject.getUser() as User; + + let mockDpopStore; + const keyPair = await CryptoUtils.generateDPoPKeys(); + if (subject["_dpopStore"]) { + mockDpopStore = jest.spyOn(subject["_dpopStore"], "get").mockResolvedValue(keyPair); + } + // act + const dpopProof = await subject.dpopProof("http://some.url", user, "POST"); + + // assert + expect(mockDpopStore).toHaveBeenCalledWith("client"); + expect(dpopProof).toBeDefined(); + expect(typeof dpopProof).toBe("string"); + }); + }); }); diff --git a/src/utils/CryptoUtils.test.ts b/src/utils/CryptoUtils.test.ts index 65a54e805..ba0e9b8ed 100644 --- a/src/utils/CryptoUtils.test.ts +++ b/src/utils/CryptoUtils.test.ts @@ -1,4 +1,5 @@ import { CryptoUtils } from "./CryptoUtils"; +import { jwtVerify, decodeProtectedHeader, importJWK, type JWK } from "jose"; const pattern = /^[0-9a-f]{8}[0-9a-f]{4}4[0-9a-f]{3}[89ab][0-9a-f]{3}[0-9a-f]{12}$/; @@ -61,4 +62,52 @@ describe("CryptoUtils", () => { await expect(CryptoUtils.customCalculateJwkThumbprint(jwk)).rejects.toThrow("Unknown jwk type"); }); }); + + describe("generateDPoPProof", () => { + it("should generate a valid proof without an access token", async () => { + // arrange + const url = "https://localhost:5005/identity"; + const httpMethod = "GET"; + const keyPair = await CryptoUtils.generateDPoPKeys(); + + // act + const dpopProof = await CryptoUtils.generateDPoPProof({ url, httpMethod, keyPair }); + const protectedHeader = decodeProtectedHeader(dpopProof); + const publicKey = await importJWK(protectedHeader.jwk); + const verifiedResult = await jwtVerify(dpopProof, publicKey); + + // assert + expect(verifiedResult.payload).toHaveProperty("htu"); + expect(verifiedResult.payload).toHaveProperty("htm"); + }); + + it("should generate a valid proof with an access token", async () => { + // arrange + const url = "https://localhost:5005/identity"; + const httpMethod = "GET"; + const accessToken = "access_token"; + const keyPair = await CryptoUtils.generateDPoPKeys(); + + // act + const dpopProof = await CryptoUtils.generateDPoPProof({ url, accessToken, httpMethod, keyPair }); + const protectedHeader = decodeProtectedHeader(dpopProof); + const publicKey = await importJWK(protectedHeader.jwk); + const verifiedResult = await jwtVerify(dpopProof, publicKey); + + // assert + expect(verifiedResult.payload).toHaveProperty("htu"); + expect(verifiedResult.payload).toHaveProperty("htm"); + expect(verifiedResult.payload).toHaveProperty("ath"); + }); + + it("should throw an exception if there is an error generating the signed JWT", async () => { + const keyPair = await CryptoUtils.generateDPoPKeys(); + const exportKeyMock = jest.spyOn(crypto.subtle, "exportKey").mockResolvedValue({} as JsonWebKey); + const generateSignedJwtMock = jest.spyOn(crypto.subtle, "sign").mockRejectedValue(new Error("Generate signed JWT error")); + await expect(CryptoUtils.generateDPoPProof({ + url: "http://example.com", keyPair: keyPair })).rejects.toThrow("Generate signed JWT error"); + exportKeyMock.mockRestore(); + generateSignedJwtMock.mockRestore(); + }); + }); }); From 77a0827db72e68935f77f920479015a0f10a0867 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 15:16:24 +1200 Subject: [PATCH 19/33] Make DPoPStore implementation optional --- docs/oidc-client-ts.api.md | 15 +++++---- src/DPoPStore.test.ts | 4 +-- src/DPoPStore.ts | 68 +++++--------------------------------- src/IndexDbDPoPStore.ts | 63 +++++++++++++++++++++++++++++++++++ src/OidcClient.ts | 21 +++++------- src/OidcClientSettings.ts | 10 ++++++ src/UserManager.test.ts | 8 ++--- src/UserManager.ts | 12 ++----- 8 files changed, 108 insertions(+), 93 deletions(-) create mode 100644 src/IndexDbDPoPStore.ts diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 3cb181d44..b039587e3 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -306,12 +306,14 @@ export class OidcClient { createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; - // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts + // Warning: (ae-forgotten-export) The symbol "IndexDbDPoPStore" needs to be exported by the entry point index.d.ts // // (undocumented) - protected readonly _dpopStore: DPoPStore | undefined; + protected readonly _dpopStore: IndexDbDPoPStore | undefined; + // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts + // // (undocumented) - getDpopProof(dpopStore: DPoPStore): Promise; + getDpopProof(dpopStore: DPoPStore): Promise; // (undocumented) protected readonly _logger: Logger; // (undocumented) @@ -359,6 +361,7 @@ export interface OidcClientSettings { disablePKCE?: boolean; display?: string; dpop?: boolean; + dpopStore?: DPoPStore; extraHeaders?: Record; extraQueryParams?: Record; // (undocumented) @@ -391,7 +394,7 @@ export interface OidcClientSettings { // @public export class OidcClientSettingsStore { - constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, dpop, }: OidcClientSettings); + constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, dpop, dpopStore, }: OidcClientSettings); // (undocumented) readonly acr_values: string | undefined; // (undocumented) @@ -409,6 +412,8 @@ export class OidcClientSettingsStore { // (undocumented) readonly dpop: boolean | undefined; // (undocumented) + readonly dpopStore: DPoPStore | undefined; + // (undocumented) readonly extraHeaders: Record; // (undocumented) readonly extraQueryParams: Record; @@ -963,8 +968,6 @@ export class UserManager { // (undocumented) protected readonly _client: OidcClient; dpopProof(url: string, user: User, httpMethod?: string): Promise; - // (undocumented) - protected readonly _dpopStore: DPoPStore | undefined; get events(): UserManagerEvents; // (undocumented) protected readonly _events: UserManagerEvents; diff --git a/src/DPoPStore.test.ts b/src/DPoPStore.test.ts index 6adda03bf..167244c00 100644 --- a/src/DPoPStore.test.ts +++ b/src/DPoPStore.test.ts @@ -1,7 +1,7 @@ -import { DPoPStore } from "./DPoPStore"; +import { IndexDbDPoPStore } from "./IndexDbDPoPStore"; describe("DPoPStore", () => { - const subject = new DPoPStore(); + const subject = new IndexDbDPoPStore(); let data: CryptoKeyPair; diff --git a/src/DPoPStore.ts b/src/DPoPStore.ts index 1ae5ea0bd..b9fd3247a 100644 --- a/src/DPoPStore.ts +++ b/src/DPoPStore.ts @@ -1,61 +1,9 @@ -export class DPoPStore { - readonly _dbName: string = "oidc"; - readonly _storeName: string = "dpop"; - - public async set(key: string, value: CryptoKeyPair): Promise { - const store = await this.createStore(this._dbName, this._storeName); - await store("readwrite", (str: IDBObjectStore) => { - str.put(value, key); - return this.promisifyRequest(str.transaction); - }); - } - - public async get(key: string): Promise { - const store = await this.createStore(this._dbName, this._storeName); - return await store("readonly", (str) => { - return this.promisifyRequest(str.get(key)); - }) as CryptoKeyPair; - } - - public async remove(key: string): Promise { - const item = await this.get(key); - const store = await this.createStore(this._dbName, this._storeName); - await store("readwrite", (str) => { - return this.promisifyRequest(str.delete(key)); - }); - return item; - } - - public async getAllKeys(): Promise { - const store = await this.createStore(this._dbName, this._storeName); - return await store("readonly", (str) => { - return this.promisifyRequest(str.getAllKeys()); - }) as string[]; - } - - promisifyRequest( - request: IDBRequest | IDBTransaction): Promise { - return new Promise((resolve, reject) => { - (request as IDBTransaction).oncomplete = (request as IDBRequest).onsuccess = () => resolve((request as IDBRequest).result); - (request as IDBTransaction).onabort = (request as IDBRequest).onerror = () => reject((request as IDBRequest).error); - }); - } - - async createStore( - dbName: string, - storeName: string, - ): Promise<(txMode: IDBTransactionMode, callback: (store: IDBObjectStore) => T | PromiseLike) => Promise> { - const request = indexedDB.open(dbName); - request.onupgradeneeded = () => request.result.createObjectStore(storeName); - const db = await this.promisifyRequest(request); - - return async ( - txMode: IDBTransactionMode, - callback: (store: IDBObjectStore) => T | PromiseLike, - ) => { - const tx = db.transaction(storeName, txMode); - const store = tx.objectStore(storeName); - return await callback(store); - }; - } +/** + * @public + */ +export interface DPoPStore { + set(key: string, value: T): Promise; + get(key: string): Promise; + remove(key: string): Promise; + getAllKeys(): Promise; } diff --git a/src/IndexDbDPoPStore.ts b/src/IndexDbDPoPStore.ts new file mode 100644 index 000000000..07e796464 --- /dev/null +++ b/src/IndexDbDPoPStore.ts @@ -0,0 +1,63 @@ +import type { DPoPStore } from "./DPoPStore"; + +export class IndexDbDPoPStore implements DPoPStore { + readonly _dbName: string = "oidc"; + readonly _storeName: string = "dpop"; + + public async set(key: string, value: CryptoKeyPair): Promise { + const store = await this.createStore(this._dbName, this._storeName); + await store("readwrite", (str: IDBObjectStore) => { + str.put(value, key); + return this.promisifyRequest(str.transaction); + }); + } + + public async get(key: string): Promise { + const store = await this.createStore(this._dbName, this._storeName); + return await store("readonly", (str) => { + return this.promisifyRequest(str.get(key)); + }) as CryptoKeyPair; + } + + public async remove(key: string): Promise { + const item = await this.get(key); + const store = await this.createStore(this._dbName, this._storeName); + await store("readwrite", (str) => { + return this.promisifyRequest(str.delete(key)); + }); + return item; + } + + public async getAllKeys(): Promise { + const store = await this.createStore(this._dbName, this._storeName); + return await store("readonly", (str) => { + return this.promisifyRequest(str.getAllKeys()); + }) as string[]; + } + + promisifyRequest( + request: IDBRequest | IDBTransaction): Promise { + return new Promise((resolve, reject) => { + (request as IDBTransaction).oncomplete = (request as IDBRequest).onsuccess = () => resolve((request as IDBRequest).result); + (request as IDBTransaction).onabort = (request as IDBRequest).onerror = () => reject((request as IDBRequest).error); + }); + } + + async createStore( + dbName: string, + storeName: string, + ): Promise<(txMode: IDBTransactionMode, callback: (store: IDBObjectStore) => T | PromiseLike) => Promise> { + const request = indexedDB.open(dbName); + request.onupgradeneeded = () => request.result.createObjectStore(storeName); + const db = await this.promisifyRequest(request); + + return async ( + txMode: IDBTransactionMode, + callback: (store: IDBObjectStore) => T | PromiseLike, + ) => { + const tx = db.transaction(storeName, txMode); + const store = tx.objectStore(storeName); + return await callback(store); + }; + } +} diff --git a/src/OidcClient.ts b/src/OidcClient.ts index c2ef59214..360346c45 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -15,7 +15,8 @@ import { SigninState } from "./SigninState"; import { State } from "./State"; import { TokenClient } from "./TokenClient"; import { ClaimsService } from "./ClaimsService"; -import { DPoPStore } from "./DPoPStore"; +import { IndexDbDPoPStore } from "./IndexDbDPoPStore"; +import type { DPoPStore } from "./DPoPStore"; /** * @public @@ -79,7 +80,7 @@ export class OidcClient { protected readonly _claimsService: ClaimsService; protected readonly _validator: ResponseValidator; protected readonly _tokenClient: TokenClient; - protected readonly _dpopStore: DPoPStore | undefined; + protected readonly _dpopStore: IndexDbDPoPStore | undefined; public constructor(settings: OidcClientSettings); public constructor(settings: OidcClientSettingsStore, metadataService: MetadataService); @@ -90,10 +91,6 @@ export class OidcClient { this._claimsService = new ClaimsService(this.settings); this._validator = new ResponseValidator(this.settings, this.metadataService, this._claimsService); this._tokenClient = new TokenClient(this.settings, this.metadataService); - - if (this.settings.dpop) { - this._dpopStore = new DPoPStore(); - } } public async createSigninRequest({ @@ -179,22 +176,22 @@ export class OidcClient { const { state, response } = await this.readSigninResponseState(url, true); logger.debug("received state from storage; validating response"); - if (this.settings.dpop && this._dpopStore) { - const dpopProof = await this.getDpopProof(this._dpopStore); + if (this.settings.dpop && this.settings.dpopStore) { + const dpopProof = await this.getDpopProof(this.settings.dpopStore); extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; } await this._validator.validateSigninResponse(response, state, extraHeaders); return response; } - async getDpopProof(dpopStore: DPoPStore): Promise { + async getDpopProof(dpopStore: DPoPStore): Promise { let keyPair: CryptoKeyPair; if (!(await dpopStore.getAllKeys()).includes(this.settings.client_id)) { keyPair = await CryptoUtils.generateDPoPKeys(); await dpopStore.set(this.settings.client_id, keyPair); } else { - keyPair = await dpopStore.get(this.settings.client_id) as CryptoKeyPair; + keyPair = await dpopStore.get(this.settings.client_id); } return await CryptoUtils.generateDPoPProof({ @@ -240,8 +237,8 @@ export class OidcClient { scope = providedScopes.filter(s => allowableScopes.includes(s)).join(" "); } - if (this.settings.dpop && this._dpopStore) { - const dpopProof = await this.getDpopProof(this._dpopStore); + if (this.settings.dpop && this.settings.dpopStore) { + const dpopProof = await this.getDpopProof(this.settings.dpopStore); extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; } diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 69168df67..778d4fa85 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -5,6 +5,8 @@ import { WebStorageStateStore } from "./WebStorageStateStore"; import type { OidcMetadata } from "./OidcMetadata"; import type { StateStore } from "./StateStore"; import { InMemoryWebStorage } from "./InMemoryWebStorage"; +import type { DPoPStore } from "./DPoPStore"; +import { IndexDbDPoPStore } from "./IndexDbDPoPStore"; const DefaultResponseType = "code"; const DefaultScope = "openid"; @@ -124,6 +126,11 @@ export interface OidcClientSettings { */ dpop?: boolean; + /** + * DPoP store to use + */ + dpopStore?: DPoPStore; + /** * Will check the content type header of the response of the revocation endpoint to match these passed values (default: []) */ @@ -188,6 +195,7 @@ export class OidcClientSettingsStore { public readonly extraQueryParams: Record; public readonly extraTokenParams: Record; public readonly dpop: boolean | undefined; + public readonly dpopStore: DPoPStore | undefined; public readonly extraHeaders: Record; public readonly revokeTokenAdditionalContentTypes?: string[]; @@ -220,6 +228,7 @@ export class OidcClientSettingsStore { extraTokenParams = {}, extraHeaders = {}, dpop = false, + dpopStore, }: OidcClientSettings) { this.authority = authority; @@ -279,5 +288,6 @@ export class OidcClientSettingsStore { this.extraTokenParams = extraTokenParams; this.extraHeaders = extraHeaders; this.dpop = dpop; + this.dpopStore = dpop && dpopStore ? dpopStore : new IndexDbDPoPStore(); } } diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 4870a4ecd..ff4d060af 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -203,8 +203,8 @@ describe("UserManager", () => { const storeUserMock = jest.spyOn(subject, "storeUser"); const unloadMock = jest.spyOn(subject["_events"], "unload"); let dpopStoreMock; - if (subject["_dpopStore"]) { - dpopStoreMock = jest.spyOn(subject["_dpopStore"], "remove"); + if (subject.settings.dpopStore) { + dpopStoreMock = jest.spyOn(subject.settings.dpopStore, "remove"); } // act @@ -1070,8 +1070,8 @@ describe("UserManager", () => { let mockDpopStore; const keyPair = await CryptoUtils.generateDPoPKeys(); - if (subject["_dpopStore"]) { - mockDpopStore = jest.spyOn(subject["_dpopStore"], "get").mockResolvedValue(keyPair); + if (subject.settings.dpopStore) { + mockDpopStore = jest.spyOn(subject.settings.dpopStore, "get").mockResolvedValue(keyPair); } // act const dpopProof = await subject.dpopProof("http://some.url", user, "POST"); diff --git a/src/UserManager.ts b/src/UserManager.ts index bd115f52f..18b4e6f0e 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -16,7 +16,6 @@ import type { MetadataService } from "./MetadataService"; import { RefreshState } from "./RefreshState"; import type { SigninResponse } from "./SigninResponse"; import type { ExtraHeader } from "./OidcClientSettings"; -import { DPoPStore } from "./DPoPStore"; /** * @public @@ -90,7 +89,6 @@ export class UserManager { protected readonly _events: UserManagerEvents; protected readonly _silentRenewService: SilentRenewService; protected readonly _sessionMonitor: SessionMonitor | null; - protected readonly _dpopStore: DPoPStore | undefined; public constructor(settings: UserManagerSettings, redirectNavigator?: INavigator, popupNavigator?: INavigator, iframeNavigator?: INavigator) { this.settings = new UserManagerSettingsStore(settings); @@ -113,10 +111,6 @@ export class UserManager { if (this.settings.monitorSession) { this._sessionMonitor = new SessionMonitor(this); } - - if (this.settings.dpop) { - this._dpopStore = new DPoPStore(); - } } /** @@ -159,8 +153,8 @@ export class UserManager { public async removeUser(): Promise { const logger = this._logger.create("removeUser"); await this.storeUser(null); - if (this.settings.dpop) { - await this._dpopStore?.remove(this.settings.client_id); + if (this.settings.dpop && this.settings.dpopStore) { + await this.settings.dpopStore.remove(this.settings.client_id); } logger.info("user removed from storage"); await this._events.unload(); @@ -803,7 +797,7 @@ export class UserManager { * @returns A promise containing the DPoP proof or undefined if DPoP is not enabled/no user is found. */ public async dpopProof(url: string, user: User, httpMethod?: string): Promise { - const dpopKey = await this._dpopStore?.get(this.settings.client_id); + const dpopKey = await this.settings.dpopStore?.get(this.settings.client_id); if (dpopKey) { return await CryptoUtils.generateDPoPProof({ url, From 4cc3d750478203e88da5bc4784835f0d17873889 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 15:29:54 +1200 Subject: [PATCH 20/33] Undo changes to StateStore --- docs/oidc-client-ts.api.md | 16 ++++++++-------- src/OidcClientSettings.test.ts | 4 ++-- src/OidcClientSettings.ts | 4 ++-- src/State.ts | 2 +- src/StateStore.ts | 8 ++++---- src/WebStorageStateStore.ts | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index b039587e3..678921616 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -388,7 +388,7 @@ export interface OidcClientSettings { scope?: string; signingKeys?: SigningKey[]; staleStateAgeInSeconds?: number; - stateStore?: StateStore; + stateStore?: StateStore; ui_locales?: string; } @@ -460,7 +460,7 @@ export class OidcClientSettingsStore { // (undocumented) readonly staleStateAgeInSeconds: number; // (undocumented) - readonly stateStore: StateStore; + readonly stateStore: StateStore; // (undocumented) readonly ui_locales: string | undefined; } @@ -877,7 +877,7 @@ export class State { url_state?: string; }); // (undocumented) - static clearStaleState(storage: StateStore, age: number): Promise; + static clearStaleState(storage: StateStore, age: number): Promise; // (undocumented) readonly created: number; readonly data?: unknown; @@ -894,15 +894,15 @@ export class State { } // @public (undocumented) -export interface StateStore { +export interface StateStore { // (undocumented) - get(key: string): Promise; + get(key: string): Promise; // (undocumented) getAllKeys(): Promise; // (undocumented) - remove(key: string): Promise; + remove(key: string): Promise; // (undocumented) - set(key: string, value: T): Promise; + set(key: string, value: string): Promise; } // @public (undocumented) @@ -1167,7 +1167,7 @@ export type UserUnloadedCallback = () => Promise | void; export const Version: string; // @public (undocumented) -export class WebStorageStateStore implements StateStore { +export class WebStorageStateStore implements StateStore { constructor({ prefix, store, }?: { prefix?: string; store?: AsyncStorage | Storage; diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts index efca4f0b2..9bd1adb2c 100644 --- a/src/OidcClientSettings.test.ts +++ b/src/OidcClientSettings.test.ts @@ -405,7 +405,7 @@ describe("OidcClientSettings", () => { it("should return value from initial settings", () => { // arrange - const temp = {} as StateStore; + const temp = {} as StateStore; // act const subject = new OidcClientSettingsStore({ @@ -424,7 +424,7 @@ describe("OidcClientSettings", () => { it("should return value from initial settings", () => { // arrange - const temp = {} as StateStore; + const temp = {} as StateStore; // act const subject = new OidcClientSettingsStore({ diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 778d4fa85..915605535 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -106,7 +106,7 @@ export interface OidcClientSettings { * Storage object used to persist interaction state (default: window.localStorage, InMemoryWebStorage iff no window). * E.g. `stateStore: new WebStorageStateStore({ store: window.localStorage })` */ - stateStore?: StateStore; + stateStore?: StateStore; /** * An object containing additional query string parameters to be including in the authorization request. @@ -189,7 +189,7 @@ export class OidcClientSettingsStore { public readonly staleStateAgeInSeconds: number; public readonly mergeClaimsStrategy: { array: "replace" | "merge" }; - public readonly stateStore: StateStore; + public readonly stateStore: StateStore; // extra public readonly extraQueryParams: Record; diff --git a/src/State.ts b/src/State.ts index 0c0f61116..b9294b1cd 100644 --- a/src/State.ts +++ b/src/State.ts @@ -52,7 +52,7 @@ export class State { return Promise.resolve(new State(JSON.parse(storageString))); } - public static async clearStaleState(storage: StateStore, age: number): Promise { + public static async clearStaleState(storage: StateStore, age: number): Promise { const logger = Logger.createStatic("State", "clearStaleState"); const cutoff = Timer.getEpochTime() - age; diff --git a/src/StateStore.ts b/src/StateStore.ts index 413361ae5..6271499a7 100644 --- a/src/StateStore.ts +++ b/src/StateStore.ts @@ -1,9 +1,9 @@ /** * @public */ -export interface StateStore { - set(key: string, value: T): Promise; - get(key: string): Promise; - remove(key: string): Promise; +export interface StateStore { + set(key: string, value: string): Promise; + get(key: string): Promise; + remove(key: string): Promise; getAllKeys(): Promise; } diff --git a/src/WebStorageStateStore.ts b/src/WebStorageStateStore.ts index 13ec9aa52..112408942 100644 --- a/src/WebStorageStateStore.ts +++ b/src/WebStorageStateStore.ts @@ -8,7 +8,7 @@ import type { AsyncStorage } from "./AsyncStorage"; /** * @public */ -export class WebStorageStateStore implements StateStore { +export class WebStorageStateStore implements StateStore { private readonly _logger = new Logger("WebStorageStateStore"); private readonly _store: AsyncStorage | Storage; From f145f32e5b8c7fb5455f56a0f1d481593d07016a Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 15:33:47 +1200 Subject: [PATCH 21/33] Simplify DPoPStore interface --- docs/oidc-client-ts.api.md | 6 +++--- src/DPoPStore.ts | 8 ++++---- src/IndexDbDPoPStore.ts | 2 +- src/OidcClient.ts | 2 +- src/OidcClientSettings.ts | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 678921616..a042dd016 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -313,7 +313,7 @@ export class OidcClient { // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts // // (undocumented) - getDpopProof(dpopStore: DPoPStore): Promise; + getDpopProof(dpopStore: DPoPStore): Promise; // (undocumented) protected readonly _logger: Logger; // (undocumented) @@ -361,7 +361,7 @@ export interface OidcClientSettings { disablePKCE?: boolean; display?: string; dpop?: boolean; - dpopStore?: DPoPStore; + dpopStore?: DPoPStore; extraHeaders?: Record; extraQueryParams?: Record; // (undocumented) @@ -412,7 +412,7 @@ export class OidcClientSettingsStore { // (undocumented) readonly dpop: boolean | undefined; // (undocumented) - readonly dpopStore: DPoPStore | undefined; + readonly dpopStore: DPoPStore | undefined; // (undocumented) readonly extraHeaders: Record; // (undocumented) diff --git a/src/DPoPStore.ts b/src/DPoPStore.ts index b9fd3247a..3dd06f6f4 100644 --- a/src/DPoPStore.ts +++ b/src/DPoPStore.ts @@ -1,9 +1,9 @@ /** * @public */ -export interface DPoPStore { - set(key: string, value: T): Promise; - get(key: string): Promise; - remove(key: string): Promise; +export interface DPoPStore { + set(key: string, value: CryptoKeyPair): Promise; + get(key: string): Promise; + remove(key: string): Promise; getAllKeys(): Promise; } diff --git a/src/IndexDbDPoPStore.ts b/src/IndexDbDPoPStore.ts index 07e796464..f8d734390 100644 --- a/src/IndexDbDPoPStore.ts +++ b/src/IndexDbDPoPStore.ts @@ -1,6 +1,6 @@ import type { DPoPStore } from "./DPoPStore"; -export class IndexDbDPoPStore implements DPoPStore { +export class IndexDbDPoPStore implements DPoPStore { readonly _dbName: string = "oidc"; readonly _storeName: string = "dpop"; diff --git a/src/OidcClient.ts b/src/OidcClient.ts index 360346c45..03cbda38d 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -184,7 +184,7 @@ export class OidcClient { return response; } - async getDpopProof(dpopStore: DPoPStore): Promise { + async getDpopProof(dpopStore: DPoPStore): Promise { let keyPair: CryptoKeyPair; if (!(await dpopStore.getAllKeys()).includes(this.settings.client_id)) { diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 915605535..dc1bc92c1 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -129,7 +129,7 @@ export interface OidcClientSettings { /** * DPoP store to use */ - dpopStore?: DPoPStore; + dpopStore?: DPoPStore; /** * Will check the content type header of the response of the revocation endpoint to match these passed values (default: []) @@ -195,7 +195,7 @@ export class OidcClientSettingsStore { public readonly extraQueryParams: Record; public readonly extraTokenParams: Record; public readonly dpop: boolean | undefined; - public readonly dpopStore: DPoPStore | undefined; + public readonly dpopStore: DPoPStore | undefined; public readonly extraHeaders: Record; public readonly revokeTokenAdditionalContentTypes?: string[]; From e2e07eddcb9a1b10f948abd9eeb8cf934800ec98 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 15:36:16 +1200 Subject: [PATCH 22/33] Rename IndexDbDPoPStore to IndexedDbDPoPStore --- docs/oidc-client-ts.api.md | 4 ++-- src/{DPoPStore.test.ts => IndexedDbDPoPStore.test.ts} | 4 ++-- src/{IndexDbDPoPStore.ts => IndexedDbDPoPStore.ts} | 2 +- src/OidcClient.ts | 4 ++-- src/OidcClientSettings.ts | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) rename src/{DPoPStore.test.ts => IndexedDbDPoPStore.test.ts} (96%) rename src/{IndexDbDPoPStore.ts => IndexedDbDPoPStore.ts} (97%) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index a042dd016..2c095b286 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -306,10 +306,10 @@ export class OidcClient { createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; - // Warning: (ae-forgotten-export) The symbol "IndexDbDPoPStore" needs to be exported by the entry point index.d.ts + // Warning: (ae-forgotten-export) The symbol "IndexedDbDPoPStore" needs to be exported by the entry point index.d.ts // // (undocumented) - protected readonly _dpopStore: IndexDbDPoPStore | undefined; + protected readonly _dpopStore: IndexedDbDPoPStore | undefined; // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts // // (undocumented) diff --git a/src/DPoPStore.test.ts b/src/IndexedDbDPoPStore.test.ts similarity index 96% rename from src/DPoPStore.test.ts rename to src/IndexedDbDPoPStore.test.ts index 167244c00..e9d23ff3a 100644 --- a/src/DPoPStore.test.ts +++ b/src/IndexedDbDPoPStore.test.ts @@ -1,7 +1,7 @@ -import { IndexDbDPoPStore } from "./IndexDbDPoPStore"; +import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; describe("DPoPStore", () => { - const subject = new IndexDbDPoPStore(); + const subject = new IndexedDbDPoPStore(); let data: CryptoKeyPair; diff --git a/src/IndexDbDPoPStore.ts b/src/IndexedDbDPoPStore.ts similarity index 97% rename from src/IndexDbDPoPStore.ts rename to src/IndexedDbDPoPStore.ts index f8d734390..830371f0b 100644 --- a/src/IndexDbDPoPStore.ts +++ b/src/IndexedDbDPoPStore.ts @@ -1,6 +1,6 @@ import type { DPoPStore } from "./DPoPStore"; -export class IndexDbDPoPStore implements DPoPStore { +export class IndexedDbDPoPStore implements DPoPStore { readonly _dbName: string = "oidc"; readonly _storeName: string = "dpop"; diff --git a/src/OidcClient.ts b/src/OidcClient.ts index 03cbda38d..42059b859 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -15,7 +15,7 @@ import { SigninState } from "./SigninState"; import { State } from "./State"; import { TokenClient } from "./TokenClient"; import { ClaimsService } from "./ClaimsService"; -import { IndexDbDPoPStore } from "./IndexDbDPoPStore"; +import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; import type { DPoPStore } from "./DPoPStore"; /** @@ -80,7 +80,7 @@ export class OidcClient { protected readonly _claimsService: ClaimsService; protected readonly _validator: ResponseValidator; protected readonly _tokenClient: TokenClient; - protected readonly _dpopStore: IndexDbDPoPStore | undefined; + protected readonly _dpopStore: IndexedDbDPoPStore | undefined; public constructor(settings: OidcClientSettings); public constructor(settings: OidcClientSettingsStore, metadataService: MetadataService); diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index dc1bc92c1..fb5998abf 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -6,7 +6,7 @@ import type { OidcMetadata } from "./OidcMetadata"; import type { StateStore } from "./StateStore"; import { InMemoryWebStorage } from "./InMemoryWebStorage"; import type { DPoPStore } from "./DPoPStore"; -import { IndexDbDPoPStore } from "./IndexDbDPoPStore"; +import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; const DefaultResponseType = "code"; const DefaultScope = "openid"; @@ -288,6 +288,6 @@ export class OidcClientSettingsStore { this.extraTokenParams = extraTokenParams; this.extraHeaders = extraHeaders; this.dpop = dpop; - this.dpopStore = dpop && dpopStore ? dpopStore : new IndexDbDPoPStore(); + this.dpopStore = dpop && dpopStore ? dpopStore : new IndexedDbDPoPStore(); } } From fcb17fd2a7bf3bce3639284454d3c2e27a191013 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 20:35:23 +1200 Subject: [PATCH 23/33] Re-implement dpop key auth code binding --- docs/oidc-client-ts.api.md | 16 +++-- src/OidcClient.test.ts | 11 +++- src/OidcClient.ts | 11 ++-- src/OidcClientSettings.ts | 27 ++++---- src/UserManager.test.ts | 122 +++++++++++++++++++++++++++++++++++-- src/UserManager.ts | 37 ++++++++++- 6 files changed, 188 insertions(+), 36 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 2c095b286..c368f822c 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -303,7 +303,7 @@ export class OidcClient { // (undocumented) clearStaleState(): Promise; // (undocumented) - createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, }: CreateSigninRequestArgs): Promise; + createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, dpopJkt, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; // Warning: (ae-forgotten-export) The symbol "IndexedDbDPoPStore" needs to be exported by the entry point index.d.ts @@ -360,8 +360,8 @@ export interface OidcClientSettings { client_secret?: string; disablePKCE?: boolean; display?: string; - dpop?: boolean; - dpopStore?: DPoPStore; + // Warning: (ae-forgotten-export) The symbol "DPoPSettings" needs to be exported by the entry point index.d.ts + dpop?: DPoPSettings | undefined; extraHeaders?: Record; extraQueryParams?: Record; // (undocumented) @@ -394,7 +394,7 @@ export interface OidcClientSettings { // @public export class OidcClientSettingsStore { - constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, dpop, dpopStore, }: OidcClientSettings); + constructor({ authority, metadataUrl, metadata, signingKeys, metadataSeed, client_id, client_secret, response_type, scope, redirect_uri, post_logout_redirect_uri, client_authentication, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, filterProtocolClaims, loadUserInfo, staleStateAgeInSeconds, mergeClaimsStrategy, disablePKCE, stateStore, revokeTokenAdditionalContentTypes, fetchRequestCredentials, refreshTokenAllowedScope, extraQueryParams, extraTokenParams, extraHeaders, dpop, }: OidcClientSettings); // (undocumented) readonly acr_values: string | undefined; // (undocumented) @@ -410,9 +410,7 @@ export class OidcClientSettingsStore { // (undocumented) readonly display: string | undefined; // (undocumented) - readonly dpop: boolean | undefined; - // (undocumented) - readonly dpopStore: DPoPStore | undefined; + readonly dpop: DPoPSettings | undefined; // (undocumented) readonly extraHeaders: Record; // (undocumented) @@ -1069,8 +1067,8 @@ export interface UserManagerSettings extends OidcClientSettings { accessTokenExpiringNotificationTimeInSeconds?: number; automaticSilentRenew?: boolean; checkSessionIntervalInSeconds?: number; - // Warning: (ae-forgotten-export) The symbol "DPoPSettings" needs to be exported by the entry point index.d.ts - dpopSettings?: DPoPSettings; + // Warning: (ae-forgotten-export) The symbol "DPoPSettings_2" needs to be exported by the entry point index.d.ts + dpopSettings?: DPoPSettings_2; iframeNotifyParentOrigin?: string; iframeScriptOrigin?: string; includeIdTokenInSilentRenew?: boolean; diff --git a/src/OidcClient.test.ts b/src/OidcClient.test.ts index cc026ad30..a2bd231ed 100644 --- a/src/OidcClient.test.ts +++ b/src/OidcClient.test.ts @@ -14,6 +14,7 @@ import { SignoutResponse } from "./SignoutResponse"; import { RefreshState } from "./RefreshState"; import { SigninResponse } from "./SigninResponse"; import type { UserProfile } from "./User"; +import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; describe("OidcClient", () => { let subject: OidcClient; @@ -369,7 +370,10 @@ describe("OidcClient", () => { client_id: "client", redirect_uri: "redirect", post_logout_redirect_uri: "http://app", - dpop: true, + dpop: { + bind_authorization_code: false, + store: new IndexedDbDPoPStore(), + }, }); // arrange @@ -579,7 +583,10 @@ describe("OidcClient", () => { client_id: "client", redirect_uri: "redirect", post_logout_redirect_uri: "http://app", - dpop: true, + dpop: { + bind_authorization_code: false, + store: new IndexedDbDPoPStore(), + }, }; subject = new OidcClient(settings); diff --git a/src/OidcClient.ts b/src/OidcClient.ts index 42059b859..d7d1a8749 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -115,6 +115,7 @@ export class OidcClient { response_mode = this.settings.response_mode, extraQueryParams = this.settings.extraQueryParams, extraTokenParams = this.settings.extraTokenParams, + dpopJkt, }: CreateSigninRequestArgs): Promise { const logger = this._logger.create("createSigninRequest"); @@ -134,7 +135,7 @@ export class OidcClient { scope, state_data: state, url_state, - prompt, display, max_age, ui_locales, id_token_hint, login_hint, acr_values, + prompt, display, max_age, ui_locales, id_token_hint, login_hint, acr_values, dpopJkt, resource, request, request_uri, extraQueryParams, extraTokenParams, request_type, response_mode, client_secret: this.settings.client_secret, skipUserInfo, @@ -176,8 +177,8 @@ export class OidcClient { const { state, response } = await this.readSigninResponseState(url, true); logger.debug("received state from storage; validating response"); - if (this.settings.dpop && this.settings.dpopStore) { - const dpopProof = await this.getDpopProof(this.settings.dpopStore); + if (this.settings.dpop && this.settings.dpop.store) { + const dpopProof = await this.getDpopProof(this.settings.dpop.store); extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; } await this._validator.validateSigninResponse(response, state, extraHeaders); @@ -237,8 +238,8 @@ export class OidcClient { scope = providedScopes.filter(s => allowableScopes.includes(s)).join(" "); } - if (this.settings.dpop && this.settings.dpopStore) { - const dpopProof = await this.getDpopProof(this.settings.dpopStore); + if (this.settings.dpop && this.settings.dpop.store) { + const dpopProof = await this.getDpopProof(this.settings.dpop.store); extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; } diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index fb5998abf..066c923db 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -23,6 +23,15 @@ export type SigningKey = Record; */ export type ExtraHeader = string | (() => string); +/** + * Optional DPoP settings + * @public + */ +export interface DPoPSettings { + bind_authorization_code?: boolean; + store?: DPoPStore; +} + /** * The settings used to configure the {@link OidcClient}. * @@ -124,12 +133,7 @@ export interface OidcClientSettings { /** * DPoP enabled or disabled */ - dpop?: boolean; - - /** - * DPoP store to use - */ - dpopStore?: DPoPStore; + dpop?: DPoPSettings | undefined; /** * Will check the content type header of the response of the revocation endpoint to match these passed values (default: []) @@ -194,8 +198,7 @@ export class OidcClientSettingsStore { // extra public readonly extraQueryParams: Record; public readonly extraTokenParams: Record; - public readonly dpop: boolean | undefined; - public readonly dpopStore: DPoPStore | undefined; + public readonly dpop: DPoPSettings | undefined; public readonly extraHeaders: Record; public readonly revokeTokenAdditionalContentTypes?: string[]; @@ -227,8 +230,7 @@ export class OidcClientSettingsStore { extraQueryParams = {}, extraTokenParams = {}, extraHeaders = {}, - dpop = false, - dpopStore, + dpop, }: OidcClientSettings) { this.authority = authority; @@ -280,6 +282,7 @@ export class OidcClientSettingsStore { else { const store = typeof window !== "undefined" ? window.localStorage : new InMemoryWebStorage(); this.stateStore = new WebStorageStateStore({ store }); + this.stateStore = new WebStorageStateStore({ store }); } this.refreshTokenAllowedScope = refreshTokenAllowedScope; @@ -288,6 +291,8 @@ export class OidcClientSettingsStore { this.extraTokenParams = extraTokenParams; this.extraHeaders = extraHeaders; this.dpop = dpop; - this.dpopStore = dpop && dpopStore ? dpopStore : new IndexedDbDPoPStore(); + if (this.dpop && !dpop?.store) { + this.dpop.store = new IndexedDbDPoPStore(); + } } } diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index ff4d060af..b8606c3d5 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -21,6 +21,7 @@ import type { State } from "./State"; import { mocked } from "jest-mock"; import { CryptoUtils } from "./utils"; +import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; describe("UserManager", () => { let userStoreMock: WebStorageStateStore; @@ -197,14 +198,17 @@ describe("UserManager", () => { token_endpoint: "http://sts/oidc/token", revocation_endpoint: "http://sts/oidc/revoke", }, - dpop: true, + dpop: { + bind_authorization_code: false, + store: new IndexedDbDPoPStore(), + }, }); const storeUserMock = jest.spyOn(subject, "storeUser"); const unloadMock = jest.spyOn(subject["_events"], "unload"); let dpopStoreMock; - if (subject.settings.dpopStore) { - dpopStoreMock = jest.spyOn(subject.settings.dpopStore, "remove"); + if (subject.settings.dpop?.store) { + dpopStoreMock = jest.spyOn(subject.settings.dpop.store, "remove"); } // act @@ -449,6 +453,59 @@ describe("UserManager", () => { handle, ); }); + + it("should pass dpopJkt to _signin if dpop.bind_authorization_code is true", async () => { + // arrange + subject = new UserManager({ + authority: "http://sts/oidc", + client_id: "client", + redirect_uri: "http://app/cb", + monitorSession : false, + userStore: userStoreMock, + metadata: { + authorization_endpoint: "http://sts/oidc/authorize", + end_session_endpoint: "http://sts/oidc/logout", + token_endpoint: "http://sts/oidc/token", + revocation_endpoint: "http://sts/oidc/revoke", + }, + dpop: { + bind_authorization_code: true, + store: new IndexedDbDPoPStore(), + }, + }); + + const handle = {} as PopupWindow; + jest.spyOn(subject["_popupNavigator"], "prepare") + .mockImplementation(() => Promise.resolve(handle)); + + const keyPair = await CryptoUtils.generateDPoPKeys(); + await subject.settings.dpop?.store?.set("client", keyPair); + + const user = new User({ + access_token: "access_token", + token_type: "token_type", + profile: { + sub: "sub", + nickname: "Nicholas", + } as UserProfile, + }); + + subject["_signin"] = jest.fn().mockResolvedValue(user); + + subject["_loadUser"] = jest.fn().mockResolvedValue(user); + + // act + await subject.signinPopup({ resource: "resource" }); + + // assert + expect(subject["_signin"]).toHaveBeenCalledWith({ + request_type: "si:p", + redirect_uri: "http://app/cb", + dpopJkt: expect.any(String), + resource: "resource", + display: "popup", + }, expect.any(Object)); + }); }); describe("signinPopupCallback", () => { @@ -627,6 +684,56 @@ describe("UserManager", () => { }), ); }); + + it("should pass dpopJkt if dpop.bind_authorization_code is true", async () => { + // arrange + subject = new UserManager({ + authority: "http://sts/oidc", + client_id: "client", + redirect_uri: "http://app/cb", + monitorSession : false, + userStore: userStoreMock, + metadata: { + authorization_endpoint: "http://sts/oidc/authorize", + end_session_endpoint: "http://sts/oidc/logout", + token_endpoint: "http://sts/oidc/token", + revocation_endpoint: "http://sts/oidc/revoke", + }, + dpop: { + bind_authorization_code: true, + store: new IndexedDbDPoPStore(), + }, + }); + + const keyPair = await CryptoUtils.generateDPoPKeys(); + await subject.settings.dpop?.store?.set("client", keyPair); + + const user = new User({ + access_token: "access_token", + token_type: "token_type", + profile: { + sub: "sub", + nickname: "Nicholas", + } as UserProfile, + }); + + subject["_signin"] = jest.fn().mockResolvedValue(user); + + subject["_loadUser"] = jest.fn().mockResolvedValue(user); + + // act + await subject.signinSilent({ resource: "resource" }); + + // assert + expect(subject["_signin"]).toHaveBeenCalledWith({ + request_type: "si:s", + id_token_hint: undefined, + redirect_uri: "http://app/cb", + prompt: "none", + dpopJkt: expect.any(String), + resource: "resource", + }, expect.any(Object), expect.any(String)); + }); }); describe("signinSilentCallback", () => { @@ -1063,15 +1170,18 @@ describe("UserManager", () => { token_endpoint: "http://sts/oidc/token", revocation_endpoint: "http://sts/oidc/revoke", }, - dpop: true, + dpop: { + bind_authorization_code: false, + store: new IndexedDbDPoPStore(), + }, }); const user = await subject.getUser() as User; let mockDpopStore; const keyPair = await CryptoUtils.generateDPoPKeys(); - if (subject.settings.dpopStore) { - mockDpopStore = jest.spyOn(subject.settings.dpopStore, "get").mockResolvedValue(keyPair); + if (subject.settings.dpop?.store) { + mockDpopStore = jest.spyOn(subject.settings.dpop.store, "get").mockResolvedValue(keyPair); } // act const dpopProof = await subject.dpopProof("http://some.url", user, "POST"); diff --git a/src/UserManager.ts b/src/UserManager.ts index 18b4e6f0e..453f43954 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -153,8 +153,8 @@ export class UserManager { public async removeUser(): Promise { const logger = this._logger.create("removeUser"); await this.storeUser(null); - if (this.settings.dpop && this.settings.dpopStore) { - await this.settings.dpopStore.remove(this.settings.client_id); + if (this.settings.dpop && this.settings.dpop.store) { + await this.settings.dpop.store.remove(this.settings.client_id); } logger.info("user removed from storage"); await this._events.unload(); @@ -173,9 +173,21 @@ export class UserManager { redirectMethod, ...requestArgs } = args; + + let dpopJkt: string | undefined; + if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { + let dpopKeys = await this.settings.dpop.store!.get(this.settings.client_id); + if (!dpopKeys) { + dpopKeys = await CryptoUtils.generateDPoPKeys(); + await this.settings.dpop.store!.set(this.settings.client_id, dpopKeys); + } + dpopJkt = await CryptoUtils.generateDPoPJkt(dpopKeys); + } + const handle = await this._redirectNavigator.prepare({ redirectMethod }); await this._signinStart({ request_type: "si:r", + dpopJkt: dpopJkt, ...requestArgs, }, handle); } @@ -238,7 +250,17 @@ export class UserManager { */ public async signinPopup(args: SigninPopupArgs = {}): Promise { const logger = this._logger.create("signinPopup"); + let dpopJkt: string | undefined; + if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { + let dpopKeys = await this.settings.dpop.store!.get(this.settings.client_id); + if (!dpopKeys) { + dpopKeys = await CryptoUtils.generateDPoPKeys(); + await this.settings.dpop.store!.set(this.settings.client_id, dpopKeys); + } + dpopJkt = await CryptoUtils.generateDPoPJkt(dpopKeys); + } + const { popupWindowFeatures, popupWindowTarget, @@ -308,7 +330,16 @@ export class UserManager { extraHeaders: extraHeaders, }); } + let dpopJkt: string | undefined; + if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { + let dpopKeys = await this.settings.dpop.store!.get(this.settings.client_id); + if (!dpopKeys) { + dpopKeys = await CryptoUtils.generateDPoPKeys(); + await this.settings.dpop.store!.set(this.settings.client_id, dpopKeys); + } + dpopJkt = await CryptoUtils.generateDPoPJkt(dpopKeys); + } const url = this.settings.silent_redirect_uri; if (!url) { @@ -797,7 +828,7 @@ export class UserManager { * @returns A promise containing the DPoP proof or undefined if DPoP is not enabled/no user is found. */ public async dpopProof(url: string, user: User, httpMethod?: string): Promise { - const dpopKey = await this.settings.dpopStore?.get(this.settings.client_id); + const dpopKey = await this.settings.dpop?.store?.get(this.settings.client_id); if (dpopKey) { return await CryptoUtils.generateDPoPProof({ url, From fe2eeff48e648bb36ccd53c78fe452446c3fd7d6 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Thu, 13 Jun 2024 20:41:21 +1200 Subject: [PATCH 24/33] Remove redundant import and dpopstore prop on oidcClient --- docs/oidc-client-ts.api.md | 4 ---- src/OidcClient.ts | 2 -- 2 files changed, 6 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index c368f822c..f35ccf77f 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -306,10 +306,6 @@ export class OidcClient { createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, dpopJkt, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; - // Warning: (ae-forgotten-export) The symbol "IndexedDbDPoPStore" needs to be exported by the entry point index.d.ts - // - // (undocumented) - protected readonly _dpopStore: IndexedDbDPoPStore | undefined; // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts // // (undocumented) diff --git a/src/OidcClient.ts b/src/OidcClient.ts index d7d1a8749..00c4d0a48 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -15,7 +15,6 @@ import { SigninState } from "./SigninState"; import { State } from "./State"; import { TokenClient } from "./TokenClient"; import { ClaimsService } from "./ClaimsService"; -import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; import type { DPoPStore } from "./DPoPStore"; /** @@ -80,7 +79,6 @@ export class OidcClient { protected readonly _claimsService: ClaimsService; protected readonly _validator: ResponseValidator; protected readonly _tokenClient: TokenClient; - protected readonly _dpopStore: IndexedDbDPoPStore | undefined; public constructor(settings: OidcClientSettings); public constructor(settings: OidcClientSettingsStore, metadataService: MetadataService); From 31a55c031cdae917719c5763568a26822d4d1575 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Fri, 14 Jun 2024 15:07:05 +1200 Subject: [PATCH 25/33] Add tests for dpop settings --- src/OidcClientSettings.test.ts | 30 ++++++++++++++++++++++++++++++ src/UserManager.ts | 10 ++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts index 9bd1adb2c..a9f300042 100644 --- a/src/OidcClientSettings.test.ts +++ b/src/OidcClientSettings.test.ts @@ -3,6 +3,7 @@ import { OidcClientSettingsStore } from "./OidcClientSettings"; import type { StateStore } from "./StateStore"; +import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; describe("OidcClientSettings", () => { describe("client_id", () => { @@ -530,4 +531,33 @@ describe("OidcClientSettings", () => { expect(subject.extraHeaders).toEqual(extraHeaders); }); }); + + describe("dpop", () => { + it("should not be defined by default", () => { + // act + const subject = new OidcClientSettingsStore({ + authority: "authority", + client_id: "client", + redirect_uri: "redirect", + }); + + // assert + expect(subject.dpop).toBeUndefined(); + }); + + it("dpop.store should be an IndexedDBDOoPstore by default when enabled", () => { + // act + const subject = new OidcClientSettingsStore({ + authority: "authority", + client_id: "client", + redirect_uri: "redirect", + dpop: { bind_authorization_code: true }, + }); + + // assert + expect(subject.dpop).toBeDefined(); + expect(subject.dpop?.store).toBeDefined(); + expect(subject.dpop?.store).toBeInstanceOf(IndexedDbDPoPStore); + }); + }); }); diff --git a/src/UserManager.ts b/src/UserManager.ts index 453f43954..c110cbdcd 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -153,8 +153,8 @@ export class UserManager { public async removeUser(): Promise { const logger = this._logger.create("removeUser"); await this.storeUser(null); - if (this.settings.dpop && this.settings.dpop.store) { - await this.settings.dpop.store.remove(this.settings.client_id); + if (this.settings.dpop) { + await this.settings.dpop.store!.remove(this.settings.client_id); } logger.info("user removed from storage"); await this._events.unload(); @@ -205,7 +205,8 @@ export class UserManager { const user = await this._signinEnd(url); if (user.profile && user.profile.sub) { logger.info("success, signed in subject", user.profile.sub); - } else { + } + else { logger.info("no subject"); } @@ -364,7 +365,8 @@ export class UserManager { if (user) { if (user.profile?.sub) { logger.info("success, signed in subject", user.profile.sub); - } else { + } + else { logger.info("no subject"); } } From 478bbb25d9c91d94d016d6f160e5ef0c8385e2cd Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Fri, 14 Jun 2024 19:25:53 +1200 Subject: [PATCH 26/33] Remove redundant dpopsettings on UserManagerSettings --- docs/oidc-client-ts.api.md | 2 -- src/UserManagerSettings.ts | 4 ---- 2 files changed, 6 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index f35ccf77f..c6e819a77 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -1063,8 +1063,6 @@ export interface UserManagerSettings extends OidcClientSettings { accessTokenExpiringNotificationTimeInSeconds?: number; automaticSilentRenew?: boolean; checkSessionIntervalInSeconds?: number; - // Warning: (ae-forgotten-export) The symbol "DPoPSettings_2" needs to be exported by the entry point index.d.ts - dpopSettings?: DPoPSettings_2; iframeNotifyParentOrigin?: string; iframeScriptOrigin?: string; includeIdTokenInSilentRenew?: boolean; diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index 6564ae88f..c384cedb7 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -60,10 +60,6 @@ export interface UserManagerSettings extends OidcClientSettings { validateSubOnSilentRenew?: boolean; /** Flag to control if id_token is included as id_token_hint in silent renew calls (default: false) */ includeIdTokenInSilentRenew?: boolean; - /** Indicates whether to apply Dynamic Proof Of Possession when requesting an access token - * See https://datatracker.ietf.org/doc/html/rfc9449 - */ - dpopSettings?: DPoPSettings; /** Will raise events for when user has performed a signout at the OP (default: false) */ monitorSession?: boolean; monitorAnonymousSession?: boolean; From 155e9f3301d5de34731e7ce07a30465d6cc441e4 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Mon, 17 Jun 2024 19:51:46 +1200 Subject: [PATCH 27/33] refactor dpopJkt into private method, add test for signinRedirect --- docs/oidc-client-ts.api.md | 2 ++ src/UserManager.test.ts | 43 ++++++++++++++++++++++++++++++++++++++ src/UserManager.ts | 32 ++++++++++++---------------- src/UserManagerSettings.ts | 5 ----- 4 files changed, 58 insertions(+), 24 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index c6e819a77..a2c367286 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -965,6 +965,8 @@ export class UserManager { get events(): UserManagerEvents; // (undocumented) protected readonly _events: UserManagerEvents; + // (undocumented) + generateDPoPJkt(dpopSettings: DPoPSettings): Promise; getUser(): Promise; // (undocumented) protected readonly _iframeNavigator: INavigator; diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index b8606c3d5..e18c9ab1f 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -337,6 +337,44 @@ describe("UserManager", () => { }), ); }); + + it("should pass dpopJkt to _signin if dpop.bind_authorization_code is true", async () => { + // arrange + subject = new UserManager({ + authority: "http://sts/oidc", + client_id: "client", + redirect_uri: "http://app/cb", + monitorSession : false, + userStore: userStoreMock, + metadata: { + authorization_endpoint: "http://sts/oidc/authorize", + end_session_endpoint: "http://sts/oidc/logout", + token_endpoint: "http://sts/oidc/token", + revocation_endpoint: "http://sts/oidc/revoke", + }, + dpop: { + bind_authorization_code: true, + store: new IndexedDbDPoPStore(), + }, + }); + jest.spyOn(subject["_redirectNavigator"], "prepare"); + subject["_signinStart"] = jest.fn(); + + const generateDPoPJktSpy = jest.spyOn(subject, "generateDPoPJkt"); + + const keyPair = await CryptoUtils.generateDPoPKeys(); + await subject.settings.dpop?.store?.set("client", keyPair); + + // act + await subject.signinRedirect(); + + // assert + expect(generateDPoPJktSpy).toHaveBeenCalled(); + expect(subject["_signinStart"]).toHaveBeenCalledWith({ + request_type: "si:r", + dpopJkt: expect.any(String), + }, expect.any(Object)); + }); }); describe("signinRedirectCallback", () => { @@ -477,6 +515,7 @@ describe("UserManager", () => { const handle = {} as PopupWindow; jest.spyOn(subject["_popupNavigator"], "prepare") .mockImplementation(() => Promise.resolve(handle)); + const generateDPoPJktSpy = jest.spyOn(subject, "generateDPoPJkt"); const keyPair = await CryptoUtils.generateDPoPKeys(); await subject.settings.dpop?.store?.set("client", keyPair); @@ -498,6 +537,7 @@ describe("UserManager", () => { await subject.signinPopup({ resource: "resource" }); // assert + expect(generateDPoPJktSpy).toHaveBeenCalled(); expect(subject["_signin"]).toHaveBeenCalledWith({ request_type: "si:p", redirect_uri: "http://app/cb", @@ -721,10 +761,13 @@ describe("UserManager", () => { subject["_loadUser"] = jest.fn().mockResolvedValue(user); + const generateDPoPJktSpy = jest.spyOn(subject, "generateDPoPJkt"); + // act await subject.signinSilent({ resource: "resource" }); // assert + expect(generateDPoPJktSpy).toHaveBeenCalled(); expect(subject["_signin"]).toHaveBeenCalledWith({ request_type: "si:s", id_token_hint: undefined, diff --git a/src/UserManager.ts b/src/UserManager.ts index c110cbdcd..2a86a5406 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -15,7 +15,7 @@ import type { SignoutResponse } from "./SignoutResponse"; import type { MetadataService } from "./MetadataService"; import { RefreshState } from "./RefreshState"; import type { SigninResponse } from "./SigninResponse"; -import type { ExtraHeader } from "./OidcClientSettings"; +import type { ExtraHeader, DPoPSettings } from "./OidcClientSettings"; /** * @public @@ -176,12 +176,7 @@ export class UserManager { let dpopJkt: string | undefined; if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { - let dpopKeys = await this.settings.dpop.store!.get(this.settings.client_id); - if (!dpopKeys) { - dpopKeys = await CryptoUtils.generateDPoPKeys(); - await this.settings.dpop.store!.set(this.settings.client_id, dpopKeys); - } - dpopJkt = await CryptoUtils.generateDPoPJkt(dpopKeys); + dpopJkt = await this.generateDPoPJkt(this.settings.dpop); } const handle = await this._redirectNavigator.prepare({ redirectMethod }); @@ -254,12 +249,7 @@ export class UserManager { let dpopJkt: string | undefined; if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { - let dpopKeys = await this.settings.dpop.store!.get(this.settings.client_id); - if (!dpopKeys) { - dpopKeys = await CryptoUtils.generateDPoPKeys(); - await this.settings.dpop.store!.set(this.settings.client_id, dpopKeys); - } - dpopJkt = await CryptoUtils.generateDPoPJkt(dpopKeys); + dpopJkt = await this.generateDPoPJkt(this.settings.dpop); } const { @@ -334,12 +324,7 @@ export class UserManager { let dpopJkt: string | undefined; if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { - let dpopKeys = await this.settings.dpop.store!.get(this.settings.client_id); - if (!dpopKeys) { - dpopKeys = await CryptoUtils.generateDPoPKeys(); - await this.settings.dpop.store!.set(this.settings.client_id, dpopKeys); - } - dpopJkt = await CryptoUtils.generateDPoPJkt(dpopKeys); + dpopJkt = await this.generateDPoPJkt(this.settings.dpop); } const url = this.settings.silent_redirect_uri; @@ -841,4 +826,13 @@ export class UserManager { } return undefined; } + + async generateDPoPJkt(dpopSettings: DPoPSettings): Promise { + let dpopKeys = await dpopSettings.store!.get(this.settings.client_id); + if (!dpopKeys) { + dpopKeys = await CryptoUtils.generateDPoPKeys(); + await dpopSettings.store!.set(this.settings.client_id, dpopKeys); + } + return await CryptoUtils.generateDPoPJkt(dpopKeys); + } } diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index c384cedb7..e90a4c08d 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -17,11 +17,6 @@ const DefaultAccessTokenExpiringNotificationTimeInSeconds = 60; const DefaultCheckSessionIntervalInSeconds = 2; export const DefaultSilentRequestTimeoutInSeconds = 10; -export interface DPoPSettings { - enabled: boolean; - bind_authorization_code?: boolean; -} - /** * The settings used to configure the {@link UserManager}. * From ea62cf5d11283817355576962335e61999466d3b Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Mon, 17 Jun 2024 19:55:31 +1200 Subject: [PATCH 28/33] Undo unneccessary changes to UserManager --- docs/oidc-client-ts.api.md | 2 +- src/UserManagerSettings.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index a2c367286..5799eabe2 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -1137,7 +1137,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { // (undocumented) readonly stopCheckSessionOnError: boolean; // (undocumented) - userStore: WebStorageStateStore; + readonly userStore: WebStorageStateStore; // (undocumented) readonly validateSubOnSilentRenew: boolean; } diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index e90a4c08d..b88ae211e 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -55,6 +55,7 @@ export interface UserManagerSettings extends OidcClientSettings { validateSubOnSilentRenew?: boolean; /** Flag to control if id_token is included as id_token_hint in silent renew calls (default: false) */ includeIdTokenInSilentRenew?: boolean; + /** Will raise events for when user has performed a signout at the OP (default: false) */ monitorSession?: boolean; monitorAnonymousSession?: boolean; @@ -119,7 +120,7 @@ export class UserManagerSettingsStore extends OidcClientSettingsStore { public readonly accessTokenExpiringNotificationTimeInSeconds: number; - public userStore: WebStorageStateStore; + public readonly userStore: WebStorageStateStore; public constructor(args: UserManagerSettings) { const { From c0a66efb219667f79ad44936282b1a9b8928e50d Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Tue, 18 Jun 2024 19:11:00 +1200 Subject: [PATCH 29/33] Tidy up --- src/OidcClientSettings.ts | 1 - src/UserManagerSettings.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 066c923db..707b03ca8 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -282,7 +282,6 @@ export class OidcClientSettingsStore { else { const store = typeof window !== "undefined" ? window.localStorage : new InMemoryWebStorage(); this.stateStore = new WebStorageStateStore({ store }); - this.stateStore = new WebStorageStateStore({ store }); } this.refreshTokenAllowedScope = refreshTokenAllowedScope; diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index b88ae211e..7353cc1a1 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -55,7 +55,6 @@ export interface UserManagerSettings extends OidcClientSettings { validateSubOnSilentRenew?: boolean; /** Flag to control if id_token is included as id_token_hint in silent renew calls (default: false) */ includeIdTokenInSilentRenew?: boolean; - /** Will raise events for when user has performed a signout at the OP (default: false) */ monitorSession?: boolean; monitorAnonymousSession?: boolean; From a92f2dba7e349395d3a799c5fd67240fdcc45eef Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Mon, 24 Jun 2024 20:07:45 +1200 Subject: [PATCH 30/33] Fix styling and make dpop.store non optional. --- docs/oidc-client-ts.api.md | 24 ++++++++++++++++++++++-- src/OidcClient.ts | 1 + src/OidcClientSettings.test.ts | 5 ++++- src/OidcClientSettings.ts | 7 ++----- src/UserManager.ts | 16 +++++++--------- src/UserManagerSettings.ts | 1 + src/index.ts | 1 + 7 files changed, 38 insertions(+), 17 deletions(-) diff --git a/docs/oidc-client-ts.api.md b/docs/oidc-client-ts.api.md index 5799eabe2..82ab0e73b 100644 --- a/docs/oidc-client-ts.api.md +++ b/docs/oidc-client-ts.api.md @@ -141,6 +141,28 @@ export interface INavigator { prepare(params: unknown): Promise; } +// Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +export class IndexedDbDPoPStore implements DPoPStore { + // (undocumented) + createStore(dbName: string, storeName: string): Promise<(txMode: IDBTransactionMode, callback: (store: IDBObjectStore) => T | PromiseLike) => Promise>; + // (undocumented) + readonly _dbName: string; + // (undocumented) + get(key: string): Promise; + // (undocumented) + getAllKeys(): Promise; + // (undocumented) + promisifyRequest(request: IDBRequest | IDBTransaction): Promise; + // (undocumented) + remove(key: string): Promise; + // (undocumented) + set(key: string, value: CryptoKeyPair): Promise; + // (undocumented) + readonly _storeName: string; +} + // @public (undocumented) export class InMemoryWebStorage implements Storage { // (undocumented) @@ -306,8 +328,6 @@ export class OidcClient { createSigninRequest({ state, request, request_uri, request_type, id_token_hint, login_hint, skipUserInfo, nonce, url_state, response_type, scope, redirect_uri, prompt, display, max_age, ui_locales, acr_values, resource, response_mode, extraQueryParams, extraTokenParams, dpopJkt, }: CreateSigninRequestArgs): Promise; // (undocumented) createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise; - // Warning: (ae-forgotten-export) The symbol "DPoPStore" needs to be exported by the entry point index.d.ts - // // (undocumented) getDpopProof(dpopStore: DPoPStore): Promise; // (undocumented) diff --git a/src/OidcClient.ts b/src/OidcClient.ts index 00c4d0a48..046ae818e 100644 --- a/src/OidcClient.ts +++ b/src/OidcClient.ts @@ -179,6 +179,7 @@ export class OidcClient { const dpopProof = await this.getDpopProof(this.settings.dpop.store); extraHeaders = { ...extraHeaders, "DPoP": dpopProof }; } + await this._validator.validateSigninResponse(response, state, extraHeaders); return response; } diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts index a9f300042..37df7ac36 100644 --- a/src/OidcClientSettings.test.ts +++ b/src/OidcClientSettings.test.ts @@ -551,7 +551,10 @@ describe("OidcClientSettings", () => { authority: "authority", client_id: "client", redirect_uri: "redirect", - dpop: { bind_authorization_code: true }, + dpop: { + bind_authorization_code: true, + store: new IndexedDbDPoPStore(), + }, }); // assert diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 707b03ca8..8fb654d04 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -6,7 +6,6 @@ import type { OidcMetadata } from "./OidcMetadata"; import type { StateStore } from "./StateStore"; import { InMemoryWebStorage } from "./InMemoryWebStorage"; import type { DPoPStore } from "./DPoPStore"; -import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; const DefaultResponseType = "code"; const DefaultScope = "openid"; @@ -29,7 +28,7 @@ export type ExtraHeader = string | (() => string); */ export interface DPoPSettings { bind_authorization_code?: boolean; - store?: DPoPStore; + store: DPoPStore; } /** @@ -289,9 +288,7 @@ export class OidcClientSettingsStore { this.extraQueryParams = extraQueryParams; this.extraTokenParams = extraTokenParams; this.extraHeaders = extraHeaders; + this.dpop = dpop; - if (this.dpop && !dpop?.store) { - this.dpop.store = new IndexedDbDPoPStore(); - } } } diff --git a/src/UserManager.ts b/src/UserManager.ts index 2a86a5406..39edb4ec6 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -154,7 +154,7 @@ export class UserManager { const logger = this._logger.create("removeUser"); await this.storeUser(null); if (this.settings.dpop) { - await this.settings.dpop.store!.remove(this.settings.client_id); + await this.settings.dpop.store.remove(this.settings.client_id); } logger.info("user removed from storage"); await this._events.unload(); @@ -175,14 +175,14 @@ export class UserManager { } = args; let dpopJkt: string | undefined; - if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { + if (this.settings.dpop?.bind_authorization_code) { dpopJkt = await this.generateDPoPJkt(this.settings.dpop); } const handle = await this._redirectNavigator.prepare({ redirectMethod }); await this._signinStart({ request_type: "si:r", - dpopJkt: dpopJkt, + dpopJkt, ...requestArgs, }, handle); } @@ -248,7 +248,7 @@ export class UserManager { const logger = this._logger.create("signinPopup"); let dpopJkt: string | undefined; - if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { + if (this.settings.dpop?.bind_authorization_code) { dpopJkt = await this.generateDPoPJkt(this.settings.dpop); } @@ -311,19 +311,17 @@ export class UserManager { if (user?.refresh_token) { logger.debug("using refresh token"); const state = new RefreshState(user as Required); - const extraHeaders: Record = {}; return await this._useRefreshToken({ state, redirect_uri: requestArgs.redirect_uri, resource: requestArgs.resource, extraTokenParams: requestArgs.extraTokenParams, timeoutInSeconds: silentRequestTimeoutInSeconds, - extraHeaders: extraHeaders, }); } let dpopJkt: string | undefined; - if (this.settings.dpop && this.settings.dpop.bind_authorization_code) { + if (this.settings.dpop?.bind_authorization_code) { dpopJkt = await this.generateDPoPJkt(this.settings.dpop); } @@ -828,10 +826,10 @@ export class UserManager { } async generateDPoPJkt(dpopSettings: DPoPSettings): Promise { - let dpopKeys = await dpopSettings.store!.get(this.settings.client_id); + let dpopKeys = await dpopSettings.store.get(this.settings.client_id); if (!dpopKeys) { dpopKeys = await CryptoUtils.generateDPoPKeys(); - await dpopSettings.store!.set(this.settings.client_id, dpopKeys); + await dpopSettings.store.set(this.settings.client_id, dpopKeys); } return await CryptoUtils.generateDPoPJkt(dpopKeys); } diff --git a/src/UserManagerSettings.ts b/src/UserManagerSettings.ts index 7353cc1a1..cf6ae3662 100644 --- a/src/UserManagerSettings.ts +++ b/src/UserManagerSettings.ts @@ -55,6 +55,7 @@ export interface UserManagerSettings extends OidcClientSettings { validateSubOnSilentRenew?: boolean; /** Flag to control if id_token is included as id_token_hint in silent renew calls (default: false) */ includeIdTokenInSilentRenew?: boolean; + /** Will raise events for when user has performed a signout at the OP (default: false) */ monitorSession?: boolean; monitorAnonymousSession?: boolean; diff --git a/src/index.ts b/src/index.ts index 387328c67..88acca6ec 100644 --- a/src/index.ts +++ b/src/index.ts @@ -44,3 +44,4 @@ export { UserManagerSettingsStore } from "./UserManagerSettings"; export type { UserManagerSettings } from "./UserManagerSettings"; export { Version } from "./Version"; export { WebStorageStateStore } from "./WebStorageStateStore"; +export { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; From e686ad5b4a2b2ce994ad4b8f74ebdd71209710e4 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Tue, 25 Jun 2024 13:53:00 +1200 Subject: [PATCH 31/33] Throw exception if dpop configured without a store --- src/OidcClientSettings.test.ts | 15 +++++---------- src/OidcClientSettings.ts | 3 +++ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/OidcClientSettings.test.ts b/src/OidcClientSettings.test.ts index 37df7ac36..fc8ed0eef 100644 --- a/src/OidcClientSettings.test.ts +++ b/src/OidcClientSettings.test.ts @@ -3,7 +3,6 @@ import { OidcClientSettingsStore } from "./OidcClientSettings"; import type { StateStore } from "./StateStore"; -import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore"; describe("OidcClientSettings", () => { describe("client_id", () => { @@ -545,22 +544,18 @@ describe("OidcClientSettings", () => { expect(subject.dpop).toBeUndefined(); }); - it("dpop.store should be an IndexedDBDOoPstore by default when enabled", () => { + it("should throw if dpop is configured without a store", () => { // act - const subject = new OidcClientSettingsStore({ + expect(() => new OidcClientSettingsStore({ authority: "authority", client_id: "client", redirect_uri: "redirect", + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore dpop: { bind_authorization_code: true, - store: new IndexedDbDPoPStore(), }, - }); - - // assert - expect(subject.dpop).toBeDefined(); - expect(subject.dpop?.store).toBeDefined(); - expect(subject.dpop?.store).toBeInstanceOf(IndexedDbDPoPStore); + })).toThrow("A DPoPStore is required when dpop is enabled"); }); }); }); diff --git a/src/OidcClientSettings.ts b/src/OidcClientSettings.ts index 8fb654d04..1382ba3f0 100644 --- a/src/OidcClientSettings.ts +++ b/src/OidcClientSettings.ts @@ -290,5 +290,8 @@ export class OidcClientSettingsStore { this.extraHeaders = extraHeaders; this.dpop = dpop; + if (this.dpop && !this.dpop?.store) { + throw new Error("A DPoPStore is required when dpop is enabled"); + } } } From 12ac53f6fd760978decb8a89a7300c4cfd074031 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Tue, 25 Jun 2024 19:50:27 +1200 Subject: [PATCH 32/33] Move removal of dpop Keys into UserManager.storeUser when user is null --- src/UserManager.test.ts | 36 ++++++++++++++++++++++++++++++++++++ src/UserManager.ts | 6 +++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index e18c9ab1f..30cd6d6c9 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -1196,6 +1196,42 @@ describe("UserManager", () => { const storageString = await subject.settings.userStore.get(subject["_userStoreKey"]); expect(storageString).toBeNull(); }); + + it("should remove dpop keys from the dpop store if dpop enabled", async () => { + // arrange + subject = new UserManager({ + authority: "http://sts/oidc", + client_id: "client", + redirect_uri: "http://app/cb", + monitorSession : false, + userStore: userStoreMock, + metadata: { + authorization_endpoint: "http://sts/oidc/authorize", + end_session_endpoint: "http://sts/oidc/logout", + token_endpoint: "http://sts/oidc/token", + revocation_endpoint: "http://sts/oidc/revoke", + }, + dpop: { store: new IndexedDbDPoPStore() }, + }); + + const user = new User({ + access_token: "access_token", + token_type: "token_type", + profile: {} as UserProfile, + }); + await subject.storeUser(user); + const dpopKeyPair = await CryptoUtils.generateDPoPKeys(); + await subject.settings.dpop?.store.set("client", dpopKeyPair); + + // act + await subject.storeUser(null); + + // assert + const storageString = await subject.settings.userStore.get(subject["_userStoreKey"]); + expect(storageString).toBeNull(); + const dpopKeys = await subject.settings.dpop?.store.get("client"); + expect(dpopKeys).toBeUndefined(); + }); }); describe("getDPoPProof",() => { diff --git a/src/UserManager.ts b/src/UserManager.ts index 80cc73584..ccc6471b2 100644 --- a/src/UserManager.ts +++ b/src/UserManager.ts @@ -153,9 +153,6 @@ export class UserManager { public async removeUser(): Promise { const logger = this._logger.create("removeUser"); await this.storeUser(null); - if (this.settings.dpop) { - await this.settings.dpop.store.remove(this.settings.client_id); - } logger.info("user removed from storage"); await this._events.unload(); } @@ -795,6 +792,9 @@ export class UserManager { } else { this._logger.debug("removing user"); await this.settings.userStore.remove(this._userStoreKey); + if (this.settings.dpop) { + await this.settings.dpop.store.remove(this.settings.client_id); + } } } From 77640787ae080709be25720bfeb9334af5054965 Mon Sep 17 00:00:00 2001 From: Chris Keogh Date: Tue, 25 Jun 2024 19:57:32 +1200 Subject: [PATCH 33/33] Remove if statements from tests --- src/UserManager.test.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/UserManager.test.ts b/src/UserManager.test.ts index 30cd6d6c9..59fe4ab4c 100644 --- a/src/UserManager.test.ts +++ b/src/UserManager.test.ts @@ -206,10 +206,8 @@ describe("UserManager", () => { const storeUserMock = jest.spyOn(subject, "storeUser"); const unloadMock = jest.spyOn(subject["_events"], "unload"); - let dpopStoreMock; - if (subject.settings.dpop?.store) { - dpopStoreMock = jest.spyOn(subject.settings.dpop.store, "remove"); - } + + const dpopStoreMock = jest.spyOn(subject.settings.dpop!.store, "remove"); // act await subject.removeUser(); @@ -1256,12 +1254,9 @@ describe("UserManager", () => { }); const user = await subject.getUser() as User; - - let mockDpopStore; const keyPair = await CryptoUtils.generateDPoPKeys(); - if (subject.settings.dpop?.store) { - mockDpopStore = jest.spyOn(subject.settings.dpop.store, "get").mockResolvedValue(keyPair); - } + const mockDpopStore = jest.spyOn(subject.settings.dpop!.store, "get").mockResolvedValue(keyPair); + // act const dpopProof = await subject.dpopProof("http://some.url", user, "POST");