Skip to content

Commit

Permalink
runfix: 1:1 conversation with user without key packages (#17371)
Browse files Browse the repository at this point in the history
* refactor: update conversaiton readonly state

* runfix: optionally do not allow unestablished mls 1:1

* feat: add a placeholder message for readonly 1:1 with a user without keys

* runfix: update copy

* test: readonly mls 1:1 no available keys
  • Loading branch information
PatrykBuniX committed May 16, 2024
1 parent 1588a84 commit 8dfc2cd
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 25 deletions.
1 change: 1 addition & 0 deletions src/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,7 @@
"ongoingGroupVideoCall": "Ongoing video conference call with {{conversationName}}, your camera is {{cameraStatus}}.",
"ongoingVideoCall": "Ongoing video call with {{conversationName}}, your camera is {{cameraStatus}}.",
"otherUserNotSupportMLSMsg": "You can't communicate with {{participantName}} anymore, as you two now use different protocols. When {{participantName}} gets an update, you can call and send messages and files again.",
"otherUserNoAvailableKeyPackages": "You can't communicate with {{participantName}} at the moment. When {{participantName}} logs in again, you can call and send messages and files again.",
"participantDevicesDetailHeadline": "Verify that this matches the fingerprint shown on [bold]{{user}}’s device[/bold].",
"participantDevicesDetailHowTo": "How do I do that?",
"participantDevicesDetailResetSession": "Reset session",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,14 @@ describe('ReadOnlyConversationMessage', () => {

expect(reloadAppMock).toHaveBeenCalled();
});

it("renders a conversation with a user that don't have any key pakages available", () => {
const conversation = generateConversation(CONVERSATION_READONLY_STATE.READONLY_ONE_TO_ONE_NO_KEY_PACKAGES, true);

const {getByText} = render(
withTheme(<ReadOnlyConversationMessage reloadApp={() => {}} conversation={conversation} />),
);

expect(getByText('otherUserNoAvailableKeyPackages')).toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ export const ReadOnlyConversationMessage: FC<ReadOnlyConversationMessageProps> =
</>
</ReadOnlyConversationMessageBase>
);
case CONVERSATION_READONLY_STATE.READONLY_ONE_TO_ONE_NO_KEY_PACKAGES:
return (
<ReadOnlyConversationMessageBase>
<span>
{replaceReactComponents(t('otherUserNoAvailableKeyPackages'), [
{
exactMatch: '{{participantName}}',
render: () => <strong>{user.name()}</strong>,
},
])}
</span>
</ReadOnlyConversationMessageBase>
);
}
}

Expand Down
50 changes: 50 additions & 0 deletions src/script/conversation/ConversationRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
} from '@wireapp/api-client/lib/event/';
import {BackendError, BackendErrorLabel} from '@wireapp/api-client/lib/http';
import {QualifiedId} from '@wireapp/api-client/lib/user';
import {ClientMLSError, ClientMLSErrorLabel} from '@wireapp/core/lib/messagingProtocols/mls';
import {amplify} from 'amplify';
import {StatusCodes as HTTP_STATUS} from 'http-status-codes';
import ko from 'knockout';
Expand Down Expand Up @@ -915,6 +916,55 @@ describe('ConversationRepository', () => {
);
});

it('marks mls 1:1 conversation as read-only if both users support mls but the other user has no keys available', async () => {
const conversationRepository = testFactory.conversation_repository!;
const userRepository = testFactory.user_repository!;

const otherUserId = {id: 'a718410c-3833-479d-bd80-a5df03f38414', domain: 'test-domain'};
const otherUser = new User(otherUserId.id, otherUserId.domain);
otherUser.supportedProtocols([ConversationProtocol.MLS]);
userRepository['userState'].users.push(otherUser);

const selfUserId = {id: '1a9da9ca-a495-47a8-ac70-9ffbe924b2d0', domain: 'test-domain'};
const selfUser = new User(selfUserId.id, selfUserId.domain);
selfUser.supportedProtocols([ConversationProtocol.MLS]);
jest.spyOn(conversationRepository['userState'], 'self').mockReturnValue(selfUser);

const mls1to1ConversationResponse = generateAPIConversation({
id: {id: '0aab891e-ccf1-4dba-9d74-bacec64b5b1e', domain: 'test-domain'},
type: CONVERSATION_TYPE.ONE_TO_ONE,
protocol: ConversationProtocol.MLS,
overwites: {group_id: 'groupId'},
}) as BackendMLSConversation;

const noKeysError = new ClientMLSError(ClientMLSErrorLabel.NO_KEY_PACKAGES_AVAILABLE);

jest
.spyOn(container.resolve(Core).service!.conversation, 'establishMLS1to1Conversation')
.mockRejectedValueOnce(noKeysError);

const [mls1to1Conversation] = conversationRepository.mapConversations([mls1to1ConversationResponse]);

const connection = new ConnectionEntity();
connection.conversationId = mls1to1Conversation.qualifiedId;
connection.userId = otherUserId;
otherUser.connection(connection);
mls1to1Conversation.connection(connection);

conversationRepository['conversationState'].conversations.push(mls1to1Conversation);

jest
.spyOn(conversationRepository['conversationService'], 'isMLSGroupEstablishedLocally')
.mockResolvedValueOnce(false);

const conversationEntity = await conversationRepository.getInitialised1To1Conversation(otherUser.qualifiedId);

expect(conversationEntity?.serialize()).toEqual(mls1to1Conversation.serialize());
expect(conversationEntity?.readOnlyState()).toEqual(
CONVERSATION_READONLY_STATE.READONLY_ONE_TO_ONE_NO_KEY_PACKAGES,
);
});

