From 07542b0c405f964f017b9805f81132ce8a79f8c6 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Thu, 5 May 2022 09:02:48 +0200 Subject: [PATCH] Fix inviting users with undisclosed profiles (#17269) Signed-off-by: Michael Weimann Signed-off-by: Michael Weimann --- src/utils/MultiInviter.ts | 17 +++- test/utils/MultiInviter-test.ts | 147 ++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 test/utils/MultiInviter-test.ts diff --git a/src/utils/MultiInviter.ts b/src/utils/MultiInviter.ts index 9916916f8c..cace1cb876 100644 --- a/src/utils/MultiInviter.ts +++ b/src/utils/MultiInviter.ts @@ -168,10 +168,19 @@ export default class MultiInviter { } if (!ignoreProfile && SettingsStore.getValue("promptBeforeInviteUnknownUsers", this.roomId)) { - const profile = await this.matrixClient.getProfileInfo(addr); - if (!profile) { - // noinspection ExceptionCaughtLocallyJS - throw new Error("User has no profile"); + try { + await this.matrixClient.getProfileInfo(addr); + } catch (err) { + // The error handling during the invitation process covers any API. + // Some errors must to me mapped from profile API errors to more specific ones to avoid collisions. + switch (err.errcode) { + case 'M_FORBIDDEN': + throw new MatrixError({ errcode: 'M_PROFILE_UNDISCLOSED' }); + case 'M_NOT_FOUND': + throw new MatrixError({ errcode: 'M_USER_NOT_FOUND' }); + default: + throw err; + } } } diff --git a/test/utils/MultiInviter-test.ts b/test/utils/MultiInviter-test.ts new file mode 100644 index 0000000000..1d5420aa8f --- /dev/null +++ b/test/utils/MultiInviter-test.ts @@ -0,0 +1,147 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { mocked } from 'jest-mock'; +import { MatrixClient } from 'matrix-js-sdk/src/matrix'; + +import { MatrixClientPeg } from '../../src/MatrixClientPeg'; +import Modal, { ModalManager } from '../../src/Modal'; +import SettingsStore from '../../src/settings/SettingsStore'; +import MultiInviter, { CompletionStates } from '../../src/utils/MultiInviter'; +import * as TestUtilsMatrix from '../test-utils'; + +const ROOMID = '!room:server'; + +const MXID1 = '@user1:server'; +const MXID2 = '@user2:server'; +const MXID3 = '@user3:server'; + +const MXID_PROFILE_STATES = { + [MXID1]: Promise.resolve({}), + [MXID2]: Promise.reject({ errcode: 'M_FORBIDDEN' }), + [MXID3]: Promise.reject({ errcode: 'M_NOT_FOUND' }), +}; + +jest.mock('../../src/Modal', () => ({ + createTrackedDialog: jest.fn(), +})); + +jest.mock('../../src/settings/SettingsStore', () => ({ + getValue: jest.fn(), +})); + +const mockPromptBeforeInviteUnknownUsers = (value: boolean) => { + mocked(SettingsStore.getValue).mockImplementation( + (settingName: string, roomId: string = null, _excludeDefault = false): any => { + if (settingName === 'promptBeforeInviteUnknownUsers' && roomId === ROOMID) { + return value; + } + }, + ); +}; + +const mockCreateTrackedDialog = (callbackName: 'onInviteAnyways'|'onGiveUp') => { + mocked(Modal.createTrackedDialog).mockImplementation( + ( + _analyticsAction: string, + _analyticsInfo: string, + ...rest: Parameters + ): any => { + rest[1][callbackName](); + }, + ); +}; + +const expectAllInvitedResult = (result: CompletionStates) => { + expect(result).toEqual({ + [MXID1]: 'invited', + [MXID2]: 'invited', + [MXID3]: 'invited', + }); +}; + +describe('MultiInviter', () => { + let client: jest.Mocked; + let inviter: MultiInviter; + + beforeEach(() => { + jest.resetAllMocks(); + + TestUtilsMatrix.stubClient(); + client = MatrixClientPeg.get() as jest.Mocked; + + client.invite = jest.fn(); + client.invite.mockResolvedValue({}); + + client.getProfileInfo = jest.fn(); + client.getProfileInfo.mockImplementation((userId: string) => { + return MXID_PROFILE_STATES[userId] || Promise.reject(); + }); + + inviter = new MultiInviter(ROOMID); + }); + + describe('invite', () => { + describe('with promptBeforeInviteUnknownUsers = false', () => { + beforeEach(() => mockPromptBeforeInviteUnknownUsers(false)); + + it('should invite all users', async () => { + const result = await inviter.invite([MXID1, MXID2, MXID3]); + + expect(client.invite).toHaveBeenCalledTimes(3); + expect(client.invite).toHaveBeenNthCalledWith(1, ROOMID, MXID1, undefined, undefined); + expect(client.invite).toHaveBeenNthCalledWith(2, ROOMID, MXID2, undefined, undefined); + expect(client.invite).toHaveBeenNthCalledWith(3, ROOMID, MXID3, undefined, undefined); + + expectAllInvitedResult(result); + }); + }); + + describe('with promptBeforeInviteUnknownUsers = true and', () => { + beforeEach(() => mockPromptBeforeInviteUnknownUsers(true)); + + describe('confirming the unknown user dialog', () => { + beforeEach(() => mockCreateTrackedDialog('onInviteAnyways')); + + it('should invite all users', async () => { + const result = await inviter.invite([MXID1, MXID2, MXID3]); + + expect(client.invite).toHaveBeenCalledTimes(3); + expect(client.invite).toHaveBeenNthCalledWith(1, ROOMID, MXID1, undefined, undefined); + expect(client.invite).toHaveBeenNthCalledWith(2, ROOMID, MXID2, undefined, undefined); + expect(client.invite).toHaveBeenNthCalledWith(3, ROOMID, MXID3, undefined, undefined); + + expectAllInvitedResult(result); + }); + }); + + describe('declining the unknown user dialog', () => { + beforeEach(() => mockCreateTrackedDialog('onGiveUp')); + + it('should only invite existing users', async () => { + const result = await inviter.invite([MXID1, MXID2, MXID3]); + + expect(client.invite).toHaveBeenCalledTimes(1); + expect(client.invite).toHaveBeenNthCalledWith(1, ROOMID, MXID1, undefined, undefined); + + // The resolved state is 'invited' for all users. + // With the above client expectations, the test ensures that only the first user is invited. + expectAllInvitedResult(result); + }); + }); + }); + }); +});