From d5552e4a179ccd975367d412168430518779d797 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 5 Sep 2019 17:51:27 +0100 Subject: [PATCH 1/7] Add bound 3PID warning when changing IS as well This extends the bound 3PID warning from the disconnect button to also appear when changing the IS as well. At the moment, the text is a bit terse, but will be improved separately. Fixes https://github.com/vector-im/riot-web/issues/10749 --- src/components/views/settings/SetIdServer.js | 134 ++++++++++++------- src/i18n/strings/en_EN.json | 6 +- 2 files changed, 87 insertions(+), 53 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index a718d87fa6..e3d6d18c5b 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -137,7 +137,12 @@ export default class SetIdServer extends React.Component { MatrixClientPeg.get().setAccountData("m.identity_server", { base_url: fullUrl, }); - this.setState({idServer: '', busy: false, error: null}); + this.setState({ + busy: false, + error: null, + currentClientIdServer: fullUrl, + idServer: '', + }); }; _checkIdServer = async (e) => { @@ -157,14 +162,34 @@ export default class SetIdServer extends React.Component { const authClient = new IdentityAuthClient(fullUrl); await authClient.getAccessToken(); + let save = true; + // Double check that the identity server even has terms of service. const terms = await MatrixClientPeg.get().getTerms(SERVICE_TYPES.IS, fullUrl); if (!terms || !terms["policies"] || Object.keys(terms["policies"]).length <= 0) { - this._showNoTermsWarning(fullUrl); - return; + save &= await this._showNoTermsWarning(fullUrl); } - this._saveIdServer(fullUrl); + // Show a general warning, possibly with details about any bound + // 3PIDs that would be left behind. + if (this.state.currentClientIdServer) { + save &= await this._showServerChangeWarning({ + title: _t("Change identity server"), + unboundMessage: _t( + "Disconnect from the identity server and " + + "connect to instead?", {}, + { + current: sub => {abbreviateUrl(this.state.currentClientIdServer)}, + new: sub => {abbreviateUrl(this.state.idServer)}, + }, + ), + button: _t("Continue"), + }); + } + + if (save) { + this._saveIdServer(fullUrl); + } } catch (e) { console.error(e); if (e.cors === "rejected" || e.httpStatus === 404) { @@ -179,73 +204,80 @@ export default class SetIdServer extends React.Component { checking: false, error: errStr, currentClientIdServer: MatrixClientPeg.get().getIdentityServerUrl(), - idServer: this.state.idServer, }); }; _showNoTermsWarning(fullUrl) { const QuestionDialog = sdk.getComponent("views.dialogs.QuestionDialog"); - Modal.createTrackedDialog('No Terms Warning', '', QuestionDialog, { - title: _t("Identity server has no terms of service"), - description: ( -
- - {_t("The identity server you have chosen does not have any terms of service.")} - - -  {_t("Only continue if you trust the owner of the server.")} - -
- ), - button: _t("Continue"), - onFinished: async (confirmed) => { - if (!confirmed) return; - this._saveIdServer(fullUrl); - }, + return new Promise(resolve => { + Modal.createTrackedDialog('No Terms Warning', '', QuestionDialog, { + title: _t("Identity server has no terms of service"), + description: ( +
+ + {_t("The identity server you have chosen does not have any terms of service.")} + + +  {_t("Only continue if you trust the owner of the server.")} + +
+ ), + button: _t("Continue"), + onFinished: resolve, + }); }); } _onDisconnectClicked = async () => { this.setState({disconnectBusy: true}); try { - const threepids = await getThreepidBindStatus(MatrixClientPeg.get()); - - const boundThreepids = threepids.filter(tp => tp.bound); - let message; - if (boundThreepids.length) { - message = _t( - "You are currently sharing email addresses or phone numbers on the identity " + - "server . You will need to reconnect to to stop " + - "sharing them.", {}, - { - idserver: sub => {abbreviateUrl(this.state.currentClientIdServer)}, - // XXX: https://github.com/vector-im/riot-web/issues/9086 - idserver2: sub => {abbreviateUrl(this.state.currentClientIdServer)}, - }, - ); - } else { - message = _t( + const confirmed = await this._showServerChangeWarning({ + title: _t("Disconnect identity server"), + unboundMessage: _t( "Disconnect from the identity server ?", {}, {idserver: sub => {abbreviateUrl(this.state.currentClientIdServer)}}, - ); - } - - const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); - Modal.createTrackedDialog('Identity Server Disconnect Warning', '', QuestionDialog, { - title: _t("Disconnect Identity Server"), - description: message, + ), button: _t("Disconnect"), - onFinished: (confirmed) => { - if (confirmed) { - this._disconnectIdServer(); - } - }, }); + if (confirmed) { + this._disconnectIdServer(); + } } finally { this.setState({disconnectBusy: false}); } }; + async _showServerChangeWarning({ title, unboundMessage, button }) { + const threepids = await getThreepidBindStatus(MatrixClientPeg.get()); + + const boundThreepids = threepids.filter(tp => tp.bound); + let message; + if (boundThreepids.length) { + message = _t( + "You are currently sharing email addresses or phone numbers on the identity " + + "server . You will need to reconnect to to stop " + + "sharing them.", {}, + { + idserver: sub => {abbreviateUrl(this.state.currentClientIdServer)}, + // XXX: https://github.com/vector-im/riot-web/issues/9086 + idserver2: sub => {abbreviateUrl(this.state.currentClientIdServer)}, + }, + ); + } else { + message = unboundMessage; + } + + const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); + return new Promise(resolve => { + Modal.createTrackedDialog('Identity Server Bound Warning', '', QuestionDialog, { + title, + description: message, + button, + onFinished: resolve, + }); + }); + } + _disconnectIdServer = () => { // Account data change will update localstorage, client, etc through dispatcher MatrixClientPeg.get().setAccountData("m.identity_server", { diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index dd0894b813..f31fcc7157 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -554,14 +554,16 @@ "Not a valid Identity Server (status code %(code)s)": "Not a valid Identity Server (status code %(code)s)", "Could not connect to Identity Server": "Could not connect to Identity Server", "Checking server": "Checking server", + "Change identity server": "Change identity server", + "Disconnect from the identity server and connect to instead?": "Disconnect from the identity server and connect to instead?", "Terms of service not accepted or the identity server is invalid.": "Terms of service not accepted or the identity server is invalid.", "Identity server has no terms of service": "Identity server has no terms of service", "The identity server you have chosen does not have any terms of service.": "The identity server you have chosen does not have any terms of service.", "Only continue if you trust the owner of the server.": "Only continue if you trust the owner of the server.", - "You are currently sharing email addresses or phone numbers on the identity server . You will need to reconnect to to stop sharing them.": "You are currently sharing email addresses or phone numbers on the identity server . You will need to reconnect to to stop sharing them.", + "Disconnect identity server": "Disconnect identity server", "Disconnect from the identity server ?": "Disconnect from the identity server ?", - "Disconnect Identity Server": "Disconnect Identity Server", "Disconnect": "Disconnect", + "You are currently sharing email addresses or phone numbers on the identity server . You will need to reconnect to to stop sharing them.": "You are currently sharing email addresses or phone numbers on the identity server . You will need to reconnect to to stop sharing them.", "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.", "If you don't want to use to discover and be discoverable by existing contacts you know, enter another identity server below.": "If you don't want to use to discover and be discoverable by existing contacts you know, enter another identity server below.", From 2ff592c54259757aa4a2fcda8ad898543a4a019d Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 6 Sep 2019 10:48:24 +0100 Subject: [PATCH 2/7] Use existing modal promises --- src/components/views/settings/SetIdServer.js | 52 ++++++++++---------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index e3d6d18c5b..ebd97cebc1 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -167,13 +167,14 @@ export default class SetIdServer extends React.Component { // Double check that the identity server even has terms of service. const terms = await MatrixClientPeg.get().getTerms(SERVICE_TYPES.IS, fullUrl); if (!terms || !terms["policies"] || Object.keys(terms["policies"]).length <= 0) { - save &= await this._showNoTermsWarning(fullUrl); + const [confirmed] = await this._showNoTermsWarning(fullUrl); + save &= confirmed; } // Show a general warning, possibly with details about any bound // 3PIDs that would be left behind. - if (this.state.currentClientIdServer) { - save &= await this._showServerChangeWarning({ + if (save && this.state.currentClientIdServer) { + const [confirmed] = await this._showServerChangeWarning({ title: _t("Change identity server"), unboundMessage: _t( "Disconnect from the identity server and " + @@ -185,6 +186,7 @@ export default class SetIdServer extends React.Component { ), button: _t("Continue"), }); + save &= confirmed; } if (save) { @@ -209,29 +211,27 @@ export default class SetIdServer extends React.Component { _showNoTermsWarning(fullUrl) { const QuestionDialog = sdk.getComponent("views.dialogs.QuestionDialog"); - return new Promise(resolve => { - Modal.createTrackedDialog('No Terms Warning', '', QuestionDialog, { - title: _t("Identity server has no terms of service"), - description: ( -
- - {_t("The identity server you have chosen does not have any terms of service.")} - - -  {_t("Only continue if you trust the owner of the server.")} - -
- ), - button: _t("Continue"), - onFinished: resolve, - }); + const { finished } = Modal.createTrackedDialog('No Terms Warning', '', QuestionDialog, { + title: _t("Identity server has no terms of service"), + description: ( +
+ + {_t("The identity server you have chosen does not have any terms of service.")} + + +  {_t("Only continue if you trust the owner of the server.")} + +
+ ), + button: _t("Continue"), }); + return finished; } _onDisconnectClicked = async () => { this.setState({disconnectBusy: true}); try { - const confirmed = await this._showServerChangeWarning({ + const [confirmed] = await this._showServerChangeWarning({ title: _t("Disconnect identity server"), unboundMessage: _t( "Disconnect from the identity server ?", {}, @@ -268,14 +268,12 @@ export default class SetIdServer extends React.Component { } const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); - return new Promise(resolve => { - Modal.createTrackedDialog('Identity Server Bound Warning', '', QuestionDialog, { - title, - description: message, - button, - onFinished: resolve, - }); + const { finished } = Modal.createTrackedDialog('Identity Server Bound Warning', '', QuestionDialog, { + title, + description: message, + button, }); + return finished; } _disconnectIdServer = () => { From 19fff51b01a1494dfca432d00ed3c0a91cdbad61 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 6 Sep 2019 11:11:54 +0100 Subject: [PATCH 3/7] Rework handling for terms CORS error Earlier changes in this branch removed the "next step" of saving from the dialogs, so we need to fold in the CORS error case. --- src/components/views/settings/SetIdServer.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index ebd97cebc1..20524d8ae3 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -165,7 +165,18 @@ export default class SetIdServer extends React.Component { let save = true; // Double check that the identity server even has terms of service. - const terms = await MatrixClientPeg.get().getTerms(SERVICE_TYPES.IS, fullUrl); + let terms; + try { + terms = await MatrixClientPeg.get().getTerms(SERVICE_TYPES.IS, fullUrl); + } catch (e) { + console.error(e); + if (e.cors === "rejected" || e.httpStatus === 404) { + terms = null; + } else { + throw e; + } + } + if (!terms || !terms["policies"] || Object.keys(terms["policies"]).length <= 0) { const [confirmed] = await this._showNoTermsWarning(fullUrl); save &= confirmed; @@ -194,10 +205,6 @@ export default class SetIdServer extends React.Component { } } catch (e) { console.error(e); - if (e.cors === "rejected" || e.httpStatus === 404) { - this._showNoTermsWarning(fullUrl); - return; - } errStr = _t("Terms of service not accepted or the identity server is invalid."); } } From df03477907dd7173a9fdf983ab592f9d2346f2a8 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 6 Sep 2019 11:18:25 +0100 Subject: [PATCH 4/7] Show change warning only when different from current --- src/components/views/settings/SetIdServer.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index 20524d8ae3..6460ad4b1f 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -147,10 +147,11 @@ export default class SetIdServer extends React.Component { _checkIdServer = async (e) => { e.preventDefault(); + const { idServer, currentClientIdServer } = this.state; this.setState({busy: true, checking: true, error: null}); - const fullUrl = unabbreviateUrl(this.state.idServer); + const fullUrl = unabbreviateUrl(idServer); let errStr = await checkIdentityServerUrl(fullUrl); if (!errStr) { @@ -179,25 +180,26 @@ export default class SetIdServer extends React.Component { if (!terms || !terms["policies"] || Object.keys(terms["policies"]).length <= 0) { const [confirmed] = await this._showNoTermsWarning(fullUrl); - save &= confirmed; + save = confirmed; } // Show a general warning, possibly with details about any bound // 3PIDs that would be left behind. - if (save && this.state.currentClientIdServer) { + console.log(fullUrl, idServer, currentClientIdServer) + if (save && currentClientIdServer && fullUrl !== currentClientIdServer) { const [confirmed] = await this._showServerChangeWarning({ title: _t("Change identity server"), unboundMessage: _t( "Disconnect from the identity server and " + "connect to instead?", {}, { - current: sub => {abbreviateUrl(this.state.currentClientIdServer)}, - new: sub => {abbreviateUrl(this.state.idServer)}, + current: sub => {abbreviateUrl(currentClientIdServer)}, + new: sub => {abbreviateUrl(idServer)}, }, ), button: _t("Continue"), }); - save &= confirmed; + save = confirmed; } if (save) { From 11a5fca7024b8f489c39aaebf773f6e07e931908 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 6 Sep 2019 13:44:44 +0100 Subject: [PATCH 5/7] Remove logs --- src/components/views/settings/SetIdServer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index 6460ad4b1f..d3fc944a70 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -185,7 +185,6 @@ export default class SetIdServer extends React.Component { // Show a general warning, possibly with details about any bound // 3PIDs that would be left behind. - console.log(fullUrl, idServer, currentClientIdServer) if (save && currentClientIdServer && fullUrl !== currentClientIdServer) { const [confirmed] = await this._showServerChangeWarning({ title: _t("Change identity server"), From 3ad88604cc32e95a92a43af054790a8c2d6408d9 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 6 Sep 2019 13:43:21 +0100 Subject: [PATCH 6/7] Stregthen bound 3PID warning dialog This tweaks the bound 3PID text and adds danger styling. Fixes https://github.com/vector-im/riot-web/issues/10750 --- src/components/views/settings/SetIdServer.js | 16 +++++++++++----- src/i18n/strings/en_EN.json | 5 +++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index d3fc944a70..afb2d62e0e 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -260,17 +260,21 @@ export default class SetIdServer extends React.Component { const boundThreepids = threepids.filter(tp => tp.bound); let message; + let danger = false; if (boundThreepids.length) { message = _t( - "You are currently sharing email addresses or phone numbers on the identity " + - "server . You will need to reconnect to to stop " + - "sharing them.", {}, + "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.", {}, { idserver: sub => {abbreviateUrl(this.state.currentClientIdServer)}, - // XXX: https://github.com/vector-im/riot-web/issues/9086 - idserver2: sub => {abbreviateUrl(this.state.currentClientIdServer)}, + b: sub => {sub}, + br: () =>
, }, ); + danger = true; + button = _t("Disconnect anyway"); } else { message = unboundMessage; } @@ -280,6 +284,8 @@ export default class SetIdServer extends React.Component { title, description: message, button, + cancelButton: _t("Go back"), + danger, }); return finished; } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index f31fcc7157..02fd05b067 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -563,7 +563,9 @@ "Disconnect identity server": "Disconnect identity server", "Disconnect from the identity server ?": "Disconnect from the identity server ?", "Disconnect": "Disconnect", - "You are currently sharing email addresses or phone numbers on the identity server . You will need to reconnect to to stop sharing them.": "You are currently sharing email addresses or phone numbers on the identity server . You will need to reconnect to to stop sharing them.", + "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.": "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.", + "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.", "If you don't want to use to discover and be discoverable by existing contacts you know, enter another identity server below.": "If you don't want to use to discover and be discoverable by existing contacts you know, enter another identity server below.", @@ -1287,7 +1289,6 @@ "If you run into any bugs or have feedback you'd like to share, please let us know on GitHub.": "If you run into any bugs or have feedback you'd like to share, please let us know on GitHub.", "To help avoid duplicate issues, please view existing issues first (and add a +1) or create a new issue if you can't find it.": "To help avoid duplicate issues, please view existing issues first (and add a +1) or create a new issue if you can't find it.", "Report bugs & give feedback": "Report bugs & give feedback", - "Go back": "Go back", "Room Settings - %(roomName)s": "Room Settings - %(roomName)s", "Failed to upgrade room": "Failed to upgrade room", "The room upgrade could not be completed": "The room upgrade could not be completed", From c3adddb5ac41ec1516f275a8c1b33eb676fafe1e Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Mon, 9 Sep 2019 10:27:02 +0100 Subject: [PATCH 7/7] Change to paragraphs outside the strings --- src/components/views/settings/SetIdServer.js | 25 +++++++++++--------- src/i18n/strings/en_EN.json | 3 ++- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index afb2d62e0e..af7416518d 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -262,17 +262,20 @@ export default class SetIdServer extends React.Component { let message; let danger = false; if (boundThreepids.length) { - message = _t( - "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.", {}, - { - idserver: sub => {abbreviateUrl(this.state.currentClientIdServer)}, - b: sub => {sub}, - br: () =>
, - }, - ); + message =
+

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

+

{_t( + "We recommend that you remove your email addresses and phone numbers " + + "from the identity server before disconnecting.", + )}

+
; danger = true; button = _t("Disconnect anyway"); } else { diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 02fd05b067..26db20fe93 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -563,7 +563,8 @@ "Disconnect identity server": "Disconnect identity server", "Disconnect from the identity server ?": "Disconnect from the identity server ?", "Disconnect": "Disconnect", - "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.": "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.", + "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)",