From ad01218942222e779216581eb01a2e3ff974e6e9 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 22 Jan 2025 13:48:28 +0000 Subject: [PATCH] Switch OIDC primarily to new `/auth_metadata` API (#29019) * Switch OIDC primarily to new `/auth_metadata` API Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Simplify the world Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../settings/devices/LoginWithQRSection.tsx | 3 +- .../settings/tabs/user/SessionManagerTab.tsx | 7 +-- src/stores/oidc/OidcClientStore.ts | 18 ++---- src/utils/AutoDiscoveryUtils.tsx | 4 +- src/utils/oidc/authorize.ts | 2 +- src/utils/oidc/isUserRegistrationSupported.ts | 4 +- src/utils/oidc/registerClient.ts | 4 +- test/test-utils/oidc.ts | 39 +----------- test/unit-tests/Lifecycle-test.ts | 19 ++---- .../components/structures/auth/Login-test.tsx | 14 ++--- .../structures/auth/Registration-test.tsx | 14 +++-- .../tabs/user/SessionManagerTab-test.tsx | 17 +++-- .../stores/oidc/OidcClientStore-test.ts | 63 ++++++++++--------- .../utils/AutoDiscoveryUtils-test.tsx | 14 ++--- .../utils/oidc/TokenRefresher-test.ts | 2 +- test/unit-tests/utils/oidc/authorize-test.ts | 5 +- .../utils/oidc/registerClient-test.ts | 12 ++-- 17 files changed, 92 insertions(+), 149 deletions(-) diff --git a/src/components/views/settings/devices/LoginWithQRSection.tsx b/src/components/views/settings/devices/LoginWithQRSection.tsx index 4043d28c805..3cd762c126c 100644 --- a/src/components/views/settings/devices/LoginWithQRSection.tsx +++ b/src/components/views/settings/devices/LoginWithQRSection.tsx @@ -31,8 +31,7 @@ export function shouldShowQr( ): boolean { const msc4108Supported = !!versions?.unstable_features?.["org.matrix.msc4108"]; - const deviceAuthorizationGrantSupported = - oidcClientConfig?.metadata?.grant_types_supported.includes(DEVICE_CODE_SCOPE); + const deviceAuthorizationGrantSupported = oidcClientConfig?.grant_types_supported.includes(DEVICE_CODE_SCOPE); return ( !!deviceAuthorizationGrantSupported && diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index bc4355652b6..d6a64426f14 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import React, { lazy, Suspense, useCallback, useContext, useEffect, useRef, useState } from "react"; -import { discoverAndValidateOIDCIssuerWellKnown, MatrixClient } from "matrix-js-sdk/src/matrix"; +import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import { defer } from "matrix-js-sdk/src/utils"; @@ -163,10 +163,7 @@ const SessionManagerTab: React.FC<{ const clientVersions = useAsyncMemo(() => matrixClient.getVersions(), [matrixClient]); const oidcClientConfig = useAsyncMemo(async () => { try { - const authIssuer = await matrixClient?.getAuthIssuer(); - if (authIssuer) { - return discoverAndValidateOIDCIssuerWellKnown(authIssuer.issuer); - } + return await matrixClient?.getAuthMetadata(); } catch (e) { logger.error("Failed to discover OIDC metadata", e); } diff --git a/src/stores/oidc/OidcClientStore.ts b/src/stores/oidc/OidcClientStore.ts index f814b1a6cc2..b58405c2dbc 100644 --- a/src/stores/oidc/OidcClientStore.ts +++ b/src/stores/oidc/OidcClientStore.ts @@ -50,11 +50,8 @@ export class OidcClientStore { } else { // We are not in OIDC Native mode, as we have no locally stored issuer. Check if the server delegates auth to OIDC. try { - const authIssuer = await this.matrixClient.getAuthIssuer(); - const { accountManagementEndpoint, metadata } = await discoverAndValidateOIDCIssuerWellKnown( - authIssuer.issuer, - ); - this.setAccountManagementEndpoint(accountManagementEndpoint, metadata.issuer); + const authMetadata = await this.matrixClient.getAuthMetadata(); + this.setAccountManagementEndpoint(authMetadata.account_management_uri, authMetadata.issuer); } catch (e) { console.log("Auth issuer not found", e); } @@ -153,14 +150,11 @@ export class OidcClientStore { try { const clientId = getStoredOidcClientId(); - const { accountManagementEndpoint, metadata, signingKeys } = await discoverAndValidateOIDCIssuerWellKnown( - this.authenticatedIssuer, - ); - this.setAccountManagementEndpoint(accountManagementEndpoint, metadata.issuer); + const authMetadata = await discoverAndValidateOIDCIssuerWellKnown(this.authenticatedIssuer); + this.setAccountManagementEndpoint(authMetadata.account_management_uri, authMetadata.issuer); this.oidcClient = new OidcClient({ - ...metadata, - authority: metadata.issuer, - signingKeys, + authority: authMetadata.issuer, + signingKeys: authMetadata.signingKeys ?? undefined, redirect_uri: PlatformPeg.get()!.getOidcCallbackUrl().href, client_id: clientId, }); diff --git a/src/utils/AutoDiscoveryUtils.tsx b/src/utils/AutoDiscoveryUtils.tsx index efc8f285cfd..a9dd44e2c33 100644 --- a/src/utils/AutoDiscoveryUtils.tsx +++ b/src/utils/AutoDiscoveryUtils.tsx @@ -11,7 +11,6 @@ import { AutoDiscovery, AutoDiscoveryError, ClientConfig, - discoverAndValidateOIDCIssuerWellKnown, IClientWellKnown, MatrixClient, MatrixError, @@ -293,8 +292,7 @@ export default class AutoDiscoveryUtils { let delegatedAuthenticationError: Error | undefined; try { const tempClient = new MatrixClient({ baseUrl: preferredHomeserverUrl }); - const { issuer } = await tempClient.getAuthIssuer(); - delegatedAuthentication = await discoverAndValidateOIDCIssuerWellKnown(issuer); + delegatedAuthentication = await tempClient.getAuthMetadata(); } catch (e) { if (e instanceof MatrixError && e.httpStatus === 404 && e.errcode === "M_UNRECOGNIZED") { // 404 M_UNRECOGNIZED means the server does not support OIDC diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 3f39683cdfe..339d71414fb 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -39,7 +39,7 @@ export const startOidcLogin = async ( const prompt = isRegistration ? "create" : undefined; const authorizationUrl = await generateOidcAuthorizationUrl({ - metadata: delegatedAuthConfig.metadata, + metadata: delegatedAuthConfig, redirectUri, clientId, homeserverUrl, diff --git a/src/utils/oidc/isUserRegistrationSupported.ts b/src/utils/oidc/isUserRegistrationSupported.ts index 8c91ee543b9..22d32173ad0 100644 --- a/src/utils/oidc/isUserRegistrationSupported.ts +++ b/src/utils/oidc/isUserRegistrationSupported.ts @@ -15,8 +15,6 @@ import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; * @returns whether user registration is supported */ export const isUserRegistrationSupported = (delegatedAuthConfig: OidcClientConfig): boolean => { - // The OidcMetadata type from oidc-client-ts does not include `prompt_values_supported` - // even though it is part of the OIDC spec, so cheat TS here to access it - const supportedPrompts = (delegatedAuthConfig.metadata as Record)["prompt_values_supported"]; + const supportedPrompts = delegatedAuthConfig.prompt_values_supported; return Array.isArray(supportedPrompts) && supportedPrompts?.includes("create"); }; diff --git a/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 61ec4ee3f29..72ea11ddcd2 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -40,9 +40,9 @@ export const getOidcClientId = async ( delegatedAuthConfig: OidcClientConfig, staticOidcClients?: IConfigOptions["oidc_static_clients"], ): Promise => { - const staticClientId = getStaticOidcClientId(delegatedAuthConfig.metadata.issuer, staticOidcClients); + const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); if (staticClientId) { - logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.metadata.issuer}`); + logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); return staticClientId; } return await registerOidcClient(delegatedAuthConfig, await PlatformPeg.get()!.getOidcClientMetadata()); diff --git a/test/test-utils/oidc.ts b/test/test-utils/oidc.ts index 72cdad04ef1..c8032551b0c 100644 --- a/test/test-utils/oidc.ts +++ b/test/test-utils/oidc.ts @@ -6,41 +6,4 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; -import { ValidatedIssuerMetadata } from "matrix-js-sdk/src/oidc/validate"; - -/** - * Makes a valid OidcClientConfig with minimum valid values - * @param issuer used as the base for all other urls - * @returns OidcClientConfig - */ -export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClientConfig => { - const metadata = mockOpenIdConfiguration(issuer); - - return { - accountManagementEndpoint: issuer + "account", - registrationEndpoint: metadata.registration_endpoint, - authorizationEndpoint: metadata.authorization_endpoint, - tokenEndpoint: metadata.token_endpoint, - metadata, - }; -}; - -/** - * Useful for mocking /.well-known/openid-configuration - * @param issuer used as the base for all other urls - * @returns ValidatedIssuerMetadata - */ -export const mockOpenIdConfiguration = (issuer = "https://auth.org/"): ValidatedIssuerMetadata => ({ - issuer, - revocation_endpoint: issuer + "revoke", - token_endpoint: issuer + "token", - authorization_endpoint: issuer + "auth", - registration_endpoint: issuer + "registration", - device_authorization_endpoint: issuer + "device", - jwks_uri: issuer + "jwks", - response_types_supported: ["code"], - grant_types_supported: ["authorization_code", "refresh_token"], - code_challenge_methods_supported: ["S256"], - account_management_uri: issuer + "account", -}); +export { makeDelegatedAuthConfig, mockOpenIdConfiguration } from "matrix-js-sdk/src/testing"; diff --git a/test/unit-tests/Lifecycle-test.ts b/test/unit-tests/Lifecycle-test.ts index ba5885c414c..284a3c6531e 100644 --- a/test/unit-tests/Lifecycle-test.ts +++ b/test/unit-tests/Lifecycle-test.ts @@ -749,11 +749,8 @@ describe("Lifecycle", () => { "eyJhbGciOiJSUzI1NiIsImtpZCI6Imh4ZEhXb0Y5bW4ifQ.eyJzdWIiOiIwMUhQUDJGU0JZREU5UDlFTU04REQ3V1pIUiIsImlzcyI6Imh0dHBzOi8vYXV0aC1vaWRjLmxhYi5lbGVtZW50LmRldi8iLCJpYXQiOjE3MTUwNzE5ODUsImF1dGhfdGltZSI6MTcwNzk5MDMxMiwiY19oYXNoIjoidGt5R1RhUjU5aTk3YXoyTU4yMGdidyIsImV4cCI6MTcxNTA3NTU4NSwibm9uY2UiOiJxaXhwM0hFMmVaIiwiYXVkIjoiMDFIWDk0Mlg3QTg3REgxRUs2UDRaNjI4WEciLCJhdF9oYXNoIjoiNFlFUjdPRlVKTmRTeEVHV2hJUDlnZyJ9.HxODneXvSTfWB5Vc4cf7b8GiN2gdwUuTiyVqZuupWske2HkZiJZUt5Lsxg9BW3gz28POkE0Ln17snlkmy02B_AD3DQxKOOxQCzIIARHdfFvZxgGWsMdFcVQZDW7rtXcqgj-SpVaUQ_8acsgxSrz_DF2o0O4tto0PT6wVUiw8KlBmgWTscWPeAWe-39T-8EiQ8Wi16h6oSPcz2NzOQ7eOM_S9fDkOorgcBkRGLl1nrahrPSdWJSGAeruk5mX4YxN714YThFDyEA2t9YmKpjaiSQ2tT-Xkd7tgsZqeirNs2ni9mIiFX3bRX6t2AhUNzA7MaX9ZyizKGa6go3BESO_oDg"; beforeAll(() => { - fetchMock.get( - `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, - delegatedAuthConfig.metadata, - ); - fetchMock.get(`${delegatedAuthConfig.metadata.issuer}jwks`, { + fetchMock.get(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`, delegatedAuthConfig); + fetchMock.get(`${delegatedAuthConfig.issuer}jwks`, { status: 200, headers: { "Content-Type": "application/json", @@ -772,9 +769,7 @@ describe("Lifecycle", () => { await setLoggedIn(credentials); // didn't try to initialise token refresher - expect(fetchMock).not.toHaveFetched( - `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, - ); + expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); }); it("should not try to create a token refresher without a deviceId", async () => { @@ -785,9 +780,7 @@ describe("Lifecycle", () => { }); // didn't try to initialise token refresher - expect(fetchMock).not.toHaveFetched( - `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, - ); + 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 () => { @@ -803,9 +796,7 @@ describe("Lifecycle", () => { }); // didn't try to initialise token refresher - expect(fetchMock).not.toHaveFetched( - `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, - ); + expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); }); it("should create a client with a tokenRefreshFunction", async () => { diff --git a/test/unit-tests/components/structures/auth/Login-test.tsx b/test/unit-tests/components/structures/auth/Login-test.tsx index 3b1a85b3e9b..ee3c608256a 100644 --- a/test/unit-tests/components/structures/auth/Login-test.tsx +++ b/test/unit-tests/components/structures/auth/Login-test.tsx @@ -384,7 +384,7 @@ describe("Login", function () { await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); // didn't try to register - expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint); + expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registration_endpoint); // continued with normal setup expect(mockClient.loginFlows).toHaveBeenCalled(); // normal password login rendered @@ -394,25 +394,25 @@ describe("Login", function () { it("should attempt to register oidc client", async () => { // dont mock, spy so we can check config values were correctly passed jest.spyOn(registerClientUtils, "getOidcClientId"); - fetchMock.post(delegatedAuth.registrationEndpoint!, { status: 500 }); + fetchMock.post(delegatedAuth.registration_endpoint!, { status: 500 }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); // tried to register - expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object)); + expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registration_endpoint, expect.any(Object)); // called with values from config expect(registerClientUtils.getOidcClientId).toHaveBeenCalledWith(delegatedAuth, oidcStaticClientsConfig); }); it("should fallback to normal login when client registration fails", async () => { - fetchMock.post(delegatedAuth.registrationEndpoint!, { status: 500 }); + fetchMock.post(delegatedAuth.registration_endpoint!, { status: 500 }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); // tried to register - expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registrationEndpoint, expect.any(Object)); + expect(fetchMock).toHaveBeenCalledWith(delegatedAuth.registration_endpoint, expect.any(Object)); expect(logger.error).toHaveBeenCalledWith(new Error(OidcError.DynamicRegistrationFailed)); // continued with normal setup @@ -423,7 +423,7 @@ describe("Login", function () { // short term during active development, UI will be added in next PRs it("should show continue button when oidc native flow is correctly configured", async () => { - fetchMock.post(delegatedAuth.registrationEndpoint!, { client_id: "abc123" }); + fetchMock.post(delegatedAuth.registration_endpoint!, { client_id: "abc123" }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); @@ -455,7 +455,7 @@ describe("Login", function () { await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); // didn't try to register - expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registrationEndpoint); + expect(fetchMock).not.toHaveBeenCalledWith(delegatedAuth.registration_endpoint); // continued with normal setup expect(mockClient.loginFlows).toHaveBeenCalled(); // oidc-aware 'continue' button displayed diff --git a/test/unit-tests/components/structures/auth/Registration-test.tsx b/test/unit-tests/components/structures/auth/Registration-test.tsx index 82ac0b666cd..a8c170eac7f 100644 --- a/test/unit-tests/components/structures/auth/Registration-test.tsx +++ b/test/unit-tests/components/structures/auth/Registration-test.tsx @@ -158,24 +158,26 @@ describe("Registration", function () { describe("when delegated authentication is configured and enabled", () => { const authConfig = makeDelegatedAuthConfig(); const clientId = "test-client-id"; - // @ts-ignore - authConfig.metadata["prompt_values_supported"] = ["create"]; + authConfig.prompt_values_supported = ["create"]; beforeEach(() => { // mock a statically registered client to avoid dynamic registration SdkConfig.put({ oidc_static_clients: { - [authConfig.metadata.issuer]: { + [authConfig.issuer]: { client_id: clientId, }, }, }); fetchMock.get(`${defaultHsUrl}/_matrix/client/unstable/org.matrix.msc2965/auth_issuer`, { - issuer: authConfig.metadata.issuer, + issuer: authConfig.issuer, }); - fetchMock.get("https://auth.org/.well-known/openid-configuration", authConfig.metadata); - fetchMock.get(authConfig.metadata.jwks_uri!, { keys: [] }); + fetchMock.get("https://auth.org/.well-known/openid-configuration", { + ...authConfig, + signingKeys: undefined, + }); + fetchMock.get(authConfig.jwks_uri!, { keys: [] }); }); it("should display oidc-native continue button", async () => { diff --git a/test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx index 030f769de21..f334c6b28f8 100644 --- a/test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx +++ b/test/unit-tests/components/views/settings/tabs/user/SessionManagerTab-test.tsx @@ -57,7 +57,7 @@ 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"; -import { mockOpenIdConfiguration } from "../../../../../../test-utils/oidc"; +import { makeDelegatedAuthConfig } from "../../../../../../test-utils/oidc"; import MatrixClientContext from "../../../../../../../src/contexts/MatrixClientContext"; mockPlatformPeg(); @@ -215,7 +215,7 @@ describe("", () => { getPushers: jest.fn(), setPusher: jest.fn(), setLocalNotificationSettings: jest.fn(), - getAuthIssuer: jest.fn().mockReturnValue(new Promise(() => {})), + getAuthMetadata: jest.fn().mockRejectedValue(new MatrixError({ errcode: "M_UNRECOGNIZED" }, 404)), }); jest.clearAllMocks(); jest.spyOn(logger, "error").mockRestore(); @@ -1615,7 +1615,6 @@ describe("", () => { describe("MSC4108 QR code login", () => { const settingsValueSpy = jest.spyOn(SettingsStore, "getValue"); const issuer = "https://issuer.org"; - const openIdConfiguration = mockOpenIdConfiguration(issuer); beforeEach(() => { settingsValueSpy.mockClear().mockReturnValue(true); @@ -1631,16 +1630,16 @@ describe("", () => { enabled: true, }, }); - mockClient.getAuthIssuer.mockResolvedValue({ issuer }); - mockCrypto.exportSecretsBundle = jest.fn(); - fetchMock.mock(`${issuer}/.well-known/openid-configuration`, { - ...openIdConfiguration, + const delegatedAuthConfig = makeDelegatedAuthConfig(issuer); + mockClient.getAuthMetadata.mockResolvedValue({ + ...delegatedAuthConfig, grant_types_supported: [ - ...openIdConfiguration.grant_types_supported, + ...delegatedAuthConfig.grant_types_supported, "urn:ietf:params:oauth:grant-type:device_code", ], }); - fetchMock.mock(openIdConfiguration.jwks_uri!, { + mockCrypto.exportSecretsBundle = jest.fn(); + fetchMock.mock(delegatedAuthConfig.jwks_uri!, { status: 200, headers: { "Content-Type": "application/json", diff --git a/test/unit-tests/stores/oidc/OidcClientStore-test.ts b/test/unit-tests/stores/oidc/OidcClientStore-test.ts index 8de1d9dad10..164f90f5319 100644 --- a/test/unit-tests/stores/oidc/OidcClientStore-test.ts +++ b/test/unit-tests/stores/oidc/OidcClientStore-test.ts @@ -15,7 +15,7 @@ import { OidcError } from "matrix-js-sdk/src/oidc/error"; import { OidcClientStore } from "../../../../src/stores/oidc/OidcClientStore"; import { flushPromises, getMockClientWithEventEmitter, mockPlatformPeg } from "../../../test-utils"; -import { mockOpenIdConfiguration } from "../../../test-utils/oidc"; +import { makeDelegatedAuthConfig } from "../../../test-utils/oidc"; jest.mock("matrix-js-sdk/src/matrix", () => ({ ...jest.requireActual("matrix-js-sdk/src/matrix"), @@ -24,28 +24,30 @@ jest.mock("matrix-js-sdk/src/matrix", () => ({ describe("OidcClientStore", () => { const clientId = "test-client-id"; - const metadata = mockOpenIdConfiguration(); - const account = metadata.issuer + "account"; + const authConfig = makeDelegatedAuthConfig(); + const account = authConfig.issuer + "account"; const mockClient = getMockClientWithEventEmitter({ - getAuthIssuer: jest.fn(), + getAuthMetadata: jest.fn(), }); beforeEach(() => { localStorage.clear(); localStorage.setItem("mx_oidc_client_id", clientId); - localStorage.setItem("mx_oidc_token_issuer", metadata.issuer); - - mocked(discoverAndValidateOIDCIssuerWellKnown).mockClear().mockResolvedValue({ - metadata, - accountManagementEndpoint: account, - authorizationEndpoint: "authorization-endpoint", - tokenEndpoint: "token-endpoint", - }); + localStorage.setItem("mx_oidc_token_issuer", authConfig.issuer); + + mocked(discoverAndValidateOIDCIssuerWellKnown) + .mockClear() + .mockResolvedValue({ + ...authConfig, + account_management_uri: account, + authorization_endpoint: "authorization-endpoint", + token_endpoint: "token-endpoint", + }); jest.spyOn(logger, "error").mockClear(); - fetchMock.get(`${metadata.issuer}.well-known/openid-configuration`, metadata); - fetchMock.get(`${metadata.issuer}jwks`, { keys: [] }); + fetchMock.get(`${authConfig.issuer}.well-known/openid-configuration`, authConfig); + fetchMock.get(`${authConfig.issuer}jwks`, { keys: [] }); mockPlatformPeg(); }); @@ -116,7 +118,7 @@ describe("OidcClientStore", () => { const client = await store.getOidcClient(); expect(client?.settings.client_id).toEqual(clientId); - expect(client?.settings.authority).toEqual(metadata.issuer); + expect(client?.settings.authority).toEqual(authConfig.issuer); }); it("should set account management endpoint when configured", async () => { @@ -129,17 +131,19 @@ describe("OidcClientStore", () => { }); it("should set account management endpoint to issuer when not configured", async () => { - mocked(discoverAndValidateOIDCIssuerWellKnown).mockClear().mockResolvedValue({ - metadata, - accountManagementEndpoint: undefined, - authorizationEndpoint: "authorization-endpoint", - tokenEndpoint: "token-endpoint", - }); + mocked(discoverAndValidateOIDCIssuerWellKnown) + .mockClear() + .mockResolvedValue({ + ...authConfig, + account_management_uri: undefined, + authorization_endpoint: "authorization-endpoint", + token_endpoint: "token-endpoint", + }); const store = new OidcClientStore(mockClient); await store.readyPromise; - expect(store.accountManagementEndpoint).toEqual(metadata.issuer); + expect(store.accountManagementEndpoint).toEqual(authConfig.issuer); }); it("should reuse initialised oidc client", async () => { @@ -175,7 +179,7 @@ describe("OidcClientStore", () => { fetchMock.resetHistory(); fetchMock.post( - metadata.revocation_endpoint, + authConfig.revocation_endpoint, { status: 200, }, @@ -197,7 +201,7 @@ describe("OidcClientStore", () => { await store.revokeTokens(accessToken, refreshToken); - expect(fetchMock).toHaveFetchedTimes(2, metadata.revocation_endpoint); + expect(fetchMock).toHaveFetchedTimes(2, authConfig.revocation_endpoint); expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(accessToken, "access_token"); expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(refreshToken, "refresh_token"); }); @@ -206,14 +210,14 @@ describe("OidcClientStore", () => { // fail once, then succeed fetchMock .postOnce( - metadata.revocation_endpoint, + authConfig.revocation_endpoint, { status: 404, }, { overwriteRoutes: true, sendAsJson: true }, ) .post( - metadata.revocation_endpoint, + authConfig.revocation_endpoint, { status: 200, }, @@ -226,7 +230,7 @@ describe("OidcClientStore", () => { "Failed to revoke tokens", ); - expect(fetchMock).toHaveFetchedTimes(2, metadata.revocation_endpoint); + expect(fetchMock).toHaveFetchedTimes(2, authConfig.revocation_endpoint); expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(accessToken, "access_token"); }); }); @@ -237,7 +241,10 @@ describe("OidcClientStore", () => { }); it("should resolve account management endpoint", async () => { - mockClient.getAuthIssuer.mockResolvedValue({ issuer: metadata.issuer }); + mockClient.getAuthMetadata.mockResolvedValue({ + ...authConfig, + account_management_uri: account, + }); const store = new OidcClientStore(mockClient); await store.readyPromise; expect(store.accountManagementEndpoint).toBe(account); diff --git a/test/unit-tests/utils/AutoDiscoveryUtils-test.tsx b/test/unit-tests/utils/AutoDiscoveryUtils-test.tsx index a49707e3101..575dc885a00 100644 --- a/test/unit-tests/utils/AutoDiscoveryUtils-test.tsx +++ b/test/unit-tests/utils/AutoDiscoveryUtils-test.tsx @@ -355,21 +355,19 @@ describe("AutoDiscoveryUtils", () => { hsNameIsDifferent: true, hsName: serverName, delegatedAuthentication: expect.objectContaining({ - accountManagementActionsSupported: [ + issuer, + account_management_actions_supported: [ "org.matrix.profile", "org.matrix.sessions_list", "org.matrix.session_view", "org.matrix.session_end", "org.matrix.cross_signing_reset", ], - accountManagementEndpoint: "https://auth.matrix.org/account/", - authorizationEndpoint: "https://auth.matrix.org/auth", - metadata: expect.objectContaining({ - issuer, - }), - registrationEndpoint: "https://auth.matrix.org/registration", + account_management_uri: "https://auth.matrix.org/account/", + authorization_endpoint: "https://auth.matrix.org/auth", + registration_endpoint: "https://auth.matrix.org/registration", signingKeys: [], - tokenEndpoint: "https://auth.matrix.org/token", + token_endpoint: "https://auth.matrix.org/token", }), warning: null, }); diff --git a/test/unit-tests/utils/oidc/TokenRefresher-test.ts b/test/unit-tests/utils/oidc/TokenRefresher-test.ts index 643f61faacb..66f0e800e02 100644 --- a/test/unit-tests/utils/oidc/TokenRefresher-test.ts +++ b/test/unit-tests/utils/oidc/TokenRefresher-test.ts @@ -38,7 +38,7 @@ describe("TokenRefresher", () => { }; beforeEach(() => { - fetchMock.get(`${issuer}.well-known/openid-configuration`, authConfig.metadata); + fetchMock.get(`${issuer}.well-known/openid-configuration`, authConfig); fetchMock.get(`${issuer}jwks`, { status: 200, headers: { diff --git a/test/unit-tests/utils/oidc/authorize-test.ts b/test/unit-tests/utils/oidc/authorize-test.ts index bea9724fce3..af2fd432bc0 100644 --- a/test/unit-tests/utils/oidc/authorize-test.ts +++ b/test/unit-tests/utils/oidc/authorize-test.ts @@ -61,10 +61,7 @@ describe("OIDC authorization", () => { }); beforeAll(() => { - fetchMock.get( - `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, - delegatedAuthConfig.metadata, - ); + fetchMock.get(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`, delegatedAuthConfig); }); afterAll(() => { diff --git a/test/unit-tests/utils/oidc/registerClient-test.ts b/test/unit-tests/utils/oidc/registerClient-test.ts index 0c6a41bc36d..197beeb2bf8 100644 --- a/test/unit-tests/utils/oidc/registerClient-test.ts +++ b/test/unit-tests/utils/oidc/registerClient-test.ts @@ -58,7 +58,7 @@ describe("getOidcClientId()", () => { const authConfigWithoutRegistration: OidcClientConfig = makeDelegatedAuthConfig( "https://issuerWithoutStaticClientId.org/", ); - authConfigWithoutRegistration.registrationEndpoint = undefined; + authConfigWithoutRegistration.registration_endpoint = undefined; await expect(getOidcClientId(authConfigWithoutRegistration, staticOidcClients)).rejects.toThrow( OidcError.DynamicRegistrationNotSupported, ); @@ -69,7 +69,7 @@ describe("getOidcClientId()", () => { it("should handle when staticOidcClients object is falsy", async () => { const authConfigWithoutRegistration: OidcClientConfig = { ...delegatedAuthConfig, - registrationEndpoint: undefined, + registration_endpoint: undefined, }; await expect(getOidcClientId(authConfigWithoutRegistration)).rejects.toThrow( OidcError.DynamicRegistrationNotSupported, @@ -79,14 +79,14 @@ describe("getOidcClientId()", () => { }); it("should make correct request to register client", async () => { - fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, { + fetchMockJest.post(delegatedAuthConfig.registration_endpoint!, { status: 200, body: JSON.stringify({ client_id: dynamicClientId }), }); expect(await getOidcClientId(delegatedAuthConfig)).toEqual(dynamicClientId); // didn't try to register expect(fetchMockJest).toHaveBeenCalledWith( - delegatedAuthConfig.registrationEndpoint!, + delegatedAuthConfig.registration_endpoint!, expect.objectContaining({ headers: { "Accept": "application/json", @@ -111,14 +111,14 @@ describe("getOidcClientId()", () => { }); it("should throw when registration request fails", async () => { - fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, { + fetchMockJest.post(delegatedAuthConfig.registration_endpoint!, { status: 500, }); await expect(getOidcClientId(delegatedAuthConfig)).rejects.toThrow(OidcError.DynamicRegistrationFailed); }); it("should throw when registration response is invalid", async () => { - fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, { + fetchMockJest.post(delegatedAuthConfig.registration_endpoint!, { status: 200, // no clientId in response body: "{}",