diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 7842ead8c4..980fd9ae1a 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -795,7 +795,7 @@ async function createOidcTokenRefresher(credentials: IMatrixClientCreds): Promis throw new Error("Expected deviceId in user credentials."); } const tokenRefresher = new TokenRefresher( - { issuer: tokenIssuer }, + tokenIssuer, clientId, redirectUri, deviceId, diff --git a/src/components/views/dialogs/ServerPickerDialog.tsx b/src/components/views/dialogs/ServerPickerDialog.tsx index ee8a3751da..d0597f5284 100644 --- a/src/components/views/dialogs/ServerPickerDialog.tsx +++ b/src/components/views/dialogs/ServerPickerDialog.tsx @@ -80,9 +80,6 @@ export default class ServerPickerDialog extends React.PureComponent({ deriveData: async ({ value }): Promise<{ error?: string }> => { let hsUrl = (value ?? "").trim(); // trim to account for random whitespace @@ -91,7 +88,10 @@ export default class ServerPickerDialog extends React.PureComponent { - const wellKnown = await this.matrixClient.waitForClientWellKnown(); - if (!wellKnown && !this.authenticatedIssuer) { + if (!this.authenticatedIssuer) { logger.error("Cannot initialise OIDC client without issuer."); return; } - const delegatedAuthConfig = - (wellKnown && M_AUTHENTICATION.findIn(wellKnown)) ?? undefined; try { const clientId = getStoredOidcClientId(); - const { account, metadata, signingKeys } = await discoverAndValidateAuthenticationConfig( - // if HS has valid delegated auth config in .well-known, use it - // otherwise fallback to the known issuer - delegatedAuthConfig ?? { issuer: this.authenticatedIssuer! }, + const { accountManagementEndpoint, metadata, signingKeys } = await discoverAndValidateOIDCIssuerWellKnown( + this.authenticatedIssuer, ); // if no account endpoint is configured default to the issuer - this._accountManagementEndpoint = account ?? metadata.issuer; + this._accountManagementEndpoint = accountManagementEndpoint ?? metadata.issuer; this.oidcClient = new OidcClient({ ...metadata, authority: metadata.issuer, diff --git a/src/utils/AutoDiscoveryUtils.tsx b/src/utils/AutoDiscoveryUtils.tsx index cb91c53b0f..c1f2a7a7d0 100644 --- a/src/utils/AutoDiscoveryUtils.tsx +++ b/src/utils/AutoDiscoveryUtils.tsx @@ -19,9 +19,11 @@ import { AutoDiscovery, AutoDiscoveryError, ClientConfig, - OidcClientConfig, - M_AUTHENTICATION, + discoverAndValidateOIDCIssuerWellKnown, IClientWellKnown, + MatrixClient, + MatrixError, + OidcClientConfig, } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; @@ -217,12 +219,12 @@ export default class AutoDiscoveryUtils { * @param {boolean} isSynthetic If true, then the discoveryResult was synthesised locally. * @returns {Promise} Resolves to the validated configuration. */ - public static buildValidatedConfigFromDiscovery( + public static async buildValidatedConfigFromDiscovery( serverName?: string, discoveryResult?: ClientConfig, syntaxOnly = false, isSynthetic = false, - ): ValidatedServerConfig { + ): Promise { if (!discoveryResult?.["m.homeserver"]) { // This shouldn't happen without major misconfiguration, so we'll log a bit of information // in the log so we can find this bit of code but otherwise tell the user "it broke". @@ -293,26 +295,20 @@ export default class AutoDiscoveryUtils { throw new UserFriendlyError("auth|autodiscovery_unexpected_error_hs"); } + // This isn't inherently auto-discovery but used to be in an earlier incarnation of the MSC, + // and shuttling the data together makes a lot of sense let delegatedAuthentication: OidcClientConfig | undefined; - if (discoveryResult[M_AUTHENTICATION.stable!]?.state === AutoDiscovery.SUCCESS) { - const { - authorizationEndpoint, - registrationEndpoint, - tokenEndpoint, - account, - issuer, - metadata, - signingKeys, - } = discoveryResult[M_AUTHENTICATION.stable!] as OidcClientConfig; - delegatedAuthentication = Object.freeze({ - authorizationEndpoint, - registrationEndpoint, - tokenEndpoint, - account, - issuer, - metadata, - signingKeys, - }); + let delegatedAuthenticationError: Error | undefined; + try { + const tempClient = new MatrixClient({ baseUrl: preferredHomeserverUrl }); + const { issuer } = await tempClient.getAuthIssuer(); + delegatedAuthentication = await discoverAndValidateOIDCIssuerWellKnown(issuer); + } catch (e) { + if (e instanceof MatrixError && e.httpStatus === 404 && e.errcode === "M_UNRECOGNIZED") { + // 404 M_UNRECOGNIZED means the server does not support OIDC + } else { + delegatedAuthenticationError = e as Error; + } } return { @@ -321,7 +317,7 @@ export default class AutoDiscoveryUtils { hsNameIsDifferent: url.hostname !== preferredHomeserverName, isUrl: preferredIdentityUrl, isDefault: false, - warning: hsResult.error, + warning: hsResult.error ?? delegatedAuthenticationError ?? null, isNameResolvable: !isSynthetic, delegatedAuthentication, } as ValidatedServerConfig; diff --git a/src/utils/ValidatedServerConfig.ts b/src/utils/ValidatedServerConfig.ts index 14a28b058d..cf49ebadda 100644 --- a/src/utils/ValidatedServerConfig.ts +++ b/src/utils/ValidatedServerConfig.ts @@ -14,10 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { OidcClientConfig, IDelegatedAuthConfig } from "matrix-js-sdk/src/matrix"; -import { ValidatedIssuerConfig } from "matrix-js-sdk/src/oidc/validate"; - -export type ValidatedDelegatedAuthConfig = IDelegatedAuthConfig & ValidatedIssuerConfig; +import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; export interface ValidatedServerConfig { hsUrl: string; @@ -34,9 +31,9 @@ export interface ValidatedServerConfig { /** * Config related to delegated authentication - * Included when delegated auth is configured and valid, otherwise undefined - * From homeserver .well-known m.authentication, and issuer's .well-known/openid-configuration - * Used for OIDC native flow authentication + * Included when delegated auth is configured and valid, otherwise undefined. + * From issuer's .well-known/openid-configuration. + * Used for OIDC native flow authentication. */ delegatedAuthentication?: OidcClientConfig; } diff --git a/src/utils/oidc/TokenRefresher.ts b/src/utils/oidc/TokenRefresher.ts index a6a0be29be..1297a2cb60 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, AccessTokens } from "matrix-js-sdk/src/matrix"; +import { OidcTokenRefresher, AccessTokens } from "matrix-js-sdk/src/matrix"; import { IdTokenClaims } from "oidc-client-ts"; import PlatformPeg from "../../PlatformPeg"; @@ -28,14 +28,14 @@ export class TokenRefresher extends OidcTokenRefresher { private readonly deviceId!: string; public constructor( - authConfig: IDelegatedAuthConfig, + issuer: string, clientId: string, redirectUri: string, deviceId: string, idTokenClaims: IdTokenClaims, private readonly userId: string, ) { - super(authConfig, clientId, deviceId, redirectUri, idTokenClaims); + super(issuer, clientId, deviceId, redirectUri, idTokenClaims); this.deviceId = deviceId; } diff --git a/src/utils/oidc/getDelegatedAuthAccountUrl.ts b/src/utils/oidc/getDelegatedAuthAccountUrl.ts deleted file mode 100644 index cfb61cb443..0000000000 --- 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/src/utils/oidc/registerClient.ts b/src/utils/oidc/registerClient.ts index 9f112293b6..f554e62e4e 100644 --- a/src/utils/oidc/registerClient.ts +++ b/src/utils/oidc/registerClient.ts @@ -15,10 +15,9 @@ limitations under the License. */ import { logger } from "matrix-js-sdk/src/logger"; -import { registerOidcClient } from "matrix-js-sdk/src/oidc/register"; +import { registerOidcClient, OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { IConfigOptions } from "../../IConfigOptions"; -import { ValidatedDelegatedAuthConfig } from "../ValidatedServerConfig"; import PlatformPeg from "../../PlatformPeg"; /** @@ -46,12 +45,12 @@ const getStaticOidcClientId = ( * @throws if no clientId is found */ export const getOidcClientId = async ( - delegatedAuthConfig: ValidatedDelegatedAuthConfig, + delegatedAuthConfig: OidcClientConfig, staticOidcClients?: IConfigOptions["oidc_static_clients"], ): Promise => { - const staticClientId = getStaticOidcClientId(delegatedAuthConfig.issuer, staticOidcClients); + const staticClientId = getStaticOidcClientId(delegatedAuthConfig.metadata.issuer, staticOidcClients); if (staticClientId) { - logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.issuer}`); + logger.debug(`Using static clientId for issuer ${delegatedAuthConfig.metadata.issuer}`); return staticClientId; } return await registerOidcClient(delegatedAuthConfig, await PlatformPeg.get()!.getOidcClientMetadata()); diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 2f48ba4bdb..9c998734bf 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -683,10 +683,10 @@ describe("Lifecycle", () => { beforeAll(() => { fetchMock.get( - `${delegatedAuthConfig.issuer}.well-known/openid-configuration`, + `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, delegatedAuthConfig.metadata, ); - fetchMock.get(`${delegatedAuthConfig.issuer}jwks`, { + fetchMock.get(`${delegatedAuthConfig.metadata.issuer}jwks`, { status: 200, headers: { "Content-Type": "application/json", @@ -696,12 +696,6 @@ describe("Lifecycle", () => { }); beforeEach(() => { - // mock oidc config for oidc client initialisation - mockClient.waitForClientWellKnown.mockResolvedValue({ - "m.authentication": { - issuer: issuer, - }, - }); initSessionStorageMock(); // set values in session storage as they would be after a successful oidc authentication persistOidcAuthenticatedSettings(clientId, issuer, idTokenClaims); @@ -711,7 +705,9 @@ describe("Lifecycle", () => { await setLoggedIn(credentials); // didn't try to initialise token refresher - expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); + expect(fetchMock).not.toHaveFetched( + `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, + ); }); it("should not try to create a token refresher without a deviceId", async () => { @@ -722,7 +718,9 @@ describe("Lifecycle", () => { }); // didn't try to initialise token refresher - expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); + expect(fetchMock).not.toHaveFetched( + `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, + ); }); it("should not try to create a token refresher without an issuer in session storage", async () => { @@ -738,7 +736,9 @@ describe("Lifecycle", () => { }); // didn't try to initialise token refresher - expect(fetchMock).not.toHaveFetched(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`); + expect(fetchMock).not.toHaveFetched( + `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, + ); }); it("should create a client with a tokenRefreshFunction", async () => { diff --git a/test/components/structures/MatrixChat-test.tsx b/test/components/structures/MatrixChat-test.tsx index 441de19df9..63eec01ae3 100644 --- a/test/components/structures/MatrixChat-test.tsx +++ b/test/components/structures/MatrixChat-test.tsx @@ -211,6 +211,11 @@ describe("", () => { unstable_features: {}, versions: SERVER_SUPPORTED_MATRIX_VERSIONS, }); + fetchMock.catch({ + status: 404, + body: '{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}', + headers: { "content-type": "application/json" }, + }); jest.spyOn(StorageManager, "idbLoad").mockReset(); jest.spyOn(StorageManager, "idbSave").mockResolvedValue(undefined); diff --git a/test/components/structures/auth/Registration-test.tsx b/test/components/structures/auth/Registration-test.tsx index 8a84bf17a4..6fdeb79e15 100644 --- a/test/components/structures/auth/Registration-test.tsx +++ b/test/components/structures/auth/Registration-test.tsx @@ -37,7 +37,6 @@ jest.mock("matrix-js-sdk/src/matrix", () => ({ ...jest.requireActual("matrix-js-sdk/src/matrix"), createClient: jest.fn(), })); -jest.useFakeTimers(); /** The matrix versions our mock server claims to support */ const SERVER_SUPPORTED_MATRIX_VERSIONS = ["v1.1", "v1.5", "v1.6", "v1.8", "v1.9"]; @@ -160,11 +159,17 @@ describe("Registration", function () { // mock a statically registered client to avoid dynamic registration SdkConfig.put({ oidc_static_clients: { - [authConfig.issuer]: { + [authConfig.metadata.issuer]: { client_id: clientId, }, }, }); + + fetchMock.get(`${defaultHsUrl}/_matrix/client/unstable/org.matrix.msc2965/auth_issuer`, { + issuer: authConfig.metadata.issuer, + }); + fetchMock.get("https://auth.org/.well-known/openid-configuration", authConfig.metadata); + fetchMock.get(authConfig.metadata.jwks_uri!, { keys: [] }); }); describe("when oidc native flow is not enabled in settings", () => { @@ -192,14 +197,14 @@ describe("Registration", function () { // no form expect(container.querySelector("form")).toBeFalsy(); - expect(screen.getByText("Continue")).toBeTruthy(); + expect(await screen.findByText("Continue")).toBeTruthy(); }); it("should start OIDC login flow as registration on button click", async () => { getComponent(defaultHsUrl, defaultIsUrl, authConfig); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); - fireEvent.click(screen.getByText("Continue")); + fireEvent.click(await screen.findByText("Continue")); expect(startOidcLogin).toHaveBeenCalledWith( authConfig, diff --git a/test/components/views/dialogs/ServerPickerDialog-test.tsx b/test/components/views/dialogs/ServerPickerDialog-test.tsx index 02930c9581..06fbca9550 100644 --- a/test/components/views/dialogs/ServerPickerDialog-test.tsx +++ b/test/components/views/dialogs/ServerPickerDialog-test.tsx @@ -64,6 +64,11 @@ describe("", () => { }); fetchMock.resetHistory(); + fetchMock.catch({ + status: 404, + body: '{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}', + headers: { "content-type": "application/json" }, + }); }); it("should render dialog", () => { diff --git a/test/stores/oidc/OidcClientStore-test.ts b/test/stores/oidc/OidcClientStore-test.ts index 01798a65e6..8f53b4ae9c 100644 --- a/test/stores/oidc/OidcClientStore-test.ts +++ b/test/stores/oidc/OidcClientStore-test.ts @@ -17,17 +17,17 @@ 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/matrix"; import { logger } from "matrix-js-sdk/src/logger"; -import { discoverAndValidateAuthenticationConfig } from "matrix-js-sdk/src/oidc/discovery"; +import { discoverAndValidateOIDCIssuerWellKnown } from "matrix-js-sdk/src/matrix"; 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"; -jest.mock("matrix-js-sdk/src/oidc/discovery", () => ({ - discoverAndValidateAuthenticationConfig: jest.fn(), +jest.mock("matrix-js-sdk/src/matrix", () => ({ + ...jest.requireActual("matrix-js-sdk/src/matrix"), + discoverAndValidateOIDCIssuerWellKnown: jest.fn(), })); describe("OidcClientStore", () => { @@ -36,7 +36,7 @@ describe("OidcClientStore", () => { const account = metadata.issuer + "account"; const mockClient = getMockClientWithEventEmitter({ - waitForClientWellKnown: jest.fn().mockResolvedValue({}), + getAuthIssuer: jest.fn(), }); beforeEach(() => { @@ -44,20 +44,16 @@ describe("OidcClientStore", () => { localStorage.setItem("mx_oidc_client_id", clientId); localStorage.setItem("mx_oidc_token_issuer", metadata.issuer); - mocked(discoverAndValidateAuthenticationConfig).mockClear().mockResolvedValue({ + mocked(discoverAndValidateOIDCIssuerWellKnown).mockClear().mockResolvedValue({ metadata, - account, - issuer: metadata.issuer, - }); - mockClient.waitForClientWellKnown.mockResolvedValue({ - [M_AUTHENTICATION.stable!]: { - issuer: metadata.issuer, - account, - }, + accountManagementEndpoint: account, + authorizationEndpoint: "authorization-endpoint", + tokenEndpoint: "token-endpoint", }); jest.spyOn(logger, "error").mockClear(); fetchMock.get(`${metadata.issuer}.well-known/openid-configuration`, metadata); + fetchMock.get(`${metadata.issuer}jwks`, { keys: [] }); mockPlatformPeg(); }); @@ -78,7 +74,6 @@ describe("OidcClientStore", () => { describe("initialising oidcClient", () => { it("should initialise oidc client from constructor", () => { - mockClient.waitForClientWellKnown.mockResolvedValue(undefined as any); const store = new OidcClientStore(mockClient); // started initialising @@ -87,7 +82,6 @@ describe("OidcClientStore", () => { }); 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); // successfully created oidc client @@ -109,10 +103,10 @@ describe("OidcClientStore", () => { }); it("should log and return when discovery and validation fails", async () => { - mocked(discoverAndValidateAuthenticationConfig).mockRejectedValue(new Error(OidcError.OpSupport)); + mocked(discoverAndValidateOIDCIssuerWellKnown).mockRejectedValue(new Error(OidcError.OpSupport)); const store = new OidcClientStore(mockClient); - await flushPromises(); + await store.readyPromise; expect(logger.error).toHaveBeenCalledWith( "Failed to initialise OidcClientStore", @@ -143,15 +137,15 @@ describe("OidcClientStore", () => { }); it("should set account management endpoint to issuer when not configured", async () => { - mocked(discoverAndValidateAuthenticationConfig).mockClear().mockResolvedValue({ + mocked(discoverAndValidateOIDCIssuerWellKnown).mockClear().mockResolvedValue({ metadata, - account: undefined, - issuer: metadata.issuer, + accountManagementEndpoint: undefined, + authorizationEndpoint: "authorization-endpoint", + tokenEndpoint: "token-endpoint", }); const store = new OidcClientStore(mockClient); - // @ts-ignore private property - await store.getOidcClient(); + await store.readyPromise; expect(store.accountManagementEndpoint).toEqual(metadata.issuer); }); @@ -175,7 +169,7 @@ describe("OidcClientStore", () => { // only called once for multiple calls to getOidcClient // before and after initialisation is complete - expect(discoverAndValidateAuthenticationConfig).toHaveBeenCalledTimes(1); + expect(discoverAndValidateOIDCIssuerWellKnown).toHaveBeenCalledTimes(1); }); }); @@ -199,7 +193,6 @@ describe("OidcClientStore", () => { it("should throw when oidcClient could not be initialised", async () => { // make oidcClient initialisation fail - mockClient.waitForClientWellKnown.mockResolvedValue(undefined as any); localStorage.removeItem("mx_oidc_token_issuer"); const store = new OidcClientStore(mockClient); @@ -245,4 +238,17 @@ describe("OidcClientStore", () => { expect(OidcClient.prototype.revokeToken).toHaveBeenCalledWith(accessToken, "access_token"); }); }); + + describe("OIDC Aware", () => { + beforeEach(() => { + localStorage.clear(); + }); + + it("should resolve account management endpoint", async () => { + mockClient.getAuthIssuer.mockResolvedValue({ issuer: metadata.issuer }); + const store = new OidcClientStore(mockClient); + await store.readyPromise; + expect(store.accountManagementEndpoint).toBe(account); + }); + }); }); diff --git a/test/test-utils/oidc.ts b/test/test-utils/oidc.ts index 2432e3350f..1a064fe5e7 100644 --- a/test/test-utils/oidc.ts +++ b/test/test-utils/oidc.ts @@ -26,8 +26,7 @@ export const makeDelegatedAuthConfig = (issuer = "https://auth.org/"): OidcClien const metadata = mockOpenIdConfiguration(issuer); return { - issuer, - account: issuer + "account", + accountManagementEndpoint: issuer + "account", registrationEndpoint: metadata.registration_endpoint, authorizationEndpoint: metadata.authorization_endpoint, tokenEndpoint: metadata.token_endpoint, @@ -50,4 +49,5 @@ export const mockOpenIdConfiguration = (issuer = "https://auth.org/"): Validated response_types_supported: ["code"], grant_types_supported: ["authorization_code", "refresh_token"], code_challenge_methods_supported: ["S256"], + account_management_uri: issuer + "account", }); diff --git a/test/utils/AutoDiscoveryUtils-test.tsx b/test/utils/AutoDiscoveryUtils-test.tsx index 7cd5210a99..452287225b 100644 --- a/test/utils/AutoDiscoveryUtils-test.tsx +++ b/test/utils/AutoDiscoveryUtils-test.tsx @@ -14,12 +14,22 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { AutoDiscovery, AutoDiscoveryAction, ClientConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix"; +import { AutoDiscovery, AutoDiscoveryAction, ClientConfig } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; +import fetchMock from "fetch-mock-jest"; import AutoDiscoveryUtils from "../../src/utils/AutoDiscoveryUtils"; +import { mockOpenIdConfiguration } from "../test-utils/oidc"; describe("AutoDiscoveryUtils", () => { + beforeEach(() => { + fetchMock.catch({ + status: 404, + body: '{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}', + headers: { "content-type": "application/json" }, + }); + }); + describe("buildValidatedConfigFromDiscovery()", () => { const serverName = "my-server"; @@ -56,24 +66,24 @@ describe("AutoDiscoveryUtils", () => { isUrl: "identity.com", }; - it("throws an error when discovery result is falsy", () => { - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, undefined as any)).toThrow( - "Unexpected error resolving homeserver configuration", - ); + it("throws an error when discovery result is falsy", async () => { + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, undefined as any), + ).rejects.toThrow("Unexpected error resolving homeserver configuration"); expect(logger.error).toHaveBeenCalled(); }); - it("throws an error when discovery result does not include homeserver config", () => { + it("throws an error when discovery result does not include homeserver config", async () => { const discoveryResult = { ...validIsConfig, } as unknown as ClientConfig; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Unexpected error resolving homeserver configuration", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Unexpected error resolving homeserver configuration"); expect(logger.error).toHaveBeenCalled(); }); - it("throws an error when identity server config has fail error and recognised error string", () => { + it("throws an error when identity server config has fail error and recognised error string", async () => { const discoveryResult = { ...validHsConfig, "m.identity_server": { @@ -81,13 +91,13 @@ describe("AutoDiscoveryUtils", () => { error: "GenericFailure", }, }; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Unexpected error resolving identity server configuration", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Unexpected error resolving identity server configuration"); expect(logger.error).toHaveBeenCalled(); }); - it("throws an error when homeserver config has fail error and recognised error string", () => { + it("throws an error when homeserver config has fail error and recognised error string", async () => { const discoveryResult = { ...validIsConfig, "m.homeserver": { @@ -95,25 +105,25 @@ describe("AutoDiscoveryUtils", () => { error: AutoDiscovery.ERROR_INVALID_HOMESERVER, }, }; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Homeserver URL does not appear to be a valid Matrix homeserver", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Homeserver URL does not appear to be a valid Matrix homeserver"); expect(logger.error).toHaveBeenCalled(); }); - it("throws an error with fallback message identity server config has fail error", () => { + it("throws an error with fallback message identity server config has fail error", async () => { const discoveryResult = { ...validHsConfig, "m.identity_server": { state: AutoDiscoveryAction.FAIL_ERROR, }, }; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Unexpected error resolving identity server configuration", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Unexpected error resolving identity server configuration"); }); - it("throws an error when error is ERROR_INVALID_HOMESERVER", () => { + it("throws an error when error is ERROR_INVALID_HOMESERVER", async () => { const discoveryResult = { ...validIsConfig, "m.homeserver": { @@ -121,12 +131,12 @@ describe("AutoDiscoveryUtils", () => { error: AutoDiscovery.ERROR_INVALID_HOMESERVER, }, }; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Homeserver URL does not appear to be a valid Matrix homeserver", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Homeserver URL does not appear to be a valid Matrix homeserver"); }); - it("throws an error when homeserver base_url is falsy", () => { + it("throws an error when homeserver base_url is falsy", async () => { const discoveryResult = { ...validIsConfig, "m.homeserver": { @@ -134,13 +144,13 @@ describe("AutoDiscoveryUtils", () => { base_url: "", }, }; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Unexpected error resolving homeserver configuration", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Unexpected error resolving homeserver configuration"); expect(logger.error).toHaveBeenCalledWith("No homeserver URL configured"); }); - it("throws an error when homeserver base_url is not a valid URL", () => { + it("throws an error when homeserver base_url is not a valid URL", async () => { const discoveryResult = { ...validIsConfig, "m.homeserver": { @@ -148,24 +158,25 @@ describe("AutoDiscoveryUtils", () => { base_url: "banana", }, }; - expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow( - "Invalid URL: banana", - ); + await expect(() => + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).rejects.toThrow("Invalid URL: banana"); }); - it("uses hs url hostname when serverName is falsy in args and config", () => { + it("uses hs url hostname when serverName is falsy in args and config", async () => { const discoveryResult = { ...validIsConfig, ...validHsConfig, }; - expect(AutoDiscoveryUtils.buildValidatedConfigFromDiscovery("", discoveryResult)).toEqual({ + await expect(AutoDiscoveryUtils.buildValidatedConfigFromDiscovery("", discoveryResult)).resolves.toEqual({ ...expectedValidatedConfig, hsNameIsDifferent: false, hsName: "matrix.org", + warning: null, }); }); - it("uses serverName from props", () => { + it("uses serverName from props", async () => { const discoveryResult = { ...validIsConfig, "m.homeserver": { @@ -174,16 +185,17 @@ describe("AutoDiscoveryUtils", () => { }, }; const syntaxOnly = true; - expect( + await expect( AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), - ).toEqual({ + ).resolves.toEqual({ ...expectedValidatedConfig, hsNameIsDifferent: true, hsName: serverName, + warning: null, }); }); - it("ignores liveliness error when checking syntax only", () => { + it("ignores liveliness error when checking syntax only", async () => { const discoveryResult = { ...validIsConfig, "m.homeserver": { @@ -193,60 +205,15 @@ describe("AutoDiscoveryUtils", () => { }, }; const syntaxOnly = true; - expect( + await expect( AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), - ).toEqual({ + ).resolves.toEqual({ ...expectedValidatedConfig, warning: "Homeserver URL does not appear to be a valid Matrix homeserver", }); }); - it("ignores delegated auth config when discovery was not successful", () => { - const discoveryResult = { - ...validIsConfig, - ...validHsConfig, - [M_AUTHENTICATION.stable!]: { - state: AutoDiscoveryAction.FAIL_ERROR, - error: "", - }, - }; - const syntaxOnly = true; - expect( - AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), - ).toEqual({ - ...expectedValidatedConfig, - delegatedAuthentication: undefined, - warning: undefined, - }); - }); - - it("sets delegated auth config when discovery was successful", () => { - const authConfig = { - issuer: "https://test.com/", - authorizationEndpoint: "https://test.com/auth", - registrationEndpoint: "https://test.com/registration", - tokenEndpoint: "https://test.com/token", - }; - const discoveryResult: ClientConfig = { - ...validIsConfig, - ...validHsConfig, - [M_AUTHENTICATION.stable!]: { - state: AutoDiscoveryAction.SUCCESS, - error: null, - ...authConfig, - }, - }; - const syntaxOnly = true; - expect( - AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), - ).toEqual({ - ...expectedValidatedConfig, - delegatedAuthentication: authConfig, - warning: undefined, - }); - }); - - it("handles homeserver too old error", () => { + it("handles homeserver too old error", async () => { const discoveryResult: ClientConfig = { ...validIsConfig, "m.homeserver": { @@ -256,12 +223,165 @@ describe("AutoDiscoveryUtils", () => { }, }; const syntaxOnly = true; - expect(() => + await expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult, syntaxOnly), - ).toThrow( + ).rejects.toThrow( "Your homeserver is too old and does not support the minimum API version required. Please contact your server owner, or upgrade your server.", ); }); + + it("should validate delegated oidc auth", async () => { + const issuer = "https://auth.matrix.org/"; + fetchMock.get( + `${validHsConfig["m.homeserver"].base_url}/_matrix/client/unstable/org.matrix.msc2965/auth_issuer`, + { + issuer, + }, + ); + fetchMock.get(`${issuer}.well-known/openid-configuration`, { + ...mockOpenIdConfiguration(issuer), + "scopes_supported": ["openid", "email"], + "response_modes_supported": ["form_post", "query", "fragment"], + "token_endpoint_auth_methods_supported": [ + "client_secret_basic", + "client_secret_post", + "client_secret_jwt", + "private_key_jwt", + "none", + ], + "token_endpoint_auth_signing_alg_values_supported": [ + "HS256", + "HS384", + "HS512", + "RS256", + "RS384", + "RS512", + "PS256", + "PS384", + "PS512", + "ES256", + "ES384", + "ES256K", + ], + "revocation_endpoint_auth_methods_supported": [ + "client_secret_basic", + "client_secret_post", + "client_secret_jwt", + "private_key_jwt", + "none", + ], + "revocation_endpoint_auth_signing_alg_values_supported": [ + "HS256", + "HS384", + "HS512", + "RS256", + "RS384", + "RS512", + "PS256", + "PS384", + "PS512", + "ES256", + "ES384", + "ES256K", + ], + "introspection_endpoint": `${issuer}oauth2/introspect`, + "introspection_endpoint_auth_methods_supported": [ + "client_secret_basic", + "client_secret_post", + "client_secret_jwt", + "private_key_jwt", + "none", + ], + "introspection_endpoint_auth_signing_alg_values_supported": [ + "HS256", + "HS384", + "HS512", + "RS256", + "RS384", + "RS512", + "PS256", + "PS384", + "PS512", + "ES256", + "ES384", + "ES256K", + ], + "userinfo_endpoint": `${issuer}oauth2/userinfo`, + "subject_types_supported": ["public"], + "id_token_signing_alg_values_supported": [ + "RS256", + "RS384", + "RS512", + "ES256", + "ES384", + "PS256", + "PS384", + "PS512", + "ES256K", + ], + "userinfo_signing_alg_values_supported": [ + "RS256", + "RS384", + "RS512", + "ES256", + "ES384", + "PS256", + "PS384", + "PS512", + "ES256K", + ], + "display_values_supported": ["page"], + "claim_types_supported": ["normal"], + "claims_supported": ["iss", "sub", "aud", "iat", "exp", "nonce", "auth_time", "at_hash", "c_hash"], + "claims_parameter_supported": false, + "request_parameter_supported": false, + "request_uri_parameter_supported": false, + "prompt_values_supported": ["none", "login", "create"], + "device_authorization_endpoint": `${issuer}oauth2/device`, + "org.matrix.matrix-authentication-service.graphql_endpoint": `${issuer}graphql`, + "account_management_uri": `${issuer}account/`, + "account_management_actions_supported": [ + "org.matrix.profile", + "org.matrix.sessions_list", + "org.matrix.session_view", + "org.matrix.session_end", + "org.matrix.cross_signing_reset", + ], + }); + fetchMock.get(`${issuer}jwks`, { + keys: [], + }); + + const discoveryResult = { + ...validIsConfig, + ...validHsConfig, + }; + await expect( + AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult), + ).resolves.toEqual({ + ...expectedValidatedConfig, + hsNameIsDifferent: true, + hsName: serverName, + delegatedAuthentication: expect.objectContaining({ + accountManagementActionsSupported: [ + "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", + signingKeys: [], + tokenEndpoint: "https://auth.matrix.org/token", + }), + warning: null, + }); + }); }); describe("authComponentStateForError", () => { diff --git a/test/utils/oidc/TokenRefresher-test.ts b/test/utils/oidc/TokenRefresher-test.ts index 46b33da52a..f2e5bea8cc 100644 --- a/test/utils/oidc/TokenRefresher-test.ts +++ b/test/utils/oidc/TokenRefresher-test.ts @@ -46,8 +46,8 @@ describe("TokenRefresher", () => { }; beforeEach(() => { - fetchMock.get(`${authConfig.issuer}.well-known/openid-configuration`, authConfig.metadata); - fetchMock.get(`${authConfig.issuer}jwks`, { + fetchMock.get(`${issuer}.well-known/openid-configuration`, authConfig.metadata); + fetchMock.get(`${issuer}jwks`, { status: 200, headers: { "Content-Type": "application/json", @@ -68,7 +68,7 @@ describe("TokenRefresher", () => { const getPickleKey = jest.fn().mockResolvedValue(pickleKey); mockPlatformPeg({ getPickleKey }); - const refresher = new TokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims, userId); + const refresher = new TokenRefresher(issuer, clientId, redirectUri, deviceId, idTokenClaims, userId); await refresher.oidcClientReady; @@ -83,7 +83,7 @@ describe("TokenRefresher", () => { const getPickleKey = jest.fn().mockResolvedValue(null); mockPlatformPeg({ getPickleKey }); - const refresher = new TokenRefresher(authConfig, clientId, redirectUri, deviceId, idTokenClaims, userId); + const refresher = new TokenRefresher(issuer, clientId, redirectUri, deviceId, idTokenClaims, userId); await refresher.oidcClientReady; diff --git a/test/utils/oidc/authorize-test.ts b/test/utils/oidc/authorize-test.ts index 2b95dd15fb..a323fc95a1 100644 --- a/test/utils/oidc/authorize-test.ts +++ b/test/utils/oidc/authorize-test.ts @@ -69,7 +69,10 @@ describe("OIDC authorization", () => { }); beforeAll(() => { - fetchMock.get(`${delegatedAuthConfig.issuer}.well-known/openid-configuration`, delegatedAuthConfig.metadata); + fetchMock.get( + `${delegatedAuthConfig.metadata.issuer}.well-known/openid-configuration`, + delegatedAuthConfig.metadata, + ); }); afterAll(() => { diff --git a/test/utils/oidc/getDelegatedAuthAccountUrl-test.ts b/test/utils/oidc/getDelegatedAuthAccountUrl-test.ts deleted file mode 100644 index e4ba4c5756..0000000000 --- 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"); - }); -}); diff --git a/test/utils/oidc/registerClient-test.ts b/test/utils/oidc/registerClient-test.ts index 2a187b3155..bf8d179329 100644 --- a/test/utils/oidc/registerClient-test.ts +++ b/test/utils/oidc/registerClient-test.ts @@ -16,15 +16,15 @@ limitations under the License. import fetchMockJest from "fetch-mock-jest"; import { OidcError } from "matrix-js-sdk/src/oidc/error"; +import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { getOidcClientId } from "../../../src/utils/oidc/registerClient"; -import { ValidatedDelegatedAuthConfig } from "../../../src/utils/ValidatedServerConfig"; import { mockPlatformPeg } from "../../test-utils"; import PlatformPeg from "../../../src/PlatformPeg"; +import { makeDelegatedAuthConfig } from "../../test-utils/oidc"; describe("getOidcClientId()", () => { const issuer = "https://auth.com/"; - const registrationEndpoint = "https://auth.com/register"; const clientName = "Element"; const baseUrl = "https://just.testing"; const dynamicClientId = "xyz789"; @@ -33,12 +33,7 @@ describe("getOidcClientId()", () => { client_id: "abc123", }, }; - const delegatedAuthConfig = { - issuer, - registrationEndpoint, - authorizationEndpoint: issuer + "auth", - tokenEndpoint: issuer + "token", - }; + const delegatedAuthConfig = makeDelegatedAuthConfig(issuer); beforeEach(() => { fetchMockJest.mockClear(); @@ -63,11 +58,10 @@ describe("getOidcClientId()", () => { }); it("should throw when no static clientId is configured and no registration endpoint", async () => { - const authConfigWithoutRegistration: ValidatedDelegatedAuthConfig = { - ...delegatedAuthConfig, - issuer: "https://issuerWithoutStaticClientId.org/", - registrationEndpoint: undefined, - }; + const authConfigWithoutRegistration: OidcClientConfig = makeDelegatedAuthConfig( + "https://issuerWithoutStaticClientId.org/", + ); + authConfigWithoutRegistration.registrationEndpoint = undefined; await expect(getOidcClientId(authConfigWithoutRegistration, staticOidcClients)).rejects.toThrow( OidcError.DynamicRegistrationNotSupported, ); @@ -76,7 +70,7 @@ describe("getOidcClientId()", () => { }); it("should handle when staticOidcClients object is falsy", async () => { - const authConfigWithoutRegistration: ValidatedDelegatedAuthConfig = { + const authConfigWithoutRegistration: OidcClientConfig = { ...delegatedAuthConfig, registrationEndpoint: undefined, }; @@ -88,14 +82,14 @@ describe("getOidcClientId()", () => { }); it("should make correct request to register client", async () => { - fetchMockJest.post(registrationEndpoint, { + fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, { status: 200, body: JSON.stringify({ client_id: dynamicClientId }), }); expect(await getOidcClientId(delegatedAuthConfig)).toEqual(dynamicClientId); // didn't try to register expect(fetchMockJest).toHaveBeenCalledWith( - registrationEndpoint, + delegatedAuthConfig.registrationEndpoint!, expect.objectContaining({ headers: { "Accept": "application/json", @@ -120,14 +114,14 @@ describe("getOidcClientId()", () => { }); it("should throw when registration request fails", async () => { - fetchMockJest.post(registrationEndpoint, { + fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, { status: 500, }); await expect(getOidcClientId(delegatedAuthConfig)).rejects.toThrow(OidcError.DynamicRegistrationFailed); }); it("should throw when registration response is invalid", async () => { - fetchMockJest.post(registrationEndpoint, { + fetchMockJest.post(delegatedAuthConfig.registrationEndpoint!, { status: 200, // no clientId in response body: "{}",