From 74ea2adfc2b92da944a11eb029dba2b117ef272b Mon Sep 17 00:00:00 2001 From: Dariusz Niemczyk <3636685+Palid@users.noreply.github.com> Date: Tue, 4 Jan 2022 10:22:51 +0100 Subject: [PATCH] Fix room alias address isn't checked for validity before being shown as added (#7107) Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- .../views/elements/RoomAliasField.tsx | 93 +++++++++++++++---- .../views/room_settings/AliasSettings.tsx | 65 ++++++------- src/i18n/strings/en_EN.json | 3 + 3 files changed, 114 insertions(+), 47 deletions(-) diff --git a/src/components/views/elements/RoomAliasField.tsx b/src/components/views/elements/RoomAliasField.tsx index 9c383989b11..8e82f20a095 100644 --- a/src/components/views/elements/RoomAliasField.tsx +++ b/src/components/views/elements/RoomAliasField.tsx @@ -18,12 +18,12 @@ import React, { createRef, KeyboardEventHandler } from "react"; import { _t } from '../../../languageHandler'; import withValidation from './Validation'; -import { MatrixClientPeg } from '../../../MatrixClientPeg'; import { replaceableComponent } from "../../../utils/replaceableComponent"; import Field, { IValidateOpts } from "./Field"; +import MatrixClientContext from "../../../contexts/MatrixClientContext"; interface IProps { - domain: string; + domain?: string; value: string; label?: string; placeholder?: string; @@ -39,6 +39,9 @@ interface IState { // Controlled form component wrapping Field for inputting a room alias scoped to a given domain @replaceableComponent("views.elements.RoomAliasField") export default class RoomAliasField extends React.PureComponent { + static contextType = MatrixClientContext; + public context!: React.ContextType; + private fieldRef = createRef(); constructor(props, context) { @@ -50,25 +53,38 @@ export default class RoomAliasField extends React.PureComponent } private asFullAlias(localpart: string): string { - return `#${localpart}:${this.props.domain}`; + const hashAlias = `#${ localpart }`; + if (this.props.domain) { + return `${hashAlias}:${this.props.domain}`; + } + return hashAlias; + } + + private get domainProps() { + const { domain } = this.props; + const prefix = #; + const postfix = domain ? ({ `:${domain}` }) : ; + const maxlength = domain ? 255 - domain.length - 2 : 255 - 1; // 2 for # and : + const value = domain ? + this.props.value.substring(1, this.props.value.length - this.props.domain.length - 1) : + this.props.value.substring(1); + + return { prefix, postfix, value, maxlength }; } render() { - const poundSign = (#); - const aliasPostfix = ":" + this.props.domain; - const domain = ({ aliasPostfix }); - const maxlength = 255 - this.props.domain.length - 2; // 2 for # and : + const { prefix, postfix, value, maxlength } = this.domainProps; return ( private validationRules = withValidation({ rules: [ + { key: "hasDomain", + test: async ({ value }) => { + // Ignore if we have passed domain + if (!value || this.props.domain) { + return true; + } + + if (value.split(':').length < 2) { + return false; + } + return true; + }, + invalid: () => _t("Missing domain separator e.g. (:domain.org)"), + }, + { + key: "hasLocalpart", + test: async ({ value }) => { + if (!value || this.props.domain) { + return true; + } + + const split = value.split(':'); + if (split.length < 2) { + return true; // hasDomain check will fail here instead + } + + // Define the value invalid if there's no first part (roomname) + if (split[0].length < 1) { + return false; + } + return true; + }, + invalid: () => _t("Missing room name or separator e.g. (my-room:domain.org)"), + }, { key: "safeLocalpart", test: async ({ value }) => { if (!value) { return true; } - const fullAlias = this.asFullAlias(value); - // XXX: FIXME https://github.com/matrix-org/matrix-doc/issues/668 - return !value.includes("#") && !value.includes(":") && !value.includes(",") && - encodeURI(fullAlias) === fullAlias; + if (!this.props.domain) { + return true; + } else { + const fullAlias = this.asFullAlias(value); + const hasColon = this.props.domain ? !value.includes(":") : true; + // XXX: FIXME https://github.com/matrix-org/matrix-doc/issues/668 + // NOTE: We could probably use linkifyjs to parse those aliases here? + return !value.includes("#") && + hasColon && + !value.includes(",") && + encodeURI(fullAlias) === fullAlias; + } }, invalid: () => _t("Some characters not allowed"), }, { @@ -114,12 +172,13 @@ export default class RoomAliasField extends React.PureComponent if (!value) { return true; } - const client = MatrixClientPeg.get(); + const client = this.context; try { await client.getRoomIdForAlias(this.asFullAlias(value)); // we got a room id, so the alias is taken return false; } catch (err) { + console.log(err); // any server error code will do, // either it M_NOT_FOUND or the alias is invalid somehow, // in which case we don't want to show the invalid message @@ -127,7 +186,9 @@ export default class RoomAliasField extends React.PureComponent } }, valid: () => _t("This address is available to use"), - invalid: () => _t("This address is already in use"), + invalid: () => this.props.domain ? + _t("This address is already in use") : + _t("This address had invalid server or is already in use"), }, ], }); diff --git a/src/components/views/room_settings/AliasSettings.tsx b/src/components/views/room_settings/AliasSettings.tsx index f2de4d58ec2..9cf035e3bc3 100644 --- a/src/components/views/room_settings/AliasSettings.tsx +++ b/src/components/views/room_settings/AliasSettings.tsx @@ -14,12 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { ChangeEvent, createRef } from "react"; +import React, { ChangeEvent, ContextType, createRef } from "react"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { logger } from "matrix-js-sdk/src/logger"; import EditableItemList from "../elements/EditableItemList"; -import { MatrixClientPeg } from "../../../MatrixClientPeg"; import { _t } from '../../../languageHandler'; import Field from "../elements/Field"; import Spinner from "../elements/Spinner"; @@ -29,6 +28,7 @@ import Modal from "../../../Modal"; import RoomPublishSetting from "./RoomPublishSetting"; import { replaceableComponent } from "../../../utils/replaceableComponent"; import RoomAliasField from "../elements/RoomAliasField"; +import MatrixClientContext from "../../../contexts/MatrixClientContext"; import SettingsFieldset from "../settings/SettingsFieldset"; interface IEditableAliasesListProps { @@ -51,11 +51,6 @@ class EditableAliasesList extends EditableItemList { }; protected renderNewItemField() { - // if we don't need the RoomAliasField, - // we don't need to overriden version of renderNewItemField - if (!this.props.domain) { - return super.renderNewItemField(); - } const onChange = (alias) => this.onNewItemChanged({ target: { value: alias } }); return (
{ + public static contextType = MatrixClientContext; + context: ContextType; + static defaultProps = { canSetAliases: false, canSetCanonicalAlias: false, }; - constructor(props) { - super(props); + constructor(props, context: ContextType) { + super(props, context); const state = { altAliases: [], // [ #alias:domain.tld, ... ] @@ -138,10 +136,10 @@ export default class AliasSettings extends React.Component { private async loadLocalAliases() { this.setState({ localAliasesLoading: true }); try { - const cli = MatrixClientPeg.get(); + const mxClient = this.context; let localAliases = []; - if (await cli.doesServerSupportUnstableFeature("org.matrix.msc2432")) { - const response = await cli.unstableGetLocalAliases(this.props.roomId); + if (await mxClient.doesServerSupportUnstableFeature("org.matrix.msc2432")) { + const response = await mxClient.unstableGetLocalAliases(this.props.roomId); if (Array.isArray(response.aliases)) { localAliases = response.aliases; } @@ -171,7 +169,7 @@ export default class AliasSettings extends React.Component { if (alias) eventContent["alias"] = alias; - MatrixClientPeg.get().sendStateEvent(this.props.roomId, "m.room.canonical_alias", + this.context.sendStateEvent(this.props.roomId, "m.room.canonical_alias", eventContent, "").catch((err) => { logger.error(err); Modal.createTrackedDialog('Error updating main address', '', ErrorDialog, { @@ -192,7 +190,6 @@ export default class AliasSettings extends React.Component { this.setState({ updatingCanonicalAlias: true, - altAliases, }); const eventContent = {}; @@ -204,19 +201,25 @@ export default class AliasSettings extends React.Component { eventContent["alt_aliases"] = altAliases; } - MatrixClientPeg.get().sendStateEvent(this.props.roomId, "m.room.canonical_alias", - eventContent, "").catch((err) => { - logger.error(err); - Modal.createTrackedDialog('Error updating alternative addresses', '', ErrorDialog, { - title: _t("Error updating main address"), - description: _t( - "There was an error updating the room's alternative addresses. " + + this.context.sendStateEvent(this.props.roomId, "m.room.canonical_alias", eventContent, "") + .then(() => { + this.setState({ + altAliases, + }); + }) + .catch((err) => { + // TODO: Add error handling based upon server validation + logger.error(err); + Modal.createTrackedDialog('Error updating alternative addresses', '', ErrorDialog, { + title: _t("Error updating main address"), + description: _t( + "There was an error updating the room's alternative addresses. " + "It may not be allowed by the server or a temporary failure occurred.", - ), + ), + }); + }).finally(() => { + this.setState({ updatingCanonicalAlias: false }); }); - }).finally(() => { - this.setState({ updatingCanonicalAlias: false }); - }); } private onNewAliasChanged = (value: string) => { @@ -226,10 +229,10 @@ export default class AliasSettings extends React.Component { private onLocalAliasAdded = (alias: string) => { if (!alias || alias.length === 0) return; // ignore attempts to create blank aliases - const localDomain = MatrixClientPeg.get().getDomain(); + const localDomain = this.context.getDomain(); if (!alias.includes(':')) alias += ':' + localDomain; - MatrixClientPeg.get().createAlias(alias, this.props.roomId).then(() => { + this.context.createAlias(alias, this.props.roomId).then(() => { this.setState({ localAliases: this.state.localAliases.concat(alias), newAlias: null, @@ -253,7 +256,7 @@ export default class AliasSettings extends React.Component { const alias = this.state.localAliases[index]; // TODO: In future, we should probably be making sure that the alias actually belongs // to this room. See https://github.com/vector-im/element-web/issues/7353 - MatrixClientPeg.get().deleteAlias(alias).then(() => { + this.context.deleteAlias(alias).then(() => { const localAliases = this.state.localAliases.filter(a => a !== alias); this.setState({ localAliases }); @@ -322,9 +325,9 @@ export default class AliasSettings extends React.Component { } render() { - const cli = MatrixClientPeg.get(); - const localDomain = cli.getDomain(); - const isSpaceRoom = cli.getRoom(this.props.roomId)?.isSpaceRoom(); + const mxClient = this.context; + const localDomain = mxClient.getDomain(); + const isSpaceRoom = mxClient.getRoom(this.props.roomId)?.isSpaceRoom(); let found = false; const canonicalValue = this.state.canonicalAlias || ""; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 3f176fcf7b9..8b88bb40912 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2280,10 +2280,13 @@ "In reply to this message": "In reply to this message", "Room address": "Room address", "e.g. my-room": "e.g. my-room", + "Missing domain separator e.g. (:domain.org)": "Missing domain separator e.g. (:domain.org)", + "Missing room name or separator e.g. (my-room:domain.org)": "Missing room name or separator e.g. (my-room:domain.org)", "Some characters not allowed": "Some characters not allowed", "Please provide an address": "Please provide an address", "This address is available to use": "This address is available to use", "This address is already in use": "This address is already in use", + "This address had invalid server or is already in use": "This address had invalid server or is already in use", "Server Options": "Server Options", "You can use the custom server options to sign into other Matrix servers by specifying a different homeserver URL. This allows you to use Element with an existing Matrix account on a different homeserver.": "You can use the custom server options to sign into other Matrix servers by specifying a different homeserver URL. This allows you to use Element with an existing Matrix account on a different homeserver.", "Join millions for free on the largest public server": "Join millions for free on the largest public server",