diff --git a/src/Terms.ts b/src/Terms.ts index 02b67545da..a1870ebe21 100644 --- a/src/Terms.ts +++ b/src/Terms.ts @@ -7,11 +7,12 @@ Please see LICENSE files in the repository root for full details. */ import classNames from "classnames"; -import { SERVICE_TYPES, MatrixClient } from "matrix-js-sdk/src/matrix"; +import { SERVICE_TYPES, MatrixClient, Terms, Policy, InternationalisedPolicy } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import Modal from "./Modal"; import TermsDialog from "./components/views/dialogs/TermsDialog"; +import { pickBestLanguage } from "./languageHandler.tsx"; export class TermsNotSignedError extends Error {} @@ -32,23 +33,8 @@ export class Service { ) {} } -export interface LocalisedPolicy { - name: string; - url: string; -} - -export interface Policy { - // @ts-ignore: No great way to express indexed types together with other keys - version: string; - [lang: string]: LocalisedPolicy; -} - -export type Policies = { - [policy: string]: Policy; -}; - export type ServicePolicyPair = { - policies: Policies; + policies: Terms["policies"]; service: Service; }; @@ -58,6 +44,11 @@ export type TermsInteractionCallback = ( extraClassNames?: string, ) => Promise; +export function pickBestPolicyLanguage(policy: Policy): InternationalisedPolicy | undefined { + const termsLang = pickBestLanguage(Object.keys(policy).filter((k) => k !== "version")); + return policy[termsLang]; +} + /** * Start a flow where the user is presented with terms & conditions for some services * @@ -96,7 +87,7 @@ export async function startTermsFlow( * } */ - const terms: { policies: Policies }[] = await Promise.all(termsPromises); + const terms: Terms[] = await Promise.all(termsPromises); const policiesAndServicePairs = terms.map((t, i) => { return { service: services[i], policies: t.policies }; }); @@ -113,11 +104,11 @@ export async function startTermsFlow( // things they've not agreed to yet. const unagreedPoliciesAndServicePairs: ServicePolicyPair[] = []; for (const { service, policies } of policiesAndServicePairs) { - const unagreedPolicies: Policies = {}; + const unagreedPolicies: Terms["policies"] = {}; for (const [policyName, policy] of Object.entries(policies)) { let policyAgreed = false; for (const lang of Object.keys(policy)) { - if (lang === "version") continue; + if (lang === "version" || typeof policy[lang] === "string") continue; if (agreedUrlSet.has(policy[lang].url)) { policyAgreed = true; break; @@ -154,7 +145,7 @@ export async function startTermsFlow( const urlsForService = Array.from(agreedUrlSet).filter((url) => { for (const policy of Object.values(policiesAndService.policies)) { for (const lang of Object.keys(policy)) { - if (lang === "version") continue; + if (lang === "version" || typeof policy[lang] === "string") continue; if (policy[lang].url === url) return true; } } diff --git a/src/components/views/auth/InteractiveAuthEntryComponents.tsx b/src/components/views/auth/InteractiveAuthEntryComponents.tsx index d493e5c3ca..68ea886554 100644 --- a/src/components/views/auth/InteractiveAuthEntryComponents.tsx +++ b/src/components/views/auth/InteractiveAuthEntryComponents.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import classNames from "classnames"; -import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { InternationalisedPolicy, Terms, MatrixClient } from "matrix-js-sdk/src/matrix"; import { AuthType, AuthDict, IInputs, IStageStatus } from "matrix-js-sdk/src/interactive-auth"; import { logger } from "matrix-js-sdk/src/logger"; import React, { ChangeEvent, createRef, FormEvent, Fragment } from "react"; @@ -16,14 +16,13 @@ import PopOutIcon from "@vector-im/compound-design-tokens/assets/web/icons/pop-o import EmailPromptIcon from "../../../../res/img/element-icons/email-prompt.svg"; import { _t } from "../../../languageHandler"; -import SettingsStore from "../../../settings/SettingsStore"; -import { LocalisedPolicy, Policies } from "../../../Terms"; import { AuthHeaderModifier } from "../../structures/auth/header/AuthHeaderModifier"; import AccessibleButton, { AccessibleButtonKind, ButtonEvent } from "../elements/AccessibleButton"; import Field from "../elements/Field"; import Spinner from "../elements/Spinner"; import CaptchaForm from "./CaptchaForm"; import { Flex } from "../../utils/Flex"; +import { pickBestPolicyLanguage } from "../../../Terms.ts"; /* This file contains a collection of components which are used by the * InteractiveAuth to prompt the user to enter the information needed @@ -235,12 +234,10 @@ export class RecaptchaAuthEntry extends React.Component; } -interface LocalisedPolicyWithId extends LocalisedPolicy { +interface LocalisedPolicyWithId extends InternationalisedPolicy { id: string; } @@ -278,7 +275,6 @@ export class TermsAuthEntry extends React.Component = {}; const pickedPolicies: { id: string; @@ -287,17 +283,7 @@ export class TermsAuthEntry extends React.Component e !== "version"); - langPolicy = firstLang ? policy[firstLang] : undefined; - } + const langPolicy = pickBestPolicyLanguage(policy); if (!langPolicy) throw new Error("Failed to find a policy to show the user"); initToggles[policyId] = false; diff --git a/src/components/views/dialogs/TermsDialog.tsx b/src/components/views/dialogs/TermsDialog.tsx index c71a77c85b..f1cdb3f199 100644 --- a/src/components/views/dialogs/TermsDialog.tsx +++ b/src/components/views/dialogs/TermsDialog.tsx @@ -9,10 +9,10 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { SERVICE_TYPES } from "matrix-js-sdk/src/matrix"; -import { _t, pickBestLanguage } from "../../../languageHandler"; +import { _t } from "../../../languageHandler"; import DialogButtons from "../elements/DialogButtons"; import BaseDialog from "./BaseDialog"; -import { ServicePolicyPair } from "../../../Terms"; +import { pickBestPolicyLanguage, ServicePolicyPair } from "../../../Terms"; import ExternalLink from "../elements/ExternalLink"; import { parseUrl } from "../../../utils/UrlUtils"; @@ -126,8 +126,8 @@ export default class TermsDialog extends React.PureComponent k !== "version")); + const internationalisedPolicy = pickBestPolicyLanguage(policyValues[i]); + if (!internationalisedPolicy) continue; let serviceName: JSX.Element | undefined; let summary: JSX.Element | undefined; if (i === 0) { @@ -136,19 +136,19 @@ export default class TermsDialog extends React.PureComponent + {serviceName} {summary} - - {termDoc[termsLang].name} + + {internationalisedPolicy.name} , @@ -164,7 +164,7 @@ export default class TermsDialog extends React.PureComponent { agreedUrls: null, // From the startTermsFlow callback resolve: null, // Promise resolve function for startTermsFlow callback }); - const [hasTerms, setHasTerms] = useState(false); + const [mustAgreeToTerms, setMustAgreeToTerms] = useState(false); const getThreepidState = useCallback(async () => { setIsLoadingThreepids(true); @@ -103,7 +103,7 @@ export const DiscoverySettings: React.FC = () => { (policiesAndServices, agreedUrls, extraClassNames) => { return new Promise((resolve) => { setIdServerName(abbreviateUrl(idServerUrl)); - setHasTerms(true); + setMustAgreeToTerms(true); setRequiredPolicyInfo({ policiesAndServices, agreedUrls, @@ -113,7 +113,7 @@ export const DiscoverySettings: React.FC = () => { }, ); // User accepted all terms - setHasTerms(false); + setMustAgreeToTerms(false); } catch (e) { logger.warn( `Unable to reach identity server at ${idServerUrl} to check ` + `for terms in Settings`, @@ -126,7 +126,7 @@ export const DiscoverySettings: React.FC = () => { if (!SettingsStore.getValue(UIFeature.ThirdPartyID)) return null; - if (hasTerms && requiredPolicyInfo.policiesAndServices) { + if (mustAgreeToTerms && requiredPolicyInfo.policiesAndServices) { const intro = ( {_t("settings|general|discovery_needs_terms", { serverName: idServerName })} @@ -160,7 +160,7 @@ export const DiscoverySettings: React.FC = () => { medium={ThreepidMedium.Email} threepids={emails} onChange={getThreepidState} - disabled={!hasTerms} + disabled={mustAgreeToTerms} isLoading={isLoadingThreepids} /> @@ -174,7 +174,7 @@ export const DiscoverySettings: React.FC = () => { medium={ThreepidMedium.Phone} threepids={phoneNumbers} onChange={getThreepidState} - disabled={!hasTerms} + disabled={mustAgreeToTerms} isLoading={isLoadingThreepids} /> diff --git a/src/components/views/terms/InlineTermsAgreement.tsx b/src/components/views/terms/InlineTermsAgreement.tsx index 2e4f48712f..14a9199f80 100644 --- a/src/components/views/terms/InlineTermsAgreement.tsx +++ b/src/components/views/terms/InlineTermsAgreement.tsx @@ -8,11 +8,11 @@ Please see LICENSE files in the repository root for full details. import React from "react"; -import { _t, pickBestLanguage } from "../../../languageHandler"; +import { _t } from "../../../languageHandler"; import { objectClone } from "../../../utils/objects"; import StyledCheckbox from "../elements/StyledCheckbox"; import AccessibleButton from "../elements/AccessibleButton"; -import { ServicePolicyPair } from "../../../Terms"; +import { pickBestPolicyLanguage, ServicePolicyPair } from "../../../Terms"; interface IProps { policiesAndServicePairs: ServicePolicyPair[]; @@ -47,11 +47,12 @@ export default class InlineTermsAgreement extends React.Component p !== "version")); + const internationalisedPolicy = pickBestPolicyLanguage(policy); + if (!internationalisedPolicy) continue; const renderablePolicy: Policy = { checked: false, - url: policy[language].url, - name: policy[language].name, + url: internationalisedPolicy.url, + name: internationalisedPolicy.name, }; policies.push(renderablePolicy); } diff --git a/src/utils/IdentityServerUtils.ts b/src/utils/IdentityServerUtils.ts index fd3de6b2f4..aa1153acfd 100644 --- a/src/utils/IdentityServerUtils.ts +++ b/src/utils/IdentityServerUtils.ts @@ -6,11 +6,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { SERVICE_TYPES, HTTPError, MatrixClient } from "matrix-js-sdk/src/matrix"; +import { SERVICE_TYPES, HTTPError, MatrixClient, Terms } from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import SdkConfig from "../SdkConfig"; -import { Policies } from "../Terms"; export function getDefaultIdentityServerUrl(): string | undefined { return SdkConfig.get("validated_server_config")?.isUrl; @@ -25,7 +24,7 @@ export function setToDefaultIdentityServer(matrixClient: MatrixClient): void { } export async function doesIdentityServerHaveTerms(matrixClient: MatrixClient, fullUrl: string): Promise { - let terms: { policies?: Policies } | null; + let terms: Partial | null; try { terms = await matrixClient.getTerms(SERVICE_TYPES.IS, fullUrl); } catch (e) { diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 18da07db92..f752daf530 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -217,6 +217,7 @@ export function createTestClient(): MatrixClient { registerWithIdentityServer: jest.fn().mockResolvedValue({}), getIdentityAccount: jest.fn().mockResolvedValue({}), getTerms: jest.fn().mockResolvedValue({ policies: [] }), + agreeToTerms: jest.fn(), doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(undefined), isVersionSupported: jest.fn().mockResolvedValue(undefined), getPushRules: jest.fn().mockResolvedValue(undefined), diff --git a/test/unit-tests/ScalarAuthClient-test.ts b/test/unit-tests/ScalarAuthClient-test.ts index bbf29a8652..4221fca0ed 100644 --- a/test/unit-tests/ScalarAuthClient-test.ts +++ b/test/unit-tests/ScalarAuthClient-test.ts @@ -97,7 +97,7 @@ describe("ScalarAuthClient", function () { body: { errcode: "M_TERMS_NOT_SIGNED" }, }); sac.exchangeForScalarToken = jest.fn(() => Promise.resolve("testtoken1")); - mocked(client.getTerms).mockResolvedValue({ policies: [] }); + mocked(client.getTerms).mockResolvedValue({ policies: {} }); await expect(sac.registerForToken()).resolves.toBe("testtoken1"); }); diff --git a/test/unit-tests/Terms-test.tsx b/test/unit-tests/Terms-test.tsx index 9fc29bde9a..042e0c7826 100644 --- a/test/unit-tests/Terms-test.tsx +++ b/test/unit-tests/Terms-test.tsx @@ -6,9 +6,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { MatrixEvent, EventType, SERVICE_TYPES } from "matrix-js-sdk/src/matrix"; +import { EventType, MatrixEvent, Policy, SERVICE_TYPES, Terms } from "matrix-js-sdk/src/matrix"; +import { screen, within } from "jest-matrix-react"; -import { startTermsFlow, Service } from "../../src/Terms"; +import { dialogTermsInteractionCallback, Service, startTermsFlow } from "../../src/Terms"; import { getMockClientWithEventEmitter } from "../test-utils"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; @@ -18,7 +19,7 @@ const POLICY_ONE = { name: "The first policy", url: "http://example.com/one", }, -}; +} satisfies Policy; const POLICY_TWO = { version: "IX", @@ -26,7 +27,7 @@ const POLICY_TWO = { name: "The second policy", url: "http://example.com/two", }, -}; +} satisfies Policy; const IM_SERVICE_ONE = new Service(SERVICE_TYPES.IM, "https://imone.test", "a token token"); const IM_SERVICE_TWO = new Service(SERVICE_TYPES.IM, "https://imtwo.test", "a token token"); @@ -42,7 +43,7 @@ describe("Terms", function () { beforeEach(function () { jest.clearAllMocks(); mockClient.getAccountData.mockReturnValue(undefined); - mockClient.getTerms.mockResolvedValue(null); + mockClient.getTerms.mockResolvedValue({ policies: {} }); mockClient.setAccountData.mockResolvedValue({}); }); @@ -141,22 +142,25 @@ describe("Terms", function () { }); mockClient.getAccountData.mockReturnValue(directEvent); - mockClient.getTerms.mockImplementation(async (_serviceTypes: SERVICE_TYPES, baseUrl: string) => { - switch (baseUrl) { - case "https://imone.test": - return { - policies: { - policy_the_first: POLICY_ONE, - }, - }; - case "https://imtwo.test": - return { - policies: { - policy_the_second: POLICY_TWO, - }, - }; - } - }); + mockClient.getTerms.mockImplementation( + async (_serviceTypes: SERVICE_TYPES, baseUrl: string): Promise => { + switch (baseUrl) { + case "https://imone.test": + return { + policies: { + policy_the_first: POLICY_ONE, + }, + }; + case "https://imtwo.test": + return { + policies: { + policy_the_second: POLICY_TWO, + }, + }; + } + return { policies: {} }; + }, + ); const interactionCallback = jest.fn().mockResolvedValue(["http://example.com/one", "http://example.com/two"]); await startTermsFlow(mockClient, [IM_SERVICE_ONE, IM_SERVICE_TWO], interactionCallback); @@ -180,3 +184,29 @@ describe("Terms", function () { ]); }); }); + +describe("dialogTermsInteractionCallback", () => { + it("should render a dialog with the expected terms", async () => { + dialogTermsInteractionCallback( + [ + { + service: new Service(SERVICE_TYPES.IS, "http://base_url", "access_token"), + policies: { + sample: { + version: "VERSION", + en: { + name: "Terms", + url: "http://base_url/terms", + }, + }, + }, + }, + ], + [], + ); + + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByRole("link")).toHaveAttribute("href", "http://base_url/terms"); + expect(dialog).toMatchSnapshot(); + }); +}); diff --git a/test/unit-tests/__snapshots__/Terms-test.tsx.snap b/test/unit-tests/__snapshots__/Terms-test.tsx.snap new file mode 100644 index 0000000000..a35963cd51 --- /dev/null +++ b/test/unit-tests/__snapshots__/Terms-test.tsx.snap @@ -0,0 +1,113 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`dialogTermsInteractionCallback should render a dialog with the expected terms 1`] = ` +
+
+

+ Terms of Service +

+
+
+

+ To continue you need to accept the terms of this service. +

+ + + + + + + + + + + + + + + +
+ Service + + Summary + + Document + + Accept +
+
+ Identity server +
+ ( + base_url + ) +
+ 		+
+ Find others by phone or email +
+ Be found by phone or email +
+ 		+ + Terms + + + + +
+
+
+ + + + +
+
+`; diff --git a/test/unit-tests/components/views/auth/InteractiveAuthEntryComponents-test.tsx b/test/unit-tests/components/views/auth/InteractiveAuthEntryComponents-test.tsx index 41408edd19..5e145d2bf9 100644 --- a/test/unit-tests/components/views/auth/InteractiveAuthEntryComponents-test.tsx +++ b/test/unit-tests/components/views/auth/InteractiveAuthEntryComponents-test.tsx @@ -10,10 +10,12 @@ import React from "react"; import { render, screen, waitFor, act, fireEvent } from "jest-matrix-react"; import { AuthType } from "matrix-js-sdk/src/interactive-auth"; import userEvent from "@testing-library/user-event"; +import { Policy } from "matrix-js-sdk/src/matrix"; import { EmailIdentityAuthEntry, MasUnlockCrossSigningAuthEntry, + TermsAuthEntry, } from "../../../../../src/components/views/auth/InteractiveAuthEntryComponents"; import { createTestClient } from "../../../../test-utils"; @@ -99,3 +101,38 @@ describe("", () => { expect(submitAuthDict).toHaveBeenCalledWith({}); }); }); + +describe("", () => { + const renderAuth = (policy: Policy, props = {}) => { + const matrixClient = createTestClient(); + + return render( + , + ); + }; + + test("should render", () => { + const { container } = renderAuth({ + version: "alpha", + en: { + name: "Test Policy", + url: "https://example.com/en", + }, + }); + expect(container).toMatchSnapshot(); + }); +}); diff --git a/test/unit-tests/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap b/test/unit-tests/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap index 78c95a945a..99364af5d4 100644 --- a/test/unit-tests/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap +++ b/test/unit-tests/components/views/auth/__snapshots__/InteractiveAuthEntryComponents-test.tsx.snap @@ -82,3 +82,38 @@ exports[` should render 1`] = ` `; + +exports[` should render 1`] = ` +
+
+

+ Please review and accept the policies of this homeserver: +

+ +
+ Accept +
+
+
+`; diff --git a/test/unit-tests/components/views/settings/discovery/DiscoverySettings-test.tsx b/test/unit-tests/components/views/settings/discovery/DiscoverySettings-test.tsx index 404735f63a..261a6f1d72 100644 --- a/test/unit-tests/components/views/settings/discovery/DiscoverySettings-test.tsx +++ b/test/unit-tests/components/views/settings/discovery/DiscoverySettings-test.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { act, render, screen } from "jest-matrix-react"; -import { MatrixClient } from "matrix-js-sdk/src/matrix"; +import { MatrixClient, MatrixEvent, Terms, ThreepidMedium } from "matrix-js-sdk/src/matrix"; import { mocked } from "jest-mock"; import userEvent from "@testing-library/user-event"; @@ -26,6 +26,18 @@ jest.mock("../../../../../../src/IdentityAuthClient", () => })), ); +const sampleTerms = { + policies: { + terms: { version: "alpha", en: { name: "No ball games", url: "https://foobar" } }, + }, +} satisfies Terms; + +const invalidTerms = { + policies: { + terms: { version: "invalid" }, + }, +} satisfies Terms; + describe("DiscoverySettings", () => { let client: MatrixClient; @@ -51,20 +63,17 @@ describe("DiscoverySettings", () => { it("displays alert if an identity server needs terms accepting", async () => { mocked(client).getIdentityServerUrl.mockReturnValue("https://example.com"); - mocked(client).getTerms.mockResolvedValue({ - ["policies"]: { en: "No ball games" }, - }); + mocked(client).getTerms.mockResolvedValue(sampleTerms); render(, { wrapper: DiscoveryWrapper }); - await expect(await screen.findByText("Let people find you")).toBeInTheDocument(); + expect(await screen.findByText("Let people find you")).toBeInTheDocument(); + expect(screen.getByRole("link")).toHaveAttribute("href", "https://foobar"); }); it("button to accept terms is disabled if checkbox not checked", async () => { mocked(client).getIdentityServerUrl.mockReturnValue("https://example.com"); - mocked(client).getTerms.mockResolvedValue({ - ["policies"]: { en: "No ball games" }, - }); + mocked(client).getTerms.mockResolvedValue(sampleTerms); render(, { wrapper: DiscoveryWrapper }); @@ -93,4 +102,40 @@ describe("DiscoverySettings", () => { expect(client.getThreePids).toHaveBeenCalled(); }); + + it("should not disable share button if terms accepted", async () => { + mocked(client).getThreePids.mockResolvedValue({ + threepids: [ + { + medium: ThreepidMedium.Email, + address: "test@email.com", + bound: false, + added_at: 123, + validated_at: 234, + }, + ], + }); + mocked(client).getIdentityServerUrl.mockReturnValue("https://example.com"); + mocked(client).getTerms.mockResolvedValue(sampleTerms); + mocked(client).getAccountData.mockReturnValue( + new MatrixEvent({ + content: { accepted: [sampleTerms.policies["terms"]["en"].url] }, + }), + ); + + render(, { wrapper: DiscoveryWrapper }); + + const shareButton = await screen.findByRole("button", { name: "Share" }); + expect(shareButton).not.toHaveAttribute("aria-disabled", "true"); + }); + + it("should not show invalid terms", async () => { + mocked(client).getIdentityServerUrl.mockReturnValue("https://example.com"); + mocked(client).getTerms.mockResolvedValue(invalidTerms); + + render(, { wrapper: DiscoveryWrapper }); + + expect(await screen.findByText("Let people find you")).toBeInTheDocument(); + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + }); });