Skip to content

Commit 205b877

Browse files
BillCarsonFrflorianduros
authored andcommitted
Fix local room encryption status always not enabled (#30461)
* Fix local room encryption status always not enabled * refactor: put back the e2e test after merge * fix: look at e2eStatus in composer of local room * doc: add docs to `LocalRoom.isEncryptionEnabled` * test(e2e): check composer doesn't display unencrypted state * test: update existing tests * test(e2e): update existing tests * refactor: move room encryption check in a dedicated function * refactor: make `isEncryptionEnabled` cleaner * test: add tests for `LocalRoom.isEncrypted` * doc: fix `useIsEncrypted` comment --------- Co-authored-by: Florian Duros <[email protected]>
1 parent 77a4c1e commit 205b877

File tree

9 files changed

+105
-34
lines changed

9 files changed

+105
-34
lines changed

playwright/e2e/crypto/crypto.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ test.describe("Cryptography", function () {
154154
await app.client.bootstrapCrossSigning(aliceCredentials);
155155
await startDMWithBob(page, bob);
156156
// send first message
157-
await page.getByRole("textbox", { name: "Send an unencrypted message…" }).fill("Hey!");
158-
await page.getByRole("textbox", { name: "Send an unencrypted message…" }).press("Enter");
157+
await page.getByRole("textbox", { name: "Send a message…" }).fill("Hey!");
158+
await page.getByRole("textbox", { name: "Send a message…" }).press("Enter");
159159
await checkDMRoom(page);
160160
const bobRoomId = await bobJoin(page, bob);
161161
// We no longer show the grey badge in the composer, check that it is not there.

playwright/e2e/room/create-room.spec.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,21 @@ test.describe("Create Room", () => {
4444
},
4545
);
4646

47+
test("should allow us to start a chat and show encryption state", async ({ page, user, app }) => {
48+
await page.getByRole("button", { name: "Add", exact: true }).click();
49+
await page.getByText("Start new chat").click();
50+
51+
await page.getByTestId("invite-dialog-input").fill(user.userId);
52+
53+
await page.getByRole("button", { name: "Go" }).click();
54+
55+
await expect(page.getByText("Encryption enabled")).toBeVisible();
56+
await expect(page.getByText("Send your first message to")).toBeVisible();
57+
58+
const composer = page.getByRole("region", { name: "Message composer" });
59+
await expect(composer.getByRole("textbox", { name: "Send a message…" })).toBeVisible();
60+
});
61+
4762
test("should create a video room", { tag: "@screenshot" }, async ({ page, user, app }) => {
4863
await app.settings.setValue("feature_video_rooms", null, SettingLevel.DEVICE, true);
4964

playwright/e2e/spotlight/spotlight.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ async function startDM(app: ElementAppPage, page: Page, name: string): Promise<v
3030
await result.first().click();
3131

3232
// send first message to start DM
33-
const locator = page.getByRole("textbox", { name: "Send an unencrypted message…" });
33+
const locator = page.getByRole("textbox", { name: "Send a message…" });
3434
await expect(locator).toBeFocused();
3535
await locator.fill("Hey!");
3636
await locator.press("Enter");
@@ -260,7 +260,7 @@ test.describe("Spotlight", () => {
260260

261261
// Send first message to actually start DM
262262
await expect(roomHeaderName(page)).toHaveText(bot2.credentials.displayName);
263-
const locator = page.getByRole("textbox", { name: "Send an unencrypted message…" });
263+
const locator = page.getByRole("textbox", { name: "Send a message…" });
264264
await locator.fill("Hey!");
265265
await locator.press("Enter");
266266

src/components/structures/RoomView.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ import { PinnedMessageBanner } from "../views/rooms/PinnedMessageBanner";
133133
import { ScopedRoomContextProvider, useScopedRoomContext } from "../../contexts/ScopedRoomContext";
134134
import { DeclineAndBlockInviteDialog } from "../views/dialogs/DeclineAndBlockInviteDialog";
135135
import { type FocusMessageSearchPayload } from "../../dispatcher/payloads/FocusMessageSearchPayload.ts";
136+
import { isRoomEncrypted } from "../../hooks/useIsEncrypted";
136137

137138
const DEBUG = false;
138139
const PREVENT_MULTIPLE_JITSI_WITHIN = 30_000;
@@ -257,6 +258,7 @@ interface LocalRoomViewProps {
257258
roomView: RefObject<HTMLElement | null>;
258259
onFileDrop: (dataTransfer: DataTransfer) => Promise<void>;
259260
mainSplitContentType: MainSplitContentType;
261+
e2eStatus?: E2EStatus;
260262
}
261263

262264
/**
@@ -304,6 +306,7 @@ function LocalRoomView(props: LocalRoomViewProps): ReactElement {
304306
} else {
305307
composer = (
306308
<MessageComposer
309+
e2eStatus={props.e2eStatus}
307310
room={props.localRoom}
308311
resizeNotifier={props.resizeNotifier}
309312
permalinkCreator={props.permalinkCreator}
@@ -1397,10 +1400,13 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
13971400
}
13981401

13991402
private async getIsRoomEncrypted(roomId = this.state.roomId): Promise<boolean> {
1403+
if (!roomId) return false;
1404+
1405+
const room = this.context.client?.getRoom(roomId);
14001406
const crypto = this.context.client?.getCrypto();
1401-
if (!crypto || !roomId) return false;
1407+
if (!room || !crypto) return false;
14021408

1403-
return await crypto.isEncryptionEnabledInRoom(roomId);
1409+
return isRoomEncrypted(room, crypto);
14041410
}
14051411

14061412
private async calculateRecommendedVersion(room: Room): Promise<void> {
@@ -2061,6 +2067,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
20612067
return (
20622068
<ScopedRoomContextProvider {...this.state}>
20632069
<LocalRoomView
2070+
e2eStatus={this.state.e2eStatus}
20642071
localRoom={localRoom}
20652072
resizeNotifier={this.props.resizeNotifier}
20662073
permalinkCreator={this.permalinkCreator}

src/hooks/useIsEncrypted.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,29 @@ Please see LICENSE files in the repository root for full details.
77
*/
88

99
import { type MatrixClient, type MatrixEvent, type Room, EventType } from "matrix-js-sdk/src/matrix";
10+
import { type CryptoApi } from "matrix-js-sdk/src/crypto-api";
1011

1112
import { useRoomState } from "./useRoomState.ts";
1213
import { useAsyncMemo } from "./useAsyncMemo.ts";
14+
import { LocalRoom } from "../models/LocalRoom.ts";
15+
16+
/**
17+
* Check if a room is encrypted.
18+
* If the room is a LocalRoom, check the state directly.
19+
* Otherwise, use the crypto API to check if encryption is enabled in the room.
20+
*
21+
* @param room - The room to check.
22+
* @param cryptoApi - The crypto API from the Matrix client.
23+
*/
24+
export async function isRoomEncrypted(room: Room, cryptoApi: CryptoApi): Promise<boolean> {
25+
if (room instanceof LocalRoom) {
26+
// For local room check the state.
27+
// The crypto check fails because the room ID is not valid (it is a local id)
28+
return (room as LocalRoom).isEncryptionEnabled();
29+
}
30+
31+
return await cryptoApi.isEncryptionEnabledInRoom(room.roomId);
32+
}
1333

1434
// Hook to simplify watching whether a Matrix room is encrypted, returns null if room is undefined or the state is loading
1535
export function useIsEncrypted(cli: MatrixClient, room?: Room): boolean | null {
@@ -22,7 +42,7 @@ export function useIsEncrypted(cli: MatrixClient, room?: Room): boolean | null {
2242
const crypto = cli.getCrypto();
2343
if (!room || !crypto) return null;
2444

25-
return crypto.isEncryptionEnabledInRoom(room.roomId);
45+
return isRoomEncrypted(room, crypto);
2646
},
2747
[room, encryptionStateEvent],
2848
null,

src/models/LocalRoom.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,14 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import { type MatrixClient, Room, PendingEventOrdering } from "matrix-js-sdk/src/matrix";
9+
import {
10+
type MatrixClient,
11+
Room,
12+
PendingEventOrdering,
13+
MatrixEvent,
14+
Direction,
15+
EventType,
16+
} from "matrix-js-sdk/src/matrix";
1017

1118
import { type Member } from "../utils/direct-messages";
1219

@@ -50,4 +57,20 @@ export class LocalRoom extends Room {
5057
public get isError(): boolean {
5158
return this.state === LocalRoomState.ERROR;
5259
}
60+
61+
/**
62+
* Check if encryption is enabled in this room.
63+
* True if the room has any encryption state event
64+
*/
65+
public isEncryptionEnabled(): boolean {
66+
const roomState = this.getLiveTimeline().getState(Direction.Forward);
67+
if (!roomState) return false;
68+
69+
const stateEvents = roomState.getStateEvents(EventType.RoomEncryption);
70+
if (stateEvents.length === 0) return false;
71+
72+
// if there is an encryption state event, it is encrypted.
73+
// Regardless of the content/algorithm, we assume it is encrypted.
74+
return stateEvents[0] instanceof MatrixEvent;
75+
}
5376
}

test/unit-tests/components/structures/__snapshots__/RoomView-test.tsx.snap

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,7 +1183,7 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
11831183
</div>
11841184
<div
11851185
aria-label="Message composer"
1186-
class="mx_MessageComposer mx_MessageComposer_e2eStatus"
1186+
class="mx_MessageComposer"
11871187
role="region"
11881188
>
11891189
<div
@@ -1192,25 +1192,6 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
11921192
<div
11931193
class="mx_MessageComposer_row"
11941194
>
1195-
<div
1196-
class="mx_MessageComposer_e2eIconWrapper"
1197-
>
1198-
<svg
1199-
aria-label="Messages in this room are not end-to-end encrypted"
1200-
aria-labelledby="«rfm»"
1201-
class="mx_E2EIcon mx_MessageComposer_e2eIcon"
1202-
color="var(--cpd-color-icon-info-primary)"
1203-
fill="currentColor"
1204-
height="12"
1205-
viewBox="0 0 24 24"
1206-
width="12"
1207-
xmlns="http://www.w3.org/2000/svg"
1208-
>
1209-
<path
1210-
d="M6 22q-.825 0-1.412-.587A1.93 1.93 0 0 1 4 20V10q0-.825.588-1.412a2 2 0 0 1 .702-.463L1.333 4.167a1 1 0 0 1 1.414-1.414L7 7.006v-.012l13 13v.012l1.247 1.247a1 1 0 1 1-1.414 1.414l-.896-.896A1.94 1.94 0 0 1 18 22zm14-4.834V10q0-.825-.587-1.412A1.93 1.93 0 0 0 18 8h-1V6q0-2.075-1.463-3.537Q14.075 1 12 1T8.463 2.463a4.9 4.9 0 0 0-1.22 1.946L9 6.166V6q0-1.25.875-2.125A2.9 2.9 0 0 1 12 3q1.25 0 2.125.875T15 6v2h-4.166z"
1211-
/>
1212-
</svg>
1213-
</div>
12141195
<div
12151196
class="mx_SendMessageComposer"
12161197
>
@@ -1269,14 +1250,14 @@ exports[`RoomView for a local room in state NEW that is encrypted should match t
12691250
aria-autocomplete="list"
12701251
aria-disabled="false"
12711252
aria-haspopup="listbox"
1272-
aria-label="Send an unencrypted message…"
1253+
aria-label="Send a message…"
12731254
aria-multiline="true"
12741255
class="mx_BasicMessageComposer_input mx_BasicMessageComposer_input_shouldShowPillAvatar mx_BasicMessageComposer_inputEmpty"
12751256
contenteditable="true"
12761257
data-testid="basicmessagecomposer"
12771258
dir="auto"
12781259
role="textbox"
1279-
style="--placeholder: 'Send\\ an\\ unencrypted\\ message…';"
1260+
style="--placeholder: 'Send\\ a\\ message…';"
12801261
tabindex="0"
12811262
translate="no"
12821263
>

test/unit-tests/components/views/messages/EncryptionEvent-test.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,19 @@ describe("EncryptionEvent", () => {
112112
});
113113

114114
describe("for an encrypted local room", () => {
115+
let localRoom: LocalRoom;
116+
115117
beforeEach(() => {
116118
event.event.content!.algorithm = algorithm;
117-
jest.spyOn(client.getCrypto()!, "isEncryptionEnabledInRoom").mockResolvedValue(true);
118-
const localRoom = new LocalRoom(roomId, client, client.getUserId()!);
119+
// jest.spyOn(client.getCrypto()!, "isEncryptionEnabledInRoom").mockResolvedValue(true);
120+
localRoom = new LocalRoom(roomId, client, client.getUserId()!);
121+
jest.spyOn(localRoom, "isEncryptionEnabled").mockReturnValue(true);
119122
mocked(client.getRoom).mockReturnValue(localRoom);
120123
renderEncryptionEvent(client, event);
121124
});
122125

123126
it("should show the expected texts", async () => {
124-
expect(client.getCrypto()!.isEncryptionEnabledInRoom).toHaveBeenCalledWith(roomId);
127+
expect(localRoom.isEncryptionEnabled).toHaveBeenCalled();
125128
await checkTexts("Encryption enabled", "Messages in this chat will be end-to-end encrypted.");
126129
});
127130
});

test/unit-tests/models/LocalRoom-test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import { type MatrixClient } from "matrix-js-sdk/src/matrix";
9+
import { Direction, EventType, type MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix";
1010

1111
import { LocalRoom, LocalRoomState, LOCAL_ROOM_ID_PREFIX } from "../../../src/models/LocalRoom";
1212
import { createTestClient } from "../../test-utils";
@@ -79,4 +79,26 @@ describe("LocalRoom", () => {
7979
});
8080
});
8181
});
82+
83+
it("should return false for isEncryptionEnabled with no state", () => {
84+
expect(room.isEncryptionEnabled()).toBe(false);
85+
});
86+
87+
it("should return true for isEncryptionEnabled with an encryption state event", () => {
88+
const encryptionEvent = new MatrixEvent({
89+
type: EventType.RoomEncryption,
90+
state_key: "",
91+
content: {
92+
algorithm: "m.megolm.v1.aes-sha2",
93+
},
94+
sender: "@test:localhost",
95+
room_id: room.roomId,
96+
event_id: "$test:localhost",
97+
});
98+
99+
const roomState = room.getLiveTimeline().getState(Direction.Forward);
100+
roomState?.setStateEvents([encryptionEvent]);
101+
102+
expect(room.isEncryptionEnabled()).toBe(true);
103+
});
82104
});

0 commit comments

Comments
 (0)