Address review comments

libsignal-service/src/cipher.rs

@@ -414,12 +414,12 @@ pub async fn get_preferred_protocol_address<S: SessionStore>(
     address: &ServiceAddress,
     device_id: DeviceId,
 ) -> Result<ProtocolAddress, libsignal_protocol::error::SignalProtocolError> {
-    let address = ProtocolAddress::new(address.to_string(), device_id);
+    let address = address.to_protocol_address(device_id);
     if session_store.load_session(&address, None).await?.is_some() {
         return Ok(address);
     }
 
-    Ok(ProtocolAddress::new(address.to_string(), device_id))
+    Ok(address)
 }
 
 /// Decrypt a Sealed Sender message `ciphertext` in either the v1 or v2 format, validate its sender

libsignal-service/src/content.rs

@@ -22,7 +22,7 @@ pub struct Metadata {
 
 impl Metadata {
     pub(crate) fn protocol_address(&self) -> ProtocolAddress {
-        ProtocolAddress::new(self.sender.to_string(), self.sender_device.into())
+        self.sender.to_protocol_address(self.sender_device)
     }
 }
 

libsignal-service/src/envelope.rs

@@ -121,7 +121,7 @@ impl Envelope {
     }
 
     pub fn source_address(&self) -> ServiceAddress {
-        let uuid: Uuid = self
+        let uuid = self
             .source_uuid
             .as_deref()
             .and_then(|u| Uuid::parse_str(u).ok())

libsignal-service/src/profile_service.rs

@@ -19,19 +19,18 @@ impl ProfileService {
         address: ServiceAddress,
         profile_key: Option<zkgroup::profiles::ProfileKey>,
     ) -> Result<SignalServiceProfile, ServiceError> {
-        let endpoint = match (profile_key, address.uuid) {
-            (Some(key), uuid) => {
-                let uid_bytes = uuid.as_bytes();
+        let endpoint = match profile_key {
+            Some(key) => {
+                let uid_bytes = address.uuid.as_bytes();
                 let version = bincode::serialize(
                     &key.get_profile_key_version(*uid_bytes),
                 )?;
                 let version = std::str::from_utf8(&version)
                     .expect("hex encoded profile key version");
-                format!("/v1/profile/{}/{}", uuid, version)
+                format!("/v1/profile/{}/{}", address.uuid, version)
             },
-            (_, uuid) => {
-                // TODO: check if this is even possible?
-                format!("/v1/profile/{}", uuid)
+            None => {
+                format!("/v1/profile/{}", address.uuid)
             },
         };
 

libsignal-service/src/push_service.rs

@@ -580,7 +580,7 @@ pub trait PushService: MaybeSend {
         &mut self,
         messages: OutgoingPushMessages,
     ) -> Result<SendMessageResponse, ServiceError> {
-        let path = format!("/v1/messages/{}", messages.recipient);
+        let path = format!("/v1/messages/{}", messages.recipient.uuid);
         self.put_json(
             Endpoint::Service,
             &path,
@@ -668,9 +668,9 @@ pub trait PushService: MaybeSend {
                 bincode::serialize(&key.get_profile_key_version(*uid_bytes))?;
             let version = std::str::from_utf8(&version)
                 .expect("hex encoded profile key version");
-            format!("/v1/profile/{}/{}", address, version)
+            format!("/v1/profile/{}/{}", address.uuid, version)
         } else {
-            format!("/v1/profile/{}", address)
+            format!("/v1/profile/{}", address.uuid)
         };
         // TODO: set locale to en_US
         self.get_json(
@@ -700,7 +700,7 @@ pub trait PushService: MaybeSend {
         destination: &ServiceAddress,
         device_id: u32,
     ) -> Result<PreKeyBundle, ServiceError> {
-        let path = format!("/v2/keys/{}/{}", destination, device_id);
+        let path = format!("/v2/keys/{}/{}", destination.uuid, device_id);
 
         let mut pre_key_response: PreKeyResponse = self
             .get_json(Endpoint::Service, &path, HttpAuthOverride::NoOverride)
@@ -718,9 +718,9 @@ pub trait PushService: MaybeSend {
         device_id: u32,
     ) -> Result<Vec<PreKeyBundle>, ServiceError> {
         let path = if device_id == 1 {
-            format!("/v2/keys/{}/*", destination)
+            format!("/v2/keys/{}/*", destination.uuid)
         } else {
-            format!("/v2/keys/{}/{}", destination, device_id)
+            format!("/v2/keys/{}/{}", destination.uuid, device_id)
         };
         let pre_key_response: PreKeyResponse = self
             .get_json(Endpoint::Service, &path, HttpAuthOverride::NoOverride)

libsignal-service/src/sender.rs

@@ -3,8 +3,8 @@ use std::time::SystemTime;
 use chrono::prelude::*;
 use libsignal_protocol::{
     process_prekey_bundle, DeviceId, IdentityKeyStore, PreKeyStore,
-    ProtocolAddress, SenderCertificate, SenderKeyStore, SessionStore,
-    SignalProtocolError, SignedPreKeyStore,
+    SenderCertificate, SenderKeyStore, SessionStore, SignalProtocolError,
+    SignedPreKeyStore,
 };
 use log::{info, trace};
 use rand::{CryptoRng, Rng};
@@ -104,7 +104,7 @@ pub enum MessageSenderError {
     #[error("Failed to upload attachment {0}")]
     AttachmentUploadError(#[from] AttachmentUploadError),
 
-    #[error("Untrusted identity key with {address}")]
+    #[error("Untrusted identity key with {address:?}")]
     UntrustedIdentity { address: ServiceAddress },
 
     #[error("Exceeded maximum number of retries")]
@@ -359,7 +359,7 @@ where
         }
 
         if end_session {
-            log::debug!("ending session with {}", recipient);
+            log::debug!("ending session with {}", recipient.uuid);
             self.session_store
                 .delete_all_sessions(&recipient.uuid.to_string())
                 .await?;
@@ -495,10 +495,8 @@ where
                             "creating session with missing device {}",
                             missing_device_id
                         );
-                        let remote_address = ProtocolAddress::new(
-                            recipient.to_string(),
-                            (*missing_device_id).into(),
-                        );
+                        let remote_address =
+                            recipient.to_protocol_address(*missing_device_id);
                         let pre_key = self
                             .service
                             .get_pre_key(&recipient, *missing_device_id)
@@ -770,14 +768,14 @@ where
                         ..
                     } = sent;
                     UnidentifiedDeliveryStatus {
-                        destination_uuid: Some(recipient.to_string()),
+                        destination_uuid: Some(recipient.uuid.to_string()),
                         unidentified: Some(*unidentified),
                     }
                 })
                 .collect();
         ContentBody::SynchronizeMessage(SyncMessage {
             sent: Some(sync_message::Sent {
-                destination_uuid: recipient.map(|r| r.to_string()),
+                destination_uuid: recipient.map(|r| r.uuid.to_string()),
                 destination_e164: None,
                 message: data_message,
                 timestamp: Some(timestamp),

libsignal-service/src/service_address.rs

@@ -1,5 +1,6 @@
-use std::{convert::TryFrom, fmt};
+use std::convert::TryFrom;
 
+use libsignal_protocol::{DeviceId, ProtocolAddress};
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
@@ -8,7 +9,7 @@ pub enum ParseServiceAddressError {
     #[error("Supplied UUID could not be parsed")]
     InvalidUuid(#[from] uuid::Error),
 
-    #[error("Envelope with no Uuid")]
+    #[error("Envelope without UUID")]
     NoUuid,
 }
 
@@ -17,9 +18,12 @@ pub struct ServiceAddress {
     pub uuid: Uuid,
 }
 
-impl fmt::Display for ServiceAddress {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.uuid)
+impl ServiceAddress {
+    pub fn to_protocol_address(
+        &self,
+        device_id: impl Into<DeviceId>,
+    ) -> ProtocolAddress {
+        ProtocolAddress::new(self.uuid.to_string(), device_id.into())
     }
 }
 

libsignal-service/src/session_store.rs

@@ -41,10 +41,7 @@ pub trait SessionStoreExt: SessionStore {
     ) -> Result<usize, SignalProtocolError> {
         let mut count = 0;
         match self
-            .delete_session(&ProtocolAddress::new(
-                address.to_string(),
-                device_id.into(),
-            ))
+            .delete_session(&address.to_protocol_address(device_id))
             .await
         {
             Ok(()) => {

libsignal-service/src/websocket/sender.rs

@@ -7,7 +7,7 @@ impl SignalWebSocket {
         &mut self,
         messages: OutgoingPushMessages,
     ) -> Result<SendMessageResponse, ServiceError> {
-        let path = format!("/v1/messages/{}", messages.recipient);
+        let path = format!("/v1/messages/{}", messages.recipient.uuid);
         self.put_json(&path, messages).await
     }
 }