From b67d317ace125b0b8fe0edbf0bcc52fd49397221 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 9 Oct 2023 16:24:00 +1300 Subject: [PATCH 1/8] update uses of ValidatedDelegatedAuthConfig to broader OidcClientConfig type --- src/Login.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Login.ts b/src/Login.ts index 5ca4e9e5a25..6647f7ff756 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -22,12 +22,12 @@ import { DELEGATED_OIDC_COMPATIBILITY, ILoginFlow, LoginRequest, + OidcClientConfig, } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import { IMatrixClientCreds } from "./MatrixClientPeg"; import SecurityCustomisations from "./customisations/Security"; -import { ValidatedDelegatedAuthConfig } from "./utils/ValidatedServerConfig"; import { getOidcClientId } from "./utils/oidc/registerClient"; import { IConfigOptions } from "./IConfigOptions"; import SdkConfig from "./SdkConfig"; @@ -47,13 +47,13 @@ interface ILoginOptions { * If this property is set, we will attempt an OIDC login using the delegated auth settings. * The caller is responsible for checking that OIDC is enabled in the labs settings. */ - delegatedAuthentication?: ValidatedDelegatedAuthConfig; + delegatedAuthentication?: OidcClientConfig; } export default class Login { private flows: Array = []; private readonly defaultDeviceDisplayName?: string; - private readonly delegatedAuthentication?: ValidatedDelegatedAuthConfig; + private readonly delegatedAuthentication?: OidcClientConfig; private tempClient: MatrixClient | null = null; // memoize public constructor( @@ -213,7 +213,7 @@ export interface OidcNativeFlow extends ILoginFlow { * @throws when client can't register with OP, or any unexpected error */ const tryInitOidcNativeFlow = async ( - delegatedAuthConfig: ValidatedDelegatedAuthConfig, + delegatedAuthConfig: OidcClientConfig, brand: string, oidcStaticClients?: IConfigOptions["oidc_static_clients"], ): Promise => { From 9863cb6b76fededb6cac51a9d3d035e8825e47ee Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Mon, 9 Oct 2023 16:51:45 +1300 Subject: [PATCH 2/8] add OIDC register flow to registration page --- src/Login.ts | 22 ++++++- .../structures/auth/Registration.tsx | 58 +++++++++++++++++-- src/utils/oidc/isUserRegistrationSupported.ts | 30 ++++++++++ 3 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 src/utils/oidc/isUserRegistrationSupported.ts diff --git a/src/Login.ts b/src/Login.ts index 6647f7ff756..96f897d2a4a 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -31,6 +31,7 @@ import SecurityCustomisations from "./customisations/Security"; import { getOidcClientId } from "./utils/oidc/registerClient"; import { IConfigOptions } from "./IConfigOptions"; import SdkConfig from "./SdkConfig"; +import { isUserRegistrationSupported } from "./utils/oidc/isUserRegistrationSupported"; /** * Login flows supported by this client @@ -53,7 +54,7 @@ interface ILoginOptions { export default class Login { private flows: Array = []; private readonly defaultDeviceDisplayName?: string; - private readonly delegatedAuthentication?: OidcClientConfig; + private delegatedAuthentication?: OidcClientConfig; private tempClient: MatrixClient | null = null; // memoize public constructor( @@ -84,6 +85,11 @@ export default class Login { this.isUrl = isUrl; } + public setDelegatedAuthentication(delegatedAuthentication?: ValidatedDelegatedAuthConfig): void { + this.tempClient = null; // clear memoization + this.delegatedAuthentication = delegatedAuthentication; + } + /** * Get a temporary MatrixClient, which can be used for login or register * requests. @@ -99,7 +105,12 @@ export default class Login { return this.tempClient; } - public async getFlows(): Promise> { + /** + * Get supported login flows + * @param isRegistration OPTIONAL used to verify registration is supported in delegated authentication config + * @returns Promise that resolves to supported login flows + */ + public async getFlows(isRegistration?: boolean): Promise> { // try to use oidc native flow if we have delegated auth config if (this.delegatedAuthentication) { try { @@ -107,6 +118,7 @@ export default class Login { this.delegatedAuthentication, SdkConfig.get().brand, SdkConfig.get().oidc_static_clients, + isRegistration ); return [oidcFlow]; } catch (error) { @@ -209,6 +221,7 @@ export interface OidcNativeFlow extends ILoginFlow { * @param delegatedAuthConfig Auth config from ValidatedServerConfig * @param clientName Client name to register with the OP, eg 'Element', used during client registration with OP * @param staticOidcClientIds static client config from config.json, used during client registration with OP + * @param isRegistration true when we are attempting registration * @returns Promise when oidc native authentication flow is supported and correctly configured * @throws when client can't register with OP, or any unexpected error */ @@ -216,7 +229,12 @@ const tryInitOidcNativeFlow = async ( delegatedAuthConfig: OidcClientConfig, brand: string, oidcStaticClients?: IConfigOptions["oidc_static_clients"], + isRegistration?: boolean, ): Promise => { + // if registration is not supported, bail before attempting registration + if (isRegistration && !isUserRegistrationSupported(delegatedAuthConfig)) { + throw new Error("Registration is not supported by OP") + } const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClients); const flow = { diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx index f4e8ff1ca7d..89d0d43d744 100644 --- a/src/components/structures/auth/Registration.tsx +++ b/src/components/structures/auth/Registration.tsx @@ -38,7 +38,7 @@ import AutoDiscoveryUtils from "../../../utils/AutoDiscoveryUtils"; import * as Lifecycle from "../../../Lifecycle"; import { IMatrixClientCreds, MatrixClientPeg } from "../../../MatrixClientPeg"; import AuthPage from "../../views/auth/AuthPage"; -import Login from "../../../Login"; +import Login, { OidcNativeFlow } from "../../../Login"; import dis from "../../../dispatcher/dispatcher"; import SSOButtons from "../../views/elements/SSOButtons"; import ServerPicker from "../../views/elements/ServerPicker"; @@ -52,6 +52,8 @@ import { AuthHeaderDisplay } from "./header/AuthHeaderDisplay"; import { AuthHeaderProvider } from "./header/AuthHeaderProvider"; import SettingsStore from "../../../settings/SettingsStore"; import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; +import { Features } from "../../../settings/Settings"; +import { startOidcLogin } from "../../../utils/oidc/authorize"; const debuglog = (...args: any[]): void => { if (SettingsStore.getValue("debug_registration")) { @@ -123,12 +125,17 @@ interface IState { // the SSO flow definition, this is fetched from /login as that's the only // place it is exposed. ssoFlow?: SSOFlow; + // the OIDC native login flow, when supported and enabled + // if present, must be used for registration + oidcNativeFlow?: OidcNativeFlow; } export default class Registration extends React.Component { private readonly loginLogic: Login; // `replaceClient` tracks latest serverConfig to spot when it changes under the async method which fetches flows private latestServerConfig?: ValidatedServerConfig; + // cache value from settings store + private oidcNativeFlowEnabled = false; public constructor(props: IProps) { super(props); @@ -147,9 +154,16 @@ export default class Registration extends React.Component { serverDeadError: "", }; - const { hsUrl, isUrl } = this.props.serverConfig; + // only set on a config level, so we don't need to watch + this.oidcNativeFlowEnabled = SettingsStore.getValue(Features.OidcNativeFlow); + + const { hsUrl, isUrl, delegatedAuthentication } = this.props.serverConfig; this.loginLogic = new Login(hsUrl, isUrl, null, { defaultDeviceDisplayName: "Element login check", // We shouldn't ever be used + // if native OIDC is enabled in the client pass the server's delegated auth settings + delegatedAuthentication: this.oidcNativeFlowEnabled + ? delegatedAuthentication + : undefined, }); } @@ -219,12 +233,21 @@ export default class Registration extends React.Component { this.loginLogic.setHomeserverUrl(hsUrl); this.loginLogic.setIdentityServerUrl(isUrl); + // if native OIDC is enabled in the client pass the server's delegated auth settings + const delegatedAuthentication = this.oidcNativeFlowEnabled + ? serverConfig.delegatedAuthentication + : undefined; + + this.loginLogic.setDelegatedAuthentication(delegatedAuthentication); let ssoFlow: SSOFlow | undefined; + let oidcNativeFlow: OidcNativeFlow | undefined; try { - const loginFlows = await this.loginLogic.getFlows(); + const loginFlows = await this.loginLogic.getFlows(true); if (serverConfig !== this.latestServerConfig) return; // discard, serverConfig changed from under us ssoFlow = loginFlows.find((f) => f.type === "m.login.sso" || f.type === "m.login.cas") as SSOFlow; + oidcNativeFlow = loginFlows.find(f => f.type === "oidcNativeFlow") as OidcNativeFlow; + } catch (e) { if (serverConfig !== this.latestServerConfig) return; // discard, serverConfig changed from under us logger.error("Failed to get login flows to check for SSO support", e); @@ -233,9 +256,19 @@ export default class Registration extends React.Component { this.setState({ matrixClient: cli, ssoFlow, + oidcNativeFlow, + // if we are using oidc native we won't continue with flow discovery on HS + // so set an empty array to indicate flows are no longer loading + flows: oidcNativeFlow ? [] : this.state.flows, busy: false, }); + // don't need to check with homeserver for login flows + if (oidcNativeFlow) { + return; + } + + try { // We do the first registration request ourselves to discover whether we need to // do SSO instead. If we've already started the UI Auth process though, we don't @@ -513,7 +546,24 @@ export default class Registration extends React.Component { ); - } else if (this.state.matrixClient && this.state.flows.length) { + } else if (this.state.matrixClient && this.state.oidcNativeFlow) { + return { + await startOidcLogin( + this.props.serverConfig.delegatedAuthentication!, + this.state.oidcNativeFlow!.clientId, + this.props.serverConfig.hsUrl, + this.props.serverConfig.isUrl, + true /* isRegistration */ + ); + }} + > + {_t("action|continue")} + + } + else if (this.state.matrixClient && this.state.flows.length) { let ssoSection: JSX.Element | undefined; if (this.state.ssoFlow) { let continueWithSection; diff --git a/src/utils/oidc/isUserRegistrationSupported.ts b/src/utils/oidc/isUserRegistrationSupported.ts new file mode 100644 index 00000000000..fea548a4dc5 --- /dev/null +++ b/src/utils/oidc/isUserRegistrationSupported.ts @@ -0,0 +1,30 @@ +/* +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 { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; + +/** + * Check the create prompt is supported by the OP, if so, we can do a registration flow + * https://openid.net/specs/openid-connect-prompt-create-1_0.html + * @param delegatedAuthConfig config as returned from discovery + * @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'] + return Array.isArray(supportedPrompts) && supportedPrompts?.includes("create"); +} \ No newline at end of file From e97090df0c3ec50e71965f7eeea0e015cff23d1a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 10:38:42 +1300 Subject: [PATCH 3/8] pass prompt param to auth url creation --- src/Login.ts | 2 +- src/utils/oidc/authorize.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Login.ts b/src/Login.ts index 96f897d2a4a..912ba8bc614 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -231,7 +231,7 @@ const tryInitOidcNativeFlow = async ( oidcStaticClients?: IConfigOptions["oidc_static_clients"], isRegistration?: boolean, ): Promise => { - // if registration is not supported, bail before attempting registration + // if registration is not supported, bail before attempting to get the clientId if (isRegistration && !isUserRegistrationSupported(delegatedAuthConfig)) { throw new Error("Registration is not supported by OP") } diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 3e6de7de14b..e4968336921 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -35,11 +35,14 @@ export const startOidcLogin = async ( clientId: string, homeserverUrl: string, identityServerUrl?: string, + isRegistration?: boolean, ): Promise => { const redirectUri = window.location.origin; const nonce = randomString(10); + const prompt = isRegistration ? 'create' : undefined; + const authorizationUrl = await generateOidcAuthorizationUrl({ metadata: delegatedAuthConfig.metadata, redirectUri, @@ -47,8 +50,11 @@ export const startOidcLogin = async ( homeserverUrl, identityServerUrl, nonce, + prompt }); + console.log('hhh', authorizationUrl); + window.location.href = authorizationUrl; }; From 26bdba93dd77065e077f70eebadb91df24a0660a Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 15:12:07 +1300 Subject: [PATCH 4/8] update type --- src/Login.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Login.ts b/src/Login.ts index 912ba8bc614..d2c6ec4c686 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -85,7 +85,7 @@ export default class Login { this.isUrl = isUrl; } - public setDelegatedAuthentication(delegatedAuthentication?: ValidatedDelegatedAuthConfig): void { + public setDelegatedAuthentication(delegatedAuthentication?: OidcClientConfig): void { this.tempClient = null; // clear memoization this.delegatedAuthentication = delegatedAuthentication; } From 2328c5133b94e2dd840d369ce591f30e4524d822 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 15:15:54 +1300 Subject: [PATCH 5/8] lint --- src/Login.ts | 4 +- .../structures/auth/Registration.tsx | 49 +++++++++---------- src/utils/oidc/authorize.ts | 6 +-- src/utils/oidc/isUserRegistrationSupported.ts | 6 +-- 4 files changed, 30 insertions(+), 35 deletions(-) diff --git a/src/Login.ts b/src/Login.ts index d2c6ec4c686..3ba0adc23c4 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -118,7 +118,7 @@ export default class Login { this.delegatedAuthentication, SdkConfig.get().brand, SdkConfig.get().oidc_static_clients, - isRegistration + isRegistration, ); return [oidcFlow]; } catch (error) { @@ -233,7 +233,7 @@ const tryInitOidcNativeFlow = async ( ): Promise => { // if registration is not supported, bail before attempting to get the clientId if (isRegistration && !isUserRegistrationSupported(delegatedAuthConfig)) { - throw new Error("Registration is not supported by OP") + throw new Error("Registration is not supported by OP"); } const clientId = await getOidcClientId(delegatedAuthConfig, brand, window.location.origin, oidcStaticClients); diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx index 89d0d43d744..60cd19d28d4 100644 --- a/src/components/structures/auth/Registration.tsx +++ b/src/components/structures/auth/Registration.tsx @@ -161,9 +161,7 @@ export default class Registration extends React.Component { this.loginLogic = new Login(hsUrl, isUrl, null, { defaultDeviceDisplayName: "Element login check", // We shouldn't ever be used // if native OIDC is enabled in the client pass the server's delegated auth settings - delegatedAuthentication: this.oidcNativeFlowEnabled - ? delegatedAuthentication - : undefined, + delegatedAuthentication: this.oidcNativeFlowEnabled ? delegatedAuthentication : undefined, }); } @@ -234,9 +232,7 @@ export default class Registration extends React.Component { this.loginLogic.setHomeserverUrl(hsUrl); this.loginLogic.setIdentityServerUrl(isUrl); // if native OIDC is enabled in the client pass the server's delegated auth settings - const delegatedAuthentication = this.oidcNativeFlowEnabled - ? serverConfig.delegatedAuthentication - : undefined; + const delegatedAuthentication = this.oidcNativeFlowEnabled ? serverConfig.delegatedAuthentication : undefined; this.loginLogic.setDelegatedAuthentication(delegatedAuthentication); @@ -246,8 +242,7 @@ export default class Registration extends React.Component { const loginFlows = await this.loginLogic.getFlows(true); if (serverConfig !== this.latestServerConfig) return; // discard, serverConfig changed from under us ssoFlow = loginFlows.find((f) => f.type === "m.login.sso" || f.type === "m.login.cas") as SSOFlow; - oidcNativeFlow = loginFlows.find(f => f.type === "oidcNativeFlow") as OidcNativeFlow; - + oidcNativeFlow = loginFlows.find((f) => f.type === "oidcNativeFlow") as OidcNativeFlow; } catch (e) { if (serverConfig !== this.latestServerConfig) return; // discard, serverConfig changed from under us logger.error("Failed to get login flows to check for SSO support", e); @@ -268,7 +263,6 @@ export default class Registration extends React.Component { return; } - try { // We do the first registration request ourselves to discover whether we need to // do SSO instead. If we've already started the UI Auth process though, we don't @@ -546,24 +540,25 @@ export default class Registration extends React.Component { ); - } else if (this.state.matrixClient && this.state.oidcNativeFlow) { - return { - await startOidcLogin( - this.props.serverConfig.delegatedAuthentication!, - this.state.oidcNativeFlow!.clientId, - this.props.serverConfig.hsUrl, - this.props.serverConfig.isUrl, - true /* isRegistration */ - ); - }} - > - {_t("action|continue")} - - } - else if (this.state.matrixClient && this.state.flows.length) { + } else if (this.state.matrixClient && this.state.oidcNativeFlow) { + return ( + { + await startOidcLogin( + this.props.serverConfig.delegatedAuthentication!, + this.state.oidcNativeFlow!.clientId, + this.props.serverConfig.hsUrl, + this.props.serverConfig.isUrl, + true /* isRegistration */, + ); + }} + > + {_t("action|continue")} + + ); + } else if (this.state.matrixClient && this.state.flows.length) { let ssoSection: JSX.Element | undefined; if (this.state.ssoFlow) { let continueWithSection; diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index e4968336921..b607b01a7a9 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -41,7 +41,7 @@ export const startOidcLogin = async ( const nonce = randomString(10); - const prompt = isRegistration ? 'create' : undefined; + const prompt = isRegistration ? "create" : undefined; const authorizationUrl = await generateOidcAuthorizationUrl({ metadata: delegatedAuthConfig.metadata, @@ -50,10 +50,10 @@ export const startOidcLogin = async ( homeserverUrl, identityServerUrl, nonce, - prompt + prompt, }); - console.log('hhh', authorizationUrl); + console.log("hhh", authorizationUrl); window.location.href = authorizationUrl; }; diff --git a/src/utils/oidc/isUserRegistrationSupported.ts b/src/utils/oidc/isUserRegistrationSupported.ts index fea548a4dc5..229a9d767e7 100644 --- a/src/utils/oidc/isUserRegistrationSupported.ts +++ b/src/utils/oidc/isUserRegistrationSupported.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; +import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; /** * Check the create prompt is supported by the OP, if so, we can do a registration flow @@ -25,6 +25,6 @@ import { OidcClientConfig } from "matrix-js-sdk/src/autodiscovery"; 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.metadata as Record)["prompt_values_supported"]; return Array.isArray(supportedPrompts) && supportedPrompts?.includes("create"); -} \ No newline at end of file +}; From ba58b35505c592ad6fc1873027d25cf6d6683479 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Tue, 10 Oct 2023 15:58:36 +1300 Subject: [PATCH 6/8] test registration oidc button --- src/utils/oidc/authorize.ts | 2 - .../components/structures/auth/Login-test.tsx | 21 ++-- .../structures/auth/Registration-test.tsx | 108 +++++++++++++++--- test/test-utils/test-utils.ts | 5 +- 4 files changed, 103 insertions(+), 33 deletions(-) diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index b607b01a7a9..6b9b46f5fcc 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -53,8 +53,6 @@ export const startOidcLogin = async ( prompt, }); - console.log("hhh", authorizationUrl); - window.location.href = authorizationUrl; }; diff --git a/test/components/structures/auth/Login-test.tsx b/test/components/structures/auth/Login-test.tsx index d4c8f457f88..1e0e0de78a7 100644 --- a/test/components/structures/auth/Login-test.tsx +++ b/test/components/structures/auth/Login-test.tsx @@ -18,7 +18,7 @@ import React from "react"; import { fireEvent, render, screen, waitForElementToBeRemoved } from "@testing-library/react"; import { mocked, MockedObject } from "jest-mock"; import fetchMock from "fetch-mock-jest"; -import { DELEGATED_OIDC_COMPATIBILITY, IdentityProviderBrand } from "matrix-js-sdk/src/matrix"; +import { DELEGATED_OIDC_COMPATIBILITY, IdentityProviderBrand, OidcClientConfig } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import * as Matrix from "matrix-js-sdk/src/matrix"; import { OidcError } from "matrix-js-sdk/src/oidc/error"; @@ -29,8 +29,8 @@ import Login from "../../../../src/components/structures/auth/Login"; import BasePlatform from "../../../../src/BasePlatform"; import SettingsStore from "../../../../src/settings/SettingsStore"; import { Features } from "../../../../src/settings/Settings"; -import { ValidatedDelegatedAuthConfig } from "../../../../src/utils/ValidatedServerConfig"; import * as registerClientUtils from "../../../../src/utils/oidc/registerClient"; +import { makeDelegatedAuthConfig } from "../../../test-utils/oidc"; jest.useRealTimers(); @@ -85,7 +85,7 @@ describe("Login", function () { function getRawComponent( hsUrl = "https://matrix.org", isUrl = "https://vector.im", - delegatedAuthentication?: ValidatedDelegatedAuthConfig, + delegatedAuthentication?: OidcClientConfig, ) { return ( { jest.spyOn(logger, "error"); jest.spyOn(SettingsStore, "getValue").mockImplementation( @@ -412,7 +407,7 @@ 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.registrationEndpoint!, { status: 500 }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); @@ -429,7 +424,7 @@ describe("Login", function () { }); it("should fallback to normal login when client registration fails", async () => { - fetchMock.post(delegatedAuth.registrationEndpoint, { status: 500 }); + fetchMock.post(delegatedAuth.registrationEndpoint!, { status: 500 }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); @@ -446,7 +441,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.registrationEndpoint!, { client_id: "abc123" }); getComponent(hsUrl, isUrl, delegatedAuth); await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); diff --git a/test/components/structures/auth/Registration-test.tsx b/test/components/structures/auth/Registration-test.tsx index 0123d6867ed..e3b941d5a73 100644 --- a/test/components/structures/auth/Registration-test.tsx +++ b/test/components/structures/auth/Registration-test.tsx @@ -17,13 +17,21 @@ limitations under the License. import React from "react"; import { fireEvent, render, screen, waitForElementToBeRemoved } from "@testing-library/react"; -import { createClient, MatrixClient, MatrixError } from "matrix-js-sdk/src/matrix"; -import { mocked } from "jest-mock"; +import { createClient, MatrixClient, MatrixError, OidcClientConfig } from "matrix-js-sdk/src/matrix"; +import { mocked, MockedObject } from "jest-mock"; import fetchMock from "fetch-mock-jest"; import SdkConfig, { DEFAULTS } from "../../../../src/SdkConfig"; -import { mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../test-utils"; +import { getMockClientWithEventEmitter, mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../test-utils"; import Registration from "../../../../src/components/structures/auth/Registration"; +import { makeDelegatedAuthConfig } from "../../../test-utils/oidc"; +import SettingsStore from "../../../../src/settings/SettingsStore"; +import { Features } from "../../../../src/settings/Settings"; +import { startOidcLogin } from "../../../../src/utils/oidc/authorize"; + +jest.mock("../../../../src/utils/oidc/authorize", () => ({ + startOidcLogin: jest.fn(), +})); jest.mock("matrix-js-sdk/src/matrix", () => ({ ...jest.requireActual("matrix-js-sdk/src/matrix"), @@ -32,18 +40,18 @@ jest.mock("matrix-js-sdk/src/matrix", () => ({ jest.useFakeTimers(); describe("Registration", function () { - const registerRequest = jest.fn(); - const mockClient = mocked({ - registerRequest, - loginFlows: jest.fn(), - getVersions: jest.fn().mockResolvedValue({ versions: ["v1.1"] }), - } as unknown as MatrixClient); + let mockClient!: MockedObject; beforeEach(function () { SdkConfig.put({ ...DEFAULTS, disable_custom_urls: true, }); + mockClient = getMockClientWithEventEmitter({ + registerRequest: jest.fn(), + loginFlows: jest.fn(), + getVersions: jest.fn().mockResolvedValue({ versions: ["v1.1"] }), + }); mockClient.registerRequest.mockRejectedValueOnce( new MatrixError( { @@ -52,12 +60,13 @@ describe("Registration", function () { 401, ), ); - mockClient.loginFlows.mockClear().mockResolvedValue({ flows: [{ type: "m.login.password" }] }); + mockClient.loginFlows.mockResolvedValue({ flows: [{ type: "m.login.password" }] }); mocked(createClient).mockImplementation((opts) => { mockClient.idBaseUrl = opts.idBaseUrl; mockClient.baseUrl = opts.baseUrl; return mockClient; }); + fetchMock.catch(404); fetchMock.get("https://matrix.org/_matrix/client/versions", { unstable_features: {}, versions: ["v1.1"], @@ -68,6 +77,7 @@ describe("Registration", function () { }); afterEach(function () { + jest.restoreAllMocks(); fetchMock.restore(); SdkConfig.reset(); // we touch the config, so clean up unmockPlatformPeg(); @@ -80,12 +90,15 @@ describe("Registration", function () { onServerConfigChange: jest.fn(), }; - function getRawComponent(hsUrl = "https://matrix.org", isUrl = "https://vector.im") { - return ; + const defaultHsUrl = "https://matrix.org"; + const defaultIsUrl = "https://vector.im"; + + function getRawComponent(hsUrl = defaultHsUrl, isUrl = defaultIsUrl, authConfig?: OidcClientConfig) { + return ; } - function getComponent(hsUrl?: string, isUrl?: string) { - return render(getRawComponent(hsUrl, isUrl)); + function getComponent(hsUrl?: string, isUrl?: string, authConfig?: OidcClientConfig) { + return render(getRawComponent(hsUrl, isUrl, authConfig)); } it("should show server picker", async function () { @@ -121,7 +134,7 @@ describe("Registration", function () { await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); fireEvent.click(container.querySelector(".mx_SSOButton")!); - expect(registerRequest.mock.instances[0].baseUrl).toBe("https://matrix.org"); + expect(mockClient.baseUrl).toBe("https://matrix.org"); fetchMock.get("https://server2/_matrix/client/versions", { unstable_features: {}, @@ -131,6 +144,69 @@ describe("Registration", function () { await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); fireEvent.click(container.querySelector(".mx_SSOButton")!); - expect(registerRequest.mock.instances[1].baseUrl).toBe("https://server2"); + expect(mockClient.baseUrl).toBe("https://server2"); + }); + + describe("when delegated authentication is configured and enabled", () => { + const authConfig = makeDelegatedAuthConfig(); + const clientId = "test-client-id"; + // @ts-ignore + authConfig.metadata["prompt_values_supported"] = ["create"]; + + beforeEach(() => { + // mock a statically registered client to avoid dynamic registration + SdkConfig.put({ + oidc_static_clients: { + [authConfig.issuer]: { + client_id: clientId, + }, + }, + }); + }); + + describe("when oidc native flow is not enabled in settings", () => { + beforeEach(() => { + jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); + }); + + it("should display user/pass registration form", async () => { + const { container } = getComponent(defaultHsUrl, defaultIsUrl, authConfig); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + expect(container.querySelector("form")).toBeTruthy(); + expect(mockClient.loginFlows).toHaveBeenCalled(); + expect(mockClient.registerRequest).toHaveBeenCalled(); + }); + }); + + describe("when oidc native flow is enabled in settings", () => { + beforeEach(() => { + jest.spyOn(SettingsStore, "getValue").mockImplementation((key) => key === Features.OidcNativeFlow); + }); + + it("should display oidc-native continue button", async () => { + const { container } = getComponent(defaultHsUrl, defaultIsUrl, authConfig); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); + // no form + expect(container.querySelector("form")).toBeFalsy(); + + expect(screen.getByText("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")); + + expect(startOidcLogin).toHaveBeenCalledWith( + authConfig, + clientId, + defaultHsUrl, + defaultIsUrl, + // isRegistration + true, + ); + }); + }); }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index f8605a08c3f..b54c0589f69 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -37,6 +37,7 @@ import { RelationType, JoinRule, IEventDecryptionResult, + OidcClientConfig, } from "matrix-js-sdk/src/matrix"; import { normalize } from "matrix-js-sdk/src/utils"; import { ReEmitter } from "matrix-js-sdk/src/ReEmitter"; @@ -47,7 +48,7 @@ import { MapperOpts } from "matrix-js-sdk/src/event-mapper"; import type { GroupCall } from "matrix-js-sdk/src/matrix"; import { MatrixClientPeg as peg } from "../../src/MatrixClientPeg"; -import { ValidatedDelegatedAuthConfig, ValidatedServerConfig } from "../../src/utils/ValidatedServerConfig"; +import { ValidatedServerConfig } from "../../src/utils/ValidatedServerConfig"; import { EnhancedMap } from "../../src/utils/maps"; import { AsyncStoreWithClient } from "../../src/stores/AsyncStoreWithClient"; import MatrixClientBackedSettingsHandler from "../../src/settings/handlers/MatrixClientBackedSettingsHandler"; @@ -653,7 +654,7 @@ export function mkStubRoom( export function mkServerConfig( hsUrl: string, isUrl: string, - delegatedAuthentication?: ValidatedDelegatedAuthConfig, + delegatedAuthentication?: OidcClientConfig, ): ValidatedServerConfig { return { hsUrl, From 9690a3179439f1f5f350979c1604f4fa5ca53a52 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Wed, 11 Oct 2023 15:27:56 +1300 Subject: [PATCH 7/8] fix: reference state inside setState --- src/components/structures/auth/Registration.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx index 60cd19d28d4..4da7282660e 100644 --- a/src/components/structures/auth/Registration.tsx +++ b/src/components/structures/auth/Registration.tsx @@ -248,17 +248,18 @@ export default class Registration extends React.Component { logger.error("Failed to get login flows to check for SSO support", e); } - this.setState({ + this.setState(({ flows }) => ({ matrixClient: cli, ssoFlow, oidcNativeFlow, // if we are using oidc native we won't continue with flow discovery on HS // so set an empty array to indicate flows are no longer loading - flows: oidcNativeFlow ? [] : this.state.flows, + flows: oidcNativeFlow ? [] : flows, busy: false, - }); + })); // don't need to check with homeserver for login flows + // since we are going to use OIDC native flow if (oidcNativeFlow) { return; } From ca7cfb46a498a0c3cdaf9e78464a215015c1e75d Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 12 Oct 2023 10:03:29 +1300 Subject: [PATCH 8/8] comment --- src/Login.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Login.ts b/src/Login.ts index 3ba0adc23c4..447542a64d8 100644 --- a/src/Login.ts +++ b/src/Login.ts @@ -85,6 +85,10 @@ export default class Login { this.isUrl = isUrl; } + /** + * Set delegated authentication config, clears tempClient. + * @param delegatedAuthentication delegated auth config, from ValidatedServerConfig + */ public setDelegatedAuthentication(delegatedAuthentication?: OidcClientConfig): void { this.tempClient = null; // clear memoization this.delegatedAuthentication = delegatedAuthentication;