From 048d8d2ec706c6fcb3f8e0891842b85ee0968032 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 13 Jun 2019 16:24:09 +0100 Subject: [PATCH 1/2] Simplify email registration 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. https://github.com/vector-im/riot-web/issues/9586 --- .../structures/auth/Registration.js | 108 ++++++++++++++---- src/i18n/strings/en_EN.json | 3 + 2 files changed, 91 insertions(+), 20 deletions(-) diff --git a/src/components/structures/auth/Registration.js b/src/components/structures/auth/Registration.js index bf4a86e410..9a7c927603 100644 --- a/src/components/structures/auth/Registration.js +++ b/src/components/structures/auth/Registration.js @@ -28,6 +28,7 @@ import { messageForResourceLimitError } from '../../../utils/ErrorUtils'; import * as ServerType from '../../views/auth/ServerTypeSelector'; import AutoDiscoveryUtils, {ValidatedServerConfig} from "../../../utils/AutoDiscoveryUtils"; import classNames from "classnames"; +import * as Lifecycle from '../../../Lifecycle'; // Phases // Show controls to configure server details @@ -80,6 +81,9 @@ module.exports = React.createClass({ // Phase of the overall registration dialog. phase: PHASE_REGISTRATION, flows: null, + // If set, we've registered but are not going to log + // the user in to their new account automatically. + completedNoSignin: false, // We perform liveliness checks later, but for now suppress the errors. // We also track the server dead errors independently of the regular errors so @@ -209,6 +213,7 @@ module.exports = React.createClass({ errorText: _t("Registration has been disabled on this homeserver."), }); } else { + console.log("Unable to query for supported registration methods.", e); this.setState({ errorText: _t("Unable to query for supported registration methods."), }); @@ -282,21 +287,27 @@ module.exports = React.createClass({ return; } - this.setState({ - // we're still busy until we get unmounted: don't show the registration form again - busy: true, + const newState = { doingUIAuth: false, - }); + }; + if (response.access_token) { + const cli = await this.props.onLoggedIn({ + userId: response.user_id, + deviceId: response.device_id, + homeserverUrl: this.state.matrixClient.getHomeserverUrl(), + identityServerUrl: this.state.matrixClient.getIdentityServerUrl(), + accessToken: response.access_token, + }); - const cli = await this.props.onLoggedIn({ - userId: response.user_id, - deviceId: response.device_id, - homeserverUrl: this.state.matrixClient.getHomeserverUrl(), - identityServerUrl: this.state.matrixClient.getIdentityServerUrl(), - accessToken: response.access_token, - }); + this._setupPushers(cli); + // we're still busy until we get unmounted: don't show the registration form again + newState.busy = true; + } else { + newState.busy = false; + newState.completedNoSignin = true; + } - this._setupPushers(cli); + this.setState(newState); }, _setupPushers: function(matrixClient) { @@ -352,7 +363,16 @@ module.exports = React.createClass({ }); }, - _makeRegisterRequest: function(auth) { + _makeRegisterRequest: function(auth, inhibitLogin) { + // default is to inhibit login if we're trying to register with an email address + // We do this so that the client that gets spawned when clicking on the email + // verification link doesn't get logged in (it can't choose different params + // because it doesn't have the password and it can only supply a complete + // set of parameters). If the original client is still around when the + // registration completes, it can resubmit with inhibitLogin=false to + // log itself in! + if (inhibitLogin === undefined) inhibitLogin = Boolean(this.state.formVals.email); + // 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). @@ -360,6 +380,8 @@ module.exports = React.createClass({ email: true, msisdn: true, } : {}; + // Likewise inhibitLogin + if (!this.state.formVals.password) inhibitLogin = null; return this.state.matrixClient.register( this.state.formVals.username, @@ -368,6 +390,7 @@ module.exports = React.createClass({ auth, bindThreepids, null, + inhibitLogin, ); }, @@ -379,6 +402,19 @@ module.exports = React.createClass({ }; }, + // Links to the login page shown after registration is completed are routed through this + // which checks the user hasn't already logged in somewhere else (perhaps we should do + // this more generally?) + _onLoginClickWithCheck: async function(ev) { + ev.preventDefault(); + + const sessionLoaded = await Lifecycle.loadSession({}); + if (!sessionLoaded) { + // ok fine, there's still no session: really go to the login page + this.props.onLoginClick(); + } + }, + renderServerComponent() { const ServerTypeSelector = sdk.getComponent("auth.ServerTypeSelector"); const ServerConfig = sdk.getComponent("auth.ServerConfig"); @@ -528,17 +564,49 @@ module.exports = React.createClass({ ; } + let body; + if (this.state.completedNoSignin) { + let regDoneText; + if (this.state.formVals.password) { + // We're the client that started the registration + regDoneText = _t( + "Log in to your new account.", {}, + { + a: (sub) => {sub}, + }, + ); + } else { + // We're not the original client: the user probably got to us by clicking the + // email validation link. We can't offer a 'go straight to your account' link + // as we don't have the original creds. + regDoneText = _t( + "You can now close this window or log in to your new account.", {}, + { + a: (sub) => {sub}, + }, + ); + } + body =
+

{_t("Registration Successful")}

+

{ regDoneText }

+
; + } else { + body =
+

{ _t('Create your account') }

+ { errorText } + { serverDeadSection } + { this.renderServerComponent() } + { this.renderRegisterComponent() } + { goBack } + { signIn } +
; + } + return ( -

{ _t('Create your account') }

- { errorText } - { serverDeadSection } - { this.renderServerComponent() } - { this.renderRegisterComponent() } - { goBack } - { signIn } + { body } ); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 69ac59f984..53fd82f6f2 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1557,6 +1557,9 @@ "Registration has been disabled on this homeserver.": "Registration has been disabled on this homeserver.", "Unable to query for supported registration methods.": "Unable to query for supported registration methods.", "This server does not support authentication with a phone number.": "This server does not support authentication with a phone number.", + "Log in to your new account.": "Log in to your new account.", + "You can now close this window or log in to your new account.": "You can now close this window or log in to your new account.", + "Registration Successful": "Registration Successful", "Create your account": "Create your account", "Commands": "Commands", "Results from DuckDuckGo": "Results from DuckDuckGo", From 81327264f7dcbb6b5ba05ab20b355a0a803c6a08 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 13 Jun 2019 17:44:00 +0100 Subject: [PATCH 2/2] Remove unused inhibitlogin param and fix docs. --- src/components/structures/auth/Registration.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/components/structures/auth/Registration.js b/src/components/structures/auth/Registration.js index 9a7c927603..3103ee41df 100644 --- a/src/components/structures/auth/Registration.js +++ b/src/components/structures/auth/Registration.js @@ -363,15 +363,12 @@ module.exports = React.createClass({ }); }, - _makeRegisterRequest: function(auth, inhibitLogin) { - // default is to inhibit login if we're trying to register with an email address - // We do this so that the client that gets spawned when clicking on the email - // verification link doesn't get logged in (it can't choose different params - // because it doesn't have the password and it can only supply a complete - // set of parameters). If the original client is still around when the - // registration completes, it can resubmit with inhibitLogin=false to - // log itself in! - if (inhibitLogin === undefined) inhibitLogin = Boolean(this.state.formVals.email); + _makeRegisterRequest: function(auth) { + // We inhibit login if we're trying to register with an email address: this + // avoids a lot of complex race conditions that can occur if we try to log + // the user in one one or both of the tabs they might end up with after + // clicking the email link. + let inhibitLogin = Boolean(this.state.formVals.email); // 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