From 9a3699160d695713eb5160f74faa78988ab98be0 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 01:16:13 +0900 Subject: [PATCH 01/12] Fix test name --- plume-common/src/activity_pub/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plume-common/src/activity_pub/mod.rs b/plume-common/src/activity_pub/mod.rs index fc5c7bd7..6d93cdba 100644 --- a/plume-common/src/activity_pub/mod.rs +++ b/plume-common/src/activity_pub/mod.rs @@ -592,7 +592,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 { From d36f13e9849fc6b4f251cba812b00ce81824cea7 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:19:10 +0900 Subject: [PATCH 02/12] Add test for deserializing CustomGroup --- plume-common/src/activity_pub/mod.rs | 68 +++++++++++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/plume-common/src/activity_pub/mod.rs b/plume-common/src/activity_pub/mod.rs index 6d93cdba..34637399 100644 --- a/plume-common/src/activity_pub/mod.rs +++ b/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}; @@ -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()); From 399af4004a3bf034573f1d35e838ebe62521fe20 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:20:25 +0900 Subject: [PATCH 03/12] Build CustomPerson from source string at once --- plume-models/src/users.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 0314bf0b..200260fe 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -246,20 +246,7 @@ impl User { fn fetch(url: &str) -> Result { let res = get(url, Self::get_sender(), CONFIG.proxy().cloned())?; let text = &res.text()?; - // without this workaround, publicKey is not correctly deserialized - let ap_sign = serde_json::from_str::(text)?; - let person = serde_json::from_str::(text)?; - let json = CustomPerson::new( - ApActor::new( - person - .clone() - .id_unchecked() - .ok_or(Error::MissingApProperty)? - .to_owned(), - person, - ), - ap_sign, - ); // FIXME: Don't clone() + let json = serde_json::from_str::(text)?; Ok(json) } From f138ae6ed9702f5905482b12210598bc93fdbd3f Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:20:57 +0900 Subject: [PATCH 04/12] Allow empty avatar for remote users --- plume-models/src/users.rs | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 200260fe..3908739c 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -256,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( - conn, - json.ap_actor_ref() - .icon() - .ok_or(Error::MissingApProperty)? // FIXME: Fails when icon is not set - .iter() - .next() - .and_then(|i| { - i.clone() - .extend::() // FIXME: Don't clone() - .ok()? - .and_then(|url| Some(url.id_unchecked()?.to_string())) - }) - .ok_or(Error::MissingApProperty)?, - self, - ) - .ok(); + let avatar = json + .icon() + .and_then(|icon| icon.iter().next()) + .and_then(|i| i.clone().extend::().ok()) + .and_then(|image| image) + .and_then(|image| image.id_unchecked().map(|url| url.to_string())) + .and_then(|url| Media::save_remote(conn, url, self).ok()); let pub_key = &json.ext_one.public_key.public_key_pem; diesel::update(self) From 85cacf4239178215b2085f46f7b1c0b745703d21 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:23:50 +0900 Subject: [PATCH 05/12] Format --- plume-models/src/instance.rs | 2 +- plume-models/src/remote_fetch_actor.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plume-models/src/instance.rs b/plume-models/src/instance.rs index 5f8e09af..3da8cbb8 100644 --- a/plume-models/src/instance.rs +++ b/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)] diff --git a/plume-models/src/remote_fetch_actor.rs b/plume-models/src/remote_fetch_actor.rs index 365a0180..ea0ecc2d 100644 --- a/plume-models/src/remote_fetch_actor.rs +++ b/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 From e746a0b03f2223d8ac0357a78a1a5fe6a0d221ad Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:25:58 +0900 Subject: [PATCH 06/12] Add error log for invalid preferredUsername --- plume-models/src/blogs.rs | 1 + plume-models/src/users.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs index 489589a1..0458895c 100644 --- a/plume-models/src/blogs.rs +++ b/plume-models/src/blogs.rs @@ -381,6 +381,7 @@ impl FromId for Blog { .ok_or(Error::MissingApProperty)? .to_string(); if name.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\t'][..]) { + tracing::error!("preferredUsername includes invalid character(s): {}", &name); return Err(Error::InvalidValue); } ( diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 3908739c..8dbd2712 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -937,6 +937,7 @@ impl FromId for User { .to_string(); if username.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\t'][..]) { + tracing::error!("preferredUsername includes invalid character(s): {}", &username); return Err(Error::InvalidValue); } From 2d10ddb9fa524a73d49d231e70594ff3d8651f72 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:33:29 +0900 Subject: [PATCH 07/12] Clippy --- plume-common/src/activity_pub/inbox.rs | 6 +++--- plume-common/src/activity_pub/request.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plume-common/src/activity_pub/inbox.rs b/plume-common/src/activity_pub/inbox.rs index e2a769ee..e8d36ccb 100644 --- a/plume-common/src/activity_pub/inbox.rs +++ b/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 = Lazy::new(|| MySigner::new()); + static MY_SIGNER: Lazy = 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::(None) .with::(None) .done(); diff --git a/plume-common/src/activity_pub/request.rs b/plume-common/src/activity_pub/request.rs index 3d60b820..4d5a4097 100644 --- a/plume-common/src/activity_pub/request.rs +++ b/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()); From 9776374d17822d5a4bb43e833a1830310cbbfecc Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 02:46:07 +0900 Subject: [PATCH 08/12] Rejectd illegal characters from blog name --- src/routes/blogs.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/routes/blogs.rs b/src/routes/blogs.rs index 295d08a8..40150ca3 100644 --- a/src/routes/blogs.rs +++ b/src/routes/blogs.rs @@ -82,6 +82,8 @@ fn valid_slug(title: &str) -> Result<(), ValidationError> { let slug = Blog::slug(title); if slug.is_empty() { Err(ValidationError::new("empty_slug")) + } else if slug.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\n', '\t'][..]) { + Err(ValidationError::new("slug_illegal_char")) } else { Ok(()) } From d741238ccb966ff69c8aeeaa7c7538b14844102a Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 03:11:20 +0900 Subject: [PATCH 09/12] Revert "Rejectd illegal characters from blog name" This reverts commit 9776374d17822d5a4bb43e833a1830310cbbfecc. --- src/routes/blogs.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/routes/blogs.rs b/src/routes/blogs.rs index 40150ca3..295d08a8 100644 --- a/src/routes/blogs.rs +++ b/src/routes/blogs.rs @@ -82,8 +82,6 @@ fn valid_slug(title: &str) -> Result<(), ValidationError> { let slug = Blog::slug(title); if slug.is_empty() { Err(ValidationError::new("empty_slug")) - } else if slug.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\n', '\t'][..]) { - Err(ValidationError::new("slug_illegal_char")) } else { Ok(()) } From 704e9aa47fb46a27ec6c7fe0be4fa43f09baf81c Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 04:13:40 +0900 Subject: [PATCH 10/12] Make blogs.fqn valid --- plume-models/src/blogs.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs index 0458895c..53f3c76e 100644 --- a/plume-models/src/blogs.rs +++ b/plume-models/src/blogs.rs @@ -18,11 +18,11 @@ use openssl::{ rsa::Rsa, sign::{Signer, Verifier}, }; -use plume_common::activity_pub::{ +use plume_common::{activity_pub::{ inbox::{AsActor, FromId}, sign, ActivityStream, ApSignature, CustomGroup, Id, IntoId, PublicKey, Source, SourceProperty, ToAsString, ToAsUri, -}; +}, utils::iri_percent_encode_seg}; use webfinger::*; #[derive(Queryable, Identifiable, Clone, AsChangeset, Debug)] @@ -83,9 +83,9 @@ 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); } } From 699fdc30d96e14e45c58d4ad29bd1d81680967ee Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 04:13:52 +0900 Subject: [PATCH 11/12] Make preferredUsername of blogs valid --- plume-models/src/blogs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs index 53f3c76e..a0ec276a 100644 --- a/plume-models/src/blogs.rs +++ b/plume-models/src/blogs.rs @@ -166,7 +166,7 @@ impl Blog { pub fn to_activity(&self, conn: &Connection) -> Result { 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()); From 3580fb04fa65505b6cb2db119ad0903ec7683140 Mon Sep 17 00:00:00 2001 From: Kitaiti Makoto Date: Thu, 5 Jan 2023 04:14:25 +0900 Subject: [PATCH 12/12] Format --- plume-models/src/blogs.rs | 19 +++++++++++++------ plume-models/src/users.rs | 5 ++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs index a0ec276a..fada72f8 100644 --- a/plume-models/src/blogs.rs +++ b/plume-models/src/blogs.rs @@ -18,11 +18,14 @@ use openssl::{ rsa::Rsa, sign::{Signer, Verifier}, }; -use plume_common::{activity_pub::{ - inbox::{AsActor, FromId}, - sign, ActivityStream, ApSignature, CustomGroup, Id, IntoId, PublicKey, Source, SourceProperty, - ToAsString, ToAsUri, -}, utils::iri_percent_encode_seg}; +use plume_common::{ + activity_pub::{ + inbox::{AsActor, FromId}, + sign, ActivityStream, ApSignature, CustomGroup, Id, IntoId, PublicKey, Source, + SourceProperty, ToAsString, ToAsUri, + }, + utils::iri_percent_encode_seg, +}; use webfinger::*; #[derive(Queryable, Identifiable, Clone, AsChangeset, Debug)] @@ -85,7 +88,11 @@ impl Blog { if instance.local { inserted.fqn = iri_percent_encode_seg(&inserted.actor_id.clone()); } else { - inserted.fqn = format!("{}@{}", iri_percent_encode_seg(&inserted.actor_id), instance.public_domain); + inserted.fqn = format!( + "{}@{}", + iri_percent_encode_seg(&inserted.actor_id), + instance.public_domain + ); } } diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs index 8dbd2712..0d2627d4 100644 --- a/plume-models/src/users.rs +++ b/plume-models/src/users.rs @@ -937,7 +937,10 @@ impl FromId for User { .to_string(); if username.contains(&['<', '>', '&', '@', '\'', '"', ' ', '\t'][..]) { - tracing::error!("preferredUsername includes invalid character(s): {}", &username); + tracing::error!( + "preferredUsername includes invalid character(s): {}", + &username + ); return Err(Error::InvalidValue); }