From 0b0d77cbcc13e9919614e6bb71e51040c37935c4 Mon Sep 17 00:00:00 2001 From: Kerry Date: Fri, 21 Jul 2023 09:30:19 +1200 Subject: [PATCH] OIDC: Persist details in session storage, create store (#11302) * utils to persist clientId and issuer after oidc authentication * add dep oidc-client-ts * persist issuer and clientId after successful oidc auth * add OidcClientStore * comments and tidy * format --- package.json | 1 + src/Lifecycle.ts | 7 +- src/contexts/SDKContext.ts | 14 ++ src/stores/oidc/OidcClientStore.ts | 88 +++++++++ src/utils/oidc/authorize.ts | 7 +- src/utils/oidc/persistOidcSettings.ts | 51 +++++ .../components/structures/MatrixChat-test.tsx | 18 ++ test/contexts/SdkContext-test.ts | 14 ++ test/stores/oidc/OidcClientStore-test.ts | 183 ++++++++++++++++++ test/utils/oidc/authorize-test.ts | 2 + test/utils/oidc/persistOidcSettings-test.ts | 63 ++++++ 11 files changed, 446 insertions(+), 2 deletions(-) create mode 100644 src/stores/oidc/OidcClientStore.ts create mode 100644 src/utils/oidc/persistOidcSettings.ts create mode 100644 test/stores/oidc/OidcClientStore-test.ts create mode 100644 test/utils/oidc/persistOidcSettings-test.ts diff --git a/package.json b/package.json index 922aa94e86..e7c22c6c81 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", diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 1c5514da77..315b3a3fc7 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -66,6 +66,7 @@ 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"; const HOMESERVER_URL_KEY = "mx_hs_url"; const ID_SERVER_URL_KEY = "mx_is_url"; @@ -215,7 +216,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 +237,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); diff --git a/src/contexts/SDKContext.ts b/src/contexts/SDKContext.ts index bb10d0df72..6c64f79292 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 0000000000..9ca96d9b63 --- /dev/null +++ b/src/stores/oidc/OidcClientStore.ts @@ -0,0 +1,88 @@ +/* +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 + * Stores information about configured OIDC provider + */ +export class OidcClientStore { + private oidcClient?: OidcClient; + private initialisingOidcClientPromise: Promise | undefined; + private authenticatedIssuer?: string; + private _accountManagementEndpoint?: string; + + public constructor(private readonly matrixClient: MatrixClient) { + 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; + } + + 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/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 705278c63d..5a11a7144b 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/src/utils/oidc/persistOidcSettings.ts b/src/utils/oidc/persistOidcSettings.ts new file mode 100644 index 0000000000..68f3ffcb16 --- /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/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 9b1ea991c7..325dfb7a86 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/contexts/SdkContext-test.ts b/test/contexts/SdkContext-test.ts index a6d1c65624..af14294e41 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 0000000000..9fe8a9d6a8 --- /dev/null +++ b/test/stores/oidc/OidcClientStore-test.ts @@ -0,0 +1,183 @@ +/* +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 account = metadata.issuer + "account"; + 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, + account, + issuer: metadata.issuer, + }); + mockClient.getClientWellKnown.mockReturnValue({ + [M_AUTHENTICATION.stable!]: { + issuer: metadata.issuer, + account, + }, + }); + 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").mockImplementation((key) => + key === "mx_oidc_token_issuer" ? metadata.issuer : 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 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); + + // @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); + }); + }); +}); diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 75720f724f..a531945a26 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, }); }); }); diff --git a/test/utils/oidc/persistOidcSettings-test.ts b/test/utils/oidc/persistOidcSettings-test.ts new file mode 100644 index 0000000000..4db5c4e75c --- /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"); + }); + }); +});