Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jaredperreault-okta committed Apr 15, 2024
1 parent 9682070 commit 5e8bd34
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 35 deletions.
44 changes: 40 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ DPoP: eyJ0eXAiOiJkcG9wK2p3dCIsIm...
```javascript
async function dpopAuthenticatedFetch (url, options) {
const { method } = options;
const dpop = await authClient.getDPoPHeaders({ url, method });
const dpop = await authClient.getDPoPAuthorizationHeaders({ url, method });
// dpop = { Authorization: "DPoP token****", Dpop: "proof****" }
const headers = new Headers({...options.headers, ...dpop});
return fetch(url, {...options, headers });
Expand All @@ -456,9 +456,9 @@ async function dpopAuthenticatedFetch (url, options) {
// resp = HTTP/1.1 401 Unauthorized...

if (!resp.ok) {
const nonce = authClient.getDPoPNonceHeader(resp.headers);
const nonce = authClient.parseUseDPoPNonceError(resp.headers);
if (nonce) {
const retryDpop = await authClient.getDPoPHeaders({ url, method, nonce });
const retryDpop = await authClient.getDPoPAuthorizationHeaders({ url, method, nonce });
const retryHeaders = new Headers({...options.headers, ...retryDpop});
return fetch(url, {...options, headers: retryHeaders });
}
Expand All @@ -481,7 +481,7 @@ useEffect(() => {
}, []);
```

#### Clear Storage (*Recommended*)
#### Clear DPoP Storage (*Recommended*)
DPoP requires the generation of a `CryptoKeyPair` which needs to persist in storage. Methods like `signOut()` or `revokeAccessToken()` will clear the key pair, however users don't always explicitly logout. It's good practice to clear storage before login to flush any orphaned key pairs generated from previously requested tokens

