From 987ad0b0db9194dfbe4df00741f413f0baeac27e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 29 Nov 2018 15:05:53 -0700 Subject: [PATCH] Check if users exist before inviting them and communicate errors Fixes https://github.com/vector-im/riot-web/issues/3283 Fixes https://github.com/vector-im/riot-web/issues/3968 Fixes https://github.com/vector-im/riot-web/issues/4308 Fixes https://github.com/vector-im/riot-web/issues/1597 Fixes https://github.com/vector-im/riot-web/issues/6790 This does 3 things: * Makes the `MultiInviter` check for a user profile before attempting an invite. This is to prove the user exists. * Use the `MultiInviter` everywhere to avoid duplicating the logic. Although a couple places only invite one user, it is still worthwhile. * Communicate errors from the `MultiInviter` to the user in all cases. This is done through dialogs, where some existed previously but were not invoked. Specifically to the 403 error not working: What was happening was the `MultiInviter` loop was setting the `fatal` flag, but that didn't resolve the promise it stored. This caused a promise to always be open, therefore never hitting a dialog. --- src/RoomInvite.js | 65 +++++++++++++----------- src/SlashCommands.js | 10 +++- src/components/views/rooms/MemberInfo.js | 11 +++- src/i18n/strings/en_EN.json | 4 ++ src/utils/MultiInviter.js | 45 ++++++++++++---- 5 files changed, 90 insertions(+), 45 deletions(-) diff --git a/src/RoomInvite.js b/src/RoomInvite.js index a96d1b2f6b..32c521bb48 100644 --- a/src/RoomInvite.js +++ b/src/RoomInvite.js @@ -1,6 +1,6 @@ /* Copyright 2016 OpenMarket Ltd -Copyright 2017 New Vector Ltd +Copyright 2017, 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,6 +15,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import React from 'react'; import MatrixClientPeg from './MatrixClientPeg'; import MultiInviter from './utils/MultiInviter'; import Modal from './Modal'; @@ -25,18 +26,6 @@ import dis from './dispatcher'; import DMRoomMap from './utils/DMRoomMap'; import { _t } from './languageHandler'; -export function inviteToRoom(roomId, addr) { - const addrType = getAddressType(addr); - - if (addrType == 'email') { - return MatrixClientPeg.get().inviteByEmail(roomId, addr); - } else if (addrType == 'mx-user-id') { - return MatrixClientPeg.get().invite(roomId, addr); - } else { - throw new Error('Unsupported address'); - } -} - /** * Invites multiple addresses to a room * Simpler interface to utils/MultiInviter but with @@ -46,9 +35,9 @@ export function inviteToRoom(roomId, addr) { * @param {string[]} addrs Array of strings of addresses to invite. May be matrix IDs or 3pids. * @returns {Promise} Promise */ -export function inviteMultipleToRoom(roomId, addrs) { +function inviteMultipleToRoom(roomId, addrs) { const inviter = new MultiInviter(roomId); - return inviter.invite(addrs); + return inviter.invite(addrs).then(addrs => Promise.resolve({addrs, inviter})); } export function showStartChatInviteDialog() { @@ -129,8 +118,8 @@ function _onStartChatFinished(shouldInvite, addrs) { createRoom().then((roomId) => { room = MatrixClientPeg.get().getRoom(roomId); return inviteMultipleToRoom(roomId, addrTexts); - }).then((addrs) => { - return _showAnyInviteErrors(addrs, room); + }).then((result) => { + return _showAnyInviteErrors(result.addrs, room, result.inviter); }).catch((err) => { console.error(err.stack); const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); @@ -148,9 +137,9 @@ function _onRoomInviteFinished(roomId, shouldInvite, addrs) { const addrTexts = addrs.map((addr) => addr.address); // Invite new users to a room - inviteMultipleToRoom(roomId, addrTexts).then((addrs) => { + inviteMultipleToRoom(roomId, addrTexts).then((result) => { const room = MatrixClientPeg.get().getRoom(roomId); - return _showAnyInviteErrors(addrs, room); + return _showAnyInviteErrors(result.addrs, room, result.inviter); }).catch((err) => { console.error(err.stack); const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); @@ -169,22 +158,36 @@ function _isDmChat(addrTexts) { } } -function _showAnyInviteErrors(addrs, room) { +function _showAnyInviteErrors(addrs, room, inviter) { // Show user any errors - const errorList = []; - for (const addr of Object.keys(addrs)) { - if (addrs[addr] === "error") { - errorList.push(addr); + const failedUsers = Object.keys(addrs).filter(a => addrs[a] === 'error'); + if (failedUsers.length === 1 && inviter.fatal) { + // Just get the first message because there was a fatal problem on the first + // user. This usually means that no other users were attempted, making it + // pointless for us to list who failed exactly. + const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); + Modal.createTrackedDialog('Failed to invite users to the room', '', ErrorDialog, { + title: _t("Failed to invite users to the room:", {roomName: room.name}), + description: inviter.getErrorText(failedUsers[0]), + }); + } else { + const errorList = []; + for (const addr of failedUsers) { + if (addrs[addr] === "error") { + const reason = inviter.getErrorText(addr); + errorList.push(addr + ": " + reason); + } + } + + if (errorList.length > 0) { + const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); + Modal.createTrackedDialog('Failed to invite the following users to the room', '', ErrorDialog, { + title: _t("Failed to invite the following users to the %(roomName)s room:", {roomName: room.name}), + description: errorList.join(
), + }); } } - if (errorList.length > 0) { - const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog"); - Modal.createTrackedDialog('Failed to invite the following users to the room', '', ErrorDialog, { - title: _t("Failed to invite the following users to the %(roomName)s room:", {roomName: room.name}), - description: errorList.join(", "), - }); - } return addrs; } diff --git a/src/SlashCommands.js b/src/SlashCommands.js index 8a34ba7ab1..eca4075fe9 100644 --- a/src/SlashCommands.js +++ b/src/SlashCommands.js @@ -26,6 +26,7 @@ import Modal from './Modal'; import SettingsStore, {SettingLevel} from './settings/SettingsStore'; import {MATRIXTO_URL_PATTERN} from "./linkify-matrix"; import * as querystring from "querystring"; +import MultiInviter from './utils/MultiInviter'; class Command { @@ -142,7 +143,14 @@ export const CommandMap = { if (args) { const matches = args.match(/^(\S+)$/); if (matches) { - return success(MatrixClientPeg.get().invite(roomId, matches[1])); + // We use a MultiInviter to re-use the invite logic, even though + // we're only inviting one user. + const userId = matches[1]; + const inviter = new MultiInviter(roomId); + return success(inviter.invite([userId]).then(() => { + if (inviter.getCompletionState(userId) !== "invited") + throw new Error(inviter.getErrorText(userId)); + })); } } return reject(this.getUsage()); diff --git a/src/components/views/rooms/MemberInfo.js b/src/components/views/rooms/MemberInfo.js index 7ff52ecbb6..17b1311c4f 100644 --- a/src/components/views/rooms/MemberInfo.js +++ b/src/components/views/rooms/MemberInfo.js @@ -41,6 +41,7 @@ import withMatrixClient from '../../../wrappers/withMatrixClient'; import AccessibleButton from '../elements/AccessibleButton'; import RoomViewStore from '../../../stores/RoomViewStore'; import SdkConfig from '../../../SdkConfig'; +import MultiInviter from "../../../utils/MultiInviter"; module.exports = withMatrixClient(React.createClass({ displayName: 'MemberInfo', @@ -714,12 +715,18 @@ module.exports = withMatrixClient(React.createClass({ const roomId = member && member.roomId ? member.roomId : RoomViewStore.getRoomId(); const onInviteUserButton = async() => { try { - await cli.invite(roomId, member.userId); + // We use a MultiInviter to re-use the invite logic, even though + // we're only inviting one user. + const inviter = new MultiInviter(roomId); + await inviter.invite([member.userId]).then(() => { + if (inviter.getCompletionState(userId) !== "invited") + throw new Error(inviter.getErrorText(userId)); + }); } catch (err) { const ErrorDialog = sdk.getComponent('dialogs.ErrorDialog'); Modal.createTrackedDialog('Failed to invite', '', ErrorDialog, { title: _t('Failed to invite'), - description: ((err && err.message) ? err.message : "Operation failed"), + description: ((err && err.message) ? err.message : _t("Operation failed")), }); } }; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index f9980f5645..5d5a65cb63 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -106,6 +106,7 @@ "Failed to invite user": "Failed to invite user", "Operation failed": "Operation failed", "Failed to invite": "Failed to invite", + "Failed to invite users to the room:": "Failed to invite users to the room:", "Failed to invite the following users to the %(roomName)s room:": "Failed to invite the following users to the %(roomName)s room:", "You need to be logged in.": "You need to be logged in.", "You need to be able to invite users to do that.": "You need to be able to invite users to do that.", @@ -220,6 +221,9 @@ "Your browser does not support the required cryptography extensions": "Your browser does not support the required cryptography extensions", "Not a valid Riot keyfile": "Not a valid Riot keyfile", "Authentication check failed: incorrect password?": "Authentication check failed: incorrect password?", + "You do not have permission to invite people to this room.": "You do not have permission to invite people to this room.", + "User %(user_id)s does not exist": "User %(user_id)s does not exist", + "Unknown server error": "Unknown server error", "Sorry, your homeserver is too old to participate in this room.": "Sorry, your homeserver is too old to participate in this room.", "Please contact your homeserver administrator.": "Please contact your homeserver administrator.", "Failed to join room": "Failed to join room", diff --git a/src/utils/MultiInviter.js b/src/utils/MultiInviter.js index b3e7fc495a..ad10f28edf 100644 --- a/src/utils/MultiInviter.js +++ b/src/utils/MultiInviter.js @@ -1,6 +1,6 @@ /* Copyright 2016 OpenMarket Ltd -Copyright 2017 New Vector Ltd +Copyright 2017, 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,9 +17,9 @@ limitations under the License. import MatrixClientPeg from '../MatrixClientPeg'; import {getAddressType} from '../UserAddress'; -import {inviteToRoom} from '../RoomInvite'; import GroupStore from '../stores/GroupStore'; import Promise from 'bluebird'; +import {_t} from "../languageHandler"; /** * Invites multiple addresses to a room or group, handling rate limiting from the server @@ -49,7 +49,7 @@ export default class MultiInviter { * Invite users to this room. This may only be called once per * instance of the class. * - * @param {array} addresses Array of addresses to invite + * @param {array} addrs Array of addresses to invite * @returns {Promise} Resolved when all invitations in the queue are complete */ invite(addrs) { @@ -88,12 +88,30 @@ export default class MultiInviter { return this.errorTexts[addr]; } + async _inviteToRoom(roomId, addr) { + const addrType = getAddressType(addr); + + if (addrType === 'email') { + return MatrixClientPeg.get().inviteByEmail(roomId, addr); + } else if (addrType === 'mx-user-id') { + const profile = await MatrixClientPeg.get().getProfileInfo(addr); + if (!profile) { + return Promise.reject({errcode: "M_NOT_FOUND", error: "User does not have a profile."}); + } + + return MatrixClientPeg.get().invite(roomId, addr); + } else { + throw new Error('Unsupported address'); + } + } + + _inviteMore(nextIndex) { if (this._canceled) { return; } - if (nextIndex == this.addrs.length) { + if (nextIndex === this.addrs.length) { this.busy = false; this.deferred.resolve(this.completionStates); return; @@ -111,7 +129,7 @@ export default class MultiInviter { // don't re-invite (there's no way in the UI to do this, but // for sanity's sake) - if (this.completionStates[addr] == 'invited') { + if (this.completionStates[addr] === 'invited') { this._inviteMore(nextIndex + 1); return; } @@ -120,7 +138,7 @@ export default class MultiInviter { if (this.groupId !== null) { doInvite = GroupStore.inviteUserToGroup(this.groupId, addr); } else { - doInvite = inviteToRoom(this.roomId, addr); + doInvite = this._inviteToRoom(this.roomId, addr); } doInvite.then(() => { @@ -129,29 +147,34 @@ export default class MultiInviter { this.completionStates[addr] = 'invited'; this._inviteMore(nextIndex + 1); - }, (err) => { + }).catch((err) => { if (this._canceled) { return; } let errorText; let fatal = false; - if (err.errcode == 'M_FORBIDDEN') { + if (err.errcode === 'M_FORBIDDEN') { fatal = true; - errorText = 'You do not have permission to invite people to this room.'; - } else if (err.errcode == 'M_LIMIT_EXCEEDED') { + errorText = _t('You do not have permission to invite people to this room.'); + } else if (err.errcode === 'M_LIMIT_EXCEEDED') { // we're being throttled so wait a bit & try again setTimeout(() => { this._inviteMore(nextIndex); }, 5000); return; + } else if(err.errcode === "M_NOT_FOUND") { + errorText = _t("User %(user_id)s does not exist", {user_id: addr}); } else { - errorText = 'Unknown server error'; + errorText = _t('Unknown server error'); } this.completionStates[addr] = 'error'; this.errorTexts[addr] = errorText; this.busy = !fatal; + this.fatal = fatal; if (!fatal) { this._inviteMore(nextIndex + 1); + } else { + this.deferred.resolve(this.completionStates); } }); }