Skip to content

Commit

Permalink
Implement profile avatar retention
Browse files Browse the repository at this point in the history
  • Loading branch information
rubdos committed Nov 11, 2022
1 parent 0e851d8 commit 2c65081
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
20 changes: 14 additions & 6 deletions libsignal-service/src/account_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use serde::{Deserialize, Serialize};
use sha2::Sha256;
use zkgroup::profiles::ProfileKey;

use crate::push_service::AvatarWrite;
use crate::ServiceAddress;
use crate::{
configuration::{Endpoint, ServiceCredentials},
Expand Down Expand Up @@ -292,21 +293,28 @@ impl<Service: PushService> AccountManager<Service> {
///
/// Convenience method for
/// ```ignore
/// manager.upload_versioned_profile::<std::io::Cursor<Vec<u8>>, _>(uuid, name, about, about_emoji, None)
/// manager.upload_versioned_profile::<std::io::Cursor<Vec<u8>>, _>(uuid, name, about, about_emoji, _)
/// ```
/// in which the `retain_avatar` parameter sets whether to remove (`false`) or retain (`true`) the
/// currently set avatar.
pub async fn upload_versioned_profile_without_avatar<S: AsRef<str>>(
&mut self,
uuid: uuid::Uuid,
name: ProfileName<S>,
about: Option<String>,
about_emoji: Option<String>,
retain_avatar: bool,
) -> Result<(), ProfileManagerError> {
self.upload_versioned_profile::<std::io::Cursor<Vec<u8>>, _>(
uuid,
name,
about,
about_emoji,
None,
if retain_avatar {
AvatarWrite::RetainAvatar
} else {
AvatarWrite::NoAvatar
},
)
.await?;
Ok(())
Expand Down Expand Up @@ -344,7 +352,7 @@ impl<Service: PushService> AccountManager<Service> {
name: ProfileName<S>,
about: Option<String>,
about_emoji: Option<String>,
avatar: Option<&'s mut C>,
avatar: AvatarWrite<&'s mut C>,
) -> Result<Option<String>, ProfileManagerError> {
let profile_key =
self.profile_key.expect("set profile key in AccountManager");
Expand All @@ -359,7 +367,7 @@ impl<Service: PushService> AccountManager<Service> {
let about_emoji = profile_cipher.encrypt_emoji(about_emoji)?;

// If avatar -> upload
if let Some(_avatar) = avatar {
if matches!(avatar, AvatarWrite::NewAvatar(_)) {
// FIXME ProfileCipherOutputStream.java
// It's just AES GCM, but a bit of work to decently implement it with a stream.
unimplemented!("Setting avatar requires ProfileCipherStream")
Expand All @@ -372,13 +380,13 @@ impl<Service: PushService> AccountManager<Service> {

Ok(self
.service
.write_profile(
.write_profile::<C, S>(
&profile_key_version,
&name,
&about,
&about_emoji,
&commitment,
None, // FIXME avatar
avatar,
)
.await?)
}
Expand Down
25 changes: 18 additions & 7 deletions libsignal-service/src/push_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ pub enum HttpAuthOverride {
Identified(HttpAuth),
}

#[derive(Debug, Clone, Eq, PartialEq)]
pub enum AvatarWrite<C> {
NewAvatar(C),
RetainAvatar,
NoAvatar,
}

impl fmt::Debug for HttpAuth {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "HTTP auth with username {}", self.username)
Expand Down Expand Up @@ -766,15 +773,14 @@ pub trait PushService: MaybeSend {
/// See [`AccountManager`][struct@crate::AccountManager] for a convenience method.
///
/// Java equivalent: `writeProfile`
async fn write_profile(
async fn write_profile<'s, C: std::io::Read + Send + 's, S: AsRef<str>>(
&mut self,
version: &ProfileKeyVersion,
name: &[u8],
about: &[u8],
emoji: &[u8],
commitment: &ProfileKeyCommitment,
// FIXME cfr also account manager
avatar: Option<()>,
avatar: AvatarWrite<&mut C>,
) -> Result<Option<String>, ServiceError> {
#[derive(Debug, Serialize)]
#[serde(rename_all = "camelCase")]
Expand All @@ -788,6 +794,7 @@ pub trait PushService: MaybeSend {
#[serde(with = "serde_base64")]
about_emoji: &'s [u8],
avatar: bool,
same_avatar: bool,
#[serde(with = "serde_base64")]
commitment: &'s [u8],
}
Expand All @@ -803,7 +810,8 @@ pub trait PushService: MaybeSend {
name,
about,
about_emoji: emoji,
avatar: avatar.is_some(),
avatar: matches!(avatar, AvatarWrite::NewAvatar(_)),
same_avatar: matches!(avatar, AvatarWrite::RetainAvatar),
commitment: &commitment,
};

Expand All @@ -817,19 +825,22 @@ pub trait PushService: MaybeSend {
)
.await;
match (response, avatar) {
(Ok(_url), Some(_avatar)) => {
(Ok(_url), AvatarWrite::NewAvatar(_avatar)) => {
// FIXME
unreachable!("Uploading avatar unimplemented");
},
// FIXME cleanup when #54883 is stable and MSRV:
// or-patterns syntax is experimental
// see issue #54883 <https://github.com/rust-lang/rust/issues/54883> for more information
(Err(ServiceError::JsonDecodeError { .. }), None) => {
(
Err(ServiceError::JsonDecodeError { .. }),
AvatarWrite::RetainAvatar | AvatarWrite::NoAvatar,
) => {
// OWS sends an empty string when there's no attachment
Ok(None)
},
(Err(e), _) => Err(e),
(Ok(_resp), None) => {
(Ok(_resp), AvatarWrite::RetainAvatar | AvatarWrite::NoAvatar) => {
log::warn!(
"No avatar supplied but got avatar upload URL. Ignoring"
);
Expand Down

0 comments on commit 2c65081

Please sign in to comment.