From 97c0f717d60624d70cfd1a5831562e1cd98c8291 Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Tue, 3 Oct 2023 12:20:34 +1300
Subject: [PATCH 01/11] add tokenRefresher class

---
 spec/unit/oidc/tokenRefresher.spec.ts | 266 ++++++++++++++++++++++++++
 src/http-api/interface.ts             |   4 +
 src/oidc/tokenRefresher.ts            | 135 +++++++++++++
 3 files changed, 405 insertions(+)
 create mode 100644 spec/unit/oidc/tokenRefresher.spec.ts
 create mode 100644 src/oidc/tokenRefresher.ts

diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts
new file mode 100644
index 00000000000..6eb2a0fda72
--- /dev/null
+++ b/spec/unit/oidc/tokenRefresher.spec.ts
@@ -0,0 +1,266 @@
+/**
+ * @jest-environment jsdom
+ */
+
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import fetchMock from "fetch-mock-jest";
+import { IdTokenClaims } from "oidc-client-ts";
+
+import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src";
+import { makeDelegatedAuthConfig } from "../../test-utils/oidc";
+
+describe("TokenRefresher()", () => {
+    const authConfig = {
+        issuer: "https://issuer.org/",
+    };
+    const clientId = "test-client-id";
+    const redirectUri = "https://test.org";
+    const deviceId = "abc123";
+    const idTokenClaims = {
+        exp: Date.now() / 1000 + 100000,
+        aud: clientId,
+        iss: authConfig.issuer,
+        sub: "123",
+        iat: 123,
+    };
+    const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`;
+    class TestClass extends OidcTokenRefresher {
+        public constructor(
+            authConfig: IDelegatedAuthConfig,
+            clientId: string,
+            redirectUri: string,
+            deviceId: string,
+            idTokenClaims: IdTokenClaims,
+        ) {
+            super(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+        }
+
+        public async persistTokens(_tokens: { accessToken: string; refreshToken?: string | undefined }): Promise<void> {
+            // NOOP
+        }
+    }
+
+    const config = makeDelegatedAuthConfig(authConfig.issuer);
+
+    const makeTokenResponse = (accessToken: string, refreshToken?: string) => ({
+        access_token: accessToken,
+        refresh_token: refreshToken,
+        token_type: "Bearer",
+        expires_in: 300,
+        scope: scope,
+    });
+
+    beforeEach(() => {
+        fetchMock.get(`${config.metadata.issuer}.well-known/openid-configuration`, config.metadata);
+        fetchMock.get(`${config.metadata.issuer}jwks`, {
+            status: 200,
+            headers: {
+                "Content-Type": "application/json",
+            },
+            keys: [],
+        });
+
+        fetchMock.post(config.metadata.token_endpoint, {
+            status: 200,
+            headers: {
+                "Content-Type": "application/json",
+            },
+            ...makeTokenResponse("new-access-token", "new-refresh-token"),
+        });
+    });
+
+    afterEach(() => {
+        fetchMock.resetBehavior();
+    });
+
+    it("initialises oidc client", async () => {
+        const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+        await refresher.oidcClientReady;
+
+        // @ts-ignore peek at private property to see we initialised the client correctly
+        expect(refresher.oidcClient.settings).toEqual(
+            expect.objectContaining({
+                client_id: clientId,
+                redirect_uri: redirectUri,
+                authority: authConfig.issuer,
+                scope,
+            }),
+        );
+    });
+
+    describe("doRefreshAccessToken()", () => {
+        it("should throw when oidcClient has not been initialised", async () => {
+            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            await expect(refresher.doRefreshAccessToken("token")).rejects.toThrow(
+                "Cannot get new token before OIDC client is initialised.",
+            );
+        });
+
+        it("should refresh the tokens", async () => {
+            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            await refresher.oidcClientReady;
+
+            const refreshToken = "refresh-token";
+            const result = await refresher.doRefreshAccessToken(refreshToken);
+
+            expect(fetchMock).toHaveFetched(config.metadata.token_endpoint, {
+                method: "POST",
+            });
+
+            expect(result).toEqual({
+                accessToken: "new-access-token",
+                refreshToken: "new-refresh-token",
+            });
+        });
+
+        it("should persist the new tokens", async () => {
+            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            await refresher.oidcClientReady;
+            // spy on our stub
+            jest.spyOn(refresher, "persistTokens");
+
+            const refreshToken = "refresh-token";
+            await refresher.doRefreshAccessToken(refreshToken);
+
+            expect(refresher.persistTokens).toHaveBeenCalledWith({
+                accessToken: "new-access-token",
+                refreshToken: "new-refresh-token",
+            });
+        });
+
+        it("should only have one inflight refresh request at once", async () => {
+            fetchMock
+                .postOnce(
+                    config.metadata.token_endpoint,
+                    {
+                        status: 200,
+                        headers: {
+                            "Content-Type": "application/json",
+                        },
+                        ...makeTokenResponse("first-new-access-token", "first-new-refresh-token"),
+                    },
+                    { overwriteRoutes: true },
+                )
+                .postOnce(
+                    config.metadata.token_endpoint,
+                    {
+                        status: 200,
+                        headers: {
+                            "Content-Type": "application/json",
+                        },
+                        ...makeTokenResponse("second-new-access-token", "second-new-refresh-token"),
+                    },
+                    { overwriteRoutes: false },
+                );
+
+            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            await refresher.oidcClientReady;
+            // reset call counts
+            fetchMock.resetHistory();
+
+            const refreshToken = "refresh-token";
+            const first = refresher.doRefreshAccessToken(refreshToken);
+            const second = refresher.doRefreshAccessToken(refreshToken);
+
+            const result1 = await second;
+            const result2 = await first;
+
+            // only one call to token endpoint
+            expect(fetchMock).toHaveFetchedTimes(1, config.metadata.token_endpoint);
+            expect(result1).toEqual({
+                accessToken: "first-new-access-token",
+                refreshToken: "first-new-refresh-token",
+            });
+            // same response
+            expect(result1).toEqual(result2);
+
+            // call again after first request resolves
+            const third = await refresher.doRefreshAccessToken("first-new-refresh-token");
+
+            // called token endpoint, got new tokens
+            expect(third).toEqual({
+                accessToken: "second-new-access-token",
+                refreshToken: "second-new-refresh-token",
+            });
+        });
+
+        it("should log and rethrow when token refresh fails", async () => {
+            fetchMock.post(
+                config.metadata.token_endpoint,
+                {
+                    status: 503,
+                    headers: {
+                        "Content-Type": "application/json",
+                    },
+                },
+                { overwriteRoutes: true },
+            );
+
+            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            await refresher.oidcClientReady;
+
+            const refreshToken = "refreshToken";
+            await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow();
+        });
+
+        it("should make fresh request after a failed request", async () => {
+            // make sure inflight request is cleared after a failure
+            fetchMock
+                .postOnce(
+                    config.metadata.token_endpoint,
+                    {
+                        status: 503,
+                        headers: {
+                            "Content-Type": "application/json",
+                        },
+                    },
+                    { overwriteRoutes: true },
+                )
+                .postOnce(
+                    config.metadata.token_endpoint,
+                    {
+                        status: 200,
+                        headers: {
+                            "Content-Type": "application/json",
+                        },
+                        ...makeTokenResponse("second-new-access-token", "second-new-refresh-token"),
+                    },
+                    { overwriteRoutes: false },
+                );
+
+            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            await refresher.oidcClientReady;
+            // reset call counts
+            fetchMock.resetHistory();
+
+            const refreshToken = "refresh-token";
+
+            // first call fails
+            await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow();
+
+            // call again after first request resolves
+            const result = await refresher.doRefreshAccessToken("first-new-refresh-token");
+
+            // called token endpoint, got new tokens
+            expect(result).toEqual({
+                accessToken: "second-new-access-token",
+                refreshToken: "second-new-refresh-token",
+            });
+        });
+    });
+});
diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts
index 57e8a18e851..f927f368756 100644
--- a/src/http-api/interface.ts
+++ b/src/http-api/interface.ts
@@ -18,6 +18,10 @@ import { MatrixError } from "./errors";
 
 export type Body = Record<string, any> | BodyInit;
 
+export type TokenRefreshFunction = (refreshToken: string) => Promise<{
+    accessToken: string;
+    refreshToken?: string;
+}>;
 export interface IHttpOpts {
     fetchFn?: typeof global.fetch;
 
diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts
new file mode 100644
index 00000000000..4ca30c1da4b
--- /dev/null
+++ b/src/oidc/tokenRefresher.ts
@@ -0,0 +1,135 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { IdTokenClaims, OidcClient, WebStorageStateStore } from "oidc-client-ts";
+
+import { TokenRefreshFunction } from "../http-api";
+import { IDelegatedAuthConfig } from "../client";
+import { generateScope } from "./authorize";
+import { discoverAndValidateAuthenticationConfig } from "./discovery";
+import { logger } from "../logger";
+
+/**
+ * @experimental
+ * Class responsible to refreshing OIDC access tokens
+ */
+export abstract class OidcTokenRefresher {
+    public readonly oidcClientReady!: Promise<void>;
+    private oidcClient!: OidcClient;
+    private inflightRefreshRequest?: ReturnType<TokenRefreshFunction>;
+
+    public constructor(
+        /**
+         * Delegated auth config as found in matrix client .well-known
+         */
+        authConfig: IDelegatedAuthConfig,
+        /**
+         * id of this client as registered with the OP
+         */
+        clientId: string,
+        /**
+         * redirectUri as registered with OP
+         */
+        redirectUri: string,
+        deviceId: string,
+        /**
+         * idTokenClaims as returned from authorization grant
+         * used to validate tokens
+         */
+        private readonly idTokenClaims: IdTokenClaims,
+    ) {
+        this.oidcClientReady = this.initialiseOidcClient(authConfig, clientId, deviceId, redirectUri);
+    }
+
+    private async initialiseOidcClient(
+        authConfig: IDelegatedAuthConfig,
+        clientId: string,
+        deviceId: string,
+        redirectUri: string,
+    ): Promise<void> {
+        const config = await discoverAndValidateAuthenticationConfig(authConfig);
+
+        const scope = generateScope(deviceId);
+
+        this.oidcClient = new OidcClient({
+            ...config.metadata,
+            client_id: clientId,
+            scope,
+            redirect_uri: redirectUri,
+            authority: config.metadata.issuer,
+            stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }),
+        });
+    }
+
+    /**
+     * Attempt token refresh using given access token
+     * When successful, persist new tokens
+     * @param refreshToken - refresh token to use in request with token issuer
+     * @returns tokens - Promise that resolves with new access and refresh tokens
+     * @throws when token refresh fails
+     */
+    public async doRefreshAccessToken(refreshToken: string): ReturnType<TokenRefreshFunction> {
+        if (!this.inflightRefreshRequest) {
+            this.inflightRefreshRequest = this.getNewToken(refreshToken);
+        }
+        try {
+            const tokens = await this.inflightRefreshRequest;
+            await this.persistTokens(tokens);
+            return tokens;
+        } catch (error) {
+            logger.error("Failed to refresh access token", error);
+            throw error;
+        } finally {
+            this.inflightRefreshRequest = undefined;
+        }
+    }
+
+    /**
+     * Persist the new tokens after successfully refreshing
+     * @param accessToken - new access token
+     * @param refreshToken - OPTIONAL new refresh token
+     */
+    public abstract persistTokens({
+        accessToken,
+        refreshToken,
+    }: {
+        accessToken: string;
+        refreshToken?: string;
+    }): Promise<void>;
+
+    private async getNewToken(refreshToken: string): ReturnType<TokenRefreshFunction> {
+        if (!this.oidcClient) {
+            throw new Error("Cannot get new token before OIDC client is initialised.");
+        }
+
+        const refreshTokenState = {
+            refresh_token: refreshToken,
+            session_state: "test",
+            data: undefined,
+            profile: this.idTokenClaims,
+        };
+
+        const response = await this.oidcClient.useRefreshToken({
+            state: refreshTokenState,
+            timeoutInSeconds: 300,
+        });
+
+        return {
+            accessToken: response.access_token,
+            refreshToken: response.refresh_token,
+        };
+    }
+}

From f499c9edfd5022d55c66c74145ebe93c8842666b Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Tue, 3 Oct 2023 12:20:50 +1300
Subject: [PATCH 02/11] export generateScope

---
 src/oidc/authorize.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/oidc/authorize.ts b/src/oidc/authorize.ts
index df802aa0a4f..0b49da21cab 100644
--- a/src/oidc/authorize.ts
+++ b/src/oidc/authorize.ts
@@ -51,9 +51,9 @@ export type AuthorizationParams = {
  * Generate the scope used in authorization request with OIDC OP
  * @returns scope
  */
-const generateScope = (): string => {
-    const deviceId = randomString(10);
-    return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`;
+export const generateScope = (deviceId?: string): string => {
+    const safeDeviceId = deviceId ?? randomString(10);
+    return `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${safeDeviceId}`;
 };
 
 // https://www.rfc-editor.org/rfc/rfc7636

From 4879f941528af514ca57352718edc0bb633b641c Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Tue, 3 Oct 2023 12:20:59 +1300
Subject: [PATCH 03/11] export oidc from matrix

---
 src/matrix.ts     |  1 +
 src/oidc/index.ts | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 src/oidc/index.ts

diff --git a/src/matrix.ts b/src/matrix.ts
index 26ca8c36b18..20f894a2dc1 100644
--- a/src/matrix.ts
+++ b/src/matrix.ts
@@ -42,6 +42,7 @@ export * from "./models/typed-event-emitter";
 export * from "./models/user";
 export * from "./models/device";
 export * from "./models/search-result";
+export * from "./oidc";
 export * from "./scheduler";
 export * from "./filter";
 export * from "./timeline-window";
diff --git a/src/oidc/index.ts b/src/oidc/index.ts
new file mode 100644
index 00000000000..81ae1833b94
--- /dev/null
+++ b/src/oidc/index.ts
@@ -0,0 +1,17 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+export * from "./tokenRefresher";

From f5168ee39b2c9cd335d556d736f2452aec59ff7a Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Wed, 4 Oct 2023 10:50:49 +1300
Subject: [PATCH 04/11] mark experimental

---
 src/http-api/interface.ts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts
index f927f368756..7d19cac6cdb 100644
--- a/src/http-api/interface.ts
+++ b/src/http-api/interface.ts
@@ -18,6 +18,9 @@ import { MatrixError } from "./errors";
 
 export type Body = Record<string, any> | BodyInit;
 
+/**
+ * @experimental
+ */
 export type TokenRefreshFunction = (refreshToken: string) => Promise<{
     accessToken: string;
     refreshToken?: string;

From 06b00a2298aeb870de3dec7f81e9ce242f241ba7 Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Thu, 5 Oct 2023 09:57:56 +1300
Subject: [PATCH 05/11] Apply suggestions from code review

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 spec/unit/oidc/tokenRefresher.spec.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts
index 6eb2a0fda72..265eccc87c7 100644
--- a/spec/unit/oidc/tokenRefresher.spec.ts
+++ b/spec/unit/oidc/tokenRefresher.spec.ts
@@ -24,7 +24,7 @@ import { IdTokenClaims } from "oidc-client-ts";
 import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src";
 import { makeDelegatedAuthConfig } from "../../test-utils/oidc";
 
-describe("TokenRefresher()", () => {
+describe("OidcTokenRefresher", () => {
     const authConfig = {
         issuer: "https://issuer.org/",
     };

From c0ec8875252ad45d2b94f69087a9d26819cb3d1d Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Thu, 5 Oct 2023 10:03:56 +1300
Subject: [PATCH 06/11] remove some vars in test

---
 spec/unit/oidc/tokenRefresher.spec.ts | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts
index 265eccc87c7..498e08af8f4 100644
--- a/spec/unit/oidc/tokenRefresher.spec.ts
+++ b/spec/unit/oidc/tokenRefresher.spec.ts
@@ -115,8 +115,7 @@ describe("OidcTokenRefresher", () => {
             const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
 
-            const refreshToken = "refresh-token";
-            const result = await refresher.doRefreshAccessToken(refreshToken);
+            const result = await refresher.doRefreshAccessToken("refresh-token");
 
             expect(fetchMock).toHaveFetched(config.metadata.token_endpoint, {
                 method: "POST",
@@ -134,8 +133,7 @@ describe("OidcTokenRefresher", () => {
             // spy on our stub
             jest.spyOn(refresher, "persistTokens");
 
-            const refreshToken = "refresh-token";
-            await refresher.doRefreshAccessToken(refreshToken);
+            await refresher.doRefreshAccessToken("refresh-token");
 
             expect(refresher.persistTokens).toHaveBeenCalledWith({
                 accessToken: "new-access-token",
@@ -214,8 +212,7 @@ describe("OidcTokenRefresher", () => {
             const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
 
-            const refreshToken = "refreshToken";
-            await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow();
+            await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow();
         });
 
         it("should make fresh request after a failed request", async () => {
@@ -248,10 +245,8 @@ describe("OidcTokenRefresher", () => {
             // reset call counts
             fetchMock.resetHistory();
 
-            const refreshToken = "refresh-token";
-
             // first call fails
-            await expect(refresher.doRefreshAccessToken(refreshToken)).rejects.toThrow();
+            await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow();
 
             // call again after first request resolves
             const result = await refresher.doRefreshAccessToken("first-new-refresh-token");

From cc62fac31f9de3fdda126144dfb46e6c71685727 Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Thu, 5 Oct 2023 10:49:20 +1300
Subject: [PATCH 07/11] make TokenRefresher un-abstract, comments and
 improvements

---
 spec/unit/oidc/tokenRefresher.spec.ts | 54 +++++++++--------
 src/http-api/interface.ts             |  8 ++-
 src/oidc/tokenRefresher.ts            | 84 ++++++++++++++++-----------
 3 files changed, 85 insertions(+), 61 deletions(-)

diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts
index 498e08af8f4..85737f1b679 100644
--- a/spec/unit/oidc/tokenRefresher.spec.ts
+++ b/spec/unit/oidc/tokenRefresher.spec.ts
@@ -19,9 +19,9 @@ limitations under the License.
 */
 
 import fetchMock from "fetch-mock-jest";
-import { IdTokenClaims } from "oidc-client-ts";
 
-import { IDelegatedAuthConfig, OidcTokenRefresher } from "../../../src";
+import { OidcTokenRefresher } from "../../../src";
+import { logger } from "../../../src/logger";
 import { makeDelegatedAuthConfig } from "../../test-utils/oidc";
 
 describe("OidcTokenRefresher", () => {
@@ -38,22 +38,8 @@ describe("OidcTokenRefresher", () => {
         sub: "123",
         iat: 123,
     };
+    // used to mock a valid token response, as consumed by OidcClient library
     const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`;
-    class TestClass extends OidcTokenRefresher {
-        public constructor(
-            authConfig: IDelegatedAuthConfig,
-            clientId: string,
-            redirectUri: string,
-            deviceId: string,
-            idTokenClaims: IdTokenClaims,
-        ) {
-            super(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
-        }
-
-        public async persistTokens(_tokens: { accessToken: string; refreshToken?: string | undefined }): Promise<void> {
-            // NOOP
-        }
-    }
 
     const config = makeDelegatedAuthConfig(authConfig.issuer);
 
@@ -85,11 +71,31 @@ describe("OidcTokenRefresher", () => {
     });
 
     afterEach(() => {
+        jest.restoreAllMocks();
         fetchMock.resetBehavior();
     });
 
+    it("throws when oidc client cannot be initialised", async () => {
+        jest.spyOn(logger, "error");
+        fetchMock.get(
+            `${config.metadata.issuer}.well-known/openid-configuration`,
+            {
+                ok: false,
+                status: 404,
+            },
+            { overwriteRoutes: true },
+        );
+        const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+        await expect(refresher.oidcClientReady).rejects.toThrow();
+        expect(logger.error).toHaveBeenCalledWith(
+            "Failed to initialise OIDC client.",
+            // error from OidcClient
+            expect.any(Error),
+        );
+    });
+
     it("initialises oidc client", async () => {
-        const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+        const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
         await refresher.oidcClientReady;
 
         // @ts-ignore peek at private property to see we initialised the client correctly
@@ -105,14 +111,14 @@ describe("OidcTokenRefresher", () => {
 
     describe("doRefreshAccessToken()", () => {
         it("should throw when oidcClient has not been initialised", async () => {
-            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await expect(refresher.doRefreshAccessToken("token")).rejects.toThrow(
                 "Cannot get new token before OIDC client is initialised.",
             );
         });
 
         it("should refresh the tokens", async () => {
-            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
 
             const result = await refresher.doRefreshAccessToken("refresh-token");
@@ -128,7 +134,7 @@ describe("OidcTokenRefresher", () => {
         });
 
         it("should persist the new tokens", async () => {
-            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
             // spy on our stub
             jest.spyOn(refresher, "persistTokens");
@@ -166,7 +172,7 @@ describe("OidcTokenRefresher", () => {
                     { overwriteRoutes: false },
                 );
 
-            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
             // reset call counts
             fetchMock.resetHistory();
@@ -209,7 +215,7 @@ describe("OidcTokenRefresher", () => {
                 { overwriteRoutes: true },
             );
 
-            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
 
             await expect(refresher.doRefreshAccessToken("refresh-token")).rejects.toThrow();
@@ -240,7 +246,7 @@ describe("OidcTokenRefresher", () => {
                     { overwriteRoutes: false },
                 );
 
-            const refresher = new TestClass(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
+            const refresher = new OidcTokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims);
             await refresher.oidcClientReady;
             // reset call counts
             fetchMock.resetHistory();
diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts
index 7d19cac6cdb..e068894f7b1 100644
--- a/src/http-api/interface.ts
+++ b/src/http-api/interface.ts
@@ -21,10 +21,14 @@ export type Body = Record<string, any> | BodyInit;
 /**
  * @experimental
  */
-export type TokenRefreshFunction = (refreshToken: string) => Promise<{
+export type AccessTokens = {
     accessToken: string;
     refreshToken?: string;
-}>;
+};
+/**
+ * @experimental
+ */
+export type TokenRefreshFunction = (refreshToken: string) => Promise<AccessTokens>;
 export interface IHttpOpts {
     fetchFn?: typeof global.fetch;
 
diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts
index 4ca30c1da4b..50cb5653694 100644
--- a/src/oidc/tokenRefresher.ts
+++ b/src/oidc/tokenRefresher.ts
@@ -16,7 +16,7 @@ limitations under the License.
 
 import { IdTokenClaims, OidcClient, WebStorageStateStore } from "oidc-client-ts";
 
-import { TokenRefreshFunction } from "../http-api";
+import { AccessTokens } from "../http-api";
 import { IDelegatedAuthConfig } from "../client";
 import { generateScope } from "./authorize";
 import { discoverAndValidateAuthenticationConfig } from "./discovery";
@@ -24,12 +24,21 @@ import { logger } from "../logger";
 
 /**
  * @experimental
- * Class responsible to refreshing OIDC access tokens
+ * Class responsible for refreshing OIDC access tokens
+ *
+ * Client implementations will likely want to override {@link persistTokens} to persist tokens after successful refresh
+ *
+ * You can then use {@link doRefreshAccessToken} in {@link ICreateClientOpts.tokenRefreshFunction}.
  */
-export abstract class OidcTokenRefresher {
+export class OidcTokenRefresher {
+    /**
+     * Used to determine the OidcClient has been initialised
+     * and is ready to start refreshing tokens
+     * throws when client initialisation fails
+     */
     public readonly oidcClientReady!: Promise<void>;
     private oidcClient!: OidcClient;
-    private inflightRefreshRequest?: ReturnType<TokenRefreshFunction>;
+    private inflightRefreshRequest?: Promise<AccessTokens>;
 
     public constructor(
         /**
@@ -44,6 +53,9 @@ export abstract class OidcTokenRefresher {
          * redirectUri as registered with OP
          */
         redirectUri: string,
+        /**
+         * Device ID of current session
+         */
         deviceId: string,
         /**
          * idTokenClaims as returned from authorization grant
@@ -60,57 +72,55 @@ export abstract class OidcTokenRefresher {
         deviceId: string,
         redirectUri: string,
     ): Promise<void> {
-        const config = await discoverAndValidateAuthenticationConfig(authConfig);
-
-        const scope = generateScope(deviceId);
-
-        this.oidcClient = new OidcClient({
-            ...config.metadata,
-            client_id: clientId,
-            scope,
-            redirect_uri: redirectUri,
-            authority: config.metadata.issuer,
-            stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }),
-        });
+        try {
+            const config = await discoverAndValidateAuthenticationConfig(authConfig);
+
+            const scope = generateScope(deviceId);
+
+            this.oidcClient = new OidcClient({
+                ...config.metadata,
+                client_id: clientId,
+                scope,
+                redirect_uri: redirectUri,
+                authority: config.metadata.issuer,
+                stateStore: new WebStorageStateStore({ prefix: "mx_oidc_", store: window.sessionStorage }),
+            });
+        } catch (error) {
+            logger.error("Failed to initialise OIDC client.", error);
+            throw new Error("Failed to initialise OIDC client.");
+        }
     }
 
     /**
-     * Attempt token refresh using given access token
-     * When successful, persist new tokens
+     * Attempt token refresh using given refresh token
      * @param refreshToken - refresh token to use in request with token issuer
      * @returns tokens - Promise that resolves with new access and refresh tokens
      * @throws when token refresh fails
      */
-    public async doRefreshAccessToken(refreshToken: string): ReturnType<TokenRefreshFunction> {
+    public async doRefreshAccessToken(refreshToken: string): Promise<AccessTokens> {
         if (!this.inflightRefreshRequest) {
-            this.inflightRefreshRequest = this.getNewToken(refreshToken);
+            this.inflightRefreshRequest = this.getNewTokens(refreshToken);
         }
         try {
             const tokens = await this.inflightRefreshRequest;
-            await this.persistTokens(tokens);
             return tokens;
-        } catch (error) {
-            logger.error("Failed to refresh access token", error);
-            throw error;
         } finally {
             this.inflightRefreshRequest = undefined;
         }
     }
 
     /**
-     * Persist the new tokens after successfully refreshing
+     * Persist the new tokens
+     * Called after tokens are successfully refreshed
+     * This function is intended to be overriden by the consumer when persistence is necessary
      * @param accessToken - new access token
      * @param refreshToken - OPTIONAL new refresh token
      */
-    public abstract persistTokens({
-        accessToken,
-        refreshToken,
-    }: {
-        accessToken: string;
-        refreshToken?: string;
-    }): Promise<void>;
-
-    private async getNewToken(refreshToken: string): ReturnType<TokenRefreshFunction> {
+    public async persistTokens(_tokens: { accessToken: string; refreshToken?: string }): Promise<void> {
+        // NOOP
+    }
+
+    private async getNewTokens(refreshToken: string): Promise<AccessTokens> {
         if (!this.oidcClient) {
             throw new Error("Cannot get new token before OIDC client is initialised.");
         }
@@ -127,9 +137,13 @@ export abstract class OidcTokenRefresher {
             timeoutInSeconds: 300,
         });
 
-        return {
+        const tokens = {
             accessToken: response.access_token,
             refreshToken: response.refresh_token,
         };
+
+        await this.persistTokens(tokens);
+
+        return tokens;
     }
 }

From 43070f9e43542fb0058c1e2acf85a294533778a5 Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Thu, 5 Oct 2023 13:51:30 +1300
Subject: [PATCH 08/11] remove invalid jsdoc

---
 src/oidc/tokenRefresher.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts
index 50cb5653694..e1087818dbd 100644
--- a/src/oidc/tokenRefresher.ts
+++ b/src/oidc/tokenRefresher.ts
@@ -28,7 +28,6 @@ import { logger } from "../logger";
  *
  * Client implementations will likely want to override {@link persistTokens} to persist tokens after successful refresh
  *
- * You can then use {@link doRefreshAccessToken} in {@link ICreateClientOpts.tokenRefreshFunction}.
  */
 export class OidcTokenRefresher {
     /**

From 4b1b5ef8a6d223eacc6f6812d6acc7026767b7e3 Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Fri, 6 Oct 2023 10:53:07 +1300
Subject: [PATCH 09/11] Update src/oidc/tokenRefresher.ts

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 src/oidc/tokenRefresher.ts | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts
index e1087818dbd..70c62d49df4 100644
--- a/src/oidc/tokenRefresher.ts
+++ b/src/oidc/tokenRefresher.ts
@@ -31,9 +31,10 @@ import { logger } from "../logger";
  */
 export class OidcTokenRefresher {
     /**
-     * Used to determine the OidcClient has been initialised
-     * and is ready to start refreshing tokens
-     * throws when client initialisation fails
+     * Promise which will complete once the OidcClient has been initialised
+     * and is ready to start refreshing tokens.
+     *
+     * Will reject if the client initialisation fails.
      */
     public readonly oidcClientReady!: Promise<void>;
     private oidcClient!: OidcClient;

From ac1a44bae8c18bacf9dac02aa68171e61ec2d499 Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Fri, 6 Oct 2023 11:00:52 +1300
Subject: [PATCH 10/11] Code review improvements

---
 spec/unit/oidc/tokenRefresher.spec.ts | 3 +++
 src/oidc/tokenRefresher.ts            | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/spec/unit/oidc/tokenRefresher.spec.ts b/spec/unit/oidc/tokenRefresher.spec.ts
index 85737f1b679..803a63d9aac 100644
--- a/spec/unit/oidc/tokenRefresher.spec.ts
+++ b/spec/unit/oidc/tokenRefresher.spec.ts
@@ -25,6 +25,8 @@ import { logger } from "../../../src/logger";
 import { makeDelegatedAuthConfig } from "../../test-utils/oidc";
 
 describe("OidcTokenRefresher", () => {
+    // OidcTokenRefresher props
+    // see class declaration for info
     const authConfig = {
         issuer: "https://issuer.org/",
     };
@@ -41,6 +43,7 @@ describe("OidcTokenRefresher", () => {
     // used to mock a valid token response, as consumed by OidcClient library
     const scope = `openid urn:matrix:org.matrix.msc2967.client:api:* urn:matrix:org.matrix.msc2967.client:device:${deviceId}`;
 
+    // auth config used in mocked calls to OP .well-known
     const config = makeDelegatedAuthConfig(authConfig.issuer);
 
     const makeTokenResponse = (accessToken: string, refreshToken?: string) => ({
diff --git a/src/oidc/tokenRefresher.ts b/src/oidc/tokenRefresher.ts
index 70c62d49df4..10c9bc48ea3 100644
--- a/src/oidc/tokenRefresher.ts
+++ b/src/oidc/tokenRefresher.ts
@@ -110,9 +110,10 @@ export class OidcTokenRefresher {
     }
 
     /**
-     * Persist the new tokens
-     * Called after tokens are successfully refreshed
-     * This function is intended to be overriden by the consumer when persistence is necessary
+     * Persist the new tokens, called after tokens are successfully refreshed.
+     *
+     * This function is intended to be overriden by the consumer when persistence is necessary.
+     *
      * @param accessToken - new access token
      * @param refreshToken - OPTIONAL new refresh token
      */

From 2c2f0fb97c26962233cc292126e6e02941514431 Mon Sep 17 00:00:00 2001
From: Kerry Archibald <kerrya@element.io>
Date: Mon, 9 Oct 2023 10:10:06 +1300
Subject: [PATCH 11/11] document TokenRefreshFunction

---
 src/http-api/interface.ts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts
index e068894f7b1..a9bf7712741 100644
--- a/src/http-api/interface.ts
+++ b/src/http-api/interface.ts
@@ -20,13 +20,17 @@ export type Body = Record<string, any> | BodyInit;
 
 /**
  * @experimental
+ * Unencrypted access and (optional) refresh token
  */
 export type AccessTokens = {
     accessToken: string;
     refreshToken?: string;
 };
+// @TODO(kerrya) add link to IHttpOpts and CreateClientOpts when token refresh is added there
 /**
  * @experimental
+ * Function that performs token refresh using the given refreshToken.
+ * Returns a promise that resolves to the refreshed access and (optional) refresh tokens.
  */
 export type TokenRefreshFunction = (refreshToken: string) => Promise<AccessTokens>;
 export interface IHttpOpts {