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

Simplify email registration #3101

Merged
merged 2 commits into from
Jun 13, 2019
Merged

Simplify email registration #3101

merged 2 commits into from
Jun 13, 2019

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jun 13, 2019

You now don't get automatically logged in after finishing registration (if you registered with an email address). This makes a whole class of failures involving race conditions and multiple devices impossible.

Instead, this happens on the page you started on:
Screenshot 2019-06-13 at 16 33 03

...and this happens on the page you got from clicking the email link:
Screenshot 2019-06-13 at 16 33 23

Fixes element-hq/element-web#9586
Requires matrix-org/matrix-js-sdk#953

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
@dbkr dbkr requested a review from a team June 13, 2019 15:35
@bwindels bwindels requested review from bwindels and removed request for a team June 13, 2019 16:23
Copy link
Contributor

@bwindels bwindels 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, just one question

// Only send the bind params if we're sending username / pw params
// (Since we need to send no params at all to use the ones saved in the
// session).
const bindThreepids = this.state.formVals.password ? {
email: true,
msisdn: true,
} : {};
// Likewise inhibitLogin
if (!this.state.formVals.password) inhibitLogin = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the render method, I guess this means the client that was opened from the e-mail. If so, could you clarify in the comment above? Also, I thought that if you also had to click "Log in" in the browser that was opened from the e-mail? Probably misunderstanding something ...

Copy link
Member Author

Choose a reason for hiding this comment

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

ah yes, sorry - this was from when I was going to put a button in the original client to re-submit the request, which we've now decided not to do.

@dbkr dbkr merged commit 446b20c into develop Jun 13, 2019
dbkr added a commit that referenced this pull request Jun 14, 2019
#3101 meant we
don't get logged straight in after registering if using an email
address, but this was the point at which we made a chat with the
welcome user. Instead, set a flag in memory that we should try &
make a chat with the welcome user for that user ID if we get a
session for them.

Of course, if the user logs in on both tabs, this would mean each
would make a chat with the welcome user (although actually this
was a problem with the old code too). Check our m.direct to see if
we've started a chat with the welcome user before making one (which
also means we have to make sure the cached sync is up to date...
see comments).
@dbkr dbkr mentioned this pull request Jun 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registration with email is unnecessarily complicated, potentially causing myriad problems
2 participants