Skip to content

Commit

Permalink
Delabs native OIDC support (#28615)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Telatynski <[email protected]>
  • Loading branch information
t3chguy authored Dec 20, 2024
1 parent b07d10c commit db02f26
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 81 deletions.
10 changes: 1 addition & 9 deletions src/components/structures/auth/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -90,17 +89,13 @@ 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>;

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,
Expand Down Expand Up @@ -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;

Expand Down
14 changes: 2 additions & 12 deletions src/components/structures/auth/Registration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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);
Expand All @@ -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,
});
}

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -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\".",
Expand Down
10 changes: 0 additions & 10 deletions src/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ export enum LabGroup {

export enum Features {
NotificationSettings2 = "feature_notification_settings2",
OidcNativeFlow = "feature_oidc_native_flow",
ReleaseAnnouncement = "feature_release_announcement",
}

Expand Down Expand Up @@ -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}
*/
Expand Down
4 changes: 0 additions & 4 deletions test/unit-tests/components/structures/UserMenu-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 0 additions & 4 deletions test/unit-tests/components/structures/auth/Login-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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(() => {
Expand Down
58 changes: 18 additions & 40 deletions test/unit-tests/components/structures/auth/Registration-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => ({
Expand Down Expand Up @@ -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", () => {
Expand Down

0 comments on commit db02f26

Please sign in to comment.