From d7631ed9f8f08c912d5e970824bf973aa9b6bff1 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 11 Oct 2019 15:52:15 +0100 Subject: [PATCH 1/3] Catch errors in Settings when IS is unreachable A few bits of Settings try to talk to the IS when Settings is opened. This changes them to handle failure by logging warnings to the console. --- .../tabs/user/GeneralUserSettingsTab.js | 54 ++++++++++++------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js index 64aafe6046..78961ad663 100644 --- a/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/GeneralUserSettingsTab.js @@ -102,7 +102,17 @@ export default class GeneralUserSettingsTab extends React.Component { // Need to get 3PIDs generally for Account section and possibly also for // Discovery (assuming we have an IS and terms are agreed). - const threepids = await getThreepidsWithBindStatus(cli); + let threepids = []; + try { + threepids = await getThreepidsWithBindStatus(cli); + } catch (e) { + const idServerUrl = MatrixClientPeg.get().getIdentityServerUrl(); + console.warn( + `Unable to reach identity server at ${idServerUrl} to check ` + + `for 3PIDs bindings in Settings`, + ); + console.warn(e); + } this.setState({ emails: threepids.filter((a) => a.medium === 'email') }); this.setState({ msisdns: threepids.filter((a) => a.medium === 'msisdn') }); } @@ -115,32 +125,40 @@ export default class GeneralUserSettingsTab extends React.Component { // By starting the terms flow we get the logic for checking which terms the user has signed // for free. So we might as well use that for our own purposes. + const idServerUrl = MatrixClientPeg.get().getIdentityServerUrl(); const authClient = new IdentityAuthClient(); const idAccessToken = await authClient.getAccessToken({ check: false }); - startTermsFlow([new Service( - SERVICE_TYPES.IS, - MatrixClientPeg.get().getIdentityServerUrl(), - idAccessToken, - )], (policiesAndServices, agreedUrls, extraClassNames) => { - return new Promise((resolve, reject) => { - this.setState({ - idServerName: abbreviateUrl(MatrixClientPeg.get().getIdentityServerUrl()), - requiredPolicyInfo: { - hasTerms: true, - policiesAndServices, - agreedUrls, - resolve, - }, - }); + try { + await startTermsFlow([new Service( + SERVICE_TYPES.IS, + idServerUrl, + idAccessToken, + )], (policiesAndServices, agreedUrls, extraClassNames) => { + return new Promise((resolve, reject) => { + this.setState({ + idServerName: abbreviateUrl(idServerUrl), + requiredPolicyInfo: { + hasTerms: true, + policiesAndServices, + agreedUrls, + resolve, + }, + }); + }); }); - }).then(() => { // User accepted all terms this.setState({ requiredPolicyInfo: { hasTerms: false, }, }); - }); + } catch (e) { + console.warn( + `Unable to reach identity server at ${idServerUrl} to check ` + + `for terms in Settings`, + ); + console.warn(e); + } } _onLanguageChange = (newLanguage) => { From e6a81c5733a901adce38c2da4cd98ca00db6b7ac Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 11 Oct 2019 15:57:12 +0100 Subject: [PATCH 2/3] Show warning dialog when changing unreachable IS If the IS is unreachable, this handles the error by showing a warning encouraging the user to check after their personal data and resolve the situation, but still allows them to continue if they want. Fixes https://github.com/vector-im/riot-web/issues/10909 --- src/components/views/settings/SetIdServer.js | 49 +++++++++++++++++--- src/i18n/strings/en_EN.json | 7 ++- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index 9ef5fb295e..8359c09d87 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -249,20 +249,55 @@ export default class SetIdServer extends React.Component { }; async _showServerChangeWarning({ title, unboundMessage, button }) { - const threepids = await getThreepidsWithBindStatus(MatrixClientPeg.get()); + const { currentClientIdServer } = this.state; + + let threepids = []; + let currentServerReachable = true; + try { + threepids = await getThreepidsWithBindStatus(MatrixClientPeg.get()); + } catch (e) { + currentServerReachable = false; + console.warn( + `Unable to reach identity server at ${currentClientIdServer} to check ` + + `for 3PIDs during IS change flow`, + ); + console.warn(e); + } const boundThreepids = threepids.filter(tp => tp.bound); let message; let danger = false; - if (boundThreepids.length) { + const messageElements = { + idserver: sub => {abbreviateUrl(currentClientIdServer)}, + b: sub => {sub}, + }; + if (!currentServerReachable) { + message =
+

{_t( + "You should remove your personal data from identity server " + + " before disconnecting. Unfortunately, identity server " + + " is currently offline or cannot be reached.", + {}, messageElements, + )}

+

{_t("You should:")}

+
    +
  • {_t( + "check your browser plugins for anything that might block " + + "the identity server (such as Privacy Badger)", + )}
    • +
  • {_t("contact the administrators of identity server ", {}, { + idserver: messageElements.idserver, + })}
    • +
  • {_t("wait and try again later")}
    • +
+
; + danger = true; + button = _t("Disconnect anyway"); + } else if (boundThreepids.length) { message =

{_t( "You are still sharing your personal data on the identity " + - "server .", {}, - { - idserver: sub => {abbreviateUrl(this.state.currentClientIdServer)}, - b: sub => {sub}, - }, + "server .", {}, messageElements, )}

{_t( "We recommend that you remove your email addresses and phone numbers " + diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index f524a22d4b..2b7ef28ace 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -568,9 +568,14 @@ "Disconnect identity server": "Disconnect identity server", "Disconnect from the identity server ?": "Disconnect from the identity server ?", "Disconnect": "Disconnect", + "You should remove your personal data from identity server before disconnecting. Unfortunately, identity server is currently offline or cannot be reached.": "You should remove your personal data from identity server before disconnecting. Unfortunately, identity server is currently offline or cannot be reached.", + "You should:": "You should:", + "check your browser plugins for anything that might block the identity server (such as Privacy Badger)": "check your browser plugins for anything that might block the identity server (such as Privacy Badger)", + "contact the administrators of identity server ": "contact the administrators of identity server ", + "wait and try again later": "wait and try again later", + "Disconnect anyway": "Disconnect anyway", "You are still sharing your personal data on the identity server .": "You are still sharing your personal data on the identity server .", "We recommend that you remove your email addresses and phone numbers from the identity server before disconnecting.": "We recommend that you remove your email addresses and phone numbers from the identity server before disconnecting.", - "Disconnect anyway": "Disconnect anyway", "Go back": "Go back", "Identity Server (%(server)s)": "Identity Server (%(server)s)", "You are currently using to discover and be discoverable by existing contacts you know. You can change your identity server below.": "You are currently using to discover and be discoverable by existing contacts you know. You can change your identity server below.", From 579ada3ca27ffd4ef299f02140c581dc9bcda89e Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 18 Oct 2019 12:40:50 +0100 Subject: [PATCH 3/3] Add an overall reachability timeout of 10s This adds a reachability timeout of 10s when checking the IS for 3PID bindings. This ensures we stop in a reasonable time, rather than waiting for a long list of requests to eventually timeout via some general mechanism. Part of https://github.com/vector-im/riot-web/issues/10909 --- src/components/views/settings/SetIdServer.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index 8359c09d87..7f4a50d391 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -28,6 +28,9 @@ import {SERVICE_TYPES} from "matrix-js-sdk"; import {abbreviateUrl, unabbreviateUrl} from "../../../utils/UrlUtils"; import { getDefaultIdentityServerUrl } from '../../../utils/IdentityServerUtils'; +// We'll wait up to this long when checking for 3PID bindings on the IS. +const REACHABILITY_TIMEOUT = 10000; // ms + /** * Check an IS URL is valid, including liveness check * @@ -254,7 +257,14 @@ export default class SetIdServer extends React.Component { let threepids = []; let currentServerReachable = true; try { - threepids = await getThreepidsWithBindStatus(MatrixClientPeg.get()); + threepids = await Promise.race([ + getThreepidsWithBindStatus(MatrixClientPeg.get()), + new Promise((resolve, reject) => { + setTimeout(() => { + reject(new Error("Timeout attempting to reach identity server")); + }, REACHABILITY_TIMEOUT); + }), + ]); } catch (e) { currentServerReachable = false; console.warn( @@ -263,7 +273,6 @@ export default class SetIdServer extends React.Component { ); console.warn(e); } - const boundThreepids = threepids.filter(tp => tp.bound); let message; let danger = false;