Skip to content

Commit

Permalink
fix: factor in the selfClient when it comes to the calculation of isM…
Browse files Browse the repository at this point in the history 
…LSVerified (#16030)

* fix: also take selfclient into account when updating and reading certificate info

* fix: give the user access to the localClient without connection to the ClientState

* feat: remove observable from current client.

* chore: remove unused code

* fix: tests and typescript errors
  • Loading branch information
@aweiss-dev
aweiss-dev authored Oct 19, 2023
1 parent 9ef573e commit 0630a9d
Show file tree
Hide file tree
Showing 17 changed files with 116 additions and 82 deletions.
31 changes: 16 additions & 15 deletions src/script/client/ClientRepository.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('ClientRepository', () => {
const client = new ClientEntity(false, null);
client.id = clientId;

testFactory.client_repository['clientState'].currentClient(client);
testFactory.client_repository['clientState'].currentClient = client;

const clients = [
{class: ClientClassification.DESKTOP, id: '706f64373b1bcf79'},
Expand Down Expand Up @@ -112,9 +112,9 @@ describe('ClientRepository', () => {
spyOn(clientService, 'getClientById').and.returnValue(Promise.resolve(clientPayloadServer));
spyOn(clientService, 'putClientCapabilities').and.returnValue(Promise.resolve());

return testFactory.client_repository.getValidLocalClient().then(clientObservable => {
expect(clientObservable).toBeDefined();
expect(clientObservable().id).toBe(clientId);
return testFactory.client_repository.getValidLocalClient().then(client => {
expect(client).toBeDefined();
expect(client.id).toBe(clientId);
});
});

Expand Down Expand Up @@ -168,7 +168,7 @@ describe('ClientRepository', () => {
beforeEach(() => {
(jasmine as any).getEnv().allowRespy(true);
spyOn(Runtime, 'isDesktopApp').and.returnValue(false);
testFactory.client_repository['clientState'].currentClient(undefined);
testFactory.client_repository['clientState'].currentClient = undefined;
});

it('returns true on Electron', () => {
Expand All @@ -179,7 +179,7 @@ describe('ClientRepository', () => {
type: ClientType.PERMANENT,
};
const clientEntity = ClientMapper.mapClient(clientPayload, true, null);
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
spyOn(Runtime, 'isDesktopApp').and.returnValue(true);
const isPermanent = testFactory.client_repository.isCurrentClientPermanent();

Expand All @@ -194,7 +194,7 @@ describe('ClientRepository', () => {
type: ClientType.TEMPORARY,
};
const clientEntity = ClientMapper.mapClient(clientPayload, true, null);
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
spyOn(Runtime, 'isDesktopApp').and.returnValue(true);
const isPermanent = testFactory.client_repository.isCurrentClientPermanent();

Expand All @@ -216,7 +216,7 @@ describe('ClientRepository', () => {
type: ClientType.PERMANENT,
};
const clientEntity = ClientMapper.mapClient(clientPayload, true, null);
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
const isPermanent = testFactory.client_repository.isCurrentClientPermanent();

expect(isPermanent).toBeTruthy();
Expand All @@ -230,7 +230,7 @@ describe('ClientRepository', () => {
type: ClientType.TEMPORARY,
};
const clientEntity = ClientMapper.mapClient(clientPayload, true, null);
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
const isPermanent = testFactory.client_repository.isCurrentClientPermanent();

expect(isPermanent).toBeFalsy();
Expand All @@ -244,12 +244,13 @@ describe('ClientRepository', () => {
});

describe('isCurrentClient', () => {
beforeEach(() => testFactory.client_repository['clientState'].currentClient(undefined));
//@ts-ignore
beforeEach(() => (testFactory.client_repository['clientState'].currentClient = undefined));

it('returns true if user ID and client ID match', () => {
const clientEntity = new ClientEntity(false, null);
clientEntity.id = clientId;
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
testFactory.client_repository.selfUser(new User(userId, null));
const result = testFactory.client_repository['isCurrentClient']({domain: '', id: userId}, clientId);

Expand All @@ -259,7 +260,7 @@ describe('ClientRepository', () => {
it('returns false if only the user ID matches', () => {
const clientEntity = new ClientEntity(false, null);
clientEntity.id = clientId;
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
const result = testFactory.client_repository['isCurrentClient']({domain: '', id: userId}, 'ABCDE');

expect(result).toBeFalsy();
Expand All @@ -268,7 +269,7 @@ describe('ClientRepository', () => {
it('returns false if only the client ID matches', () => {
const clientEntity = new ClientEntity(false, null);
clientEntity.id = clientId;
testFactory.client_repository['clientState'].currentClient(clientEntity);
testFactory.client_repository['clientState'].currentClient = clientEntity;
const result = testFactory.client_repository['isCurrentClient']({domain: '', id: 'ABCDE'}, clientId);

expect(result).toBeFalsy();
Expand All @@ -281,14 +282,14 @@ describe('ClientRepository', () => {
});

it('throws an error if client ID is not specified', () => {
testFactory.client_repository['clientState'].currentClient(new ClientEntity(false, null));
testFactory.client_repository['clientState'].currentClient = new ClientEntity(false, null);
const functionCall = () => testFactory.client_repository['isCurrentClient']({domain: '', id: userId}, undefined);

expect(functionCall).toThrow(ClientError);
});

it('throws an error if user ID is not specified', () => {
testFactory.client_repository['clientState'].currentClient(new ClientEntity(false, null));
testFactory.client_repository['clientState'].currentClient = new ClientEntity(false, null);
const functionCall = () => testFactory.client_repository['isCurrentClient'](undefined, clientId);

expect(functionCall).toThrow(ClientError);
Expand Down
35 changes: 19 additions & 16 deletions src/script/client/ClientRepository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ export class ClientRepository {
}

const currentClient = ClientMapper.mapClient(clientRecord, true, clientRecord.domain);
this.clientState.currentClient(currentClient);
return this.clientState.currentClient();
this.clientState.currentClient = currentClient;
return this.clientState.currentClient;
});
}

Expand Down Expand Up @@ -245,13 +245,16 @@ export class ClientRepository {
* Get and validate the local client.
* @returns Resolves with an observable containing the client if valid
*/
async getValidLocalClient(): Promise<ko.Observable<ClientEntity>> {
async getValidLocalClient(): Promise<ClientEntity> {
try {
const clientEntity = await this.getCurrentClientFromDb();
await this.getClientByIdFromBackend(clientEntity.id);
const currentClient = this.clientState.currentClient;

await this.clientService.putClientCapabilities(currentClient().id, {
if (!currentClient) {
throw new ClientError(ClientError.TYPE.CLIENT_NOT_SET, ClientError.MESSAGE.CLIENT_NOT_SET);
}
await this.clientService.putClientCapabilities(currentClient.id, {
capabilities: [ClientCapability.LEGAL_HOLD_IMPLICIT_CONSENT],
});

Expand All @@ -270,9 +273,9 @@ export class ClientRepository {
* Load current client type from amplify store.
* @returns Type of current client
*/
private loadCurrentClientType(): ClientType.PERMANENT | ClientType.TEMPORARY {
if (this.clientState.currentClient()) {
return this.clientState.currentClient().type;
private loadCurrentClientType(): ClientType.PERMANENT | ClientType.TEMPORARY | undefined {
if (this.clientState.currentClient) {
return this.clientState.currentClient.type;
}
const isPermanent = loadValue(StorageKey.AUTH.PERSIST);
const type = isPermanent ? ClientType.PERMANENT : ClientType.TEMPORARY;
Expand All @@ -299,8 +302,8 @@ export class ClientRepository {
}

logoutClient = async (): Promise<void> => {
if (this.clientState.currentClient()) {
if (this.clientState.isTemporaryClient()) {
if (this.clientState.currentClient) {
if (this.clientState.currentClient.isTemporary()) {
await this.deleteLocalTemporaryClient();
amplify.publish(WebAppEvents.LIFECYCLE.SIGN_OUT, SIGN_OUT_REASON.USER_REQUESTED, true);
} else {
Expand Down Expand Up @@ -387,10 +390,10 @@ export class ClientRepository {
* @returns Type of current client is permanent
*/
isCurrentClientPermanent(): boolean {
if (!this.clientState.currentClient()) {
if (!this.clientState.currentClient) {
throw new ClientError(ClientError.TYPE.CLIENT_NOT_SET, ClientError.MESSAGE.CLIENT_NOT_SET);
}
return Runtime.isDesktopApp() || this.clientState.currentClient().isPermanent();
return Runtime.isDesktopApp() || this.clientState.currentClient.isPermanent();
}

/**
Expand Down Expand Up @@ -450,7 +453,7 @@ export class ClientRepository {

delete clientsFromBackend[clientId];

if (this.clientState.currentClient() && this.isCurrentClient(userId, clientId)) {
if (this.clientState.currentClient && this.isCurrentClient(userId, clientId)) {
this.logger.warn(`Removing duplicate local self client`);
await this.removeClient(userId, clientId);
}
Expand All @@ -477,7 +480,7 @@ export class ClientRepository {
for (const clientId in clientsFromBackend) {
const clientPayload = clientsFromBackend[clientId];

if (this.clientState.currentClient() && this.isCurrentClient(userId, clientId)) {
if (this.clientState.currentClient && this.isCurrentClient(userId, clientId)) {
continue;
}

Expand Down Expand Up @@ -512,7 +515,7 @@ export class ClientRepository {
* @returns Is the client the current local client
*/
private isCurrentClient(userId: QualifiedId, clientId: string): boolean {
if (!this.clientState.currentClient()) {
if (!this.clientState.currentClient) {
throw new ClientError(ClientError.TYPE.CLIENT_NOT_SET, ClientError.MESSAGE.CLIENT_NOT_SET);
}
if (!userId) {
Expand All @@ -521,7 +524,7 @@ export class ClientRepository {
if (!clientId) {
throw new ClientError(ClientError.TYPE.NO_CLIENT_ID, ClientError.MESSAGE.NO_CLIENT_ID);
}
return matchQualifiedIds(userId, this.selfUser()) && clientId === this.clientState.currentClient().id;
return matchQualifiedIds(userId, this.selfUser()) && clientId === this.clientState.currentClient.id;
}

//##############################################################################
Expand Down Expand Up @@ -563,7 +566,7 @@ export class ClientRepository {
return;
}

const isCurrentClient = clientId === this.clientState.currentClient().id;
const isCurrentClient = clientId === this.clientState.currentClient.id;
if (isCurrentClient) {
// If the current client has been removed, we need to sign out
amplify.publish(WebAppEvents.LIFECYCLE.SIGN_OUT, SIGN_OUT_REASON.CLIENT_REMOVED, true);
Expand Down
8 changes: 2 additions & 6 deletions src/script/client/ClientState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ import {ClientEntity} from './ClientEntity';
@singleton()
export class ClientState {
clients: ko.PureComputed<ClientEntity[]>;
currentClient: ko.Observable<ClientEntity>;
isTemporaryClient: ko.PureComputed<boolean>;
currentClient: ClientEntity | undefined;

constructor() {
this.currentClient = ko.observable();
this.isTemporaryClient = ko.pureComputed(() => this.currentClient()?.isTemporary());
}
constructor() {}
}
22 changes: 13 additions & 9 deletions src/script/components/HistoryExport/HistoryExport.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,19 @@ const HistoryExport = ({switchContent, user, clientState = container.resolve(Cli
setNumberOfRecords(numberOfRecords);
setNumberOfProcessedRecords(0);

const archiveBlob = await backupRepository.generateHistory(
user,
clientState.currentClient().id,
onProgress,
password,
);

onSuccess(archiveBlob);
logger.log(`Completed export of '${numberOfRecords}' records from history`);
if (clientState.currentClient) {
const archiveBlob = await backupRepository.generateHistory(
user,
clientState.currentClient.id,
onProgress,
password,
);

onSuccess(archiveBlob);
logger.log(`Completed export of '${numberOfRecords}' records from history`);
} else {
throw new Error('No local client found');
}
} catch (error) {
onError(error as Error);
}
Expand Down
5 changes: 2 additions & 3 deletions src/script/components/userDevices/SelfFingerprint.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {container} from 'tsyringe';

import {WebAppEvents} from '@wireapp/webapp-events';

import {useKoSubscribableChildren} from 'Util/ComponentUtil';
import {t} from 'Util/LocalizerUtil';

import {DeviceCard} from './DeviceCard';
Expand All @@ -50,11 +49,11 @@ const SelfFingerprint: React.FC<SelfFingerprintProps> = ({
cryptographyRepository.getLocalFingerprint().then(setLocalFingerprint);
}, [cryptographyRepository]);

const {currentClient} = useKoSubscribableChildren(clientState, ['currentClient']);
const currentClient = clientState.currentClient;

return (
<div className={cx('participant-devices__header', {'participant-devices__header--padding': !noPadding})}>
<DeviceCard device={currentClient} />
{currentClient && <DeviceCard device={currentClient} />}
<div className="participant-devices__fingerprint">
<DeviceId deviceId={localFingerprint} />
</div>
Expand Down
33 changes: 30 additions & 3 deletions src/script/conversation/ConversationVerificationStateHandler/MLS/MLSStateHandler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {ConversationProtocol} from '@wireapp/api-client/lib/conversation/NewConv

import {ClientEntity} from 'src/script/client';
import {Conversation} from 'src/script/entity/Conversation';
import {User} from 'src/script/entity/User';
import {Core} from 'src/script/service/CoreSingleton';
import {createUuid} from 'Util/uuid';

Expand All @@ -39,19 +40,20 @@ describe('MLSConversationVerificationStateHandler', () => {
let mockCore: jest.Mocked<Core>;
const groupId = 'groupIdXYZ';
const clientEntityId = 'clientIdXYZ';
const conversation: Conversation = new Conversation(uuid, '', ConversationProtocol.MLS);
const selfClientEntityId = 'selfClientIdXYZ';
const clientEntity: ClientEntity = new ClientEntity(false, '', clientEntityId);
const selfClientEntity: ClientEntity = new ClientEntity(false, '', selfClientEntityId);
const conversation: Conversation = new Conversation(uuid, '', ConversationProtocol.MLS);

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

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

mockOnConversationVerificationStateChange = jest.fn();
// Mock the conversation state
mockConversationState = {
filteredConversations: () => [conversation],
} as unknown as jest.Mocked<ConversationState>;
Expand All @@ -66,6 +68,10 @@ describe('MLSConversationVerificationStateHandler', () => {
certificate: 'mockCertificate',
clientId: clientEntityId,
},
{
certificate: 'mockCertificate',
clientId: selfClientEntityId,
},
]),
},
},
Expand Down Expand Up @@ -177,5 +183,26 @@ describe('MLSConversationVerificationStateHandler', () => {

expect(clientEntity.meta.isMLSVerified?.()).toBe(true);
});

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

const user = new User();
user.isMe = true;
user.localClient = selfClientEntity;
conversation.getAllUserEntities = jest.fn().mockReturnValue([user]);

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

expect(selfClientEntity.meta.isMLSVerified?.()).toBe(false);

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

expect(selfClientEntity.meta.isMLSVerified?.()).toBe(true);
});
});
});
6 changes: 5 additions & 1 deletion src/script/conversation/ConversationVerificationStateHandler/MLS/MLSStateHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,11 @@ export class MLSConversationVerificationStateHandler {
const allClients: ClientEntity[] = [];
const allIdentities: WireIdentity[] = [];
userEntities.forEach(async userEntity => {
const devices = userEntity.devices();
let devices = userEntity.devices();
// Add the localClient to the devices array if it is the current user
if (userEntity.isMe && userEntity.localClient) {
devices = [...devices, userEntity.localClient];
}
const deviceUserPairs = devices
.map(device => ({
[device.id]: userEntity.qualifiedId,
Expand Down
2 changes: 1 addition & 1 deletion src/script/conversation/MessageRepository.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ async function buildMessageRepository(): Promise<[MessageRepository, MessageRepo
const userState = new UserState();
userState.self(selfUser);
const clientState = new ClientState();
clientState.currentClient(new ClientEntity(true, ''));
clientState.currentClient = new ClientEntity(true, '');
const core = new Account();

const conversationState = new ConversationState(userState);
Expand Down
2 changes: 1 addition & 1 deletion src/script/conversation/MessageRepository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ export class MessageRepository {

const injectOptimisticEvent = async () => {
if (!skipInjection) {
const senderId = this.clientState.currentClient().id;
const senderId = this.clientState.currentClient?.id;
const currentTimestamp = this.serverTimeHandler.toServerTimestamp();
const optimisticEvent = EventBuilder.buildMessageAdd(
conversation,
Expand Down
Loading

0 comments on commit 0630a9d

Please sign in to comment.