Skip to content

Commit

Permalink
feat(crypto): Support verification violation composer banner
Browse files Browse the repository at this point in the history
  • Loading branch information
@BillCarsonFr
BillCarsonFr committed Jan 22, 2025
1 parent 84c6146 commit e839ab2
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 46 deletions.
6 changes: 6 additions & 0 deletions res/css/views/rooms/_UserIdentityWarning.pcss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ Please see LICENSE files in the repository root for full details.
margin-left: var(--cpd-space-6x);
flex-grow: 1;
}
.mx_UserIdentityWarning_main.critical {
color: var(--cpd-color-text-critical-primary);
}
}
}
.mx_UserIdentityWarning.critical {
background: linear-gradient(180deg, var(--cpd-color-red-100) 0%, var(--cpd-color-theme-bg) 100%);
}

.mx_MessageComposer.mx_MessageComposer--compact > .mx_UserIdentityWarning {
margin-left: calc(-25px + var(--RoomView_MessageList-padding));
Expand Down
164 changes: 119 additions & 45 deletions src/components/views/rooms/UserIdentityWarning.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,23 @@ interface UserIdentityWarningProps {
key: string;
}

type ViolationType = "PinViolation" | "VerificationViolation";

/**
* Does the given user's identity need to be approved?
* Does the given user's identity has a status violation.
*/
async function userNeedsApproval(crypto: CryptoApi, userId: string): Promise<boolean> {
async function userNeedsApproval(crypto: CryptoApi, userId: string): Promise<ViolationType | null> {
const verificationStatus = await crypto.getUserVerificationStatus(userId);
return verificationStatus.needsUserApproval;
return mapToViolationType(verificationStatus);
}

function mapToViolationType(verificationStatus: UserVerificationStatus): ViolationType | null {
if (verificationStatus.wasCrossSigningVerified() && !verificationStatus.isCrossSigningVerified()) {
return "VerificationViolation";
} else if (verificationStatus.needsUserApproval) {
return "PinViolation";
}
return null;
}

/**
Expand All @@ -46,6 +57,11 @@ enum InitialisationStatus {
Completed,
}

type ViolationPrompt = {
member: RoomMember;
type: ViolationType;
};

/**
* Displays a banner warning when there is an issue with a user's identity.
*
Expand All @@ -58,13 +74,13 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }

// The current room member that we are prompting the user to approve.
// `undefined` means we are not currently showing a prompt.
const [currentPrompt, setCurrentPrompt] = useState<RoomMember | undefined>(undefined);
const [currentPrompt, setCurrentPrompt] = useState<ViolationPrompt | undefined>(undefined);

// Whether or not we've already initialised the component by loading the
// room membership.
const initialisedRef = useRef<InitialisationStatus>(InitialisationStatus.Uninitialised);
// Which room members need their identity approved.
const membersNeedingApprovalRef = useRef<Map<string, RoomMember>>(new Map());
const membersNeedingApprovalRef = useRef<Map<string, ViolationPrompt>>(new Map());
// For each user, we assign a sequence number to each verification status
// that we get, or fetch.
//
Expand Down Expand Up @@ -100,7 +116,8 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }
setCurrentPrompt((currentPrompt) => {
// If we're already displaying a warning, and that user still needs
// approval, continue showing that user.
if (currentPrompt && membersNeedingApproval.has(currentPrompt.userId)) return currentPrompt;
if (currentPrompt && membersNeedingApproval.get(currentPrompt.member.userId)?.type === currentPrompt.type)
return currentPrompt;

if (membersNeedingApproval.size === 0) {
if (currentPrompt) {
Expand All @@ -113,7 +130,10 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }
// We pick the user with the smallest user ID.
const keys = Array.from(membersNeedingApproval.keys()).sort((a, b) => a.localeCompare(b));
const selection = membersNeedingApproval.get(keys[0]!);
logger.debug(`UserIdentityWarning: now warning about user ${selection?.userId}`);
logger.debug(`UserIdentityWarning: selection is ${JSON.stringify(selection)}`);
logger.debug(
`UserIdentityWarning: now warning about user ${selection?.member.userId} for a ${selection?.type} violation`,
);
return selection;
});
}, []);
Expand All @@ -123,15 +143,24 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }
// member of the room. If they are not a member, this function will do
// nothing.
const addMemberNeedingApproval = useCallback(
(userId: string, member?: RoomMember): void => {
(userId: string, violation?: ViolationType, member?: RoomMember): void => {
logger.debug(`UserIdentityWarning: add member ${userId} for violation ${violation}`);

if (userId === cli.getUserId()) {
// We always skip our own user, because we can't pin our own identity.
return;
}
if (violation === undefined) return;

// Member might be already resolved depending on the context. When called after a
// CryptoEvent.UserTrustStatusChanged event emitted it will not yet be resolved.
member = member ?? room.getMember(userId) ?? undefined;
if (!member) return;
if (!member) {
logger.debug(`UserIdentityWarning: user ${userId} not found in room members, ignoring violation`);
return;
}

membersNeedingApprovalRef.current.set(userId, member);
membersNeedingApprovalRef.current.set(userId, { member, type: violation });
// We only select the prompt if we are done initialising,
// because we will select the prompt after we're done
// initialising, and we want to start by displaying a warning
Expand Down Expand Up @@ -159,12 +188,12 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }
const userId = member.userId;
const sequenceNum = incrementVerificationStatusSequence(userId);
promises.push(
userNeedsApproval(crypto!, userId).then((needsApproval) => {
if (needsApproval) {
userNeedsApproval(crypto!, userId).then((type) => {
if (type != null) {
// Only actually update the list if we have the most
// recent value.
if (verificationStatusSequences.get(userId) === sequenceNum) {
addMemberNeedingApproval(userId, member);
addMemberNeedingApproval(userId, type, member);
}
}
}),
Expand Down Expand Up @@ -231,8 +260,10 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }

incrementVerificationStatusSequence(userId);

if (verificationStatus.needsUserApproval) {
addMemberNeedingApproval(userId);
const violation = mapToViolationType(verificationStatus);

if (violation) {
addMemberNeedingApproval(userId, violation);
} else {
removeMemberNeedingApproval(userId);
}
Expand Down Expand Up @@ -296,39 +327,82 @@ export const UserIdentityWarning: React.FC<UserIdentityWarningProps> = ({ room }
if (!crypto || !currentPrompt) return null;

const confirmIdentity = async (): Promise<void> => {
await crypto.pinCurrentUserIdentity(currentPrompt.userId);
if (currentPrompt.type === "VerificationViolation") {
await crypto.withdrawVerificationRequirement(currentPrompt.member.userId);

Check failure on line 331 in src/components/views/rooms/UserIdentityWarning.tsx

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check 

Property 'withdrawVerificationRequirement' does not exist on type 'CryptoApi'.
} else if (currentPrompt.type === "PinViolation") {
await crypto.pinCurrentUserIdentity(currentPrompt.member.userId);
}
};

return (
<div className="mx_UserIdentityWarning">
<Separator />
<div className="mx_UserIdentityWarning_row">
<MemberAvatar member={currentPrompt} title={currentPrompt.userId} size="30px" />
<span className="mx_UserIdentityWarning_main">
{currentPrompt.rawDisplayName === currentPrompt.userId
? _t(
"encryption|pinned_identity_changed_no_displayname",
{ userId: currentPrompt.userId },
{
a: substituteATag,
b: substituteBTag,
},
)
: _t(
"encryption|pinned_identity_changed",
{ displayName: currentPrompt.rawDisplayName, userId: currentPrompt.userId },
{
a: substituteATag,
b: substituteBTag,
},
)}
</span>
<Button kind="primary" size="sm" onClick={confirmIdentity}>
{_t("action|ok")}
</Button>
if (currentPrompt.type === "VerificationViolation") {
return (
<div className="mx_UserIdentityWarning critical">
<Separator />
<div className="mx_UserIdentityWarning_row">
<MemberAvatar member={currentPrompt.member} title={currentPrompt.member.userId} size="30px" />
<span className="mx_UserIdentityWarning_main critical">
{currentPrompt.member.rawDisplayName === currentPrompt.member.userId
? _t(
"encryption|verified_identity_changed_no_displayname",
{ userId: currentPrompt.member.userId },
{
a: substituteATag,
b: substituteBTag,
},
)
: _t(
"encryption|verified_identity_changed",
{
displayName: currentPrompt.member.rawDisplayName,
userId: currentPrompt.member.userId,
},
{
a: substituteATag,
b: substituteBTag,
},
)}
</span>
<Button kind="secondary" size="sm" onClick={confirmIdentity}>
{_t("encryption|withdraw_verification_action")}
</Button>
</div>
</div>
</div>
);
);
} else {
return (
<div className="mx_UserIdentityWarning">
<Separator />
<div className="mx_UserIdentityWarning_row">
<MemberAvatar member={currentPrompt.member} title={currentPrompt.member.userId} size="30px" />
<span className="mx_UserIdentityWarning_main">
{currentPrompt.member.rawDisplayName === currentPrompt.member.userId
? _t(
"encryption|pinned_identity_changed_no_displayname",
{ userId: currentPrompt.member.userId },
{
a: substituteATag,
b: substituteBTag,
},
)
: _t(
"encryption|pinned_identity_changed",
{
displayName: currentPrompt.member.rawDisplayName,
userId: currentPrompt.member.userId,
},
{
a: substituteATag,
b: substituteBTag,
},
)}
</span>
<Button kind="primary" size="sm" onClick={confirmIdentity}>
{_t("action|ok")}
</Button>
</div>
</div>
);
}
};

function substituteATag(sub: string): React.ReactNode {
Expand Down
5 changes: 4 additions & 1 deletion src/i18n/strings/en_EN.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1020,8 +1020,11 @@
"waiting_other_user": "Waiting for %(displayName)s to verify…"
},
"verification_requested_toast_title": "Verification requested",
"verified_identity_changed": "%(displayName)s's (<b>%(userId)s</b>) verified identity has changed. <a>Learn more</a>",
"verified_identity_changed_no_displayname": "<b>%(userId)s</b>'s verified identity has changed. <a>Learn more</a>",
"verify_toast_description": "Other users may not trust it",
"verify_toast_title": "Verify this session"
"verify_toast_title": "Verify this session",
"withdraw_verification_action": "Withdraw verification"
},
"error": {
"admin_contact": "Please <a>contact your service administrator</a> to continue using this service.",
Expand Down
80 changes: 80 additions & 0 deletions test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ describe("UserIdentityWarning", () => {
jest.spyOn(room, "getEncryptionTargetMembers").mockResolvedValue([
mockRoomMember("@alice:example.org", "Alice"),
]);
// jest.spyOn(room, "getMember").mockReturnValue(
// mockRoomMember("@alice:example.org", "Alice")
// );
const crypto = client.getCrypto()!;
jest.spyOn(crypto, "getUserVerificationStatus").mockResolvedValue(
new UserVerificationStatus(false, false, false, true),
Expand All @@ -109,6 +112,49 @@ describe("UserIdentityWarning", () => {
await waitFor(() => expect(crypto.pinCurrentUserIdentity).toHaveBeenCalledWith("@alice:example.org"));
});

// This tests the basic functionality of the component. If we have a room
// member whose identity is in verification violation, we should display a warning. When
// the "Withdraw verification" button gets pressed, it should call `withdrawVerification`.
it("displays a warning when a user's identity is in verification violation", async () => {
jest.spyOn(room, "getEncryptionTargetMembers").mockResolvedValue([
mockRoomMember("@alice:example.org", "Alice"),
]);
const crypto = client.getCrypto()!;
jest.spyOn(crypto, "getUserVerificationStatus").mockResolvedValue(
new UserVerificationStatus(false, true, false, true),
);
crypto.withdrawVerificationRequirement = jest.fn();

Check failure on line 126 in test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check 

Property 'withdrawVerificationRequirement' does not exist on type 'CryptoApi'.
renderComponent(client, room);

await waitFor(() =>
expect(getWarningByText("Alice's (@alice:example.org) verified identity has changed.")).toBeInTheDocument(),
);

expect(
screen.getByRole("button", {
name: "Withdraw verification",
}),
).toBeInTheDocument();
await userEvent.click(screen.getByRole("button")!);
await waitFor(() => expect(crypto.withdrawVerificationRequirement).toHaveBeenCalledWith("@alice:example.org"));

Check failure on line 139 in test/unit-tests/components/views/rooms/UserIdentityWarning-test.tsx

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check 

Property 'withdrawVerificationRequirement' does not exist on type 'CryptoApi'.
});

it("Should not display a warning if the user was verified and is still verified", async () => {
jest.spyOn(room, "getEncryptionTargetMembers").mockResolvedValue([
mockRoomMember("@alice:example.org", "Alice"),
]);
const crypto = client.getCrypto()!;
jest.spyOn(crypto, "getUserVerificationStatus").mockResolvedValue(
new UserVerificationStatus(true, true, false, false),
);

renderComponent(client, room);
await sleep(10); // give it some time to finish initialising

expect(() => getWarningByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow();
expect(() => getWarningByText("Alice's (@alice:example.org) verified identity has changed.")).toThrow();
});

// We don't display warnings in non-encrypted rooms, but if encryption is
// enabled, then we should display a warning if there are any users whose
// identity need accepting.
Expand Down Expand Up @@ -472,6 +518,40 @@ describe("UserIdentityWarning", () => {
);
});

it("displays the next user when the verification requirement is withdrawn", async () => {
jest.spyOn(room, "getEncryptionTargetMembers").mockResolvedValue([
mockRoomMember("@alice:example.org", "Alice"),
mockRoomMember("@bob:example.org"),
]);
const crypto = client.getCrypto()!;
jest.spyOn(crypto, "getUserVerificationStatus").mockImplementation(async (userId) => {
if (userId == "@alice:example.org") {
return new UserVerificationStatus(false, true, false, true);
} else {
return new UserVerificationStatus(false, false, false, true);
}
});

renderComponent(client, room);
// We should warn about Alice's identity first.
await waitFor(() =>
expect(getWarningByText("Alice's (@alice:example.org) verified identity has changed.")).toBeInTheDocument(),
);

// Simulate Alice's new identity having been approved, so now we warn
// about Bob's identity.
act(() => {
client.emit(
CryptoEvent.UserTrustStatusChanged,
"@alice:example.org",
new UserVerificationStatus(false, false, false, false),
);
});
await waitFor(() =>
expect(getWarningByText("@bob:example.org's identity appears to have changed.")).toBeInTheDocument(),
);
});

// If we get an update for a user's verification status while we're fetching
// that user's verification status, we should display based on the updated
// value.
Expand Down

0 comments on commit e839ab2

Please sign in to comment.