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.

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

Registration with email is unnecessarily complicated, potentially causing myriad problems #9586

Closed
lampholder opened this issue Apr 29, 2019 · 11 comments · Fixed by matrix-org/matrix-react-sdk#3101

Comments

@lampholder
Copy link
Member

Today, if I register for a fresh account on matrix.org, with an email address, using Riot Web:

  • enter mxid, password and email address
  • riot tells me to check my emails
  • i open my email and click the verif link, which opens in a new tab
  • now I have two tabs!
  • both tabs ask me to complete a CAPTCHA
  • both tabs ask me to accept the terms and conditions
  • both tabs start logging in, and: probably something weird is going to happen now.

We've seen:

  • you're logged in as a guest
  • you are logged in as yourselve but with 2 - 10 devices also logged in
  • one tab asks you to accept the terms and conditions, the other tells you that it's missing session data (and you should Sign Out)
@lampholder

This comment has been minimized.

@lampholder

This comment has been minimized.

@lampholder
Copy link
Member Author

lampholder commented Apr 29, 2019

After discussion, this is better:

  • enter mxid, password and email address,
  • CAPTCHA, t's and c's
  • riot tells me to check my emails to complete registration
    • this tab sits there polling for registration completion to give it the go ahead to log in
  • i open my email and click the verif link, which opens a new tab
    • this tabs says 'well done email verifed thanks'
  • your original tab completes login by itself, no racings

