Skip to content

Commit

Permalink
piggy-back on current authentication request
Browse files Browse the repository at this point in the history
It was previously possible for multiple outbound
/user_management/authenticate requests to happen simultaneously if
`getAccessToken` was called multiple times when the current access token
had expired.

This PR moves the authenticate request into the client state so that
subsequent calls to `getAccessToken` can just await the existing one.
  • Loading branch information
cmatheson committed Nov 28, 2024
1 parent 52990d2 commit ddfca6d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 23 deletions.
25 changes: 17 additions & 8 deletions src/create-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,17 +385,26 @@ describe("create-client", () => {
refreshScope.done();
});

it("sends a request for each call", async () => {
it("only issues one request for multiple calls", async () => {
const client = await clientWithExpiredAccessToken();

const { scope: refresh1 } = nockRefresh();
const { scope: refresh2 } = nockRefresh();
const { scope: refreshScope } = nockRefresh({
accessTokenClaims: {
jti: "refreshed-token",
},
});

const accessToken1 = client.getAccessToken();
const accessToken2 = client.getAccessToken();
await Promise.all([accessToken1, accessToken2]);
refresh1.done();
refresh2.done();
const accessToken1 = client.getAccessToken().then((token) => {
expect(getClaims(token).jti).toEqual("refreshed-token");
});
const accessToken2 = client.getAccessToken().then((token) => {
expect(getClaims(token).jti).toEqual("refreshed-token");
});
const accessToken3 = client.getAccessToken().then((token) => {
expect(getClaims(token).jti).toEqual("refreshed-token");
});
await Promise.all([accessToken1, accessToken2, accessToken3]);
refreshScope.done();
});

it("throws an error if the refresh fails", async () => {
Expand Down
49 changes: 36 additions & 13 deletions src/create-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ interface RedirectOptions {

type State =
| { tag: "INITIAL" }
| { tag: "AUTHENTICATING" }
| { tag: "AUTHENTICATING"; response: Promise<AuthenticationResponse> }
| { tag: "AUTHENTICATED" }
| { tag: "ERROR" };

Expand Down Expand Up @@ -171,7 +171,6 @@ export class Client {
const code = url.searchParams.get("code");
const stateParam = url.searchParams.get("state");
const state = stateParam ? JSON.parse(stateParam) : undefined;
this.#state = { tag: "AUTHENTICATING" };

// grab the previously stored code verifier from session storage
const codeVerifier = window.sessionStorage.getItem(
Expand All @@ -181,13 +180,17 @@ export class Client {
if (code) {
if (codeVerifier) {
try {
const authenticationResponse = await authenticateWithCode({
baseUrl: this.#baseUrl,
clientId: this.#clientId,
code,
codeVerifier,
useCookie: this.#useCookie,
});
this.#state = {
tag: "AUTHENTICATING",
response: authenticateWithCode({
baseUrl: this.#baseUrl,
clientId: this.#clientId,
code,
codeVerifier,
useCookie: this.#useCookie,
}),
};
const authenticationResponse = await this.#state.response;

if (authenticationResponse) {
this.#state = { tag: "AUTHENTICATED" };
Expand Down Expand Up @@ -258,11 +261,31 @@ An authorization_code was supplied for a login which did not originate at the ap
}

async #refreshSession({ organizationId }: { organizationId?: string } = {}) {
if (this.#state.tag === "AUTHENTICATING") {
await this.#state.response;
return;
}

const beginningState = this.#state;
this.#state = { tag: "AUTHENTICATING" };

this.#state = {
tag: "AUTHENTICATING",
response: this.#doRefresh({ organizationId, beginningState }),
};

await this.#state.response;
return;
}

async #doRefresh({
organizationId,
beginningState,
}: {
organizationId?: string;
beginningState: State;
}): Promise<AuthenticationResponse> {
try {
await withLock(REFRESH_LOCK_NAME, async () => {
return await withLock(REFRESH_LOCK_NAME, async () => {
if (organizationId) {
sessionStorage.setItem(
ORGANIZATION_ID_SESSION_STORAGE_KEY,
Expand Down Expand Up @@ -290,6 +313,7 @@ An authorization_code was supplied for a login which did not originate at the ap
this.#state = { tag: "AUTHENTICATED" };
setSessionData(authenticationResponse, { devMode: this.#devMode });
this.#onRefresh && this.#onRefresh(authenticationResponse);
return authenticationResponse;
});
} catch (error) {
if (
Expand All @@ -300,8 +324,7 @@ An authorization_code was supplied for a login which did not originate at the ap

// preserving the original state so that we can try again next time
this.#state = beginningState;

return;
throw error;
}

if (beginningState.tag !== "INITIAL") {
Expand Down
1 change: 1 addition & 0 deletions src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export class AuthKitError extends Error {}
export class RefreshError extends AuthKitError {}
export class CodeExchangeError extends AuthKitError {}
export class LoginRequiredError extends AuthKitError {
readonly message: string = "No access token available";
}
6 changes: 4 additions & 2 deletions src/utils/authenticate-with-code.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CodeExchangeError } from "../errors";
import { AuthenticationResponseRaw } from "../interfaces";
import { deserializeAuthenticationResponse } from "../serializers";

Expand Down Expand Up @@ -31,7 +32,8 @@ export async function authenticateWithCode(
if (response.ok) {
const data = (await response.json()) as AuthenticationResponseRaw;
return deserializeAuthenticationResponse(data);
} else {
console.log("error", await response.json());
}

const error = (await response.json()) as any;
throw new CodeExchangeError(error.error_description);
}

0 comments on commit ddfca6d

Please sign in to comment.