Disable 3pid fields in settings when m.3pid_changes capability is disabled (#11430)

* check m.3pid_changes capability in user settings

* comments

* assume 3pid_changes is true for older evrsions
This commit is contained in:
Kerry 2023-08-22 18:07:10 +12:00 committed by GitHub
parent dfded8d4d3
commit 5c1b62cf99
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 440 additions and 18 deletions

View file

@ -44,6 +44,10 @@ that is available.
interface IExistingEmailAddressProps {
email: ThirdPartyIdentifier;
onRemoved: (emails: ThirdPartyIdentifier) => void;
/**
* Disallow removal of this email address when truthy
*/
disabled?: boolean;
}
interface IExistingEmailAddressState {
@ -121,7 +125,7 @@ export class ExistingEmailAddress extends React.Component<IExistingEmailAddressP
<span className="mx_GeneralUserSettingsTab_section--discovery_existing_address">
{this.props.email.address}
</span>
<AccessibleButton onClick={this.onRemove} kind="danger_sm">
<AccessibleButton onClick={this.onRemove} kind="danger_sm" disabled={this.props.disabled}>
{_t("Remove")}
</AccessibleButton>
</div>
@ -132,6 +136,10 @@ export class ExistingEmailAddress extends React.Component<IExistingEmailAddressP
interface IProps {
emails: ThirdPartyIdentifier[];
onEmailsChange: (emails: ThirdPartyIdentifier[]) => void;
/**
* Adding or removing emails is disabled when truthy
*/
disabled?: boolean;
}
interface IState {
@ -248,11 +256,18 @@ export default class EmailAddresses extends React.Component<IProps, IState> {
public render(): React.ReactNode {
const existingEmailElements = this.props.emails.map((e) => {
return <ExistingEmailAddress email={e} onRemoved={this.onRemoved} key={e.address} />;
return (
<ExistingEmailAddress
email={e}
onRemoved={this.onRemoved}
key={e.address}
disabled={this.props.disabled}
/>
);
});
let addButton = (
<AccessibleButton onClick={this.onAddClick} kind="primary">
<AccessibleButton onClick={this.onAddClick} kind="primary" disabled={this.props.disabled}>
{_t("Add")}
</AccessibleButton>
);
@ -283,7 +298,7 @@ export default class EmailAddresses extends React.Component<IProps, IState> {
type="text"
label={_t("Email Address")}
autoComplete="email"
disabled={this.state.verifying}
disabled={this.props.disabled || this.state.verifying}
value={this.state.newEmailAddress}
onChange={this.onChangeNewEmailAddress}
/>

View file

@ -39,6 +39,10 @@ This is a copy/paste of EmailAddresses, mostly.
interface IExistingPhoneNumberProps {
msisdn: ThirdPartyIdentifier;
onRemoved: (phoneNumber: ThirdPartyIdentifier) => void;
/**
* Disable removing phone number
*/
disabled?: boolean;
}
interface IExistingPhoneNumberState {
@ -116,7 +120,7 @@ export class ExistingPhoneNumber extends React.Component<IExistingPhoneNumberPro
<span className="mx_GeneralUserSettingsTab_section--discovery_existing_address">
+{this.props.msisdn.address}
</span>
<AccessibleButton onClick={this.onRemove} kind="danger_sm">
<AccessibleButton onClick={this.onRemove} kind="danger_sm" disabled={this.props.disabled}>
{_t("Remove")}
</AccessibleButton>
</div>
@ -127,6 +131,10 @@ export class ExistingPhoneNumber extends React.Component<IExistingPhoneNumberPro
interface IProps {
msisdns: ThirdPartyIdentifier[];
onMsisdnsChange: (phoneNumbers: ThirdPartyIdentifier[]) => void;
/**
* Adding or removing phone numbers is disabled when truthy
*/
disabled?: boolean;
}
interface IState {
@ -251,11 +259,18 @@ export default class PhoneNumbers extends React.Component<IProps, IState> {
public render(): React.ReactNode {
const existingPhoneElements = this.props.msisdns.map((p) => {
return <ExistingPhoneNumber msisdn={p} onRemoved={this.onRemoved} key={p.address} />;
return (
<ExistingPhoneNumber
msisdn={p}
onRemoved={this.onRemoved}
key={p.address}
disabled={this.props.disabled}
/>
);
});
let addVerifySection = (
<AccessibleButton onClick={this.onAddClick} kind="primary">
<AccessibleButton onClick={this.onAddClick} kind="primary" disabled={this.props.disabled}>
{_t("Add")}
</AccessibleButton>
);
@ -277,14 +292,18 @@ export default class PhoneNumbers extends React.Component<IProps, IState> {
type="text"
label={_t("Verification code")}
autoComplete="off"
disabled={this.state.continueDisabled}
disabled={this.props.disabled || this.state.continueDisabled}
value={this.state.newPhoneNumberCode}
onChange={this.onChangeNewPhoneNumberCode}
/>
<AccessibleButton
onClick={this.onContinueClick}
kind="primary"
disabled={this.state.continueDisabled || this.state.newPhoneNumberCode.length === 0}
disabled={
this.props.disabled ||
this.state.continueDisabled ||
this.state.newPhoneNumberCode.length === 0
}
>
{_t("Continue")}
</AccessibleButton>
@ -313,7 +332,7 @@ export default class PhoneNumbers extends React.Component<IProps, IState> {
type="text"
label={_t("Phone Number")}
autoComplete="tel-national"
disabled={this.state.verifying}
disabled={this.props.disabled || this.state.verifying}
prefixComponent={phoneCountry}
value={this.state.newPhoneNumber}
onChange={this.onChangeNewPhoneNumber}

View file

@ -46,6 +46,7 @@ TODO: Reduce all the copying between account vs. discovery components.
interface IEmailAddressProps {
email: ThirdPartyIdentifier;
disabled?: boolean;
}
interface IEmailAddressState {
@ -194,6 +195,7 @@ export class EmailAddress extends React.Component<IEmailAddressProps, IEmailAddr
className="mx_GeneralUserSettingsTab_section--discovery_existing_button"
kind="danger_sm"
onClick={this.onRevokeClick}
disabled={this.props.disabled}
>
{_t("Revoke")}
</AccessibleButton>
@ -204,6 +206,7 @@ export class EmailAddress extends React.Component<IEmailAddressProps, IEmailAddr
className="mx_GeneralUserSettingsTab_section--discovery_existing_button"
kind="primary_sm"
onClick={this.onShareClick}
disabled={this.props.disabled}
>
{_t("Share")}
</AccessibleButton>
@ -221,6 +224,7 @@ export class EmailAddress extends React.Component<IEmailAddressProps, IEmailAddr
interface IProps {
emails: ThirdPartyIdentifier[];
isLoading?: boolean;
disabled?: boolean;
}
export default class EmailAddresses extends React.Component<IProps> {
@ -230,7 +234,7 @@ export default class EmailAddresses extends React.Component<IProps> {
content = <InlineSpinner />;
} else if (this.props.emails.length > 0) {
content = this.props.emails.map((e) => {
return <EmailAddress email={e} key={e.address} />;
return <EmailAddress email={e} key={e.address} disabled={this.props.disabled} />;
});
}

View file

@ -38,6 +38,7 @@ This is a copy/paste of EmailAddresses, mostly.
interface IPhoneNumberProps {
msisdn: ThirdPartyIdentifier;
disabled?: boolean;
}
interface IPhoneNumberState {
@ -202,6 +203,7 @@ export class PhoneNumber extends React.Component<IPhoneNumberProps, IPhoneNumber
className="mx_GeneralUserSettingsTab_section--discovery_existing_button"
kind="danger_sm"
onClick={this.onRevokeClick}
disabled={this.props.disabled}
>
{_t("Revoke")}
</AccessibleButton>
@ -212,6 +214,7 @@ export class PhoneNumber extends React.Component<IPhoneNumberProps, IPhoneNumber
className="mx_GeneralUserSettingsTab_section--discovery_existing_button"
kind="primary_sm"
onClick={this.onShareClick}
disabled={this.props.disabled}
>
{_t("Share")}
</AccessibleButton>
@ -230,6 +233,7 @@ export class PhoneNumber extends React.Component<IPhoneNumberProps, IPhoneNumber
interface IProps {
msisdns: ThirdPartyIdentifier[];
isLoading?: boolean;
disabled?: boolean;
}
export default class PhoneNumbers extends React.Component<IProps> {
@ -239,7 +243,7 @@ export default class PhoneNumbers extends React.Component<IProps> {
content = <InlineSpinner />;
} else if (this.props.msisdns.length > 0) {
content = this.props.msisdns.map((e) => {
return <PhoneNumber msisdn={e} key={e.address} />;
return <PhoneNumber msisdn={e} key={e.address} disabled={this.props.disabled} />;
});
}

View file

@ -91,6 +91,7 @@ interface IState {
canChangePassword: boolean;
idServerName?: string;
externalAccountManagementUrl?: string;
canMake3pidChanges: boolean;
}
export default class GeneralUserSettingsTab extends React.Component<IProps, IState> {
@ -120,6 +121,7 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
msisdns: [],
loading3pids: true, // whether or not the emails and msisdns have been loaded
canChangePassword: false,
canMake3pidChanges: false,
};
this.dispatcherRef = dis.register(this.onAction);
@ -174,8 +176,12 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
const canChangePassword = !changePasswordCap || changePasswordCap["enabled"] !== false;
const externalAccountManagementUrl = getDelegatedAuthAccountUrl(cli.getClientWellKnown());
// 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
const canMake3pidChanges = !capabilities["m.3pid_changes"] || capabilities["m.3pid_changes"].enabled === true;
this.setState({ canChangePassword, externalAccountManagementUrl });
this.setState({ canChangePassword, externalAccountManagementUrl, canMake3pidChanges });
}
private async getThreepidState(): Promise<void> {
@ -323,12 +329,20 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
const emails = this.state.loading3pids ? (
<InlineSpinner />
) : (
<AccountEmailAddresses emails={this.state.emails} onEmailsChange={this.onEmailsChange} />
<AccountEmailAddresses
emails={this.state.emails}
onEmailsChange={this.onEmailsChange}
disabled={!this.state.canMake3pidChanges}
/>
);
const msisdns = this.state.loading3pids ? (
<InlineSpinner />
) : (
<AccountPhoneNumbers msisdns={this.state.msisdns} onMsisdnsChange={this.onMsisdnsChange} />
<AccountPhoneNumbers
msisdns={this.state.msisdns}
onMsisdnsChange={this.onMsisdnsChange}
disabled={!this.state.canMake3pidChanges}
/>
);
threepidSection = (
<>
@ -463,8 +477,16 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
const threepidSection = this.state.haveIdServer ? (
<>
<DiscoveryEmailAddresses emails={this.state.emails} isLoading={this.state.loading3pids} />
<DiscoveryPhoneNumbers msisdns={this.state.msisdns} isLoading={this.state.loading3pids} />
<DiscoveryEmailAddresses
emails={this.state.emails}
isLoading={this.state.loading3pids}
disabled={!this.state.canMake3pidChanges}
/>
<DiscoveryPhoneNumbers
msisdns={this.state.msisdns}
isLoading={this.state.loading3pids}
disabled={!this.state.canMake3pidChanges}
/>
</>
) : null;

View file

@ -15,6 +15,7 @@ import { fireEvent, render, screen, within } from "@testing-library/react";
import React from "react";
import { M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import { ThreepidMedium } from "matrix-js-sdk/src/@types/threepids";
import GeneralUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/GeneralUserSettingsTab";
import MatrixClientContext from "../../../../../../src/contexts/MatrixClientContext";
@ -38,6 +39,10 @@ describe("<GeneralUserSettingsTab />", () => {
const mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
...mockClientMethodsServer(),
getCapabilities: jest.fn(),
getThreePids: jest.fn(),
getIdentityServerUrl: jest.fn(),
deleteThreePid: jest.fn(),
});
const getComponent = () => (
@ -46,15 +51,23 @@ describe("<GeneralUserSettingsTab />", () => {
</MatrixClientContext.Provider>
);
jest.spyOn(SettingsStore, "getValue").mockReturnValue(false);
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();
mockClient.getCapabilities.mockResolvedValue({});
mockClient.getThreePids.mockResolvedValue({
threepids: [],
});
mockClient.deleteThreePid.mockResolvedValue({
id_server_unbind_result: "success",
});
});
it("does not show account management link when not available", () => {
@ -159,4 +172,174 @@ describe("<GeneralUserSettingsTab />", () => {
expect(screen.getByText("Deactivate account").parentElement!).toMatchSnapshot();
});
});
describe("3pids", () => {
beforeEach(() => {
mockClient.getCapabilities.mockResolvedValue({
"m.3pid_changes": {
enabled: true,
},
});
mockClient.getThreePids.mockResolvedValue({
threepids: [
{
medium: ThreepidMedium.Email,
address: "test@test.io",
validated_at: 1685067124552,
added_at: 1685067124552,
},
{
medium: ThreepidMedium.Phone,
address: "123456789",
validated_at: 1685067124552,
added_at: 1685067124552,
},
],
});
mockClient.getIdentityServerUrl.mockReturnValue(undefined);
});
it("should show loaders while 3pids load", () => {
render(getComponent());
expect(
within(screen.getByTestId("mx_AccountEmailAddresses")).getByLabelText("Loading…"),
).toBeInTheDocument();
expect(within(screen.getByTestId("mx_AccountPhoneNumbers")).getByLabelText("Loading…")).toBeInTheDocument();
});
it("should display 3pid email addresses and phone numbers", async () => {
render(getComponent());
await flushPromises();
expect(screen.getByTestId("mx_AccountEmailAddresses")).toMatchSnapshot();
expect(screen.getByTestId("mx_AccountPhoneNumbers")).toMatchSnapshot();
});
it("should allow removing an existing email addresses", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountEmailAddresses");
fireEvent.click(within(section).getByText("Remove"));
// confirm removal
expect(screen.getByText("Remove test@test.io?")).toBeInTheDocument();
fireEvent.click(within(section).getByText("Remove"));
expect(mockClient.deleteThreePid).toHaveBeenCalledWith(ThreepidMedium.Email, "test@test.io");
});
it("should allow adding a new email address", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountEmailAddresses");
// just check the fields are enabled
expect(within(section).getByLabelText("Email Address")).not.toBeDisabled();
expect(within(section).getByText("Add")).not.toHaveAttribute("aria-disabled");
});
it("should allow removing an existing phone number", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountPhoneNumbers");
fireEvent.click(within(section).getByText("Remove"));
// confirm removal
expect(screen.getByText("Remove 123456789?")).toBeInTheDocument();
fireEvent.click(within(section).getByText("Remove"));
expect(mockClient.deleteThreePid).toHaveBeenCalledWith(ThreepidMedium.Phone, "123456789");
});
it("should allow adding a new phone number", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountPhoneNumbers");
// just check the fields are enabled
expect(within(section).getByLabelText("Phone Number")).not.toBeDisabled();
});
it("should allow 3pid changes when capabilities does not have 3pid_changes", async () => {
// 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
mockClient.getCapabilities.mockResolvedValue({});
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountEmailAddresses");
// just check the fields are enabled
expect(within(section).getByLabelText("Email Address")).not.toBeDisabled();
expect(within(section).getByText("Add")).not.toHaveAttribute("aria-disabled");
});
describe("when 3pid changes capability is disabled", () => {
beforeEach(() => {
mockClient.getCapabilities.mockResolvedValue({
"m.3pid_changes": {
enabled: false,
},
});
});
it("should not allow removing email addresses", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountEmailAddresses");
expect(within(section).getByText("Remove")).toHaveAttribute("aria-disabled");
});
it("should not allow adding a new email addresses", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountEmailAddresses");
// fields are not enabled
expect(within(section).getByLabelText("Email Address")).toBeDisabled();
expect(within(section).getByText("Add")).toHaveAttribute("aria-disabled");
});
it("should not allow removing phone numbers", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountPhoneNumbers");
expect(within(section).getByText("Remove")).toHaveAttribute("aria-disabled");
});
it("should not allow adding a new phone number", async () => {
render(getComponent());
await flushPromises();
const section = screen.getByTestId("mx_AccountPhoneNumbers");
expect(within(section).getByLabelText("Phone Number")).toBeDisabled();
});
});
});
});

View file

@ -1,5 +1,180 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`<GeneralUserSettingsTab /> 3pids should display 3pid email addresses and phone numbers 1`] = `
<div
class="mx_SettingsSubsection"
data-testid="mx_AccountEmailAddresses"
>
<div
class="mx_SettingsSubsectionHeading"
>
<h3
class="mx_Heading_h4 mx_SettingsSubsectionHeading_heading"
>
Email addresses
</h3>
</div>
<div
class="mx_SettingsSubsection_content mx_SettingsSubsection_contentStretch"
>
<div
class="mx_GeneralUserSettingsTab_section--discovery_existing"
>
<span
class="mx_GeneralUserSettingsTab_section--discovery_existing_address"
>
test@test.io
</span>
<div
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_danger_sm"
role="button"
tabindex="0"
>
Remove
</div>
</div>
<form
autocomplete="off"
novalidate=""
>
<div
class="mx_Field mx_Field_input"
>
<input
autocomplete="email"
id="mx_Field_57"
label="Email Address"
placeholder="Email Address"
type="text"
value=""
/>
<label
for="mx_Field_57"
>
Email Address
</label>
</div>
<div
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
role="button"
tabindex="0"
>
Add
</div>
</form>
</div>
</div>
`;
exports[`<GeneralUserSettingsTab /> 3pids should display 3pid email addresses and phone numbers 2`] = `
<div
class="mx_SettingsSubsection"
data-testid="mx_AccountPhoneNumbers"
>
<div
class="mx_SettingsSubsectionHeading"
>
<h3
class="mx_Heading_h4 mx_SettingsSubsectionHeading_heading"
>
Phone numbers
</h3>
</div>
<div
class="mx_SettingsSubsection_content mx_SettingsSubsection_contentStretch"
>
<div
class="mx_GeneralUserSettingsTab_section--discovery_existing"
>
<span
class="mx_GeneralUserSettingsTab_section--discovery_existing_address"
>
+
123456789
</span>
<div
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_danger_sm"
role="button"
tabindex="0"
>
Remove
</div>
</div>
<form
autocomplete="off"
class="mx_PhoneNumbers_new"
novalidate=""
>
<div
class="mx_PhoneNumbers_input"
>
<div
class="mx_Field mx_Field_input mx_Field_labelAlwaysTopLeft"
>
<span
class="mx_Field_prefix"
>
<div
class="mx_Dropdown mx_PhoneNumbers_country mx_CountryDropdown"
>
<div
aria-describedby="mx_CountryDropdown_value"
aria-expanded="false"
aria-haspopup="listbox"
aria-label="Country Dropdown"
aria-owns="mx_CountryDropdown_input"
class="mx_AccessibleButton mx_Dropdown_input mx_no_textinput"
role="button"
tabindex="0"
>
<div
class="mx_Dropdown_option"
id="mx_CountryDropdown_value"
>
<span
class="mx_CountryDropdown_shortOption"
>
<div
class="mx_Dropdown_option_emoji"
>
🇺🇸
</div>
+1
</span>
</div>
<span
class="mx_Dropdown_arrow"
/>
</div>
</div>
</span>
<input
autocomplete="tel-national"
id="mx_Field_58"
label="Phone Number"
placeholder="Phone Number"
type="text"
value=""
/>
<label
for="mx_Field_58"
>
Phone Number
</label>
</div>
</div>
</form>
<div
class="mx_AccessibleButton mx_AccessibleButton_hasKind mx_AccessibleButton_kind_primary"
role="button"
tabindex="0"
>
Add
</div>
</div>
</div>
`;
exports[`<GeneralUserSettingsTab /> Manage integrations should render manage integrations sections 1`] = `
<label
class="mx_SetIntegrationManager"