element-hq / element-meta

Shared/meta documentation and project artefacts for Element clients
75 stars 12 forks source link

Invisible Crypto: Rust for EX: display a warning when an *unverified* user changes identity #2526

Closed andybalaam closed 4 weeks ago

andybalaam commented 1 month ago

Followup to https://github.com/element-hq/element-meta/issues/2492. Part of https://github.com/element-hq/element-meta/issues/2491, itself part of Invisible crypto.

When an unverified user changes their identity, we need to make our user aware of this. In the long term, the intention is just to show a notice in the timeline (#2493); however, that is difficult to implement and we need a stop-gap.

This task is for matrix-sdk-ui, to support https://github.com/element-hq/element-meta/issues/2525 and https://github.com/element-hq/element-meta/issues/2524 .

We think the best way to do this will be to allow listening for pin violation events on a room.

We think there may already be some kind of event when an identity changes, but we need to make sure you can listen on a room for identity changes of members of that room.

These pin violations will be triggered by a /keys/query response - either because some other person's identity changed, or because our own did.

Additionally, we need to make sure there is a suitable API to call when the user clicks "OK" - something like OtherUserIdentity::pin_new_identity exists, but we need to make sure it will work.

Later update: this is currently implemented by providing a stream of changes to users' identities, but in retrospect we think (thanks @BillCarsonFr for the suggestion) that it would be better to manage the set of all violating users either by returning all violating users in each update (which would mean the UI does not need to store any state, but might send a lot of data in rooms where something bad has happened so lots of identities are violating) or returning a delta to the set of all violating users. If/when we get a chance, we should consider changing to this approach.

andybalaam commented 1 month ago

@BillCarsonFr notes there is something at the store level - maybe identities_broadcaster that could be part-way to a solution for this.

andybalaam commented 1 month ago

Implementation: I had a chat with @stefanceriu and he said he'd like to see this information in a matrix_sdk_ffi::room_info::RoomInfo that is received via a RoomInfoUpdateListener in JoinedRoomProxy.swift.

RoomInfo is populated from a matrix_sdk::Room which contains a matrix_sdk_base::rooms::normal::Room that holds a SharedObservable of matrix_sdk_base::rooms::normal::RoomInfo.

So if we update matrix_sdk_base::rooms::normal::RoomInfo for every room they are a member of, adding info about their pinnedness, whenever someone becomes unpinned or repinned, and trace back out to matrix_sdk_ffi::room_info::RoomInfo it should work.

Additionally, we need to expose a method to "re-pin" this identity (i.e. dismiss the warning). There is already such a method on the identity structs in the Rust crypto layer, but we would need to expose this to the higher levels.

@poljar if you were able to review the above and comment on whether it is sane that would be very helpful.

poljar commented 1 month ago

Sorry, I missed the ping initially.

We think there may already be some kind of event when an identity changes, but we need to make sure you can listen on a room for identity changes of members of that room.

This should happen for every room the member is in? Presumably we're only interested in this information when a room is actually open?

So if we update matrix_sdk_base::rooms::normal::RoomInfo for every room they are a member of, adding info about their pinnedness, whenever someone becomes unpinned or repinned, and trace back out to matrix_sdk_ffi::room_info::RoomInfo it should work.

This sounds expensive and you need to watch out for races, i.e. multiple things might want to update the RoomInfo, besides the RoomInfo is already holding too many things and is supposed to only contain things that are necessary to render the room list. Are we showing any info about pin violations in the room list?

I think that this needs a new concept and it doesn't really belong into the RoomInfo.