Fix friend request bugs & improve documentation

We were attaching the user's profile picture URL to friend request messages, and there was a bug in multi device friend request acceptance
pull/166/head
nielsandriesse 4 years ago
parent 8b50cca7bd
commit d3125c5039

@ -23,10 +23,11 @@ def shared_pods
# pod 'Curve25519Kit', path: '../Curve25519Kit', testspecs: ["Tests"]
# Don't update SignalMetadataKit. There's some Loki specific stuff in there that gets overwritten otherwise.
# FIXME: We should fork this, make it work with Cocoapods, and keep it up to date with Signal's repo.
# pod 'SignalMetadataKit', git: 'https://github.com/signalapp/SignalMetadataKit', testspecs: ["Tests"]
# pod 'SignalMetadataKit', path: '../SignalMetadataKit', testspecs: ["Tests"]
pod 'SignalServiceKit', path: '.', testspecs: ["Tests"]
pod 'SignalServiceKit', path: '.', testspecs: ["Tests"] # TODO: Signal moved this into the main repo. We should probably do the same eventually.
# Project does not compile with PromiseKit 6.7.1
# see: https://github.com/mxcl/PromiseKit/issues/990
@ -57,7 +58,7 @@ def shared_pods
###
# third party pods
####
###
pod 'AFNetworking', inhibit_warnings: true
pod 'PureLayout', :inhibit_warnings => true
@ -70,8 +71,11 @@ target 'Signal' do
shared_pods
pod 'SSZipArchive', :inhibit_warnings => true
# Loki
pod 'GCDWebServer', '~> 3.0', :inhibit_warnings => true
###
# Loki third party pods
###
pod 'GCDWebServer', '~> 3.0', :inhibit_warnings => true # TODO: We can probably ditch this as we're not doing P2P anymore
pod 'FeedKit', '~> 8.1', :inhibit_warnings => true
pod 'CryptoSwift', '~> 1.0', :inhibit_warnings => true
pod 'FirebaseCore', '~> 6.0', :inhibit_warnings => true # Used for internal testing
@ -91,8 +95,13 @@ end
target 'LokiPushNotificationService' do
project 'Signal'
pod 'CryptoSwift', '~> 1.0', :inhibit_warnings => true
shared_pods
###
# Loki third party pods
###
pod 'CryptoSwift', '~> 1.0', :inhibit_warnings => true
end
target 'SignalMessaging' do

@ -32,7 +32,7 @@ public class RefreshPreKeysOperation: OWSOperation {
Logger.debug("")
guard tsAccountManager.isRegistered() else {
Logger.debug("skipping - not registered")
Logger.debug("Skipping pre key refresh; user isn't registered.")
return
}
@ -88,18 +88,18 @@ public class RefreshPreKeysOperation: OWSOperation {
switch error {
case let networkManagerError as NetworkManagerError:
guard !networkManagerError.isNetworkError else {
Logger.debug("don't report SPK rotation failure w/ network error")
Logger.debug("Don't report SPK rotation failure w/ network error")
return
}
guard networkManagerError.statusCode >= 400 && networkManagerError.statusCode <= 599 else {
Logger.debug("don't report SPK rotation failure w/ non application error")
Logger.debug("Don't report SPK rotation failure w/ non application error")
return
}
TSPreKeyManager.incrementPreKeyUpdateFailureCount()
default:
Logger.debug("don't report SPK rotation failure w/ non NetworkManager error: \(error)")
Logger.debug("Don't report SPK rotation failure w/ non NetworkManager error: \(error)")
}
}
}

@ -198,13 +198,8 @@ static const NSUInteger kMaxPrekeyUpdateFailureCount = 5;
+ (void)checkPreKeys
{
if (!CurrentAppContext().isMainApp) {
return;
}
if (!self.tsAccountManager.isRegisteredAndReady) {
return;
}
if (!CurrentAppContext().isMainApp) { return; }
if (!self.tsAccountManager.isRegisteredAndReady) { return; }
SSKRefreshPreKeysOperation *operation = [SSKRefreshPreKeysOperation new];
[self.operationQueue addOperation:operation];
}

