Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delabs native OIDC support #28615

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -93,17 +92,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 @@ -361,10 +356,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 @@ -133,8 +132,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 @@ -153,14 +150,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 @@ -230,10 +223,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 @@ -1465,8 +1465,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
Loading