diff --git a/playwright/e2e/settings/room-settings/room-security-tab.spec.ts b/playwright/e2e/settings/room-settings/room-security-tab.spec.ts index 01a861e86b..605d38abc2 100644 --- a/playwright/e2e/settings/room-settings/room-security-tab.spec.ts +++ b/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(); + }, + ); }); diff --git a/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-linux.png b/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-linux.png index 4f5f95c95e..7d03f4b6ef 100644 Binary files a/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-linux.png and b/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-linux.png differ diff --git a/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-world-readable-linux.png b/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-world-readable-linux.png new file mode 100644 index 0000000000..ff18bcf702 Binary files /dev/null and b/playwright/snapshots/settings/room-settings/room-security-tab.spec.ts/room-security-settings-world-readable-linux.png differ diff --git a/src/components/views/settings/JoinRuleSettings.tsx b/src/components/views/settings/JoinRuleSettings.tsx index 400dc7a865..f3189b3fcf 100644 --- a/src/components/views/settings/JoinRuleSettings.tsx +++ b/src/components/views/settings/JoinRuleSettings.tsx @@ -35,7 +35,6 @@ export interface JoinRuleSettingsProps { closeSettingsFn(): void; onError(error: unknown): void; beforeChange?(joinRule: JoinRule): Promise; // if returns false then aborts the change - aliasWarning?: ReactNode; disabledOptions?: Set; hiddenOptions?: Set; recommendedOption?: JoinRule; @@ -44,7 +43,6 @@ export interface JoinRuleSettingsProps { const JoinRuleSettings: React.FC = ({ room, promptUpgrade, - aliasWarning, onError, beforeChange, closeSettingsFn, @@ -209,12 +207,7 @@ const JoinRuleSettings: React.FC = ({ { value: JoinRule.Public, label: withRecommendLabel(_t("common|public"), JoinRule.Public), - description: ( - <> - {_t("room_settings|security|join_rule_public_description")} - {aliasWarning} - - ), + description: <>{_t("room_settings|security|join_rule_public_description")}, }, ]; diff --git a/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx b/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx index 89ae96f8a4..37faaea5d8 100644 --- a/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx +++ b/src/components/views/settings/tabs/room/SecurityRoomSettingsTab.tsx @@ -251,19 +251,28 @@ export default class SecurityRoomSettingsTab extends React.Component - - {_t("room_settings|security|public_without_alias_warning")} - - ); - } - const description = _t("room_settings|security|join_rule_description", { - roomName: room.name, - }); + const isPublic = room.getJoinRule() === JoinRule.Public; + const description = ( + <> +

+ {_t("room_settings|security|join_rule_description", { + roomName: room.name, + })} +

+ {isPublic && this.state.history === HistoryVisibility.WorldReadable && ( +
+ + {_t("room_settings|security|join_rule_world_readable_description")} +
+ )} + {isPublic && !this.state.hasAliases && ( +
+ + {_t("room_settings|security|public_without_alias_warning")} +
+ )} + + ); let advanced: JSX.Element | undefined; if (room.getJoinRule() === JoinRule.Public) { @@ -290,7 +299,6 @@ export default class SecurityRoomSettingsTab extends React.Component {advanced} @@ -342,6 +350,57 @@ export default class SecurityRoomSettingsTab extends React.Component + {_t( + "room_settings|security|cannot_change_to_private_due_to_missing_history_visiblity_permissions|description", + )} +

+ ), + }); + 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; }; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a82c06a124..00ac3eb116 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1128,6 +1128,7 @@ "tls": "Can't connect to homeserver - please check your connectivity, ensure your homeserver's SSL certificate 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. Learn more about encryption.", "enable_encryption_confirm_title": "Enable encryption?", "enable_encryption_public_room_confirm_description_1": "It's not recommended to add encryption to public rooms. 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.", diff --git a/test/unit-tests/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx b/test/unit-tests/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx index bb5300fc03..6078fa4fe9 100644 --- a/test/unit-tests/components/views/settings/tabs/room/SecurityRoomSettingsTab-test.tsx +++ b/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("", () => { ); }); + 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"); diff --git a/test/unit-tests/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap b/test/unit-tests/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap index 6556debd05..a7ff055c29 100644 --- a/test/unit-tests/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap +++ b/test/unit-tests/components/views/settings/tabs/room/__snapshots__/SecurityRoomSettingsTab-test.tsx.snap @@ -15,7 +15,7 @@ exports[` history visibility uses shared as default h
- 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.