@ -19,6 +19,11 @@ This document outlines the main abstractions (layers) in the Session app. These
* Customized sync messages protocol on top of Signal's implementation
* Customized transcripts, receipts & typing indicators protocol on top of Signal's implementation
# Signal Protocol Layer
Don't touch this. Ever.
# Push Notifications Layer
Only applicable to mobile. Speaks for itself.

@ -15,15 +15,15 @@ NS_ASSUME_NONNULL_BEGIN
- (PreKeyRecord *_Nullable)getPreKeyForContact:(NSString *)pubKey transaction:(YapDatabaseReadTransaction *)transaction;
- (PreKeyRecord *)getOrCreatePreKeyForContact:(NSString *)pubKey;
# pragma mark - Pre Key Bundle
# pragma mark - Pre Key Management
/**
* Generates a pre key bundle but doesn't store it as pre key bundles are supposed to be sent to other users without ever being stored.
* Generates a pre key bundle for the given contact. Doesn't store the pre key bundle (pre key bundles are supposed to be sent without ever being stored).
*/
- (PreKeyBundle *)generatePreKeyBundleForContact:(NSString *)pubKey;
- (PreKeyBundle *_Nullable)getPreKeyBundleForContact:(NSString *)pubKey;
- (void)setPreKeyBundle:(PreKeyBundle *)bundle forContact:(NSString *)pubKey transaction:(YapDatabaseReadWriteTransaction *)transaction;
- (void)removePreKeyBundleForContact:(NSString *)pubKey transaction:(YapDatabaseReadWriteTransaction *)transaction;
- (PreKeyBundle *)generatePreKeyBundleForContact:(NSString *)hexEncodedPublicKey;
- (PreKeyBundle *_Nullable)getPreKeyBundleForContact:(NSString *)hexEncodedPublicKey;
- (void)setPreKeyBundle:(PreKeyBundle *)bundle forContact:(NSString *)hexEncodedPublicKey transaction:(YapDatabaseReadWriteTransaction *)transaction;
- (void)removePreKeyBundleForContact:(NSString *)hexEncodedPublicKey transaction:(YapDatabaseReadWriteTransaction *)transaction;
# pragma mark - Last Message Hash

