element-meta icon indicating copy to clipboard operation
element-meta copied to clipboard

Invisible Crypto: Web: display a warning when an *unverified* user changes identity

Open richvdh opened this issue 1 year ago • 2 comments

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 Element Web.

The proposal is to show a warning above the composer, in much the same way as we would for verified users (#2491), but without locking the composer.

Figma designs:

richvdh avatar Aug 28 '24 15:08 richvdh

Implementation notes:

  • ~~in matrix-sdk-crypto, we need to extend UserIdentities to expose identity_needs_user_approval~~ not sure this is needed
  • [x] in matrix-sdk-crypto-wasm, extend UserIdentity and OwnUserIdentity to expose identity_needs_user_approval
    • already done for UserIdentity by https://github.com/matrix-org/matrix-rust-sdk-crypto-wasm/pull/141. Don't think it's needed for OwnUserIdentity.
  • [x] in matrix-js-sdk, extend RustCrypto.getUserVerificationStatus and UserVerificationStatus to include identityNeedsUserApproval
    • https://github.com/matrix-org/matrix-js-sdk/pull/4415
  • [ ] in matrix-react-sdk, create a component which will:
    • on start, call Room.getEncryptionTargetMembers to get a list of users of interest, and then call getUserVerificationStatus on each, to build a list of users with pin violations
    • register callback for room membership updates; when we get one, check if the user is on the target encryption list, and check for pin violations (or remove from list if they have left)
    • register callback for trust updates; when we get one:
      • if the user is not in our list and there is a pin violation, check if they are on the target encryption list, and update the list if so
      • if the user is on our list and there is not a pin violation, remove them immediately

richvdh avatar Sep 10 '24 11:09 richvdh

It would be great if we can re-use the logic I am writing here https://github.com/matrix-org/matrix-rust-sdk/compare/main...andybalaam/room-identity-stream .

The core logic is inside the crypto crate, so there is some chance we can re-use it.

andybalaam avatar Sep 20 '24 10:09 andybalaam

Is there a particular reason we want the banner to say "... appears to have changed" instead of simply "... has changed"? ( @mxandreas ?)

uhoreg avatar Oct 02 '24 17:10 uhoreg

Is there a particular reason we want the banner to say "... appears to have changed" instead of simply "... has changed"?

Great question :) I was told that there is one, but I do not know what it exactly is. I also challenged it a while ago, and the response was that there had been a long debate on it already, thus I left it as there were more important topics. FWIW - I think it helps to distinguish between the pinned vs verified identity change, so supports debugging with users, etc.

mxandreas avatar Oct 03 '24 05:10 mxandreas

Yeah, the wording is from https://github.com/matrix-org/matrix-spec-proposals/blob/andybalaam/crypto-terminology/proposals/4161-crypto-terminology.md#identity and it was crafted over quite a lot of time.

The point is that there are 2 possible cases:

  • EITHER 1. the person is the same, and their cryptographic identity has changed,
  • OR 2. the actual physical identity of the person is different - i.e. their account has been hijacked.

The wording is saying "it looks like 2 happened".

Make sense?

andybalaam avatar Oct 03 '24 09:10 andybalaam

It would be great if we can re-use the logic I am writing here matrix-org/[email protected]/room-identity-stream .

The core logic is inside the crypto crate, so there is some chance we can re-use it.

For reference, this is also https://github.com/matrix-org/matrix-rust-sdk/pull/4022

uhoreg avatar Oct 03 '24 20:10 uhoreg

If there are multiple users whose identity has changed, as a first cut, I'll just show one user at a time. It may be slightly annoying, but hopefully there won't be too many users that need acknowledging at a time.

uhoreg avatar Oct 03 '24 23:10 uhoreg

If there are multiple users whose identity has changed, as a first cut, I'll just show one user at a time. It may be slightly annoying, but hopefully there won't be too many users that need acknowledging at a time.

Yes, that was the plan for now, also for EX.

mxandreas avatar Oct 04 '24 05:10 mxandreas

For visibility: https://github.com/element-hq/element-meta/issues/2525#issuecomment-2394051527

mxandreas avatar Oct 04 '24 16:10 mxandreas

image

uhoreg avatar Oct 09 '24 21:10 uhoreg

@uhoreg Andy already mentioned but just in case - if you implement this, then please do it so that in clear rooms the banner is not shown: https://github.com/element-hq/element-meta/issues/2560

mxandreas avatar Oct 14 '24 13:10 mxandreas

@uhoreg And another similar follow-up to me mentioned is https://github.com/element-hq/element-meta/issues/2565

mxandreas avatar Oct 15 '24 05:10 mxandreas