Skip to content

Commit

Permalink
Update the display of decryption failures due to failed trust require…
Browse files Browse the repository at this point in the history
…ment (#28300)

* update the display of decryption failures due to failed trust requirement

* add test for not showing shield
  • Loading branch information
uhoreg authored Nov 4, 2024
1 parent 502cc91 commit 2631b90
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 33 deletions.
2 changes: 1 addition & 1 deletion playwright/e2e/crypto/invisible-crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
19 changes: 4 additions & 15 deletions res/css/views/messages/_DecryptionFailureBody.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 10 additions & 4 deletions src/components/views/messages/DecryptionFailureBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -41,15 +41,20 @@ function getErrorMessage(mxEvent: MatrixEvent, isVerified: boolean | undefined):
case DecryptionFailureCode.SENDER_IDENTITY_PREVIOUSLY_VERIFIED:
return (
<span>
<WarningIcon className="mx_Icon mx_Icon_16" />
<BlockIcon className="mx_Icon mx_Icon_16" />
{_t("timeline|decryption_failure|sender_identity_previously_verified")}
</span>
);

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 (
<span>
<BlockIcon className="mx_Icon mx_Icon_16" />
{_t("timeline|decryption_failure|sender_unsigned_device")}
</span>
);
}
return _t("timeline|decryption_failure|unable_to_decrypt");
}
Expand All @@ -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;
Expand Down
10 changes: 9 additions & 1 deletion src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -719,7 +720,14 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>

// event could not be decrypted
if (ev.isDecryptionFailure()) {
return <E2ePadlockDecryptionFailure />;
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 <E2ePadlockDecryptionFailure />;
}
}

if (this.state.shieldColour !== EventShieldColour.NONE) {
Expand Down
4 changes: 2 additions & 2 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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`] = `
<div>
<div
class="mx_DecryptionFailureBody mx_EventTile_content mx_DecryptionFailureVerifiedIdentityChanged"
class="mx_DecryptionFailureBody mx_EventTile_content mx_DecryptionFailureSenderTrustRequirement"
>
<span>
<svg
Expand All @@ -35,15 +35,10 @@ exports[`DecryptionFailureBody should handle messages from users who change iden
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M12.713 17.713A.968.968 0 0 1 12 18a.968.968 0 0 1-.713-.287A.967.967 0 0 1 11 17a.97.97 0 0 1 .287-.712A.968.968 0 0 1 12 16a.97.97 0 0 1 .713.288A.968.968 0 0 1 13 17a.97.97 0 0 1-.287.713Zm0-4A.968.968 0 0 1 12 14a.968.968 0 0 1-.713-.287A.967.967 0 0 1 11 13V9a.97.97 0 0 1 .287-.712A.968.968 0 0 1 12 8a.97.97 0 0 1 .713.288A.968.968 0 0 1 13 9v4a.97.97 0 0 1-.287.713Z"
/>
<path
clip-rule="evenodd"
d="M10.264 3.039c.767-1.344 2.705-1.344 3.472 0l8.554 14.969c.762 1.333-.2 2.992-1.736 2.992H3.446c-1.535 0-2.498-1.659-1.736-2.992l8.553-14.969ZM3.446 19 12 4.031l8.554 14.97H3.446Z"
fill-rule="evenodd"
d="M12 22a9.738 9.738 0 0 1-3.9-.788 10.099 10.099 0 0 1-3.175-2.137c-.9-.9-1.612-1.958-2.137-3.175A9.738 9.738 0 0 1 2 12a9.74 9.74 0 0 1 .788-3.9 10.099 10.099 0 0 1 2.137-3.175c.9-.9 1.958-1.612 3.175-2.137A9.738 9.738 0 0 1 12 2a9.74 9.74 0 0 1 3.9.788 10.098 10.098 0 0 1 3.175 2.137c.9.9 1.613 1.958 2.137 3.175A9.738 9.738 0 0 1 22 12a9.738 9.738 0 0 1-.788 3.9 10.098 10.098 0 0 1-2.137 3.175c-.9.9-1.958 1.613-3.175 2.137A9.738 9.738 0 0 1 12 22Zm0-2c2.233 0 4.125-.775 5.675-2.325C19.225 16.125 20 14.233 20 12c0-.9-.146-1.767-.438-2.6A7.951 7.951 0 0 0 18.3 7.1L7.1 18.3c.7.55 1.467.97 2.3 1.262.833.292 1.7.438 2.6.438Zm-6.3-3.1L16.9 5.7a7.95 7.95 0 0 0-2.3-1.263A7.813 7.813 0 0 0 12 4c-2.233 0-4.125.775-5.675 2.325C4.775 7.875 4 9.767 4 12c0 .9.146 1.767.438 2.6A7.95 7.95 0 0 0 5.7 16.9Z"
/>
</svg>
Verified identity has changed
Sender's verified identity has changed
</span>
</div>
</div>
Expand Down
34 changes: 33 additions & 1 deletion test/unit-tests/components/views/rooms/EventTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<IEventDecryptionResult> => {
throw new Error("can't decrypt");
},
} as Parameters<MatrixEvent["attemptDecryption"]>[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 () => {
Expand Down

0 comments on commit 2631b90

Please sign in to comment.