Automatically adjust history visibility when making a room private (#30713)

* Refactor StyledRadioButton to provide proper labels.

* Automatically change history settings to members only if room is made private

* Add tests

* lint

* lint further

* Fix clickable buttons

* Revert functional component-ing

* text tweaks

* update snapshots

* Add unit test for history vis changes

* lint

* Update snapshots

* Fix flakes

* lint

playwright/e2e/settings/room-settings/room-security-tab.spec.ts

@@ -19,7 +19,16 @@ test.describe("Roles & Permissions room settings tab", () => {
     let settings: Locator;
 
     test.beforeEach(async ({ user, app }) => {
-        await app.client.createRoom({ name: roomName });
+        await app.client.createRoom({
+            name: roomName,
+            power_level_content_override: {
+                events: {
+                    // Set the join rules as lower than the history vis to test an edge case.
+                    ["m.room.join_rules"]: 80,
+                    ["m.room.history_visibility"]: 100,
+                },
+            },
+        });
         await app.viewRoomByName(roomName);
         settings = await app.settings.openRoomSettings("Security & Privacy");
     });
@@ -45,4 +54,68 @@ test.describe("Roles & Permissions room settings tab", () => {
             await expect(settings).toMatchScreenshot("room-security-settings.png");
         },
     );
+
+    test(
+        "should automatically adjust history visibility when a room is changed from public to private",
+        { tag: "@screenshot" },
+        async ({ page, app, user, axe }) => {
+            await page.setViewportSize({ width: 1024, height: 1400 });
+
+            const settingsGroupAccess = page.getByRole("group", { name: "Access" });
+            const settingsGroupHistory = page.getByRole("group", { name: "Who can read history?" });
+
+            await settingsGroupAccess.getByText("Public").click();
+            await settingsGroupHistory.getByText("Anyone").click();
+
+            // Test that we have the warning appear.
+            axe.disableRules("color-contrast"); // XXX: Inheriting colour contrast issues from room view.
+            await expect(axe).toHaveNoViolations();
+            await expect(settings).toMatchScreenshot("room-security-settings-world-readable.png");
+
+            await settingsGroupAccess.getByText("Private (invite only)").click();
+            // Element should have automatically set the room to "sharing" history visibility
+            await expect(
+                settingsGroupHistory.getByText("Members only (since the point in time of selecting this option)"),
+            ).toBeChecked();
+        },
+    );
+
+    test(
+        "should disallow changing from public to private if the user cannot alter history",
+        { tag: "@screenshot" },
+        async ({ page, app, user, bot }) => {
+            await page.setViewportSize({ width: 1024, height: 1400 });
+
+            const settingsGroupAccess = page.getByRole("group", { name: "Access" });
+            const settingsGroupHistory = page.getByRole("group", { name: "Who can read history?" });
+
+            await settingsGroupAccess.getByText("Public").click();
+            await settingsGroupHistory.getByText("Anyone").click();
+
+            // De-op ourselves
+            await app.settings.switchTab("Roles & Permissions");
+
+            // Wait for the permissions list to be visible
+            await expect(settings.getByRole("heading", { name: "Permissions" })).toBeVisible();
+
+            const ourComboBox = settings.getByRole("combobox", { name: user.userId });
+            await ourComboBox.selectOption("Custom level");
+            const ourPl = settings.getByRole("spinbutton", { name: user.userId });
+            await ourPl.fill("80");
+            await ourPl.blur(); // Shows a warning on
+
+            // Accept the de-op
+            await page.getByRole("button", { name: "Continue" }).click();
+            await settings.getByRole("button", { name: "Apply", disabled: false }).click();
+
+            await app.settings.switchTab("Security & Privacy");
+
+            await settingsGroupAccess.getByText("Private (invite only)").click();
+            // Element should have automatically set the room to "sharing" history visibility
+            const errorDialog = page.getByRole("heading", { name: "Cannot make room private" });
+            await expect(errorDialog).toBeVisible();
+            await errorDialog.getByLabel("OK");
+            await expect(settingsGroupHistory.getByText("Anyone")).toBeChecked();
+        },
+    );
 });

src/components/views/settings/JoinRuleSettings.tsx

@@ -35,7 +35,6 @@ export interface JoinRuleSettingsProps {
     closeSettingsFn(): void;
     onError(error: unknown): void;
     beforeChange?(joinRule: JoinRule): Promise<boolean>; // if returns false then aborts the change
-    aliasWarning?: ReactNode;
     disabledOptions?: Set<JoinRule>;
     hiddenOptions?: Set<JoinRule>;
     recommendedOption?: JoinRule;
@@ -44,7 +43,6 @@ export interface JoinRuleSettingsProps {
 const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
     room,
     promptUpgrade,
-    aliasWarning,
     onError,
     beforeChange,
     closeSettingsFn,
@@ -209,12 +207,7 @@ const JoinRuleSettings: React.FC<JoinRuleSettingsProps> = ({
         {
             value: JoinRule.Public,
             label: withRecommendLabel(_t("common|public"), JoinRule.Public),
-            description: (
+            description: <>{_t("room_settings|security|join_rule_public_description")}</>,
-                <>
-                    {_t("room_settings|security|join_rule_public_description")}
-                    {aliasWarning}
-                </>
-            ),
         },
     ];
 

src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx

@@ -251,19 +251,28 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
 
     private renderJoinRule(): JSX.Element {
         const room = this.props.room;
-
+        const isPublic = room.getJoinRule() === JoinRule.Public;
-        let aliasWarning: JSX.Element | undefined;
+        const description = (
-        if (room.getJoinRule() === JoinRule.Public && !this.state.hasAliases) {
+            <>
-            aliasWarning = (
+                <p>
-                <div className="mx_SecurityRoomSettingsTab_warning">
+                    {_t("room_settings|security|join_rule_description", {
-                    <WarningIcon width={15} height={15} />
+                        roomName: room.name,
-                    <span>{_t("room_settings|security|public_without_alias_warning")}</span>
+                    })}
-                </div>
+                </p>
-            );
+                {isPublic && this.state.history === HistoryVisibility.WorldReadable && (
-        }
+                    <div className="mx_SecurityRoomSettingsTab_warning">
-        const description = _t("room_settings|security|join_rule_description", {
+                        <WarningIcon width={15} height={15} />
-            roomName: room.name,
+                        <span>{_t("room_settings|security|join_rule_world_readable_description")}</span>
-        });
+                    </div>
+                )}
+                {isPublic && !this.state.hasAliases && (
+                    <div className="mx_SecurityRoomSettingsTab_warning">
+                        <WarningIcon width={15} height={15} />
+                        <span>{_t("room_settings|security|public_without_alias_warning")}</span>
+                    </div>
+                )}
+            </>
+        );
 
         let advanced: JSX.Element | undefined;
         if (room.getJoinRule() === JoinRule.Public) {
@@ -290,7 +299,6 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
                     onError={this.onJoinRuleChangeError}
                     closeSettingsFn={this.props.closeSettingsFn}
                     promptUpgrade={true}
-                    aliasWarning={aliasWarning}
                 />
                 {advanced}
             </SettingsFieldset>
@@ -342,6 +350,57 @@ export default class SecurityRoomSettingsTab extends React.Component<IProps, ISt
             if (!confirm) return false;
         }
 
+        // If the room is going from public to private AND the room is join readable, we want to encourage the user
+        // to change the history visibility.
+        const currentlyPublic = this.props.room.getJoinRule() === JoinRule.Public;
+        if (this.state.history === HistoryVisibility.WorldReadable && currentlyPublic && joinRule !== JoinRule.Public) {
+            const client = this.context;
+            const canChangeHistory = this.props.room.currentState?.mayClientSendStateEvent(
+                EventType.RoomHistoryVisibility,
+                client,
+            );
+
+            // If we can't change the history visibility, then don't allow the join rule transition. This is a unlikely occurance
+            // and if this is the case, a room administator should step in.
+            if (!canChangeHistory) {
+                const dialog = Modal.createDialog(ErrorDialog, {
+                    title: _t(
+                        "room_settings|security|cannot_change_to_private_due_to_missing_history_visiblity_permissions|title",
+                    ),
+                    description: (
+                        <p>
+                            {_t(
+                                "room_settings|security|cannot_change_to_private_due_to_missing_history_visiblity_permissions|description",
+                            )}
+                        </p>
+                    ),
+                });
+                await dialog.finished;
+                return false;
+            }
+
+            // Adjust the history visibility first.
+            try {
+                await this.context.sendStateEvent(
+                    this.props.room.roomId,
+                    EventType.RoomHistoryVisibility,
+                    {
+                        history_visibility: HistoryVisibility.Shared,
+                    },
+                    "",
+                );
+                this.setState({ history: HistoryVisibility.Shared });
+            } catch (ex) {
+                logger.error("Failed to change history visibility", ex);
+                Modal.createDialog(ErrorDialog, {
+                    title: _t("common|error"),
+                    description: _t("error|update_history_visibility"),
+                });
+                // If we fail to update the history visibility
+                return false;
+            }
+        }
+
         return true;
     };
 

src/i18n/strings/en_EN.json

@@ -1128,6 +1128,7 @@
         "tls": "Can't connect to homeserver - please check your connectivity, ensure your <a>homeserver's SSL certificate</a> is trusted, and that a browser extension is not blocking requests.",
         "unknown": "Unknown error",
         "unknown_error_code": "unknown error code",
+        "update_history_visibility": "Failed to change history visibility",
         "update_power_level": "Failed to change power level"
     },
     "error_app_open_in_another_tab": "Switch to the other tab to connect to %(brand)s. This tab can now be closed.",
@@ -2385,6 +2386,10 @@
             "users_default": "Default role"
         },
         "security": {
+            "cannot_change_to_private_due_to_missing_history_visiblity_permissions": {
+                "description": "You do not have permissions to alter the history visibility of the room. This is dangerous as it could allow unjoined users to read messages.",
+                "title": "Cannot make room private"
+            },
             "enable_encryption_confirm_description": "Once enabled, encryption for a room cannot be disabled. Messages sent in an encrypted room cannot be seen by the server, only by the participants of the room. Enabling encryption may prevent many bots and bridges from working correctly. <a>Learn more about encryption.</a>",
             "enable_encryption_confirm_title": "Enable encryption?",
             "enable_encryption_public_room_confirm_description_1": "<b>It's not recommended to add encryption to public rooms.</b> Anyone can find and join public rooms, so anyone can read messages in them. You'll get none of the benefits of encryption, and you won't be able to turn it off later. Encrypting messages in a public room will make receiving and sending messages slower.",
@@ -2402,7 +2407,7 @@
             "history_visibility_joined": "Members only (since they joined)",
             "history_visibility_legend": "Who can read history?",
             "history_visibility_shared": "Members only (since the point in time of selecting this option)",
-            "history_visibility_warning": "Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged.",
+            "history_visibility_warning": "The visibility of existing history will not be changed.",
             "history_visibility_world_readable": "Anyone",
             "join_rule_description": "Decide who can join %(roomName)s.",
             "join_rule_invite": "Private (invite only)",
@@ -2445,6 +2450,7 @@
                 "other": "Updating spaces... (%(progress)s out of %(count)s)"
             },
             "join_rule_upgrade_upgrading_room": "Upgrading room",
+            "join_rule_world_readable_description": "Changing who can join the room will change the visibility of future messages too.",
             "public_without_alias_warning": "To link to this room, please add an address.",
             "publish_room": "Make this room visible in the public room directory.",
             "publish_space": "Make this space visible in the public room directory.",

test/unit-tests/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx

@@ -11,6 +11,7 @@ import { fireEvent, render, screen, waitFor, within } from "jest-matrix-react";
 import { EventType, GuestAccess, HistoryVisibility, JoinRule, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
 import { logger } from "matrix-js-sdk/src/logger";
 import { mocked } from "jest-mock";
+import { type RoomPowerLevelsEventContent } from "matrix-js-sdk/src/types";
 
 import SecurityRoomSettingsTab from "../../../../../../../src/components/views/settings/tabs/room/SecurityRoomSettingsTab";
 import MatrixClientContext from "../../../../../../../src/contexts/MatrixClientContext";
@@ -123,6 +124,88 @@ describe("<SecurityRoomSettingsTab />", () => {
             );
         });
 
+        it("handles changing room to private with world_readable history visiblity", async () => {
+            const room = new Room(roomId, client, userId);
+            setRoomStateEvents(room, JoinRule.Public, undefined, HistoryVisibility.WorldReadable);
+
+            getComponent(room);
+
+            fireEvent.click(screen.getByLabelText("Private (invite only)"));
+
+            await flushPromises();
+
+            expect(client.sendStateEvent).toHaveBeenCalledWith(
+                room.roomId,
+                EventType.RoomHistoryVisibility,
+                {
+                    history_visibility: HistoryVisibility.Shared,
+                },
+                "",
+            );
+            expect(client.sendStateEvent).toHaveBeenCalledWith(
+                room.roomId,
+                EventType.RoomJoinRules,
+                {
+                    join_rule: JoinRule.Invite,
+                },
+                "",
+            );
+        });
+
+        it("doesn't change room to private when user lacks permissions for history visibility", async () => {
+            const room = new Room(roomId, client, userId);
+            setRoomStateEvents(room, JoinRule.Public, undefined, HistoryVisibility.WorldReadable);
+            room.currentState.setStateEvents([
+                new MatrixEvent({
+                    type: EventType.RoomPowerLevels,
+                    content: {
+                        users: { [userId]: 50 },
+                        state_default: 50,
+                        events: {
+                            [EventType.RoomJoinRules]: 50,
+                            [EventType.RoomHistoryVisibility]: 100,
+                        },
+                    } as RoomPowerLevelsEventContent,
+                    sender: userId,
+                    state_key: "",
+                    room_id: room.roomId,
+                }),
+            ]);
+
+            getComponent(room);
+            fireEvent.click(screen.getByLabelText("Private (invite only)"));
+            await flushPromises();
+            // Ensure we don't make any changes
+            expect(client.sendStateEvent).not.toHaveBeenCalled();
+        });
+
+        it("doesn't change room to private when history visibility change fails", async () => {
+            client.sendStateEvent.mockRejectedValue("Failed");
+            const room = new Room(roomId, client, userId);
+            setRoomStateEvents(room, JoinRule.Public, undefined, HistoryVisibility.WorldReadable);
+
+            getComponent(room);
+            fireEvent.click(screen.getByLabelText("Private (invite only)"));
+            await flushPromises();
+            expect(client.sendStateEvent).toHaveBeenCalledWith(
+                room.roomId,
+                EventType.RoomHistoryVisibility,
+                {
+                    history_visibility: HistoryVisibility.Shared,
+                },
+                "",
+            );
+            // Ensure we don't make any changes
+            expect(client.sendStateEvent).not.toHaveBeenCalledWith(
+                room.roomId,
+                EventType.RoomJoinRules,
+                {
+                    join_rule: JoinRule.Invite,
+                },
+                "",
+            );
+        });
+
         it("handles error when updating join rule fails", async () => {
             const room = new Room(roomId, client, userId);
             client.sendStateEvent.mockRejectedValue("oups");

test/unit-tests/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap

@@ -15,7 +15,7 @@ exports[`<SecurityRoomSettingsTab /> history visibility uses shared as default h
     <div
       class="mx_SettingsSubsection_text"
     >
-      Changes to who can read history will only apply to future messages in this room. The visibility of existing history will be unchanged.
+      The visibility of existing history will not be changed.
     </div>
   </div>
   <div