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

Fix JSONDictionary of keys query responses #1707

Merged
merged 1 commit into from
Feb 2, 2023
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
6 changes: 6 additions & 0 deletions MatrixSDK.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1874,6 +1874,8 @@
ED51943A28462D130006EEC6 /* MXRoomStateUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED51943828462D130006EEC6 /* MXRoomStateUnitTests.swift */; };
ED51943C284630090006EEC6 /* MXRestClientStub.m in Sources */ = {isa = PBXBuildFile; fileRef = ED51943B284630090006EEC6 /* MXRestClientStub.m */; };
ED51943D284630090006EEC6 /* MXRestClientStub.m in Sources */ = {isa = PBXBuildFile; fileRef = ED51943B284630090006EEC6 /* MXRestClientStub.m */; };
ED555F59298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED555F58298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift */; };
ED555F5A298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED555F58298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift */; };
ED558068296F0361003443E3 /* MXCryptoMigrationStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED558067296F0361003443E3 /* MXCryptoMigrationStore.swift */; };
ED558069296F0361003443E3 /* MXCryptoMigrationStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED558067296F0361003443E3 /* MXCryptoMigrationStore.swift */; };
ED55806D296F0E3A003443E3 /* MXCryptoMigrationStoreUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED55806C296F0E3A003443E3 /* MXCryptoMigrationStoreUnitTests.swift */; };
Expand Down Expand Up @@ -3095,6 +3097,7 @@
ED51943828462D130006EEC6 /* MXRoomStateUnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXRoomStateUnitTests.swift; sourceTree = "<group>"; };
ED51943B284630090006EEC6 /* MXRestClientStub.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MXRestClientStub.m; sourceTree = "<group>"; };
ED51943E284630100006EEC6 /* MXRestClientStub.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MXRestClientStub.h; sourceTree = "<group>"; };
ED555F58298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXKeysQueryResponseUnitTests.swift; sourceTree = "<group>"; };
ED558067296F0361003443E3 /* MXCryptoMigrationStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMigrationStore.swift; sourceTree = "<group>"; };
ED55806C296F0E3A003443E3 /* MXCryptoMigrationStoreUnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMigrationStoreUnitTests.swift; sourceTree = "<group>"; };
ED55806F296F1BEE003443E3 /* MXCryptoMigrationV2Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MXCryptoMigrationV2Tests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -5782,6 +5785,7 @@
children = (
321809B819EEBF3000377451 /* MXEventTests.m */,
EDB4209827DF842F0036AF39 /* MXEventFixtures.swift */,
ED555F58298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift */,
);
path = JSONModels;
sourceTree = "<group>";
Expand Down Expand Up @@ -7369,6 +7373,7 @@
32B090FD26201C8D002924AA /* MXAsyncTaskQueueUnitTests.swift in Sources */,
322A51D81D9E846800C8536D /* MXCryptoTests.m in Sources */,
B146D4FF21A5C0BD00D8C2C6 /* MXMediaScanStoreUnitTests.m in Sources */,
ED555F59298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift in Sources */,
32BD34BE1E84134A006EDC0D /* MatrixSDKTestsE2EData.m in Sources */,
EDC8C40E2968C37F003792C5 /* MXKeysQuerySchedulerUnitTests.swift in Sources */,
ED751DAE28EDEC7E003748C3 /* MXKeyVerificationStateResolverUnitTests.swift in Sources */,
Expand Down Expand Up @@ -8035,6 +8040,7 @@
32B4778E2638133D00EA5800 /* MXFilterUnitTests.m in Sources */,
32B090FE26201C8D002924AA /* MXAsyncTaskQueueUnitTests.swift in Sources */,
B1E09A242397FCE90057C069 /* MXPeekingRoomTests.m in Sources */,
ED555F5A298BB27200C5BD63 /* MXKeysQueryResponseUnitTests.swift in Sources */,
B1E09A452397FD990057C069 /* MXLazyLoadingTests.m in Sources */,
EDC8C40D2968C37E003792C5 /* MXKeysQuerySchedulerUnitTests.swift in Sources */,
ED751DAF28EDEC7E003748C3 /* MXKeyVerificationStateResolverUnitTests.swift in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion MatrixSDK/Categories/MXRestClient+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public extension MXRestClient {
failure: @escaping (_ error: NSError?) -> Void) -> MXHTTPOperation {

// Do not chunk if not needed
if users.count < chunkSize {
if users.count <= chunkSize {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naughty naughty. Will write a test for this in a separate PR as it requires some integration test stubbing

return self.downloadKeys(forUsers: users, token: token) { response in
switch response {
case .success(let keysQueryResponse):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,16 @@ struct MXRoomEventEncryption: MXRoomEventEncrypting {
for: room,
historyVisibility: state.historyVisibility
)
log.debug("Collected \(users.count) eligible users")

let settings = try encryptionSettings(for: state)
try await handler.shareRoomKeysIfNecessary(
roomId: roomId,
users: users,
settings: settings
)

log.debug("Encryption and room keys up to date")
}

/// Make sure that we recognize (and store if necessary) the claimed room encryption algorithm
Expand Down
22 changes: 21 additions & 1 deletion MatrixSDK/Crypto/CryptoMachine/MXCryptoMachine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ extension MXCryptoMachine: MXCryptoSyncing {
}

func downloadKeysIfNecessary(users: [String]) async throws {
log.debug("Checking if keys need to be downloaded for \(users.count) user(s)")

try machine.updateTrackedUsers(users: users)

// Out-of-sync check if there is a pending outgoing request for some of these users
Expand Down Expand Up @@ -384,15 +386,22 @@ extension MXCryptoMachine: MXCryptoRoomEventEncrypting {
users: [String],
settings: EncryptionSettings
) async throws {
log.debug("Checking room keys in room \(roomId)")

try await sessionsQueue.sync { [weak self] in
try await self?.downloadKeysIfNecessary(users: users)
self?.log.debug("Fetching missing sessions")
try await self?.getMissingSessions(users: users)
}

log.debug("Acquiring room lock for room \(roomId)")
let roomQueue = await roomQueues.getQueue(for: roomId)

try await roomQueue.sync { [weak self] in
self?.log.debug("Sharing room keys")
try await self?.shareRoomKey(roomId: roomId, users: users, settings: settings)
}

log.debug("All room keys have been shared")
}

func encryptRoomEvent(
Expand Down Expand Up @@ -423,13 +432,22 @@ extension MXCryptoMachine: MXCryptoRoomEventEncrypting {
let request = try machine.getMissingSessions(users: users),
case .keysClaim = request
else {
log.debug("No sessions are missing")
return
}

log.debug("Claiming new keys")
try await handleRequest(request)
}

private func shareRoomKey(roomId: String, users: [String], settings: EncryptionSettings) async throws {
let requests = try machine.shareRoomKey(roomId: roomId, users: users, settings: settings)
guard !requests.isEmpty else {
log.debug("Room keys do not need to be shared")
return
}

log.debug("Sharing room keys via \(requests.count) request(s")
try await withThrowingTaskGroup(of: Void.self) { [weak self] group in
guard let self = self else { return }

Expand All @@ -445,6 +463,8 @@ extension MXCryptoMachine: MXCryptoRoomEventEncrypting {

try await group.waitForAll()
}

log.debug("All room keys have been shared")
}
}

Expand Down
4 changes: 2 additions & 2 deletions MatrixSDK/Crypto/Devices/Data/MXDeviceInfo.m
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ - (NSDictionary *)JSONDictionary
JSONDictionary[@"unsigned"] = _unsignedData;
}

return JSONDictionary;
return JSONDictionary.copy;
}

- (NSDictionary *)signalableJSONDictionary
Expand All @@ -165,7 +165,7 @@ - (NSDictionary *)signalableJSONDictionary
signalableJSONDictionary[@"keys"] = _keys;
}

return signalableJSONDictionary;
return signalableJSONDictionary.copy;
}


Expand Down
37 changes: 27 additions & 10 deletions MatrixSDK/JSONModels/MXJSONModels.m
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ - (NSDictionary *)JSONDictionary
if (_avatarUrl) { jsonDictionary[@"avatar_url"] = _avatarUrl; }
if (_roomTypeString) { jsonDictionary[@"room_type"] = _roomTypeString; }

return jsonDictionary;
return jsonDictionary.copy;
}

@end
Expand Down Expand Up @@ -1126,11 +1126,6 @@ - (NSDictionary *)JSONDictionary
@end

@interface MXKeysQueryResponse ()

/**
The original JSON used to create the response model
*/
@property (nonatomic, copy) NSDictionary *responseJSON;
@end

