Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Rework cross-signing login flow #5727

Merged
merged 19 commits into from
Mar 11, 2021
Merged

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Mar 8, 2021

This thread really struck home (I assume it's one of the reasons we lost Canonical), and the fact that we've failed to budget time to iterate and improve the deeply suboptimal cross-signing login flows since launching it at FOSDEM 2020 is very frustrating. So I went on a hack this evening to apply a series of targeted strikes to fix it:

  • When logging in to an existing account and setting up encryption, you now get an explicit "verify using another session" button rather expecting the user to figure out they need to initiate verification from another session. Likewise when you hit the "Verify this session" toast.
  • We don't show the dialog at all if the user doesn't have an MSK to self-verify, and has no devices to verify against.
  • The "new login" notification now doesn't prompt you to verify, which rarely worked given the new login would likely be stuck in the middle of an initial sync and not able to be verified. Instead it prompts you to check your devices (taking you to the place you'd kick out the new login if you didn't recognise it)
  • Adds an explicit link to edit your devices from the verification UI on your own UserInfo
  • If you accidentally click in the background it now cancels the process correctly
  • Rename the annoying "Review your logins!" prompt to actually explain why it's asking you to review your logins.
  • We now report IPs of new logins and self-verification attempts

This PR obviously should be split into 7 or so small PRs, but for expediency i've bundled them together here. However, the commits are nicely structured so they can be reviewed one by one.

Depends on matrix-org/matrix-js-sdk#1632


@jryans: I've collected some open issues that I believe are solved by this.

Fixes element-hq/element-web#16252
Fixes element-hq/element-web#15877
Fixes element-hq/element-web#15672
Fixes element-hq/element-web#15429
Fixes element-hq/element-web#13289
Fixes element-hq/element-web#13510

@ara4n ara4n requested a review from a team March 8, 2021 05:35
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, it seems like a more reliable workflow, thanks! 😄

I've added a few details we should work out before merging.

@jryans jryans changed the title Rework cross-signing login flow. Rework cross-signing login flow Mar 8, 2021
@ara4n
Copy link
Member Author

ara4n commented Mar 9, 2021

@jryans PTAL again

@t3chguy t3chguy requested a review from jryans March 9, 2021 00:23
@jryans
Copy link
Collaborator

jryans commented Mar 9, 2021

Adding Design review as well, so they can take a look at these workflow changes before merging.

@jryans jryans requested a review from a team March 9, 2021 11:11
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me code wise, with one tiny cleanup needed. I have added this PR to the Design queue as well.

@jryans
Copy link
Collaborator

jryans commented Mar 9, 2021

It will likely to take another day anyway given Design review, but I would suggest we wait until after the RC is cut tomorrow to merge this, so we can have maximal bake time on develop.

Copy link
Contributor

@nadonomy nadonomy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to remove the unnecessary paths. It would be good to merge this in to test more generally and test stability on develop, but @jryans we should park some time Mar 22-24 ahead of the next RC to tweak some of the copy. I'll book something in our calendars as a reminder.

@jryans jryans merged commit 6a939c4 into develop Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.