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.

Sign up for GitHub

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

feat: MLSStateHandler updates clientEntities directly #16008

Merged
merged 6 commits into from
Oct 16, 2023
Merged

feat: MLSStateHandler updates clientEntities directly #16008

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6446b2a
feat: MLSStateHandler updates clientEntities directly
aweiss-dev Oct 16, 2023
ac7dac3
Update package.json
aweiss-dev Oct 16, 2023
afd9a56
fix: yarn.lock
aweiss-dev Oct 16, 2023
9ca69fa
fix: push identity only if it is verified
aweiss-dev Oct 16, 2023
3f9be0c
fix: add mlsConversation check
aweiss-dev Oct 16, 2023
4345b18
Merge branch 'dev' into feat/update_devices
aweiss-dev Oct 16, 2023
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"@peculiar/x509": "1.9.5",
"@wireapp/avs": "9.4.18",
"@wireapp/commons": "5.2.1",
"@wireapp/core": "42.13.0",
"@wireapp/core": "42.14.0",
"@wireapp/lru-cache": "3.8.1",
"@wireapp/react-ui-kit": "9.9.10",
"@wireapp/store-engine-dexie": "2.1.6",
Expand Down
64 changes: 32 additions & 32 deletions server/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,32 @@ __metadata:
linkType: hard

Copy link
Member Author

Choose a reason for hiding this comment

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

I needed to fix yarn.lock issues.

Did that by clearing the locks and rebuilding them freshly. Thats why some packages got updates

"@babel/compat-data@npm:^7.22.9":
version: 7.22.20
resolution: "@babel/compat-data@npm:7.22.20"
checksum: efedd1d18878c10fde95e4d82b1236a9aba41395ef798cbb651f58dbf5632dbff475736c507b8d13d4c8f44809d41c0eb2ef0d694283af9ba5dd8339b6dab451
version: 7.23.2
resolution: "@babel/compat-data@npm:7.23.2"
checksum: d8dc27437d40907b271161d4c88ffe72ccecb034c730deb1960a417b59a14d7c5ebca8cd80dd458a01cd396a7a329eb48cddcc3791b5a84da33d7f278f7bec6a
languageName: node
linkType: hard

"@babel/core@npm:^7.11.6, @babel/core@npm:^7.12.3":
version: 7.23.0
resolution: "@babel/core@npm:7.23.0"
version: 7.23.2
resolution: "@babel/core@npm:7.23.2"
dependencies:
"@ampproject/remapping": ^2.2.0
"@babel/code-frame": ^7.22.13
"@babel/generator": ^7.23.0
"@babel/helper-compilation-targets": ^7.22.15
"@babel/helper-module-transforms": ^7.23.0
"@babel/helpers": ^7.23.0
"@babel/helpers": ^7.23.2
"@babel/parser": ^7.23.0
"@babel/template": ^7.22.15
"@babel/traverse": ^7.23.0
"@babel/traverse": ^7.23.2
"@babel/types": ^7.23.0
convert-source-map: ^2.0.0
debug: ^4.1.0
gensync: ^1.0.0-beta.2
json5: ^2.2.3
semver: ^6.3.1
checksum: cebd9b48dbc970a7548522f207f245c69567e5ea17ebb1a4e4de563823cf20a01177fe8d2fe19b6e1461361f92fa169fd0b29f8ee9d44eeec84842be1feee5f2
checksum: 003897718ded16f3b75632d63cd49486bf67ff206cc7ebd1a10d49e2456f8d45740910d5ec7e42e3faf0deec7a2e96b1a02e766d19a67a8309053f0d4e57c0fe
languageName: node
linkType: hard

Expand Down Expand Up @@ -176,14 +176,14 @@ __metadata:
languageName: node
linkType: hard

