Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
OIDC: use delegated auth account URL from OidcClientStore (#11723)
Browse files Browse the repository at this point in the history 
* test persistCredentials without a pickle key

* test setLoggedIn with pickle key

* lint

* type error

* extract token persisting code into function, persist refresh token

* store has_refresh_token too

* pass refreshToken from oidcAuthGrant into credentials

* rest restore session with pickle key

* retreive stored refresh token and add to credentials

* extract token decryption into function

* remove TODO

* very messy poc

* utils to persist clientId and issuer after oidc authentication

* add dep oidc-client-ts

* persist issuer and clientId after successful oidc auth

* add OidcClientStore

* comments and tidy

* expose getters for stored refresh and access tokens in Lifecycle

* revoke tokens with oidc provider

* test logout action in MatrixChat

* comments

* prettier

* test OidcClientStore.revokeTokens

* put pickle key destruction back

* comment pedantry

* working refresh without persistence

* extract token persistence functions to utils

* add sugar

* implement TokenRefresher class with persistence

* tidying

* persist idTokenClaims

* persist idTokenClaims

* tests

* remove unused cde

* create token refresher during doSetLoggedIn

* tidying

* also tidying

* OidcClientStore.initClient use stored issuer when client well known unavailable

* test Lifecycle.logout

* update Lifecycle test replaceUsingCreds calls

* fix test

* add sdkContext to UserSettingsDialog

* use sdkContext and oidcClientStore in session manager

* use sdkContext and OidcClientStore in generalusersettingstab

* tidy

* test tokenrefresher creation in login flow

* test token refresher

* Update src/utils/oidc/TokenRefresher.ts

Co-authored-by: Richard van der Hoff <[email protected]>

* use literal value for m.authentication

Co-authored-by: Richard van der Hoff <[email protected]>

* improve comments

* fix test mock, comment

* typo

* add sdkContext to SoftLogout, pass oidcClientStore to logout

* fullstops

* comments

* fussy comment formatting

---------

Co-authored-by: Richard van der Hoff <[email protected]>
  • Loading branch information
@richvdh
Kerry and richvdh authored Oct 15, 2023
1 parent 84ca519 commit d9d52fb
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 177 deletions.
2 changes: 1 addition & 1 deletion src/components/structures/MatrixChat.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ export default class MatrixChat extends React.PureComponent<IProps, IState> {
const tabPayload = payload as OpenToTabPayload;
Modal.createDialog(
UserSettingsDialog,
{ initialTabId: tabPayload.initialTabId as UserTab },
{ initialTabId: tabPayload.initialTabId as UserTab, sdkContext: this.stores },
/*className=*/ undefined,
/*isPriority=*/ false,
/*isStatic=*/ true,
Expand Down
35 changes: 21 additions & 14 deletions src/components/views/dialogs/UserSettingsDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import KeyboardUserSettingsTab from "../settings/tabs/user/KeyboardUserSettingsT
import SessionManagerTab from "../settings/tabs/user/SessionManagerTab";
import { UserTab } from "./UserTab";
import { NonEmptyArray } from "../../../@types/common";
import { SDKContext, SdkContextClass } from "../../../contexts/SDKContext";

interface IProps {
initialTabId?: UserTab;
sdkContext: SdkContextClass;
onFinished(): void;
}

Expand Down Expand Up @@ -197,20 +199,25 @@ export default class UserSettingsDialog extends React.Component<IProps, IState>

public render(): React.ReactNode {
return (
<BaseDialog
className="mx_UserSettingsDialog"
hasCancel={true}
onFinished={this.props.onFinished}
title={_t("common|settings")}
>
<div className="mx_SettingsDialog_content">
<TabbedView
tabs={this.getTabs()}
initialTabId={this.props.initialTabId}
screenName="UserSettings"
/>
</div>
</BaseDialog>
// XXX: SDKContext is provided within the LoggedInView subtree.
// Modals function outside the MatrixChat React tree, so sdkContext is reprovided here to simulate that.
// The longer term solution is to move our ModalManager into the React tree to inherit contexts properly.
<SDKContext.Provider value={this.props.sdkContext}>
<BaseDialog
className="mx_UserSettingsDialog"
hasCancel={true}
onFinished={this.props.onFinished}
title={_t("common|settings")}
>
<div className="mx_SettingsDialog_content">
<TabbedView
tabs={this.getTabs()}
initialTabId={this.props.initialTabId}
screenName="UserSettings"
/>
</div>
</BaseDialog>
</SDKContext.Provider>
);
}
}
5 changes: 3 additions & 2 deletions src/components/views/settings/devices/useOwnDevices.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ import { VerificationRequest } from "matrix-js-sdk/src/crypto-api";
import { logger } from "matrix-js-sdk/src/logger";
import { CryptoEvent } from "matrix-js-sdk/src/crypto";

import MatrixClientContext from "../../../../contexts/MatrixClientContext";
import { _t } from "../../../../languageHandler";
import { getDeviceClientInformation, pruneClientInformation } from "../../../../utils/device/clientInformation";
import { DevicesDictionary, ExtendedDevice, ExtendedDeviceAppInfo } from "./types";
import { useEventEmitter } from "../../../../hooks/useEventEmitter";
import { parseUserAgent } from "../../../../utils/device/parseUserAgent";
import { isDeviceVerified } from "../../../../utils/device/isDeviceVerified";
import { SDKContext } from "../../../../contexts/SDKContext";

const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyDevice): ExtendedDeviceAppInfo => {
const { name, version, url } = getDeviceClientInformation(matrixClient, device.device_id);
Expand Down Expand Up @@ -90,7 +90,8 @@ export type DevicesState = {
supportsMSC3881?: boolean | undefined;
};
export const useOwnDevices = (): DevicesState => {
const matrixClient = useContext(MatrixClientContext);
const sdkContext = useContext(SDKContext);
const matrixClient = sdkContext.client!;

const currentDeviceId = matrixClient.getDeviceId()!;
const userId = matrixClient.getSafeUserId();
Expand Down
27 changes: 14 additions & 13 deletions src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ import SettingsSubsection, { SettingsSubsectionText } from "../../shared/Setting
import { SettingsSubsectionHeading } from "../../shared/SettingsSubsectionHeading";
import Heading from "../../../typography/Heading";
import InlineSpinner from "../../../elements/InlineSpinner";
import MatrixClientContext from "../../../../../contexts/MatrixClientContext";
import { ThirdPartyIdentifier } from "../../../../../AddThreepid";
import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl";
import { SDKContext } from "../../../../../contexts/SDKContext";

interface IProps {
closeSettingsFn: () => void;
Expand Down Expand Up @@ -94,20 +93,22 @@ interface IState {
}

export default class GeneralUserSettingsTab extends React.Component<IProps, IState> {
public static contextType = MatrixClientContext;
public context!: React.ContextType<typeof MatrixClientContext>;
public static contextType = SDKContext;
public context!: React.ContextType<typeof SDKContext>;

private readonly dispatcherRef: string;

public constructor(props: IProps, context: React.ContextType<typeof MatrixClientContext>) {
public constructor(props: IProps, context: React.ContextType<typeof SDKContext>) {
super(props);
this.context = context;

const cli = this.context.client!;

this.state = {
language: languageHandler.getCurrentLanguage(),
spellCheckEnabled: false,
spellCheckLanguages: [],
haveIdServer: Boolean(this.context.getIdentityServerUrl()),
haveIdServer: Boolean(cli.getIdentityServerUrl()),
idServerHasUnsignedTerms: false,
requiredPolicyInfo: {
// This object is passed along to a component for handling
Expand Down Expand Up @@ -150,7 +151,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta

private onAction = (payload: ActionPayload): void => {
if (payload.action === "id_server_changed") {
this.setState({ haveIdServer: Boolean(this.context.getIdentityServerUrl()) });
this.setState({ haveIdServer: Boolean(this.context.client!.getIdentityServerUrl()) });
this.getThreepidState();
}
};
Expand All @@ -164,7 +165,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
};

private async getCapabilities(): Promise<void> {
const cli = this.context;
const cli = this.context.client!;

const capabilities = await cli.getCapabilities(); // this is cached
const changePasswordCap = capabilities["m.change_password"];
Expand All @@ -174,7 +175,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
// the enabled flag value.
const canChangePassword = !changePasswordCap || changePasswordCap["enabled"] !== false;

const externalAccountManagementUrl = getDelegatedAuthAccountUrl(cli.getClientWellKnown());
const externalAccountManagementUrl = this.context.oidcClientStore.accountManagementEndpoint;
// https://spec.matrix.org/v1.7/client-server-api/#m3pid_changes-capability
// We support as far back as v1.1 which doesn't have m.3pid_changes
// so the behaviour for when it is missing has to be assume true
Expand All @@ -184,7 +185,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
}

private async getThreepidState(): Promise<void> {
const cli = this.context;
const cli = this.context.client!;

// Check to see if terms need accepting
this.checkTerms();
Expand All @@ -195,7 +196,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
try {
threepids = await getThreepidsWithBindStatus(cli);
} catch (e) {
const idServerUrl = this.context.getIdentityServerUrl();
const idServerUrl = cli.getIdentityServerUrl();
logger.warn(
`Unable to reach identity server at ${idServerUrl} to check ` + `for 3PIDs bindings in Settings`,
);
Expand All @@ -211,7 +212,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
private async checkTerms(): Promise<void> {
// By starting the terms flow we get the logic for checking which terms the user has signed
// for free. So we might as well use that for our own purposes.
const idServerUrl = this.context.getIdentityServerUrl();
const idServerUrl = this.context.client!.getIdentityServerUrl();
if (!this.state.haveIdServer || !idServerUrl) {
this.setState({ idServerHasUnsignedTerms: false });
return;
Expand All @@ -221,7 +222,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
try {
const idAccessToken = await authClient.getAccessToken({ check: false });
await startTermsFlow(
this.context,
this.context.client!,
[new Service(SERVICE_TYPES.IS, idServerUrl, idAccessToken!)],
(policiesAndServices, agreedUrls, extraClassNames) => {
return new Promise((resolve, reject) => {
Expand Down
8 changes: 4 additions & 4 deletions src/components/views/settings/tabs/user/SessionManagerTab.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";

import { _t } from "../../../../../languageHandler";
import MatrixClientContext from "../../../../../contexts/MatrixClientContext";
import Modal from "../../../../../Modal";
import SettingsSubsection from "../../shared/SettingsSubsection";
import SetupEncryptionDialog from "../../../dialogs/security/SetupEncryptionDialog";
Expand All @@ -39,8 +38,8 @@ import QuestionDialog from "../../../dialogs/QuestionDialog";
import { FilterVariation } from "../../devices/filter";
import { OtherSessionsSectionHeading } from "../../devices/OtherSessionsSectionHeading";
import { SettingsSection } from "../../shared/SettingsSection";
import { getDelegatedAuthAccountUrl } from "../../../../../utils/oidc/getDelegatedAuthAccountUrl";
import { OidcLogoutDialog } from "../../../dialogs/oidc/OidcLogoutDialog";
import { SDKContext } from "../../../../../contexts/SDKContext";

const confirmSignOut = async (sessionsToSignOutCount: number): Promise<boolean> => {
const { finished } = Modal.createDialog(QuestionDialog, {
Expand Down Expand Up @@ -167,13 +166,14 @@ const SessionManagerTab: React.FC = () => {
const filteredDeviceListRef = useRef<HTMLDivElement>(null);
const scrollIntoViewTimeoutRef = useRef<number>();

const matrixClient = useContext(MatrixClientContext);
const sdkContext = useContext(SDKContext);
const matrixClient = sdkContext.client!;
/**
* If we have a delegated auth account management URL, all sessions but the current session need to be managed in the
* delegated auth provider.
* See https://github.com/matrix-org/matrix-spec-proposals/pull/3824
*/
const delegatedAuthAccountUrl = getDelegatedAuthAccountUrl(matrixClient.getClientWellKnown());
const delegatedAuthAccountUrl = sdkContext.oidcClientStore.accountManagementEndpoint;
const disableMultipleSignout = !!delegatedAuthAccountUrl;

const userId = matrixClient?.getUserId();
Expand Down
27 changes: 0 additions & 27 deletions src/utils/oidc/getDelegatedAuthAccountUrl.ts
View file

This file was deleted.

18 changes: 12 additions & 6 deletions test/components/views/dialogs/UserSettingsDialog-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ limitations under the License.

import React, { ReactElement } from "react";
import { render } from "@testing-library/react";
import { mocked } from "jest-mock";
import { mocked, MockedObject } from "jest-mock";
import { MatrixClient } from "matrix-js-sdk/src/matrix";

import SettingsStore, { CallbackFn } from "../../../../src/settings/SettingsStore";
import SdkConfig from "../../../../src/SdkConfig";
Expand All @@ -30,6 +31,7 @@ import {
} from "../../../test-utils";
import { UIFeature } from "../../../../src/settings/UIFeature";
import { SettingLevel } from "../../../../src/settings/SettingLevel";
import { SdkContextClass } from "../../../../src/contexts/SDKContext";

mockPlatformPeg({
supportsSpellCheckSettings: jest.fn().mockReturnValue(false),
Expand All @@ -55,18 +57,22 @@ describe("<UserSettingsDialog />", () => {
const userId = "@alice:server.org";
const mockSettingsStore = mocked(SettingsStore);
const mockSdkConfig = mocked(SdkConfig);
getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
...mockClientMethodsServer(),
});
let mockClient!: MockedObject<MatrixClient>;

let sdkContext: SdkContextClass;
const defaultProps = { onFinished: jest.fn() };
const getComponent = (props: Partial<typeof defaultProps & { initialTabId?: UserTab }> = {}): ReactElement => (
<UserSettingsDialog {...defaultProps} {...props} />
<UserSettingsDialog sdkContext={sdkContext} {...defaultProps} {...props} />
);

beforeEach(() => {
jest.clearAllMocks();
mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
...mockClientMethodsServer(),
});
sdkContext = new SdkContextClass();
sdkContext.client = mockClient;
mockSettingsStore.getValue.mockReturnValue(false);
mockSettingsStore.getFeatureSettingNames.mockReturnValue([]);
mockSdkConfig.get.mockReturnValue({ brand: "Test" });
Expand Down
42 changes: 23 additions & 19 deletions test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ limitations under the License.

import { fireEvent, render, screen, within } from "@testing-library/react";
import React from "react";
import { M_AUTHENTICATION, ThreepidMedium } from "matrix-js-sdk/src/matrix";
import { ThreepidMedium } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";

import GeneralUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/GeneralUserSettingsTab";
import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext";
import { SdkContextClass, SDKContext } from "../../../../../../src/contexts/SDKContext";
import SettingsStore from "../../../../../../src/settings/SettingsStore";
import {
getMockClientWithEventEmitter,
Expand All @@ -28,6 +28,7 @@ import {
} from "../../../../../test-utils";
import { UIFeature } from "../../../../../../src/settings/UIFeature";
import { SettingLevel } from "../../../../../../src/settings/SettingLevel";
import { OidcClientStore } from "../../../../../../src/stores/oidc/OidcClientStore";

describe("<GeneralUserSettingsTab />", () => {
const defaultProps = {
Expand All @@ -44,19 +45,18 @@ describe("<GeneralUserSettingsTab />", () => {
deleteThreePid: jest.fn(),
});

let stores: SdkContextClass;

const getComponent = () => (
<MatrixClientContext.Provider value={mockClient}>
<SDKContext.Provider value={stores}>
<GeneralUserSettingsTab {...defaultProps} />
</MatrixClientContext.Provider>
</SDKContext.Provider>
);

const clientWellKnownSpy = jest.spyOn(mockClient, "getClientWellKnown");

beforeEach(() => {
jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
mockPlatformPeg();
jest.clearAllMocks();
clientWellKnownSpy.mockReturnValue({});
jest.spyOn(SettingsStore, "getValue").mockRestore();
jest.spyOn(logger, "error").mockRestore();

Expand All @@ -67,6 +67,12 @@ describe("<GeneralUserSettingsTab />", () => {
mockClient.deleteThreePid.mockResolvedValue({
id_server_unbind_result: "success",
});

stores = new SdkContextClass();
stores.client = mockClient;
// stub out this store completely to avoid mocking initialisation
const mockOidcClientStore = {} as unknown as OidcClientStore;
jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore);
});

it("does not show account management link when not available", () => {
Expand All @@ -78,12 +84,11 @@ describe("<GeneralUserSettingsTab />", () => {

it("show account management link in expected format", async () => {
const accountManagementLink = "https://id.server.org/my-account";
clientWellKnownSpy.mockReturnValue({
[M_AUTHENTICATION.name]: {
issuer: "https://id.server.org",
account: accountManagementLink,
},
});
const mockOidcClientStore = {
accountManagementEndpoint: accountManagementLink,
} as unknown as OidcClientStore;
jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore);

const { getByTestId } = render(getComponent());

// wait for well-known call to settle
Expand Down Expand Up @@ -167,12 +172,11 @@ describe("<GeneralUserSettingsTab />", () => {
(settingName) => settingName === UIFeature.Deactivate,
);
// account is managed externally when we have delegated auth configured
mockClient.getClientWellKnown.mockReturnValue({
[M_AUTHENTICATION.name]: {
issuer: "https://issuer.org",
account: "https://issuer.org/account",
},
});
const accountManagementLink = "https://id.server.org/my-account";
const mockOidcClientStore = {
accountManagementEndpoint: accountManagementLink,
} as unknown as OidcClientStore;
jest.spyOn(stores, "oidcClientStore", "get").mockReturnValue(mockOidcClientStore);
render(getComponent());

await flushPromises();
Expand Down
Loading

0 comments on commit d9d52fb

Please sign in to comment.