From 3ff455cc9ae61e54f8befa4c427e8bfcbdbd82fd Mon Sep 17 00:00:00 2001 From: Otto the Bot Date: Thu, 1 Feb 2024 13:58:24 +0100 Subject: [PATCH] chore: Stop processing team events (WPB-4538) (#16471) (#16711) * chore: Stop processing team events (WPB-4538) * bump core * add User is not part of the conversation * add testing * handle pure MemberLeaveReason.USER_DELETED * Update src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.ts * chore: lint --------- Co-authored-by: Amir Ghezelbash Co-authored-by: Thomas Belin --- .../conversation/ConversationRepository.ts | 15 +++- .../EventStorageMiddleware.test.ts | 79 ++++++++++++++++++- .../EventStorageMiddleware.ts | 42 ++++++++++ src/script/team/TeamRepository.ts | 61 -------------- test/helper/EventGenerator.ts | 22 ++++++ yarn.lock | 6 +- 6 files changed, 157 insertions(+), 68 deletions(-) diff --git a/src/script/conversation/ConversationRepository.ts b/src/script/conversation/ConversationRepository.ts index c7f7dbed07f..7eee1f51dab 100644 --- a/src/script/conversation/ConversationRepository.ts +++ b/src/script/conversation/ConversationRepository.ts @@ -26,7 +26,7 @@ import { MessageSendingStatus, RemoteConversations, } from '@wireapp/api-client/lib/conversation'; -import {ConversationReceiptModeUpdateData} from '@wireapp/api-client/lib/conversation/data/'; +import {MemberLeaveReason, ConversationReceiptModeUpdateData} from '@wireapp/api-client/lib/conversation/data'; import {CONVERSATION_TYPING} from '@wireapp/api-client/lib/conversation/data/ConversationTypingData'; import { ConversationCreateEvent, @@ -3012,6 +3012,19 @@ export class ConversationRepository { return this.onMemberJoin(conversationEntity, eventJson); case CONVERSATION_EVENT.MEMBER_LEAVE: + if (eventJson.data.reason === MemberLeaveReason.USER_DELETED) { + eventJson.data.qualified_user_ids?.forEach(qualifiedUserId => { + const user = this.userState.users().find(user => matchQualifiedIds(user.qualifiedId, qualifiedUserId)); + if (!user?.teamId) { + return; + } + + void this.teamMemberLeave(user?.teamId, user?.qualifiedId, new Date(eventJson.time).getTime()); + }); + return; + } + return this.onMemberLeave(conversationEntity, eventJson); + case ClientEvent.CONVERSATION.TEAM_MEMBER_LEAVE: return this.onMemberLeave(conversationEntity, eventJson); diff --git a/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.test.ts b/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.test.ts index 710e443f411..a042642aa73 100644 --- a/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.test.ts +++ b/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.test.ts @@ -20,9 +20,16 @@ import {Asset as ProtobufAsset} from '@wireapp/protocol-messaging'; import {AssetTransferState} from 'src/script/assets/AssetTransferState'; +import {ConversationState} from 'src/script/conversation/ConversationState'; +import {Conversation} from 'src/script/entity/Conversation'; import {User} from 'src/script/entity/User'; import {EventError} from 'src/script/error/EventError'; -import {createAssetAddEvent, createMessageAddEvent, toSavedEvent} from 'test/helper/EventGenerator'; +import { + createAssetAddEvent, + createMemberLeaveEvent, + createMessageAddEvent, + toSavedEvent, +} from 'test/helper/EventGenerator'; import {createUuid} from 'Util/uuid'; import {EventStorageMiddleware} from './EventStorageMiddleware'; @@ -37,9 +44,15 @@ function buildEventStorageMiddleware() { replaceEvent: jest.fn(event => event), deleteEvent: jest.fn(), } as unknown as jest.Mocked; - + const conversationState = { + findConversation: jest.fn(), + } as unknown as jest.Mocked; const selfUser = new User(createUuid()); - return [new EventStorageMiddleware(eventService, selfUser), {eventService, selfUser}] as const; + + return [ + new EventStorageMiddleware(eventService, selfUser, conversationState), + {eventService, conversationState, selfUser}, + ] as const; } describe('EventStorageMiddleware', () => { @@ -74,6 +87,66 @@ describe('EventStorageMiddleware', () => { ); }); + it('fails for a member leave event when users are not part of the conversation', async () => { + const [eventStorageMiddleware, {conversationState}] = buildEventStorageMiddleware(); + const conversationId = createUuid(); + const userIds = [createUuid(), createUuid(), createUuid(), createUuid()]; + const conversation = new Conversation(conversationId, ''); + + conversationState.findConversation.mockImplementation(() => conversation); + + const event = createMemberLeaveEvent(conversationId, userIds); + + await expect(eventStorageMiddleware.processEvent(event)).rejects.toEqual( + new EventError( + EventError.TYPE.VALIDATION_FAILED, + 'Event validation failed: User is not part of the conversation', + ), + ); + }); + + it('fails for a member leave event when users are part of the conversation but are deleted already', async () => { + const [eventStorageMiddleware, {conversationState}] = buildEventStorageMiddleware(); + const conversationId = createUuid(); + const userIds = [createUuid(), createUuid(), createUuid()]; + const user1 = new User(userIds[0]); + const user2 = new User(userIds[1]); + const user3 = new User(userIds[2]); + user1.isDeleted = true; + user2.isDeleted = true; + user3.isDeleted = true; + const conversation = new Conversation(conversationId, ''); + conversation.participating_user_ets([user1, user2, user3]); + + conversationState.findConversation.mockImplementation(() => conversation); + + const event = createMemberLeaveEvent(conversationId, userIds); + + await expect(eventStorageMiddleware.processEvent(event)).rejects.toEqual( + new EventError( + EventError.TYPE.VALIDATION_FAILED, + 'Event validation failed: User is not part of the conversation', + ), + ); + }); + + it('does not return an error for a member leave event when users are part of the conversation', async () => { + const [eventStorageMiddleware, {conversationState}] = buildEventStorageMiddleware(); + const conversationId = createUuid(); + const userIds = [createUuid(), createUuid(), createUuid()]; + const user1 = new User(userIds[0]); + const user2 = new User(userIds[1]); + const user3 = new User(userIds[2]); + const conversation = new Conversation(conversationId, ''); + conversation.participating_user_ets([user1, user2, user3]); + + conversationState.findConversation.mockImplementation(() => conversation); + + const event = createMemberLeaveEvent(conversationId, userIds); + + await expect(eventStorageMiddleware.processEvent(event)).resolves.toEqual(event); + }); + it('fails for a non-"text message" with an ID previously used by the same user', async () => { const [eventStorageMiddleware, {eventService}] = buildEventStorageMiddleware(); const event = createMessageAddEvent(); diff --git a/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.ts b/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.ts index b31a3aa3eb2..56ebe0320bc 100644 --- a/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.ts +++ b/src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.ts @@ -17,7 +17,13 @@ * */ +import {CONVERSATION_EVENT} from '@wireapp/api-client/lib/event'; +import {container} from 'tsyringe'; + +import {ConversationState} from 'src/script/conversation/ConversationState'; import {User} from 'src/script/entity/User'; +import {UserFilter} from 'src/script/user/UserFilter'; +import {matchQualifiedIds} from 'Util/QualifiedId'; import {handleLinkPreviewEvent, handleEditEvent, handleAssetEvent, handleReactionEvent} from './eventHandlers'; import {EventValidationError} from './eventHandlers/EventValidationError'; @@ -34,6 +40,7 @@ export class EventStorageMiddleware implements EventMiddleware { constructor( private readonly eventService: EventService, private readonly selfUser: User, + private readonly conversationState: ConversationState = container.resolve(ConversationState), ) {} async processEvent(event: IncomingEvent) { @@ -73,9 +80,44 @@ export class EventStorageMiddleware implements EventMiddleware { } private validateEvent(event: HandledEvents, duplicateEvent?: EventRecord) { + if (event.type === CONVERSATION_EVENT.MEMBER_LEAVE) { + /* + When we receive a `member-leave` event, + we should check that the user is actually still part of the + conversation before forwarding the event. If the user is already not part + of the conversation, then we can throw a validation error + (that means the user was already removed by another member-leave event) + */ + if (!event.qualified_conversation) { + return; + } + const conversation = this.conversationState.findConversation(event.qualified_conversation); + + const qualifiedUserIds = event.data.qualified_user_ids; + + if (!conversation || !qualifiedUserIds) { + return; + } + + const usersNotPartofConversation = qualifiedUserIds.reduce((acc, qualifiedUserId) => { + const isDeleted = conversation + .getAllUserEntities() + .find(user => matchQualifiedIds(user.qualifiedId, qualifiedUserId))?.isDeleted; + + const isParticipant = UserFilter.isParticipant(conversation, qualifiedUserId); + + return acc || isDeleted || !isParticipant; + }, false); + + if (usersNotPartofConversation) { + throw new EventValidationError('User is not part of the conversation'); + } + } + if (!duplicateEvent) { return; } + if (duplicateEvent.from !== event.from) { throw new EventValidationError('ID previously used by another user'); } diff --git a/src/script/team/TeamRepository.ts b/src/script/team/TeamRepository.ts index eaf17acfdbe..ad87d929bc5 100644 --- a/src/script/team/TeamRepository.ts +++ b/src/script/team/TeamRepository.ts @@ -22,10 +22,7 @@ import type { TeamConversationDeleteEvent, TeamDeleteEvent, TeamEvent, - TeamMemberJoinEvent, TeamMemberLeaveEvent, - TeamMemberUpdateEvent, - TeamUpdateEvent, } from '@wireapp/api-client/lib/event'; import {TEAM_EVENT} from '@wireapp/api-client/lib/event/TeamEvent'; import {FeatureStatus, FeatureList} from '@wireapp/api-client/lib/team/feature/'; @@ -251,22 +248,10 @@ export class TeamRepository extends TypedEventEmitter { this.onDelete(eventJson); break; } - case TEAM_EVENT.MEMBER_JOIN: { - this._onMemberJoin(eventJson); - break; - } case TEAM_EVENT.MEMBER_LEAVE: { this.onMemberLeave(eventJson); break; } - case TEAM_EVENT.MEMBER_UPDATE: { - await this.onMemberUpdate(eventJson); - break; - } - case TEAM_EVENT.UPDATE: { - this.onUpdate(eventJson); - break; - } case TEAM_EVENT.CONVERSATION_CREATE: default: { this.onUnhandled(eventJson); @@ -356,21 +341,6 @@ export class TeamRepository extends TypedEventEmitter { amplify.publish(WebAppEvents.CONVERSATION.DELETE, {domain: '', id: conversationId}); } - private async _onMemberJoin(eventJson: TeamMemberJoinEvent) { - const { - data: {user: userId}, - team: teamId, - } = eventJson; - const isLocalTeam = this.teamState.team().id === teamId; - const isOtherUser = this.userState.self().id !== userId; - - if (isLocalTeam && isOtherUser) { - await this.userRepository.getUserById({domain: this.userState.self().domain, id: userId}); - const member = await this.getTeamMember(teamId, userId); - this.updateMemberRoles([member]); - } - } - private readonly updateTeamConfig = async (handlingNotifications: NOTIFICATION_HANDLING_STATE): Promise => { const shouldFetchConfig = handlingNotifications === NOTIFICATION_HANDLING_STATE.WEB_SOCKET; @@ -397,28 +367,6 @@ export class TeamRepository extends TypedEventEmitter { } } - private async onMemberUpdate(eventJson: TeamMemberUpdateEvent): Promise { - const { - data: {permissions, user: userId}, - team: teamId, - } = eventJson; - const isLocalTeam = this.teamState.team().id === teamId; - if (!isLocalTeam) { - return; - } - - const isSelfUser = this.userState.self().id === userId; - - if (isSelfUser) { - const memberEntity = permissions ? {permissions} : await this.getTeamMember(teamId, userId); - this.updateUserRole(this.userState.self(), memberEntity.permissions); - await this.sendAccountInfo(); - } else { - const member = await this.getTeamMember(teamId, userId); - this.updateMemberRoles([member]); - } - } - private updateUserRole(user: User, permissions: PermissionsData): void { user.teamRole(roleFromTeamPermissions(permissions)); } @@ -447,15 +395,6 @@ export class TeamRepository extends TypedEventEmitter { this.logger.log(`Received '${eventJson.type}' event from backend which is not yet handled`, eventJson); } - private onUpdate(eventJson: TeamUpdateEvent): void { - const {data: teamData, team: teamId} = eventJson; - - if (this.teamState.team().id === teamId) { - this.teamMapper.updateTeamFromObject(teamData, this.teamState.team()); - this.sendAccountInfo(); - } - } - public getTeamSupportedProtocols(): ConversationProtocol[] { const mlsFeature = this.teamState.teamFeatures()?.mls; diff --git a/test/helper/EventGenerator.ts b/test/helper/EventGenerator.ts index 496c0bd46a8..a9f7c46ea80 100644 --- a/test/helper/EventGenerator.ts +++ b/test/helper/EventGenerator.ts @@ -17,11 +17,15 @@ * */ +import {MemberLeaveReason} from '@wireapp/api-client/lib/conversation/data/'; +import {CONVERSATION_EVENT} from '@wireapp/api-client/lib/event'; + import {AssetTransferState} from 'src/script/assets/AssetTransferState'; import { AssetAddEvent, DeleteEvent, EventBuilder, + MemberLeaveEvent, MessageAddEvent, ReactionEvent, } from 'src/script/conversation/EventBuilder'; @@ -68,6 +72,24 @@ export function createReactionEvent(targetMessageId: string, reaction: string = }; } +export function createMemberLeaveEvent(conversationId: string, userIds: string[]): MemberLeaveEvent { + const conversationQualifiedId = {id: conversationId, domain: ''}; + + return { + conversation: conversationId, + qualified_conversation: conversationQualifiedId, + data: { + qualified_user_ids: userIds.map(userId => ({id: userId, domain: ''})), + reason: MemberLeaveReason.USER_DELETED, + user_ids: userIds, + }, + from: createUuid(), + id: createUuid(), + time: new Date().toISOString(), + type: CONVERSATION_EVENT.MEMBER_LEAVE, + }; +} + export function createDeleteEvent(deleteMessageId: string, conversationId: string = createUuid()): DeleteEvent { return { conversation: conversationId, diff --git a/yarn.lock b/yarn.lock index 42b16fcf5f7..e80cc6cef44 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9078,12 +9078,12 @@ __metadata: linkType: hard "follow-redirects@npm:^1.15.4": - version: 1.15.4 - resolution: "follow-redirects@npm:1.15.4" + version: 1.15.5 + resolution: "follow-redirects@npm:1.15.5" peerDependenciesMeta: debug: optional: true - checksum: e178d1deff8b23d5d24ec3f7a94cde6e47d74d0dc649c35fc9857041267c12ec5d44650a0c5597ef83056ada9ea6ca0c30e7c4f97dbf07d035086be9e6a5b7b6 + checksum: 5ca49b5ce6f44338cbfc3546823357e7a70813cecc9b7b768158a1d32c1e62e7407c944402a918ea8c38ae2e78266312d617dc68783fac502cbb55e1047b34ec languageName: node linkType: hard