And as a stretch goal we could add some better wording to the email-verified tab that tells you what you should do now (go back to the original tab or login fresh at https://riot.instance/login_uri)

@lampholder
Copy link
Member Author

We should think carefully about this in the context of other clients/riot instances/homeservers.

@turt2live
Copy link
Member

can we please also consider supporting post-registration verification of email? It really suck trying to get going when you're being asked to verify your email now instead of in the next 24-48 hours.

@dikruege

This comment has been minimized.

@jryans jryans changed the title Registration with email is unnecessarily complicated, potentially causing myriad problems. Registration with email is unnecessarily complicated, potentially causing myriad problems May 1, 2019
@dbkr
Copy link
Member

dbkr commented May 1, 2019

How do we want to handle the case where the user closes the original tab?

can we please also consider supporting post-registration verification of email? It really suck trying to get going when you're being asked to verify your email now instead of in the next 24-48 hours.

The problem with this is that we have HSes that require email addresses to create accounts, so we would have to fairly fundamentally change registration so you could register an account and then 'activate' it later by completing email verification.

@dbkr
Copy link
Member

dbkr commented May 1, 2019

How do we want to handle the case where the user closes the original tab?

Answer: the link-clicking tab would complete the register but not log in, so if you closed the original tab, you'll just have to log in to your new account.

dbkr added a commit to matrix-org/synapse that referenced this issue May 10, 2019
This allows the client to complete the email last which is more
natual for the user. Without this stage, if the client would
complete the recaptcha (and terms, if enabled) stages and then the
registration request would complete because you've now completed a
flow, even if you were intending to complete the flow that's the
same except has email auth at the end.

Adding a dummy auth stage to the recaptcha-only flow means it's
always unambiguous which flow the client was trying to complete.
Longer term we should think about changing the protocol so the
client explicitly says which flow it's trying to complete.

element-hq/element-web#9586
dbkr added a commit to matrix-org/synapse that referenced this issue May 10, 2019
This allows the client to complete the email last which is more
natual for the user. Without this stage, if the client would
complete the recaptcha (and terms, if enabled) stages and then the
registration request would complete because you've now completed a
flow, even if you were intending to complete the flow that's the
same except has email auth at the end.

Adding a dummy auth stage to the recaptcha-only flow means it's
always unambiguous which flow the client was trying to complete.
Longer term we should think about changing the protocol so the
client explicitly says which flow it's trying to complete.

element-hq/element-web#9586
dbkr added a commit to matrix-org/synapse that referenced this issue May 14, 2019
We checked that 3pids were not already in use before we checked if
we were going to return the account previously registered in the
same UI auth session, in which case the 3pids will definitely
be in use.

element-hq/element-web#9586
@dbkr
Copy link
Member

dbkr commented May 17, 2019

Some updates on this, mostly for my own notes, because quite a lot has happened:

  • you're logged in as a guest

This one was matrix-org/matrix-react-sdk#2967

CAPTCHA, t's and c's
riot tells me to check my emails to complete registration

matrix-org/synapse#5174 re-ordered the stages so email comes last. The problem with this is that if your email is already registered, you don't get told until after you've done through a captcha and the t&cs. We can fix this by sending the mail right after you hit submit on the form, before captcha/t&cs, but only telling you to go & check your mail after you've done the captcha / t&cs. This also gives some time for the mail to arrive which is kind of nice.

i open my email and click the verif link, which opens a new tab
this tabs says 'well done email verifed thanks'
your original tab completes login by itself, no racings

The problem with this is that the link-clicking client needs to do a register request in order to actually make the account (otherwise if you closed the original tab, the email will get verified but your account won't get created). If we do this though, we end up with a login, even if we don't want one. One idea was to supply inhibit_login to the original register request, so no session gets created, but then in the original client once the registration completes, submit the request again with out inhibitLogin to get our session. This didn't work because the parameters for the UI auth session get replaced and the tabs race, so if the link-clicking client gets there last, the stored parameters will now have no inhibitLogin.

The conclusion here is to add a 'click here to continue' after the email stage so the clients don't log in automatically when registration is done, but you can click to continue if you do actually want to be logged in. If the server didn't do email auth last you might get another auth stage after clicking continue, but that's OK.

dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue May 22, 2019
interactive-auth now has a callback to request the email token which
it will call at the appropriate time (now at the start of the
auth process if the chosen flow contain an email auth stage).

This does make this a breaking change, although not sure this is
used outside of Riot. We could make it backwards compatible by
having an option for the new behaviour. It may not be worthwhile
though.

element-hq/element-web#9586
dbkr added a commit to matrix-org/matrix-react-sdk that referenced this issue May 22, 2019
We previously sent it in componentWillMount of the email token
auth component which definitely gets us on react's naughtly list.
We now pass the js-sdk a callback it can call at the appropriate
time to send the token (matrix-org/matrix-js-sdk#926).

We should make password reset and adding email addresses work the
same way, but currently they don't even use the interactive-auth
helpers(!) so they're unaffected.

element-hq/element-web#9586
@dbkr
Copy link
Member

dbkr commented May 23, 2019

Clarifying because I went off to do other things before implementing this and am now about to do so again. We can't just add a 'click to continue' to both sides because that means if you don't click continue in either place, your account doesn't get registered.

I think the best we can do is:

  • Use inhibitLogin
  • On link-clicking client, just dead-end with a link to login page. If the user wants to log in, they click the link & enter their creds.
  • On original client, poll as we do now, but don't supply the email sid: instead let the link-clicking client actually create the account. When it succeeds, offer a continue button that, ,if pressed, will do the request without inhibitLogin. The request will replace the stored params on the server to remove inhibitLogin but because we didn't supply the email sid, we know the link-clicking client has done their register request so we won't race.
  • The only problem is if the user clicks the continue link, then clicks the validation link again, at which point they will be logged straight in, creating another device, whether they wanted one or not (because the stored params have now been replaced with the set that does not have inhibitLogin.

@jryans
Copy link
Collaborator

jryans commented Jun 11, 2019

Related to registration with email, I just got an error when trying to complete the validation link. I'm mentioning this here in case it's a regression related to recent work in this area.

dbkr added a commit to matrix-org/matrix-react-sdk that referenced this issue Jun 13, 2019
You now don't get automatically logged in after finishing
registration. This makes a whole class of failures involving race
conditions and multiple devices impossible.

element-hq/element-web#9586
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants