From db02f26005a023c51ce5de0ec79d54bca7423612 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 20 Dec 2024 13:13:41 +0000 Subject: [PATCH] Delabs native OIDC support (#28615) Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/components/structures/auth/Login.tsx | 10 +--- .../structures/auth/Registration.tsx | 14 +---- src/i18n/strings/en_EN.json | 2 - src/settings/Settings.tsx | 10 ---- .../components/structures/UserMenu-test.tsx | 4 -- .../components/structures/auth/Login-test.tsx | 4 -- .../structures/auth/Registration-test.tsx | 58 ++++++------------- 7 files changed, 21 insertions(+), 81 deletions(-) diff --git a/src/components/structures/auth/Login.tsx b/src/components/structures/auth/Login.tsx index d9c853bc3a5..34cea1317cc 100644 --- a/src/components/structures/auth/Login.tsx +++ b/src/components/structures/auth/Login.tsx @@ -30,7 +30,6 @@ import AuthHeader from "../../views/auth/AuthHeader"; import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleButton"; import { ValidatedServerConfig } from "../../../utils/ValidatedServerConfig"; import { filterBoolean } from "../../../utils/arrays"; -import { Features } from "../../../settings/Settings"; import { startOidcLogin } from "../../../utils/oidc/authorize"; interface IProps { @@ -90,7 +89,6 @@ type OnPasswordLogin = { */ export default class LoginComponent extends React.PureComponent { private unmounted = false; - private oidcNativeFlowEnabled = false; private loginLogic!: Login; private readonly stepRendererMap: Record ReactNode>; @@ -98,9 +96,6 @@ export default class LoginComponent extends React.PureComponent public constructor(props: IProps) { super(props); - // only set on a config level, so we don't need to watch - this.oidcNativeFlowEnabled = SettingsStore.getValue(Features.OidcNativeFlow); - this.state = { busy: false, errorText: null, @@ -358,10 +353,7 @@ export default class LoginComponent extends React.PureComponent const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, { defaultDeviceDisplayName: this.props.defaultDeviceDisplayName, - // if native OIDC is enabled in the client pass the server's delegated auth settings - delegatedAuthentication: this.oidcNativeFlowEnabled - ? this.props.serverConfig.delegatedAuthentication - : undefined, + delegatedAuthentication: this.props.serverConfig.delegatedAuthentication, }); this.loginLogic = loginLogic; diff --git a/src/components/structures/auth/Registration.tsx b/src/components/structures/auth/Registration.tsx index 91fe5c5faa0..fa20efe3493 100644 --- a/src/components/structures/auth/Registration.tsx +++ b/src/components/structures/auth/Registration.tsx @@ -44,7 +44,6 @@ 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 => { @@ -130,8 +129,6 @@ 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); @@ -150,14 +147,10 @@ export default class Registration extends React.Component { serverDeadError: "", }; - // 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, + delegatedAuthentication, }); } @@ -227,10 +220,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; - - this.loginLogic.setDelegatedAuthentication(delegatedAuthentication); + this.loginLogic.setDelegatedAuthentication(serverConfig.delegatedAuthentication); let ssoFlow: SSOFlow | undefined; let oidcNativeFlow: OidcNativeFlow | undefined; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index f3c514fca38..e0edd074c25 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1461,8 +1461,6 @@ "notification_settings_beta_caption": "Introducing a simpler way to change your notification settings. Customize your %(brand)s, just the way you like.", "notification_settings_beta_title": "Notification Settings", "notifications": "Enable the notifications panel in the room header", - "oidc_native_flow": "OIDC native authentication", - "oidc_native_flow_description": "⚠ WARNING: Experimental. Use OIDC native authentication when supported by the server.", "release_announcement": "Release announcement", "render_reaction_images": "Render custom images in reactions", "render_reaction_images_description": "Sometimes referred to as \"custom emojis\".", diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 6cd5b15a515..81c4fb6ef28 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -86,7 +86,6 @@ export enum LabGroup { export enum Features { NotificationSettings2 = "feature_notification_settings2", - OidcNativeFlow = "feature_oidc_native_flow", ReleaseAnnouncement = "feature_release_announcement", } @@ -438,15 +437,6 @@ export const SETTINGS: { [setting: string]: ISetting } = { shouldWarn: true, default: false, }, - [Features.OidcNativeFlow]: { - isFeature: true, - labsGroup: LabGroup.Developer, - supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG_PRIORITISED, - supportedLevelsAreOrdered: true, - displayName: _td("labs|oidc_native_flow"), - description: _td("labs|oidc_native_flow_description"), - default: false, - }, /** * @deprecated in favor of {@link fontSizeDelta} */ diff --git a/test/unit-tests/components/structures/UserMenu-test.tsx b/test/unit-tests/components/structures/UserMenu-test.tsx index 907bf664b7f..be79d61461b 100644 --- a/test/unit-tests/components/structures/UserMenu-test.tsx +++ b/test/unit-tests/components/structures/UserMenu-test.tsx @@ -19,9 +19,6 @@ import { TestSdkContext } from "../../TestSdkContext"; import defaultDispatcher from "../../../../src/dispatcher/dispatcher"; import LogoutDialog from "../../../../src/components/views/dialogs/LogoutDialog"; import Modal from "../../../../src/Modal"; -import SettingsStore from "../../../../src/settings/SettingsStore"; -import { Features } from "../../../../src/settings/Settings"; -import { SettingLevel } from "../../../../src/settings/SettingLevel"; import { mockOpenIdConfiguration } from "../../../test-utils/oidc"; import { Action } from "../../../../src/dispatcher/actions"; import { UserTab } from "../../../../src/components/views/dialogs/UserTab"; @@ -137,7 +134,6 @@ describe("", () => { isCrossSigningReady: jest.fn().mockResolvedValue(true), exportSecretsBundle: jest.fn().mockResolvedValue({}), } as unknown as CryptoApi); - await SettingsStore.setValue(Features.OidcNativeFlow, null, SettingLevel.DEVICE, true); const spy = jest.spyOn(defaultDispatcher, "dispatch"); const UserMenu = wrapInSdkContext(UnwrappedUserMenu, sdkContext); diff --git a/test/unit-tests/components/structures/auth/Login-test.tsx b/test/unit-tests/components/structures/auth/Login-test.tsx index 7105de4d223..c0c52e489f5 100644 --- a/test/unit-tests/components/structures/auth/Login-test.tsx +++ b/test/unit-tests/components/structures/auth/Login-test.tsx @@ -19,7 +19,6 @@ import { mkServerConfig, mockPlatformPeg, unmockPlatformPeg } from "../../../../ 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 * as registerClientUtils from "../../../../../src/utils/oidc/registerClient"; import { makeDelegatedAuthConfig } from "../../../../test-utils/oidc"; @@ -371,9 +370,6 @@ describe("Login", function () { const delegatedAuth = makeDelegatedAuthConfig(issuer); beforeEach(() => { jest.spyOn(logger, "error"); - jest.spyOn(SettingsStore, "getValue").mockImplementation( - (settingName) => settingName === Features.OidcNativeFlow, - ); }); afterEach(() => { diff --git a/test/unit-tests/components/structures/auth/Registration-test.tsx b/test/unit-tests/components/structures/auth/Registration-test.tsx index 526008a7e6a..bdc516b577b 100644 --- a/test/unit-tests/components/structures/auth/Registration-test.tsx +++ b/test/unit-tests/components/structures/auth/Registration-test.tsx @@ -22,8 +22,6 @@ import { } 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", () => ({ @@ -180,49 +178,29 @@ describe("Registration", function () { fetchMock.get(authConfig.metadata.jwks_uri!, { keys: [] }); }); - describe("when oidc native flow is not enabled in settings", () => { - beforeEach(() => { - jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); - }); + 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(); - 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(); - }); + expect(await screen.findByText("Continue")).toBeTruthy(); }); - describe("when oidc native flow is enabled in settings", () => { - beforeEach(() => { - jest.spyOn(SettingsStore, "getValue").mockImplementation((key) => key === Features.OidcNativeFlow); - }); + it("should start OIDC login flow as registration on button click", async () => { + getComponent(defaultHsUrl, defaultIsUrl, authConfig); + await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); - 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(await screen.findByText("Continue")).toBeTruthy(); - }); + fireEvent.click(await screen.findByText("Continue")); - it("should start OIDC login flow as registration on button click", async () => { - getComponent(defaultHsUrl, defaultIsUrl, authConfig); - await waitForElementToBeRemoved(() => screen.queryAllByLabelText("Loading…")); - - fireEvent.click(await screen.findByText("Continue")); - - expect(startOidcLogin).toHaveBeenCalledWith( - authConfig, - clientId, - defaultHsUrl, - defaultIsUrl, - // isRegistration - true, - ); - }); + expect(startOidcLogin).toHaveBeenCalledWith( + authConfig, + clientId, + defaultHsUrl, + defaultIsUrl, + // isRegistration + true, + ); }); describe("when is mobile registeration", () => {