Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dpop nonce support #1569

Merged
merged 9 commits into from
Jul 9, 2024
Merged
19 changes: 14 additions & 5 deletions docs/oidc-client-ts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ export type CreateSignoutRequestArgs = Omit<SignoutRequestArgs, "url" | "state_d
state?: unknown;
};

// @public (undocumented)
export class DPoPState {
constructor(keys: CryptoKeyPair, nonce?: string | undefined);
// (undocumented)
readonly keys: CryptoKeyPair;
// (undocumented)
nonce?: string | undefined;
}

// @public
export class ErrorResponse extends Error {
constructor(args: {
Expand Down Expand Up @@ -150,15 +159,15 @@ export class IndexedDbDPoPStore implements DPoPStore {
// (undocumented)
readonly _dbName: string;
// (undocumented)
get(key: string): Promise<CryptoKeyPair>;
get(key: string): Promise<DPoPState>;
// (undocumented)
getAllKeys(): Promise<string[]>;
// (undocumented)
promisifyRequest<T = undefined>(request: IDBRequest<T> | IDBTransaction): Promise<T>;
// (undocumented)
remove(key: string): Promise<CryptoKeyPair>;
remove(key: string): Promise<DPoPState>;
// (undocumented)
set(key: string, value: CryptoKeyPair): Promise<void>;
set(key: string, value: DPoPState): Promise<void>;
// (undocumented)
readonly _storeName: string;
}
Expand Down Expand Up @@ -329,7 +338,7 @@ export class OidcClient {
// (undocumented)
createSignoutRequest({ state, id_token_hint, client_id, request_type, post_logout_redirect_uri, extraQueryParams, }?: CreateSignoutRequestArgs): Promise<SignoutRequest>;
// (undocumented)
getDpopProof(dpopStore: DPoPStore): Promise<string>;
getDpopProof(dpopStore: DPoPStore, nonce?: string): Promise<string>;
// (undocumented)
protected readonly _logger: Logger;
// (undocumented)
Expand Down Expand Up @@ -989,7 +998,7 @@ export class UserManager {
clearStaleState(): Promise<void>;
// (undocumented)
protected readonly _client: OidcClient;
dpopProof(url: string, user: User, httpMethod?: string): Promise<string | undefined>;
dpopProof(url: string, user: User, httpMethod?: string, nonce?: string): Promise<string | undefined>;
get events(): UserManagerEvents;
// (undocumented)
protected readonly _events: UserManagerEvents;
Expand Down
16 changes: 13 additions & 3 deletions src/DPoPStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,18 @@
* @public
*/
export interface DPoPStore {
set(key: string, value: CryptoKeyPair): Promise<void>;
get(key: string): Promise<CryptoKeyPair>;
remove(key: string): Promise<CryptoKeyPair>;
set(key: string, value: DPoPState): Promise<void>;
get(key: string): Promise<DPoPState>;
remove(key: string): Promise<DPoPState>;
getAllKeys(): Promise<string[]>;
}

/**
* @public
*/
export class DPoPState {
public constructor(
public readonly keys: CryptoKeyPair,
public nonce?: string,
) { }
}
19 changes: 16 additions & 3 deletions src/IndexedDbDPoPStore.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore";
import { DPoPState } from "./DPoPStore";

describe("DPoPStore", () => {
const subject = new IndexedDbDPoPStore();

let data: CryptoKeyPair;
let data: DPoPState;

const createCryptoKeyPair = async () => {
return await window.crypto.subtle.generateKey(
Expand All @@ -17,7 +18,8 @@ describe("DPoPStore", () => {
};

beforeEach(async () => {
data = await createCryptoKeyPair();
const crytoKeys = await createCryptoKeyPair();
data = new DPoPState(crytoKeys);
});

describe("set", () => {
Expand All @@ -37,6 +39,16 @@ describe("DPoPStore", () => {

expect(result).toEqual(data);
});

it("should store a nonce if provided in IndexedDB", async () => {
data = new DPoPState(data.keys, "some-nonce");
await subject.set("foo", data);

const result = await subject.get("foo");

expect(result).toEqual(data);
expect(result.nonce).toEqual("some-nonce");
});
});

describe("remove", () => {
Expand Down Expand Up @@ -82,7 +94,8 @@ describe("DPoPStore", () => {

it("should get all keys in IndexedDB", async () => {
await subject.set("foo", data);
const dataTwo = await createCryptoKeyPair();
const crytoKeys = await createCryptoKeyPair();
const dataTwo = new DPoPState(crytoKeys);
await subject.set("boo", dataTwo);

const result = await subject.getAllKeys();
Expand Down
10 changes: 5 additions & 5 deletions src/IndexedDbDPoPStore.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { DPoPStore } from "./DPoPStore";
import { DPoPState, type DPoPStore } from "./DPoPStore";

/**
* Provides a default implementation of the DPoP store using IndexedDB.
Expand All @@ -9,22 +9,22 @@ export class IndexedDbDPoPStore implements DPoPStore {
readonly _dbName: string = "oidc";
readonly _storeName: string = "dpop";

public async set(key: string, value: CryptoKeyPair): Promise<void> {
public async set(key: string, value: DPoPState): Promise<void> {
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<CryptoKeyPair> {
public async get(key: string): Promise<DPoPState> {
const store = await this.createStore(this._dbName, this._storeName);
return await store("readonly", (str) => {
return this.promisifyRequest(str.get(key));
}) as CryptoKeyPair;
}) as DPoPState;
}

public async remove(key: string): Promise<CryptoKeyPair> {
public async remove(key: string): Promise<DPoPState> {
const item = await this.get(key);
const store = await this.createStore(this._dbName, this._storeName);
await store("readwrite", (str) => {
Expand Down
19 changes: 19 additions & 0 deletions src/JsonService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { JsonService } from "./JsonService";

import { mocked } from "jest-mock";
import type { ExtraHeader } from "./OidcClientSettings";
import { ErrorDPoPNonce } from "./errors/ErrorDPoPNonce";

describe("JsonService", () => {
let subject: JsonService;
Expand Down Expand Up @@ -530,6 +531,24 @@ describe("JsonService", () => {
.rejects.toThrow("Invalid response Content-Type: text/html");
});

it("should reject promise and throw ErrorDPoPNonce error when http response is 400 and response headers include dpop-nonce", async () => {
// arrange
const json = { foo: 1, bar: "test" };
mocked(fetch).mockResolvedValue({
status: 400,
ok: false,
headers: new Headers({
"dpop-nonce": "some-nonce",
}),
text: () => Promise.resolve(JSON.stringify(json)),
} as Response);

// act
await expect(subject.postForm("http://test", { body: new URLSearchParams("payload=dummy") }))
// assert
.rejects.toThrow(ErrorDPoPNonce);
});

it("should reject promise when http response is not 200", async () => {
// arrange
const json = {};
Expand Down
5 changes: 5 additions & 0 deletions src/JsonService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { ErrorResponse, ErrorTimeout } from "./errors";
import type { ExtraHeader } from "./OidcClientSettings";
import { Logger } from "./utils";
import { ErrorDPoPNonce } from "./errors/ErrorDPoPNonce";

/**
* @internal
Expand Down Expand Up @@ -180,6 +181,10 @@ export class JsonService {

if (!response.ok) {
logger.error("Error from server:", json);
if (response.headers.has("dpop-nonce")) {
const nonce = response.headers.get("dpop-nonce") as string;
throw new ErrorDPoPNonce(nonce, `${JSON.stringify(json)}`);
}
if (json.error) {
throw new ErrorResponse(json, body);
}
Expand Down
150 changes: 149 additions & 1 deletion src/OidcClient.test.ts
Original file line number Diff line number Diff line change
@@ -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 { JwtUtils } from "./utils";
import { CryptoUtils, JwtUtils } from "./utils";
import type { ErrorResponse } from "./errors";
import type { JwtClaims } from "./Claims";
import { OidcClient } from "./OidcClient";
Expand All @@ -15,6 +15,8 @@ import { RefreshState } from "./RefreshState";
import { SigninResponse } from "./SigninResponse";
import type { UserProfile } from "./User";
import { IndexedDbDPoPStore } from "./IndexedDbDPoPStore";
import { ErrorDPoPNonce } from "./errors/ErrorDPoPNonce";
import { DPoPState } from "./DPoPStore";

describe("OidcClient", () => {
let subject: OidcClient;
Expand Down Expand Up @@ -426,6 +428,43 @@ describe("OidcClient", () => {
expect(metadataServiceMock).toHaveBeenCalled();
expect(validateSigninResponseMock).toHaveBeenCalledWith(response, item, { "DPoP": expect.any(String) });
});

it("should retry code request if original fails with ErrorDPoPNonce exception", async () => {
subject = new OidcClient({
authority: "authority",
client_id: "client",
redirect_uri: "redirect",
post_logout_redirect_uri: "http://app",
dpop: {
bind_authorization_code: false,
store: new IndexedDbDPoPStore(),
},
});

// 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")
.mockRejectedValueOnce(new ErrorDPoPNonce("some-nonce"));
const getDpopProofMock = jest.spyOn(subject, "getDpopProof");
const metadataServiceMock = jest.spyOn(subject["metadataService"], "getTokenEndpoint").mockResolvedValue("http://sts/token");
// act
await subject.processSigninResponse("http://app/cb?state=1");

// assert
expect(metadataServiceMock).toHaveBeenCalled();
expect(validateSigninResponseMock).toHaveBeenCalledTimes(2);
expect(getDpopProofMock).toHaveBeenCalledTimes(2);
expect(getDpopProofMock).toHaveBeenNthCalledWith(2, { "_dbName": "oidc", "_storeName": "dpop" }, "some-nonce");
});
});

describe("processResourceOwnerPasswordCredentials", () => {
Expand Down Expand Up @@ -654,6 +693,50 @@ describe("OidcClient", () => {
expect(response).toHaveProperty("scope", state.scope);
});

it("should retry token exchange if tokenClient exchange throws ErrorDPoPNonce exception", async () => {
// arrange
const settings: OidcClientSettings = {
authority: "authority",
client_id: "client",
redirect_uri: "redirect",
post_logout_redirect_uri: "http://app",
dpop: {
bind_authorization_code: false,
store: new IndexedDbDPoPStore(),
},
};

subject = new OidcClient(settings);

const tokenResponse = {
access_token: "new_access_token",
};
const exchangeRefreshTokenMock =
jest.spyOn(subject["_tokenClient"], "exchangeRefreshToken")
.mockRejectedValueOnce(new ErrorDPoPNonce("some-nonce"))
.mockResolvedValueOnce(tokenResponse);
jest.spyOn(JwtUtils, "decode").mockReturnValue({ sub: "sub" });
jest.spyOn(subject["metadataService"], "getTokenEndpoint").mockResolvedValue("http://sts/token");
const getDpopProofSpy = jest.spyOn(subject, "getDpopProof");

const state = new RefreshState({
refresh_token: "refresh_token",
id_token: "id_token",
session_state: "session_state",
scope: "openid",
profile: {} as UserProfile,
});

// act
await subject.useRefreshToken({ state, resource: "resource" });

// assert

expect(exchangeRefreshTokenMock).toHaveBeenCalledTimes(2);
expect(getDpopProofSpy).toHaveBeenCalledTimes(2);
expect(getDpopProofSpy).toHaveBeenNthCalledWith(2, { "_dbName": "oidc", "_storeName": "dpop" }, "some-nonce");
});

it("should pass extraHeaders to tokenClient.exchangeRefreshToken if supplied", async () => {
// arrange
const tokenResponse = {
Expand Down Expand Up @@ -1032,4 +1115,69 @@ describe("OidcClient", () => {
});
});
});

describe("getDpopProof", () => {
it("stores a nonce if one is provided", async () => {
// arrange
const store = new IndexedDbDPoPStore();
const keyPair = await CryptoUtils.generateDPoPKeys();
jest.spyOn(CryptoUtils, "generateDPoPKeys").mockResolvedValue(keyPair);

const settings: OidcClientSettings = {
authority: "authority",
client_id: "dpop_client",
redirect_uri: "redirect",
post_logout_redirect_uri: "http://app",
dpop: {
bind_authorization_code: false,
store: store,
},
};

subject = new OidcClient(settings);

jest.spyOn(subject["metadataService"], "getTokenEndpoint").mockResolvedValue("http://sts/token");

// act
await subject.getDpopProof(store, "some-nonce");
const storedDPoPState = await subject.settings.dpop?.store.get("dpop_client");
// assert

expect(storedDPoPState?.nonce).toEqual("some-nonce");
});

it("updates the stored nonce if a new one is provided", async () => {
// arrange
const store = new IndexedDbDPoPStore();
const keyPair = await CryptoUtils.generateDPoPKeys();
jest.spyOn(CryptoUtils, "generateDPoPKeys").mockResolvedValue(keyPair);
const dpopState = new DPoPState(keyPair, "some-nonce");

const settings: OidcClientSettings = {
authority: "authority",
client_id: "dpop_client",
redirect_uri: "redirect",
post_logout_redirect_uri: "http://app",
dpop: {
bind_authorization_code: false,
store: store,
},
};

subject = new OidcClient(settings);

jest.spyOn(subject["metadataService"], "getTokenEndpoint").mockResolvedValue("http://sts/token");
await subject.getDpopProof(store, "some-nonce");
let storedDPoPState = await subject.settings.dpop?.store.get("dpop_client");
expect(storedDPoPState?.nonce).toEqual("some-nonce");
dpopState.nonce = "some-other-nonce";

// act
await subject.getDpopProof(store, "some-other-nonce");
storedDPoPState = await subject.settings.dpop?.store.get("dpop_client");

// assert
expect(storedDPoPState?.nonce).toEqual("some-other-nonce");
});
});
});
Loading
Loading