From 676cf3a3212043ff5458280cd0a6a95fdcc919b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Mon, 20 Nov 2023 10:21:34 +0100 Subject: [PATCH] Upsert a contact when seeing it for the first time (#211) Also: * Remove unimplemented CLI subcommands * Fix upserting profile keys when receiving messages --- presage-cli/src/main.rs | 29 +----- presage-store-sled/src/lib.rs | 13 +-- presage/src/manager/registered.rs | 157 +++++++++++++++++++++--------- presage/src/store.rs | 11 +-- 4 files changed, 123 insertions(+), 87 deletions(-) diff --git a/presage-cli/src/main.rs b/presage-cli/src/main.rs index 40d8fab0d..ff9887a8f 100644 --- a/presage-cli/src/main.rs +++ b/presage-cli/src/main.rs @@ -78,11 +78,6 @@ enum Cmd { #[clap(long, help = "Force to register again if already registered")] force: bool, }, - #[clap(about = "Unregister from Signal")] - Unregister, - #[clap( - about = "Generate a QR code to scan with Signal for iOS or Android to link this client as secondary device" - )] LinkDevice { /// Possible values: staging, production #[clap(long, short = 's', default_value = "production")] @@ -106,16 +101,6 @@ enum Cmd { #[clap(long, value_parser = parse_base64_profile_key)] profile_key: Option, }, - #[clap(about = "Set a name, status and avatar")] - UpdateProfile, - #[clap(about = "Check if a user is registered on Signal")] - GetUserStatus, - #[clap(about = "Block contacts or groups")] - Block, - #[clap(about = "Unblock contacts or groups")] - Unblock, - #[clap(about = "Update the details of a contact")] - UpdateContact, #[clap(about = "Receive all pending messages and saves them to disk")] Receive { #[clap(long = "notifications", short = 'n')] @@ -420,15 +405,15 @@ fn print_message( let ts = content.timestamp(); let (prefix, body) = match msg { Msg::Received(Thread::Contact(sender), body) => { - let contact = format_contact(*sender); + let contact = format_contact(sender); (format!("From {contact} @ {ts}: "), body) } Msg::Sent(Thread::Contact(recipient), body) => { - let contact = format_contact(*recipient); + let contact = format_contact(recipient); (format!("To {contact} @ {ts}"), body) } Msg::Received(Thread::Group(key), body) => { - let sender = format_contact(content.metadata.sender.uuid); + let sender = format_contact(&content.metadata.sender.uuid); let group = format_group(*key); (format!("From {sender} to group {group} @ {ts}: "), body) } @@ -571,7 +556,6 @@ async fn run(subcommand: Cmd, config_store: S) -> anyhow::Re send(&mut manager, Recipient::Group(master_key), data_message).await?; } - Cmd::Unregister => unimplemented!(), Cmd::RetrieveProfile { uuid, mut profile_key, @@ -599,11 +583,6 @@ async fn run(subcommand: Cmd, config_store: S) -> anyhow::Re }; println!("{profile:#?}"); } - Cmd::UpdateProfile => unimplemented!(), - Cmd::GetUserStatus => unimplemented!(), - Cmd::Block => unimplemented!(), - Cmd::Unblock => unimplemented!(), - Cmd::UpdateContact => unimplemented!(), Cmd::ListGroups => { let manager = Manager::load_registered(config_store).await?; for group in manager.store().groups()? { @@ -648,7 +627,7 @@ async fn run(subcommand: Cmd, config_store: S) -> anyhow::Re } Cmd::GetContact { ref uuid } => { let manager = Manager::load_registered(config_store).await?; - match manager.store().contact_by_id(*uuid)? { + match manager.store().contact_by_id(uuid)? { Some(contact) => println!("{contact:#?}"), None => eprintln!("Could not find contact for {uuid}"), } diff --git a/presage-store-sled/src/lib.rs b/presage-store-sled/src/lib.rs index 87cf26640..abfd8b9b0 100644 --- a/presage-store-sled/src/lib.rs +++ b/presage-store-sled/src/lib.rs @@ -428,14 +428,9 @@ impl ContentsStore for SledStore { Ok(()) } - fn save_contacts( - &mut self, - contacts: impl Iterator, - ) -> Result<(), SledStoreError> { - for contact in contacts { - self.insert(SLED_TREE_CONTACTS, contact.uuid, contact)?; - } - debug!("saved contacts"); + fn save_contact(&mut self, contact: &Contact) -> Result<(), SledStoreError> { + self.insert(SLED_TREE_CONTACTS, contact.uuid, contact)?; + debug!("saved contact"); Ok(()) } @@ -447,7 +442,7 @@ impl ContentsStore for SledStore { }) } - fn contact_by_id(&self, id: Uuid) -> Result, SledStoreError> { + fn contact_by_id(&self, id: &Uuid) -> Result, SledStoreError> { self.get(SLED_TREE_CONTACTS, id) } diff --git a/presage/src/manager/registered.rs b/presage/src/manager/registered.rs index 4227537bc..1af2c938b 100644 --- a/presage/src/manager/registered.rs +++ b/presage/src/manager/registered.rs @@ -13,10 +13,11 @@ use libsignal_service::messagepipe::{Incoming, MessagePipe, ServiceCredentials}; use libsignal_service::models::Contact; use libsignal_service::prelude::phonenumber::PhoneNumber; use libsignal_service::prelude::Uuid; +use libsignal_service::profile_cipher::ProfileCipher; use libsignal_service::proto::data_message::Delete; use libsignal_service::proto::{ sync_message, AttachmentPointer, DataMessage, EditMessage, GroupContextV2, NullMessage, - SyncMessage, + SyncMessage, Verified, }; use libsignal_service::protocol::SenderCertificate; use libsignal_service::protocol::{PrivateKey, PublicKey}; @@ -508,6 +509,7 @@ impl Manager { encrypted_messages: S, message_receiver: MessageReceiver, service_cipher: ServiceCipher, + push_service: HyperPushService, store: C, groups_manager: GroupsManager, mode: ReceivingMode, @@ -517,6 +519,7 @@ impl Manager { encrypted_messages: Box::pin(self.receive_messages_encrypted().await?), message_receiver: MessageReceiver::new(self.identified_push_service()), service_cipher: self.new_service_cipher()?, + push_service: self.identified_push_service(), store: self.store.clone(), groups_manager: self.groups_manager()?, mode, @@ -539,15 +542,11 @@ impl Manager { match state.message_receiver.retrieve_contacts(contacts).await { Ok(contacts) => { let _ = state.store.clear_contacts(); - match state - .store - .save_contacts(contacts.filter_map(Result::ok)) - { - Ok(()) => { - info!("saved contacts"); - } - Err(e) => { + info!("saving contacts"); + for contact in contacts.filter_map(Result::ok) { + if let Err(e) = state.store.save_contact(&contact) { warn!("failed to save contacts: {e}"); + break; } } } @@ -603,7 +602,13 @@ impl Manager { } } - if let Err(e) = save_message(&mut state.store, content.clone()) { + if let Err(e) = save_message( + &mut state.store, + &mut state.push_service, + content.clone(), + ) + .await + { error!("Error saving message to store: {}", e); } @@ -704,7 +709,8 @@ impl Manager { body: content_body, }; - save_message(&mut self.store, content)?; + let mut push_service = self.identified_push_service(); + save_message(&mut self.store, &mut push_service, content).await?; Ok(()) } @@ -802,7 +808,8 @@ impl Manager { body: content_body, }; - save_message(&mut self.store, content)?; + let mut push_service = self.identified_push_service(); + save_message(&mut self.store, &mut push_service, content).await?; Ok(()) } @@ -899,7 +906,7 @@ impl Manager { pub async fn thread_title(&self, thread: &Thread) -> Result> { match thread { Thread::Contact(uuid) => { - let contact = match self.store.contact_by_id(*uuid) { + let contact = match self.store.contact_by_id(uuid) { Ok(contact) => contact, Err(e) => { info!("Error getting contact by id: {}, {:?}", e, uuid); @@ -925,7 +932,7 @@ impl Manager { /// Note: this only currently works when linked as secondary device (the contacts are sent by the primary device at linking time) #[deprecated = "use the store handle directly"] pub fn contact_by_id(&self, id: &Uuid) -> Result, Error> { - Ok(self.store.contact_by_id(*id)?) + Ok(self.store.contact_by_id(id)?) } /// Returns an iterator on contacts stored in the [Store]. @@ -1010,52 +1017,110 @@ async fn upsert_group( Ok(store.group(master_key_bytes.try_into()?)?) } -fn save_message(store: &mut S, message: Content) -> Result<(), Error> { +async fn save_message( + store: &mut S, + push_service: &mut HyperPushService, + message: Content, +) -> Result<(), Error> { // derive the thread from the message type let thread = Thread::try_from(&message)?; // only save DataMessage and SynchronizeMessage (sent) let message = match message.body { ContentBody::NullMessage(_) => Some(message), - ContentBody::DataMessage(ref data_message) + ContentBody::DataMessage( + ref data_message @ DataMessage { + ref profile_key, .. + }, + ) | ContentBody::SynchronizeMessage(SyncMessage { sent: Some(sync_message::Sent { - message: Some(ref data_message), + message: + Some( + ref data_message @ DataMessage { + ref profile_key, .. + }, + ), .. }), .. - }) => match data_message { - DataMessage { - profile_key: Some(profile_key_bytes), - delete: - Some(Delete { - target_sent_timestamp: Some(ts), - }), - .. - } => { - // update recipient profile key - if let Ok(profile_key_bytes) = profile_key_bytes.clone().try_into() { - let sender_uuid = message.metadata.sender.uuid; - let profile_key = ProfileKey::create(profile_key_bytes); - debug!("inserting profile key for {sender_uuid}"); - store.upsert_profile_key(&sender_uuid, profile_key)?; + }) => { + // update recipient profile key if changed + if let Some(profile_key_bytes) = profile_key.clone().and_then(|p| p.try_into().ok()) { + let sender_uuid = message.metadata.sender.uuid; + let profile_key = ProfileKey::create(profile_key_bytes); + debug!("inserting profile key for {sender_uuid}"); + + // Either: + // - insert a new contact with the profile information + // - update the contact if the profile key has changed + // TODO: mark this contact as "created by us" maybe to know whether we should update it or not + if store.contact_by_id(&sender_uuid)?.is_none() + || !store + .profile_key(&sender_uuid)? + .is_some_and(|p| p.bytes == profile_key.bytes) + { + let encrypted_profile = push_service + .retrieve_profile_by_id(sender_uuid.into(), Some(profile_key)) + .await?; + let profile_cipher = ProfileCipher::from(profile_key); + let decrypted_profile = encrypted_profile.decrypt(profile_cipher).unwrap(); + + let contact = Contact { + uuid: sender_uuid, + phone_number: None, + name: decrypted_profile + .name + // FIXME: this assumes [firstname] [lastname] + .map(|pn| { + if let Some(family_name) = pn.family_name { + format!("{} {}", pn.given_name, family_name) + } else { + pn.given_name + } + }) + .unwrap_or_default(), + profile_key: profile_key.bytes.to_vec(), + color: None, + blocked: false, + expire_timer: 0, + inbox_position: 0, + archived: false, + avatar: None, + verified: Verified::default(), + }; + + info!("saved contact after seeing {sender_uuid} for the first time"); + store.save_contact(&contact)?; } - // replace an existing message by an empty NullMessage - if let Some(mut existing_msg) = store.message(&thread, *ts)? { - existing_msg.metadata.sender.uuid = Uuid::nil(); - existing_msg.body = NullMessage::default().into(); - store.save_message(&thread, existing_msg)?; - debug!("message in thread {thread} @ {ts} deleted"); - None - } else { - warn!("could not find message to delete in thread {thread} @ {ts}"); - None + store.upsert_profile_key(&sender_uuid, profile_key)?; + } + + match data_message { + DataMessage { + delete: + Some(Delete { + target_sent_timestamp: Some(ts), + }), + .. + } => { + // replace an existing message by an empty NullMessage + if let Some(mut existing_msg) = store.message(&thread, *ts)? { + existing_msg.metadata.sender.uuid = Uuid::nil(); + existing_msg.body = NullMessage::default().into(); + store.save_message(&thread, existing_msg)?; + debug!("message in thread {thread} @ {ts} deleted"); + None + } else { + warn!("could not find message to delete in thread {thread} @ {ts}"); + None + } } + _ => Some(message), } - _ => Some(message), - }, + } ContentBody::EditMessage(EditMessage { target_sent_timestamp: Some(ts), data_message: Some(data_message), @@ -1088,8 +1153,8 @@ fn save_message(store: &mut S, message: Content) -> Result<(), Error Some(message), - ContentBody::SynchronizeMessage(s) => { - debug!("skipping saving sync message without interesting fields: {s:?}"); + ContentBody::SynchronizeMessage(_) => { + debug!("skipping saving sync message without interesting fields"); None } ContentBody::ReceiptMessage(_) => { diff --git a/presage/src/store.rs b/presage/src/store.rs index a98a248ac..335d87d1e 100644 --- a/presage/src/store.rs +++ b/presage/src/store.rs @@ -162,7 +162,7 @@ pub trait ContentsStore { /// or [Group::disappearing_messages_timer]. fn expire_timer(&self, thread: &Thread) -> Result, Self::ContentsStoreError> { match thread { - Thread::Contact(uuid) => Ok(self.contact_by_id(*uuid)?.map(|c| c.expire_timer)), + Thread::Contact(uuid) => Ok(self.contact_by_id(uuid)?.map(|c| c.expire_timer)), Thread::Group(key) => Ok(self .group(*key)? .and_then(|g| g.disappearing_messages_timer) @@ -175,17 +175,14 @@ pub trait ContentsStore { /// Clear all saved synchronized contact data fn clear_contacts(&mut self) -> Result<(), Self::ContentsStoreError>; - /// Replace all contact data - fn save_contacts( - &mut self, - contacts: impl Iterator, - ) -> Result<(), Self::ContentsStoreError>; + /// Save a contact + fn save_contact(&mut self, contacts: &Contact) -> Result<(), Self::ContentsStoreError>; /// Get an iterator on all stored (synchronized) contacts fn contacts(&self) -> Result; /// Get contact data for a single user by its [Uuid]. - fn contact_by_id(&self, id: Uuid) -> Result, Self::ContentsStoreError>; + fn contact_by_id(&self, id: &Uuid) -> Result, Self::ContentsStoreError>; /// Delete all cached group data fn clear_groups(&mut self) -> Result<(), Self::ContentsStoreError>;