Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
MSC2366: Key verification flow additions: m.key.verification.ready and m.key.verification.done #2366
MSC2366: Key verification flow additions: m.key.verification.ready and m.key.verification.done #2366
Changes from 1 commit
33df2d1
abbba47
5f5f99f
5742c30
17abe05
22f7eaa
4672a70
2e0ef11
c741049
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How long should the other client wait until it receives a done, as old clients won't send that? The normal timeout of 10min sounds pretty long.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same question. Riot doesn't send this, when you are not verifying in DMs it seems. So we kinda need a behaviour for the transition phase, when a client follows the current spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It definitely should be sent in DMs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be only sent, when verifying via DMs. So it seems like it is not sent in to_device messaging. This proposal suggests, that clients would be sending them already, but they don't. Unless I am missing something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry. I read your sentence several times, and managed to miss the "not" every time. :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried in Element Web in a non-DM self-verification, and it does send an
m.key.verification.ready
in response tom.key.verification.request
. Trying to directly verify another user's device seems to send anm.key.verification.start
directly, so it is not affected by this case.I haven't tried Element Android or Element iOS yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that matches my tests as well. It can deal with request, but if you start it from Element, it uses start (for to_device verifications).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested Element Web, and it sent an
m.key.verification.done
after verifying using to-device. I haven't tested Android or iOS. In response to the original question in this thread:I think that we're at the point where we don't need to worry about "old" clients (it looks like Element is doing this, and just waiting until the normal timeout), but if you do want to worry about old clients, you could 1) wait for the
.done
, but allow the user to exit the verification UI, or 2) exit the verification UI once it's completed on your side, without waiting for the.done
to come in from the other user.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does element-web also send a
.done
if you don't initiate verification via popup but via the device menu on the right? The device menu on the right also skips the.request
and starts directly with a.start
, so yeahThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I tested initialting verification both via the popup and via the device menu. In both situations, it sent the
.done
in my tests. (I also noticed that initiating from the device menu skipped the.request
step.)