Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Verification fails to start (intermittently) when there is no existing DM with the target user #23819

Closed
richvdh opened this issue Nov 23, 2022 · 4 comments · Fixed by matrix-org/matrix-js-sdk#2910 or matrix-org/matrix-js-sdk#2914
Closed

Verification fails to start (intermittently) when there is no existing DM with the target user #23819

richvdh opened this issue Nov 23, 2022 · 4 comments · Fixed by matrix-org/matrix-js-sdk#2910 or matrix-org/matrix-js-sdk#2914
Labels
A-E2EE-SAS-Verification O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect

Comments

@richvdh
Copy link
Member

richvdh commented Nov 23, 2022 •
edited
Loading

Steps to reproduce

  1. Share a room, but not a DM with target user
  2. Open target user's details and click "verify"

Outcome

What did you expect?

Verification process starts correctly

What happened instead?

  1. Target user receives invite to DM, but there are no messages in the room:
    image

  2. Originating user sees an error in the DM:
    image

Clicking "resend" resumes the verification process.

Operating system

Linux

Browser information

Firefox

URL for webapp

https://vdhtest.riot.sw1v.org/

Application version

element v1.11.15

Homeserver

No response

Will you send logs?

Yes
@richvdh
Copy link
Member Author

richvdh commented Nov 23, 2022

