Skip to content

Commit

Permalink
chore: Stop processing team events (WPB-4538) (#16471) (#16711)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
Co-authored-by: Thomas Belin <[email protected]>
  • Loading branch information
@otto-the-bot @thisisamir98 @atomrc
3 people authored Feb 1, 2024
1 parent 9db570a commit 3ff455c
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 68 deletions.
15 changes: 14 additions & 1 deletion src/script/conversation/ConversationRepository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down
79 changes: 76 additions & 3 deletions src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -37,9 +44,15 @@ function buildEventStorageMiddleware() {
replaceEvent: jest.fn(event => event),
deleteEvent: jest.fn(),
} as unknown as jest.Mocked<EventService>;

const conversationState = {
findConversation: jest.fn(),
} as unknown as jest.Mocked<ConversationState>;
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', () => {
Expand Down Expand Up @@ -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();
Expand Down
42 changes: 42 additions & 0 deletions src/script/event/preprocessor/EventStorageMiddleware/EventStorageMiddleware.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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) {
Expand Down Expand Up @@ -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');
}
Expand Down
61 changes: 0 additions & 61 deletions src/script/team/TeamRepository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/';
Expand Down Expand Up @@ -251,22 +248,10 @@ export class TeamRepository extends TypedEventEmitter<Events> {
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);
Expand Down Expand Up @@ -356,21 +341,6 @@ export class TeamRepository extends TypedEventEmitter<Events> {
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<void> => {
const shouldFetchConfig = handlingNotifications === NOTIFICATION_HANDLING_STATE.WEB_SOCKET;

Expand All @@ -397,28 +367,6 @@ export class TeamRepository extends TypedEventEmitter<Events> {
}
}

private async onMemberUpdate(eventJson: TeamMemberUpdateEvent): Promise<void> {
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));
}
Expand Down Expand Up @@ -447,15 +395,6 @@ export class TeamRepository extends TypedEventEmitter<Events> {
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;

Expand Down
22 changes: 22 additions & 0 deletions test/helper/EventGenerator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 3ff455c

Please sign in to comment.