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

runfix: 1:1 conversation with user without key packages #17371

Merged
merged 5 commits into from
May 16, 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
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);
};
Comment on lines -1506 to -1512
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to save the state manually, there's a subscription that updates the db whenever the value changes. See https://github.com/wireapp/wire-webapp/blob/runfix/1to1-migration-no-keys/src/script/entity/Conversation.ts#L582


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
Loading