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 d9c853bc3a..34cea1317c 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<IProps, IState> {
private unmounted = false;
- private oidcNativeFlowEnabled = false;
private loginLogic!: Login;
private readonly stepRendererMap: Record<string, () => ReactNode>;
@@ -98,9 +96,6 @@ export default class LoginComponent extends React.PureComponent<IProps, IState>
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<IProps, IState>
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 91fe5c5faa..fa20efe349 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<IProps, IState> {
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<IProps, IState> {
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<IProps, IState> {
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 f3c514fca3..e0edd074c2 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 6cd5b15a51..81c4fb6ef2 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 907bf664b7..be79d61461 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("<UserMenu>", () => {
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 7105de4d22..c0c52e489f 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 526008a7e6..bdc516b577 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();
+ fireEvent.click(await screen.findByText("Continue"));
- 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(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", () => {