From ec9a436625d70e4af616a4fadb19d16d9577bdef Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 8 Jun 2017 11:56:48 +0100 Subject: [PATCH 1/6] Formatting --- src/Login.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Login.js b/src/Login.js index 87731744e9..659fdb92bb 100644 --- a/src/Login.js +++ b/src/Login.js @@ -161,8 +161,7 @@ export default class Login { error.friendlyText = ( _t('This Home Server does not support login using email address.') ); - } - else if (error.httpStatus === 403) { + } else if (error.httpStatus === 403) { error.friendlyText = ( _t('Incorrect username and/or password.') ); @@ -185,8 +184,7 @@ export default class Login { throw error; }); } - } - else { + } else { error.friendlyText = ( _t("There was a problem logging in.") + ' (HTTP ' + error.httpStatus + ")" ); From 6d1fa775a04c24b37bf81524f1eb03516176e62e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 8 Jun 2017 14:14:45 +0100 Subject: [PATCH 2/6] Formatting --- src/components/structures/login/Login.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/structures/login/Login.js b/src/components/structures/login/Login.js index 840f0874a7..be440febdc 100644 --- a/src/components/structures/login/Login.js +++ b/src/components/structures/login/Login.js @@ -219,8 +219,8 @@ module.exports = React.createClass({ if (err.cors === 'rejected') { if (window.location.protocol === 'https:' && (this.state.enteredHomeserverUrl.startsWith("http:") || - !this.state.enteredHomeserverUrl.startsWith("http"))) - { + !this.state.enteredHomeserverUrl.startsWith("http")) + ) { errorText = { _tJsx("Can't connect to homeserver via HTTP when an HTTPS URL is in your browser bar. " + "Either use HTTPS or enable unsafe scripts.", @@ -228,8 +228,7 @@ module.exports = React.createClass({ (sub) => { return { sub }; } )} ; - } - else { + } else { errorText = { _tJsx("Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted.", /(.*?)<\/a>/, From d691c891e7353e95d309474f92bf2d1ff7cc6546 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 8 Jun 2017 15:49:48 +0100 Subject: [PATCH 3/6] Move all login error string generation into view This makes all the various hits done by login report the same useful error messages and gets rid of the broken ones like printing the http status code even if it was undefined. Also add text for the case of overzealous browser extensions because lots of people get bitten by it. --- src/Login.js | 18 +-------- src/components/structures/login/Login.js | 51 ++++++++++++++++-------- src/i18n/strings/en_EN.json | 2 +- 3 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/Login.js b/src/Login.js index 659fdb92bb..8db6e99b89 100644 --- a/src/Login.js +++ b/src/Login.js @@ -97,11 +97,6 @@ export default class Login { guest: true }; }, (error) => { - if (error.httpStatus === 403) { - error.friendlyText = _t("Guest access is disabled on this Home Server."); - } else { - error.friendlyText = _t("Failed to register as guest:") + ' ' + error.data; - } throw error; }); } @@ -157,14 +152,7 @@ export default class Login { accessToken: data.access_token }); }, function(error) { - if (error.httpStatus == 400 && loginParams.medium) { - error.friendlyText = ( - _t('This Home Server does not support login using email address.') - ); - } else if (error.httpStatus === 403) { - error.friendlyText = ( - _t('Incorrect username and/or password.') - ); + if (error.httpStatus === 403) { if (self._fallbackHsUrl) { var fbClient = Matrix.createClient({ baseUrl: self._fallbackHsUrl, @@ -184,10 +172,6 @@ export default class Login { throw error; }); } - } else { - error.friendlyText = ( - _t("There was a problem logging in.") + ' (HTTP ' + error.httpStatus + ")" - ); } throw error; }); diff --git a/src/components/structures/login/Login.js b/src/components/structures/login/Login.js index be440febdc..7d283e0e36 100644 --- a/src/components/structures/login/Login.js +++ b/src/components/structures/login/Login.js @@ -87,7 +87,24 @@ module.exports = React.createClass({ ).then((data) => { this.props.onLoggedIn(data); }, (error) => { - this._setStateFromError(error, true); + let errorText; + + // Some error strings only apply for logging in + const usingEmail = username.indexOf("@"); + if (error.httpStatus == 400 && usingEmail) { + errorText = _t('This Home Server does not support login using email address.'); + } else if (error.httpStatus == 401 || error.httpStatus === 403) { + errorText = _t('Incorrect username and/or password.'); + } else { + // other errors, not specific to doing a password login + errorText = this._errorTextFromError(error); + } + + this.setState({ + errorText: errorText, + // https://matrix.org/jira/browse/SYN-744 + loginIncorrect: error.httpStatus == 401 || error.httpStatus == 403 + }); }).finally(() => { this.setState({ busy: false @@ -110,7 +127,16 @@ module.exports = React.createClass({ this._loginLogic.loginAsGuest().then(function(data) { self.props.onLoggedIn(data); }, function(error) { - self._setStateFromError(error, true); + let errorText; + if (error.httpStatus === 403) { + errorText = _t("Guest access is disabled on this Home Server."); + } else { + errorText = self._errorTextFromError(error); + } + self.setState({ + errorText: errorText, + loginIncorrect: false, + }); }).finally(function() { self.setState({ busy: false @@ -183,31 +209,22 @@ module.exports = React.createClass({ currentFlow: self._getCurrentFlowStep(), }); }, function(err) { - self._setStateFromError(err, false); + self.setState({ + errorText: self._errorTextFromError(err), + loginIncorrect: false, + }); }).finally(function() { self.setState({ busy: false, }); - }); + }).done(); }, _getCurrentFlowStep: function() { return this._loginLogic ? this._loginLogic.getCurrentFlowStep() : null; }, - _setStateFromError: function(err, isLoginAttempt) { - this.setState({ - errorText: this._errorTextFromError(err), - // https://matrix.org/jira/browse/SYN-744 - loginIncorrect: isLoginAttempt && (err.httpStatus == 401 || err.httpStatus == 403) - }); - }, - _errorTextFromError(err) { - if (err.friendlyText) { - return err.friendlyText; - } - let errCode = err.errcode; if (!errCode && err.httpStatus) { errCode = "HTTP " + err.httpStatus; @@ -230,7 +247,7 @@ module.exports = React.createClass({ ; } else { errorText = - { _tJsx("Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted.", + { _tJsx("Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted and that a browser extension is not blocking requests.", /(.*?)<\/a>/, (sub) => { return { sub }; } )} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 8dbbb98423..6984d46ed4 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -180,7 +180,7 @@ "Bug Report": "Bug Report", "Bulk Options": "Bulk Options", "Call Timeout": "Call Timeout", - "Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted.": "Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted.", + "Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted and that a browser extension is not blocking requests.": "Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted and that a browser extension is not blocking requests.", "Can't connect to homeserver via HTTP when an HTTPS URL is in your browser bar. Either use HTTPS or enable unsafe scripts.": "Can't connect to homeserver via HTTP when an HTTPS URL is in your browser bar. Either use HTTPS or enable unsafe scripts.", "Can't load user settings": "Can't load user settings", "Change Password": "Change Password", From 8dc20606c8be5ae78859178b91f2760ea37d0827 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 8 Jun 2017 16:07:29 +0100 Subject: [PATCH 4/6] Use comma for list of three things --- src/components/structures/login/Login.js | 2 +- src/i18n/strings/en_EN.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/structures/login/Login.js b/src/components/structures/login/Login.js index 7d283e0e36..6d0e929c93 100644 --- a/src/components/structures/login/Login.js +++ b/src/components/structures/login/Login.js @@ -247,7 +247,7 @@ module.exports = React.createClass({ ; } else { errorText = - { _tJsx("Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted and that a browser extension is not blocking requests.", + { _tJsx("Can't connect to homeserver - please check your connectivity, ensure your homeserver's SSL certificate is trusted, and that a browser extension is not blocking requests.", /(.*?)<\/a>/, (sub) => { return { sub }; } )} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 6984d46ed4..c6086faf0a 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -180,7 +180,7 @@ "Bug Report": "Bug Report", "Bulk Options": "Bulk Options", "Call Timeout": "Call Timeout", - "Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted and that a browser extension is not blocking requests.": "Can't connect to homeserver - please check your connectivity and ensure your homeserver's SSL certificate is trusted and that a browser extension is not blocking requests.", + "Can't connect to homeserver - please check your connectivity, ensure your homeserver's SSL certificate is trusted, and that a browser extension is not blocking requests.": "Can't connect to homeserver - please check your connectivity, ensure your homeserver's SSL certificate is trusted, and that a browser extension is not blocking requests.", "Can't connect to homeserver via HTTP when an HTTPS URL is in your browser bar. Either use HTTPS or enable unsafe scripts.": "Can't connect to homeserver via HTTP when an HTTPS URL is in your browser bar. Either use HTTPS or enable unsafe scripts.", "Can't load user settings": "Can't load user settings", "Change Password": "Change Password", From 9def0bb5c2947ecfc53ce1abbe8e959cc2ab23d8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 8 Jun 2017 16:08:19 +0100 Subject: [PATCH 5/6] Oops, fix email check --- src/components/structures/login/Login.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/login/Login.js b/src/components/structures/login/Login.js index 6d0e929c93..bbfff3712e 100644 --- a/src/components/structures/login/Login.js +++ b/src/components/structures/login/Login.js @@ -90,7 +90,7 @@ module.exports = React.createClass({ let errorText; // Some error strings only apply for logging in - const usingEmail = username.indexOf("@"); + const usingEmail = username.indexOf("@") > 0; if (error.httpStatus == 400 && usingEmail) { errorText = _t('This Home Server does not support login using email address.'); } else if (error.httpStatus == 401 || error.httpStatus === 403) { From 5e55b6a6435f8d342ab84281d31407f14792b54c Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 8 Jun 2017 16:23:43 +0100 Subject: [PATCH 6/6] PR feedback --- src/components/structures/login/Login.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/structures/login/Login.js b/src/components/structures/login/Login.js index bbfff3712e..27c0c200be 100644 --- a/src/components/structures/login/Login.js +++ b/src/components/structures/login/Login.js @@ -93,7 +93,7 @@ module.exports = React.createClass({ const usingEmail = username.indexOf("@") > 0; if (error.httpStatus == 400 && usingEmail) { errorText = _t('This Home Server does not support login using email address.'); - } else if (error.httpStatus == 401 || error.httpStatus === 403) { + } else if (error.httpStatus === 401 || error.httpStatus === 403) { errorText = _t('Incorrect username and/or password.'); } else { // other errors, not specific to doing a password login @@ -102,8 +102,11 @@ module.exports = React.createClass({ this.setState({ errorText: errorText, - // https://matrix.org/jira/browse/SYN-744 - loginIncorrect: error.httpStatus == 401 || error.httpStatus == 403 + // 401 would be the sensible status code for 'incorrect password' + // but the login API gives a 403 https://matrix.org/jira/browse/SYN-744 + // mentions this (although the bug is for UI auth which is not this) + // We treat both as an incorrect password + loginIncorrect: error.httpStatus === 401 || error.httpStatus == 403, }); }).finally(() => { this.setState({