"@babel/helpers@npm:^7.23.0":
version: 7.23.1
resolution: "@babel/helpers@npm:7.23.1"
"@babel/helpers@npm:^7.23.2":
version: 7.23.2
resolution: "@babel/helpers@npm:7.23.2"
dependencies:
"@babel/template": ^7.22.15
"@babel/traverse": ^7.23.0
"@babel/traverse": ^7.23.2
"@babel/types": ^7.23.0
checksum: acfc345102045c24ea2a4d60e00dcf8220e215af3add4520e2167700661338e6a80bd56baf44bb764af05ec6621101c9afc315dc107e18c61fa6da8acbdbb893
checksum: aaf4828df75ec460eaa70e5c9f66e6dadc28dae3728ddb7f6c13187dbf38030e142194b83d81aa8a31bbc35a5529a5d7d3f3cf59d5d0b595f5dd7f9d8f1ced8e
languageName: node
linkType: hard

Expand Down Expand Up @@ -372,9 +372,9 @@ __metadata:
languageName: node
linkType: hard

"@babel/traverse@npm:^7.23.0":
version: 7.23.0
resolution: "@babel/traverse@npm:7.23.0"
"@babel/traverse@npm:^7.23.2":
version: 7.23.2
resolution: "@babel/traverse@npm:7.23.2"
dependencies:
"@babel/code-frame": ^7.22.13
"@babel/generator": ^7.23.0
Expand All @@ -386,7 +386,7 @@ __metadata:
"@babel/types": ^7.23.0
debug: ^4.1.0
globals: ^11.1.0
checksum: 0b17fae53269e1af2cd3edba00892bc2975ad5df9eea7b84815dab07dfec2928c451066d51bc65b4be61d8499e77db7e547ce69ef2a7b0eca3f96269cb43a0b0
checksum: 26a1eea0dde41ab99dde8b9773a013a0dc50324e5110a049f5d634e721ff08afffd54940b3974a20308d7952085ac769689369e9127dea655f868c0f6e1ab35d
languageName: node
linkType: hard

Expand Down Expand Up @@ -1069,11 +1069,11 @@ __metadata:
linkType: hard

"@types/node@npm:*":
version: 20.8.4
resolution: "@types/node@npm:20.8.4"
version: 20.8.6
resolution: "@types/node@npm:20.8.6"
dependencies:
undici-types: ~5.25.1
checksum: 2106b9ef9750297cac68249428d7067c4d22c26908854165b70a164e34e900f4c34bb9bf3887c9391206b500d3e87171d03b1846e25788925236a0354390d278
checksum: ccfb7ac482c5a96edeb239893c5c099f5257fcc2ed9ae62fefdfbc782b79e16dbc2af9a85b379665237bf759904b44ca2be68e75d239e0297882aad42f61905c
languageName: node
linkType: hard

Expand Down Expand Up @@ -1632,9 +1632,9 @@ __metadata:
linkType: hard

"caniuse-lite@npm:^1.0.30001541":
version: 1.0.30001547
resolution: "caniuse-lite@npm:1.0.30001547"
checksum: ec0fc2b46721887f6f4aca1f3902f03d9a1a07416d16a86b7cd4bfba60e7b6b03ab3969659d3ea0158cc2f298972c80215c06c9457eb15c649d7780e8f5e91a7
version: 1.0.30001549
resolution: "caniuse-lite@npm:1.0.30001549"
checksum: 7f2abeedc8cf8b92cc0613855d71b995ce436068c0bcdd798c5af7d297ccf9f52496b00181beda42d82d25079dd4b6e389c67486156d40d8854e5707a25cb054
languageName: node
linkType: hard

Expand Down Expand Up @@ -2074,9 +2074,9 @@ __metadata:
linkType: hard

"electron-to-chromium@npm:^1.4.535":
version: 1.4.549
resolution: "electron-to-chromium@npm:1.4.549"
checksum: 6aa2a71fa359642e8cc77c0bbf4ded752ff769405173fdb91f790ae3b29cc076eb98579622a87fda86d087ef6f18ca13fa05d229083756c546761b1258c3dd48
version: 1.4.554
resolution: "electron-to-chromium@npm:1.4.554"
checksum: cbac43c50b43777327f4a7bf149ee3088c1da8b91bbcd80f78d2cc77bc52763f6d0941574499239d9caefd3430d3093f865e5f1093371418f7d6b70301eeae9b
languageName: node
linkType: hard

Expand Down Expand Up @@ -2546,9 +2546,9 @@ __metadata:
linkType: hard

