From 2631b908b628b6247c34f41246b269bf734f9d92 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 4 Nov 2024 07:46:38 -0500 Subject: [PATCH] Update the display of decryption failures due to failed trust requirement (#28300) * update the display of decryption failures due to failed trust requirement * add test for not showing shield --- .../e2e/crypto/invisible-crypto.spec.ts | 2 +- .../messages/_DecryptionFailureBody.pcss | 19 +++-------- .../views/messages/DecryptionFailureBody.tsx | 14 +++++--- src/components/views/rooms/EventTile.tsx | 10 +++++- src/i18n/strings/en_EN.json | 4 +-- .../messages/DecryptionFailureBody-test.tsx | 2 +- .../DecryptionFailureBody-test.tsx.snap | 11 ++---- .../components/views/rooms/EventTile-test.tsx | 34 ++++++++++++++++++- 8 files changed, 63 insertions(+), 33 deletions(-) diff --git a/playwright/e2e/crypto/invisible-crypto.spec.ts b/playwright/e2e/crypto/invisible-crypto.spec.ts index c53bacd32c..f207d2c6bb 100644 --- a/playwright/e2e/crypto/invisible-crypto.spec.ts +++ b/playwright/e2e/crypto/invisible-crypto.spec.ts @@ -51,6 +51,6 @@ test.describe("Invisible cryptography", () => { /* should show an error for a message from a previously verified device */ await bobSecondDevice.sendMessage(testRoomId, "test encrypted from user that was previously verified"); const lastTile = page.locator(".mx_EventTile_last"); - await expect(lastTile).toContainText("Verified identity has changed"); + await expect(lastTile).toContainText("Sender's verified identity has changed"); }); }); diff --git a/res/css/views/messages/_DecryptionFailureBody.pcss b/res/css/views/messages/_DecryptionFailureBody.pcss index 64a09be7ef..516e7bcc89 100644 --- a/res/css/views/messages/_DecryptionFailureBody.pcss +++ b/res/css/views/messages/_DecryptionFailureBody.pcss @@ -11,22 +11,11 @@ Please see LICENSE files in the repository root for full details. font-style: italic; } -/* Formatting for the "Verified identity has changed" error */ -.mx_DecryptionFailureVerifiedIdentityChanged > span { - /* Show it in red */ - color: var(--cpd-color-text-critical-primary); - background-color: var(--cpd-color-bg-critical-subtle); - - /* With a red border */ - border: 1px solid var(--cpd-color-border-critical-subtle); - border-radius: $font-16px; - - /* Some space inside the border */ - padding: var(--cpd-space-1x) var(--cpd-space-3x) var(--cpd-space-1x) var(--cpd-space-2x); - - /* some space between the (!) icon and text */ +/* Formatting for errors due to sender trust requirement failures */ +.mx_DecryptionFailureSenderTrustRequirement > span { + /* some space between the (/) icon and text */ display: inline-flex; - gap: var(--cpd-space-2x); + gap: var(--cpd-space-1x); /* Center vertically */ align-items: center; diff --git a/src/components/views/messages/DecryptionFailureBody.tsx b/src/components/views/messages/DecryptionFailureBody.tsx index 81894fa51f..108ec45b03 100644 --- a/src/components/views/messages/DecryptionFailureBody.tsx +++ b/src/components/views/messages/DecryptionFailureBody.tsx @@ -10,7 +10,7 @@ import classNames from "classnames"; import React, { forwardRef, ForwardRefExoticComponent, useContext } from "react"; import { MatrixEvent } from "matrix-js-sdk/src/matrix"; import { DecryptionFailureCode } from "matrix-js-sdk/src/crypto-api"; -import { WarningIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; +import { BlockIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; import { _t } from "../../../languageHandler"; import { IBodyProps } from "./IBodyProps"; @@ -41,7 +41,7 @@ function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): case DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED: return ( - + {_t("timeline|decryption_failure|sender_identity_previously_verified")} ); @@ -49,7 +49,12 @@ function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): case DecryptionFailureCode.UNSIGNED_SENDER_DEVICE: // TODO: event should be hidden instead of showing this error. // To be revisited as part of https://github.com/element-hq/element-meta/issues/2449 - return _t("timeline|decryption_failure|sender_unsigned_device"); + return ( + + + {_t("timeline|decryption_failure|sender_unsigned_device")} + + ); } return _t("timeline|decryption_failure|unable_to_decrypt"); } @@ -58,7 +63,8 @@ function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined): function errorClassName(mxEvent: MatrixEvent): string | null { switch (mxEvent.decryptionFailureReason) { case DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED: - return "mx_DecryptionFailureVerifiedIdentityChanged"; + case DecryptionFailureCode.UNSIGNED_SENDER_DEVICE: + return "mx_DecryptionFailureSenderTrustRequirement"; default: return null; diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index 0b600275ef..41b4147473 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -28,6 +28,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import { CallErrorCode } from "matrix-js-sdk/src/webrtc/call"; import { CryptoEvent, + DecryptionFailureCode, EventShieldColour, EventShieldReason, UserVerificationStatus, @@ -719,7 +720,14 @@ export class UnwrappedEventTile extends React.Component // event could not be decrypted if (ev.isDecryptionFailure()) { - return ; + switch (ev.decryptionFailureReason) { + // These two errors get icons from DecryptionFailureBody, so we hide the padlock icon + case DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED: + case DecryptionFailureCode.UNSIGNED_SENDER_DEVICE: + return null; + default: + return ; + } } if (this.state.shieldColour !== EventShieldColour.NONE) { diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 6c1d8a8e0b..6e3764d582 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3266,8 +3266,8 @@ "historical_event_no_key_backup": "Historical messages are not available on this device", "historical_event_unverified_device": "You need to verify this device for access to historical messages", "historical_event_user_not_joined": "You don't have access to this message", - "sender_identity_previously_verified": "Verified identity has changed", - "sender_unsigned_device": "Encrypted by a device not verified by its owner.", + "sender_identity_previously_verified": "Sender's verified identity has changed", + "sender_unsigned_device": "Sent from an insecure device.", "unable_to_decrypt": "Unable to decrypt message" }, "disambiguated_profile": "%(displayName)s (%(matrixId)s)", diff --git a/test/unit-tests/components/views/messages/DecryptionFailureBody-test.tsx b/test/unit-tests/components/views/messages/DecryptionFailureBody-test.tsx index c92263f722..94495c4349 100644 --- a/test/unit-tests/components/views/messages/DecryptionFailureBody-test.tsx +++ b/test/unit-tests/components/views/messages/DecryptionFailureBody-test.tsx @@ -129,6 +129,6 @@ describe("DecryptionFailureBody", () => { const { container } = customRender(event); // Then - expect(container).toHaveTextContent("Encrypted by a device not verified by its owner"); + expect(container).toHaveTextContent("Sent from an insecure device"); }); }); diff --git a/test/unit-tests/components/views/messages/__snapshots__/DecryptionFailureBody-test.tsx.snap b/test/unit-tests/components/views/messages/__snapshots__/DecryptionFailureBody-test.tsx.snap index 67630f2031..a3b9fb205f 100644 --- a/test/unit-tests/components/views/messages/__snapshots__/DecryptionFailureBody-test.tsx.snap +++ b/test/unit-tests/components/views/messages/__snapshots__/DecryptionFailureBody-test.tsx.snap @@ -23,7 +23,7 @@ exports[`DecryptionFailureBody Should display "Unable to decrypt message" 1`] = exports[`DecryptionFailureBody should handle messages from users who change identities after verification 1`] = `
- - Verified identity has changed + Sender's verified identity has changed
diff --git a/test/unit-tests/components/views/rooms/EventTile-test.tsx b/test/unit-tests/components/views/rooms/EventTile-test.tsx index 7554c83ef0..b2835d15c0 100644 --- a/test/unit-tests/components/views/rooms/EventTile-test.tsx +++ b/test/unit-tests/components/views/rooms/EventTile-test.tsx @@ -19,7 +19,13 @@ import { Room, TweakName, } from "matrix-js-sdk/src/matrix"; -import { CryptoApi, EventEncryptionInfo, EventShieldColour, EventShieldReason } from "matrix-js-sdk/src/crypto-api"; +import { + CryptoApi, + DecryptionFailureCode, + EventEncryptionInfo, + EventShieldColour, + EventShieldReason, +} from "matrix-js-sdk/src/crypto-api"; import { mkEncryptedMatrixEvent } from "matrix-js-sdk/src/testing"; import EventTile, { EventTileProps } from "../../../../../src/components/views/rooms/EventTile"; @@ -350,6 +356,32 @@ describe("EventTile", () => { "mx_EventTile_e2eIcon_decryption_failure", ); }); + + it("should not show a shield for previously-verified users", async () => { + mxEvent = mkEvent({ + type: "m.room.encrypted", + room: room.roomId, + user: "@alice:example.org", + event: true, + content: {}, + }); + + const mockCrypto = { + decryptEvent: async (_ev): Promise => { + throw new Error("can't decrypt"); + }, + } as Parameters[0]; + await mxEvent.attemptDecryption(mockCrypto); + mxEvent["_decryptionFailureReason"] = DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED; + + const { container } = getComponent(); + await act(flushPromises); + + const eventTiles = container.getElementsByClassName("mx_EventTile"); + expect(eventTiles).toHaveLength(1); + + expect(container.getElementsByClassName("mx_EventTile_e2eIcon")).toHaveLength(0); + }); }); it("should update the warning when the event is edited", async () => {