From 7eaaab7be4923aa5db1be48ac627cb7cf678fc9b Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Sat, 10 Nov 2018 21:12:53 -0600 Subject: [PATCH 1/2] restrict self device id in message building, not in recipient data model --- .../src/Contacts/SignalRecipient.m | 30 ++----------------- .../src/Messages/OWSMessageSender.m | 17 +++++------ .../tests/Contacts/SignalRecipientTest.m | 14 ++++++++- 3 files changed, 22 insertions(+), 39 deletions(-) diff --git a/SignalServiceKit/src/Contacts/SignalRecipient.m b/SignalServiceKit/src/Contacts/SignalRecipient.m index 2e8853e3a..01d2225e0 100644 --- a/SignalServiceKit/src/Contacts/SignalRecipient.m +++ b/SignalServiceKit/src/Contacts/SignalRecipient.m @@ -71,23 +71,8 @@ NS_ASSUME_NONNULL_BEGIN if (!self) { return self; } - - OWSAssertDebug(self.tsAccountManager.localNumber.length > 0); - if ([self.tsAccountManager.localNumber isEqualToString:textSecureIdentifier]) { - // Default to no devices. - // - // This instance represents our own account and is used for sending - // sync message to linked devices. We shouldn't have any linked devices - // yet when we create the "self" SignalRecipient, and we don't need to - // send sync messages to the primary - we ARE the primary. - _devices = [NSOrderedSet new]; - } else { - // Default to sending to just primary device. - // - // OWSMessageSender will correct this if it is wrong the next time - // we send a message to this recipient. - _devices = [NSOrderedSet orderedSetWithObject:@(OWSDevicePrimaryDeviceId)]; - } + + _devices = [NSOrderedSet orderedSetWithObject:@(OWSDevicePrimaryDeviceId)]; return self; } @@ -103,11 +88,6 @@ NS_ASSUME_NONNULL_BEGIN _devices = [NSOrderedSet new]; } - if ([self.uniqueId isEqual:self.tsAccountManager.localNumber] && - [self.devices containsObject:@(OWSDevicePrimaryDeviceId)]) { - OWSFailDebug(@"self as recipient device"); - } - return self; } @@ -129,12 +109,6 @@ NS_ASSUME_NONNULL_BEGIN { OWSAssertDebug(devices.count > 0); - if ([self.uniqueId isEqual:self.tsAccountManager.localNumber] && - [devices containsObject:@(OWSDevicePrimaryDeviceId)]) { - OWSFailDebug(@"adding self as recipient device"); - return; - } - NSMutableOrderedSet *updatedDevices = [self.devices mutableCopy]; [updatedDevices unionSet:devices]; self.devices = [updatedDevices copy]; diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 8d715fa47..36fc8e8ed 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -819,8 +819,8 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; }]; } -- (nullable NSArray *)deviceMessagesForMessageSendSafe:(OWSMessageSend *)messageSend - error:(NSError **)errorHandle +- (nullable NSArray *)deviceMessagesForMessageSend:(OWSMessageSend *)messageSend + error:(NSError **)errorHandle { OWSAssertDebug(messageSend); OWSAssertDebug(errorHandle); @@ -830,7 +830,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSArray *deviceMessages; @try { - deviceMessages = [self throws_deviceMessagesForMessageSendUnsafe:messageSend]; + deviceMessages = [self throws_deviceMessagesForMessageSend:messageSend]; } @catch (NSException *exception) { if ([exception.name isEqualToString:UntrustedIdentityKeyException]) { // This *can* happen under normal usage, but it should happen relatively rarely. @@ -964,7 +964,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSError *deviceMessagesError; NSArray *_Nullable deviceMessages = - [self deviceMessagesForMessageSendSafe:messageSend error:&deviceMessagesError]; + [self deviceMessagesForMessageSend:messageSend error:&deviceMessagesError]; if (deviceMessagesError || !deviceMessages) { OWSAssertDebug(deviceMessagesError); return messageSend.failure(deviceMessagesError); @@ -1398,7 +1398,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [self sendMessageToRecipient:messageSend]; } -- (NSArray *)throws_deviceMessagesForMessageSendUnsafe:(OWSMessageSend *)messageSend +- (NSArray *)throws_deviceMessagesForMessageSend:(OWSMessageSend *)messageSend { OWSAssertDebug(messageSend.message); OWSAssertDebug(messageSend.recipient); @@ -1423,11 +1423,8 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSMutableArray *deviceIds = [recipient.devices mutableCopy]; OWSAssertDebug(deviceIds); - if (messageSend.isUDSend && messageSend.isLocalNumber) { - OWSLogVerbose(@"Adding device message for UD send to local device."); - OWSAssertDebug(![deviceIds containsObject:@(OWSDevicePrimaryDeviceId)]); - - [deviceIds addObject:@(OWSDevicePrimaryDeviceId)]; + if (messageSend.isLocalNumber) { + [deviceIds removeObject:@(OWSDevicePrimaryDeviceId)]; } for (NSNumber *deviceId in deviceIds) { diff --git a/SignalServiceKit/tests/Contacts/SignalRecipientTest.m b/SignalServiceKit/tests/Contacts/SignalRecipientTest.m index 630719613..ea92dab36 100644 --- a/SignalServiceKit/tests/Contacts/SignalRecipientTest.m +++ b/SignalServiceKit/tests/Contacts/SignalRecipientTest.m @@ -2,10 +2,10 @@ // Copyright (c) 2018 Open Whisper Systems. All rights reserved. // -#import "SignalRecipient.h" #import "MockSSKEnvironment.h" #import "OWSPrimaryStorage.h" #import "SSKBaseTestObjC.h" +#import "SignalRecipient.h" #import "TSAccountManager.h" #import "TestAppContext.h" #import @@ -49,4 +49,16 @@ }]; } +- (void)testRecipientWithExistingRecord +{ + // Sanity Check + XCTAssertNotNil(self.localNumber); + NSString *recipientId = @"+15551231234"; + [self readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [SignalRecipient markRecipientAsRegisteredAndGet:recipientId transaction:transaction]; + + XCTAssertTrue([SignalRecipient isRegisteredRecipient:recipientId transaction:transaction]); + }]; +} + @end From b25a704039eda01ab990f4b12d2bc5c577d763b2 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 12 Nov 2018 10:42:37 -0600 Subject: [PATCH 2/2] Fix: Compose thread picker doesn't show self --- SignalServiceKit/src/Contacts/SignalRecipient.m | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/SignalServiceKit/src/Contacts/SignalRecipient.m b/SignalServiceKit/src/Contacts/SignalRecipient.m index 01d2225e0..6f2c576c0 100644 --- a/SignalServiceKit/src/Contacts/SignalRecipient.m +++ b/SignalServiceKit/src/Contacts/SignalRecipient.m @@ -88,6 +88,15 @@ NS_ASSUME_NONNULL_BEGIN _devices = [NSOrderedSet new]; } + // Since we use device count to determine whether a user is registered or not, + // ensure the local user always has at least *this* device. + if (![_devices containsObject:@(OWSDevicePrimaryDeviceId)]) { + if ([self.uniqueId isEqualToString:self.tsAccountManager.localNumber]) { + DDLogInfo(@"Adding primary device to self recipient."); + [self addDevices:[NSSet setWithObject:@(OWSDevicePrimaryDeviceId)]]; + } + } + return self; }