From 5c1b62cf992ef42e19371557dcf90f9a40ebe0b3 Mon Sep 17 00:00:00 2001 From: Kerry Date: Tue, 22 Aug 2023 18:07:10 +1200 Subject: [PATCH] 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 --- .../views/settings/account/EmailAddresses.tsx | 23 ++- .../views/settings/account/PhoneNumbers.tsx | 31 ++- .../settings/discovery/EmailAddresses.tsx | 6 +- .../views/settings/discovery/PhoneNumbers.tsx | 6 +- .../tabs/user/GeneralUserSettingsTab.tsx | 32 ++- .../tabs/user/GeneralUserSettingsTab-test.tsx | 185 +++++++++++++++++- .../GeneralUserSettingsTab-test.tsx.snap | 175 +++++++++++++++++ 7 files changed, 440 insertions(+), 18 deletions(-) diff --git a/src/components/views/settings/account/EmailAddresses.tsx b/src/components/views/settings/account/EmailAddresses.tsx index 36d00d301c..170cb40991 100644 --- a/src/components/views/settings/account/EmailAddresses.tsx +++ b/src/components/views/settings/account/EmailAddresses.tsx @@ -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 {this.props.email.address} - + {_t("Remove")} @@ -132,6 +136,10 @@ export class ExistingEmailAddress extends React.Component void; + /** + * Adding or removing emails is disabled when truthy + */ + disabled?: boolean; } interface IState { @@ -248,11 +256,18 @@ export default class EmailAddresses extends React.Component { public render(): React.ReactNode { const existingEmailElements = this.props.emails.map((e) => { - return ; + return ( + + ); }); let addButton = ( - + {_t("Add")} ); @@ -283,7 +298,7 @@ export default class EmailAddresses extends React.Component { 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} /> diff --git a/src/components/views/settings/account/PhoneNumbers.tsx b/src/components/views/settings/account/PhoneNumbers.tsx index ad1fdf7cc4..4bbdcb2c06 100644 --- a/src/components/views/settings/account/PhoneNumbers.tsx +++ b/src/components/views/settings/account/PhoneNumbers.tsx @@ -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 +{this.props.msisdn.address} - + {_t("Remove")} @@ -127,6 +131,10 @@ export class ExistingPhoneNumber extends React.Component 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 { public render(): React.ReactNode { const existingPhoneElements = this.props.msisdns.map((p) => { - return ; + return ( + + ); }); let addVerifySection = ( - + {_t("Add")} ); @@ -277,14 +292,18 @@ export default class PhoneNumbers extends React.Component { 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} /> {_t("Continue")} @@ -313,7 +332,7 @@ export default class PhoneNumbers extends React.Component { 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} diff --git a/src/components/views/settings/discovery/EmailAddresses.tsx b/src/components/views/settings/discovery/EmailAddresses.tsx index de74c59d2c..3f3dd106aa 100644 --- a/src/components/views/settings/discovery/EmailAddresses.tsx +++ b/src/components/views/settings/discovery/EmailAddresses.tsx @@ -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 {_t("Revoke")} @@ -204,6 +206,7 @@ export class EmailAddress extends React.Component {_t("Share")} @@ -221,6 +224,7 @@ export class EmailAddress extends React.Component { @@ -230,7 +234,7 @@ export default class EmailAddresses extends React.Component { content = ; } else if (this.props.emails.length > 0) { content = this.props.emails.map((e) => { - return ; + return ; }); } diff --git a/src/components/views/settings/discovery/PhoneNumbers.tsx b/src/components/views/settings/discovery/PhoneNumbers.tsx index 6627b119f3..d1002111d0 100644 --- a/src/components/views/settings/discovery/PhoneNumbers.tsx +++ b/src/components/views/settings/discovery/PhoneNumbers.tsx @@ -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 {_t("Revoke")} @@ -212,6 +214,7 @@ export class PhoneNumber extends React.Component {_t("Share")} @@ -230,6 +233,7 @@ export class PhoneNumber extends React.Component { @@ -239,7 +243,7 @@ export default class PhoneNumbers extends React.Component { content = ; } else if (this.props.msisdns.length > 0) { content = this.props.msisdns.map((e) => { - return ; + return ; }); } diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx index 678dd702c3..f17c73e5e1 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.tsx @@ -91,6 +91,7 @@ interface IState { canChangePassword: boolean; idServerName?: string; externalAccountManagementUrl?: string; + canMake3pidChanges: boolean; } export default class GeneralUserSettingsTab extends React.Component { @@ -120,6 +121,7 @@ export default class GeneralUserSettingsTab extends React.Component { @@ -323,12 +329,20 @@ export default class GeneralUserSettingsTab extends React.Component ) : ( - + ); const msisdns = this.state.loading3pids ? ( ) : ( - + ); threepidSection = ( <> @@ -463,8 +477,16 @@ export default class GeneralUserSettingsTab extends React.Component - - + + ) : null; diff --git a/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx index b5a1c2f084..3083380a97 100644 --- a/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx +++ b/test/components/views/settings/tabs/user/GeneralUserSettingsTab-test.tsx @@ -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("", () => { 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("", () => { ); - 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("", () => { 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(); + }); + }); + }); }); diff --git a/test/components/views/settings/tabs/user/__snapshots__/GeneralUserSettingsTab-test.tsx.snap b/test/components/views/settings/tabs/user/__snapshots__/GeneralUserSettingsTab-test.tsx.snap index 8da6784d2d..e2f20a35a7 100644 --- a/test/components/views/settings/tabs/user/__snapshots__/GeneralUserSettingsTab-test.tsx.snap +++ b/test/components/views/settings/tabs/user/__snapshots__/GeneralUserSettingsTab-test.tsx.snap @@ -1,5 +1,180 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` 3pids should display 3pid email addresses and phone numbers 1`] = ` +
+
+

+ Email addresses +

+
+
+
+ + test@test.io + +
+ Remove +
+
+
+
+ + +
+
+ Add +
+
+
+
+`; + +exports[` 3pids should display 3pid email addresses and phone numbers 2`] = ` +
+
+

+ Phone numbers +

+
+
+
+ + + + 123456789 + +
+ Remove +
+
+
+
+
+ +
+ +
+ + + +
+
+
+
+ Add +
+
+
+`; + exports[` Manage integrations should render manage integrations sections 1`] = `