From 051e4dd22c388c004a0cf3e085115c06cf1c6011 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Wed, 28 Nov 2018 15:44:59 +1100 Subject: [PATCH 1/4] XAnother big refactor to tie conversation UI logic to the friendRequestStatus variable --- app/sql.js | 16 +-- js/models/conversations.js | 226 ++++++++++-------------------- js/views/conversation_view.js | 8 +- libtextsecure/outgoing_message.js | 3 - 4 files changed, 85 insertions(+), 168 deletions(-) diff --git a/app/sql.js b/app/sql.js index 2f456b98a..1160c50e6 100644 --- a/app/sql.js +++ b/app/sql.js @@ -396,7 +396,7 @@ async function updateToSchemaVersion6(currentVersion, instance) { await instance.run( `ALTER TABLE conversations - ADD COLUMN friendStatus INTEGER;` + ADD COLUMN friendRequestStatus INTEGER;` ); await instance.run( @@ -964,7 +964,7 @@ async function getConversationCount() { async function saveConversation(data) { // eslint-disable-next-line camelcase - const { id, active_at, type, members, name, friendStatus, profileName } = data; + const { id, active_at, type, members, name, friendRequestStatus, profileName } = data; await db.run( `INSERT INTO conversations ( @@ -975,7 +975,7 @@ async function saveConversation(data) { type, members, name, - friendStatus, + friendRequestStatus, profileName ) values ( $id, @@ -985,7 +985,7 @@ async function saveConversation(data) { $type, $members, $name, - $friendStatus, + $friendRequestStatus, $profileName );`, { @@ -996,7 +996,7 @@ async function saveConversation(data) { $type: type, $members: members ? members.join(' ') : null, $name: name, - $friendStatus: friendStatus, + $friendRequestStatus: friendRequestStatus, $profileName: profileName, } ); @@ -1020,7 +1020,7 @@ async function saveConversations(arrayOfConversations) { async function updateConversation(data) { // eslint-disable-next-line camelcase - const { id, active_at, type, members, name, friendStatus, profileName } = data; + const { id, active_at, type, members, name, friendRequestStatus, profileName } = data; await db.run( `UPDATE conversations SET @@ -1030,7 +1030,7 @@ async function updateConversation(data) { type = $type, members = $members, name = $name, - friendStatus = $friendStatus, + friendRequestStatus = $friendRequestStatus, profileName = $profileName WHERE id = $id;`, { @@ -1041,7 +1041,7 @@ async function updateConversation(data) { $type: type, $members: members ? members.join(' ') : null, $name: name, - $friendStatus: friendStatus, + $friendRequestStatus: friendRequestStatus, $profileName: profileName, } ); diff --git a/js/models/conversations.js b/js/models/conversations.js index ea54d7022..c5ec5925b 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -40,7 +40,7 @@ } = window.Signal.Migrations; // Possible conversation friend states - const FriendStatusEnum = Object.freeze({ + const FriendRequestStatusEnum = Object.freeze({ // New conversation, no messages sent or received none: 0, // Friend request not complete yet, input blocked @@ -69,7 +69,7 @@ return { unreadCount: 0, verified: textsecure.storage.protocol.VerifiedStatus.DEFAULT, - friendStatus: FriendStatusEnum.none, + friendRequestStatus: FriendRequestStatusEnum.none, unlockTimestamp: null, // Timestamp used for expiring friend requests. }; }, @@ -105,8 +105,6 @@ conversation: this, }); - this.pendingFriendRequest = false; - this.messageCollection.on('change:errors', this.handleMessageError, this); this.messageCollection.on('send-error', this.onMessageError, this); @@ -147,7 +145,6 @@ this.unset('lastMessage'); this.unset('lastMessageStatus'); - this.updateTextInputState(); this.setFriendRequestExpiryTimeout(); }, @@ -253,35 +250,8 @@ title: this.getTitle(), }; }, - // This function sets `pendingFriendRequest` variable in memory - async updatePendingFriendRequests() { - const pendingFriendRequest = await this.hasPendingFriendRequests(); - // Only update if we have different values - if (this.pendingFriendRequest !== pendingFriendRequest) { - this.pendingFriendRequest = pendingFriendRequest; - // trigger an update - this.trigger('change'); - } - }, // This goes through all our message history and finds a friend request - // But this is not a concurrent operation and thus updatePendingFriendRequests is used - async hasPendingFriendRequests() { - // Go through the messages and check for any pending friend requests - const messages = await window.Signal.Data.getMessagesByConversation( - this.id, - { - type: 'friend-request', - MessageCollection: Whisper.MessageCollection, - } - ); - const pendingFriendRequest = - messages.models.find(message => - message.isFriendRequest() && - message.attributes.friendStatus === 'pending' - ); - return pendingFriendRequest !== undefined; - }, - async getPendingFriendRequests(direction) { + async getPendingFriendRequests(direction = null) { // Theoretically all our messages could be friend requests, // thus we have to unfortunately go through each one :( const messages = await window.Signal.Data.getMessagesByConversation( @@ -291,12 +261,13 @@ MessageCollection: Whisper.MessageCollection, } ); - - // Get the messages that are matching the direction and the friendStatus - return messages.models.filter(m => - m.attributes.direction === direction && - m.attributes.friendStatus === 'pending' - ); + // Get the pending friend requests that match the direction + // If no direction is supplied then return all pending friend requests + return messages.models.filter(m => { + if (m.attributes.friendStatus !== 'pending') + return false; + return direction === null || m.attributes.direction === direction; + }); }, getPropsForListItem() { const result = { @@ -306,7 +277,7 @@ lastUpdated: this.get('timestamp'), unreadCount: this.get('unreadCount') || 0, isSelected: this.isSelected, - showFriendRequestIndicator: this.pendingFriendRequest, + showFriendRequestIndicator: this.isPending(), isBlocked: this.isBlocked(), lastMessage: { status: this.lastMessageStatus, @@ -459,81 +430,83 @@ return contact.isVerified(); }); }, - async waitingForFriendRequestApproval() { - // Check if we have an incoming friend request - // Or any successful outgoing ones - const incoming = await this.getPendingFriendRequests('incoming'); - const outgoing = await this.getPendingFriendRequests('outgoing'); - const successfulOutgoing = outgoing.filter(o => !o.hasErrors()); - - return (incoming.length > 0 || successfulOutgoing.length > 0); + isPending() { + return this.get('friendRequestStatus') === FriendRequestStatusEnum.pending; }, isFriend() { - return this.get('friendStatus') === FriendStatusEnum.friends; + return this.get('friendRequestStatus') === FriendRequestStatusEnum.friends; + }, + updateTextInputState() { + switch (this.get('friendRequestStatus')) { + case FriendRequestStatusEnum.none: + this.trigger('disable:input', false); + this.trigger('change:placeholder', 'friend-request'); + return; + case FriendRequestStatusEnum.pending: + this.trigger('disable:input', true); + this.trigger('change:placeholder', 'disabled'); + return; + case FriendRequestStatusEnum.friends: + this.trigger('disable:input', false); + this.trigger('change:placeholder', 'chat'); + return; + default: + throw new Error('Invalid friend request state'); + } }, - // Update any pending friend requests for the current user - async updateFriendRequestUI() { - // Enable the text inputs early + async setFriendRequestStatus(newStatus) { + this.set({ friendRequestStatus: newStatus }); + await window.Signal.Data.updateConversation(this.id, this.attributes, { + Conversation: Whisper.Conversation, + }); this.updateTextInputState(); - - // We only update our friend requests if we have the user as a friend - if (!this.isFriend()) return; - - // Update any pending outgoing messages - const pending = await this.getPendingFriendRequests('outgoing'); + }, + async respondToAllPendingFriendRequests(options) { + const { response, direction = null } = options; + // Ignore if no response supplied + if (!response) return; + const pending = await this.getPendingFriendRequests(direction); await Promise.all( pending.map(async request => { if (request.hasErrors()) return; - request.set({ friendStatus: 'accepted' }); + request.set({ friendStatus: response }); await window.Signal.Data.saveMessage(request.attributes, { Message: Whisper.Message, }); this.trigger('updateMessage', request); }) ); - - // Update our local state - await this.updatePendingFriendRequests(); - - // Send the notification if we had an outgoing friend request - if (pending.length > 0) - this.notifyFriendRequest(this.id, 'accepted') }, // We have declined an incoming friend request async onDeclineFriendRequest() { - // Should we change states for other states? (They should never happen) - if (this.get('friendStatus') === FriendStatusEnum.pending) { - this.set({ friendStatus: FriendStatusEnum.none }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, - }); - await this.updateFriendRequestUI(); - await this.updatePendingFriendRequests(); - await window.libloki.removePreKeyBundleForNumber(this.id); - } + this.setFriendRequestStatus(FriendRequestStatusEnum.none); + await this.respondToAllPendingFriendRequests({ + response: 'declined', + direction: 'incoming', + }); + await window.libloki.removePreKeyBundleForNumber(this.id); }, // We have accepted an incoming friend request async onAcceptFriendRequest() { - // Should we change states for other states? (They should never happen) - if (this.get('friendStatus') === FriendStatusEnum.pending) { - this.set({ friendStatus: FriendStatusEnum.friends }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, + if (this.isPending()) { + this.setFriendRequestStatus(FriendRequestStatusEnum.friends); + await this.respondToAllPendingFriendRequests({ + response: 'accepted', + direction: 'incoming', }); - await this.updateFriendRequestUI(); - await this.updatePendingFriendRequests(); window.libloki.sendFriendRequestAccepted(this.id); } }, // Our outgoing friend request has been accepted async onFriendRequestAccepted() { - if (this.get('friendStatus') === FriendStatusEnum.pending) { - this.set({ friendStatus: FriendStatusEnum.friends }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, + if (this.isPending()) { + this.setFriendRequestStatus(FriendRequestStatusEnum.friends); + await this.respondToAllPendingFriendRequests({ + response: 'accepted', + direction: 'outgoing', }); - await this.updateFriendRequestUI(); + this.notifyFriendRequest(this.id, 'accepted') } }, async onFriendRequestTimeout() { @@ -552,30 +525,16 @@ } // Change any pending outgoing friend requests to expired - const outgoing = await this.getPendingFriendRequests('outgoing'); - await Promise.all( - outgoing.map(async request => { - if (request.hasErrors()) return; - - request.set({ friendStatus: 'expired' }); - await window.Signal.Data.saveMessage(request.attributes, { - Message: Whisper.Message, - }); - this.trigger('updateMessage', request); - }) - ); - - // Update the UI - await this.updatePendingFriendRequests(); - await this.updateFriendRequestUI(); + await this.respondToAllPendingFriendRequests({ + response: 'expired', + direction: 'outgoing', + }); + await this.setFriendRequestStatus(FriendRequestStatusEnum.none); }, async onFriendRequestReceived() { - if (this.get('friendStatus') === FriendStatusEnum.none) { - this.set({ friendStatus: FriendStatusEnum.pending }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, - }); - await this.updateFriendRequestUI(); + if (this.get('friendRequestStatus') === FriendRequestStatusEnum.none) { + this.setFriendRequestStatus(FriendRequestStatusEnum.pending); + this.notifyFriendRequest(this.id, 'requested'); } }, async onFriendRequestSent() { @@ -587,20 +546,9 @@ const ms = 60 * 60 * 1000 * hourLockDuration; this.set({ unlockTimestamp: Date.now() + ms }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, - }); - this.setFriendRequestExpiryTimeout(); } - - if (this.get('friendStatus') === FriendStatusEnum.none) { - this.set({ friendStatus: FriendStatusEnum.pending }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, - }); - } - this.updateFriendRequestUI(); + await this.setFriendRequestStatus(FriendRequestStatusEnum.pending); }, setFriendRequestExpiryTimeout() { const unlockTimestamp = this.get('unlockTimestamp'); @@ -972,6 +920,9 @@ }, async sendMessage(body, attachments, quote) { + // Input should be blocked if there is a pending friend request + if (this.isPending()) + return; const destination = this.id; const expireTimer = this.get('expireTimer'); const recipients = this.getRecipients(); @@ -1007,12 +958,7 @@ recipients, }); } else { - // We also need to make sure we don't send a new friend request - // if we already have an existing one - const incomingRequests = await this.getPendingFriendRequests('incoming'); - if (incomingRequests.length > 0) return null; - - // Otherwise check if we have sent a friend request + // Check if we have sent a friend request const outgoingRequests = await this.getPendingFriendRequests('outgoing'); if (outgoingRequests.length > 0) { // Check if the requests have errored, if so then remove them @@ -1123,24 +1069,6 @@ return true; }); }, - async updateTextInputState() { - // Check if we need to disable the text field - if (!this.isFriend()) { - // Disable the input if we're waiting for friend request approval - const waiting = await this.waitingForFriendRequestApproval(); - if (waiting) { - this.trigger('disable:input', true); - this.trigger('change:placeholder', 'disabled'); - return; - } - // Tell the user to introduce themselves - this.trigger('disable:input', false); - this.trigger('change:placeholder', 'friend-request'); - return; - } - this.trigger('disable:input', false); - this.trigger('change:placeholder', 'chat'); - }, wrapSend(promise) { return promise.then( async result => { @@ -1328,12 +1256,6 @@ return; } - // Update our friend indicator - this.updatePendingFriendRequests(); - - // Update our text input state - await this.updateTextInputState(); - const messages = await window.Signal.Data.getMessagesByConversation( this.id, { limit: 1, MessageCollection: Whisper.MessageCollection } diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index 763fad932..1ccafb00e 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -7,10 +7,9 @@ /* global Signal: false */ /* global storage: false */ /* global Whisper: false */ -/* global BlockNumberConversation: false */ // eslint-disable-next-line func-names -(function() { +(function () { 'use strict'; window.Whisper = window.Whisper || {}; @@ -202,7 +201,7 @@ onBlockUser: () => { this.model.block(); }, - onUnblockUser: () => { + onUnblockUser: () => { this.model.unblock(); }, onChangeNickname: () => { @@ -255,8 +254,7 @@ this.$('.send-message').blur(this.unfocusBottomBar.bind(this)); this.$emojiPanelContainer = this.$('.emoji-panel-container'); - - this.model.updateFriendRequestUI(); + this.model.updateTextInputState(); }, events: { diff --git a/libtextsecure/outgoing_message.js b/libtextsecure/outgoing_message.js index ebfdc4e29..75bc086f4 100644 --- a/libtextsecure/outgoing_message.js +++ b/libtextsecure/outgoing_message.js @@ -460,9 +460,6 @@ OutgoingMessage.prototype = { }) .then(this.reloadDevicesAndSend(number, true)) .catch(error => { - if (this.fallBackEncryption && conversation) { - conversation.updateFriendRequestUI(); - } if (error.message === 'Identity key changed') { // eslint-disable-next-line no-param-reassign error = new textsecure.OutgoingIdentityKeyError( From 2de01d034335aff13698b2b55a55a69515503240 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Thu, 29 Nov 2018 15:28:53 +1100 Subject: [PATCH 2/4] Back to multiple pending states, fix some notification bugs and friend requests should now work for all cases besides message sending interruption --- js/background.js | 2 +- js/models/conversations.js | 107 +++++++++++++++--------------- js/models/messages.js | 20 +++++- libtextsecure/message_receiver.js | 56 +++++----------- 4 files changed, 89 insertions(+), 96 deletions(-) diff --git a/js/background.js b/js/background.js index 8df8adade..fc0c6d215 100644 --- a/js/background.js +++ b/js/background.js @@ -1293,7 +1293,7 @@ unread: 1, }; - if (data.type === 'friend-request') { + if (data.friendRequest) { messageData = { ...messageData, type: 'friend-request', diff --git a/js/models/conversations.js b/js/models/conversations.js index c5ec5925b..66f773108 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -43,10 +43,12 @@ const FriendRequestStatusEnum = Object.freeze({ // New conversation, no messages sent or received none: 0, - // Friend request not complete yet, input blocked - pending: 1, + // Friend request sent, awaiting response + requestSent: 1, + // Friend request received, awaiting user input + requestReceived: 2, // We did it! - friends: 2, + friends: 3, }); const COLORS = [ @@ -264,9 +266,9 @@ // Get the pending friend requests that match the direction // If no direction is supplied then return all pending friend requests return messages.models.filter(m => { - if (m.attributes.friendStatus !== 'pending') + if (m.get('friendStatus') !== 'pending') return false; - return direction === null || m.attributes.direction === direction; + return direction === null || m.get('direction') === direction; }); }, getPropsForListItem() { @@ -430,8 +432,19 @@ return contact.isVerified(); }); }, + isNone() { + return this.get('friendRequestStatus') === FriendRequestStatusEnum.none; + }, isPending() { - return this.get('friendRequestStatus') === FriendRequestStatusEnum.pending; + const status = this.get('friendRequestStatus'); + return status === FriendRequestStatusEnum.requestSent || + status === FriendRequestStatusEnum.requestSent; + }, + hasSentFriendRequest() { + return this.get('friendRequestStatus') === FriendRequestStatusEnum.requestSent; + }, + hasReceivedFriendRequest() { + return this.get('friendRequestStatus') === FriendRequestStatusEnum.requestReceived; }, isFriend() { return this.get('friendRequestStatus') === FriendRequestStatusEnum.friends; @@ -442,7 +455,8 @@ this.trigger('disable:input', false); this.trigger('change:placeholder', 'friend-request'); return; - case FriendRequestStatusEnum.pending: + case FriendRequestStatusEnum.requestReceived: + case FriendRequestStatusEnum.requestSent: this.trigger('disable:input', true); this.trigger('change:placeholder', 'disabled'); return; @@ -455,11 +469,13 @@ } }, async setFriendRequestStatus(newStatus) { - this.set({ friendRequestStatus: newStatus }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, - }); - this.updateTextInputState(); + if (this.get('friendRequestStatus') !== newStatus) { + this.set({ friendRequestStatus: newStatus }); + await window.Signal.Data.updateConversation(this.id, this.attributes, { + Conversation: Whisper.Conversation, + }); + this.updateTextInputState(); + } }, async respondToAllPendingFriendRequests(options) { const { response, direction = null } = options; @@ -489,7 +505,7 @@ }, // We have accepted an incoming friend request async onAcceptFriendRequest() { - if (this.isPending()) { + if (this.hasReceivedFriendRequest()) { this.setFriendRequestStatus(FriendRequestStatusEnum.friends); await this.respondToAllPendingFriendRequests({ response: 'accepted', @@ -500,13 +516,11 @@ }, // Our outgoing friend request has been accepted async onFriendRequestAccepted() { - if (this.isPending()) { + if (this.hasSentFriendRequest()) { this.setFriendRequestStatus(FriendRequestStatusEnum.friends); await this.respondToAllPendingFriendRequests({ response: 'accepted', - direction: 'outgoing', }); - this.notifyFriendRequest(this.id, 'accepted') } }, async onFriendRequestTimeout() { @@ -532,10 +546,24 @@ await this.setFriendRequestStatus(FriendRequestStatusEnum.none); }, async onFriendRequestReceived() { - if (this.get('friendRequestStatus') === FriendRequestStatusEnum.none) { - this.setFriendRequestStatus(FriendRequestStatusEnum.pending); - this.notifyFriendRequest(this.id, 'requested'); + if (this.isNone()) { + this.setFriendRequestStatus(FriendRequestStatusEnum.requestReceived); + } else if (this.hasSentFriendRequest()) { + await Promise.all([ + this.setFriendRequestStatus(FriendRequestStatusEnum.friends), + // Accept all outgoing FR + this.respondToAllPendingFriendRequests({ + direction: 'outgoing', + response: 'accepted', + }), + ]); } + // Delete stale incoming friend requests + const incoming = await this.getPendingFriendRequests('incoming'); + await Promise.all( + incoming.map(request => this._removeMessage(request.id)) + ); + this.trigger('change'); }, async onFriendRequestSent() { // Check if we need to set the friend request expiry @@ -548,7 +576,7 @@ this.set({ unlockTimestamp: Date.now() + ms }); this.setFriendRequestExpiryTimeout(); } - await this.setFriendRequestStatus(FriendRequestStatusEnum.pending); + await this.setFriendRequestStatus(FriendRequestStatusEnum.requestSent); }, setFriendRequestExpiryTimeout() { const unlockTimestamp = this.get('unlockTimestamp'); @@ -1219,36 +1247,8 @@ }, }; }, + // eslint-disable-next-line no-unused-vars async onNewMessage(message) { - if (message.get('type') === 'friend-request' && message.get('direction') === 'incoming') { - // We need to make sure we remove any pending requests that we may have - // This is to ensure that one user cannot spam us with multiple friend requests. - const incoming = await this.getPendingFriendRequests('incoming'); - - // Delete the old messages if it's pending - await Promise.all( - incoming - .filter(i => i.id !== message.id) - .map(request => this._removeMessage(request.id)) - ); - - // If we have an outgoing friend request then - // we auto accept the incoming friend request - const outgoing = await this.getPendingFriendRequests('outgoing'); - if (outgoing.length > 0) { - const current = this.messageCollection.find(i => i.id === message.id); - if (current) { - await current.acceptFriendRequest(); - } else { - window.log.debug('onNewMessage: Failed to find incoming friend request'); - } - } - - // Trigger an update if we removed or updated messages - if (outgoing.length > 0 || incoming.length > 0) - this.trigger('change'); - } - return this.updateLastMessage(); }, async updateLastMessage() { @@ -1903,10 +1903,11 @@ }, notify(message) { - if (!message.isIncoming()) { - if (message.isFriendRequest()) - return this.notifyFriendRequest(message.get('source'), 'requested'); - return Promise.resolve(); + if (message.isOutgoing()) return Promise.resolve(); + if (message.isFriendRequest()) { + if (this.hasSentFriendRequest()) + return this.notifyFriendRequest(message.get('source'), 'accepted') + return this.notifyFriendRequest(message.get('source'), 'requested'); } const conversationId = this.id; diff --git a/js/models/messages.js b/js/models/messages.js index 96e270e6c..ec1a5e7ef 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -1375,6 +1375,20 @@ } } + let autoAccept = false; + if (message.get('type') === 'friend-request') { + if (conversation.hasSentFriendRequest()) { + // Automatically accept incoming friend requests if we have send one already + autoAccept = true; + message.set({ friendStatus: 'accepted' }); + await conversation.onFriendRequestAccepted(); + window.libloki.sendFriendRequestAccepted(message.get('source')); + } else if (conversation.isNone()) { + await conversation.onFriendRequestReceived(); + } + } else { + await conversation.onFriendRequestAccepted(); + } const id = await window.Signal.Data.saveMessage(message.attributes, { Message: Whisper.Message, }); @@ -1422,7 +1436,11 @@ } if (message.get('unread')) { - await conversation.notify(message); + // Need to do this here because the conversation has already changed states + if (autoAccept) + await conversation.notifyFriendRequest(source, 'accepted'); + else + await conversation.notify(message); } confirm(); diff --git a/libtextsecure/message_receiver.js b/libtextsecure/message_receiver.js index 04a04e851..ea36300e6 100644 --- a/libtextsecure/message_receiver.js +++ b/libtextsecure/message_receiver.js @@ -904,10 +904,7 @@ MessageReceiver.prototype.extend({ }) ); }, - async handleFriendRequestMessage(envelope, msg) { - return this.handleDataMessage(envelope, msg, 'friend-request'); - }, - handleDataMessage(envelope, msg, type = 'data') { + handleDataMessage(envelope, msg) { window.log.info('data message from', this.getEnvelopeId(envelope)); let p = Promise.resolve(); // eslint-disable-next-line no-bitwise @@ -924,6 +921,8 @@ MessageReceiver.prototype.extend({ message.group && message.group.type === textsecure.protobuf.GroupContext.Type.QUIT ); + const friendRequest = + envelope.type === textsecure.protobuf.Envelope.Type.FRIEND_REQUEST; // Check if we need to update any profile names if (!isMe && conversation) { @@ -936,7 +935,7 @@ MessageReceiver.prototype.extend({ conversation.setProfile(profile); } - if (type === 'friend-request' && isMe) { + if (friendRequest && isMe) { window.log.info( 'refusing to add a friend request to ourselves' ); @@ -955,7 +954,7 @@ MessageReceiver.prototype.extend({ const ev = new Event('message'); ev.confirm = this.removeFromCache.bind(this, envelope); ev.data = { - type, + friendRequest, source: envelope.source, sourceDevice: envelope.sourceDevice, timestamp: envelope.timestamp.toNumber(), @@ -992,27 +991,11 @@ MessageReceiver.prototype.extend({ async innerHandleContentMessage(envelope, plaintext) { const content = textsecure.protobuf.Content.decode(plaintext); - let conversation; - try { - conversation = await window.ConversationController.getOrCreateAndWait(envelope.source, 'private'); - } catch (e) { - window.log.info('Error getting conversation: ', envelope.source); - } - if (envelope.type === textsecure.protobuf.Envelope.Type.FRIEND_REQUEST) { - conversation.onFriendRequestReceived(); - } else { - conversation.onFriendRequestAccepted(); - } - - if (content.preKeyBundleMessage) { - const preKeyBundleMessage = - this.decodePreKeyBundleMessage(content.preKeyBundleMessage); + if (content.preKeyBundleMessage) await this.savePreKeyBundleMessage( envelope.source, - preKeyBundleMessage + content.preKeyBundleMessage ); - return this.handleDataMessage(envelope, content.dataMessage, 'friend-request'); - } if (content.syncMessage) return this.handleSyncMessage(envelope, content.syncMessage); if (content.dataMessage) @@ -1024,6 +1007,12 @@ MessageReceiver.prototype.extend({ if (content.receiptMessage) return this.handleReceiptMessage(envelope, content.receiptMessage); + // Trigger conversation friend request event for empty message + const conversation = window.ConversationController.get(envelope.source); + if (conversation) { + conversation.onFriendRequestAccepted(); + conversation.notifyFriendRequest(envelope.source, 'accepted'); + } this.removeFromCache(envelope); return null; }, @@ -1227,7 +1216,7 @@ MessageReceiver.prototype.extend({ return this.removeFromCache(envelope); }, - decodePreKeyBundleMessage(preKeyBundleMessage) { + async savePreKeyBundleMessage(pubKey, preKeyBundleMessage) { const [identityKey, preKey, signedKey, signature] = [ preKeyBundleMessage.identityKey, preKeyBundleMessage.preKey, @@ -1235,24 +1224,9 @@ MessageReceiver.prototype.extend({ preKeyBundleMessage.signature, ].map(k => dcodeIO.ByteBuffer.wrap(k).toArrayBuffer()); - return { - ...preKeyBundleMessage, - identityKey, - preKey, - signedKey, - signature, - }; - }, - async savePreKeyBundleMessage(pubKey, preKeyBundleMessage) { - if (!preKeyBundleMessage) return null; - const { preKeyId, signedKeyId, - identityKey, - preKey, - signedKey, - signature, } = preKeyBundleMessage; if (pubKey !== StringView.arrayBufferToHex(identityKey)) { @@ -1261,7 +1235,7 @@ MessageReceiver.prototype.extend({ ); } - return libloki.savePreKeyBundleForNumber({ + await libloki.savePreKeyBundleForNumber({ pubKey, preKeyId, signedKeyId, From 91a8a82e39479fe4197a49d994ec35d25bf52bf2 Mon Sep 17 00:00:00 2001 From: Beaudan Date: Thu, 29 Nov 2018 17:05:58 +1100 Subject: [PATCH 3/4] More PR suggestions, now recover from early exit before friend request is sent --- js/conversation_controller.js | 2 ++ js/models/conversations.js | 37 ++++++++++++++++++++++++------- libtextsecure/outgoing_message.js | 9 +++----- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/js/conversation_controller.js b/js/conversation_controller.js index 54ca0f89c..816c3c4f4 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -257,6 +257,8 @@ conversation.updateLastMessage(), conversation.updateProfile(), conversation.updateProfileAvatar(), + conversation.resetPendingSend(), + conversation.updateProfile(), ]); }); await Promise.all(promises); diff --git a/js/models/conversations.js b/js/models/conversations.js index 66f773108..2de4beda0 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -43,12 +43,14 @@ const FriendRequestStatusEnum = Object.freeze({ // New conversation, no messages sent or received none: 0, + // This state is used to lock the input early while sending + pendingSend: 1, // Friend request sent, awaiting response - requestSent: 1, + requestSent: 2, // Friend request received, awaiting user input - requestReceived: 2, + requestReceived: 3, // We did it! - friends: 3, + friends: 4, }); const COLORS = [ @@ -435,10 +437,16 @@ isNone() { return this.get('friendRequestStatus') === FriendRequestStatusEnum.none; }, + hasInputBlocked() { + const status = this.get('friendRequestStatus'); + return status === FriendRequestStatusEnum.requestSent || + status === FriendRequestStatusEnum.requestReceived || + status === FriendRequestStatusEnum.pendingSend; + }, isPending() { const status = this.get('friendRequestStatus'); return status === FriendRequestStatusEnum.requestSent || - status === FriendRequestStatusEnum.requestSent; + status === FriendRequestStatusEnum.requestReceived; }, hasSentFriendRequest() { return this.get('friendRequestStatus') === FriendRequestStatusEnum.requestSent; @@ -455,6 +463,7 @@ this.trigger('disable:input', false); this.trigger('change:placeholder', 'friend-request'); return; + case FriendRequestStatusEnum.pendingSend: case FriendRequestStatusEnum.requestReceived: case FriendRequestStatusEnum.requestSent: this.trigger('disable:input', true); @@ -469,6 +478,9 @@ } }, async setFriendRequestStatus(newStatus) { + // Ensure that the new status is a valid FriendStatusEnum value + if (!Object.values(FriendRequestStatusEnum).some(v => v === newStatus)) + return; if (this.get('friendRequestStatus') !== newStatus) { this.set({ friendRequestStatus: newStatus }); await window.Signal.Data.updateConversation(this.id, this.attributes, { @@ -494,6 +506,11 @@ }) ); }, + async resetPendingSend() { + if (this.get('friendRequestStatus') === FriendRequestStatusEnum.pendingSend) { + await this.setFriendRequestStatus(FriendRequestStatusEnum.none); + } + }, // We have declined an incoming friend request async onDeclineFriendRequest() { this.setFriendRequestStatus(FriendRequestStatusEnum.none); @@ -949,7 +966,7 @@ async sendMessage(body, attachments, quote) { // Input should be blocked if there is a pending friend request - if (this.isPending()) + if (this.hasInputBlocked()) return; const destination = this.id; const expireTimer = this.get('expireTimer'); @@ -1007,6 +1024,7 @@ // because one of them was sent successfully if (friendRequestSent) return null; } + await this.setFriendRequestStatus(FriendRequestStatusEnum.pendingSend); // Send the friend request! messageWithSchema = await upgradeMessageSchema({ @@ -1089,7 +1107,8 @@ expireTimer, profileKey, options - ) + ), + message.isFriendRequest() ) ) ); @@ -1097,11 +1116,13 @@ return true; }); }, - wrapSend(promise) { + wrapSend(promise, isFriendRequest = false) { return promise.then( async result => { // success if (result) { + if (isFriendRequest) + this.onFriendRequestSent(); await this.handleMessageSendResult( result.failoverNumbers, result.unidentifiedDeliveries @@ -1903,12 +1924,12 @@ }, notify(message) { - if (message.isOutgoing()) return Promise.resolve(); if (message.isFriendRequest()) { if (this.hasSentFriendRequest()) return this.notifyFriendRequest(message.get('source'), 'accepted') return this.notifyFriendRequest(message.get('source'), 'requested'); } + if (!message.isIncoming()) return Promise.resolve(); const conversationId = this.id; return ConversationController.getOrCreateAndWait( diff --git a/libtextsecure/outgoing_message.js b/libtextsecure/outgoing_message.js index 75bc086f4..86131eceb 100644 --- a/libtextsecure/outgoing_message.js +++ b/libtextsecure/outgoing_message.js @@ -280,7 +280,7 @@ OutgoingMessage.prototype = { const address = new libsignal.SignalProtocolAddress(number, deviceId); const ourKey = textsecure.storage.user.getNumber(); const options = {}; - const fallBackEncryption = new libloki.FallBackSessionCipher(address); + const fallBackCipher = new libloki.FallBackSessionCipher(address); // Check if we need to attach the preKeys let sessionCipher; @@ -288,7 +288,7 @@ OutgoingMessage.prototype = { // Encrypt them with the fallback this.message.preKeyBundleMessage = await libloki.getPreKeyBundleForNumber(number); window.log.info('attaching prekeys to outgoing message'); - sessionCipher = fallBackEncryption; + sessionCipher = fallBackCipher; } else { sessionCipher = new libsignal.SessionCipher( textsecure.storage.protocol, @@ -453,13 +453,10 @@ OutgoingMessage.prototype = { log.info('Fallback encryption enabled'); this.fallBackEncryption = true; } - - if (this.fallBackEncryption && conversation) { - await conversation.onFriendRequestSent(); - } }) .then(this.reloadDevicesAndSend(number, true)) .catch(error => { + conversation.resetPendingSend(); if (error.message === 'Identity key changed') { // eslint-disable-next-line no-param-reassign error = new textsecure.OutgoingIdentityKeyError( From 28fc5793a70dc5d2a848ba485b75c6bb5d942e9d Mon Sep 17 00:00:00 2001 From: Beaudan Date: Mon, 3 Dec 2018 13:39:02 +1100 Subject: [PATCH 4/4] More explicit function names, shifted friend request sent logic to more sensible place --- js/models/conversations.js | 49 +++++++++++++++---------------- js/models/messages.js | 2 +- libtextsecure/outgoing_message.js | 1 + 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index 2de4beda0..54478ff9d 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -281,7 +281,7 @@ lastUpdated: this.get('timestamp'), unreadCount: this.get('unreadCount') || 0, isSelected: this.isSelected, - showFriendRequestIndicator: this.isPending(), + showFriendRequestIndicator: this.isPendingFriendRequest(), isBlocked: this.isBlocked(), lastMessage: { status: this.lastMessageStatus, @@ -434,20 +434,15 @@ return contact.isVerified(); }); }, - isNone() { + isFriendRequestStatusNone() { return this.get('friendRequestStatus') === FriendRequestStatusEnum.none; }, - hasInputBlocked() { + isPendingFriendRequest() { const status = this.get('friendRequestStatus'); return status === FriendRequestStatusEnum.requestSent || status === FriendRequestStatusEnum.requestReceived || status === FriendRequestStatusEnum.pendingSend; }, - isPending() { - const status = this.get('friendRequestStatus'); - return status === FriendRequestStatusEnum.requestSent || - status === FriendRequestStatusEnum.requestReceived; - }, hasSentFriendRequest() { return this.get('friendRequestStatus') === FriendRequestStatusEnum.requestSent; }, @@ -479,7 +474,7 @@ }, async setFriendRequestStatus(newStatus) { // Ensure that the new status is a valid FriendStatusEnum value - if (!Object.values(FriendRequestStatusEnum).some(v => v === newStatus)) + if (!(newStatus in Object.values(FriendRequestStatusEnum))) return; if (this.get('friendRequestStatus') !== newStatus) { this.set({ friendRequestStatus: newStatus }); @@ -563,7 +558,7 @@ await this.setFriendRequestStatus(FriendRequestStatusEnum.none); }, async onFriendRequestReceived() { - if (this.isNone()) { + if (this.isFriendRequestStatusNone()) { this.setFriendRequestStatus(FriendRequestStatusEnum.requestReceived); } else if (this.hasSentFriendRequest()) { await Promise.all([ @@ -966,7 +961,7 @@ async sendMessage(body, attachments, quote) { // Input should be blocked if there is a pending friend request - if (this.hasInputBlocked()) + if (this.isPendingFriendRequest()) return; const destination = this.id; const expireTimer = this.get('expireTimer'); @@ -1107,8 +1102,7 @@ expireTimer, profileKey, options - ), - message.isFriendRequest() + ) ) ) ); @@ -1116,34 +1110,39 @@ return true; }); }, - wrapSend(promise, isFriendRequest = false) { + wrapSend(promise) { return promise.then( async result => { // success if (result) { - if (isFriendRequest) - this.onFriendRequestSent(); - await this.handleMessageSendResult( - result.failoverNumbers, - result.unidentifiedDeliveries - ); + await this.handleMessageSendResult({ + ...result, + success: true, + }); } return result; }, async result => { // failure if (result) { - await this.handleMessageSendResult( - result.failoverNumbers, - result.unidentifiedDeliveries - ); + await this.handleMessageSendResult({ + ...result, + success: false, + }); } throw result; } ); }, - async handleMessageSendResult(failoverNumbers, unidentifiedDeliveries) { + async handleMessageSendResult({ + failoverNumbers, + unidentifiedDeliveries, + messageType, + success, + }) { + if (success && messageType === 'friend-request') + await this.onFriendRequestSent(); await Promise.all( (failoverNumbers || []).map(async number => { const conversation = ConversationController.get(number); diff --git a/js/models/messages.js b/js/models/messages.js index ec1a5e7ef..d528b524e 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -1383,7 +1383,7 @@ message.set({ friendStatus: 'accepted' }); await conversation.onFriendRequestAccepted(); window.libloki.sendFriendRequestAccepted(message.get('source')); - } else if (conversation.isNone()) { + } else if (conversation.isFriendRequestStatusNone()) { await conversation.onFriendRequestReceived(); } } else { diff --git a/libtextsecure/outgoing_message.js b/libtextsecure/outgoing_message.js index 86131eceb..0e4be306c 100644 --- a/libtextsecure/outgoing_message.js +++ b/libtextsecure/outgoing_message.js @@ -60,6 +60,7 @@ OutgoingMessage.prototype = { failoverNumbers: this.failoverNumbers, errors: this.errors, unidentifiedDeliveries: this.unidentifiedDeliveries, + messageType: this.messageType, }); } },