From 987ad0b0db9194dfbe4df00741f413f0baeac27e Mon Sep 17 00:00:00 2001
From: Travis Ralston <travpc@gmail.com>
Date: Thu, 29 Nov 2018 15:05:53 -0700
Subject: [PATCH 1/3] 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(<br />),
+ });
}
}
- 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);
}
});
}
From 84b568a2358740b9d72cae33327adb0126b92fc1 Mon Sep 17 00:00:00 2001
From: Travis Ralston <travpc@gmail.com>
Date: Thu, 29 Nov 2018 15:16:45 -0700
Subject: [PATCH 2/3] Appease the linter
---
src/SlashCommands.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/SlashCommands.js b/src/SlashCommands.js
index eca4075fe9..24328d6372 100644
--- a/src/SlashCommands.js
+++ b/src/SlashCommands.js
@@ -148,8 +148,9 @@ export const CommandMap = {
const userId = matches[1];
const inviter = new MultiInviter(roomId);
return success(inviter.invite([userId]).then(() => {
- if (inviter.getCompletionState(userId) !== "invited")
+ if (inviter.getCompletionState(userId) !== "invited") {
throw new Error(inviter.getErrorText(userId));
+ }
}));
}
}
From 28f4752c5b1672ef124743f30c6a4ac22aa29c91 Mon Sep 17 00:00:00 2001
From: Travis Ralston <travpc@gmail.com>
Date: Wed, 5 Dec 2018 11:27:48 -0700
Subject: [PATCH 3/3] Rename returned completion states to be something
sensible
---
src/RoomInvite.js | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/RoomInvite.js b/src/RoomInvite.js
index 32c521bb48..3547b9195f 100644
--- a/src/RoomInvite.js
+++ b/src/RoomInvite.js
@@ -37,7 +37,7 @@ import { _t } from './languageHandler';
*/
function inviteMultipleToRoom(roomId, addrs) {
const inviter = new MultiInviter(roomId);
- return inviter.invite(addrs).then(addrs => Promise.resolve({addrs, inviter}));
+ return inviter.invite(addrs).then(states => Promise.resolve({states, inviter}));
}
export function showStartChatInviteDialog() {
@@ -119,7 +119,7 @@ function _onStartChatFinished(shouldInvite, addrs) {
room = MatrixClientPeg.get().getRoom(roomId);
return inviteMultipleToRoom(roomId, addrTexts);
}).then((result) => {
- return _showAnyInviteErrors(result.addrs, room, result.inviter);
+ return _showAnyInviteErrors(result.states, room, result.inviter);
}).catch((err) => {
console.error(err.stack);
const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
@@ -139,7 +139,7 @@ function _onRoomInviteFinished(roomId, shouldInvite, addrs) {
// Invite new users to a room
inviteMultipleToRoom(roomId, addrTexts).then((result) => {
const room = MatrixClientPeg.get().getRoom(roomId);
- return _showAnyInviteErrors(result.addrs, room, result.inviter);
+ return _showAnyInviteErrors(result.states, room, result.inviter);
}).catch((err) => {
console.error(err.stack);
const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");