From the console (this does not appear in the rageshake for some reason; possibly because it's "uncaught in promise"):

12:04:35.378
Uncaught (in promise) Error: Room was previously configured to use encryption, but is no longer. Perhaps the homeserver is hiding the configuration event.
    encryptEvent index.ts:2817
    encryptEventIfNeeded client.ts:4107
    encryptAndSendEvent client.ts:4003
    promise callback*encryptAndSendEvent client.ts:4002
    sendCompleteEvent client.ts:3988
    sendEvent client.ts:3914
    sendCompleted InRoomChannel.ts:33
    send InRoomChannel.ts:259
    sendRequest VerificationRequest.ts:456
    requestVerificationWithChannel index.ts:2318
    requestVerificationDM index.ts:2293
    requestVerificationDM client.ts:2031
    _ EncryptionPanel.tsx:115
    React 11
    unstable_runWithPriority scheduler.production.min.js:18
    React 11
    ne init.tsx:144
    68 index.ts:229
    68 index.ts:240

@uhoreg uhoreg added S-Critical Prevents work, causes data loss and/or has no workaround A-E2EE-SAS-Verification O-Occasional Affects or can be seen by some users regularly or most users rarely labels Nov 23, 2022
@richvdh
Copy link
Member Author

richvdh commented Nov 24, 2022

related: #18463

@richvdh richvdh mentioned this issue Nov 25, 2022
Merged
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 25, 2022
@richvdh
Add a test for verifying without existing DM
1acf995 
A regression test for element-hq/element-web#23819.
@richvdh richvdh mentioned this issue Nov 25, 2022
Merged
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 25, 2022
@richvdh
Add a test for verifying without existing DM
5bea567 
A regression test for element-hq/element-web#23819.
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 25, 2022
@richvdh
Add a test for verifying without existing DM
6c8f4f5 
A regression test for element-hq/element-web#23819.
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 25, 2022
@richvdh
Add a test for verifying without existing DM
b89b4c9 
A regression test for element-hq/element-web#23819.
@richvdh richvdh mentioned this issue Nov 25, 2022
Merged
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 25, 2022
@richvdh
Show user an error if we fail to create a DM for verification
68a9698 
Related: element-hq/element-web#23819

This is still pretty poor, but at least we don't get stuck with a
'verifying...' spinner that is a total failure.
@t3chguy t3chguy reopened this Nov 28, 2022
@richvdh
Copy link
Member Author

richvdh commented Nov 28, 2022

PR was reverted by matrix-org/matrix-js-sdk#2913

@t3chguy
Copy link
Member

t3chguy commented Nov 28, 2022

Fix caused develop to break, so got reverted

richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 28, 2022
@richvdh
Pass a client into RoomNotifs.getRoomNotifsState
9c384c6 
Pass an explicit client into `RoomNotifs.getRoomNotifsState`, rather than
relying on `MatrixClientPeg`. This resolves a race condition where we have a
component which thinks it is using a particular component, but
`MatrixClientPeg` has been updated.

A precursor to fixing element-hq/element-web#23819.
This was referenced Nov 28, 2022
Merged
Merged
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 29, 2022
@richvdh
Show user an error if we fail to create a DM for verification
df96bba 
Related: element-hq/element-web#23819

This is still pretty poor, but at least we don't get stuck with a
'verifying...' spinner that is a total failure.
richvdh added a commit to matrix-org/matrix-js-sdk that referenced this issue Nov 30, 2022
@richvdh
Process m.room.encryption events before emitting RoomMember events (
1606274 
#2914)

element-hq/element-web#23819 is an intermittent failure to correctly initiate a user verification process. The
root cause is as follows:

* In matrix-react-sdk, ensureDMExists tries to create an encrypted DM room, and assumes it is ready for use
  (including sending encrypted events) as soon as it receives a RoomStateEvent.NewMember notification
  indicating that the other user has been invited or joined. 

* However, in sync.ts, we process the membership events in a /sync response (including emitting
  RoomStateEvent.NewMember notifications), which is long before we process any m.room.encryption event.
    
* The upshot is that we can end up trying to send an encrypted event in the new room before processing
  the m.room.encryption event, which causes the crypto layer to blow up with an error of "Room was 
  previously configured to use encryption, but is no longer".

Strictly speaking, ensureDMExists probably ought to be listening for ClientEvent.Room as well as RoomStateEvent.NewMember; but that doesn't help us, because ClientEvent.Room is also emitted
before we process the crypto event.

So, we need to process the crypto event before we start emitting these other events; but a corollary of that 
is that we need to do so before we store the new room in the client's store. That makes things tricky, because
currently the crypto layer expects the room to have been stored in the client first.

So... we have to rearrange everything to pass the newly-created Room object into the crypto layer, rather than
just the room id, so that it doesn't need to rely on getting the Room from the client's store.
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 30, 2022
@richvdh
Show user an error if we fail to create a DM for verification (#9624)
baaa9f5 
Related: element-hq/element-web#23819

This is still pretty poor, but at least we don't get stuck with a
'verifying...' spinner that is a total failure.
richvdh added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 30, 2022
@richvdh
Add a test for verifying without existing DM (#9619)
4b3705d 
A regression test for element-hq/element-web#23819.
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Jan 15, 2023
@su-ex
Merge tag 'v23.0.0' into sc
b9afe23 
* Process `m.room.encryption` events before emitting `RoomMember` events ([\matrix-org#2914](matrix-org#2914)). Fixes element-hq/element-web#23819.
* Don't expose `calls` on `GroupCall` ([\matrix-org#2941](matrix-org#2941)).
* Support MSC3391: Account data deletion ([\matrix-org#2967](matrix-org#2967)).
* Add a message ID on each to-device message ([\matrix-org#2938](matrix-org#2938)).
* Enable multiple users' power levels to be set at once ([\matrix-org#2892](matrix-org#2892)). Contributed by @GoodGuyMarco.
* Include pending events in thread summary and count again ([\matrix-org#2922](matrix-org#2922)). Fixes element-hq/element-web#23642.
* Make GroupCall work better with widgets ([\matrix-org#2935](matrix-org#2935)).
* Add method to get outgoing room key requests for a given event ([\matrix-org#2930](matrix-org#2930)).
* Fix messages loaded during initial fetch ending up out of order ([\matrix-org#2971](matrix-org#2971)). Fixes element-hq/element-web#23972.
* Fix #23919: Root message for new thread loaded from network ([\matrix-org#2965](matrix-org#2965)). Fixes element-hq/element-web#23919.
* Fix #23916: Prevent edits of the last message in a thread getting lost ([\matrix-org#2951](matrix-org#2951)). Fixes element-hq/element-web#23916 and element-hq/element-web#23942.
* Fix infinite loop when restoring cached read receipts ([\matrix-org#2963](matrix-org#2963)). Fixes element-hq/element-web#23951.
* Don't swallow errors coming from the shareSession call ([\matrix-org#2962](matrix-org#2962)). Fixes element-hq/element-web#23792.
* Make sure that MegolmEncryption.setupPromise always resolves  ([\matrix-org#2960](matrix-org#2960)).
* Do not calculate highlight notifs for threads unknown to the room ([\matrix-org#2957](matrix-org#2957)).
* Cache read receipts for unknown threads ([\matrix-org#2953](matrix-org#2953)).
* bugfix: sliding sync initial room timelines shouldn't notify ([\matrix-org#2933](matrix-org#2933)).
* Redo key sharing after own device verification ([\matrix-org#2921](matrix-org#2921)). Fixes element-hq/element-web#23333.
* Move updated threads to the end of the thread list ([\matrix-org#2923](matrix-org#2923)). Fixes element-hq/element-web#23876.
* Fix highlight notifications increasing when total notification is zero ([\matrix-org#2937](matrix-org#2937)). Fixes element-hq/element-web#23885.
* Fix synthesizeReceipt ([\matrix-org#2916](matrix-org#2916)). Fixes element-hq/element-web#23827 element-hq/element-web#23754 and element-hq/element-web#23847.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-E2EE-SAS-Verification O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect
Projects
None yet
Milestone
No milestone
4 participants
@uhoreg @richvdh @t3chguy and others