```javascript
Expand Down Expand Up @@ -1003,6 +1003,9 @@ This is accomplished by selecting a single tab to handle the network requests to
* [tx.resume](#txresume)
* [tx.exists](#txexists)
* [transaction.status](#transactionstatus)
* [getDPoPAuthorizationHeaders](#getdpopauthorizationheaders)
* [parseUseDPoPNonceError](#parseusedpopnonceerror)
* [clearDPoPStorage](#cleardpopstorage)
* [session](#session)
* [session.setCookieAndRedirect](#sessionsetcookieandredirectsessiontoken-redirecturi)
* [session.exists](#sessionexists)
Expand Down Expand Up @@ -1354,6 +1357,39 @@ See [authn API](docs/authn.md#txexists).

See [authn API](docs/authn.md#transactionstatus).

### `getDPoPAuthorizationHeaders(params)`

> :link: web browser only <br>
> :hourglass: async <br>
Requires [dpop](#dpop) set to `true`. Returns `Authorization` and `Dpop` header values to build a DPoP protected-request.

Params: `url` and (http) `method` are required.
* `accessToken` is optional, but will be read from `tokenStorage` if not provided
* `nonce` is optional, may be provided via `use_dpop_nonce` pattern from Resource Server ([more info](#handling-use_dpop_nonce))

### `parseUseDPoPNonceError(headers)`

> :link: web browser only <br>
Utility to extract and parse the `WWW-Authenticate` and `DPoP-Nonce` headers from a network response from a DPoP-protected request. Should the response be in the following format, the `nonce` value will be returned. Otherwise returns `null`

```
HTTP/1.1 401 Unauthorized
WWW-Authenticate: DPoP error="use_dpop_nonce", \
error_description="Resource server requires nonce in DPoP proof"
DPoP-Nonce: eyJ7S_zG.eyJH0-Z.HX4w-7v
```

### `clearDPoPStorage(clearAll=false)`

> :link: web browser only <br>
> :hourglass: async <br>
Clears storage location of `CryptoKeyPair`s generated and used by DPoP. Pass `true` to remove all key pairs as it's possible for orphaned key pairs to exist. If `clearAll` is `false`, the key pair bound to the current `accessToken` in tokenStorage will be removed.

It's recommended to call this function during user login. [See Example](#clear-dpop-storage-recommended)

### `session`

#### `session.setCookieAndRedirect(sessionToken, redirectUri)`
Expand Down
14 changes: 6 additions & 8 deletions lib/oidc/dpop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,12 @@ export async function findKeyPair (pairId?: string): Promise<CryptoKeyPair> {
throw new AuthSdkError(`Unable to locate dpop key pair required for refresh${pairId ? ` (${pairId})` : ''}`);
}

// deletes a specifc KP, or the entire db if no id is passed
export async function clearDPoPKeyPair (pairId?: string): Promise<void> {
if (pairId) {
await invokeStoreMethod('delete', pairId);
}
else {
await invokeStoreMethod('clear');
}
export async function clearDPoPKeyPair (pairId: string): Promise<void> {
await invokeStoreMethod('delete', pairId);
}

export async function clearAllDPoPKeyPairs (): Promise<void> {
await invokeStoreMethod('clear');
}

// generates a crypto (non-extractable) private key pair and writes it to indexeddb, returns key (id)
Expand Down
9 changes: 5 additions & 4 deletions lib/oidc/mixin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { getOAuthUrls, isLoginRedirect, hasResponseType } from '../util';
import {
generateDPoPProof,
clearDPoPKeyPair,
clearAllDPoPKeyPairs,
clearDPoPKeyPairAfterRevoke,
findKeyPair,
isDPoPNonceError
Expand Down Expand Up @@ -339,7 +340,7 @@ export function mixinOAuth
}

const dpopPairId = accessToken?.dpopPairId ?? refreshToken?.dpopPairId;
if (this.options.dpop) {
if (this.options.dpop && dpopPairId) {
await clearDPoPKeyPair(dpopPairId);
}

Expand Down Expand Up @@ -373,7 +374,7 @@ export function mixinOAuth
}
}

async getDPoPHeaders (params: DPoPRequest): Promise<DPoPHeaders> {
async getDPoPAuthorizationHeaders (params: DPoPRequest): Promise<DPoPHeaders> {
if (!this.options.dpop) {
throw new AuthSdkError('DPoP is not configured for this client instance');
}
Expand All @@ -397,7 +398,7 @@ export function mixinOAuth

async clearDPoPStorage (clearAll=false): Promise<void> {
if (clearAll) {
return clearDPoPKeyPair();
return clearAllDPoPKeyPairs();
}

const tokens = await this.tokenManager.getTokens();
Expand All @@ -408,7 +409,7 @@ export function mixinOAuth
}
}

getDPoPNonceHeader (headers: HeadersInit): string | null {
parseUseDPoPNonceError (headers: HeadersInit): string | null {
const wwwAuth = WWWAuthError.getWWWAuthenticateHeader(headers);
const wwwErr = WWWAuthError.parseHeader(wwwAuth ?? '');
if (isDPoPNonceError(wwwErr)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/oidc/types/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export interface OktaAuthOAuthInterface
revokeAccessToken(accessToken?: AccessToken): Promise<unknown>;
revokeRefreshToken(refreshToken?: RefreshToken): Promise<unknown>;

getDPoPHeaders(params: DPoPRequest): Promise<DPoPHeaders>;
getDPoPAuthorizationHeaders(params: DPoPRequest): Promise<DPoPHeaders>;
clearDPoPStorage(clearAll: boolean): Promise<void>;
getDPoPNonceHeader(headers: HeadersInit): string | null;
parseUseDPoPNonceError(headers: HeadersInit): string | null;
}
8 changes: 4 additions & 4 deletions test/apps/app/src/testApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ function bindFunctions(testApp: TestApp, window: Window): void {
startService: testApp.startService.bind(testApp),
stopService: testApp.stopService.bind(testApp),
enrollAuthenticator: testApp.enrollAuthenticator.bind(testApp),
getDPoPHeaders: testApp.getDPoPHeaders.bind(testApp)
getDPoPAuthorizationHeaders: testApp.getDPoPAuthorizationHeaders.bind(testApp)
};
Object.keys(boundFunctions).forEach(functionName => {
(window as any)[functionName] = makeClickHandler((boundFunctions as any)[functionName]);
Expand Down Expand Up @@ -739,9 +739,9 @@ class TestApp {
});
}

async getDPoPHeaders(): Promise<string> {
async getDPoPAuthorizationHeaders(): Promise<string> {
// passes dummy values for url and method
const headers = await this.oktaAuth.getDPoPHeaders({
const headers = await this.oktaAuth.getDPoPAuthorizationHeaders({
url: 'http://localhost:8080/foo',
method: 'POST'
});
Expand Down Expand Up @@ -983,7 +983,7 @@ class TestApp {
<a id="cross-tab-token-renew" onclick="simulateCrossTabTokenRenew(event)" class="pure-menu-link">Simulate cross-tab token renew</a>
</li>
<li class="pure-menu-item">
<a id="dpop-proof" onclick="getDPoPHeaders(event)" class="pure-menu-link">Generate DPoP Proof</a>
<a id="dpop-proof" onclick="getDPoPAuthorizationHeaders(event)" class="pure-menu-link">Generate DPoP Proof</a>
</li>
<li class="pure-menu-item">
<a id="enroll-authenticator" href="/" onclick="enrollAuthenticator(event)" class="pure-menu-link">Enroll Authenticator</a>
Expand Down
23 changes: 13 additions & 10 deletions test/spec/OktaAuth/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const mocked = {
findKeyPair: jest.fn(),
// generateDPoPProof: jest.fn(),
clearDPoPKeyPair: jest.fn(),
clearAllDPoPKeyPairs: jest.fn(),
clearDPoPKeyPairAfterRevoke: jest.fn(),
};
jest.mock('../../../lib/oidc/dpop', () => {
Expand Down Expand Up @@ -634,14 +635,14 @@ describe('OktaAuth (api)', function() {

});

describe('getDPoPHeaders', () => {
describe('getDPoPAuthorizationHeaders', () => {
beforeEach(() => {
auth = new OktaAuth({ issuer, dpop: true });
});

it('should throw if dpop is not enabled', async () => {
auth.options.dpop = false;
await expect(auth.getDPoPHeaders({
await expect(auth.getDPoPAuthorizationHeaders({
url: 'localhost:8080/test', method: 'GET'
})).rejects.toThrow(new AuthSdkError('DPoP is not configured for this client instance'));
});
Expand All @@ -651,7 +652,7 @@ describe('OktaAuth (api)', function() {
jest.spyOn(auth.tokenManager, 'getTokensSync').mockReturnValue({ accessToken });
mocked.findKeyPair.mockImplementation(async () => await generateKeyPair());

const headers = await auth.getDPoPHeaders({ url: 'localhost:8080/test', method: 'GET' });
const headers = await auth.getDPoPAuthorizationHeaders({ url: 'localhost:8080/test', method: 'GET' });

expect(headers).toMatchObject({
Authorization: `DPoP ${accessToken.accessToken}`,
Expand All @@ -660,15 +661,15 @@ describe('OktaAuth (api)', function() {
});

it('should throw if no accessToken exists', async () => {
await expect(auth.getDPoPHeaders({ url: 'localhost:8080/test', method: 'GET' }))
await expect(auth.getDPoPAuthorizationHeaders({ url: 'localhost:8080/test', method: 'GET' }))
.rejects.toThrow(new AuthSdkError('AccessToken is required to generate a DPoP Proof'));
});

it('should return DPoP headers (from options)', async () => {
const accessToken = { accessToken: 'fake' };
mocked.findKeyPair.mockImplementation(async () => await generateKeyPair());

const headers = await auth.getDPoPHeaders(
const headers = await auth.getDPoPAuthorizationHeaders(
{ url: 'localhost:8080/test', method: 'GET', accessToken }
);

Expand All @@ -686,24 +687,26 @@ describe('OktaAuth (api)', function() {

it('should clear all DPoP keys', async () => {
await auth.clearDPoPStorage(true);
expect(mocked.clearDPoPKeyPair).toHaveBeenCalled();
expect(mocked.clearDPoPKeyPair.mock.calls[0]).toEqual([]); // called with no args
expect(mocked.clearAllDPoPKeyPairs).toHaveBeenCalled();
expect(mocked.clearDPoPKeyPair).not.toHaveBeenCalled();
});

it('should no-op', async () => {
await auth.clearDPoPStorage();
expect(mocked.clearDPoPKeyPair).not.toHaveBeenCalled();
expect(mocked.clearAllDPoPKeyPairs).not.toHaveBeenCalled();
});

it('should clear specific key pair', async () => {
const accessToken = { accessToken: 'fake', dpopPairId: 'foo' };
jest.spyOn(auth.tokenManager, 'getTokens').mockReturnValue({ accessToken });
await auth.clearDPoPStorage();
expect(mocked.clearDPoPKeyPair).toHaveBeenCalledWith('foo');
expect(mocked.clearAllDPoPKeyPairs).not.toHaveBeenCalled();
});
});

describe('getDPoPNonceHeader', () => {
describe('parseUseDPoPNonceError', () => {
it('get header from Headers instance', () => {
if (!global.Headers) {
// do not fail in node tests
Expand All @@ -715,7 +718,7 @@ describe('OktaAuth (api)', function() {
'WWW-Authenticate': `DPoP error="use_dpop_nonce", error_description="Resource server requires nonce in DPoP proof"`
});

const nonce = auth.getDPoPNonceHeader(headers);
const nonce = auth.parseUseDPoPNonceError(headers);
expect(nonce).toEqual('nonceuponatime');
});

Expand All @@ -725,7 +728,7 @@ describe('OktaAuth (api)', function() {
'WWW-Authenticate': `DPoP error="use_dpop_nonce", error_description="Resource server requires nonce in DPoP proof"`
};

const nonce = auth.getDPoPNonceHeader(headers);
const nonce = auth.parseUseDPoPNonceError(headers);
expect(nonce).toEqual('nonceuponatime');
});
});
Expand Down
8 changes: 5 additions & 3 deletions test/spec/oidc/dpop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('dpop', () => {

describe('Key Store', () => {
beforeEach(async () => {
await dpop.clearDPoPKeyPair();
await dpop.clearAllDPoPKeyPairs();
indexedDB.deleteDatabase('OktaAuthJs');
});

Expand Down Expand Up @@ -62,13 +62,15 @@ describe('dpop', () => {
await expect(dpop.findKeyPair(kp1.keyPairId)).rejects.toThrow();
await expect(dpop.findKeyPair(kp2.keyPairId)).resolves.toBeDefined();
});
});

it('should delete all KPs when no keyId is provided', async () => {
describe('clearAllDPoPKeyPairs', () => {
it('should delete all KPs', async () => {
const kp1 = await dpop.createDPoPKeyPair();
const kp2 = await dpop.createDPoPKeyPair();
await expect(dpop.findKeyPair(kp1.keyPairId)).resolves.toBeDefined();
await expect(dpop.findKeyPair(kp2.keyPairId)).resolves.toBeDefined();
await dpop.clearDPoPKeyPair();
await dpop.clearAllDPoPKeyPairs();
await expect(dpop.findKeyPair(kp1.keyPairId)).rejects.toThrow();
await expect(dpop.findKeyPair(kp2.keyPairId)).rejects.toThrow();
});
Expand Down

0 comments on commit 5e8bd34

Please sign in to comment.