From 26079972452fb7855745eabee39ed29e9637d111 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 11 Jul 2023 19:27:52 +1200 Subject: [PATCH 01/56] test persistCredentials without a pickle key --- test/Lifecycle-test.ts | 222 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 test/Lifecycle-test.ts diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts new file mode 100644 index 00000000000..b837d7e54d7 --- /dev/null +++ b/test/Lifecycle-test.ts @@ -0,0 +1,222 @@ +/* +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 { mocked } from "jest-mock"; +import { logger } from "matrix-js-sdk/src/logger"; +import * as MatrixJs from "matrix-js-sdk/src/matrix"; +import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; +import { restoreFromLocalStorage } from "../src/Lifecycle"; +import { MatrixClientPeg } from "../src/MatrixClientPeg"; +import Modal from "../src/Modal"; +import PlatformPeg from "../src/PlatformPeg"; +import * as StorageManager from "../src/utils/StorageManager"; + +import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; + +describe("Lifecycle", () => { + const mockPlatform = mockPlatformPeg({ + getPickleKey: jest.fn(), + }); + + const realLocalStorage = global.localStorage; + + const mockClient = getMockClientWithEventEmitter({ + clearStores: jest.fn(), + getAccountData: jest.fn(), + getUserId: jest.fn(), + getDeviceId: jest.fn(), + isVersionSupported: jest.fn().mockResolvedValue(true), + getCrypto: jest.fn(), + getClientWellKnown: jest.fn(), + }); + + beforeEach(() => { + mockPlatform.getPickleKey.mockResolvedValue(null); + + // stub this + jest.spyOn(MatrixClientPeg, "replaceUsingCreds").mockImplementation(() => {}); + jest.spyOn(MatrixClientPeg, "start").mockResolvedValue(undefined); + + // reset any mocking + global.localStorage = realLocalStorage; + }); + + const initLocalStorageMock = (mockStore: Record = {}): void => { + jest.spyOn(localStorage.__proto__, "getItem") + .mockClear() + .mockImplementation((key: unknown) => mockStore[key as string] ?? null); + jest.spyOn(localStorage.__proto__, "removeItem") + .mockClear() + .mockImplementation((key: unknown) => mockStore[key as string] ?? null); + jest.spyOn(localStorage.__proto__, "setItem").mockClear(); + }; + + const initSessionStorageMock = (mockStore: Record = {}): void => { + jest.spyOn(sessionStorage.__proto__, "getItem") + .mockClear() + .mockImplementation((key: unknown) => mockStore[key as string] ?? null); + jest.spyOn(sessionStorage.__proto__, "removeItem") + .mockClear() + .mockImplementation((key: unknown) => mockStore[key as string] ?? null); + jest.spyOn(sessionStorage.__proto__, "setItem").mockClear(); + }; + + const initIdbMock = (mockStore: Record> = {}): void => { + jest.spyOn(StorageManager, "idbLoad") + .mockClear() + .mockImplementation( + // @ts-ignore mock type + async (table: string, key: string) => mockStore[table]?.[key] ?? null, + ); + jest.spyOn(StorageManager, "idbSave").mockClear().mockResolvedValue(undefined); + }; + + const homeserverUrl = "https://server.org"; + const identityServerUrl = "https://is.org"; + const userId = "@alice:server.org"; + const deviceId = "abc123"; + const accessToken = "test-access-token"; + const localStorageSession = { + mx_hs_url: homeserverUrl, + mx_is_url: identityServerUrl, + mx_user_id: userId, + mx_device_id: deviceId, + }; + const idbStorageSession = { + account: { + mx_access_token: accessToken, + }, + }; + + describe("restoreFromLocalStorage()", () => { + beforeEach(() => { + initLocalStorageMock(); + initSessionStorageMock(); + initIdbMock(); + + jest.clearAllMocks(); + jest.spyOn(logger, "log").mockClear(); + + jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); + + // stub this out + jest.spyOn(Modal, "createDialog").mockReturnValue({ finished: Promise.resolve([true]) }); + }); + + it("should return false when localStorage is not available", async () => { + // @ts-ignore dirty mocking + delete global.localStorage; + // @ts-ignore dirty mocking + global.localStorage = undefined; + + expect(await restoreFromLocalStorage()).toEqual(false); + }); + + it("should return false when no session data is found in local storage", async () => { + expect(await restoreFromLocalStorage()).toEqual(false); + expect(logger.log).toHaveBeenCalledWith("No previous session found."); + }); + + it("should abort login when we expect to find an access token but don't", async () => { + initLocalStorageMock({ mx_has_access_token: "true" }); + + await expect(() => restoreFromLocalStorage()).rejects.toThrow(); + expect(Modal.createDialog).toHaveBeenCalledWith(StorageEvictedDialog); + expect(mockClient.clearStores).toHaveBeenCalled(); + }); + + describe("when session is found in storage", () => { + beforeEach(() => { + initLocalStorageMock(localStorageSession); + initIdbMock(idbStorageSession); + }); + + describe("guest account", () => { + it("should ignore guest accounts when ignoreGuest is true", async () => { + initLocalStorageMock({ ...localStorageSession, mx_is_guest: "true" }); + + expect(await restoreFromLocalStorage({ ignoreGuest: true })).toEqual(false); + expect(logger.log).toHaveBeenCalledWith(`Ignoring stored guest account: ${userId}`); + }); + + it("should restore guest accounts when ignoreGuest is false", async () => { + initLocalStorageMock({ ...localStorageSession, mx_is_guest: "true" }); + + expect(await restoreFromLocalStorage({ ignoreGuest: false })).toEqual(true); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + expect.objectContaining({ + userId, + guest: true, + }), + ); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "true"); + }); + }); + + describe("without a pickle key", () => { + it("should persist credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(localStorage.setItem).toHaveBeenCalledWith("mx_user_id", userId); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true"); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "false"); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId); + + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); + // dont put accessToken in localstorage when we have idb + expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken); + }); + + it("should persist access token when idb is not available", async () => { + jest.spyOn(StorageManager, "idbSave").mockRejectedValue("oups"); + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); + // put accessToken in localstorage as fallback + expect(localStorage.setItem).toHaveBeenCalledWith("mx_access_token", accessToken); + }); + + it("should create new matrix client with credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: undefined, + }); + }); + + it("should remove fresh login flag from session storage", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(sessionStorage.removeItem).toHaveBeenCalledWith("mx_fresh_login"); + }); + + it("should start matrix client", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(MatrixClientPeg.start).toHaveBeenCalled(); + }); + }); + }); + }); +}); From 609f790c469c3d514f8503377b678ca3363768f2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 12 Jul 2023 17:58:57 +1200 Subject: [PATCH 02/56] test setLoggedIn with pickle key --- test/Lifecycle-test.ts | 230 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 228 insertions(+), 2 deletions(-) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index b837d7e54d7..36ca78d04eb 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -17,14 +17,24 @@ limitations under the License. import { mocked } from "jest-mock"; import { logger } from "matrix-js-sdk/src/logger"; import * as MatrixJs from "matrix-js-sdk/src/matrix"; +import { setCrypto } from "matrix-js-sdk/src/crypto/crypto"; +import { Crypto } from "@peculiar/webcrypto"; +import * as MatrixCryptoAes from "matrix-js-sdk/src/crypto/aes"; + import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; -import { restoreFromLocalStorage } from "../src/Lifecycle"; +import { restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; import { MatrixClientPeg } from "../src/MatrixClientPeg"; import Modal from "../src/Modal"; import PlatformPeg from "../src/PlatformPeg"; import * as StorageManager from "../src/utils/StorageManager"; +const webCrypto = new Crypto(); + +const windowCrypto = window.crypto; + import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; +import { subtleCrypto } from "matrix-js-sdk/src/crypto/crypto"; +import BasePlatform from "../src/BasePlatform"; describe("Lifecycle", () => { const mockPlatform = mockPlatformPeg({ @@ -34,6 +44,8 @@ describe("Lifecycle", () => { const realLocalStorage = global.localStorage; const mockClient = getMockClientWithEventEmitter({ + stopClient: jest.fn(), + removeAllListeners: jest.fn(), clearStores: jest.fn(), getAccountData: jest.fn(), getUserId: jest.fn(), @@ -41,6 +53,10 @@ describe("Lifecycle", () => { isVersionSupported: jest.fn().mockResolvedValue(true), getCrypto: jest.fn(), getClientWellKnown: jest.fn(), + getThirdpartyProtocols: jest.fn(), + store: { + destroy: jest.fn(), + }, }); beforeEach(() => { @@ -52,6 +68,21 @@ describe("Lifecycle", () => { // reset any mocking global.localStorage = realLocalStorage; + + setCrypto(webCrypto); + // @ts-ignore mocking + delete window.crypto; + window.crypto = webCrypto; + + jest.spyOn(MatrixCryptoAes, "encryptAES").mockRestore(); + }); + + afterAll(() => { + setCrypto(windowCrypto); + + // @ts-ignore unmocking + delete window.crypto; + window.crypto = windowCrypto; }); const initLocalStorageMock = (mockStore: Record = {}): void => { @@ -72,6 +103,7 @@ describe("Lifecycle", () => { .mockClear() .mockImplementation((key: unknown) => mockStore[key as string] ?? null); jest.spyOn(sessionStorage.__proto__, "setItem").mockClear(); + jest.spyOn(sessionStorage.__proto__, "clear").mockClear(); }; const initIdbMock = (mockStore: Record> = {}): void => { @@ -82,6 +114,7 @@ describe("Lifecycle", () => { async (table: string, key: string) => mockStore[table]?.[key] ?? null, ); jest.spyOn(StorageManager, "idbSave").mockClear().mockResolvedValue(undefined); + jest.spyOn(StorageManager, "idbDelete").mockClear().mockResolvedValue(undefined); }; const homeserverUrl = "https://server.org"; @@ -113,7 +146,10 @@ describe("Lifecycle", () => { jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); // stub this out - jest.spyOn(Modal, "createDialog").mockReturnValue({ finished: Promise.resolve([true]) }); + jest.spyOn(Modal, "createDialog").mockReturnValue( + // @ts-ignore allow bad mock + { finished: Promise.resolve([true]) }, + ); }); it("should return false when localStorage is not available", async () => { @@ -219,4 +255,194 @@ describe("Lifecycle", () => { }); }); }); + + describe("setLoggedIn()", () => { + beforeEach(() => { + initLocalStorageMock(); + initSessionStorageMock(); + initIdbMock(); + + jest.clearAllMocks(); + jest.spyOn(logger, "log").mockClear(); + + jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient); + // remove any mock implementations + jest.spyOn(mockPlatform, "createPickleKey").mockRestore(); + // but still spy and call through + jest.spyOn(mockPlatform, "createPickleKey"); + }); + + const credentials = { + homeserverUrl, + identityServerUrl, + userId, + deviceId, + accessToken, + }; + + it("should remove fresh login flag from session storage", async () => { + await setLoggedIn(credentials); + + expect(sessionStorage.removeItem).toHaveBeenCalledWith("mx_fresh_login"); + }); + + it("should start matrix client", async () => { + await setLoggedIn(credentials); + + expect(MatrixClientPeg.start).toHaveBeenCalled(); + }); + + describe("without a pickle key", () => { + beforeEach(() => { + jest.spyOn(mockPlatform, "createPickleKey").mockResolvedValue(null); + }); + + it("should persist credentials", async () => { + await setLoggedIn(credentials); + + expect(localStorage.setItem).toHaveBeenCalledWith("mx_user_id", userId); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true"); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "false"); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId); + + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); + // dont put accessToken in localstorage when we have idb + expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken); + }); + + it("should remove any access token from storage when there is none in credentials and idb save fails", async () => { + jest.spyOn(StorageManager, "idbSave").mockRejectedValue("oups"); + await setLoggedIn({ + ...credentials, + // @ts-ignore + accessToken: undefined, + }); + + expect(localStorage.removeItem).toHaveBeenCalledWith("mx_has_access_token"); + expect(localStorage.removeItem).toHaveBeenCalledWith("mx_access_token"); + }); + + it("should clear stores", async () => { + await setLoggedIn(credentials); + + expect(StorageManager.idbDelete).toHaveBeenCalledWith("account", "mx_access_token"); + expect(sessionStorage.clear).toHaveBeenCalled(); + expect(mockClient.clearStores).toHaveBeenCalled(); + }); + + it("should create new matrix client with credentials", async () => { + expect(await setLoggedIn(credentials)).toEqual(mockClient); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: null, + }); + }); + }); + + describe("with a pickle key", () => { + it("should not create a pickle key when credentials do not include deviceId", async () => { + await setLoggedIn({ + ...credentials, + deviceId: undefined, + }); + + // unpickled access token saved + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); + expect(mockPlatform.createPickleKey).not.toHaveBeenCalled(); + }); + + it("creates a pickle key with userId and deviceId", async () => { + await setLoggedIn(credentials); + + expect(mockPlatform.createPickleKey).toHaveBeenCalledWith(userId, deviceId); + }); + + it("should persist credentials", async () => { + await setLoggedIn(credentials); + + expect(localStorage.setItem).toHaveBeenCalledWith("mx_user_id", userId); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true"); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "false"); + expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId); + + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_pickle_key", "true"); + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", { + ciphertext: expect.any(String), + iv: expect.any(String), + mac: expect.any(String), + }); + expect(StorageManager.idbSave).toHaveBeenCalledWith( + "pickleKey", + [userId, deviceId], + expect.any(Object), + ); + // dont put accessToken in localstorage when we have idb + expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken); + }); + + it("should persist token when encrypting the token fails", async () => { + jest.spyOn(MatrixCryptoAes, "encryptAES").mockRejectedValue("MOCK REJECT ENCRYPTAES"); + await setLoggedIn(credentials); + + // persist the unencrypted token + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); + }); + + it("should persist token in localStorage when idb fails to save token", async () => { + // dont fail for pickle key persist + jest.spyOn(StorageManager, "idbSave").mockImplementation( + async (table: string, key: string | string[]) => { + if (table === "account" && key === "mx_access_token") { + throw new Error("oups"); + } + }, + ); + await setLoggedIn(credentials); + + // put plain accessToken in localstorage when we dont have idb + expect(localStorage.setItem).toHaveBeenCalledWith("mx_access_token", accessToken); + }); + + it("should remove any access token from storage when there is none in credentials and idb save fails", async () => { + // dont fail for pickle key persist + jest.spyOn(StorageManager, "idbSave").mockImplementation( + async (table: string, key: string | string[]) => { + if (table === "account" && key === "mx_access_token") { + throw new Error("oups"); + } + }, + ); + await setLoggedIn({ + ...credentials, + // @ts-ignore + accessToken: undefined, + }); + + expect(localStorage.removeItem).toHaveBeenCalledWith("mx_has_access_token"); + expect(localStorage.removeItem).toHaveBeenCalledWith("mx_access_token"); + }); + + it("should create new matrix client with credentials", async () => { + expect(await setLoggedIn(credentials)).toEqual(mockClient); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: expect.any(String), + }); + }); + }); + }); }); From f3092c7b5cd42a912b88a49b0e84a174df64c425 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 12 Jul 2023 18:04:28 +1200 Subject: [PATCH 03/56] lint --- test/Lifecycle-test.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 36ca78d04eb..1e3758ca3d1 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -14,28 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { mocked } from "jest-mock"; +import { Crypto } from "@peculiar/webcrypto"; import { logger } from "matrix-js-sdk/src/logger"; import * as MatrixJs from "matrix-js-sdk/src/matrix"; import { setCrypto } from "matrix-js-sdk/src/crypto/crypto"; -import { Crypto } from "@peculiar/webcrypto"; import * as MatrixCryptoAes from "matrix-js-sdk/src/crypto/aes"; import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; import { restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; import { MatrixClientPeg } from "../src/MatrixClientPeg"; import Modal from "../src/Modal"; -import PlatformPeg from "../src/PlatformPeg"; import * as StorageManager from "../src/utils/StorageManager"; +import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; const webCrypto = new Crypto(); const windowCrypto = window.crypto; -import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; -import { subtleCrypto } from "matrix-js-sdk/src/crypto/crypto"; -import BasePlatform from "../src/BasePlatform"; - describe("Lifecycle", () => { const mockPlatform = mockPlatformPeg({ getPickleKey: jest.fn(), From fad7f33a824e3c2c354f7a33a532db7d69e9fa51 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 08:04:56 +1200 Subject: [PATCH 04/56] type error --- test/Lifecycle-test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 1e3758ca3d1..6dbe89527b3 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -62,6 +62,8 @@ describe("Lifecycle", () => { jest.spyOn(MatrixClientPeg, "start").mockResolvedValue(undefined); // reset any mocking + // @ts-ignore mocking + delete global.localStorage; global.localStorage = realLocalStorage; setCrypto(webCrypto); From 32d5fb0fe692f2ac6ac22cb494ae6349ec440bd0 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 10:36:45 +1200 Subject: [PATCH 05/56] extract token persisting code into function, persist refresh token --- src/Lifecycle.ts | 100 ++++++++++++++++++++++++++--------------- src/MatrixClientPeg.ts | 1 + 2 files changed, 66 insertions(+), 35 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 1c5514da776..244fd9ce4a3 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -70,6 +70,14 @@ import { completeOidcLogin } from "./utils/oidc/authorize"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; +const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token"; +const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token"; +/** + * Used during encryption/decryption of token + */ +const ACCESS_TOKEN_NAME = "access_token"; +const REFRESH_TOKEN_NAME = "refresh_token"; + dis.register((payload) => { if (payload.action === Action.TriggerLogout) { // noinspection JSIgnoredPromiseFromCall - we don't care if it fails @@ -452,17 +460,17 @@ export async function getStoredSessionVars(): Promise> { const isUrl = localStorage.getItem(ID_SERVER_URL_KEY) ?? undefined; let accessToken: string | undefined; try { - accessToken = await StorageManager.idbLoad("account", "mx_access_token"); + accessToken = await StorageManager.idbLoad("account", ACCESS_TOKEN_STORAGE_KEY); } catch (e) { logger.error("StorageManager.idbLoad failed for account:mx_access_token", e); } if (!accessToken) { - accessToken = localStorage.getItem("mx_access_token") ?? undefined; + accessToken = localStorage.getItem(ACCESS_TOKEN_STORAGE_KEY) ?? undefined; if (accessToken) { try { // try to migrate access token to IndexedDB if we can - await StorageManager.idbSave("account", "mx_access_token", accessToken); - localStorage.removeItem("mx_access_token"); + await StorageManager.idbSave("account", ACCESS_TOKEN_STORAGE_KEY, accessToken); + localStorage.removeItem(ACCESS_TOKEN_STORAGE_KEY); } catch (e) { logger.error("migration of access token to IndexedDB failed", e); } @@ -556,7 +564,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): logger.log("Got pickle key"); if (typeof accessToken !== "string") { const encrKey = await pickleKeyToAesKey(pickleKey); - decryptedAccessToken = await decryptAES(accessToken, encrKey, "access_token"); + decryptedAccessToken = await decryptAES(accessToken, encrKey, ACCESS_TOKEN_NAME); encrKey.fill(0); } } else { @@ -761,28 +769,26 @@ async function showStorageEvictedDialog(): Promise { // `instanceof`. Babel 7 supports this natively in their class handling. class AbortLoginAndRebuildStorage extends Error {} -async function persistCredentials(credentials: IMatrixClientCreds): Promise { - localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl); - if (credentials.identityServerUrl) { - localStorage.setItem(ID_SERVER_URL_KEY, credentials.identityServerUrl); - } - localStorage.setItem("mx_user_id", credentials.userId); - localStorage.setItem("mx_is_guest", JSON.stringify(credentials.guest)); - - // store whether we expect to find an access token, to detect the case - // where IndexedDB is blown away - if (credentials.accessToken) { - localStorage.setItem("mx_has_access_token", "true"); - } else { - localStorage.removeItem("mx_has_access_token"); - } - - if (credentials.pickleKey) { - let encryptedAccessToken: IEncryptedPayload | undefined; +/** + * Persist a token in storage + * When pickle key is present, will attempt to encrypt the token + * Stores in idb, falling back to localStorage + * + * @param storageKey key used to store the token + * @param name eg "access_token" used as initialization vector during encryption + * @param token + * @param pickleKey optional pickle key used to encrypt token + */ +async function persistTokenInStorage(storageKey: string, name: string, token: string | undefined, pickleKey: IMatrixClientCreds['pickleKey']): Promise { + if (pickleKey) { + let encryptedToken: IEncryptedPayload | undefined; try { + if (!token) { + throw new Error("No token: not attempting encryption") + } // try to encrypt the access token using the pickle key - const encrKey = await pickleKeyToAesKey(credentials.pickleKey); - encryptedAccessToken = await encryptAES(credentials.accessToken, encrKey, "access_token"); + const encrKey = await pickleKeyToAesKey(pickleKey); + encryptedToken = await encryptAES(token, encrKey, name); encrKey.fill(0); } catch (e) { logger.warn("Could not encrypt access token", e); @@ -791,28 +797,52 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise { + localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl); + if (credentials.identityServerUrl) { + localStorage.setItem(ID_SERVER_URL_KEY, credentials.identityServerUrl); + } + localStorage.setItem("mx_user_id", credentials.userId); + localStorage.setItem("mx_is_guest", JSON.stringify(credentials.guest)); + + // store whether we expect to find an access token, to detect the case + // where IndexedDB is blown away + if (credentials.accessToken) { + localStorage.setItem("mx_has_access_token", "true"); + } else { + localStorage.removeItem("mx_has_access_token"); + } + + await persistTokenInStorage(ACCESS_TOKEN_STORAGE_KEY, ACCESS_TOKEN_NAME ,credentials.accessToken, credentials.pickleKey); + await persistTokenInStorage(REFRESH_TOKEN_STORAGE_KEY, REFRESH_TOKEN_NAME, credentials.refreshToken, credentials.pickleKey); + + if (credentials.pickleKey) { + localStorage.setItem("mx_has_pickle_key", String(true)); + } else { if (localStorage.getItem("mx_has_pickle_key") === "true") { logger.error("Expected a pickle key, but none provided. Encryption may not work."); } @@ -1003,7 +1033,7 @@ async function clearStorage(opts?: { deleteEverything?: boolean }): Promise Date: Thu, 13 Jul 2023 11:00:13 +1200 Subject: [PATCH 06/56] store has_refresh_token too --- src/Lifecycle.ts | 22 ++++++++++++---------- test/Lifecycle-test.ts | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 244fd9ce4a3..233945191fc 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -223,7 +223,7 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, homeserverUrl, identityServerUrl } = await completeOidcLogin(queryParams); + const { accessToken, refreshToken, homeserverUrl, identityServerUrl } = await completeOidcLogin(queryParams); const { user_id: userId, @@ -233,6 +233,7 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise const credentials = { accessToken, + refreshToken, homeserverUrl, identityServerUrl, deviceId, @@ -478,7 +479,7 @@ export async function getStoredSessionVars(): Promise> { } // if we pre-date storing "mx_has_access_token", but we retrieved an access // token, then we should say we have an access token - const hasAccessToken = localStorage.getItem("mx_has_access_token") === "true" || !!accessToken; + const hasAccessToken = localStorage.getItem(`mx_has_${ACCESS_TOKEN_NAME}`) === "true" || !!accessToken; const userId = localStorage.getItem("mx_user_id") ?? undefined; const deviceId = localStorage.getItem("mx_device_id") ?? undefined; @@ -780,6 +781,15 @@ class AbortLoginAndRebuildStorage extends Error {} * @param pickleKey optional pickle key used to encrypt token */ async function persistTokenInStorage(storageKey: string, name: string, token: string | undefined, pickleKey: IMatrixClientCreds['pickleKey']): Promise { + const hasTokenStorageKey = `mx_has_${name}`; + // store whether we expect to find a token, to detect the case + // where IndexedDB is blown away + if (token) { + localStorage.setItem(hasTokenStorageKey, "true"); + } else { + localStorage.removeItem(hasTokenStorageKey); + } + if (pickleKey) { let encryptedToken: IEncryptedPayload | undefined; try { @@ -829,14 +839,6 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise { jest.spyOn(mockPlatform, "createPickleKey"); }); + const refreshToken = 'test-refresh-token'; + const credentials = { homeserverUrl, identityServerUrl, @@ -307,6 +309,18 @@ describe("Lifecycle", () => { expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken); }); + it("should persist a refreshToken when present", async () => { + await setLoggedIn({ + ...credentials, + refreshToken + }); + + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", accessToken); + expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_refresh_token", refreshToken); + // dont put accessToken in localstorage when we have idb + expect(localStorage.setItem).not.toHaveBeenCalledWith("mx_access_token", accessToken); + }); + it("should remove any access token from storage when there is none in credentials and idb save fails", async () => { jest.spyOn(StorageManager, "idbSave").mockRejectedValue("oups"); await setLoggedIn({ From 66d57e5f8a48648e6d3238e65fabdc6d0579ca5c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 11:01:55 +1200 Subject: [PATCH 07/56] pass refreshToken from oidcAuthGrant into credentials --- src/utils/oidc/authorize.ts | 2 ++ test/components/structures/MatrixChat-test.tsx | 1 + test/utils/oidc/authorize-test.ts | 1 + 3 files changed, 4 insertions(+) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 705278c63dc..c904d3cf945 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -81,6 +81,7 @@ export const completeOidcLogin = async ( homeserverUrl: string; identityServerUrl?: string; accessToken: string; + refreshToken?: string; }> => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); const { homeserverUrl, tokenResponse, identityServerUrl } = await completeAuthorizationCodeGrant(code, state); @@ -91,5 +92,6 @@ export const completeOidcLogin = async ( homeserverUrl: homeserverUrl, identityServerUrl: identityServerUrl, accessToken: tokenResponse.access_token, + refreshToken: tokenResponse.refresh_token }; }; diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 9b1ea991c7f..8c9435f66e0 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -908,6 +908,7 @@ describe("", () => { expect(localStorageSetSpy).toHaveBeenCalledWith("mx_hs_url", homeserverUrl); expect(localStorageSetSpy).toHaveBeenCalledWith("mx_user_id", userId); expect(localStorageSetSpy).toHaveBeenCalledWith("mx_has_access_token", "true"); + expect(localStorageSetSpy).toHaveBeenCalledWith("mx_has_refresh_token", "true"); expect(localStorageSetSpy).toHaveBeenCalledWith("mx_device_id", deviceId); }); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 7a554562e9b..8953b6a9cd4 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -130,6 +130,7 @@ describe("OIDC authorization", () => { expect(result).toEqual({ accessToken: tokenResponse.access_token, + refreshToken: tokenResponse.refresh_token, homeserverUrl, identityServerUrl, }); From b33e3479fd99406e2a3fc37dddb500b5346324d1 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 13:02:01 +1200 Subject: [PATCH 08/56] rest restore session with pickle key --- test/Lifecycle-test.ts | 131 +++++++++++++++++++++++++++++++++-------- 1 file changed, 108 insertions(+), 23 deletions(-) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 6dbe89527b3..c93c5060d2a 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -32,9 +32,7 @@ const webCrypto = new Crypto(); const windowCrypto = window.crypto; describe("Lifecycle", () => { - const mockPlatform = mockPlatformPeg({ - getPickleKey: jest.fn(), - }); + const mockPlatform = mockPlatformPeg(); const realLocalStorage = global.localStorage; @@ -55,8 +53,6 @@ describe("Lifecycle", () => { }); beforeEach(() => { - mockPlatform.getPickleKey.mockResolvedValue(null); - // stub this jest.spyOn(MatrixClientPeg, "replaceUsingCreds").mockImplementation(() => {}); jest.spyOn(MatrixClientPeg, "start").mockResolvedValue(undefined); @@ -88,8 +84,16 @@ describe("Lifecycle", () => { .mockImplementation((key: unknown) => mockStore[key as string] ?? null); jest.spyOn(localStorage.__proto__, "removeItem") .mockClear() - .mockImplementation((key: unknown) => mockStore[key as string] ?? null); - jest.spyOn(localStorage.__proto__, "setItem").mockClear(); + .mockImplementation((key: unknown) => { + const { [key as string]: toRemove, ...newStore } = mockStore; + mockStore = newStore; + return toRemove; + }); + jest.spyOn(localStorage.__proto__, "setItem") + .mockClear() + .mockImplementation((key: unknown, value: unknown) => { + mockStore[key as string] = value; + }); }; const initSessionStorageMock = (mockStore: Record = {}): void => { @@ -98,8 +102,16 @@ describe("Lifecycle", () => { .mockImplementation((key: unknown) => mockStore[key as string] ?? null); jest.spyOn(sessionStorage.__proto__, "removeItem") .mockClear() - .mockImplementation((key: unknown) => mockStore[key as string] ?? null); - jest.spyOn(sessionStorage.__proto__, "setItem").mockClear(); + .mockImplementation((key: unknown) => { + const { [key as string]: toRemove, ...newStore } = mockStore; + mockStore = newStore; + return toRemove; + }); + jest.spyOn(sessionStorage.__proto__, "setItem") + .mockClear() + .mockImplementation((key: unknown, value: unknown) => { + mockStore[key as string] = value; + }); jest.spyOn(sessionStorage.__proto__, "clear").mockClear(); }; @@ -110,7 +122,16 @@ describe("Lifecycle", () => { // @ts-ignore mock type async (table: string, key: string) => mockStore[table]?.[key] ?? null, ); - jest.spyOn(StorageManager, "idbSave").mockClear().mockResolvedValue(undefined); + jest.spyOn(StorageManager, "idbSave") + .mockClear() + .mockImplementation( + // @ts-ignore mock type + async (tableKey: string, key: string, value: unknown) => { + const table = mockStore[tableKey] || {}; + table[key as string] = value; + mockStore[tableKey] = table; + }, + ); jest.spyOn(StorageManager, "idbDelete").mockClear().mockResolvedValue(undefined); }; @@ -130,6 +151,19 @@ describe("Lifecycle", () => { mx_access_token: accessToken, }, }; + const credentials = { + homeserverUrl, + identityServerUrl, + userId, + deviceId, + accessToken, + }; + + const encryptedTokenShapedObject = { + ciphertext: expect.any(String), + iv: expect.any(String), + mac: expect.any(String), + }; describe("restoreFromLocalStorage()", () => { beforeEach(() => { @@ -250,6 +284,65 @@ describe("Lifecycle", () => { expect(MatrixClientPeg.start).toHaveBeenCalled(); }); }); + + describe("with a pickle key", () => { + beforeEach(async () => { + initLocalStorageMock({}); + initIdbMock({}); + // setup storage with a session with encrypted token + await setLoggedIn(credentials); + }); + + it("should persist credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_access_token", "true"); + + // token encrypted and persisted + expect(StorageManager.idbSave).toHaveBeenCalledWith( + "account", + "mx_access_token", + encryptedTokenShapedObject, + ); + }); + + it("should persist access token when idb is not available", async () => { + // dont fail for pickle key persist + jest.spyOn(StorageManager, "idbSave").mockImplementation( + async (table: string, key: string | string[]) => { + if (table === "account" && key === "mx_access_token") { + throw new Error("oups"); + } + }, + ); + + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(StorageManager.idbSave).toHaveBeenCalledWith( + "account", + "mx_access_token", + encryptedTokenShapedObject, + ); + // put accessToken in localstorage as fallback + expect(localStorage.setItem).toHaveBeenCalledWith("mx_access_token", accessToken); + }); + + it("should create new matrix client with credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ + userId, + // decrypted accessToken + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: expect.any(String), + }); + }); + }); }); }); @@ -269,14 +362,6 @@ describe("Lifecycle", () => { jest.spyOn(mockPlatform, "createPickleKey"); }); - const credentials = { - homeserverUrl, - identityServerUrl, - userId, - deviceId, - accessToken, - }; - it("should remove fresh login flag from session storage", async () => { await setLoggedIn(credentials); @@ -370,11 +455,11 @@ describe("Lifecycle", () => { expect(localStorage.setItem).toHaveBeenCalledWith("mx_device_id", deviceId); expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_pickle_key", "true"); - expect(StorageManager.idbSave).toHaveBeenCalledWith("account", "mx_access_token", { - ciphertext: expect.any(String), - iv: expect.any(String), - mac: expect.any(String), - }); + expect(StorageManager.idbSave).toHaveBeenCalledWith( + "account", + "mx_access_token", + encryptedTokenShapedObject, + ); expect(StorageManager.idbSave).toHaveBeenCalledWith( "pickleKey", [userId, deviceId], From e91bbf488624d3ba78fae8ea4fadd13a2a3cd956 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 13:31:02 +1200 Subject: [PATCH 09/56] retreive stored refresh token and add to credentials --- src/Lifecycle.ts | 66 ++++++++++++++++++++++++--------- test/Lifecycle-test.ts | 84 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 19 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 44b9120be2a..a37e320eee6 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -446,40 +446,57 @@ export interface IStoredSession { isUrl: string; hasAccessToken: boolean; accessToken: string | IEncryptedPayload; + hasRefreshToken: boolean; + refreshToken?: string | IEncryptedPayload; userId: string; deviceId: string; isGuest: boolean; } /** - * Retrieves information about the stored session from the browser's storage. The session - * may not be valid, as it is not tested for consistency here. - * @returns {Object} Information about the session - see implementation for variables. + * Retrieve a token, as stored by `persistCredentials` + * Attempts to migrate token from localStorage to idb + * @param storageKey key used to store the token, eg ACCESS_TOKEN_STORAGE_KEY + * @returns Promise that resolves to token or undefined */ -export async function getStoredSessionVars(): Promise> { - const hsUrl = localStorage.getItem(HOMESERVER_URL_KEY) ?? undefined; - const isUrl = localStorage.getItem(ID_SERVER_URL_KEY) ?? undefined; - let accessToken: string | undefined; +async function getStoredToken(storageKey: string): Promise { + let token: string | undefined; try { - accessToken = await StorageManager.idbLoad("account", ACCESS_TOKEN_STORAGE_KEY); + token = await StorageManager.idbLoad("account", storageKey); } catch (e) { - logger.error("StorageManager.idbLoad failed for account:mx_access_token", e); + logger.error(`StorageManager.idbLoad failed for account:${storageKey}`, e); } - if (!accessToken) { - accessToken = localStorage.getItem(ACCESS_TOKEN_STORAGE_KEY) ?? undefined; - if (accessToken) { + if (!token) { + token = localStorage.getItem(storageKey) ?? undefined; + if (token) { try { // try to migrate access token to IndexedDB if we can - await StorageManager.idbSave("account", ACCESS_TOKEN_STORAGE_KEY, accessToken); - localStorage.removeItem(ACCESS_TOKEN_STORAGE_KEY); + await StorageManager.idbSave("account", storageKey, token); + localStorage.removeItem(storageKey); } catch (e) { - logger.error("migration of access token to IndexedDB failed", e); + logger.error(`migration of token ${storageKey} to IndexedDB failed`, e); } } } + return token; +} + +/** + * Retrieves information about the stored session from the browser's storage. The session + * may not be valid, as it is not tested for consistency here. + * @returns {Object} Information about the session - see implementation for variables. + */ +export async function getStoredSessionVars(): Promise> { + const hsUrl = localStorage.getItem(HOMESERVER_URL_KEY) ?? undefined; + const isUrl = localStorage.getItem(ID_SERVER_URL_KEY) ?? undefined; + + const accessToken = await getStoredToken(ACCESS_TOKEN_STORAGE_KEY); + const refreshToken = await getStoredToken(REFRESH_TOKEN_STORAGE_KEY); + // if we pre-date storing "mx_has_access_token", but we retrieved an access // token, then we should say we have an access token const hasAccessToken = localStorage.getItem(`mx_has_${ACCESS_TOKEN_NAME}`) === "true" || !!accessToken; + const hasRefreshToken = localStorage.getItem(`mx_has_${REFRESH_TOKEN_NAME}`) === "true" || !!refreshToken; const userId = localStorage.getItem("mx_user_id") ?? undefined; const deviceId = localStorage.getItem("mx_device_id") ?? undefined; @@ -491,7 +508,7 @@ export async function getStoredSessionVars(): Promise> { isGuest = localStorage.getItem("matrix-is-guest") === "true"; } - return { hsUrl, isUrl, hasAccessToken, accessToken, userId, deviceId, isGuest }; + return { hsUrl, isUrl, hasAccessToken, accessToken, refreshToken, hasRefreshToken, userId, deviceId, isGuest }; } // The pickle key is a string of unspecified length and format. For AES, we @@ -547,7 +564,8 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): return false; } - const { hsUrl, isUrl, hasAccessToken, accessToken, userId, deviceId, isGuest } = await getStoredSessionVars(); + const { hsUrl, isUrl, hasAccessToken, accessToken, refreshToken, userId, deviceId, isGuest } = + await getStoredSessionVars(); if (hasAccessToken && !accessToken) { await abortLogin(); @@ -559,6 +577,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): return false; } + // @TODO(kerrya) extract this into function DRY let decryptedAccessToken = accessToken; const pickleKey = await PlatformPeg.get()?.getPickleKey(userId, deviceId ?? ""); if (pickleKey) { @@ -572,6 +591,18 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): logger.log("No pickle key available"); } + let decryptedRefreshToken = refreshToken; + if (pickleKey) { + logger.log("Got pickle key"); + if (refreshToken && typeof refreshToken !== "string") { + const encrKey = await pickleKeyToAesKey(pickleKey); + decryptedRefreshToken = await decryptAES(refreshToken, encrKey, REFRESH_TOKEN_NAME); + encrKey.fill(0); + } + } else { + logger.log("No pickle key available"); + } + const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true"; sessionStorage.removeItem("mx_fresh_login"); @@ -581,6 +612,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): userId: userId, deviceId: deviceId, accessToken: decryptedAccessToken as string, + refreshToken: decryptedRefreshToken as string, homeserverUrl: hsUrl, identityServerUrl: isUrl, guest: isGuest, diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 309dab67653..e5b178597dc 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -159,6 +159,8 @@ describe("Lifecycle", () => { accessToken, }; + const refreshToken = "test-refresh-token"; + const encryptedTokenShapedObject = { ciphertext: expect.any(String), iv: expect.any(String), @@ -283,6 +285,45 @@ describe("Lifecycle", () => { expect(MatrixClientPeg.start).toHaveBeenCalled(); }); + + describe("with a refresh token", () => { + beforeEach(() => { + initLocalStorageMock({ + ...localStorageSession, + mx_refresh_token: refreshToken, + }); + initIdbMock(idbStorageSession); + }); + + it("should persist credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + // refresh token from storage is re-persisted + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_refresh_token", "true"); + expect(StorageManager.idbSave).toHaveBeenCalledWith( + "account", + "mx_refresh_token", + refreshToken, + ); + }); + + it("should create new matrix client with credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ + userId, + accessToken, + // refreshToken included in credentials + refreshToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: undefined, + }); + }); + }); }); describe("with a pickle key", () => { @@ -342,6 +383,47 @@ describe("Lifecycle", () => { pickleKey: expect.any(String), }); }); + + describe("with a refresh token", () => { + beforeEach(async () => { + initLocalStorageMock({}); + initIdbMock({}); + // setup storage with a session with encrypted token + await setLoggedIn({ + ...credentials, + refreshToken, + }); + }); + + it("should persist credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + // refresh token from storage is re-persisted + expect(localStorage.setItem).toHaveBeenCalledWith("mx_has_refresh_token", "true"); + expect(StorageManager.idbSave).toHaveBeenCalledWith( + "account", + "mx_refresh_token", + encryptedTokenShapedObject, + ); + }); + + it("should create new matrix client with credentials", async () => { + expect(await restoreFromLocalStorage()).toEqual(true); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ + userId, + accessToken, + // refreshToken included in credentials + refreshToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: expect.any(String), + }); + }); + }); }); }); }); @@ -362,8 +444,6 @@ describe("Lifecycle", () => { jest.spyOn(mockPlatform, "createPickleKey"); }); - const refreshToken = "test-refresh-token"; - it("should remove fresh login flag from session storage", async () => { await setLoggedIn(credentials); From 221d30692940b3981bea8603aceb5af077034db4 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 15:21:46 +1200 Subject: [PATCH 10/56] extract token decryption into function --- src/Lifecycle.ts | 55 +++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index a37e320eee6..d9c0554b102 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -547,6 +547,34 @@ async function abortLogin(): Promise { } } +const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): token is IEncryptedPayload => { + return !!token && typeof token !== "string"; +}; +/** + * Try to decrypt a token retrieved from storage + * @param pickleKey pickle key used during encryption of token, or undefined + * @param token + * @param tokenName token name used during encryption of token eg ACCESS_TOKEN_NAME + * @returns the decrypted token, or the plain text token, returns undefined when token cannot be decrypted + */ +async function tryDecryptToken( + pickleKey: string | undefined, + token: IEncryptedPayload | string | undefined, + tokenName: string, +): Promise { + if (pickleKey && isEncryptedPayload(token)) { + const encrKey = await pickleKeyToAesKey(pickleKey); + const decryptedToken = await decryptAES(token, encrKey, tokenName); + encrKey.fill(0); + return decryptedToken; + } + // if the token wasn't encrypted (plain string) just return it back + if (typeof token === "string") { + return token; + } + // otherwise return undefined +} + // returns a promise which resolves to true if a session is found in // localstorage // @@ -577,31 +605,14 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): return false; } - // @TODO(kerrya) extract this into function DRY - let decryptedAccessToken = accessToken; - const pickleKey = await PlatformPeg.get()?.getPickleKey(userId, deviceId ?? ""); + const pickleKey = (await PlatformPeg.get()?.getPickleKey(userId, deviceId ?? "")) || undefined; if (pickleKey) { logger.log("Got pickle key"); - if (typeof accessToken !== "string") { - const encrKey = await pickleKeyToAesKey(pickleKey); - decryptedAccessToken = await decryptAES(accessToken, encrKey, ACCESS_TOKEN_NAME); - encrKey.fill(0); - } - } else { - logger.log("No pickle key available"); - } - - let decryptedRefreshToken = refreshToken; - if (pickleKey) { - logger.log("Got pickle key"); - if (refreshToken && typeof refreshToken !== "string") { - const encrKey = await pickleKeyToAesKey(pickleKey); - decryptedRefreshToken = await decryptAES(refreshToken, encrKey, REFRESH_TOKEN_NAME); - encrKey.fill(0); - } } else { logger.log("No pickle key available"); } + const decryptedAccessToken = await tryDecryptToken(pickleKey, accessToken, ACCESS_TOKEN_NAME); + const decryptedRefreshToken = await tryDecryptToken(pickleKey, refreshToken, REFRESH_TOKEN_NAME); const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true"; sessionStorage.removeItem("mx_fresh_login"); @@ -611,8 +622,8 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): { userId: userId, deviceId: deviceId, - accessToken: decryptedAccessToken as string, - refreshToken: decryptedRefreshToken as string, + accessToken: decryptedAccessToken!, + refreshToken: decryptedRefreshToken, homeserverUrl: hsUrl, identityServerUrl: isUrl, guest: isGuest, From 64dbc942d106c7d2512eaa984c10c993fd0cea23 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 13 Jul 2023 15:21:52 +1200 Subject: [PATCH 11/56] remove TODO --- src/utils/oidc/authorize.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 83eec2eeff1..a7199a56bb6 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -86,8 +86,6 @@ export const completeOidcLogin = async ( const { code, state } = getCodeAndStateFromQueryParams(queryParams); const { homeserverUrl, tokenResponse, identityServerUrl } = await completeAuthorizationCodeGrant(code, state); - // @TODO(kerrya) do something with the refresh token https://github.com/vector-im/element-web/issues/25444 - return { homeserverUrl: homeserverUrl, identityServerUrl: identityServerUrl, From 1708bef230c5565f3d4739afc56204e3866c033c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 18 Jul 2023 10:49:21 +1200 Subject: [PATCH 12/56] very messy poc --- src/Lifecycle.ts | 17 +++++++++++++---- src/MatrixClientPeg.ts | 12 +++++++++++- src/utils/oidc/authorize.ts | 3 ++- src/utils/oidc/createTokenRefresher.ts | 24 ++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 src/utils/oidc/createTokenRefresher.ts diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index d9c0554b102..311e1d7e1b0 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -223,7 +223,7 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, refreshToken, homeserverUrl, identityServerUrl } = await completeOidcLogin(queryParams); + const { accessToken, refreshToken, homeserverUrl, identityServerUrl, oidcClientSettings } = await completeOidcLogin(queryParams); const { user_id: userId, @@ -242,7 +242,7 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise }; logger.debug("Logged in via OIDC native flow"); - await onSuccessfulDelegatedAuthLogin(credentials); + await onSuccessfulDelegatedAuthLogin(credentials, oidcClientSettings); return true; } catch (error) { logger.error("Failed to login via OIDC", error); @@ -348,9 +348,10 @@ export function attemptTokenLogin( * Clear storage then save new credentials in storage * @param credentials as returned from login */ -async function onSuccessfulDelegatedAuthLogin(credentials: IMatrixClientCreds): Promise { +async function onSuccessfulDelegatedAuthLogin(credentials: IMatrixClientCreds, settings): Promise { await clearStorage(); await persistCredentials(credentials); + await persistOidcClientSettings(settings); // remember that we just logged in sessionStorage.setItem("mx_fresh_login", String(true)); @@ -617,6 +618,9 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true"; sessionStorage.removeItem("mx_fresh_login"); + const oidcClientSettingsRaw = sessionStorage.getItem("oidcClientSettings"); + const oidcClientSettings = oidcClientSettingsRaw ? JSON.parse(oidcClientSettingsRaw) : undefined; + logger.log(`Restoring session for ${userId}`); await doSetLoggedIn( { @@ -629,6 +633,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): guest: isGuest, pickleKey: pickleKey ?? undefined, freshLogin: freshLogin, + oidcClientSettings, }, false, ); @@ -764,7 +769,7 @@ async function doSetLoggedIn(credentials: IMatrixClientCreds, clearStorageEnable await abortLogin(); } - MatrixClientPeg.replaceUsingCreds(credentials); + MatrixClientPeg.replaceUsingCreds(credentials, credentials.oidcClientSettings); const client = MatrixClientPeg.safeGet(); setSentryUser(credentials.userId); @@ -879,6 +884,10 @@ async function persistTokenInStorage( } } +async function persistOidcClientSettings(settings): Promise { + sessionStorage.setItem("oidcClientSettings", JSON.stringify(settings)); +} + async function persistCredentials(credentials: IMatrixClientCreds): Promise { localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl); if (credentials.identityServerUrl) { diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 6a4b8bc48e0..8a471491ba0 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -26,6 +26,7 @@ import { EventTimelineSet } from "matrix-js-sdk/src/models/event-timeline-set"; import { verificationMethods } from "matrix-js-sdk/src/crypto"; import { SHOW_QR_CODE_METHOD } from "matrix-js-sdk/src/crypto/verification/QRCode"; import { logger } from "matrix-js-sdk/src/logger"; +import { OidcTokenRefresher } from "matrix-js-sdk/src/oidc/tokenRefresher"; import createMatrixClient from "./utils/createMatrixClient"; import SettingsStore from "./settings/SettingsStore"; @@ -189,8 +190,17 @@ class MatrixClientPegClass implements IMatrixClientPeg { } } - public replaceUsingCreds(creds: IMatrixClientCreds): void { + public replaceUsingCreds(creds: IMatrixClientCreds, oidcClientSettings): void { + console.log('hhhh replaceUsingcreds', creds, oidcClientSettings); + const tokenRefresher = new OidcTokenRefresher( + creds.refreshToken, + oidcClientSettings, + oidcClientSettings.clientId, + window.location.origin, + creds.deviceId, + ) this.createClient(creds); + this.matrixClient!.http.opts.tokenRefresher = tokenRefresher; } private onUnexpectedStoreClose = async (): Promise => { diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index a7199a56bb6..5b460bea0a3 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -84,12 +84,13 @@ export const completeOidcLogin = async ( refreshToken?: string; }> => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); - const { homeserverUrl, tokenResponse, identityServerUrl } = await completeAuthorizationCodeGrant(code, state); + const { homeserverUrl, tokenResponse, identityServerUrl, oidcClientSettings } = await completeAuthorizationCodeGrant(code, state); return { homeserverUrl: homeserverUrl, identityServerUrl: identityServerUrl, accessToken: tokenResponse.access_token, refreshToken: tokenResponse.refresh_token, + oidcClientSettings }; }; diff --git a/src/utils/oidc/createTokenRefresher.ts b/src/utils/oidc/createTokenRefresher.ts new file mode 100644 index 00000000000..d6b2bb6b5f8 --- /dev/null +++ b/src/utils/oidc/createTokenRefresher.ts @@ -0,0 +1,24 @@ +import { MatrixClient } from "matrix-js-sdk/src/client"; + +class TokenRefresher { + constructor( + private refreshToken: string, + private readonly oidcClientSettings, + ) { + + } + + public async doRefreshAccessToken (setAccessToken: MatrixClient['setAccessToken']) { + + throw new Error("Not implemented"); + } +} + +export const createTokenRefresher = (refreshToken: string) => { + return { + refreshToken, + doRefreshAccessToken: async (setAccessToken: MatrixClient['setAccessToken']) => { + + } + } +} \ No newline at end of file From 2bab36b8165e31cc97dafc5b2e4086cd468f9306 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 10:25:34 +1200 Subject: [PATCH 13/56] utils to persist clientId and issuer after oidc authentication --- src/utils/oidc/persistOidcSettings.ts | 51 +++++++++++++++++ test/utils/oidc/persistOidcSettings-test.ts | 63 +++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 src/utils/oidc/persistOidcSettings.ts create mode 100644 test/utils/oidc/persistOidcSettings-test.ts diff --git a/src/utils/oidc/persistOidcSettings.ts b/src/utils/oidc/persistOidcSettings.ts new file mode 100644 index 00000000000..68f3ffcb168 --- /dev/null +++ b/src/utils/oidc/persistOidcSettings.ts @@ -0,0 +1,51 @@ +/* +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. +*/ + +const clientIdStorageKey = "mx_oidc_client_id"; +const tokenIssuerStorageKey = "mx_oidc_token_issuer"; + +/** + * Persists oidc clientId and issuer in session storage + * Only set after successful authentication + * @param clientId + * @param issuer + */ +export const persistOidcAuthenticatedSettings = (clientId: string, issuer: string): void => { + sessionStorage.setItem(clientIdStorageKey, clientId); + sessionStorage.setItem(tokenIssuerStorageKey, issuer); +}; + +/** + * Retrieve stored oidc issuer from session storage + * When user has token from OIDC issuer, this will be set + * @returns issuer or undefined + */ +export const getStoredOidcTokenIssuer = (): string | undefined => { + return sessionStorage.getItem(tokenIssuerStorageKey) ?? undefined; +}; + +/** + * Retrieves stored oidc client id from session storage + * @returns clientId + * @throws when clientId is not found in session storage + */ +export const getStoredOidcClientId = (): string => { + const clientId = sessionStorage.getItem(clientIdStorageKey); + if (!clientId) { + throw new Error("Oidc client id not found in storage"); + } + return clientId; +}; diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts new file mode 100644 index 00000000000..4db5c4e75c7 --- /dev/null +++ b/test/utils/oidc/persistOidcSettings-test.ts @@ -0,0 +1,63 @@ +/* +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 { + getStoredOidcClientId, + getStoredOidcTokenIssuer, + persistOidcAuthenticatedSettings, +} from "../../../src/utils/oidc/persistOidcSettings"; + +describe("persist OIDC settings", () => { + beforeEach(() => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockClear().mockReturnValue(null); + + jest.spyOn(sessionStorage.__proto__, "setItem").mockClear(); + }); + + const clientId = "test-client-id"; + const issuer = "https://auth.org/"; + + describe("persistOidcAuthenticatedSettings", () => { + it("should set clientId and issuer in session storage", () => { + persistOidcAuthenticatedSettings(clientId, issuer); + expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_client_id", clientId); + expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + }); + }); + + describe("getStoredOidcTokenIssuer()", () => { + it("should return issuer from session storage", () => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(issuer); + expect(getStoredOidcTokenIssuer()).toEqual(issuer); + expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_token_issuer"); + }); + + it("should return undefined when no issuer in session storage", () => { + expect(getStoredOidcTokenIssuer()).toBeUndefined(); + }); + }); + + describe("Name of the group", () => { + it("should return clientId from session storage", () => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(clientId); + expect(getStoredOidcClientId()).toEqual(clientId); + expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_client_id"); + }); + it("should throw when no clientId in session storage", () => { + expect(() => getStoredOidcClientId()).toThrow("Oidc client id not found in storage"); + }); + }); +}); From 66dc9fb74ed83940d3a1051034fc74ccd331d235 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 10:26:27 +1200 Subject: [PATCH 14/56] add dep oidc-client-ts --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 922aa94e864..e7c22c6c817 100644 --- a/package.json +++ b/package.json @@ -101,6 +101,7 @@ "matrix-widget-api": "^1.4.0", "memoize-one": "^6.0.0", "minimist": "^1.2.5", + "oidc-client-ts": "^2.2.4", "opus-recorder": "^8.0.3", "pako": "^2.0.3", "png-chunks-extract": "^1.0.0", From a5c0a517ed2375f7a56d1829164c48c837c0c49f Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 10:33:35 +1200 Subject: [PATCH 15/56] persist issuer and clientId after successful oidc auth --- src/Lifecycle.ts | 9 ++++++++- src/utils/oidc/authorize.ts | 7 ++++++- test/components/structures/MatrixChat-test.tsx | 18 ++++++++++++++++++ test/utils/oidc/authorize-test.ts | 2 ++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 1c5514da776..9e319c7bfef 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -66,6 +66,8 @@ import { OverwriteLoginPayload } from "./dispatcher/payloads/OverwriteLoginPaylo import { SdkContextClass } from "./contexts/SDKContext"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; +import { persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; +import { OidcClientStore } from "./stores/oidc/OidcClientStore"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; @@ -215,7 +217,9 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, homeserverUrl, identityServerUrl } = await completeOidcLogin(queryParams); + const { accessToken, homeserverUrl, identityServerUrl, clientId, issuer } = await completeOidcLogin( + queryParams, + ); const { user_id: userId, @@ -234,6 +238,8 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise logger.debug("Logged in via OIDC native flow"); await onSuccessfulDelegatedAuthLogin(credentials); + // this needs to happen after success handler which clears storages + persistOidcAuthenticatedSettings(clientId, issuer); return true; } catch (error) { logger.error("Failed to login via OIDC", error); @@ -853,6 +859,7 @@ export function logout(): void { _isLoggingOut = true; PlatformPeg.get()?.destroyPickleKey(client.getSafeUserId(), client.getDeviceId() ?? ""); + client.logout(true).then(onLoggedOut, (err) => { // Just throwing an error here is going to be very unhelpful // if you're trying to log out because your server's down and diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 705278c63dc..5a11a7144b1 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -81,9 +81,12 @@ export const completeOidcLogin = async ( homeserverUrl: string; identityServerUrl?: string; accessToken: string; + clientId: string; + issuer: string; }> => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); - const { homeserverUrl, tokenResponse, identityServerUrl } = await completeAuthorizationCodeGrant(code, state); + const { homeserverUrl, tokenResponse, identityServerUrl, oidcClientSettings } = + await completeAuthorizationCodeGrant(code, state); // @TODO(kerrya) do something with the refresh token https://github.com/vector-im/element-web/issues/25444 @@ -91,5 +94,7 @@ export const completeOidcLogin = async ( homeserverUrl: homeserverUrl, identityServerUrl: identityServerUrl, accessToken: tokenResponse.access_token, + clientId: oidcClientSettings.clientId, + issuer: oidcClientSettings.issuer, }; }; diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 9b1ea991c7f..325dfb7a869 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -887,6 +887,15 @@ describe("", () => { expect(loginClient.clearStores).not.toHaveBeenCalled(); }); + + it("should not store clientId or issuer", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(sessionStorageSetSpy).not.toHaveBeenCalledWith("mx_oidc_client_id", clientId); + expect(sessionStorageSetSpy).not.toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + }); }); describe("when login succeeds", () => { @@ -911,6 +920,15 @@ describe("", () => { expect(localStorageSetSpy).toHaveBeenCalledWith("mx_device_id", deviceId); }); + it("should store clientId and issuer in session storage", async () => { + getComponent({ realQueryParams }); + + await flushPromises(); + + expect(sessionStorageSetSpy).toHaveBeenCalledWith("mx_oidc_client_id", clientId); + expect(sessionStorageSetSpy).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + }); + it("should set logged in and start MatrixClient", async () => { getComponent({ realQueryParams }); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 75720f724f8..a531945a26e 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -132,6 +132,8 @@ describe("OIDC authorization", () => { accessToken: tokenResponse.access_token, homeserverUrl, identityServerUrl, + issuer, + clientId, }); }); }); From 50f3fe4e1b624de929358f4e9d20059aec14dd38 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 12:45:56 +1200 Subject: [PATCH 16/56] add OidcClientStore --- src/Lifecycle.ts | 1 - src/contexts/SDKContext.ts | 14 ++ src/stores/oidc/OidcClientStore.ts | 81 ++++++++++++ test/contexts/SdkContext-test.ts | 14 ++ test/stores/oidc/OidcClientStore-test.ts | 155 +++++++++++++++++++++++ 5 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 src/stores/oidc/OidcClientStore.ts create mode 100644 test/stores/oidc/OidcClientStore-test.ts diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 9e319c7bfef..10bff141410 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -67,7 +67,6 @@ import { SdkContextClass } from "./contexts/SDKContext"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; import { persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; -import { OidcClientStore } from "./stores/oidc/OidcClientStore"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; diff --git a/src/contexts/SDKContext.ts b/src/contexts/SDKContext.ts index bb10d0df728..6c64f792928 100644 --- a/src/contexts/SDKContext.ts +++ b/src/contexts/SDKContext.ts @@ -31,6 +31,7 @@ import TypingStore from "../stores/TypingStore"; import { UserProfilesStore } from "../stores/UserProfilesStore"; import { WidgetLayoutStore } from "../stores/widgets/WidgetLayoutStore"; import { WidgetPermissionStore } from "../stores/widgets/WidgetPermissionStore"; +import { OidcClientStore } from "../stores/oidc/OidcClientStore"; import WidgetStore from "../stores/WidgetStore"; import { VoiceBroadcastPlaybacksStore, @@ -80,6 +81,7 @@ export class SdkContextClass { protected _VoiceBroadcastPlaybacksStore?: VoiceBroadcastPlaybacksStore; protected _AccountPasswordStore?: AccountPasswordStore; protected _UserProfilesStore?: UserProfilesStore; + protected _OidcClientStore?: OidcClientStore; /** * Automatically construct stores which need to be created eagerly so they can register with @@ -203,6 +205,18 @@ export class SdkContextClass { return this._UserProfilesStore; } + public get oidcClientStore(): OidcClientStore { + if (!this.client) { + throw new Error("Unable to create OidcClientStore without a client"); + } + + if (!this._OidcClientStore) { + this._OidcClientStore = new OidcClientStore(this.client); + } + + return this._OidcClientStore; + } + public onLoggedOut(): void { this._UserProfilesStore = undefined; } diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts new file mode 100644 index 00000000000..52aa08abe69 --- /dev/null +++ b/src/stores/oidc/OidcClientStore.ts @@ -0,0 +1,81 @@ +/* +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 { IDelegatedAuthConfig, MatrixClient, M_AUTHENTICATION } from "matrix-js-sdk/src/client"; +import { discoverAndValidateAuthenticationConfig } from "matrix-js-sdk/src/oidc/discovery"; +import { logger } from "matrix-js-sdk/src/logger"; +import { OidcClient } from "oidc-client-ts"; + +import { getStoredOidcTokenIssuer, getStoredOidcClientId } from "../../utils/oidc/persistOidcSettings"; + +/** + * @experimental + */ +export class OidcClientStore { + private oidcClient?: OidcClient; + private initialisingOidcClientPromise: Promise | undefined; + private authenticatedIssuer?: string; + private _accountManagementEndpoint?: string; + + public constructor(private readonly matrixClient: MatrixClient) { + this.getOidcClient(); + this.authenticatedIssuer = getStoredOidcTokenIssuer(); + } + + public get isUserAuthenticatedWithOidc(): boolean { + return !!this.authenticatedIssuer; + } + + public get accountManagementEndpoint(): string | undefined { + return this._accountManagementEndpoint; + } + + private async getOidcClient(): Promise { + if (!this.oidcClient && !this.initialisingOidcClientPromise) { + this.initialisingOidcClientPromise = this.initOidcClient(); + } + await this.initialisingOidcClientPromise; + this.initialisingOidcClientPromise = undefined; + return this.oidcClient; + } + + private async initOidcClient(): Promise { + const wellKnown = this.matrixClient.getClientWellKnown(); + if (!wellKnown) { + logger.error("Cannot initialise OidcClientStore: client well known required."); + return; + } + + const delegatedAuthConfig = M_AUTHENTICATION.findIn(wellKnown) ?? undefined; + try { + const clientId = getStoredOidcClientId(); + const { account, metadata, signingKeys } = await discoverAndValidateAuthenticationConfig( + delegatedAuthConfig, + ); + // if no account endpoint is configured default to the issuer + this._accountManagementEndpoint = account ?? metadata.issuer; + this.oidcClient = new OidcClient({ + ...metadata, + authority: metadata.issuer, + signingKeys, + redirect_uri: window.location.origin, + client_id: clientId, + }); + } catch (error) { + logger.error("Failed to initialise OidcClientStore", error); + } + } +} diff --git a/test/contexts/SdkContext-test.ts b/test/contexts/SdkContext-test.ts index a6d1c656245..af14294e416 100644 --- a/test/contexts/SdkContext-test.ts +++ b/test/contexts/SdkContext-test.ts @@ -17,6 +17,7 @@ limitations under the License. import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { SdkContextClass } from "../../src/contexts/SDKContext"; +import { OidcClientStore } from "../../src/stores/oidc/OidcClientStore"; import { UserProfilesStore } from "../../src/stores/UserProfilesStore"; import { VoiceBroadcastPreRecordingStore } from "../../src/voice-broadcast"; import { createTestClient } from "../test-utils"; @@ -52,6 +53,12 @@ describe("SdkContextClass", () => { }).toThrow("Unable to create UserProfilesStore without a client"); }); + it("oidcClientStore should raise an error without a client", () => { + expect(() => { + sdkContext.oidcClientStore; + }).toThrow("Unable to create OidcClientStore without a client"); + }); + describe("when SDKContext has a client", () => { beforeEach(() => { sdkContext.client = client; @@ -69,5 +76,12 @@ describe("SdkContextClass", () => { sdkContext.onLoggedOut(); expect(sdkContext.userProfilesStore).not.toBe(store); }); + + it("oidcClientstore should return a OidcClientStore", () => { + const store = sdkContext.oidcClientStore; + expect(store).toBeInstanceOf(OidcClientStore); + // it should return the same instance + expect(sdkContext.oidcClientStore).toBe(store); + }); }); }); diff --git a/test/stores/oidc/OidcClientStore-test.ts b/test/stores/oidc/OidcClientStore-test.ts new file mode 100644 index 00000000000..b8898438314 --- /dev/null +++ b/test/stores/oidc/OidcClientStore-test.ts @@ -0,0 +1,155 @@ +/* +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 { mocked } from "jest-mock"; +import { M_AUTHENTICATION } from "matrix-js-sdk/src/client"; +import { logger } from "matrix-js-sdk/src/logger"; +import { discoverAndValidateAuthenticationConfig } from "matrix-js-sdk/src/oidc/discovery"; +import { OidcError } from "matrix-js-sdk/src/oidc/error"; + +import { OidcClientStore } from "../../../src/stores/oidc/OidcClientStore"; +import { flushPromises, getMockClientWithEventEmitter } from "../../test-utils"; +import { mockOpenIdConfiguration } from "../../test-utils/oidc"; + +jest.mock("matrix-js-sdk/src/oidc/discovery", () => ({ + discoverAndValidateAuthenticationConfig: jest.fn(), +})); + +describe("OidcClientStore", () => { + const clientId = "test-client-id"; + const metadata = mockOpenIdConfiguration(); + const mockSessionStorage: Record = { + mx_oidc_client_id: clientId, + mx_oidc_token_issuer: metadata.issuer, + }; + + const mockClient = getMockClientWithEventEmitter({ + getClientWellKnown: jest.fn().mockReturnValue({}), + }); + + beforeEach(() => { + jest.spyOn(sessionStorage.__proto__, "getItem") + .mockClear() + .mockImplementation((key) => mockSessionStorage[key as string] ?? null); + mocked(discoverAndValidateAuthenticationConfig).mockClear().mockResolvedValue({ + metadata, + issuer: metadata.issuer, + }); + mockClient.getClientWellKnown.mockReturnValue({ + [M_AUTHENTICATION.stable!]: { + issuer: metadata.issuer, + }, + }); + jest.spyOn(logger, "error").mockClear(); + }); + + describe("isUserAuthenticatedWithOidc()", () => { + it("should return true when an issuer is in session storage", () => { + const store = new OidcClientStore(mockClient); + + expect(store.isUserAuthenticatedWithOidc).toEqual(true); + }); + + it("should return false when no issuer is in session storage", () => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(null); + const store = new OidcClientStore(mockClient); + + expect(store.isUserAuthenticatedWithOidc).toEqual(false); + }); + }); + + describe("initialising oidcClient", () => { + it("should initialise oidc client from constructor", () => { + mockClient.getClientWellKnown.mockReturnValue(undefined); + const store = new OidcClientStore(mockClient); + + // started initialising + // @ts-ignore private property + expect(store.initialisingOidcClientPromise).toBeTruthy(); + }); + + it("should log and return when no client well known is available", async () => { + mockClient.getClientWellKnown.mockReturnValue(undefined); + const store = new OidcClientStore(mockClient); + + expect(logger.error).toHaveBeenCalledWith("Cannot initialise OidcClientStore: client well known required."); + // no oidc client + // @ts-ignore private property + expect(await store.getOidcClient()).toEqual(undefined); + }); + + it("should log and return when no clientId is found in storage", async () => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockClear().mockReturnValue(null); + + const store = new OidcClientStore(mockClient); + + expect(logger.error).toHaveBeenCalledWith( + "Failed to initialise OidcClientStore", + new Error("Oidc client id not found in storage"), + ); + // no oidc client + // @ts-ignore private property + expect(await store.getOidcClient()).toEqual(undefined); + }); + + it("should log and return when discovery and validation fails", async () => { + mocked(discoverAndValidateAuthenticationConfig).mockRejectedValue(new Error(OidcError.OpSupport)); + const store = new OidcClientStore(mockClient); + + await flushPromises(); + + expect(logger.error).toHaveBeenCalledWith( + "Failed to initialise OidcClientStore", + new Error(OidcError.OpSupport), + ); + // no oidc client + // @ts-ignore private property + expect(await store.getOidcClient()).toEqual(undefined); + }); + + it("should create oidc client correctly", async () => { + const store = new OidcClientStore(mockClient); + + // @ts-ignore private property + const client = await store.getOidcClient(); + + expect(client?.settings.client_id).toEqual(clientId); + expect(client?.settings.authority).toEqual(metadata.issuer); + }); + + it("should reuse initialised oidc client", async () => { + const store = new OidcClientStore(mockClient); + + // @ts-ignore private property + store.getOidcClient(); + // @ts-ignore private property + store.getOidcClient(); + + await flushPromises(); + + // finished initialising + // @ts-ignore private property + expect(await store.getOidcClient()).toBeTruthy(); + + // @ts-ignore private property + store.getOidcClient(); + + // only called once for multiple calls to getOidcClient + // before and after initialisation is complete + expect(discoverAndValidateAuthenticationConfig).toHaveBeenCalledTimes(1); + }); + }); +}); From 3681b2ee53cb79b7fe88751d3edd0e35f57a5cc5 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 13:30:46 +1200 Subject: [PATCH 17/56] comments and tidy --- src/stores/oidc/OidcClientStore.ts | 9 ++++++- test/stores/oidc/OidcClientStore-test.ts | 30 +++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 52aa08abe69..9ca96d9b633 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -23,6 +23,7 @@ import { getStoredOidcTokenIssuer, getStoredOidcClientId } from "../../utils/oid /** * @experimental + * Stores information about configured OIDC provider */ export class OidcClientStore { private oidcClient?: OidcClient; @@ -31,10 +32,16 @@ export class OidcClientStore { private _accountManagementEndpoint?: string; public constructor(private readonly matrixClient: MatrixClient) { - this.getOidcClient(); this.authenticatedIssuer = getStoredOidcTokenIssuer(); + // don't bother initialising store when we didnt authenticate via oidc + if (this.authenticatedIssuer) { + this.getOidcClient(); + } } + /** + * True when the active user is authenticated via OIDC + */ public get isUserAuthenticatedWithOidc(): boolean { return !!this.authenticatedIssuer; } diff --git a/test/stores/oidc/OidcClientStore-test.ts b/test/stores/oidc/OidcClientStore-test.ts index b8898438314..9fe8a9d6a8e 100644 --- a/test/stores/oidc/OidcClientStore-test.ts +++ b/test/stores/oidc/OidcClientStore-test.ts @@ -31,6 +31,7 @@ jest.mock("matrix-js-sdk/src/oidc/discovery", () => ({ describe("OidcClientStore", () => { const clientId = "test-client-id"; const metadata = mockOpenIdConfiguration(); + const account = metadata.issuer + "account"; const mockSessionStorage: Record = { mx_oidc_client_id: clientId, mx_oidc_token_issuer: metadata.issuer, @@ -46,11 +47,13 @@ describe("OidcClientStore", () => { .mockImplementation((key) => mockSessionStorage[key as string] ?? null); mocked(discoverAndValidateAuthenticationConfig).mockClear().mockResolvedValue({ metadata, + account, issuer: metadata.issuer, }); mockClient.getClientWellKnown.mockReturnValue({ [M_AUTHENTICATION.stable!]: { issuer: metadata.issuer, + account, }, }); jest.spyOn(logger, "error").mockClear(); @@ -92,7 +95,9 @@ describe("OidcClientStore", () => { }); it("should log and return when no clientId is found in storage", async () => { - jest.spyOn(sessionStorage.__proto__, "getItem").mockClear().mockReturnValue(null); + jest.spyOn(sessionStorage.__proto__, "getItem").mockImplementation((key) => + key === "mx_oidc_token_issuer" ? metadata.issuer : null, + ); const store = new OidcClientStore(mockClient); @@ -130,6 +135,29 @@ describe("OidcClientStore", () => { expect(client?.settings.authority).toEqual(metadata.issuer); }); + it("should set account management endpoint when configured", async () => { + const store = new OidcClientStore(mockClient); + + // @ts-ignore private property + await store.getOidcClient(); + + expect(store.accountManagementEndpoint).toEqual(account); + }); + + it("should set account management endpoint to issuer when not configured", async () => { + mocked(discoverAndValidateAuthenticationConfig).mockClear().mockResolvedValue({ + metadata, + account: undefined, + issuer: metadata.issuer, + }); + const store = new OidcClientStore(mockClient); + + // @ts-ignore private property + await store.getOidcClient(); + + expect(store.accountManagementEndpoint).toEqual(metadata.issuer); + }); + it("should reuse initialised oidc client", async () => { const store = new OidcClientStore(mockClient); From 880671c19385d8e12e57a6f40ab831e124b47149 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 14:00:52 +1200 Subject: [PATCH 18/56] expose getters for stored refresh and access tokens in Lifecycle --- src/Lifecycle.ts | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 50a781cca9f..ef5105af3df 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -224,9 +224,8 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } = await completeOidcLogin( - queryParams, - ); + const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } = + await completeOidcLogin(queryParams); const { user_id: userId, @@ -486,6 +485,22 @@ async function getStoredToken(storageKey: string): Promise { return token; } +/** + * Retrieve access token, as stored by `persistCredentials` + * @returns Promise that resolves to token or undefined + */ +export async function getStoredAccessToken(): Promise { + return getStoredToken(ACCESS_TOKEN_STORAGE_KEY); +} + +/** + * Retrieve refresh token, as stored by `persistCredentials` + * @returns Promise that resolves to token or undefined + */ +export async function getStoredRefreshToken(): Promise { + return getStoredToken(REFRESH_TOKEN_STORAGE_KEY); +} + /** * Retrieves information about the stored session from the browser's storage. The session * may not be valid, as it is not tested for consistency here. @@ -495,8 +510,8 @@ export async function getStoredSessionVars(): Promise> { const hsUrl = localStorage.getItem(HOMESERVER_URL_KEY) ?? undefined; const isUrl = localStorage.getItem(ID_SERVER_URL_KEY) ?? undefined; - const accessToken = await getStoredToken(ACCESS_TOKEN_STORAGE_KEY); - const refreshToken = await getStoredToken(REFRESH_TOKEN_STORAGE_KEY); + const accessToken = await getStoredAccessToken(); + const refreshToken = await getStoredRefreshToken(); // if we pre-date storing "mx_has_access_token", but we retrieved an access // token, then we should say we have an access token From 97a9c89848709f9dacbdcfd149cfb35023e5a3cf Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 17:33:30 +1200 Subject: [PATCH 19/56] revoke tokens with oidc provider --- src/Lifecycle.ts | 57 +++++++++++++++--------- src/components/structures/MatrixChat.tsx | 2 +- src/stores/oidc/OidcClientStore.ts | 15 +++++++ 3 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index ef5105af3df..e01578bc66d 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -67,6 +67,7 @@ import { SdkContextClass } from "./contexts/SDKContext"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; import { persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; +import { OidcClientStore } from "./stores/oidc/OidcClientStore"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; @@ -485,22 +486,6 @@ async function getStoredToken(storageKey: string): Promise { return token; } -/** - * Retrieve access token, as stored by `persistCredentials` - * @returns Promise that resolves to token or undefined - */ -export async function getStoredAccessToken(): Promise { - return getStoredToken(ACCESS_TOKEN_STORAGE_KEY); -} - -/** - * Retrieve refresh token, as stored by `persistCredentials` - * @returns Promise that resolves to token or undefined - */ -export async function getStoredRefreshToken(): Promise { - return getStoredToken(REFRESH_TOKEN_STORAGE_KEY); -} - /** * Retrieves information about the stored session from the browser's storage. The session * may not be valid, as it is not tested for consistency here. @@ -510,8 +495,8 @@ export async function getStoredSessionVars(): Promise> { const hsUrl = localStorage.getItem(HOMESERVER_URL_KEY) ?? undefined; const isUrl = localStorage.getItem(ID_SERVER_URL_KEY) ?? undefined; - const accessToken = await getStoredAccessToken(); - const refreshToken = await getStoredRefreshToken(); + const accessToken = await getStoredToken(ACCESS_TOKEN_STORAGE_KEY); + const refreshToken = await getStoredToken(REFRESH_TOKEN_STORAGE_KEY); // if we pre-date storing "mx_has_access_token", but we retrieved an access // token, then we should say we have an access token @@ -944,10 +929,36 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise { + if (oidcClientStore?.isUserAuthenticatedWithOidc) { + const accessToken = client.http.opts.accessToken; + + // @TODO(kerrya) https://github.com/vector-im/element-web/issues/25444 + // refresh token will be set somewhere better after refreshToken work + // and we can avoid decrypting here + const refreshToken = await getStoredToken(REFRESH_TOKEN_STORAGE_KEY); + const pickleKey = (await PlatformPeg.get()?.getPickleKey(client.getSafeUserId(), client.getDeviceId() ?? "")) || undefined; + const decryptedRefreshToken = await tryDecryptToken(pickleKey, refreshToken, REFRESH_TOKEN_NAME); + if (!accessToken) { + throw new Error("Unexpectedly found no access token"); + } + await oidcClientStore.revokeTokens(accessToken, decryptedRefreshToken!); + } else { + await client.logout(true); + } +} + /** * Logs the current session out and transitions to the logged-out state */ -export function logout(): void { +export function logout(oidcClientStore?: OidcClientStore): void { const client = MatrixClientPeg.get(); if (!client) return; @@ -962,9 +973,11 @@ export function logout(): void { } _isLoggingOut = true; - PlatformPeg.get()?.destroyPickleKey(client.getSafeUserId(), client.getDeviceId() ?? ""); - - client.logout(true).then(onLoggedOut, (err) => { + + doLogout(client, oidcClientStore).then(onLoggedOut, (err) => { + // we need the pickle key to decrypt stored access/refresh tokens to revoke them + // so clear it after logging out + PlatformPeg.get()?.destroyPickleKey(client.getSafeUserId(), client.getDeviceId() ?? ""); // Just throwing an error here is going to be very unhelpful // if you're trying to log out because your server's down and // you want to log into a different server, so just forget the diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index cbe92910eb5..2c03a0f0474 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -602,7 +602,7 @@ export default class MatrixChat extends React.PureComponent { Promise.all([ ...[...CallStore.instance.activeCalls].map((call) => call.disconnect()), cleanUpBroadcasts(this.stores), - ]).finally(() => Lifecycle.logout()); + ]).finally(() => Lifecycle.logout(this.stores.oidcClientStore)); break; case "require_registration": startAnyRegistrationFlow(payload as any); diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 9ca96d9b633..d614e3463c0 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -50,6 +50,21 @@ export class OidcClientStore { return this._accountManagementEndpoint; } + public async revokeTokens(accessToken: string, refreshToken: string): Promise { + try { + const client = await this.getOidcClient(); + + if (!client) { + throw new Error("No OIDC client") + } + + await client.revokeToken(accessToken!, 'access_token'); + await client.revokeToken(refreshToken!, 'refresh_token'); + } catch (error) { + throw error; + } + } + private async getOidcClient(): Promise { if (!this.oidcClient && !this.initialisingOidcClientPromise) { this.initialisingOidcClientPromise = this.initOidcClient(); From b75ad174c18b670f65a5933aa569fdf076cb0fee Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 20 Jul 2023 18:35:53 +1200 Subject: [PATCH 20/56] test logout action in MatrixChat --- .../components/structures/MatrixChat-test.tsx | 126 +++++++++++++++++- 1 file changed, 125 insertions(+), 1 deletion(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 9b1ea991c7f..5e461871c33 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -39,8 +39,16 @@ import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser, + mockPlatformPeg, } from "../../test-utils"; import * as leaveRoomUtils from "../../../src/utils/leave-behaviour"; +import * as voiceBroadcastUtils from "../../../src/voice-broadcast/utils/cleanUpBroadcasts"; +import LegacyCallHandler from "../../../src/LegacyCallHandler"; +import { CallStore } from "../../../src/stores/CallStore"; +import { Call } from "../../../src/models/Call"; +import { PosthogAnalytics } from "../../../src/PosthogAnalytics"; +import PlatformPeg from "../../../src/PlatformPeg"; +import EventIndexPeg from "../../../src/indexing/EventIndexPeg"; jest.mock("matrix-js-sdk/src/oidc/authorize", () => ({ completeAuthorizationCodeGrant: jest.fn(), @@ -97,6 +105,8 @@ describe("", () => { getDehydratedDevice: jest.fn(), whoami: jest.fn(), isRoomEncrypted: jest.fn(), + logout: jest.fn(), + getDeviceId: jest.fn(), }); let mockClient = getMockClientWithEventEmitter(getMockClientMethods()); const serverConfig = { @@ -250,6 +260,10 @@ describe("", () => { }); describe("onAction()", () => { + beforeEach(() => { + jest.spyOn(defaultDispatcher, "dispatch").mockClear(); + jest.spyOn(defaultDispatcher, "fire").mockClear(); + }); it("should open user device settings", async () => { await getComponentAndWaitForReady(); @@ -276,7 +290,6 @@ describe("", () => { mockClient.getRoom.mockImplementation( (id) => [room, spaceRoom].find((room) => room.roomId === id) || null, ); - jest.spyOn(defaultDispatcher, "dispatch").mockClear(); }); describe("leave_room", () => { @@ -388,6 +401,117 @@ describe("", () => { }); }); }); + + describe("logout", () => { + let logoutClient!: ReturnType; + const call1 = { disconnect: jest.fn() } as unknown as Call; + const call2 = { disconnect: jest.fn() } as unknown as Call; + + const dispatchLogoutAndWait = async (): Promise => { + defaultDispatcher.dispatch({ + action: "logout", + }); + + await flushPromises(); + }; + + beforeEach(() => { + // stub out various cleanup functions + jest.spyOn(LegacyCallHandler.instance, "hangupAllCalls") + .mockClear() + .mockImplementation(() => {}); + jest.spyOn(voiceBroadcastUtils, "cleanUpBroadcasts").mockImplementation(async () => {}); + jest.spyOn(PosthogAnalytics.instance, "logout").mockImplementation(() => {}); + jest.spyOn(EventIndexPeg, "deleteEventIndex").mockImplementation(async () => {}); + + jest.spyOn(CallStore.instance, "activeCalls", "get").mockReturnValue(new Set([call1, call2])); + + mockPlatformPeg({ + destroyPickleKey: jest.fn(), + }); + + logoutClient = getMockClientWithEventEmitter(getMockClientMethods()); + mockClient = getMockClientWithEventEmitter(getMockClientMethods()); + mockClient.logout.mockResolvedValue({}); + mockClient.getDeviceId.mockReturnValue(deviceId); + // this is used to create a temporary client to cleanup after logout + jest.spyOn(MatrixJs, "createClient").mockClear().mockReturnValue(logoutClient); + + jest.spyOn(logger, "warn").mockClear(); + }); + + afterAll(() => { + jest.spyOn(voiceBroadcastUtils, "cleanUpBroadcasts").mockRestore(); + }); + + it("should hangup all legacy calls", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + expect(LegacyCallHandler.instance.hangupAllCalls).toHaveBeenCalled(); + }); + + it("should cleanup broadcasts", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + expect(voiceBroadcastUtils.cleanUpBroadcasts).toHaveBeenCalled(); + }); + + it("should disconnect all calls", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + expect(call1.disconnect).toHaveBeenCalled(); + expect(call2.disconnect).toHaveBeenCalled(); + }); + + it("should logout of posthog", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + + expect(PosthogAnalytics.instance.logout).toHaveBeenCalled(); + }); + + it("should destroy pickle key", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + + expect(PlatformPeg.get()!.destroyPickleKey).toHaveBeenCalledWith(userId, deviceId); + }); + + describe("without delegated auth", () => { + it("should call /logout", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + + expect(mockClient.logout).toHaveBeenCalledWith(true); + }); + + it("should warn and do post-logout cleanup anyway when logout fails", async () => { + const error = new Error("test logout failed"); + mockClient.logout.mockRejectedValue(error); + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + + expect(logger.warn).toHaveBeenCalledWith( + "Failed to call logout API: token will not be invalidated", + error, + ); + + // stuff that happens in onloggedout + expect(defaultDispatcher.fire).toHaveBeenCalledWith(Action.OnLoggedOut, true); + expect(logoutClient.clearStores).toHaveBeenCalled(); + }); + + it("should do post-logout cleanup", async () => { + await getComponentAndWaitForReady(); + await dispatchLogoutAndWait(); + + // stuff that happens in onloggedout + expect(defaultDispatcher.fire).toHaveBeenCalledWith(Action.OnLoggedOut, true); + expect(EventIndexPeg.deleteEventIndex).toHaveBeenCalled(); + expect(logoutClient.clearStores).toHaveBeenCalled(); + }); + }); + }); }); }); From 70ddb4aba2f177a48703e9456b6e8155bff96ba6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Jul 2023 09:24:57 +1200 Subject: [PATCH 21/56] comments --- src/Lifecycle.ts | 23 ++++++++++++++++++----- src/utils/oidc/authorize.ts | 23 ++++++++++++----------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 44b9120be2a..c360c7115f2 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -70,13 +70,22 @@ import { completeOidcLogin } from "./utils/oidc/authorize"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; +/** + * Used as storage key + */ const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token"; const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token"; /** - * Used during encryption/decryption of token + * Used as initialization vector during encryption in persistTokenInStorage + * And decryption in restoreFromLocalStorage */ const ACCESS_TOKEN_NAME = "access_token"; const REFRESH_TOKEN_NAME = "refresh_token"; +/** + * Used in localstorage to store whether we expect a token in idb + */ +const HAS_ACCESS_TOKEN_STORAGE_KEY = "mx_has_access_token"; +const HAS_REFRESH_TOKEN_STORAGE_KEY = "mx_has_refresh_token"; dis.register((payload) => { if (payload.action === Action.TriggerLogout) { @@ -479,7 +488,7 @@ export async function getStoredSessionVars(): Promise> { } // if we pre-date storing "mx_has_access_token", but we retrieved an access // token, then we should say we have an access token - const hasAccessToken = localStorage.getItem(`mx_has_${ACCESS_TOKEN_NAME}`) === "true" || !!accessToken; + const hasAccessToken = localStorage.getItem(HAS_ACCESS_TOKEN_STORAGE_KEY) === "true" || !!accessToken; const userId = localStorage.getItem("mx_user_id") ?? undefined; const deviceId = localStorage.getItem("mx_device_id") ?? undefined; @@ -496,7 +505,7 @@ export async function getStoredSessionVars(): Promise> { // The pickle key is a string of unspecified length and format. For AES, we // need a 256-bit Uint8Array. So we HKDF the pickle key to generate the AES -// key. The AES key should be zeroed after it is used. +// key. The AES key should be zeroed after it is used async function pickleKeyToAesKey(pickleKey: string): Promise { const pickleKeyBuffer = new Uint8Array(pickleKey.length); for (let i = 0; i < pickleKey.length; i++) { @@ -777,16 +786,18 @@ class AbortLoginAndRebuildStorage extends Error {} * * @param storageKey key used to store the token * @param name eg "access_token" used as initialization vector during encryption - * @param token + * only used when pickleKey is present to encrypt with + * @param token the token to store, when undefined any existing token at the storageKey is removed from storage * @param pickleKey optional pickle key used to encrypt token + * @param hasTokenStorageKey used to store in localstorage whether we expect to have a token in idb, eg "mx_has_access_token" */ async function persistTokenInStorage( storageKey: string, name: string, token: string | undefined, pickleKey: IMatrixClientCreds["pickleKey"], + hasTokenStorageKey: string, ): Promise { - const hasTokenStorageKey = `mx_has_${name}`; // store whether we expect to find a token, to detect the case // where IndexedDB is blown away if (token) { @@ -849,12 +860,14 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise => { +export const completeOidcLogin = async (queryParams: QueryDict): Promise => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); const { homeserverUrl, tokenResponse, identityServerUrl } = await completeAuthorizationCodeGrant(code, state); - // @TODO(kerrya) do something with the refresh token https://github.com/vector-im/element-web/issues/25444 - return { homeserverUrl: homeserverUrl, identityServerUrl: identityServerUrl, From af481b222f397cf0ecc476f73796871a23606736 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Jul 2023 11:41:10 +1200 Subject: [PATCH 22/56] prettier --- src/Lifecycle.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 31222aeedd5..9b4346796d5 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -233,9 +233,8 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } = await completeOidcLogin( - queryParams, - ); + const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } = + await completeOidcLogin(queryParams); const { user_id: userId, From 07195c04b2e74dc118ac7f41309fdc5a62415765 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Jul 2023 14:02:58 +1200 Subject: [PATCH 23/56] test OidcClientStore.revokeTokens --- src/stores/oidc/OidcClientStore.ts | 53 +++++++++++++++--- test/stores/oidc/OidcClientStore-test.ts | 70 ++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 08cede8d16a..21f401660c6 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -50,18 +50,49 @@ export class OidcClientStore { return this._accountManagementEndpoint; } + /** + * Revokes provided access and refresh tokens with the configured OIDC provider + * @param accessToken + * @param refreshToken + * @returns Promise that resolves when tokens have been revoked + * @throws when OidcClient cannot be initialised, or revoking either token fails + */ public async revokeTokens(accessToken: string, refreshToken: string): Promise { - try { - const client = await this.getOidcClient(); + const client = await this.getOidcClient(); - if (!client) { - throw new Error("No OIDC client"); - } + if (!client) { + throw new Error("No OIDC client"); + } - await client.revokeToken(accessToken!, "access_token"); - await client.revokeToken(refreshToken!, "refresh_token"); + const results = await Promise.all([ + this.tryRevokeToken(client, accessToken, "access_token"), + this.tryRevokeToken(client, refreshToken, "refresh_token"), + ]); + + if (results.some((success) => !success)) { + throw new Error("Failed to revoke tokens"); + } + } + + /** + * Try to revoke a given token + * @param oidcClient + * @param token + * @param tokenType passed to revocation endpoint as token type hint + * @returns Promise that resolved with boolean whether the token revocation succeeded or not + */ + private async tryRevokeToken( + oidcClient: OidcClient, + token: string, + tokenType: "access_token" | "refresh_token", + ): Promise { + try { + console.log("here", tokenType); + await oidcClient.revokeToken(token, tokenType); + return true; } catch (error) { - throw error; + logger.error(`Failed to revoke ${tokenType}`, error); + return false; } } @@ -74,6 +105,12 @@ export class OidcClientStore { return this.oidcClient; } + /** + * Tries to initialise an OidcClient using stored clientId and oidc discovery + * Assigns this.oidcClient and accountManagement endpoint + * Logs errors and does not throw when oidc client cannot be initialised + * @returns promise that resolves when initialising OidcClient succeeds or fails + */ private async initOidcClient(): Promise { const wellKnown = this.matrixClient.getClientWellKnown(); if (!wellKnown) { diff --git a/test/stores/oidc/OidcClientStore-test.ts b/test/stores/oidc/OidcClientStore-test.ts index 9fe8a9d6a8e..b64b7876b8b 100644 --- a/test/stores/oidc/OidcClientStore-test.ts +++ b/test/stores/oidc/OidcClientStore-test.ts @@ -14,7 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ +import fetchMock from "fetch-mock-jest"; import { mocked } from "jest-mock"; +import { OidcClient } from "oidc-client-ts"; import { M_AUTHENTICATION } from "matrix-js-sdk/src/client"; import { logger } from "matrix-js-sdk/src/logger"; import { discoverAndValidateAuthenticationConfig } from "matrix-js-sdk/src/oidc/discovery"; @@ -57,6 +59,8 @@ describe("OidcClientStore", () => { }, }); jest.spyOn(logger, "error").mockClear(); + + fetchMock.get(`${metadata.issuer}.well-known/openid-configuration`, metadata); }); describe("isUserAuthenticatedWithOidc()", () => { @@ -180,4 +184,70 @@ describe("OidcClientStore", () => { expect(discoverAndValidateAuthenticationConfig).toHaveBeenCalledTimes(1); }); }); + + describe("revokeTokens()", () => { + const accessToken = "test-access-token"; + const refreshToken = "test-refresh-token"; + + beforeEach(() => { + // spy and call through + jest.spyOn(OidcClient.prototype, "revokeToken").mockClear(); + + fetchMock.resetHistory(); + fetchMock.post( + metadata.revocation_endpoint, + { + status: 200, + }, + { sendAsJson: true }, + ); + }); + + it("should throw when oidcClient could not be initialised", async () => { + // make oidcClient initialisation fail + mockClient.getClientWellKnown.mockReturnValue(undefined); + + const store = new OidcClientStore(mockClient); + + await expect(() => store.revokeTokens(accessToken, refreshToken)).rejects.toThrow("No OIDC client"); + }); + + it("should revoke access and refresh tokens", async () => { + const store = new OidcClientStore(mockClient); + + await store.revokeTokens(accessToken, refreshToken); + + expect(fetchMock).toHaveFetchedTimes(2, metadata.revocation_endpoint); + expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(accessToken, "access_token"); + expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(refreshToken, "refresh_token"); + }); + + it("should still attempt to revoke refresh token when access token revocation fails", async () => { + // fail once, then succeed + fetchMock + .postOnce( + metadata.revocation_endpoint, + { + status: 404, + }, + { overwriteRoutes: true, sendAsJson: true }, + ) + .post( + metadata.revocation_endpoint, + { + status: 200, + }, + { sendAsJson: true }, + ); + + const store = new OidcClientStore(mockClient); + + await expect(() => store.revokeTokens(accessToken, refreshToken)).rejects.toThrow( + "Failed to revoke tokens", + ); + + expect(fetchMock).toHaveFetchedTimes(2, metadata.revocation_endpoint); + expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(accessToken, "access_token"); + }); + }); }); From 4f1bb4e9edfef4121d229b3360a2048fdc666b64 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Fri, 21 Jul 2023 14:35:29 +1200 Subject: [PATCH 24/56] put pickle key destruction back --- src/Lifecycle.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 1a957c63b16..165870410ab 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -987,11 +987,9 @@ export function logout(oidcClientStore?: OidcClientStore): void { } _isLoggingOut = true; + PlatformPeg.get()?.destroyPickleKey(client.getSafeUserId(), client.getDeviceId() ?? ""); doLogout(client, oidcClientStore).then(onLoggedOut, (err) => { - // we need the pickle key to decrypt stored access/refresh tokens to revoke them - // so clear it after logging out - PlatformPeg.get()?.destroyPickleKey(client.getSafeUserId(), client.getDeviceId() ?? ""); // Just throwing an error here is going to be very unhelpful // if you're trying to log out because your server's down and // you want to log into a different server, so just forget the From 8cd5823865de1766c399aecdc78cf98f0c021083 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 25 Sep 2023 15:33:50 +1300 Subject: [PATCH 25/56] comment pedantry --- src/Lifecycle.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 3e89e4d5562..e734551e2aa 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -609,17 +609,17 @@ const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): tok * Try to decrypt a token retrieved from storage * @param pickleKey pickle key used during encryption of token, or undefined * @param token - * @param tokenName token name used during encryption of token eg ACCESS_TOKEN_IV - * @returns the decrypted token, or the plain text token, returns undefined when token cannot be decrypted + * @param tokenIv initialization vector used during encryption of token eg ACCESS_TOKEN_IV + * @returns the decrypted token, or the plain text token. Returns undefined when token cannot be decrypted */ async function tryDecryptToken( pickleKey: string | undefined, token: IEncryptedPayload | string | undefined, - tokenName: string, + tokenIv: string, ): Promise { if (pickleKey && isEncryptedPayload(token)) { const encrKey = await pickleKeyToAesKey(pickleKey); - const decryptedToken = await decryptAES(token, encrKey, tokenName); + const decryptedToken = await decryptAES(token, encrKey, tokenIv); encrKey.fill(0); return decryptedToken; } From 97fad4d74909a4e574401767dbbf325f8128af43 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 11:17:26 +1300 Subject: [PATCH 26/56] working refresh without persistence --- src/Lifecycle.ts | 37 ++++++++++++++++++++++++++++++++----- src/MatrixClientPeg.ts | 23 +++++++++++++---------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 6e95b0573cf..4400c247b44 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -65,8 +65,9 @@ import { OverwriteLoginPayload } from "./dispatcher/payloads/OverwriteLoginPaylo import { SdkContextClass } from "./contexts/SDKContext"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; -import { persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; +import { getStoredOidcClientId, getStoredOidcTokenIssuer, persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; import GenericToast from "./components/views/toasts/GenericToast"; +import { OidcTokenRefresher } from "matrix-js-sdk/src/oidc/tokenRefresher"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; @@ -675,8 +676,8 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true"; sessionStorage.removeItem("mx_fresh_login"); - const oidcClientSettingsRaw = sessionStorage.getItem("oidcClientSettings"); - const oidcClientSettings = oidcClientSettingsRaw ? JSON.parse(oidcClientSettingsRaw) : undefined; + // const oidcClientSettingsRaw = sessionStorage.getItem("oidcClientSettings"); + // const oidcClientSettings = oidcClientSettingsRaw ? JSON.parse(oidcClientSettingsRaw) : undefined; logger.log(`Restoring session for ${userId}`); await doSetLoggedIn( @@ -690,7 +691,7 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): guest: isGuest, pickleKey: pickleKey ?? undefined, freshLogin: freshLogin, - oidcClientSettings, + // oidcClientSettings, }, false, ); @@ -813,6 +814,30 @@ export async function hydrateSession(credentials: IMatrixClientCreds): Promise { + if (!credentials.refreshToken) { + return; + } + const tokenIssuer = getStoredOidcTokenIssuer(); + if (!tokenIssuer) { + return; + } + try { + const clientId = getStoredOidcClientId(); + // @TODO(kerrya) this should probably come from somewhere + const redirectUri = window.location.origin; + const deviceId = credentials.deviceId; + if (!deviceId) { + throw new Error("Expected deviceId in user credentials.") + } + const tokenRefresher = new OidcTokenRefresher(credentials.refreshToken, + { issuer: tokenIssuer }, clientId, redirectUri, deviceId); + return tokenRefresher; + } catch (error) { + logger.error("Failed to initialise OIDC token refresher", error); + } +} + /** * optionally clears localstorage, persists new credentials * to localstorage, starts the new client. @@ -854,9 +879,11 @@ async function doSetLoggedIn(credentials: IMatrixClientCreds, clearStorageEnable await abortLogin(); } + const tokenRefresher = await checkForOidcTODOName(credentials); + // check the session lock just before creating the new client checkSessionLock(); - MatrixClientPeg.replaceUsingCreds(credentials, credentials.oidcClientSettings); + MatrixClientPeg.replaceUsingCreds({ ...credentials, tokenRefresher }); const client = MatrixClientPeg.safeGet(); setSentryUser(credentials.userId); diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 1df524d7b79..74778e401ca 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -51,6 +51,7 @@ import MatrixClientBackedController from "./settings/controllers/MatrixClientBac import ErrorDialog from "./components/views/dialogs/ErrorDialog"; import PlatformPeg from "./PlatformPeg"; import { formatList } from "./utils/FormattingUtils"; +import { OidcClientStore } from "./stores/oidc/OidcClientStore"; export interface IMatrixClientCreds { homeserverUrl: string; @@ -197,17 +198,19 @@ class MatrixClientPegClass implements IMatrixClientPeg { } } - public replaceUsingCreds(creds: IMatrixClientCreds, oidcClientSettings): void { - console.log('hhhh replaceUsingcreds', creds, oidcClientSettings); - const tokenRefresher = new OidcTokenRefresher( - creds.refreshToken, - oidcClientSettings, - oidcClientSettings.clientId, - window.location.origin, - creds.deviceId, - ) + public replaceUsingCreds(creds: IMatrixClientCreds): void { + console.log('hhhh replaceUsingcreds', creds); this.createClient(creds); - this.matrixClient!.http.opts.tokenRefresher = tokenRefresher; + // const oidcClientStore = new OidcClientStore(this.matrixClient!); + // debugger; + // const tokenRefresher = new OidcTokenRefresher( + // creds.refreshToken, + // oidcClientSettings, + // oidcClientSettings?.clientId, + // window.location.origin, + // creds.deviceId, + // ) + this.matrixClient!.http.opts.tokenRefresher = creds.tokenRefresher; } private onUnexpectedStoreClose = async (): Promise => { From e3673ee71b8f0b0f9ae65dbf2e51e2eb48a1090c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 14:16:57 +1300 Subject: [PATCH 27/56] extract token persistence functions to utils --- src/Lifecycle.ts | 152 +++------------------------------ src/utils/tokens/tokens.ts | 166 +++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 141 deletions(-) create mode 100644 src/utils/tokens/tokens.ts diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 0b34e02409a..e572d51008c 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -20,7 +20,7 @@ limitations under the License. import { ReactNode } from "react"; import { createClient, MatrixClient, SSOAction } from "matrix-js-sdk/src/matrix"; import { InvalidStoreError } from "matrix-js-sdk/src/errors"; -import { decryptAES, encryptAES, IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; +import { IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; import { QueryDict } from "matrix-js-sdk/src/utils"; import { logger } from "matrix-js-sdk/src/logger"; import { MINIMUM_MATRIX_VERSION } from "matrix-js-sdk/src/version-support"; @@ -67,27 +67,20 @@ import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; import { persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; import GenericToast from "./components/views/toasts/GenericToast"; +import { + ACCESS_TOKEN_IV, + ACCESS_TOKEN_STORAGE_KEY, + HAS_ACCESS_TOKEN_STORAGE_KEY, + HAS_REFRESH_TOKEN_STORAGE_KEY, + persistTokenInStorage, + REFRESH_TOKEN_IV, + REFRESH_TOKEN_STORAGE_KEY, + tryDecryptToken, +} from "./utils/tokens/tokens"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; -/* - * Keys used when storing the tokens in indexeddb or localstorage - */ -const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token"; -const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token"; -/* - * Used as initialization vector during encryption in persistTokenInStorage - * And decryption in restoreFromLocalStorage - */ -const ACCESS_TOKEN_IV = "access_token"; -const REFRESH_TOKEN_IV = "refresh_token"; -/* - * Keys for localstorage items which indicate whether we expect a token in indexeddb. - */ -const HAS_ACCESS_TOKEN_STORAGE_KEY = "mx_has_access_token"; -const HAS_REFRESH_TOKEN_STORAGE_KEY = "mx_has_refresh_token"; - dis.register((payload) => { if (payload.action === Action.TriggerLogout) { // noinspection JSIgnoredPromiseFromCall - we don't care if it fails @@ -566,32 +559,6 @@ export async function getStoredSessionVars(): Promise> { return { hsUrl, isUrl, hasAccessToken, accessToken, refreshToken, hasRefreshToken, userId, deviceId, isGuest }; } -// The pickle key is a string of unspecified length and format. For AES, we -// need a 256-bit Uint8Array. So we HKDF the pickle key to generate the AES -// key. The AES key should be zeroed after it is used. -async function pickleKeyToAesKey(pickleKey: string): Promise { - const pickleKeyBuffer = new Uint8Array(pickleKey.length); - for (let i = 0; i < pickleKey.length; i++) { - pickleKeyBuffer[i] = pickleKey.charCodeAt(i); - } - const hkdfKey = await window.crypto.subtle.importKey("raw", pickleKeyBuffer, "HKDF", false, ["deriveBits"]); - pickleKeyBuffer.fill(0); - return new Uint8Array( - await window.crypto.subtle.deriveBits( - { - name: "HKDF", - hash: "SHA-256", - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore: https://github.com/microsoft/TypeScript-DOM-lib-generator/pull/879 - salt: new Uint8Array(32), - info: new Uint8Array(0), - }, - hkdfKey, - 256, - ), - ); -} - async function abortLogin(): Promise { const signOut = await showStorageEvictedDialog(); if (signOut) { @@ -602,36 +569,6 @@ async function abortLogin(): Promise { } } -const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): token is IEncryptedPayload => { - return !!token && typeof token !== "string"; -}; -/** - * Try to decrypt a token retrieved from storage - * Where token is not encrypted (plain text) returns the plain text token - * Where token is encrypted, attempts decryption. Returns successfully decrypted token, else undefined. - * @param pickleKey pickle key used during encryption of token, or undefined - * @param token - * @param tokenIv initialization vector used during encryption of token eg ACCESS_TOKEN_IV - * @returns the decrypted token, or the plain text token. Returns undefined when token cannot be decrypted - */ -async function tryDecryptToken( - pickleKey: string | undefined, - token: IEncryptedPayload | string | undefined, - tokenIv: string, -): Promise { - if (pickleKey && isEncryptedPayload(token)) { - const encrKey = await pickleKeyToAesKey(pickleKey); - const decryptedToken = await decryptAES(token, encrKey, tokenIv); - encrKey.fill(0); - return decryptedToken; - } - // if the token wasn't encrypted (plain string) just return it back - if (typeof token === "string") { - return token; - } - // otherwise return undefined -} - // returns a promise which resolves to true if a session is found in // localstorage // @@ -901,73 +838,6 @@ async function showStorageEvictedDialog(): Promise { // `instanceof`. Babel 7 supports this natively in their class handling. class AbortLoginAndRebuildStorage extends Error {} -/** - * Persist a token in storage - * When pickle key is present, will attempt to encrypt the token - * Stores in idb, falling back to localStorage - * - * @param storageKey key used to store the token - * @param initializationVector Initialization vector for encrypting the token. Only used when `pickleKey` is present - * @param token the token to store, when undefined any existing token at the storageKey is removed from storage - * @param pickleKey optional pickle key used to encrypt token - * @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb, eg "mx_has_access_token". - */ -async function persistTokenInStorage( - storageKey: string, - initializationVector: string, - token: string | undefined, - pickleKey: string | undefined, - hasTokenStorageKey: string, -): Promise { - // store whether we expect to find a token, to detect the case - // where IndexedDB is blown away - if (token) { - localStorage.setItem(hasTokenStorageKey, "true"); - } else { - localStorage.removeItem(hasTokenStorageKey); - } - - if (pickleKey) { - let encryptedToken: IEncryptedPayload | undefined; - try { - if (!token) { - throw new Error("No token: not attempting encryption"); - } - // try to encrypt the access token using the pickle key - const encrKey = await pickleKeyToAesKey(pickleKey); - encryptedToken = await encryptAES(token, encrKey, initializationVector); - encrKey.fill(0); - } catch (e) { - logger.warn("Could not encrypt access token", e); - } - try { - // save either the encrypted access token, or the plain access - // token if we were unable to encrypt (e.g. if the browser doesn't - // have WebCrypto). - await StorageManager.idbSave("account", storageKey, encryptedToken || token); - } catch (e) { - // if we couldn't save to indexedDB, fall back to localStorage. We - // store the access token unencrypted since localStorage only saves - // strings. - if (!!token) { - localStorage.setItem(storageKey, token); - } else { - localStorage.removeItem(storageKey); - } - } - } else { - try { - await StorageManager.idbSave("account", storageKey, token); - } catch (e) { - if (!!token) { - localStorage.setItem(storageKey, token); - } else { - localStorage.removeItem(storageKey); - } - } - } -} - async function persistCredentials(credentials: IMatrixClientCreds): Promise { localStorage.setItem(HOMESERVER_URL_KEY, credentials.homeserverUrl); if (credentials.identityServerUrl) { diff --git a/src/utils/tokens/tokens.ts b/src/utils/tokens/tokens.ts new file mode 100644 index 00000000000..fe1d05e7729 --- /dev/null +++ b/src/utils/tokens/tokens.ts @@ -0,0 +1,166 @@ +/* +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 { decryptAES, encryptAES, IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; +import { logger } from "matrix-js-sdk/src/logger"; + +import * as StorageManager from "../StorageManager"; + +/** + * Utility functions related to the storage and retrieval of access tokens + */ + +/* + * Keys used when storing the tokens in indexeddb or localstorage + */ +export const ACCESS_TOKEN_STORAGE_KEY = "mx_access_token"; +export const REFRESH_TOKEN_STORAGE_KEY = "mx_refresh_token"; +/* + * Used as initialization vector during encryption in persistTokenInStorage + * And decryption in restoreFromLocalStorage + */ +export const ACCESS_TOKEN_IV = "access_token"; +export const REFRESH_TOKEN_IV = "refresh_token"; +/* + * Keys for localstorage items which indicate whether we expect a token in indexeddb. + */ +export const HAS_ACCESS_TOKEN_STORAGE_KEY = "mx_has_access_token"; +export const HAS_REFRESH_TOKEN_STORAGE_KEY = "mx_has_refresh_token"; + +/** + * The pickle key is a string of unspecified length and format. For AES, we need a 256-bit Uint8Array. So we HKDF the pickle key to generate the AES key. The AES key should be zeroed after it is used. + * @param pickleKey + * @returns AES key + */ +async function pickleKeyToAesKey(pickleKey: string): Promise { + const pickleKeyBuffer = new Uint8Array(pickleKey.length); + for (let i = 0; i < pickleKey.length; i++) { + pickleKeyBuffer[i] = pickleKey.charCodeAt(i); + } + const hkdfKey = await window.crypto.subtle.importKey("raw", pickleKeyBuffer, "HKDF", false, ["deriveBits"]); + pickleKeyBuffer.fill(0); + return new Uint8Array( + await window.crypto.subtle.deriveBits( + { + name: "HKDF", + hash: "SHA-256", + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore: https://github.com/microsoft/TypeScript-DOM-lib-generator/pull/879 + salt: new Uint8Array(32), + info: new Uint8Array(0), + }, + hkdfKey, + 256, + ), + ); +} + +const isEncryptedPayload = (token?: IEncryptedPayload | string | undefined): token is IEncryptedPayload => { + return !!token && typeof token !== "string"; +}; +/** + * Try to decrypt a token retrieved from storage + * Where token is not encrypted (plain text) returns the plain text token + * Where token is encrypted, attempts decryption. Returns successfully decrypted token, else undefined. + * @param pickleKey pickle key used during encryption of token, or undefined + * @param token + * @param tokenIv initialization vector used during encryption of token eg ACCESS_TOKEN_IV + * @returns the decrypted token, or the plain text token. Returns undefined when token cannot be decrypted + */ +export async function tryDecryptToken( + pickleKey: string | undefined, + token: IEncryptedPayload | string | undefined, + tokenIv: string, +): Promise { + if (pickleKey && isEncryptedPayload(token)) { + const encrKey = await pickleKeyToAesKey(pickleKey); + const decryptedToken = await decryptAES(token, encrKey, tokenIv); + encrKey.fill(0); + return decryptedToken; + } + // if the token wasn't encrypted (plain string) just return it back + if (typeof token === "string") { + return token; + } + // otherwise return undefined +} + +/** + * Persist a token in storage + * When pickle key is present, will attempt to encrypt the token + * Stores in idb, falling back to localStorage + * + * @param storageKey key used to store the token + * @param initializationVector Initialization vector for encrypting the token. Only used when `pickleKey` is present + * @param token the token to store, when undefined any existing token at the storageKey is removed from storage + * @param pickleKey optional pickle key used to encrypt token + * @param hasTokenStorageKey Localstorage key for an item which stores whether we expect to have a token in indexeddb, eg "mx_has_access_token". + */ +export async function persistTokenInStorage( + storageKey: string, + initializationVector: string, + token: string | undefined, + pickleKey: string | undefined, + hasTokenStorageKey: string, +): Promise { + // store whether we expect to find a token, to detect the case + // where IndexedDB is blown away + if (token) { + localStorage.setItem(hasTokenStorageKey, "true"); + } else { + localStorage.removeItem(hasTokenStorageKey); + } + + if (pickleKey) { + let encryptedToken: IEncryptedPayload | undefined; + try { + if (!token) { + throw new Error("No token: not attempting encryption"); + } + // try to encrypt the access token using the pickle key + const encrKey = await pickleKeyToAesKey(pickleKey); + encryptedToken = await encryptAES(token, encrKey, initializationVector); + encrKey.fill(0); + } catch (e) { + logger.warn("Could not encrypt access token", e); + } + try { + // save either the encrypted access token, or the plain access + // token if we were unable to encrypt (e.g. if the browser doesn't + // have WebCrypto). + await StorageManager.idbSave("account", storageKey, encryptedToken || token); + } catch (e) { + // if we couldn't save to indexedDB, fall back to localStorage. We + // store the access token unencrypted since localStorage only saves + // strings. + if (!!token) { + localStorage.setItem(storageKey, token); + } else { + localStorage.removeItem(storageKey); + } + } + } else { + try { + await StorageManager.idbSave("account", storageKey, token); + } catch (e) { + if (!!token) { + localStorage.setItem(storageKey, token); + } else { + localStorage.removeItem(storageKey); + } + } + } +} From 1c8e8cb3cb4b379bc86008c3a1426cae43489573 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 14:51:04 +1300 Subject: [PATCH 28/56] add sugar --- src/Lifecycle.ts | 19 ++++--------------- src/utils/tokens/tokens.ts | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index e572d51008c..bfef686ea8d 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -72,7 +72,8 @@ import { ACCESS_TOKEN_STORAGE_KEY, HAS_ACCESS_TOKEN_STORAGE_KEY, HAS_REFRESH_TOKEN_STORAGE_KEY, - persistTokenInStorage, + persistAccessTokenInStorage, + persistRefreshTokenInStorage, REFRESH_TOKEN_IV, REFRESH_TOKEN_STORAGE_KEY, tryDecryptToken, @@ -846,20 +847,8 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise { + return persistTokenInStorage( + ACCESS_TOKEN_STORAGE_KEY, + ACCESS_TOKEN_IV, + token, + pickleKey, + HAS_ACCESS_TOKEN_STORAGE_KEY, + ); +} + +/** + * Wraps persistTokenInStorage with refreshToken storage keys + * @param token the token to store, when undefined any existing refreshToken is removed from storage + * @param pickleKey optional pickle key used to encrypt token + */ +export async function persistRefreshTokenInStorage( + token: string | undefined, + pickleKey: string | undefined, +): Promise { + return persistTokenInStorage( + REFRESH_TOKEN_STORAGE_KEY, + REFRESH_TOKEN_IV, + token, + pickleKey, + HAS_REFRESH_TOKEN_STORAGE_KEY, + ); +} From 5986b6c6340ecefd1c033983d76463e4608b1ce5 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 16:39:33 +1300 Subject: [PATCH 29/56] implement TokenRefresher class with persistence --- src/utils/oidc/TokenRefresher.ts | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 src/utils/oidc/TokenRefresher.ts diff --git a/src/utils/oidc/TokenRefresher.ts b/src/utils/oidc/TokenRefresher.ts new file mode 100644 index 00000000000..81f555da9ba --- /dev/null +++ b/src/utils/oidc/TokenRefresher.ts @@ -0,0 +1,49 @@ +/* +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 { IDelegatedAuthConfig, OidcTokenRefresher } from "matrix-js-sdk/src/matrix"; +import { IdTokenClaims } from "oidc-client-ts"; + +import PlatformPeg from "../../PlatformPeg"; +import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../tokens/tokens"; + +export class TokenRefresher extends OidcTokenRefresher { + private readonly deviceId!: string; + + public constructor( + authConfig: IDelegatedAuthConfig, + clientId: string, + redirectUri: string, + deviceId: string, + idTokenClaims: IdTokenClaims, + private readonly userId: string, + ) { + super(authConfig, clientId, deviceId, redirectUri, idTokenClaims); + this.deviceId = deviceId; + } + + public async persistTokens({ + accessToken, + refreshToken, + }: { + accessToken: string; + refreshToken?: string | undefined; + }): Promise { + const pickleKey = (await PlatformPeg.get()?.getPickleKey(this.userId, this.deviceId)) ?? undefined; + await persistAccessTokenInStorage(accessToken, pickleKey); + await persistRefreshTokenInStorage(refreshToken, pickleKey); + } +} From 7db72913995f7d3fe94c75d64fd325c15538c076 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 16:39:51 +1300 Subject: [PATCH 30/56] tidying --- src/utils/oidc/createTokenRefresher.ts | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 src/utils/oidc/createTokenRefresher.ts diff --git a/src/utils/oidc/createTokenRefresher.ts b/src/utils/oidc/createTokenRefresher.ts deleted file mode 100644 index d6b2bb6b5f8..00000000000 --- a/src/utils/oidc/createTokenRefresher.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { MatrixClient } from "matrix-js-sdk/src/client"; - -class TokenRefresher { - constructor( - private refreshToken: string, - private readonly oidcClientSettings, - ) { - - } - - public async doRefreshAccessToken (setAccessToken: MatrixClient['setAccessToken']) { - - throw new Error("Not implemented"); - } -} - -export const createTokenRefresher = (refreshToken: string) => { - return { - refreshToken, - doRefreshAccessToken: async (setAccessToken: MatrixClient['setAccessToken']) => { - - } - } -} \ No newline at end of file From dcd302660a37ad48d0c0a8e6119932f2ccaa371c Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 17:02:52 +1300 Subject: [PATCH 31/56] persist idTokenClaims --- src/utils/oidc/authorize.ts | 6 +++- src/utils/oidc/persistOidcSettings.ts | 22 +++++++++++++- test/utils/oidc/authorize-test.ts | 8 ++++++ test/utils/oidc/persistOidcSettings-test.ts | 32 +++++++++++++++++++-- 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index e475c1795c2..3e6de7de14b 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -18,6 +18,7 @@ import { completeAuthorizationCodeGrant, generateOidcAuthorizationUrl } from "ma import { QueryDict } from "matrix-js-sdk/src/utils"; import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { randomString } from "matrix-js-sdk/src/randomstring"; +import { IdTokenClaims } from "oidc-client-ts"; /** * Start OIDC authorization code flow @@ -81,6 +82,8 @@ type CompleteOidcLoginResponse = { clientId: string; // issuer used during authentication issuer: string; + // claims of the given access token; used during token refresh to validate new tokens + idTokenClaims: IdTokenClaims; }; /** * Attempt to complete authorization code flow to get an access token @@ -90,7 +93,7 @@ type CompleteOidcLoginResponse = { */ export const completeOidcLogin = async (queryParams: QueryDict): Promise => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); - const { homeserverUrl, tokenResponse, identityServerUrl, oidcClientSettings } = + const { homeserverUrl, tokenResponse, idTokenClaims, identityServerUrl, oidcClientSettings } = await completeAuthorizationCodeGrant(code, state); return { @@ -100,5 +103,6 @@ export const completeOidcLogin = async (queryParams: QueryDict): Promise { +export const persistOidcAuthenticatedSettings = ( + clientId: string, + issuer: string, + idTokenClaims: IdTokenClaims, +): void => { sessionStorage.setItem(clientIdStorageKey, clientId); sessionStorage.setItem(tokenIssuerStorageKey, issuer); + sessionStorage.setItem(idTokenClaimsStorageKey, JSON.stringify(idTokenClaims)); }; /** @@ -49,3 +57,15 @@ export const getStoredOidcClientId = (): string => { } return clientId; }; + +/** + * Retrieve stored id token claims from session storage + * @returns idtokenclaims or undefined + */ +export const getStoredOidcIdTokenClaims = (): IdTokenClaims | undefined => { + const idTokenClaims = sessionStorage.getItem(idTokenClaimsStorageKey); + if (!idTokenClaims) { + return; + } + return JSON.parse(idTokenClaims) as IdTokenClaims; +}; diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 2f5f42db237..a7058563b30 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -112,6 +112,13 @@ describe("OIDC authorization", () => { tokenResponse, homeserverUrl, identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + } }); }); @@ -137,6 +144,7 @@ describe("OIDC authorization", () => { identityServerUrl, issuer, clientId, + idTokenClaims: result.idTokenClaims, }); }); }); diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts index 4db5c4e75c7..03ac61d199a 100644 --- a/test/utils/oidc/persistOidcSettings-test.ts +++ b/test/utils/oidc/persistOidcSettings-test.ts @@ -14,8 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { IdTokenClaims } from "oidc-client-ts"; + import { getStoredOidcClientId, + getStoredOidcIdTokenClaims, getStoredOidcTokenIssuer, persistOidcAuthenticatedSettings, } from "../../../src/utils/oidc/persistOidcSettings"; @@ -29,12 +32,25 @@ describe("persist OIDC settings", () => { const clientId = "test-client-id"; const issuer = "https://auth.org/"; + const idTokenClaims: IdTokenClaims = { + // audience is this client + aud: "123", + // issuer matches + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }; describe("persistOidcAuthenticatedSettings", () => { it("should set clientId and issuer in session storage", () => { - persistOidcAuthenticatedSettings(clientId, issuer); + persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_client_id", clientId); expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + expect(sessionStorage.setItem).toHaveBeenCalledWith( + "mx_oidc_id_token_claims", + JSON.stringify(idTokenClaims), + ); }); }); @@ -50,7 +66,7 @@ describe("persist OIDC settings", () => { }); }); - describe("Name of the group", () => { + describe("getStoredOidcClientId()", () => { it("should return clientId from session storage", () => { jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(clientId); expect(getStoredOidcClientId()).toEqual(clientId); @@ -60,4 +76,16 @@ describe("persist OIDC settings", () => { expect(() => getStoredOidcClientId()).toThrow("Oidc client id not found in storage"); }); }); + + describe("getStoredOidcIdTokenClaims()", () => { + it("should return issuer from session storage", () => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(JSON.stringify(idTokenClaims)); + expect(getStoredOidcIdTokenClaims()).toEqual(idTokenClaims); + expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_id_token_claims"); + }); + + it("should return undefined when no issuer in session storage", () => { + expect(getStoredOidcIdTokenClaims()).toBeUndefined(); + }); + }); }); From 4921e783b4d08366f77f2bdc11e271220a024bc2 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 17:02:52 +1300 Subject: [PATCH 32/56] persist idTokenClaims --- src/utils/oidc/authorize.ts | 6 +++- src/utils/oidc/persistOidcSettings.ts | 22 +++++++++++++- test/utils/oidc/authorize-test.ts | 8 ++++++ test/utils/oidc/persistOidcSettings-test.ts | 32 +++++++++++++++++++-- 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index e475c1795c2..3e6de7de14b 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -18,6 +18,7 @@ import { completeAuthorizationCodeGrant, generateOidcAuthorizationUrl } from "ma import { QueryDict } from "matrix-js-sdk/src/utils"; import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { randomString } from "matrix-js-sdk/src/randomstring"; +import { IdTokenClaims } from "oidc-client-ts"; /** * Start OIDC authorization code flow @@ -81,6 +82,8 @@ type CompleteOidcLoginResponse = { clientId: string; // issuer used during authentication issuer: string; + // claims of the given access token; used during token refresh to validate new tokens + idTokenClaims: IdTokenClaims; }; /** * Attempt to complete authorization code flow to get an access token @@ -90,7 +93,7 @@ type CompleteOidcLoginResponse = { */ export const completeOidcLogin = async (queryParams: QueryDict): Promise => { const { code, state } = getCodeAndStateFromQueryParams(queryParams); - const { homeserverUrl, tokenResponse, identityServerUrl, oidcClientSettings } = + const { homeserverUrl, tokenResponse, idTokenClaims, identityServerUrl, oidcClientSettings } = await completeAuthorizationCodeGrant(code, state); return { @@ -100,5 +103,6 @@ export const completeOidcLogin = async (queryParams: QueryDict): Promise { +export const persistOidcAuthenticatedSettings = ( + clientId: string, + issuer: string, + idTokenClaims: IdTokenClaims, +): void => { sessionStorage.setItem(clientIdStorageKey, clientId); sessionStorage.setItem(tokenIssuerStorageKey, issuer); + sessionStorage.setItem(idTokenClaimsStorageKey, JSON.stringify(idTokenClaims)); }; /** @@ -49,3 +57,15 @@ export const getStoredOidcClientId = (): string => { } return clientId; }; + +/** + * Retrieve stored id token claims from session storage + * @returns idtokenclaims or undefined + */ +export const getStoredOidcIdTokenClaims = (): IdTokenClaims | undefined => { + const idTokenClaims = sessionStorage.getItem(idTokenClaimsStorageKey); + if (!idTokenClaims) { + return; + } + return JSON.parse(idTokenClaims) as IdTokenClaims; +}; diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 2f5f42db237..a7058563b30 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -112,6 +112,13 @@ describe("OIDC authorization", () => { tokenResponse, homeserverUrl, identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + } }); }); @@ -137,6 +144,7 @@ describe("OIDC authorization", () => { identityServerUrl, issuer, clientId, + idTokenClaims: result.idTokenClaims, }); }); }); diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts index 4db5c4e75c7..03ac61d199a 100644 --- a/test/utils/oidc/persistOidcSettings-test.ts +++ b/test/utils/oidc/persistOidcSettings-test.ts @@ -14,8 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { IdTokenClaims } from "oidc-client-ts"; + import { getStoredOidcClientId, + getStoredOidcIdTokenClaims, getStoredOidcTokenIssuer, persistOidcAuthenticatedSettings, } from "../../../src/utils/oidc/persistOidcSettings"; @@ -29,12 +32,25 @@ describe("persist OIDC settings", () => { const clientId = "test-client-id"; const issuer = "https://auth.org/"; + const idTokenClaims: IdTokenClaims = { + // audience is this client + aud: "123", + // issuer matches + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }; describe("persistOidcAuthenticatedSettings", () => { it("should set clientId and issuer in session storage", () => { - persistOidcAuthenticatedSettings(clientId, issuer); + persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_client_id", clientId); expect(sessionStorage.setItem).toHaveBeenCalledWith("mx_oidc_token_issuer", issuer); + expect(sessionStorage.setItem).toHaveBeenCalledWith( + "mx_oidc_id_token_claims", + JSON.stringify(idTokenClaims), + ); }); }); @@ -50,7 +66,7 @@ describe("persist OIDC settings", () => { }); }); - describe("Name of the group", () => { + describe("getStoredOidcClientId()", () => { it("should return clientId from session storage", () => { jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(clientId); expect(getStoredOidcClientId()).toEqual(clientId); @@ -60,4 +76,16 @@ describe("persist OIDC settings", () => { expect(() => getStoredOidcClientId()).toThrow("Oidc client id not found in storage"); }); }); + + describe("getStoredOidcIdTokenClaims()", () => { + it("should return issuer from session storage", () => { + jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(JSON.stringify(idTokenClaims)); + expect(getStoredOidcIdTokenClaims()).toEqual(idTokenClaims); + expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_id_token_claims"); + }); + + it("should return undefined when no issuer in session storage", () => { + expect(getStoredOidcIdTokenClaims()).toBeUndefined(); + }); + }); }); From 6ba08a272920258375c248732448675fddbc91c4 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 17:24:57 +1300 Subject: [PATCH 33/56] tests --- src/Lifecycle.ts | 4 +-- .../components/structures/MatrixChat-test.tsx | 27 ++++++++++----- test/utils/oidc/authorize-test.ts | 34 ++++++++++--------- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 0b34e02409a..c0557e65e70 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -278,7 +278,7 @@ export async function attemptDelegatedAuthLogin( */ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise { try { - const { accessToken, refreshToken, homeserverUrl, identityServerUrl, clientId, issuer } = + const { accessToken, refreshToken, homeserverUrl, identityServerUrl, idTokenClaims, clientId, issuer } = await completeOidcLogin(queryParams); const { @@ -300,7 +300,7 @@ async function attemptOidcNativeLogin(queryParams: QueryDict): Promise logger.debug("Logged in via OIDC native flow"); await onSuccessfulDelegatedAuthLogin(credentials); // this needs to happen after success handler which clears storages - persistOidcAuthenticatedSettings(clientId, issuer); + persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); return true; } catch (error) { logger.error("Failed to login via OIDC", error); diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index c01b0025abe..358ba70296a 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -921,15 +921,24 @@ describe("", () => { }; beforeEach(() => { - mocked(completeAuthorizationCodeGrant).mockClear().mockResolvedValue({ - oidcClientSettings: { - clientId, - issuer, - }, - tokenResponse, - homeserverUrl, - identityServerUrl, - }); + mocked(completeAuthorizationCodeGrant) + .mockClear() + .mockResolvedValue({ + oidcClientSettings: { + clientId, + issuer, + }, + tokenResponse, + homeserverUrl, + identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }, + }); jest.spyOn(logger, "error").mockClear(); }); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index a7058563b30..daf65937c84 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -104,22 +104,24 @@ describe("OIDC authorization", () => { }; beforeEach(() => { - mocked(completeAuthorizationCodeGrant).mockClear().mockResolvedValue({ - oidcClientSettings: { - clientId, - issuer, - }, - tokenResponse, - homeserverUrl, - identityServerUrl, - idTokenClaims: { - aud: "123", - iss: issuer, - sub: "123", - exp: 123, - iat: 456, - } - }); + mocked(completeAuthorizationCodeGrant) + .mockClear() + .mockResolvedValue({ + oidcClientSettings: { + clientId, + issuer, + }, + tokenResponse, + homeserverUrl, + identityServerUrl, + idTokenClaims: { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }, + }); }); it("should throw when query params do not include state and code", async () => { From c962ca101897c03761a8156f7faf8e46dd1464ce Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 2 Oct 2023 17:44:38 +1300 Subject: [PATCH 34/56] remove unused cde --- src/utils/oidc/persistOidcSettings.ts | 12 ------------ test/utils/oidc/persistOidcSettings-test.ts | 13 ------------- 2 files changed, 25 deletions(-) diff --git a/src/utils/oidc/persistOidcSettings.ts b/src/utils/oidc/persistOidcSettings.ts index da4510bbacb..f5132291be1 100644 --- a/src/utils/oidc/persistOidcSettings.ts +++ b/src/utils/oidc/persistOidcSettings.ts @@ -57,15 +57,3 @@ export const getStoredOidcClientId = (): string => { } return clientId; }; - -/** - * Retrieve stored id token claims from session storage - * @returns idtokenclaims or undefined - */ -export const getStoredOidcIdTokenClaims = (): IdTokenClaims | undefined => { - const idTokenClaims = sessionStorage.getItem(idTokenClaimsStorageKey); - if (!idTokenClaims) { - return; - } - return JSON.parse(idTokenClaims) as IdTokenClaims; -}; diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts index 03ac61d199a..f71a7f3ed66 100644 --- a/test/utils/oidc/persistOidcSettings-test.ts +++ b/test/utils/oidc/persistOidcSettings-test.ts @@ -18,7 +18,6 @@ import { IdTokenClaims } from "oidc-client-ts"; import { getStoredOidcClientId, - getStoredOidcIdTokenClaims, getStoredOidcTokenIssuer, persistOidcAuthenticatedSettings, } from "../../../src/utils/oidc/persistOidcSettings"; @@ -76,16 +75,4 @@ describe("persist OIDC settings", () => { expect(() => getStoredOidcClientId()).toThrow("Oidc client id not found in storage"); }); }); - - describe("getStoredOidcIdTokenClaims()", () => { - it("should return issuer from session storage", () => { - jest.spyOn(sessionStorage.__proto__, "getItem").mockReturnValue(JSON.stringify(idTokenClaims)); - expect(getStoredOidcIdTokenClaims()).toEqual(idTokenClaims); - expect(sessionStorage.getItem).toHaveBeenCalledWith("mx_oidc_id_token_claims"); - }); - - it("should return undefined when no issuer in session storage", () => { - expect(getStoredOidcIdTokenClaims()).toBeUndefined(); - }); - }); }); From 153ec78357266426078dca6805529a6ee3ce8c77 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 12:23:59 +1300 Subject: [PATCH 35/56] create token refresher during doSetLoggedIn --- src/Lifecycle.ts | 41 +++++++++++++++++++++++++++++------------ src/MatrixClientPeg.ts | 24 +++++++----------------- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index ddf37b954cf..8bf28528b61 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -18,7 +18,7 @@ limitations under the License. */ import { ReactNode } from "react"; -import { createClient, MatrixClient, SSOAction } from "matrix-js-sdk/src/matrix"; +import { createClient, MatrixClient, SSOAction, OidcTokenRefresher } from "matrix-js-sdk/src/matrix"; import { InvalidStoreError } from "matrix-js-sdk/src/errors"; import { IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; import { QueryDict } from "matrix-js-sdk/src/utils"; @@ -65,9 +65,13 @@ import { OverwriteLoginPayload } from "./dispatcher/payloads/OverwriteLoginPaylo import { SdkContextClass } from "./contexts/SDKContext"; import { messageForLoginError } from "./utils/ErrorUtils"; import { completeOidcLogin } from "./utils/oidc/authorize"; -import { getStoredOidcClientId, getStoredOidcTokenIssuer, persistOidcAuthenticatedSettings } from "./utils/oidc/persistOidcSettings"; +import { + getStoredOidcClientId, + getStoredOidcIdTokenClaims, + getStoredOidcTokenIssuer, + persistOidcAuthenticatedSettings, +} from "./utils/oidc/persistOidcSettings"; import GenericToast from "./components/views/toasts/GenericToast"; -import { OidcTokenRefresher } from "matrix-js-sdk/src/oidc/tokenRefresher"; import { ACCESS_TOKEN_IV, ACCESS_TOKEN_STORAGE_KEY, @@ -79,6 +83,7 @@ import { REFRESH_TOKEN_STORAGE_KEY, tryDecryptToken, } from "./utils/tokens/tokens"; +import { TokenRefresher } from "./utils/oidc/TokenRefresher"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; @@ -398,10 +403,9 @@ export function attemptTokenLogin( * Clear storage then save new credentials in storage * @param credentials as returned from login */ -async function onSuccessfulDelegatedAuthLogin(credentials: IMatrixClientCreds, settings): Promise { +async function onSuccessfulDelegatedAuthLogin(credentials: IMatrixClientCreds): Promise { await clearStorage(); await persistCredentials(credentials); - await persistOidcClientSettings(settings); // remember that we just logged in sessionStorage.setItem("mx_fresh_login", String(true)); @@ -752,7 +756,12 @@ export async function hydrateSession(credentials: IMatrixClientCreds): Promise { +/** + * When we have a refreshToken and an OIDC token issuer in storage + * @param credentials + * @returns Promise that resolves to a TokenRefresher, or undefined + */ +async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promise { if (!credentials.refreshToken) { return; } @@ -762,15 +771,23 @@ async function checkForOidcTODOName(credentials: IMatrixClientCreds): Promise => { @@ -391,11 +379,13 @@ class MatrixClientPegClass implements IMatrixClientPeg { }); } - private createClient(creds: IMatrixClientCreds): void { + private createClient(creds: IMatrixClientCreds, tokenRefresher?: OidcTokenRefresher): void { const opts: ICreateClientOpts = { baseUrl: creds.homeserverUrl, idBaseUrl: creds.identityServerUrl, accessToken: creds.accessToken, + refreshToken: creds.refreshToken, + tokenRefreshFunction: tokenRefresher?.doRefreshAccessToken.bind(tokenRefresher), userId: creds.userId, deviceId: creds.deviceId, pickleKey: creds.pickleKey, From ebdf0d51d41103c955cb2e7992968b0b75c8047b Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 12:31:33 +1300 Subject: [PATCH 36/56] tidying --- src/Lifecycle.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 8bf28528b61..6dd46cd5a7b 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -618,9 +618,6 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): const freshLogin = sessionStorage.getItem("mx_fresh_login") === "true"; sessionStorage.removeItem("mx_fresh_login"); - // const oidcClientSettingsRaw = sessionStorage.getItem("oidcClientSettings"); - // const oidcClientSettings = oidcClientSettingsRaw ? JSON.parse(oidcClientSettingsRaw) : undefined; - logger.log(`Restoring session for ${userId}`); await doSetLoggedIn( { From 2b1e73c8167eae421caf8aaec9278ab85da7afee Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 3 Oct 2023 12:33:22 +1300 Subject: [PATCH 37/56] also tidying --- src/Lifecycle.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 6dd46cd5a7b..55c7b31651c 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -630,7 +630,6 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): guest: isGuest, pickleKey: pickleKey ?? undefined, freshLogin: freshLogin, - // oidcClientSettings, }, false, ); From f345a097b3b2b1850ac95a03afa311d097fdea34 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 4 Oct 2023 14:24:03 +1300 Subject: [PATCH 38/56] OidcClientStore.initClient use stored issuer when client well known unavailable --- src/stores/oidc/OidcClientStore.ts | 20 +++++++------ test/stores/oidc/OidcClientStore-test.ts | 38 +++++++++++++++--------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index a80d2f85a29..00fdc79ab06 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -57,7 +57,7 @@ export class OidcClientStore { * @returns Promise that resolves when tokens have been revoked * @throws when OidcClient cannot be initialised, or revoking either token fails */ - public async revokeTokens(accessToken: string, refreshToken: string): Promise { + public async revokeTokens(accessToken?: string, refreshToken?: string): Promise { const client = await this.getOidcClient(); if (!client) { @@ -83,11 +83,13 @@ export class OidcClientStore { */ private async tryRevokeToken( oidcClient: OidcClient, - token: string, + token: string | undefined, tokenType: "access_token" | "refresh_token", ): Promise { try { - console.log("here", tokenType); + if (!token) { + return false; + } await oidcClient.revokeToken(token, tokenType); return true; } catch (error) { @@ -112,17 +114,17 @@ export class OidcClientStore { * @returns promise that resolves when initialising OidcClient succeeds or fails */ private async initOidcClient(): Promise { - const wellKnown = this.matrixClient.getClientWellKnown(); - if (!wellKnown) { - logger.error("Cannot initialise OidcClientStore: client well known required."); + const wellKnown = await this.matrixClient.waitForClientWellKnown(); + if (!wellKnown && !this.authenticatedIssuer) { + logger.error("Cannot initialise OIDC client without issuer."); return; } - - const delegatedAuthConfig = M_AUTHENTICATION.findIn(wellKnown) ?? undefined; + const delegatedAuthConfig = + (wellKnown && M_AUTHENTICATION.findIn(wellKnown)) ?? undefined; try { const clientId = getStoredOidcClientId(); const { account, metadata, signingKeys } = await discoverAndValidateAuthenticationConfig( - delegatedAuthConfig, + delegatedAuthConfig || { issuer: this.authenticatedIssuer! }, ); // if no account endpoint is configured default to the issuer this._accountManagementEndpoint = account ?? metadata.issuer; diff --git a/test/stores/oidc/OidcClientStore-test.ts b/test/stores/oidc/OidcClientStore-test.ts index 0ec39fced8d..d25b0fd5413 100644 --- a/test/stores/oidc/OidcClientStore-test.ts +++ b/test/stores/oidc/OidcClientStore-test.ts @@ -40,7 +40,7 @@ describe("OidcClientStore", () => { }; const mockClient = getMockClientWithEventEmitter({ - getClientWellKnown: jest.fn().mockReturnValue({}), + waitForClientWellKnown: jest.fn().mockResolvedValue({}), }); beforeEach(() => { @@ -52,7 +52,7 @@ describe("OidcClientStore", () => { account, issuer: metadata.issuer, }); - mockClient.getClientWellKnown.mockReturnValue({ + mockClient.waitForClientWellKnown.mockResolvedValue({ [M_AUTHENTICATION.stable!]: { issuer: metadata.issuer, account, @@ -80,7 +80,7 @@ describe("OidcClientStore", () => { describe("initialising oidcClient", () => { it("should initialise oidc client from constructor", () => { - mockClient.getClientWellKnown.mockReturnValue(undefined); + mockClient.waitForClientWellKnown.mockResolvedValue(undefined as any); const store = new OidcClientStore(mockClient); // started initialising @@ -88,30 +88,33 @@ describe("OidcClientStore", () => { expect(store.initialisingOidcClientPromise).toBeTruthy(); }); - it("should log and return when no client well known is available", async () => { - mockClient.getClientWellKnown.mockReturnValue(undefined); + it("should fallback to stored issuer when no client well known is available", async () => { + mockClient.waitForClientWellKnown.mockResolvedValue(undefined as any); const store = new OidcClientStore(mockClient); - expect(logger.error).toHaveBeenCalledWith("Cannot initialise OidcClientStore: client well known required."); - // no oidc client + // successfully created oidc client // @ts-ignore private property - expect(await store.getOidcClient()).toEqual(undefined); + expect(await store.getOidcClient()).toBeTruthy(); }); it("should log and return when no clientId is found in storage", async () => { - jest.spyOn(sessionStorage.__proto__, "getItem").mockImplementation((key) => - key === "mx_oidc_token_issuer" ? metadata.issuer : null, + const sessionStorageWithoutClientId: Record = { + ...mockSessionStorage, + mx_oidc_client_id: null, + }; + jest.spyOn(sessionStorage.__proto__, "getItem").mockImplementation( + (key) => sessionStorageWithoutClientId[key as string] ?? null, ); const store = new OidcClientStore(mockClient); + // no oidc client + // @ts-ignore private property + expect(await store.getOidcClient()).toEqual(undefined); expect(logger.error).toHaveBeenCalledWith( "Failed to initialise OidcClientStore", new Error("Oidc client id not found in storage"), ); - // no oidc client - // @ts-ignore private property - expect(await store.getOidcClient()).toEqual(undefined); }); it("should log and return when discovery and validation fails", async () => { @@ -205,7 +208,14 @@ describe("OidcClientStore", () => { it("should throw when oidcClient could not be initialised", async () => { // make oidcClient initialisation fail - mockClient.getClientWellKnown.mockReturnValue(undefined); + mockClient.waitForClientWellKnown.mockResolvedValue(undefined as any); + const sessionStorageWithoutIssuer: Record = { + ...mockSessionStorage, + mx_oidc_token_issuer: null, + }; + jest.spyOn(sessionStorage.__proto__, "getItem").mockImplementation( + (key) => sessionStorageWithoutIssuer[key as string] ?? null, + ); const store = new OidcClientStore(mockClient); From 83de9147735d4b5ccf96cfabe2daffabd631d6d0 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 4 Oct 2023 14:52:25 +1300 Subject: [PATCH 39/56] test Lifecycle.logout --- test/Lifecycle-test.ts | 85 +++++++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 18 deletions(-) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index c44ea31d698..f8fde9ab2ce 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -19,14 +19,16 @@ import { logger } from "matrix-js-sdk/src/logger"; import * as MatrixJs from "matrix-js-sdk/src/matrix"; import { setCrypto } from "matrix-js-sdk/src/crypto/crypto"; import * as MatrixCryptoAes from "matrix-js-sdk/src/crypto/aes"; +import { MockedObject } from "jest-mock"; import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; -import { restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; +import { logout, restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; import { MatrixClientPeg } from "../src/MatrixClientPeg"; import Modal from "../src/Modal"; import * as StorageManager from "../src/utils/StorageManager"; -import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; +import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser, mockPlatformPeg } from "./test-utils"; import ToastStore from "../src/stores/ToastStore"; +import { OidcClientStore } from "../src/stores/oidc/OidcClientStore"; const webCrypto = new Crypto(); @@ -37,24 +39,28 @@ describe("Lifecycle", () => { const realLocalStorage = global.localStorage; - const mockClient = getMockClientWithEventEmitter({ - stopClient: jest.fn(), - removeAllListeners: jest.fn(), - clearStores: jest.fn(), - getAccountData: jest.fn(), - getUserId: jest.fn(), - getDeviceId: jest.fn(), - isVersionSupported: jest.fn().mockResolvedValue(true), - getCrypto: jest.fn(), - getClientWellKnown: jest.fn(), - getThirdpartyProtocols: jest.fn(), - store: { - destroy: jest.fn(), - }, - getVersions: jest.fn().mockResolvedValue({ versions: ["v1.1"] }), - }); + let mockClient!: MockedObject; beforeEach(() => { + mockClient = getMockClientWithEventEmitter({ + ...mockClientMethodsUser(), + stopClient: jest.fn(), + removeAllListeners: jest.fn(), + clearStores: jest.fn(), + getAccountData: jest.fn(), + getDeviceId: jest.fn(), + isVersionSupported: jest.fn().mockResolvedValue(true), + getCrypto: jest.fn(), + getClientWellKnown: jest.fn(), + getThirdpartyProtocols: jest.fn(), + store: { + destroy: jest.fn(), + }, + getVersions: jest.fn().mockResolvedValue({ versions: ["v1.1"] }), + logout: jest.fn().mockResolvedValue(undefined), + getAccessToken: jest.fn(), + getRefreshToken: jest.fn(), + }); // stub this jest.spyOn(MatrixClientPeg, "replaceUsingCreds").mockImplementation(() => {}); jest.spyOn(MatrixClientPeg, "start").mockResolvedValue(undefined); @@ -641,4 +647,47 @@ describe("Lifecycle", () => { }); }); }); + + describe("logout()", () => { + let oidcClientStore!: OidcClientStore; + const accessToken = "test-access-token"; + const refreshToken = "test-refresh-token"; + + beforeEach(() => { + oidcClientStore = new OidcClientStore(mockClient); + // stub + jest.spyOn(oidcClientStore, "revokeTokens").mockResolvedValue(undefined); + + mockClient.getAccessToken.mockReturnValue(accessToken); + mockClient.getRefreshToken.mockReturnValue(refreshToken); + }); + + it("should call logout on the client when oidcClientStore is falsy", async () => { + logout(); + + await flushPromises(); + + expect(mockClient.logout).toHaveBeenCalledWith(true); + }); + + it("should call logout on the client when oidcClientStore.isUserAuthenticatedWithOidc is falsy", async () => { + jest.spyOn(oidcClientStore, "isUserAuthenticatedWithOidc", "get").mockReturnValue(false); + logout(oidcClientStore); + + await flushPromises(); + + expect(mockClient.logout).toHaveBeenCalledWith(true); + expect(oidcClientStore.revokeTokens).not.toHaveBeenCalled(); + }); + + it("should revoke tokens when user is authenticated with oidc", async () => { + jest.spyOn(oidcClientStore, "isUserAuthenticatedWithOidc", "get").mockReturnValue(true); + logout(oidcClientStore); + + await flushPromises(); + + expect(mockClient.logout).not.toHaveBeenCalled(); + expect(oidcClientStore.revokeTokens).toHaveBeenCalledWith(accessToken, refreshToken); + }); + }); }); From 7e081d4479b5a660f4e006f455b9a08f16e32572 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 4 Oct 2023 14:57:50 +1300 Subject: [PATCH 40/56] update Lifecycle test replaceUsingCreds calls --- test/Lifecycle-test.ts | 149 +++++++++++++++++++++++------------------ 1 file changed, 84 insertions(+), 65 deletions(-) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index c44ea31d698..b5085f72f26 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -233,6 +233,7 @@ describe("Lifecycle", () => { userId, guest: true, }), + undefined, ); expect(localStorage.setItem).toHaveBeenCalledWith("mx_is_guest", "true"); }); @@ -264,16 +265,19 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await restoreFromLocalStorage()).toEqual(true); - expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ - userId, - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey: undefined, - }); + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: undefined, + }, + undefined, + ); }); it("should remove fresh login flag from session storage", async () => { @@ -312,18 +316,21 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await restoreFromLocalStorage()).toEqual(true); - expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ - userId, - accessToken, - // refreshToken included in credentials - refreshToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey: undefined, - }); + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + accessToken, + // refreshToken included in credentials + refreshToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: undefined, + }, + undefined, + ); }); }); }); @@ -373,17 +380,20 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await restoreFromLocalStorage()).toEqual(true); - expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ - userId, - // decrypted accessToken - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: true, - guest: false, - pickleKey: expect.any(String), - }); + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + // decrypted accessToken + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: expect.any(String), + }, + undefined, + ); }); describe("with a refresh token", () => { @@ -412,18 +422,21 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await restoreFromLocalStorage()).toEqual(true); - expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ - userId, - accessToken, - // refreshToken included in credentials - refreshToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: false, - guest: false, - pickleKey: expect.any(String), - }); + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + accessToken, + // refreshToken included in credentials + refreshToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: false, + guest: false, + pickleKey: expect.any(String), + }, + undefined, + ); }); }); }); @@ -529,16 +542,19 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await setLoggedIn(credentials)).toEqual(mockClient); - expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ - userId, - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: true, - guest: false, - pickleKey: null, - }); + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: null, + }, + undefined, + ); }); }); @@ -628,16 +644,19 @@ describe("Lifecycle", () => { it("should create new matrix client with credentials", async () => { expect(await setLoggedIn(credentials)).toEqual(mockClient); - expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith({ - userId, - accessToken, - homeserverUrl, - identityServerUrl, - deviceId, - freshLogin: true, - guest: false, - pickleKey: expect.any(String), - }); + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + { + userId, + accessToken, + homeserverUrl, + identityServerUrl, + deviceId, + freshLogin: true, + guest: false, + pickleKey: expect.any(String), + }, + undefined, + ); }); }); }); From b2a3cd18eacb8f8e91fcd03c603ca1ad0747337e Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 5 Oct 2023 13:47:43 +1300 Subject: [PATCH 41/56] fix test --- test/components/structures/MatrixChat-test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 5b308974e38..3b5ed9390fb 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -1078,7 +1078,6 @@ describe("", () => { expect(localStorage.getItem("mx_hs_url")).toEqual(homeserverUrl); expect(localStorage.getItem("mx_user_id")).toEqual(userId); expect(localStorage.getItem("mx_has_access_token")).toEqual("true"); - expect(localStorage.getItem("mx_has_refresh_token")).toEqual("true"); expect(localStorage.getItem("mx_device_id")).toEqual(deviceId); }); From 0a1c66e94380b4d79526a415d7e045beb71abeb4 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 9 Oct 2023 12:34:40 +1300 Subject: [PATCH 42/56] add sdkContext to UserSettingsDialog --- src/components/structures/MatrixChat.tsx | 2 +- .../views/dialogs/UserSettingsDialog.tsx | 35 +++++++++++-------- .../views/dialogs/UserSettingsDialog-test.tsx | 18 ++++++---- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index d1c1da4cc32..ae5fe3e3184 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -766,7 +766,7 @@ export default class MatrixChat extends React.PureComponent { const tabPayload = payload as OpenToTabPayload; Modal.createDialog( UserSettingsDialog, - { initialTabId: tabPayload.initialTabId as UserTab }, + { initialTabId: tabPayload.initialTabId as UserTab, sdkContext: this.stores }, /*className=*/ undefined, /*isPriority=*/ false, /*isStatic=*/ true, diff --git a/src/components/views/dialogs/UserSettingsDialog.tsx b/src/components/views/dialogs/UserSettingsDialog.tsx index e8e33a1913a..2635bd27cd6 100644 --- a/src/components/views/dialogs/UserSettingsDialog.tsx +++ b/src/components/views/dialogs/UserSettingsDialog.tsx @@ -37,9 +37,11 @@ import KeyboardUserSettingsTab from "../settings/tabs/user/KeyboardUserSettingsT import SessionManagerTab from "../settings/tabs/user/SessionManagerTab"; import { UserTab } from "./UserTab"; import { NonEmptyArray } from "../../../@types/common"; +import { SDKContext, SdkContextClass } from "../../../contexts/SDKContext"; interface IProps { initialTabId?: UserTab; + sdkContext: SdkContextClass; onFinished(): void; } @@ -201,20 +203,25 @@ export default class UserSettingsDialog extends React.Component public render(): React.ReactNode { return ( - -
- -
- + // XXX: SDKContext is provided within the LoggedInView subtree. + // Modals function outside the MatrixChat React tree, so sdkContext is reprovided here to simulate that. + // The longer term solution is to move our ModalManager into the React tree to inherit contexts properly. + + +
+ +
+ + ); } } diff --git a/test/components/views/dialogs/UserSettingsDialog-test.tsx b/test/components/views/dialogs/UserSettingsDialog-test.tsx index fc0e36b93b6..586288d2bdb 100644 --- a/test/components/views/dialogs/UserSettingsDialog-test.tsx +++ b/test/components/views/dialogs/UserSettingsDialog-test.tsx @@ -16,7 +16,8 @@ limitations under the License. import React, { ReactElement } from "react"; import { render } from "@testing-library/react"; -import { mocked } from "jest-mock"; +import { mocked, MockedObject } from "jest-mock"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; import SettingsStore, { CallbackFn } from "../../../../src/settings/SettingsStore"; import SdkConfig from "../../../../src/SdkConfig"; @@ -30,6 +31,7 @@ import { } from "../../../test-utils"; import { UIFeature } from "../../../../src/settings/UIFeature"; import { SettingLevel } from "../../../../src/settings/SettingLevel"; +import { SdkContextClass } from "../../../../src/contexts/SDKContext"; mockPlatformPeg({ supportsSpellCheckSettings: jest.fn().mockReturnValue(false), @@ -55,18 +57,22 @@ describe("", () => { const userId = "@alice:server.org"; const mockSettingsStore = mocked(SettingsStore); const mockSdkConfig = mocked(SdkConfig); - getMockClientWithEventEmitter({ - ...mockClientMethodsUser(userId), - ...mockClientMethodsServer(), - }); + let mockClient!: MockedObject; + let sdkContext: SdkContextClass; const defaultProps = { onFinished: jest.fn() }; const getComponent = (props: Partial = {}): ReactElement => ( - + ); beforeEach(() => { jest.clearAllMocks(); + mockClient = getMockClientWithEventEmitter({ + ...mockClientMethodsUser(userId), + ...mockClientMethodsServer(), + }); + sdkContext = new SdkContextClass(); + sdkContext.client = mockClient; mockSettingsStore.getValue.mockReturnValue(false); mockSettingsStore.getFeatureSettingNames.mockReturnValue([]); mockSdkConfig.get.mockReturnValue({ brand: "Test" }); From 40b065fd519495b1cc9af352e3d94629ac5d8314 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 9 Oct 2023 12:35:22 +1300 Subject: [PATCH 43/56] use sdkContext and oidcClientStore in session manager --- .../views/settings/devices/useOwnDevices.ts | 5 +- .../settings/tabs/user/SessionManagerTab.tsx | 8 +-- src/utils/oidc/getDelegatedAuthAccountUrl.ts | 27 -------- .../tabs/user/SessionManagerTab-test.tsx | 46 +++++--------- .../oidc/getDelegatedAuthAccountUrl-test.ts | 61 ------------------- 5 files changed, 23 insertions(+), 124 deletions(-) delete mode 100644 src/utils/oidc/getDelegatedAuthAccountUrl.ts delete mode 100644 test/utils/oidc/getDelegatedAuthAccountUrl-test.ts diff --git a/src/components/views/settings/devices/useOwnDevices.ts b/src/components/views/settings/devices/useOwnDevices.ts index 9adb5bf287e..c735b2cbcec 100644 --- a/src/components/views/settings/devices/useOwnDevices.ts +++ b/src/components/views/settings/devices/useOwnDevices.ts @@ -32,13 +32,13 @@ import { VerificationRequest } from "matrix-js-sdk/src/crypto-api"; import { logger } from "matrix-js-sdk/src/logger"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; -import MatrixClientContext from "../../../../contexts/MatrixClientContext"; import { _t } from "../../../../languageHandler"; import { getDeviceClientInformation, pruneClientInformation } from "../../../../utils/device/clientInformation"; import { DevicesDictionary, ExtendedDevice, ExtendedDeviceAppInfo } from "./types"; import { useEventEmitter } from "../../../../hooks/useEventEmitter"; import { parseUserAgent } from "../../../../utils/device/parseUserAgent"; import { isDeviceVerified } from "../../../../utils/device/isDeviceVerified"; +import { SDKContext } from "../../../../contexts/SDKContext"; const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyDevice): ExtendedDeviceAppInfo => { const { name, version, url } = getDeviceClientInformation(matrixClient, device.device_id); @@ -90,7 +90,8 @@ export type DevicesState = { supportsMSC3881?: boolean | undefined; }; export const useOwnDevices = (): DevicesState => { - const matrixClient = useContext(MatrixClientContext); + const sdkContext = useContext(SDKContext); + const matrixClient = sdkContext.client!; const currentDeviceId = matrixClient.getDeviceId()!; const userId = matrixClient.getSafeUserId(); diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index cc88cb27e83..bc06103255c 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -19,7 +19,6 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import { _t } from "../../../../../languageHandler"; -import MatrixClientContext from "../../../../../contexts/MatrixClientContext"; import Modal from "../../../../../Modal"; import SettingsSubsection from "../../shared/SettingsSubsection"; import SetupEncryptionDialog from "../../../dialogs/security/SetupEncryptionDialog"; @@ -39,8 +38,8 @@ import QuestionDialog from "../../../dialogs/QuestionDialog"; import { FilterVariation } from "../../devices/filter"; import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading"; import { SettingsSection } from "../../shared/SettingsSection"; -import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl"; import { OidcLogoutDialog } from "../../../dialogs/oidc/OidcLogoutDialog"; +import { SDKContext } from "../../../../../contexts/SDKContext"; const confirmSignOut = async (sessionsToSignOutCount: number): Promise => { const { finished } = Modal.createDialog(QuestionDialog, { @@ -167,13 +166,14 @@ const SessionManagerTab: React.FC = () => { const filteredDeviceListRef = useRef(null); const scrollIntoViewTimeoutRef = useRef(); - const matrixClient = useContext(MatrixClientContext); + const sdkContext = useContext(SDKContext); + const matrixClient = sdkContext.client!; /** * If we have a delegated auth account management URL, all sessions but the current session need to be managed in the * delegated auth provider. * See https://github.com/matrix-org/matrix-spec-proposals/pull/3824 */ - const delegatedAuthAccountUrl = getDelegatedAuthAccountUrl(matrixClient.getClientWellKnown()); + const delegatedAuthAccountUrl = sdkContext.oidcClientStore.accountManagementEndpoint; const disableMultipleSignout = !!delegatedAuthAccountUrl; const userId = matrixClient?.getUserId(); diff --git a/src/utils/oidc/getDelegatedAuthAccountUrl.ts b/src/utils/oidc/getDelegatedAuthAccountUrl.ts deleted file mode 100644 index cfb61cb4434..00000000000 --- a/src/utils/oidc/getDelegatedAuthAccountUrl.ts +++ /dev/null @@ -1,27 +0,0 @@ -/* -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 { IClientWellKnown, IDelegatedAuthConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix"; - -/** - * Get the delegated auth account management url if configured - * @param clientWellKnown from MatrixClient.getClientWellKnown - * @returns the account management url, or undefined - */ -export const getDelegatedAuthAccountUrl = (clientWellKnown: IClientWellKnown | undefined): string | undefined => { - const delegatedAuthConfig = M_AUTHENTICATION.findIn(clientWellKnown); - return delegatedAuthConfig?.account; -}; diff --git a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx index df5037acb4f..636d3693441 100644 --- a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx +++ b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx @@ -32,9 +32,9 @@ import { CryptoApi, DeviceVerificationStatus, MatrixError, - M_AUTHENTICATION, + MatrixClient, } from "matrix-js-sdk/src/matrix"; -import { mocked } from "jest-mock"; +import { mocked, MockedObject } from "jest-mock"; import { clearAllModals, @@ -45,13 +45,14 @@ import { mockPlatformPeg, } from "../../../../../test-utils"; import SessionManagerTab from "../../../../../../src/components/views/settings/tabs/user/SessionManagerTab"; -import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext"; import Modal from "../../../../../../src/Modal"; import LogoutDialog from "../../../../../../src/components/views/dialogs/LogoutDialog"; import { DeviceSecurityVariation, ExtendedDevice } from "../../../../../../src/components/views/settings/devices/types"; import { INACTIVE_DEVICE_AGE_MS } from "../../../../../../src/components/views/settings/devices/filter"; import SettingsStore from "../../../../../../src/settings/SettingsStore"; import { getClientInformationEventType } from "../../../../../../src/utils/device/clientInformation"; +import { SDKContext, SdkContextClass } from "../../../../../../src/contexts/SDKContext"; +import { OidcClientStore } from "../../../../../../src/stores/oidc/OidcClientStore"; mockPlatformPeg(); @@ -91,31 +92,14 @@ describe("", () => { requestDeviceVerification: jest.fn().mockResolvedValue(mockVerificationRequest), } as unknown as CryptoApi); - let mockClient = getMockClientWithEventEmitter({ - ...mockClientMethodsUser(aliceId), - getCrypto: jest.fn().mockReturnValue(mockCrypto), - getDevices: jest.fn(), - getStoredDevice: jest.fn(), - getDeviceId: jest.fn().mockReturnValue(deviceId), - deleteMultipleDevices: jest.fn(), - generateClientSecret: jest.fn(), - setDeviceDetails: jest.fn(), - getAccountData: jest.fn(), - deleteAccountData: jest.fn(), - doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(true), - getPushers: jest.fn(), - setPusher: jest.fn(), - setLocalNotificationSettings: jest.fn(), - getVersions: jest.fn().mockResolvedValue({}), - getCapabilities: jest.fn().mockResolvedValue({}), - getClientWellKnown: jest.fn().mockReturnValue({}), - }); + let mockClient!: MockedObject; + let sdkContext: SdkContextClass; const defaultProps = {}; const getComponent = (props = {}): React.ReactElement => ( - + - + ); const toggleDeviceDetails = ( @@ -230,6 +214,9 @@ describe("", () => { } }); + sdkContext = new SdkContextClass(); + sdkContext.client = mockClient; + // @ts-ignore allow delete of non-optional prop delete window.location; // @ts-ignore ugly mocking @@ -1051,12 +1038,11 @@ describe("", () => { describe("for an OIDC-aware server", () => { beforeEach(() => { - mockClient.getClientWellKnown.mockReturnValue({ - [M_AUTHENTICATION.name]: { - issuer: "https://issuer.org", - account: "https://issuer.org/account", - }, - }); + // just do an ugly mock here to avoid mocking initialisation + const mockOidcClientStore = { + accountManagementEndpoint: "https://issuer.org/account", + } as unknown as OidcClientStore; + jest.spyOn(sdkContext, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore); }); // signing out the current device works as usual diff --git a/test/utils/oidc/getDelegatedAuthAccountUrl-test.ts b/test/utils/oidc/getDelegatedAuthAccountUrl-test.ts deleted file mode 100644 index e4ba4c5756d..00000000000 --- a/test/utils/oidc/getDelegatedAuthAccountUrl-test.ts +++ /dev/null @@ -1,61 +0,0 @@ -/* -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 { M_AUTHENTICATION } from "matrix-js-sdk/src/matrix"; - -import { getDelegatedAuthAccountUrl } from "../../../src/utils/oidc/getDelegatedAuthAccountUrl"; - -describe("getDelegatedAuthAccountUrl()", () => { - it("should return undefined when wk is undefined", () => { - expect(getDelegatedAuthAccountUrl(undefined)).toBeUndefined(); - }); - - it("should return undefined when wk has no authentication config", () => { - expect(getDelegatedAuthAccountUrl({})).toBeUndefined(); - }); - - it("should return undefined when wk authentication config has no configured account url", () => { - expect( - getDelegatedAuthAccountUrl({ - [M_AUTHENTICATION.stable!]: { - issuer: "issuer.org", - }, - }), - ).toBeUndefined(); - }); - - it("should return the account url for authentication config using the unstable prefix", () => { - expect( - getDelegatedAuthAccountUrl({ - [M_AUTHENTICATION.unstable!]: { - issuer: "issuer.org", - account: "issuer.org/account", - }, - }), - ).toEqual("issuer.org/account"); - }); - - it("should return the account url for authentication config using the stable prefix", () => { - expect( - getDelegatedAuthAccountUrl({ - [M_AUTHENTICATION.stable!]: { - issuer: "issuer.org", - account: "issuer.org/account", - }, - }), - ).toEqual("issuer.org/account"); - }); -}); From cbd9c888fdf1c6b172b6eff845d3b8d8b2cab2f6 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 9 Oct 2023 12:35:38 +1300 Subject: [PATCH 44/56] use sdkContext and OidcClientStore in generalusersettingstab --- .../tabs/user/GeneralUserSettingsTab.tsx | 27 ++++++------ .../tabs/user/GeneralUserSettingsTab-test.tsx | 42 ++++++++++--------- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx index 40acfb31da9..79eae267c22 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx @@ -56,9 +56,8 @@ import SettingsSubsection, { SettingsSubsectionText } from "../../shared/Setting import { SettingsSubsectionHeading } from "../../shared/SettingsSubsectionHeading"; import Heading from "../../../typography/Heading"; import InlineSpinner from "../../../elements/InlineSpinner"; -import MatrixClientContext from "../../../../../contexts/MatrixClientContext"; import { ThirdPartyIdentifier } from "../../../../../AddThreepid"; -import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl"; +import { SDKContext } from "../../../../../contexts/SDKContext"; interface IProps { closeSettingsFn: () => void; @@ -94,20 +93,22 @@ interface IState { } export default class GeneralUserSettingsTab extends React.Component { - public static contextType = MatrixClientContext; - public context!: React.ContextType; + public static contextType = SDKContext; + public context!: React.ContextType; private readonly dispatcherRef: string; - public constructor(props: IProps, context: React.ContextType) { + public constructor(props: IProps, context: React.ContextType) { super(props); this.context = context; + const cli = this.context.client!; + this.state = { language: languageHandler.getCurrentLanguage(), spellCheckEnabled: false, spellCheckLanguages: [], - haveIdServer: Boolean(this.context.getIdentityServerUrl()), + haveIdServer: Boolean(cli.getIdentityServerUrl()), idServerHasUnsignedTerms: false, requiredPolicyInfo: { // This object is passed along to a component for handling @@ -150,7 +151,7 @@ export default class GeneralUserSettingsTab extends React.Component { if (payload.action === "id_server_changed") { - this.setState({ haveIdServer: Boolean(this.context.getIdentityServerUrl()) }); + this.setState({ haveIdServer: Boolean(this.context.client!.getIdentityServerUrl()) }); this.getThreepidState(); } }; @@ -164,7 +165,7 @@ export default class GeneralUserSettingsTab extends React.Component { - const cli = this.context; + const cli = this.context.client!; const capabilities = await cli.getCapabilities(); // this is cached const changePasswordCap = capabilities["m.change_password"]; @@ -174,7 +175,7 @@ export default class GeneralUserSettingsTab extends React.Component { - const cli = this.context; + const cli = this.context.client!; // Check to see if terms need accepting this.checkTerms(); @@ -195,7 +196,7 @@ export default class GeneralUserSettingsTab extends React.Component { // By starting the terms flow we get the logic for checking which terms the user has signed // for free. So we might as well use that for our own purposes. - const idServerUrl = this.context.getIdentityServerUrl(); + const idServerUrl = this.context.client!.getIdentityServerUrl(); if (!this.state.haveIdServer || !idServerUrl) { this.setState({ idServerHasUnsignedTerms: false }); return; @@ -221,7 +222,7 @@ export default class GeneralUserSettingsTab extends React.Component { return new Promise((resolve, reject) => { diff --git a/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx index 7e7bdb4585b..9ed18fa2256 100644 --- a/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx @@ -13,11 +13,11 @@ limitations under the License. import { fireEvent, render, screen, within } from "@testing-library/react"; import React from "react"; -import { M_AUTHENTICATION, ThreepidMedium } from "matrix-js-sdk/src/matrix"; +import { ThreepidMedium } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import GeneralUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/GeneralUserSettingsTab"; -import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext"; +import { SdkContextClass, SDKContext } from "../../../../../../src/contexts/SDKContext"; import SettingsStore from "../../../../../../src/settings/SettingsStore"; import { getMockClientWithEventEmitter, @@ -28,6 +28,7 @@ import { } from "../../../../../test-utils"; import { UIFeature } from "../../../../../../src/settings/UIFeature"; import { SettingLevel } from "../../../../../../src/settings/SettingLevel"; +import { OidcClientStore } from "../../../../../../src/stores/oidc/OidcClientStore"; describe("", () => { const defaultProps = { @@ -44,19 +45,18 @@ describe("", () => { deleteThreePid: jest.fn(), }); + let stores: SdkContextClass; + const getComponent = () => ( - + - + ); - const clientWellKnownSpy = jest.spyOn(mockClient, "getClientWellKnown"); - beforeEach(() => { jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); mockPlatformPeg(); jest.clearAllMocks(); - clientWellKnownSpy.mockReturnValue({}); jest.spyOn(SettingsStore, "getValue").mockRestore(); jest.spyOn(logger, "error").mockRestore(); @@ -67,6 +67,12 @@ describe("", () => { mockClient.deleteThreePid.mockResolvedValue({ id_server_unbind_result: "success", }); + + stores = new SdkContextClass(); + stores.client = mockClient; + // stub out this store completely to avoid mocking initialisation + const mockOidcClientStore = {} as unknown as OidcClientStore; + jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore); }); it("does not show account management link when not available", () => { @@ -78,12 +84,11 @@ describe("", () => { it("show account management link in expected format", async () => { const accountManagementLink = "https://id.server.org/my-account"; - clientWellKnownSpy.mockReturnValue({ - [M_AUTHENTICATION.name]: { - issuer: "https://id.server.org", - account: accountManagementLink, - }, - }); + const mockOidcClientStore = { + accountManagementEndpoint: accountManagementLink, + } as unknown as OidcClientStore; + jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore); + const { getByTestId } = render(getComponent()); // wait for well-known call to settle @@ -167,12 +172,11 @@ describe("", () => { (settingName) => settingName === UIFeature.Deactivate, ); // account is managed externally when we have delegated auth configured - mockClient.getClientWellKnown.mockReturnValue({ - [M_AUTHENTICATION.name]: { - issuer: "https://issuer.org", - account: "https://issuer.org/account", - }, - }); + const accountManagementLink = "https://id.server.org/my-account"; + const mockOidcClientStore = { + accountManagementEndpoint: accountManagementLink, + } as unknown as OidcClientStore; + jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore); render(getComponent()); await flushPromises(); From df70f532d59f8fedd6e2d597d4dbdb3b27e677c8 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 14:13:51 +1300 Subject: [PATCH 45/56] tidy --- src/Lifecycle.ts | 10 ++++++---- src/MatrixClientPeg.ts | 12 ++++++------ src/utils/oidc/TokenRefresher.ts | 4 ++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 55c7b31651c..23873140667 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -753,14 +753,16 @@ export async function hydrateSession(credentials: IMatrixClientCreds): Promise { if (!credentials.refreshToken) { return; } + // stored token issuer indicates we authenticated via OIDC-native flow const tokenIssuer = getStoredOidcTokenIssuer(); if (!tokenIssuer) { return; @@ -768,7 +770,6 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis try { const clientId = getStoredOidcClientId(); const idTokenClaims = getStoredOidcIdTokenClaims(); - // @TODO(kerrya) this should probably come from somewhere const redirectUri = window.location.origin; const deviceId = credentials.deviceId; if (!deviceId) { @@ -782,6 +783,7 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis idTokenClaims!, credentials.userId, ); + // wait for the OIDC client to initialise await tokenRefresher.oidcClientReady; return tokenRefresher; } catch (error) { @@ -834,7 +836,7 @@ async function doSetLoggedIn(credentials: IMatrixClientCreds, clearStorageEnable // check the session lock just before creating the new client checkSessionLock(); - MatrixClientPeg.replaceUsingCreds(credentials, tokenRefresher); + MatrixClientPeg.replaceUsingCreds(credentials, tokenRefresher?.doRefreshAccessToken.bind(tokenRefresher)); const client = MatrixClientPeg.safeGet(); setSentryUser(credentials.userId); diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 3008b515b41..5bf41000b76 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -27,7 +27,7 @@ import { IStartClientOpts, MatrixClient, MemoryStore, - OidcTokenRefresher, + TokenRefreshFunction, } from "matrix-js-sdk/src/matrix"; import * as utils from "matrix-js-sdk/src/utils"; import { verificationMethods } from "matrix-js-sdk/src/crypto"; @@ -124,7 +124,7 @@ export interface IMatrixClientPeg { * * @param {IMatrixClientCreds} creds The new credentials to use. */ - replaceUsingCreds(creds: IMatrixClientCreds, tokenRefresher?: OidcTokenRefresher): void; + replaceUsingCreds(creds: IMatrixClientCreds, tokenRefreshFunction?: TokenRefreshFunction): void; } /** @@ -197,8 +197,8 @@ class MatrixClientPegClass implements IMatrixClientPeg { } } - public replaceUsingCreds(creds: IMatrixClientCreds, tokenRefresher?: OidcTokenRefresher): void { - this.createClient(creds, tokenRefresher); + public replaceUsingCreds(creds: IMatrixClientCreds, tokenRefreshFunction?: TokenRefreshFunction): void { + this.createClient(creds, tokenRefreshFunction); } private onUnexpectedStoreClose = async (): Promise => { @@ -379,13 +379,13 @@ class MatrixClientPegClass implements IMatrixClientPeg { }); } - private createClient(creds: IMatrixClientCreds, tokenRefresher?: OidcTokenRefresher): void { + private createClient(creds: IMatrixClientCreds, tokenRefreshFunction?: TokenRefreshFunction): void { const opts: ICreateClientOpts = { baseUrl: creds.homeserverUrl, idBaseUrl: creds.identityServerUrl, accessToken: creds.accessToken, refreshToken: creds.refreshToken, - tokenRefreshFunction: tokenRefresher?.doRefreshAccessToken.bind(tokenRefresher), + tokenRefreshFunction, userId: creds.userId, deviceId: creds.deviceId, pickleKey: creds.pickleKey, diff --git a/src/utils/oidc/TokenRefresher.ts b/src/utils/oidc/TokenRefresher.ts index 81f555da9ba..9d501dfb9c9 100644 --- a/src/utils/oidc/TokenRefresher.ts +++ b/src/utils/oidc/TokenRefresher.ts @@ -20,6 +20,10 @@ import { IdTokenClaims } from "oidc-client-ts"; import PlatformPeg from "../../PlatformPeg"; import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../tokens/tokens"; +/** + * OidcTokenRefresher that implements token persistence + * Stores tokens in the same was as login flows in Lifecycle + */ export class TokenRefresher extends OidcTokenRefresher { private readonly deviceId!: string; From 0b0bb614a802dc71f3b8eddc18042c3d0121ecfe Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 14:54:43 +1300 Subject: [PATCH 46/56] test tokenrefresher creation in login flow --- test/Lifecycle-test.ts | 116 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index b5085f72f26..a3c805f3478 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -19,6 +19,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import * as MatrixJs from "matrix-js-sdk/src/matrix"; import { setCrypto } from "matrix-js-sdk/src/crypto/crypto"; import * as MatrixCryptoAes from "matrix-js-sdk/src/crypto/aes"; +import fetchMock from "fetch-mock-jest"; import StorageEvictedDialog from "../src/components/views/dialogs/StorageEvictedDialog"; import { restoreFromLocalStorage, setLoggedIn } from "../src/Lifecycle"; @@ -27,6 +28,8 @@ import Modal from "../src/Modal"; import * as StorageManager from "../src/utils/StorageManager"; import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; import ToastStore from "../src/stores/ToastStore"; +import { makeDelegatedAuthConfig } from "./test-utils/oidc"; +import { persistOidcAuthenticatedSettings } from "../src/utils/oidc/persistOidcSettings"; const webCrypto = new Crypto(); @@ -659,5 +662,118 @@ describe("Lifecycle", () => { ); }); }); + + describe("when authenticated via OIDC native flow", () => { + const clientId = "test-client-id"; + const issuer = "https://auth.com/"; + + const delegatedAuthConfig = makeDelegatedAuthConfig(issuer); + const idTokenClaims = { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }; + + beforeAll(() => { + fetchMock.get( + `${delegatedAuthConfig.issuer}.well-known/openid-configuration`, + delegatedAuthConfig.metadata, + ); + fetchMock.get(`${delegatedAuthConfig.issuer}jwks`, { + status: 200, + headers: { + "Content-Type": "application/json", + }, + keys: [], + }); + }); + + beforeEach(() => { + // mock oidc config for oidc client initialisation + mockClient.getClientWellKnown.mockReturnValue({ + [MatrixJs.M_AUTHENTICATION.stable!]: { + issuer: issuer, + }, + }); + initSessionStorageMock(); + // set values in session storage as they would be after a successful oidc authentication + persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); + }); + + it("should not try to create a token refresher without a refresh token", async () => { + await setLoggedIn(credentials); + + // didn't try to initialise token refresher + expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); + }); + + it("should not try to create a token refresher without a deviceId", async () => { + await setLoggedIn({ + ...credentials, + refreshToken, + deviceId: undefined, + }); + + // didn't try to initialise token refresher + expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); + }); + + it("should not try to create a token refresher without an issuer in session storage", async () => { + persistOidcAuthenticatedSettings( + clientId, + // @ts-ignore set undefined issuer + undefined, + idTokenClaims, + ); + await setLoggedIn({ + ...credentials, + refreshToken, + }); + + // didn't try to initialise token refresher + expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); + }); + + it("should create a client with a tokenRefreshFunction", async () => { + expect( + await setLoggedIn({ + ...credentials, + refreshToken, + }), + ).toEqual(mockClient); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + expect.objectContaining({ + accessToken, + refreshToken, + }), + expect.any(Function), + ); + }); + + it("should create a client when creating token refresher fails", async () => { + // set invalid value in session storage for a malformed oidc authentication + persistOidcAuthenticatedSettings(null as any, issuer, idTokenClaims); + + // succeeded + expect( + await setLoggedIn({ + ...credentials, + refreshToken, + }), + ).toEqual(mockClient); + + expect(MatrixClientPeg.replaceUsingCreds).toHaveBeenCalledWith( + expect.objectContaining({ + accessToken, + refreshToken, + }), + // no token refresh function + undefined, + ); + }); + }); }); }); From 8a47e6e1069d7e09cadbbdf4695f767687b10a1d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 15:08:17 +1300 Subject: [PATCH 47/56] test token refresher --- src/utils/oidc/TokenRefresher.ts | 10 +-- test/utils/oidc/TokenRefresher-test.ts | 96 ++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 test/utils/oidc/TokenRefresher-test.ts diff --git a/src/utils/oidc/TokenRefresher.ts b/src/utils/oidc/TokenRefresher.ts index 9d501dfb9c9..bb679c0a6d9 100644 --- a/src/utils/oidc/TokenRefresher.ts +++ b/src/utils/oidc/TokenRefresher.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { IDelegatedAuthConfig, OidcTokenRefresher } from "matrix-js-sdk/src/matrix"; +import { IDelegatedAuthConfig, OidcTokenRefresher, AccessTokens } from "matrix-js-sdk/src/matrix"; import { IdTokenClaims } from "oidc-client-ts"; import PlatformPeg from "../../PlatformPeg"; @@ -39,13 +39,7 @@ export class TokenRefresher extends OidcTokenRefresher { this.deviceId = deviceId; } - public async persistTokens({ - accessToken, - refreshToken, - }: { - accessToken: string; - refreshToken?: string | undefined; - }): Promise { + public async persistTokens({ accessToken, refreshToken }: AccessTokens): Promise { const pickleKey = (await PlatformPeg.get()?.getPickleKey(this.userId, this.deviceId)) ?? undefined; await persistAccessTokenInStorage(accessToken, pickleKey); await persistRefreshTokenInStorage(refreshToken, pickleKey); diff --git a/test/utils/oidc/TokenRefresher-test.ts b/test/utils/oidc/TokenRefresher-test.ts new file mode 100644 index 00000000000..46b33da52a8 --- /dev/null +++ b/test/utils/oidc/TokenRefresher-test.ts @@ -0,0 +1,96 @@ +/* +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 { mocked } from "jest-mock"; + +import { TokenRefresher } from "../../../src/utils/oidc/TokenRefresher"; +import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../../../src/utils/tokens/tokens"; +import { mockPlatformPeg } from "../../test-utils"; +import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; + +jest.mock("../../../src/utils/tokens/tokens", () => ({ + persistAccessTokenInStorage: jest.fn(), + persistRefreshTokenInStorage: jest.fn(), +})); + +describe("TokenRefresher", () => { + const clientId = "test-client-id"; + const issuer = "https://auth.com/"; + const redirectUri = "https://test.com"; + const deviceId = "test-device-id"; + const userId = "@alice:server.org"; + const accessToken = "test-access-token"; + const refreshToken = "test-refresh-token"; + + const authConfig = makeDelegatedAuthConfig(issuer); + const idTokenClaims = { + aud: "123", + iss: issuer, + sub: "123", + exp: 123, + iat: 456, + }; + + beforeEach(() => { + fetchMock.get(`${authConfig.issuer}.well-known/openid-configuration`, authConfig.metadata); + fetchMock.get(`${authConfig.issuer}jwks`, { + status: 200, + headers: { + "Content-Type": "application/json", + }, + keys: [], + }); + + mocked(persistAccessTokenInStorage).mockResolvedValue(undefined); + mocked(persistRefreshTokenInStorage).mockResolvedValue(undefined); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("should persist tokens with a pickle key", async () => { + const pickleKey = "test-pickle-key"; + const getPickleKey = jest.fn().mockResolvedValue(pickleKey); + mockPlatformPeg({ getPickleKey }); + + const refresher = new TokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims, userId); + + await refresher.oidcClientReady; + + await refresher.persistTokens({ accessToken, refreshToken }); + + expect(getPickleKey).toHaveBeenCalledWith(userId, deviceId); + expect(persistAccessTokenInStorage).toHaveBeenCalledWith(accessToken, pickleKey); + expect(persistRefreshTokenInStorage).toHaveBeenCalledWith(refreshToken, pickleKey); + }); + + it("should persist tokens without a pickle key", async () => { + const getPickleKey = jest.fn().mockResolvedValue(null); + mockPlatformPeg({ getPickleKey }); + + const refresher = new TokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims, userId); + + await refresher.oidcClientReady; + + await refresher.persistTokens({ accessToken, refreshToken }); + + expect(getPickleKey).toHaveBeenCalledWith(userId, deviceId); + expect(persistAccessTokenInStorage).toHaveBeenCalledWith(accessToken, undefined); + expect(persistRefreshTokenInStorage).toHaveBeenCalledWith(refreshToken, undefined); + }); +}); From a32ca16ebfee7826a517406bd2b7c08d4f2c568d Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 11 Oct 2023 16:08:25 +1300 Subject: [PATCH 48/56] Update src/utils/oidc/TokenRefresher.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/utils/oidc/TokenRefresher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/oidc/TokenRefresher.ts b/src/utils/oidc/TokenRefresher.ts index bb679c0a6d9..9f4b30de2ae 100644 --- a/src/utils/oidc/TokenRefresher.ts +++ b/src/utils/oidc/TokenRefresher.ts @@ -22,7 +22,7 @@ import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../to /** * OidcTokenRefresher that implements token persistence - * Stores tokens in the same was as login flows in Lifecycle + * Stores tokens in the same way as login flows in Lifecycle */ export class TokenRefresher extends OidcTokenRefresher { private readonly deviceId!: string; From 537047410ed28ed8ea0c6fa3543c0202a2534733 Mon Sep 17 00:00:00 2001 From: Kerry Date: Wed, 11 Oct 2023 16:14:02 +1300 Subject: [PATCH 49/56] use literal value for m.authentication Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- test/Lifecycle-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index a3c805f3478..802556f868f 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -693,7 +693,7 @@ describe("Lifecycle", () => { beforeEach(() => { // mock oidc config for oidc client initialisation mockClient.getClientWellKnown.mockReturnValue({ - [MatrixJs.M_AUTHENTICATION.stable!]: { + "m.authentication": { issuer: issuer, }, }); From 7f40f86c252851159c22e8b14a7763ad338617fb Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 11 Oct 2023 16:26:37 +1300 Subject: [PATCH 50/56] improve comments --- src/MatrixClientPeg.ts | 2 ++ src/utils/oidc/TokenRefresher.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/MatrixClientPeg.ts b/src/MatrixClientPeg.ts index 5bf41000b76..fbc54b38967 100644 --- a/src/MatrixClientPeg.ts +++ b/src/MatrixClientPeg.ts @@ -123,6 +123,8 @@ export interface IMatrixClientPeg { * homeserver / identity server URLs and active credentials * * @param {IMatrixClientCreds} creds The new credentials to use. + * @param {TokenRefreshFunction} tokenRefreshFunction OPTIONAL function used by MatrixClient to attempt token refresh + * see {@link ICreateClientOpts.tokenRefreshFunction} */ replaceUsingCreds(creds: IMatrixClientCreds, tokenRefreshFunction?: TokenRefreshFunction): void; } diff --git a/src/utils/oidc/TokenRefresher.ts b/src/utils/oidc/TokenRefresher.ts index 9f4b30de2ae..a6a0be29be7 100644 --- a/src/utils/oidc/TokenRefresher.ts +++ b/src/utils/oidc/TokenRefresher.ts @@ -21,8 +21,8 @@ import PlatformPeg from "../../PlatformPeg"; import { persistAccessTokenInStorage, persistRefreshTokenInStorage } from "../tokens/tokens"; /** - * OidcTokenRefresher that implements token persistence - * Stores tokens in the same way as login flows in Lifecycle + * OidcTokenRefresher that implements token persistence. + * Stores tokens in the same way as login flow in Lifecycle. */ export class TokenRefresher extends OidcTokenRefresher { private readonly deviceId!: string; From 866296df21e29a7d7f8b7f13880a5b502374d033 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 12:16:16 +1300 Subject: [PATCH 51/56] fix test mock, comment --- src/Lifecycle.ts | 7 ++++--- test/Lifecycle-test.ts | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 5d6afb480b0..ce1efd9ed3d 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -923,9 +923,9 @@ async function persistCredentials(credentials: IMatrixClientCreds): Promise { isVersionSupported: jest.fn().mockResolvedValue(true), getCrypto: jest.fn(), getClientWellKnown: jest.fn(), + waitForClientWellKnown: jest.fn(), getThirdpartyProtocols: jest.fn(), store: { destroy: jest.fn(), @@ -698,7 +699,7 @@ describe("Lifecycle", () => { beforeEach(() => { // mock oidc config for oidc client initialisation - mockClient.getClientWellKnown.mockReturnValue({ + mockClient.waitForClientWellKnown.mockResolvedValue({ "m.authentication": { issuer: issuer, }, From 1a232b70c6ff8b848ab598cb3b0919341bd16e29 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 13:17:41 +1300 Subject: [PATCH 52/56] typo --- src/Lifecycle.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index ce1efd9ed3d..9f2fcaa197c 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -942,7 +942,7 @@ async function doLogout(client: MatrixClient, oidcClientStore?: OidcClientStore) /** * Logs the current session out and transitions to the logged-out state - * @param oidcClientStore store instance from SDKConfig + * @param oidcClientStore store instance from SDKContext */ export function logout(oidcClientStore?: OidcClientStore): void { const client = MatrixClientPeg.get(); From fc10e642bb74080f0e7eefab34c5b0ee531b8f54 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 13:52:22 +1300 Subject: [PATCH 53/56] add sdkContext to SoftLogout, pass oidcClientStore to logout --- src/components/structures/auth/SoftLogout.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/components/structures/auth/SoftLogout.tsx b/src/components/structures/auth/SoftLogout.tsx index dede3b612b6..250c6a6a1d3 100644 --- a/src/components/structures/auth/SoftLogout.tsx +++ b/src/components/structures/auth/SoftLogout.tsx @@ -34,6 +34,7 @@ import AccessibleButton from "../../views/elements/AccessibleButton"; import Spinner from "../../views/elements/Spinner"; import AuthHeader from "../../views/auth/AuthHeader"; import AuthBody from "../../views/auth/AuthBody"; +import { SDKContext } from "../../../contexts/SDKContext"; enum LoginView { Loading, @@ -70,8 +71,13 @@ interface IState { } export default class SoftLogout extends React.Component { - public constructor(props: IProps) { - super(props); + public static contextType = SDKContext; + public context!: React.ContextType; + + public constructor(props: IProps, context: React.ContextType) { + super(props, context); + + this.context = context; this.state = { loginView: LoginView.Loading, @@ -98,7 +104,7 @@ export default class SoftLogout extends React.Component { if (!wipeData) return; logger.log("Clearing data from soft-logged-out session"); - Lifecycle.logout(); + Lifecycle.logout(this.context.oidcClientStore); }, }); }; From 1f7dd1b7ffe17eae2effd383fb375efd051dd930 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 13:54:08 +1300 Subject: [PATCH 54/56] fullstops --- src/stores/oidc/OidcClientStore.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 00fdc79ab06..368d9f23432 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -108,9 +108,9 @@ export class OidcClientStore { } /** - * Tries to initialise an OidcClient using stored clientId and oidc discovery - * Assigns this.oidcClient and accountManagement endpoint - * Logs errors and does not throw when oidc client cannot be initialised + * Tries to initialise an OidcClient using stored clientId and OIDC discovery. + * Assigns this.oidcClient and accountManagement endpoint. + * Logs errors and does not throw when oidc client cannot be initialised. * @returns promise that resolves when initialising OidcClient succeeds or fails */ private async initOidcClient(): Promise { From f1fdd13a40393bb32b6234a4f47ed12e100dd80d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 13:58:38 +1300 Subject: [PATCH 55/56] comments --- src/stores/oidc/OidcClientStore.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 368d9f23432..15d92cdc3e2 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -121,10 +121,13 @@ export class OidcClientStore { } const delegatedAuthConfig = (wellKnown && M_AUTHENTICATION.findIn(wellKnown)) ?? undefined; + try { const clientId = getStoredOidcClientId(); const { account, metadata, signingKeys } = await discoverAndValidateAuthenticationConfig( - delegatedAuthConfig || { issuer: this.authenticatedIssuer! }, + delegatedAuthConfig ?? // if HS doesn't have .well-known configured, or we can't fetch it + // fallback to the known issuer + { issuer: this.authenticatedIssuer! }, ); // if no account endpoint is configured default to the issuer this._accountManagementEndpoint = account ?? metadata.issuer; From 09b8d1360a119c8af89f3462c0458ec5222715bf Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 14:10:34 +1300 Subject: [PATCH 56/56] fussy comment formatting --- src/stores/oidc/OidcClientStore.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index 15d92cdc3e2..e4f452fbafb 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -125,9 +125,9 @@ export class OidcClientStore { try { const clientId = getStoredOidcClientId(); const { account, metadata, signingKeys } = await discoverAndValidateAuthenticationConfig( - delegatedAuthConfig ?? // if HS doesn't have .well-known configured, or we can't fetch it - // fallback to the known issuer - { issuer: this.authenticatedIssuer! }, + // if HS has valid delegated auth config in .well-known, use it + // otherwise fallback to the known issuer + delegatedAuthConfig ?? { issuer: this.authenticatedIssuer! }, ); // if no account endpoint is configured default to the issuer this._accountManagementEndpoint = account ?? metadata.issuer;