"function-bind@npm:^1.1.1":
version: 1.1.1
resolution: "function-bind@npm:1.1.1"
checksum: b32fbaebb3f8ec4969f033073b43f5c8befbb58f1a79e12f1d7490358150359ebd92f49e72ff0144f65f2c48ea2a605bff2d07965f548f6474fd8efd95bf361a
version: 1.1.2
resolution: "function-bind@npm:1.1.2"
checksum: 2b0ff4ce708d99715ad14a6d1f894e2a83242e4a52ccfcefaee5e40050562e5f6dafc1adbb4ce2d4ab47279a45dc736ab91ea5042d843c3c092820dfe032efb1
languageName: node
linkType: hard

Expand Down Expand Up @@ -4229,9 +4229,9 @@ __metadata:
linkType: hard

"object-inspect@npm:^1.9.0":
version: 1.12.3
resolution: "object-inspect@npm:1.12.3"
checksum: dabfd824d97a5f407e6d5d24810d888859f6be394d8b733a77442b277e0808860555176719c5905e765e3743a7cada6b8b0a3b85e5331c530fd418cc8ae991db
version: 1.13.0
resolution: "object-inspect@npm:1.13.0"
checksum: 21353e910a3079466cb44adca71d8bef15bd8b87e518eb68bb33d82c5c70b83193993edce432cc92268f7dd02c4a8ab338663a011844367d0bd0559f6dde1fed
languageName: node
linkType: hard

Expand Down
38 changes: 34 additions & 4 deletions src/script/conversation/ConversationVerificationStateHandler/MLS/MLSStateHandler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import {ConversationProtocol} from '@wireapp/api-client/lib/conversation/NewConversation';