it('deos not mark mls 1:1 conversation as read-only if the other user does not support mls but mls 1:1 was already established', async () => {
const conversationRepository = testFactory.conversation_repository!;
const userRepository = testFactory.user_repository!;
Expand Down
62 changes: 38 additions & 24 deletions src/script/conversation/ConversationRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {BackendErrorLabel} from '@wireapp/api-client/lib/http/';
import type {BackendError} from '@wireapp/api-client/lib/http/';
import type {QualifiedId} from '@wireapp/api-client/lib/user/';
import {MLSCreateConversationResponse} from '@wireapp/core/lib/conversation';
import {ClientMLSError, ClientMLSErrorLabel} from '@wireapp/core/lib/messagingProtocols/mls';
import {amplify} from 'amplify';
import {StatusCodes as HTTP_STATUS} from 'http-status-codes';
import {container} from 'tsyringe';
Expand Down Expand Up @@ -167,11 +168,13 @@ type IncomingEvent = ConversationEvent | ClientConversationEvent;
export enum CONVERSATION_READONLY_STATE {
READONLY_ONE_TO_ONE_SELF_UNSUPPORTED_MLS = 'READONLY_ONE_TO_ONE_SELF_UNSUPPORTED_MLS',
READONLY_ONE_TO_ONE_OTHER_UNSUPPORTED_MLS = 'READONLY_ONE_TO_ONE_OTHER_UNSUPPORTED_MLS',
READONLY_ONE_TO_ONE_NO_KEY_PACKAGES = 'READONLY_ONE_TO_ONE_NO_KEY_PACKAGES',
}

interface GetInitialised1To1ConversationOptions {
isLiveUpdate?: boolean;
shouldRefreshUser?: boolean;
mls?: {allowUnestablished?: boolean};
}

export class ConversationRepository {
Expand Down Expand Up @@ -1325,7 +1328,11 @@ export class ConversationRepository {
* We have to add a delay to make sure the welcome message is not wasted, in case the self client would establish mls group themselves before receiving the welcome.
*/
const shouldDelayMLSGroupEstablishment = options.isLiveUpdate && isMLSSupportedByTheOtherUser;
return this.initMLS1to1Conversation(userId, isMLSSupportedByTheOtherUser, shouldDelayMLSGroupEstablishment);
return this.initMLS1to1Conversation(userId, {
isMLSSupportedByTheOtherUser,
shouldDelayGroupEstablishment: shouldDelayMLSGroupEstablishment,
allowUnestablished: options.mls?.allowUnestablished,
});
}

// There's no connection so it's a proteus conversation with a team member
Expand Down Expand Up @@ -1503,14 +1510,6 @@ export class ConversationRepository {
}
};

private readonly updateConversationReadOnlyState = async (
conversationEntity: Conversation,
conversationReadOnlyState: CONVERSATION_READONLY_STATE | null,
) => {
conversationEntity.readOnlyState(conversationReadOnlyState);
await this.saveConversationStateInDb(conversationEntity);
};

private readonly getProtocolFor1to1Conversation = async (
otherUserId: QualifiedId,
shouldRefreshUser = false,
Expand Down Expand Up @@ -1748,8 +1747,11 @@ export class ConversationRepository {
*/
private readonly initMLS1to1Conversation = async (
otherUserId: QualifiedId,
isMLSSupportedByTheOtherUser: boolean,
shouldDelayGroupEstablishment = false,
{
isMLSSupportedByTheOtherUser,
shouldDelayGroupEstablishment = false,
allowUnestablished = true,
}: {isMLSSupportedByTheOtherUser: boolean; shouldDelayGroupEstablishment?: boolean; allowUnestablished?: boolean},
): Promise<MLSConversation> => {
// When receiving some live updates via websocket, e.g. after connection request is accepted, both sides (users) of connection will react to conversation status update event.
// We want to reduce the possibility of two users trying to establish an MLS group at the same time.
Expand Down Expand Up @@ -1808,11 +1810,24 @@ export class ConversationRepository {
throw new Error('Self user is not available!');
}

const initialisedMLSConversation = await this.establishMLS1to1Conversation(mlsConversation, otherUserId);
let initialisedMLSConversation: MLSConversation = mlsConversation;

try {
initialisedMLSConversation = await this.establishMLS1to1Conversation(mlsConversation, otherUserId);
initialisedMLSConversation.readOnlyState(null);
} catch (error) {
this.logger.warn(`Failed to establish MLS 1:1 conversation with user ${otherUserId.id}`, error);
if (!allowUnestablished) {
throw error;
}

if (error instanceof ClientMLSError && error.label === ClientMLSErrorLabel.NO_KEY_PACKAGES_AVAILABLE) {
initialisedMLSConversation.readOnlyState(CONVERSATION_READONLY_STATE.READONLY_ONE_TO_ONE_NO_KEY_PACKAGES);
}
}

// If mls is supported by the other user, we can establish the group and remove readonly state from the conversation.
initialisedMLSConversation.readOnlyState(null);
await this.update1To1ConversationParticipants(mlsConversation, otherUserId);
await this.update1To1ConversationParticipants(initialisedMLSConversation, otherUserId);
await this.saveConversation(initialisedMLSConversation);

if (shouldOpenMLS1to1Conversation) {
Expand Down Expand Up @@ -1857,16 +1872,13 @@ export class ConversationRepository {
// If proteus is not supported by the other user we have to mark conversation as readonly
if (!doesOtherUserSupportProteus) {
await this.blacklistConversation(proteusConversationId);
await this.updateConversationReadOnlyState(
proteusConversation,
CONVERSATION_READONLY_STATE.READONLY_ONE_TO_ONE_SELF_UNSUPPORTED_MLS,
);
proteusConversation.readOnlyState(CONVERSATION_READONLY_STATE.READONLY_ONE_TO_ONE_SELF_UNSUPPORTED_MLS);
return proteusConversation;
}

// If proteus is supported by the other user, we just return a proteus conversation and remove readonly state from it.
await this.removeConversationFromBlacklist(proteusConversationId);
await this.updateConversationReadOnlyState(proteusConversation, null);
await proteusConversation.readOnlyState(null);
return proteusConversation;
};

Expand Down Expand Up @@ -1975,11 +1987,10 @@ export class ConversationRepository {
`Connection with user ${otherUserId.id} is accepted, using protocol ${protocol} for 1:1 conversation`,
);
if (protocol === ConversationProtocol.MLS || localMLSConversation) {
return this.initMLS1to1Conversation(
otherUserId,
return this.initMLS1to1Conversation(otherUserId, {
isMLSSupportedByTheOtherUser,
shouldDelayMLSGroupEstablishment,
);
shouldDelayGroupEstablishment: shouldDelayMLSGroupEstablishment,
});
}

if (protocol === ConversationProtocol.PROTEUS) {
Expand All @@ -1996,7 +2007,10 @@ export class ConversationRepository {
this.logger.log(
`Connection with user ${otherUserId.id} is not accepted, using already known MLS 1:1 conversation ${localMLSConversation.id}`,
);
return this.initMLS1to1Conversation(otherUserId, isMLSSupportedByTheOtherUser, shouldDelayMLSGroupEstablishment);
return this.initMLS1to1Conversation(otherUserId, {
isMLSSupportedByTheOtherUser,
shouldDelayGroupEstablishment: shouldDelayMLSGroupEstablishment,
});
}

this.logger.log(
Expand Down
5 changes: 4 additions & 1 deletion src/script/view_model/ActionsViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,10 @@ export class ActionsViewModel {
};

getOrCreate1to1Conversation = async (userEntity: User): Promise<Conversation> => {
const conversationEntity = await this.conversationRepository.getInitialised1To1Conversation(userEntity.qualifiedId);
const conversationEntity = await this.conversationRepository.getInitialised1To1Conversation(
userEntity.qualifiedId,
{mls: {allowUnestablished: false}},
);
if (conversationEntity) {
return conversationEntity;
}
Expand Down

0 comments on commit 8dfc2cd

Please sign in to comment.