From 9860baf0b4f9bffd708a7c2248ed2f54cf6f671d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 15 Aug 2019 15:59:44 -0600 Subject: [PATCH 1/5] Prompt for terms of service on identity server changes Part of https://github.com/vector-im/riot-web/issues/10539 --- src/IdentityAuthClient.js | 51 +++++++++++++--- src/components/views/settings/SetIdServer.js | 63 +++++++++++++++++--- src/i18n/strings/en_EN.json | 5 +- 3 files changed, 101 insertions(+), 18 deletions(-) diff --git a/src/IdentityAuthClient.js b/src/IdentityAuthClient.js index 755205d5e2..12c0c5b147 100644 --- a/src/IdentityAuthClient.js +++ b/src/IdentityAuthClient.js @@ -14,15 +14,50 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { SERVICE_TYPES } from 'matrix-js-sdk'; +import Matrix, { SERVICE_TYPES } from 'matrix-js-sdk'; import MatrixClientPeg from './MatrixClientPeg'; import { Service, startTermsFlow, TermsNotSignedError } from './Terms'; export default class IdentityAuthClient { - constructor() { + /** + * Creates a new identity auth client + * @param {string} identityUrl The URL to contact the identity server with. + * When provided, this class will operate solely within memory, refusing to + * persist any information such as tokens. Default null (not provided). + */ + constructor(identityUrl = null) { this.accessToken = null; this.authEnabled = true; + + if (identityUrl) { + // XXX: We shouldn't have to create a whole new MatrixClient just to + // do identity server auth. The functions don't take an identity URL + // though, and making all of them take one could lead to developer + // confusion about what the idBaseUrl does on a client. Therefore, we + // just make a new client and live with it. + this.tempClient = Matrix.createClient({ + baseUrl: "", // invalid by design + idBaseUrl: identityUrl, + }); + } else { + // Indicates that we're using the real client, not some workaround. + this.tempClient = null; + } + } + + get _matrixClient() { + return this.tempClient ? this.tempClient : MatrixClientPeg.get(); + } + + _writeToken() { + if (this.tempClient) return; // temporary client: ignore + window.localStorage.setItem("mx_is_access_token", token); + } + + _readToken() { + if (this.tempClient) return null; // temporary client: ignore + return window.localStorage.getItem("mx_is_access_token"); } hasCredentials() { @@ -38,14 +73,14 @@ export default class IdentityAuthClient { let token = this.accessToken; if (!token) { - token = window.localStorage.getItem("mx_is_access_token"); + token = this._readToken(); } if (!token) { token = await this.registerForToken(); if (token) { this.accessToken = token; - window.localStorage.setItem("mx_is_access_token", token); + this._writeToken(); } return token; } @@ -61,7 +96,7 @@ export default class IdentityAuthClient { token = await this.registerForToken(); if (token) { this.accessToken = token; - window.localStorage.setItem("mx_is_access_token", token); + this._writeToken(); } } @@ -70,13 +105,13 @@ export default class IdentityAuthClient { async _checkToken(token) { try { - await MatrixClientPeg.get().getIdentityAccount(token); + await this._matrixClient.getIdentityAccount(token); } catch (e) { if (e.errcode === "M_TERMS_NOT_SIGNED") { console.log("Identity Server requires new terms to be agreed to"); await startTermsFlow([new Service( SERVICE_TYPES.IS, - MatrixClientPeg.get().idBaseUrl, + this._matrixClient.getIdentityServerUrl(), token, )]); return; @@ -95,7 +130,7 @@ export default class IdentityAuthClient { try { const hsOpenIdToken = await MatrixClientPeg.get().getOpenIdToken(); const { access_token: identityAccessToken } = - await MatrixClientPeg.get().registerWithIdentityServer(hsOpenIdToken); + await this._matrixClient.registerWithIdentityServer(hsOpenIdToken); await this._checkToken(identityAccessToken); return identityAccessToken; } catch (e) { diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index 398e578e8d..c4842d2ae9 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -22,6 +22,8 @@ import MatrixClientPeg from "../../../MatrixClientPeg"; import SdkConfig from "../../../SdkConfig"; import Modal from '../../../Modal'; import dis from "../../../dispatcher"; +import IdentityAuthClient from "../../../IdentityAuthClient"; +import {SERVICE_TYPES} from "matrix-js-sdk"; /** * If a url has no path component, etc. abbreviate it to just the hostname @@ -98,6 +100,7 @@ export default class SetIdServer extends React.Component { idServer: defaultIdServer, error: null, busy: false, + checking: false, }; } @@ -108,14 +111,14 @@ export default class SetIdServer extends React.Component { }; _getTooltip = () => { - if (this.state.busy) { + if (this.state.checking) { const InlineSpinner = sdk.getComponent('views.elements.InlineSpinner'); return
{ _t("Checking server") }
; } else if (this.state.error) { - return this.state.error; + return {this.state.error}; } else { return null; } @@ -125,25 +128,67 @@ export default class SetIdServer extends React.Component { return !!this.state.idServer && !this.state.busy; }; + _continueTerms = (fullUrl) => { + MatrixClientPeg.get().setIdentityServerUrl(fullUrl); + localStorage.removeItem("mx_is_access_token"); + localStorage.setItem("mx_is_url", fullUrl); + dis.dispatch({action: 'id_server_changed'}); + this.setState({idServer: '', busy: false, error: null}); + }; + _saveIdServer = async (e) => { e.preventDefault(); - this.setState({busy: true}); + this.setState({busy: true, checking: true, error: null}); const fullUrl = unabbreviateUrl(this.state.idServer); - const errStr = await checkIdentityServerUrl(fullUrl); + let errStr = await checkIdentityServerUrl(fullUrl); let newFormValue = this.state.idServer; if (!errStr) { - MatrixClientPeg.get().setIdentityServerUrl(fullUrl); - localStorage.removeItem("mx_is_access_token"); - localStorage.setItem("mx_is_url", fullUrl); - dis.dispatch({action: 'id_server_changed'}); - newFormValue = ''; + try { + this.setState({checking: false}); // clear tooltip + + // Test the identity server by trying to register with it. This + // may result in a terms of service prompt. + const authClient = new IdentityAuthClient(fullUrl); + await authClient.getAccessToken(); + + // 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) { + 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._continueTerms(fullUrl); + }, + }); + return; + } + + this._continueTerms(fullUrl); + } catch (e) { + console.error(e); + errStr = _t("Terms of service not accepted or the identity server is invalid."); + } } this.setState({ busy: false, + checking: false, error: errStr, currentClientIdServer: MatrixClientPeg.get().getIdentityServerUrl(), idServer: newFormValue, diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index b982644516..09337fc7fc 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -548,6 +548,10 @@ "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", + "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.", + "Terms of service not accepted or the identity server is invalid.": "Terms of service not accepted or the identity server is invalid.", "Disconnect Identity Server": "Disconnect Identity Server", "Disconnect from the identity server ?": "Disconnect from the identity server ?", "Disconnect": "Disconnect", @@ -562,7 +566,6 @@ "Terms of service not accepted or the integration manager is invalid.": "Terms of service not accepted or the integration manager is invalid.", "Integration manager has no terms of service": "Integration manager has no terms of service", "The integration manager you have chosen does not have any terms of service.": "The integration manager 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 using %(serverName)s to manage your bots, widgets, and sticker packs.": "You are currently using %(serverName)s to manage your bots, widgets, and sticker packs.", "Add which integration manager you want to manage your bots, widgets, and sticker packs.": "Add which integration manager you want to manage your bots, widgets, and sticker packs.", "Integration Manager": "Integration Manager", From 02f8b72533dbea1cbb0ed6cc4b80bb312b1f4f78 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 15 Aug 2019 16:08:18 -0600 Subject: [PATCH 2/5] tfw the linter finds bugs for you --- src/IdentityAuthClient.js | 2 +- src/components/views/settings/SetIdServer.js | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/IdentityAuthClient.js b/src/IdentityAuthClient.js index 12c0c5b147..39785ef063 100644 --- a/src/IdentityAuthClient.js +++ b/src/IdentityAuthClient.js @@ -52,7 +52,7 @@ export default class IdentityAuthClient { _writeToken() { if (this.tempClient) return; // temporary client: ignore - window.localStorage.setItem("mx_is_access_token", token); + window.localStorage.setItem("mx_is_access_token", this.accessToken); } _readToken() { diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index c4842d2ae9..beea3f878f 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -144,8 +144,6 @@ export default class SetIdServer extends React.Component { const fullUrl = unabbreviateUrl(this.state.idServer); let errStr = await checkIdentityServerUrl(fullUrl); - - let newFormValue = this.state.idServer; if (!errStr) { try { this.setState({checking: false}); // clear tooltip @@ -191,7 +189,7 @@ export default class SetIdServer extends React.Component { checking: false, error: errStr, currentClientIdServer: MatrixClientPeg.get().getIdentityServerUrl(), - idServer: newFormValue, + idServer: this.state.idServer, }); }; From de3c489fbaa5f01ea98e7439ee15d69ffa9bca67 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 16 Aug 2019 09:41:23 -0600 Subject: [PATCH 3/5] Fix i18n --- src/i18n/strings/en_EN.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 8e49de48d3..587aa9d594 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -548,12 +548,12 @@ "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", - "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 from the identity server ?": "Disconnect from the identity server ?", "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.", "Terms of service not accepted or the identity server is invalid.": "Terms of service not accepted or the identity server is invalid.", + "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 from the identity server ?": "Disconnect from the identity server ?", "Disconnect Identity Server": "Disconnect Identity Server", "Disconnect": "Disconnect", "Identity Server (%(server)s)": "Identity Server (%(server)s)", From 32abfbbfc60642e1b6cdb5308518a557a88a15e7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 19 Aug 2019 10:27:57 -0600 Subject: [PATCH 4/5] Rename functions --- src/components/views/settings/SetIdServer.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/views/settings/SetIdServer.js b/src/components/views/settings/SetIdServer.js index 096222f124..67ca11d04f 100644 --- a/src/components/views/settings/SetIdServer.js +++ b/src/components/views/settings/SetIdServer.js @@ -130,7 +130,7 @@ export default class SetIdServer extends React.Component { return !!this.state.idServer && !this.state.busy; }; - _continueTerms = (fullUrl) => { + _saveIdServer = (fullUrl) => { MatrixClientPeg.get().setIdentityServerUrl(fullUrl); localStorage.removeItem("mx_is_access_token"); localStorage.setItem("mx_is_url", fullUrl); @@ -138,7 +138,7 @@ export default class SetIdServer extends React.Component { this.setState({idServer: '', busy: false, error: null}); }; - _saveIdServer = async (e) => { + _checkIdServer = async (e) => { e.preventDefault(); this.setState({busy: true, checking: true, error: null}); @@ -174,13 +174,13 @@ export default class SetIdServer extends React.Component { button: _t("Continue"), onFinished: async (confirmed) => { if (!confirmed) return; - this._continueTerms(fullUrl); + this._saveIdServer(fullUrl); }, }); return; } - this._continueTerms(fullUrl); + this._saveIdServer(fullUrl); } catch (e) { console.error(e); errStr = _t("Terms of service not accepted or the identity server is invalid."); @@ -299,7 +299,7 @@ export default class SetIdServer extends React.Component { } return ( -
+ {sectionTitle} @@ -313,7 +313,7 @@ export default class SetIdServer extends React.Component { tooltipContent={this._getTooltip()} /> {_t("Change")} {discoSection} From 855bf3ad0dd96fcb5a734b7ac1696e862efc2bdc Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 19 Aug 2019 10:25:20 -0600 Subject: [PATCH 5/5] Import createClient instead --- src/IdentityAuthClient.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/IdentityAuthClient.js b/src/IdentityAuthClient.js index 39785ef063..d3b4d8a6de 100644 --- a/src/IdentityAuthClient.js +++ b/src/IdentityAuthClient.js @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import Matrix, { SERVICE_TYPES } from 'matrix-js-sdk'; +import { createClient, SERVICE_TYPES } from 'matrix-js-sdk'; import MatrixClientPeg from './MatrixClientPeg'; import { Service, startTermsFlow, TermsNotSignedError } from './Terms'; @@ -36,7 +36,7 @@ export default class IdentityAuthClient { // though, and making all of them take one could lead to developer // confusion about what the idBaseUrl does on a client. Therefore, we // just make a new client and live with it. - this.tempClient = Matrix.createClient({ + this.tempClient = createClient({ baseUrl: "", // invalid by design idBaseUrl: identityUrl, });