Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Stop processing team events (WPB-4538) #16471

Merged
merged 8 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/script/conversation/ConversationRepository.ts
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
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
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) {
thisisamir98 marked this conversation as resolved.
Show resolved Hide resolved
/*
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
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
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,
thisisamir98 marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
Expand Up @@ -8816,12 +8816,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is suspicious 🤔

resolution: "follow-redirects@npm:1.15.5"
peerDependenciesMeta:
debug:
optional: true
checksum: e178d1deff8b23d5d24ec3f7a94cde6e47d74d0dc649c35fc9857041267c12ec5d44650a0c5597ef83056ada9ea6ca0c30e7c4f97dbf07d035086be9e6a5b7b6
checksum: 5ca49b5ce6f44338cbfc3546823357e7a70813cecc9b7b768158a1d32c1e62e7407c944402a918ea8c38ae2e78266312d617dc68783fac502cbb55e1047b34ec
languageName: node
linkType: hard

Expand Down
Loading