From 27504e157863640e52b381e0da9406cafdac43bf Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 15 Aug 2019 13:28:23 -0600 Subject: [PATCH 1/3] Prompt for terms of service on integration manager changes Part of https://github.com/vector-im/riot-web/issues/10539 --- src/components/views/elements/Field.js | 12 +- .../views/settings/SetIntegrationManager.js | 113 ++++++++++++++---- src/i18n/strings/en_EN.json | 6 +- .../IntegrationManagerInstance.js | 9 +- src/integrations/IntegrationManagers.js | 19 +-- 5 files changed, 122 insertions(+), 37 deletions(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 8272b36639..43a0f8459c 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -46,6 +46,8 @@ export default class Field extends React.PureComponent { // and a `feedback` react component field to provide feedback // to the user. onValidate: PropTypes.func, + // If specified, overrides the value returned by onValidate. + flagInvalid: PropTypes.bool, // If specified, contents will appear as a tooltip on the element and // validation feedback tooltips will be suppressed. tooltipContent: PropTypes.node, @@ -137,7 +139,10 @@ export default class Field extends React.PureComponent { }, VALIDATION_THROTTLE_MS); render() { - const { element, prefix, onValidate, children, tooltipContent, ...inputProps } = this.props; + const { + element, prefix, onValidate, children, tooltipContent, + flagInvalid, ...inputProps, + } = this.props; const inputElement = element || "input"; @@ -157,13 +162,16 @@ export default class Field extends React.PureComponent { prefixContainer = {prefix}; } + const hasValidationFlag = flagInvalid != null && flagInvalid !== undefined; const fieldClasses = classNames("mx_Field", `mx_Field_${inputElement}`, { // If we have a prefix element, leave the label always at the top left and // don't animate it, as it looks a bit clunky and would add complexity to do // properly. mx_Field_labelAlwaysTopLeft: prefix, mx_Field_valid: onValidate && this.state.valid === true, - mx_Field_invalid: onValidate && this.state.valid === false, + mx_Field_invalid: hasValidationFlag + ? flagInvalid + : onValidate && this.state.valid === false, }); // Handle displaying feedback on validity diff --git a/src/components/views/settings/SetIntegrationManager.js b/src/components/views/settings/SetIntegrationManager.js index 8d26fdf40e..c6496beca7 100644 --- a/src/components/views/settings/SetIntegrationManager.js +++ b/src/components/views/settings/SetIntegrationManager.js @@ -19,6 +19,10 @@ import {_t} from "../../../languageHandler"; import sdk from '../../../index'; import Field from "../elements/Field"; import {IntegrationManagers} from "../../../integrations/IntegrationManagers"; +import MatrixClientPeg from "../../../MatrixClientPeg"; +import {SERVICE_TYPES} from "matrix-js-sdk"; +import {IntegrationManagerInstance} from "../../../integrations/IntegrationManagerInstance"; +import Modal from "../../../Modal"; export default class SetIntegrationManager extends React.Component { constructor() { @@ -31,6 +35,7 @@ export default class SetIntegrationManager extends React.Component { url: "", // user-entered text error: null, busy: false, + checking: false, }; } @@ -40,14 +45,14 @@ export default class SetIntegrationManager 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; } @@ -57,22 +62,7 @@ export default class SetIntegrationManager extends React.Component { return !!this.state.url && !this.state.busy; }; - _setManager = async (ev) => { - // Don't reload the page when the user hits enter in the form. - ev.preventDefault(); - ev.stopPropagation(); - - this.setState({busy: true}); - - const manager = await IntegrationManagers.sharedInstance().tryDiscoverManager(this.state.url); - if (!manager) { - this.setState({ - busy: false, - error: _t("Integration manager offline or not accessible."), - }); - return; - } - + _continueTerms = async (manager) => { try { await IntegrationManagers.sharedInstance().overwriteManagerOnAccount(manager); this.setState({ @@ -90,6 +80,85 @@ export default class SetIntegrationManager extends React.Component { } }; + _setManager = async (ev) => { + // Don't reload the page when the user hits enter in the form. + ev.preventDefault(); + ev.stopPropagation(); + + this.setState({busy: true, checking: true, error: null}); + + let offline = false; + let manager: IntegrationManagerInstance; + try { + manager = await IntegrationManagers.sharedInstance().tryDiscoverManager(this.state.url); + offline = !manager; // no manager implies offline + } catch (e) { + console.error(e); + offline = true; // probably a connection error + } + if (offline) { + this.setState({ + busy: false, + checking: false, + error: _t("Integration manager offline or not accessible."), + }); + return; + } + + // Test the manager (causes terms of service prompt if agreement is needed) + // We also cancel the tooltip at this point so it doesn't collide with the dialog. + this.setState({checking: false}); + try { + const client = manager.getScalarClient(); + await client.connect(); + } catch (e) { + console.error(e); + this.setState({ + busy: false, + error: _t("Terms of service not accepted or the integration manager is invalid."), + }); + return; + } + + // Specifically request the terms of service to see if there are any. + // The above won't trigger a terms of service check if there are no terms to + // sign, so when there's no terms at all we need to ensure we tell the user. + let hasTerms = true; + try { + const terms = await MatrixClientPeg.get().getTerms(SERVICE_TYPES.IM, manager.trimmedApiUrl); + hasTerms = terms && terms['policies'] && Object.keys(terms['policies']).length > 0; + } catch (e) { + // Assume errors mean there are no terms. This could be a 404, 500, etc + console.error(e); + hasTerms = false; + } + if (!hasTerms) { + this.setState({busy: false}); + const QuestionDialog = sdk.getComponent("views.dialogs.QuestionDialog"); + Modal.createTrackedDialog('No Terms Warning', '', QuestionDialog, { + title: _t("Integration manager has no terms of service"), + description: ( +
+ + {_t("The integration manager 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(manager); + }, + }); + return; + } + + this._continueTerms(manager); + }; + render() { const AccessibleButton = sdk.getComponent('views.elements.AccessibleButton'); @@ -120,11 +189,15 @@ export default class SetIntegrationManager extends React.Component { {bodyText} - %(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", diff --git a/src/integrations/IntegrationManagerInstance.js b/src/integrations/IntegrationManagerInstance.js index c21fff0fd3..6744d82e75 100644 --- a/src/integrations/IntegrationManagerInstance.js +++ b/src/integrations/IntegrationManagerInstance.js @@ -40,7 +40,14 @@ export class IntegrationManagerInstance { get name(): string { const parsed = url.parse(this.uiUrl); - return parsed.hostname; + return parsed.host; + } + + get trimmedApiUrl(): string { + const parsed = url.parse(this.apiUrl); + parsed.pathname = ''; + parsed.path = ''; + return parsed.format(); } getScalarClient(): ScalarAuthClient { diff --git a/src/integrations/IntegrationManagers.js b/src/integrations/IntegrationManagers.js index 49356676e6..0e19c7add0 100644 --- a/src/integrations/IntegrationManagers.js +++ b/src/integrations/IntegrationManagers.js @@ -117,7 +117,8 @@ export class IntegrationManagers { } /** - * Attempts to discover an integration manager using only its name. + * Attempts to discover an integration manager using only its name. This will not validate that + * the integration manager is functional - that is the caller's responsibility. * @param {string} domainName The domain name to look up. * @returns {Promise} Resolves to an integration manager instance, * or null if none was found. @@ -153,20 +154,12 @@ export class IntegrationManagers { // All discovered managers are per-user managers const manager = new IntegrationManagerInstance(KIND_ACCOUNT, widget["data"]["api_url"], widget["url"]); - console.log("Got integration manager response, checking for responsiveness"); + console.log("Got an integration manager (untested)"); - // Test the manager - const client = manager.getScalarClient(); - try { - // not throwing an error is a success here - await client.connect(); - } catch (e) { - console.error(e); - console.warn("Integration manager failed liveliness check"); - return null; - } + // We don't test the manager because the caller may need to do extra + // checks or similar with it. For instance, they may need to deal with + // terms of service or want to call something particular. - console.log("Integration manager is alive and functioning"); return manager; } } From e2f013ddb4d66ea09da078eba9e9ce3a60b791f4 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 15 Aug 2019 13:31:45 -0600 Subject: [PATCH 2/3] Appease the linter This looks awkward, but should pass. --- src/components/views/elements/Field.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 43a0f8459c..9c248a81d8 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -140,9 +140,8 @@ export default class Field extends React.PureComponent { render() { const { - element, prefix, onValidate, children, tooltipContent, - flagInvalid, ...inputProps, - } = this.props; + element, prefix, onValidate, children, tooltipContent, flagInvalid, + ...inputProps} = this.props; const inputElement = element || "input"; From 5fe691cf339c0c92c8d4258eb0cfea8b10834805 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 15 Aug 2019 13:33:02 -0600 Subject: [PATCH 3/3] Double equals --- src/components/views/elements/Field.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Field.js b/src/components/views/elements/Field.js index 9c248a81d8..2d9ef27edd 100644 --- a/src/components/views/elements/Field.js +++ b/src/components/views/elements/Field.js @@ -161,7 +161,7 @@ export default class Field extends React.PureComponent { prefixContainer = {prefix}; } - const hasValidationFlag = flagInvalid != null && flagInvalid !== undefined; + const hasValidationFlag = flagInvalid !== null && flagInvalid !== undefined; const fieldClasses = classNames("mx_Field", `mx_Field_${inputElement}`, { // If we have a prefix element, leave the label always at the top left and // don't animate it, as it looks a bit clunky and would add complexity to do