Merge pull request 'Fixes #1128 Some ActivityPub related fixes' (#1129) from ap-fixes into main

Reviewed-on: https://git.joinplu.me/Plume/Plume/pulls/1129

plume-common/src/activity_pub/inbox.rs

@@ -561,7 +561,7 @@ mod tests {
     use once_cell::sync::Lazy;
     use openssl::{hash::MessageDigest, pkey::PKey, rsa::Rsa};
 
-    static MY_SIGNER: Lazy<MySigner> = Lazy::new(|| MySigner::new());
+    static MY_SIGNER: Lazy<MySigner> = Lazy::new(MySigner::new);
 
     struct MySigner {
         public_key: String,
@@ -596,7 +596,7 @@ mod tests {
                 .unwrap();
             let mut verifier = openssl::sign::Verifier::new(MessageDigest::sha256(), &key).unwrap();
             verifier.update(data.as_bytes()).unwrap();
-            verifier.verify(&signature).map_err(|_| SignError())
+            verifier.verify(signature).map_err(|_| SignError())
         }
     }
 
@@ -782,7 +782,7 @@ mod tests {
             .done();
         assert!(res.is_err());
 
-        let res: Result<(), ()> = Inbox::handle(&(), act.clone())
+        let res: Result<(), ()> = Inbox::handle(&(), act)
             .with::<FailingActor, Create, MyObject>(None)
             .with::<MyActor, Create, MyObject>(None)
             .done();

plume-common/src/activity_pub/mod.rs

@@ -518,7 +518,8 @@ mod tests {
     use super::*;
     use activitystreams::{
         activity::{ActorAndObjectRef, Create},
-        object::kind::ArticleType,
+        object::{kind::ArticleType, Image},
+        prelude::{ApActorExt, BaseExt, ExtendsExt, ObjectExt},
     };
     use assert_json_diff::assert_json_eq;
     use serde_json::{from_str, json, to_value};
@@ -592,7 +593,7 @@ mod tests {
     }
 
     #[test]
-    fn de_custom_group() {
+    fn se_custom_group() {
         let group = CustomGroup::new(
             ApActor::new("https://example.com/inbox".parse().unwrap(), Group::new()),
             ApSignature {
@@ -625,6 +626,71 @@ mod tests {
         assert_eq!(to_value(group).unwrap(), expected);
     }
 
+    #[test]
+    fn de_custom_group() {
+        let value: CustomGroup = from_str(
+            r#"
+              {
+                "icon": {
+                  "type": "Image"
+                },
+                "id": "https://plume01.localhost/~/Plume01%20Blog%202/",
+                "image": {
+                  "type": "Image"
+                },
+                "inbox": "https://plume01.localhost/~/Plume01%20Blog%202/inbox",
+                "name": "Plume01 Blog 2",
+                "outbox": "https://plume01.localhost/~/Plume01%20Blog%202/outbox",
+                "preferredUsername": "Plume01 Blog 2",
+                "publicKey": {
+                  "id": "https://plume01.localhost/~/Plume01%20Blog%202/#main-key",
+                  "owner": "https://plume01.localhost/~/Plume01%20Blog%202/",
+                  "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwPGtKkl/iMsNAyeVaJGz\noEz5PoNkjRnKK7G97MFvb4zw9zs5SpzWW7b/pKHa4dODcGDJXmkCJ1H5JWyguzN8\n2GNoFjtEOJHxEGwBHSYDsTmhuLNB0DKxMU2iu55g8iIiXhZiIW1FBNGs/Geaymvr\nh/TEtzdReN8wzloRR55kOVcU49xBkqx8cfDSk/lrrDLlpveHdqgaFnIvuw2vycK0\nxFzS3xlEUpzJk9kHxoR1uEAfZ+gCv26Sgo/HqOAhqSD5IU3QZC3kdkr/hwVqtr8U\nXGkGG6Mo1rgzhkYiCFkWrV2WoKkcEHD4nEzbgoZZ5MyuSoloxnyF3NiScqmqW+Yx\nkQIDAQAB\n-----END PUBLIC KEY-----\n"
+                },
+                "source": {
+                  "content": "",
+                  "mediaType": "text/markdown"
+                },
+                "summary": "",
+                "type": "Group"
+              }
+            "#
+        ).unwrap();
+        let mut expected = CustomGroup::new(
+            ApActor::new("https://plume01.localhost/~/Plume01%20Blog%202/inbox".parse().unwrap(), Group::new()),
+            ApSignature {
+                public_key: PublicKey {
+                    id: "https://plume01.localhost/~/Plume01%20Blog%202/#main-key".parse().unwrap(),
+                    owner: "https://plume01.localhost/~/Plume01%20Blog%202/".parse().unwrap(),
+                    public_key_pem: "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwPGtKkl/iMsNAyeVaJGz\noEz5PoNkjRnKK7G97MFvb4zw9zs5SpzWW7b/pKHa4dODcGDJXmkCJ1H5JWyguzN8\n2GNoFjtEOJHxEGwBHSYDsTmhuLNB0DKxMU2iu55g8iIiXhZiIW1FBNGs/Geaymvr\nh/TEtzdReN8wzloRR55kOVcU49xBkqx8cfDSk/lrrDLlpveHdqgaFnIvuw2vycK0\nxFzS3xlEUpzJk9kHxoR1uEAfZ+gCv26Sgo/HqOAhqSD5IU3QZC3kdkr/hwVqtr8U\nXGkGG6Mo1rgzhkYiCFkWrV2WoKkcEHD4nEzbgoZZ5MyuSoloxnyF3NiScqmqW+Yx\nkQIDAQAB\n-----END PUBLIC KEY-----\n".into(),
+                }
+            },
+            SourceProperty {
+                source: Source {
+                    content: String::from(""),
+                    media_type: String::from("text/markdown")
+                }
+            }
+        );
+        expected.set_icon(Image::new().into_any_base().unwrap());
+        expected.set_id(
+            "https://plume01.localhost/~/Plume01%20Blog%202/"
+                .parse()
+                .unwrap(),
+        );
+        expected.set_image(Image::new().into_any_base().unwrap());
+        expected.set_name("Plume01 Blog 2");
+        expected.set_outbox(
+            "https://plume01.localhost/~/Plume01%20Blog%202/outbox"
+                .parse()
+                .unwrap(),
+        );
+        expected.set_preferred_username("Plume01 Blog 2");
+        expected.set_summary("");
+
+        assert_json_eq!(value, expected);
+    }
+
     #[test]
     fn se_licensed_article() {
         let object = ApObject::new(Article::new());

plume-common/src/activity_pub/request.rs

@@ -253,7 +253,7 @@ mod tests {
                 .unwrap();
             let mut verifier = openssl::sign::Verifier::new(MessageDigest::sha256(), &key).unwrap();
             verifier.update(data.as_bytes()).unwrap();
-            verifier.verify(&signature).map_err(|_| Error())
+            verifier.verify(signature).map_err(|_| Error())
         }
     }
 
@@ -262,7 +262,7 @@ mod tests {
         let signer = MySigner::new();
         let headers = HeaderMap::new();
         let result = signature(&signer, &headers, ("post", "/inbox", None)).unwrap();
-        let fields: Vec<&str> = result.to_str().unwrap().split(",").collect();
+        let fields: Vec<&str> = result.to_str().unwrap().split(',').collect();
         assert_eq!(r#"headers="(request-target)""#, fields[2]);
         let sign = &fields[3][11..(fields[3].len() - 1)];
         assert!(signer.verify("post /inbox", sign.as_bytes()).is_ok());

plume-models/src/blogs.rs

@@ -18,10 +18,13 @@ use openssl::{
     rsa::Rsa,
     sign::{Signer, Verifier},
 };
-use plume_common::activity_pub::{
+use plume_common::{
-    inbox::{AsActor, FromId},
+    activity_pub::{
-    sign, ActivityStream, ApSignature, CustomGroup, Id, IntoId, PublicKey, Source, SourceProperty,
+        inbox::{AsActor, FromId},
-    ToAsString, ToAsUri,
+        sign, ActivityStream, ApSignature, CustomGroup, Id, IntoId, PublicKey, Source,
+        SourceProperty, ToAsString, ToAsUri,
+    },
+    utils::iri_percent_encode_seg,
 };
 use webfinger::*;
 
@@ -83,9 +86,13 @@ impl Blog {
 
         if inserted.fqn.is_empty() {
             if instance.local {
-                inserted.fqn = inserted.actor_id.clone();
+                inserted.fqn = iri_percent_encode_seg(&inserted.actor_id.clone());
             } else {
-                inserted.fqn = format!("{}@{}", inserted.actor_id, instance.public_domain);
+                inserted.fqn = format!(
+                    "{}@{}",
+                    iri_percent_encode_seg(&inserted.actor_id),
+                    instance.public_domain
+                );
             }
         }
 
@@ -166,7 +173,7 @@ impl Blog {
 
     pub fn to_activity(&self, conn: &Connection) -> Result<CustomGroup> {
         let mut blog = ApActor::new(self.inbox_url.parse()?, Group::new());
-        blog.set_preferred_username(self.actor_id.clone());
+        blog.set_preferred_username(iri_percent_encode_seg(&self.actor_id));
         blog.set_name(self.title.clone());
         blog.set_outbox(self.outbox_url.parse()?);
         blog.set_summary(self.summary_html.to_string());
@@ -381,6 +388,7 @@ impl FromId<DbConn> for Blog {
                 .ok_or(Error::MissingApProperty)?
                 .to_string();
             if name.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\t'][..]) {
+                tracing::error!("preferredUsername includes invalid character(s): {}", &name);
                 return Err(Error::InvalidValue);
             }
             (

plume-models/src/instance.rs

@@ -9,7 +9,7 @@ use crate::{
 use chrono::NaiveDateTime;
 use diesel::{self, result::Error::NotFound, ExpressionMethods, QueryDsl, RunQueryDsl};
 use once_cell::sync::OnceCell;
-use plume_common::utils::{md_to_html, iri_percent_encode_seg};
+use plume_common::utils::{iri_percent_encode_seg, md_to_html};
 use std::sync::RwLock;
 
 #[derive(Clone, Identifiable, Queryable)]

plume-models/src/remote_fetch_actor.rs

@@ -45,7 +45,10 @@ impl Actor for RemoteFetchActor {
             RemoteUserFound(user) => match self.conn.get() {
                 Ok(conn) => {
                     let conn = DbConn(conn);
-                    if user.get_instance(&conn).map_or(false, |instance| instance.blocked) {
+                    if user
+                        .get_instance(&conn)
+                        .map_or(false, |instance| instance.blocked)
+                    {
                         return;
                     }
                     // Don't call these functions in parallel

plume-models/src/users.rs

@@ -246,20 +246,7 @@ impl User {
     fn fetch(url: &str) -> Result<CustomPerson> {
         let res = get(url, Self::get_sender(), CONFIG.proxy().cloned())?;
         let text = &res.text()?;
-        // without this workaround, publicKey is not correctly deserialized
+        let json = serde_json::from_str::<CustomPerson>(text)?;
-        let ap_sign = serde_json::from_str::<ApSignature>(text)?;
-        let person = serde_json::from_str::<Person>(text)?;
-        let json = CustomPerson::new(
-            ApActor::new(
-                person
-                    .clone()
-                    .id_unchecked()
-                    .ok_or(Error::MissingApProperty)?
-                    .to_owned(),
-                person,
-            ),
-            ap_sign,
-        ); // FIXME: Don't clone()
         Ok(json)
     }
 
@@ -269,23 +256,13 @@ impl User {
 
     pub fn refetch(&self, conn: &Connection) -> Result<()> {
         User::fetch(&self.ap_url.clone()).and_then(|json| {
-            let avatar = Media::save_remote(
+            let avatar = json
-                conn,
+                .icon()
-                json.ap_actor_ref()
+                .and_then(|icon| icon.iter().next())
-                    .icon()
+                .and_then(|i| i.clone().extend::<Image, ImageType>().ok())
-                    .ok_or(Error::MissingApProperty)? // FIXME: Fails when icon is not set
+                .and_then(|image| image)
-                    .iter()
+                .and_then(|image| image.id_unchecked().map(|url| url.to_string()))
-                    .next()
+                .and_then(|url| Media::save_remote(conn, url, self).ok());
-                    .and_then(|i| {
-                        i.clone()
-                            .extend::<Image, ImageType>() // FIXME: Don't clone()
-                            .ok()?
-                            .and_then(|url| Some(url.id_unchecked()?.to_string()))
-                    })
-                    .ok_or(Error::MissingApProperty)?,
-                self,
-            )
-            .ok();
 
             let pub_key = &json.ext_one.public_key.public_key_pem;
             diesel::update(self)
@@ -960,6 +937,10 @@ impl FromId<DbConn> for User {
             .to_string();
 
         if username.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\t'][..]) {
+            tracing::error!(
+                "preferredUsername includes invalid character(s): {}",
+                &username
+            );
             return Err(Error::InvalidValue);
         }