@ -78,36 +78,35 @@
return record;
}
# pragma mark - Pre Key Bundle
# pragma mark - Pre Key Management
#define LKPreKeyBundleCollection @"LKPreKeyBundleCollection"
- (PreKeyBundle *)generatePreKeyBundleForContact:(NSString *)pubKey forceClean:(BOOL)forceClean {
// Check pre keys to make sure we have them
- (PreKeyBundle *)generatePreKeyBundleForContact:(NSString *)hexEncodedPublicKey forceClean:(BOOL)forceClean {
// Refresh signed pre key if needed
[TSPreKeyManager checkPreKeys];
ECKeyPair *_Nullable keyPair = self.identityManager.identityKeyPair;
OWSAssertDebug(keyPair);
// Refresh the signed pre key if needed
if (self.currentSignedPreKey == nil || forceClean) {
// Refresh signed pre key if needed
if (self.currentSignedPreKey == nil || forceClean) { // TODO: Is the self.currentSignedPreKey == nil check needed?
SignedPreKeyRecord *signedPreKeyRecord = [self generateRandomSignedRecord];
[signedPreKeyRecord markAsAcceptedByService];
[self storeSignedPreKey:signedPreKeyRecord.Id signedPreKeyRecord:signedPreKeyRecord];
[self setCurrentSignedPrekeyId:signedPreKeyRecord.Id];
[LKLogger print:@"[Loki] Pre keys refreshed successfully."];
[LKLogger print:@"[Loki] Signed pre key refreshed successfully."];
}
SignedPreKeyRecord *_Nullable signedPreKey = self.currentSignedPreKey;
if (!signedPreKey) {
OWSFailDebug(@"Signed pre key is null.");
if (signedPreKey == nil) {
OWSFailDebug(@"Signed pre key is nil.");
}
PreKeyRecord *preKey = [self getOrCreatePreKeyForContact:pubKey];
uint32_t registrationId = [self.accountManager getOrGenerateRegistrationId];
PreKeyRecord *preKey = [self getOrCreatePreKeyForContact:hexEncodedPublicKey];
uint32_t registrationID = [self.accountManager getOrGenerateRegistrationId];
PreKeyBundle *bundle = [[PreKeyBundle alloc] initWithRegistrationId:registrationId
PreKeyBundle *bundle = [[PreKeyBundle alloc] initWithRegistrationId:registrationID
deviceId:OWSDevicePrimaryDeviceId
preKeyId:preKey.Id
preKeyPublic:preKey.keyPair.publicKey.prependKeyType
@ -118,38 +117,42 @@
return bundle;
}
- (PreKeyBundle *)generatePreKeyBundleForContact:(NSString *)pubKey {
- (PreKeyBundle *)generatePreKeyBundleForContact:(NSString *)hexEncodedPublicKey {
NSInteger failureCount = 0;
BOOL forceClean = NO;
while (failureCount < 3) {
@try {
PreKeyBundle *preKeyBundle = [self generatePreKeyBundleForContact:pubKey forceClean:forceClean];
PreKeyBundle *preKeyBundle = [self generatePreKeyBundleForContact:hexEncodedPublicKey forceClean:forceClean];
if (![Ed25519 throws_verifySignature:preKeyBundle.signedPreKeySignature
publicKey:preKeyBundle.identityKey.throws_removeKeyType
data:preKeyBundle.signedPreKeyPublic]) {
@throw [NSException exceptionWithName:InvalidKeyException reason:@"KeyIsNotValidlySigned" userInfo:nil];
}
[LKLogger print:[NSString stringWithFormat:@"[Loki] Generated a new pre key bundle for: %@.", hexEncodedPublicKey]];
return preKeyBundle;
} @catch (NSException *exception) {
failureCount++;
failureCount += 1;
forceClean = YES;
}
}
[LKLogger print:[NSString stringWithFormat:@"[Loki] Failed to generate a valid pre key bundle for: %@.", pubKey]];
[LKLogger print:[NSString stringWithFormat:@"[Loki] Failed to generate a valid pre key bundle for: %@.", hexEncodedPublicKey]];
return nil;
}
- (PreKeyBundle *_Nullable)getPreKeyBundleForContact:(NSString *)pubKey {
return [self.dbReadConnection preKeyBundleForKey:pubKey inCollection:LKPreKeyBundleCollection];
- (PreKeyBundle *_Nullable)getPreKeyBundleForContact:(NSString *)hexEncodedPublicKey {
return [self.dbReadConnection preKeyBundleForKey:hexEncodedPublicKey inCollection:LKPreKeyBundleCollection];
}
- (void)setPreKeyBundle:(PreKeyBundle *)bundle forContact:(NSString *)pubKey transaction:(YapDatabaseReadWriteTransaction *)transaction {
[transaction setObject:bundle forKey:pubKey inCollection:LKPreKeyBundleCollection];
- (void)setPreKeyBundle:(PreKeyBundle *)bundle forContact:(NSString *)hexEncodedPublicKey transaction:(YapDatabaseReadWriteTransaction *)transaction {
[transaction setObject:bundle forKey:hexEncodedPublicKey inCollection:LKPreKeyBundleCollection];
[LKLogger print:[NSString stringWithFormat:@"[Loki] Stored pre key bundle for: %@.", hexEncodedPublicKey]];
// FIXME: I don't think the line below is good for anything
[transaction.connection flushTransactionsWithCompletionQueue:dispatch_get_main_queue() completionBlock:^{ }];
}
- (void)removePreKeyBundleForContact:(NSString *)pubKey transaction:(YapDatabaseReadWriteTransaction *)transaction {
[transaction removeObjectForKey:pubKey inCollection:LKPreKeyBundleCollection];
- (void)removePreKeyBundleForContact:(NSString *)hexEncodedPublicKey transaction:(YapDatabaseReadWriteTransaction *)transaction {
[transaction removeObjectForKey:hexEncodedPublicKey inCollection:LKPreKeyBundleCollection];
[LKLogger print:[NSString stringWithFormat:@"[Loki] Removed pre key bundle for: %@.", hexEncodedPublicKey]];
}
# pragma mark - Last Message Hash

@ -9,6 +9,7 @@ import PromiseKit
// Document the expected cases for everything.
// Express those cases in tests.
/// See [The Session Friend Request Protocol](https://github.com/loki-project/session-protocol-docs/wiki/Friend-Requests) for more information.
@objc(LKFriendRequestProtocol)
public final class FriendRequestProtocol : NSObject {
@ -20,11 +21,11 @@ public final class FriendRequestProtocol : NSObject {
// Accept all outstanding friend requests associated with this user and try to establish sessions with the
// subset of their devices that haven't sent a friend request.
let senderID = friendRequest.authorId
let linkedDeviceThreads = LokiDatabaseUtilities.getLinkedDeviceThreads(for: senderID, in: transaction)
let linkedDeviceThreads = LokiDatabaseUtilities.getLinkedDeviceThreads(for: senderID, in: transaction) // This doesn't create new threads if they don't exist yet
for thread in linkedDeviceThreads {
if thread.hasPendingFriendRequest {
// TODO: The Obj-C implementation was actually sending this to self.thread. I'm assuming that's not what we meant.
sendFriendRequestAcceptanceMessage(to: senderID, in: thread, using: transaction)
sendFriendRequestAcceptanceMessage(to: thread.contactIdentifier(), in: thread, using: transaction) // NOT senderID
thread.saveFriendRequestStatus(.friends, with: transaction)
} else {
let autoGeneratedFRMessageSend = MultiDeviceProtocol.getAutoGeneratedMultiDeviceFRMessageSend(for: senderID, in: transaction)
OWSDispatch.sendingQueue().async {
@ -33,7 +34,6 @@ public final class FriendRequestProtocol : NSObject {
}
}
}
thread.saveFriendRequestStatus(.friends, with: transaction)
}
@objc(sendFriendRequestAcceptanceMessageToHexEncodedPublicKey:in:using:)
@ -46,7 +46,8 @@ public final class FriendRequestProtocol : NSObject {
@objc(declineFriendRequest:in:using:)
public static func declineFriendRequest(_ friendRequest: TSIncomingMessage, in thread: TSThread, using transaction: YapDatabaseReadWriteTransaction) {
thread.saveFriendRequestStatus(.none, with: transaction)
// Delete pre keys
// Delete the pre key bundle for the given contact. This ensures that if we send a
// new message after this, it restarts the friend request process from scratch.
let senderID = friendRequest.authorId
storage.removePreKeyBundle(forContact: senderID, transaction: transaction)
}
@ -65,7 +66,7 @@ public final class FriendRequestProtocol : NSObject {
// This can happen if Alice sent Bob a friend request, Bob declined, but then Bob changed his
// mind and sent a friend request to Alice. In this case we want Alice to auto-accept the request
// and send a friend request accepted message back to Bob. We don't check that sending the
// friend request accepted message succeeded. Even if it doesn't, the thread's current friend
// friend request accepted message succeeds. Even if it doesn't, the thread's current friend
// request status will be set to LKThreadFriendRequestStatusFriends for Alice making it possible
// for Alice to send messages to Bob. When Bob receives a message, his thread's friend request status
// will then be set to LKThreadFriendRequestStatusFriends. If we do check for a successful send
@ -101,11 +102,13 @@ public final class FriendRequestProtocol : NSObject {
existingFriendRequestMessage.isFriendRequest {
existingFriendRequestMessage.saveFriendRequestStatus(.accepted, with: transaction)
}
/*
// Send our P2P details
if let addressMessage = LokiP2PAPI.onlineBroadcastMessage(forThread: thread) {
let messageSenderJobQueue = SSKEnvironment.shared.messageSenderJobQueue
messageSenderJobQueue.add(message: addressMessage, transaction: transaction)
}
*/
}
@objc(handleFriendRequestMessageIfNeeded:associatedWith:wrappedIn:in:using:)
@ -146,8 +149,7 @@ public final class FriendRequestProtocol : NSObject {
let linkedDeviceThreads = LokiDatabaseUtilities.getLinkedDeviceThreads(for: hexEncodedPublicKey, in: transaction)
for thread in linkedDeviceThreads {
thread.enumerateInteractions(with: transaction) { interaction, _ in
guard let incomingMessage = interaction as? TSIncomingMessage,
incomingMessage.friendRequestStatus != .none else { return }
guard let incomingMessage = interaction as? TSIncomingMessage, incomingMessage.friendRequestStatus != .none else { return }
incomingMessage.saveFriendRequestStatus(.none, with: transaction)
}
}

@ -0,0 +1,7 @@
#import "TSOutgoingMessage.h"
/// See [The Session Friend Request Protocol](https://github.com/loki-project/session-protocol-docs/wiki/Friend-Requests) for more information.
NS_SWIFT_NAME(FriendRequestMessage)
@interface LKFriendRequestMessage : TSOutgoingMessage
@end

@ -1,7 +1,9 @@
#import "LKFriendRequestMessage.h"
#import "OWSPrimaryStorage+Loki.h"
#import "NSDate+OWS.h"
#import "ProfileManagerProtocol.h"
#import "SignalRecipient.h"
#import "SSKEnvironment.h"
#import <SignalServiceKit/SignalServiceKit-Swift.h>
@implementation LKFriendRequestMessage
@ -9,6 +11,7 @@
#pragma mark Building
- (SSKProtoContentBuilder *)prepareCustomContentBuilder:(SignalRecipient *)recipient {
SSKProtoContentBuilder *contentBuilder = SSKProtoContent.builder;
// Attach the pre key bundle for the contact in question
PreKeyBundle *preKeyBundle = [OWSPrimaryStorage.sharedManager generatePreKeyBundleForContact:recipient.recipientId];
SSKProtoPrekeyBundleMessageBuilder *preKeyBundleMessageBuilder = [SSKProtoPrekeyBundleMessage builderFromPreKeyBundle:preKeyBundle];
NSError *error;

@ -1,6 +0,0 @@
#import "TSOutgoingMessage.h"
NS_SWIFT_NAME(FriendRequestMessage)
@interface LKFriendRequestMessage : TSOutgoingMessage
@end

@ -83,6 +83,7 @@ public final class MultiDeviceProtocol : NSObject {
messageSender.sendMessage(messageSendCopy)
} else {
var frMessageSend: OWSMessageSend!
// FIXME: This crashes sometimes due to transaction nesting
storage.dbReadWriteConnection.readWrite { transaction in // TODO: Yet another transaction
frMessageSend = getAutoGeneratedMultiDeviceFRMessageSend(for: slaveDestination.hexEncodedPublicKey, in: transaction)
}
@ -107,7 +108,7 @@ public final class MultiDeviceProtocol : NSObject {
let thread = TSContactThread.getOrCreateThread(withContactId: hexEncodedPublicKey, transaction: transaction)
let masterHexEncodedPublicKey = storage.getMasterHexEncodedPublicKey(for: hexEncodedPublicKey, in: transaction)
let isSlaveDeviceThread = masterHexEncodedPublicKey != hexEncodedPublicKey
thread.isForceHidden = isSlaveDeviceThread
thread.isForceHidden = isSlaveDeviceThread // TODO: Could we make this computed?
if thread.friendRequestStatus == .none || thread.friendRequestStatus == .requestExpired {
thread.saveFriendRequestStatus(.requestSent, with: transaction) // TODO: Should we always immediately mark the slave device as a friend?
}

@ -41,7 +41,7 @@ public final class SessionManagementProtocol : NSObject {
// It's done automatically when we generate a pre key bundle to send to a contact (generatePreKeyBundleForContact:).
// You can use getOrCreatePreKeyForContact: to generate one if needed.
guard storage.currentSignedPrekeyId() == nil else {
print("[Loki] Skipping pre key refresh; using existing signed pre key.")
print("[Loki] Skipping signed pre key refresh; using existing signed pre key.")
return
}
let signedPreKeyRecord = storage.generateRandomSignedRecord()
@ -50,7 +50,7 @@ public final class SessionManagementProtocol : NSObject {
storage.setCurrentSignedPrekeyId(signedPreKeyRecord.id)
TSPreKeyManager.clearPreKeyUpdateFailureCount()
TSPreKeyManager.clearSignedPreKeyRecords()
print("[Loki] Pre keys refreshed successfully.")
print("[Loki] Signed pre key refreshed successfully.")
}
@objc(rotateSignedPreKey)

@ -1087,27 +1087,30 @@ NSString *NSStringForOutgoingMessageRecipientState(OWSOutgoingMessageRecipientSt
{
OWSAssertDebug(self.thread);
SSKProtoDataMessageBuilder *_Nullable builder = [self dataMessageBuilder];
if (!builder) {
OWSFailDebug(@"could not build protobuf.");
if (builder == nil) {
OWSFailDebug(@"Couldn't build protobuf.");
return nil;
}
[ProtoUtils addLocalProfileKeyIfNecessary:self.thread recipientId:recipientId dataMessageBuilder:builder];
// Loki: Set display name & profile picture
// Loki: Set display name & profile picture (exclude the profile picture if this is a friend request
// to prevent unsolicited content from being sent)
id<ProfileManagerProtocol> profileManager = SSKEnvironment.shared.profileManager;
NSString *displayName = profileManager.localProfileName;
NSString *profilePictureURL = profileManager.profilePictureURL;
SSKProtoDataMessageLokiProfileBuilder *profileBuilder = [SSKProtoDataMessageLokiProfile builder];
[profileBuilder setDisplayName:displayName];
[profileBuilder setProfilePicture:profilePictureURL ?: @""];
if (![self isKindOfClass:LKFriendRequestMessage.class]) {
[profileBuilder setProfilePicture:profilePictureURL ?: @""];
}
SSKProtoDataMessageLokiProfile *profile = [profileBuilder buildAndReturnError:nil];
[builder setProfile:profile];
NSError *error;
SSKProtoDataMessage *_Nullable dataProto = [builder buildAndReturnError:&error];
if (error || !dataProto) {
OWSFailDebug(@"could not build protobuf: %@", error);
if (error || dataProto == nil) {
OWSFailDebug(@"Couldn't build protobuf due to error: %@.", error);
return nil;
}
return dataProto;

@ -1705,8 +1705,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
if (!bundle) {
NSString *missingPrekeyBundleException = @"missingPrekeyBundleException";
OWSRaiseException(
missingPrekeyBundleException, @"Can't get a prekey bundle from the server with required information");
OWSRaiseException(missingPrekeyBundleException, @"Missing pre key bundle for: %@.", recipientID);
} else {
SessionBuilder *builder = [[SessionBuilder alloc] initWithSessionStore:storage
preKeyStore:storage
@ -1718,7 +1717,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException";
@try {
[builder throws_processPrekeyBundle:bundle protocolContext:transaction];
// Loki: Discard the pre key bundle here since the session has been established
// Loki: Discard the pre key bundle as the session has now been established
[storage removePreKeyBundleForContact:recipientID transaction:transaction];
} @catch (NSException *caughtException) {
exception = caughtException;

Loading…
Cancel
Save