import {ClientEntity} from 'src/script/client';
import {Conversation} from 'src/script/entity/Conversation';
import {Core} from 'src/script/service/CoreSingleton';
import {createUuid} from 'Util/uuid';
Expand All @@ -36,12 +37,20 @@ describe('MLSConversationVerificationStateHandler', () => {
let mockOnConversationVerificationStateChange: jest.Mock;
let mockConversationState: jest.Mocked<ConversationState>;
let mockCore: jest.Mocked<Core>;
const conversation: Conversation = new Conversation(uuid, '', ConversationProtocol.MLS);
const groupId = 'groupIdXYZ';
const clientEntityId = 'clientIdXYZ';
const conversation: Conversation = new Conversation(uuid, '', ConversationProtocol.MLS);
const clientEntity: ClientEntity = new ClientEntity(false, '', clientEntityId);

beforeEach(() => {
conversation.groupId = groupId;

conversation.getAllUserEntities = jest.fn().mockReturnValue([
{
devices: () => [clientEntity],
},
]);

mockOnConversationVerificationStateChange = jest.fn();
mockConversationState = {
filteredConversations: () => [conversation],
Expand All @@ -51,7 +60,14 @@ describe('MLSConversationVerificationStateHandler', () => {
mls: {
on: jest.fn(),
},
e2eIdentity: {},
e2eIdentity: {
getUserDeviceEntities: jest.fn().mockResolvedValue([
{
certificate: 'mockCertificate',
clientId: clientEntityId,
},
]),
},
},
} as unknown as jest.Mocked<Core>;

Expand Down Expand Up @@ -110,7 +126,7 @@ describe('MLSConversationVerificationStateHandler', () => {
epoch: 12345,
};

jest.spyOn(handler as any, 'getAllUserEntitiesInConversation').mockResolvedValue({
jest.spyOn(handler as any, 'updateUserDevices').mockResolvedValue({
isResultComplete: false,
identities: [],
qualifiedIds: [],
Expand All @@ -129,7 +145,7 @@ describe('MLSConversationVerificationStateHandler', () => {
epoch: 12345,
};

jest.spyOn(handler as any, 'getAllUserEntitiesInConversation').mockResolvedValue({
jest.spyOn(handler as any, 'updateUserDevices').mockResolvedValue({
isResultComplete: true,
identities: [
{
Expand All @@ -145,5 +161,19 @@ describe('MLSConversationVerificationStateHandler', () => {

expect((handler as any).verifyConversation).toHaveBeenCalled();
});

it('should update ClientEntity isMLSVerified observable', async () => {
const mockData = {
groupId,
epoch: 12345,
};

jest.spyOn(handler as any, 'isCertificateActiveAndValid').mockReturnValue(true);
jest.spyOn(handler as any, 'verifyConversation').mockImplementation(() => null);

await (handler as any).checkEpoch(mockData); // Calling the private method

expect(clientEntity.meta.isMLSVerified?.()).toBe(true);
});
});
});
57 changes: 34 additions & 23 deletions src/script/conversation/ConversationVerificationStateHandler/MLS/MLSStateHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@

import {X509Certificate} from '@peculiar/x509';
import {QualifiedId} from '@wireapp/api-client/lib/user';
import {WireIdentity} from '@wireapp/core/lib/messagingProtocols/mls';
import {container} from 'tsyringe';

import {ClientEntity} from 'src/script/client';
import {Conversation} from 'src/script/entity/Conversation';
import {VerificationMessageType} from 'src/script/message/VerificationMessageType';
import {Core} from 'src/script/service/CoreSingleton';
Expand Down Expand Up @@ -95,31 +97,41 @@ export class MLSConversationVerificationStateHandler {
* This function returns the WireIdentity of all userDeviceEntities in a conversation, as long as they have a certificate.
* If the conversation has userDeviceEntities without a certificate, it will not be included in the returned array
*
* It also updates the isMLSVerified observable of all the devices in the conversation
*/
private getAllUserEntitiesInConversation = async (conversation: Conversation) => {
if (!conversation.groupId) {
this.logger.error('Conversation has no groupId', conversation.name);
throw new Error('Conversation has no groupId');
}

private updateUserDevices = async (conversation: Conversation) => {
const userEntities = conversation.getAllUserEntities();

const deviceUserPairs = userEntities
.flatMap(userEntity => {
return userEntity.devices().map(device => ({[device.id]: userEntity.qualifiedId}));
})
.reduce((acc, current) => {
return {...acc, ...current};
}, {});

const identities = await this.core.service!.e2eIdentity!.getUserDeviceEntities(
conversation.groupId,
deviceUserPairs,
);
const allClients: ClientEntity[] = [];
const allIdentities: WireIdentity[] = [];
userEntities.forEach(async userEntity => {
const devices = userEntity.devices();
const deviceUserPairs = devices
.map(device => ({
[device.id]: userEntity.qualifiedId,
}))
.reduce((acc, current) => {
return {...acc, ...current};
}, {});
const identities = await this.core.service!.e2eIdentity!.getUserDeviceEntities(
conversation.groupId!,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is a isMLSConversation that will also make sure that this groupId is present ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

have a look at the ConversationSelector.ts

Copy link
Member Author

Choose a reason for hiding this comment

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

good point. Changed the types and added the guard

deviceUserPairs,
);
identities.forEach(identity => {
if (identity.certificate) {
const device = devices.find(device => device.id === identity.clientId);
/**
* ToDo: Change the current implementation of isMLSVerified to be stored in Zustand instead of ko.observable
*/
device?.meta.isMLSVerified?.(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

This meta is saved to the DB, right?

Copy link
Member Author

@aweiss-dev aweiss-dev Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it should. I added all the entries for that.
The DB entries get initialized with it, and it is also added in the devices toJSON method.

}
});
allIdentities.push(...identities);
allClients.push(...devices);
});

return {
identities,
isResultComplete: Object.keys(deviceUserPairs).length === identities.length,
identities: allIdentities,
isResultComplete: allClients.length === allIdentities.length,
qualifiedIds: userEntities.map(userEntity => userEntity.qualifiedId),
};
};
Expand All @@ -140,8 +152,7 @@ export class MLSConversationVerificationStateHandler {
return;
}

const {isResultComplete, identities, qualifiedIds} =
await this.getAllUserEntitiesInConversation(conversationEntity);
const {isResultComplete, identities, qualifiedIds} = await this.updateUserDevices(conversationEntity);

// If the number of userDevicePairs is not equal to the number of identities, our Conversation is not secure
if (!isResultComplete) {
Expand Down
Loading