@implementation MXKeysQueryResponse
Expand All @@ -1140,8 +1135,6 @@ + (id)modelFromJSON:(NSDictionary *)JSONDictionary
MXKeysQueryResponse *keysQueryResponse = [[MXKeysQueryResponse alloc] init];
if (keysQueryResponse)
{
keysQueryResponse.responseJSON = JSONDictionary;

// Devices keys
NSMutableDictionary *map = [NSMutableDictionary dictionary];

Expand Down Expand Up @@ -1225,7 +1218,31 @@ + (id)modelFromJSON:(NSDictionary *)JSONDictionary

- (NSDictionary *)JSONDictionary
{
return self.responseJSON;
NSMutableDictionary *deviceKeys = [[NSMutableDictionary alloc] init];
for (NSString *userId in self.deviceKeys.userIds) {
NSMutableDictionary *devices = [[NSMutableDictionary alloc] init];
for (NSString *deviceId in [self.deviceKeys deviceIdsForUser:userId]) {
devices[deviceId] = [self.deviceKeys objectForDevice:deviceId forUser:userId].JSONDictionary.copy;
}
deviceKeys[userId] = devices.copy;
}

NSMutableDictionary *master = [[NSMutableDictionary alloc] init];
NSMutableDictionary *selfSigning = [[NSMutableDictionary alloc] init];
NSMutableDictionary *userSigning = [[NSMutableDictionary alloc] init];
for (NSString *userId in self.crossSigningKeys) {
master[userId] = self.crossSigningKeys[userId].masterKeys.JSONDictionary.copy;
selfSigning[userId] = self.crossSigningKeys[userId].selfSignedKeys.JSONDictionary.copy;
userSigning[userId] = self.crossSigningKeys[userId].userSignedKeys.JSONDictionary.copy;
}

return @{
@"device_keys": deviceKeys.copy ?: @{},
@"failures": self.failures.copy ?: @{},
@"master_keys": master.copy ?: @{},
@"self_signing_keys": selfSigning.copy ?: @{},
@"user_signing_keys": userSigning.copy ?: @{}
};
}

@end
Expand Down Expand Up @@ -2203,7 +2220,7 @@ - (NSDictionary *)JSONDictionary
{
dictionary[@"passphrase"] = self.passphrase;
}
return dictionary;
return dictionary.copy;
}

@end
Expand Down
Loading