From 876cf19327bd8bdd4449f818cce44559c4d3a9ff Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 14:43:48 +0200 Subject: [PATCH 1/7] chore: instrument inbox/outbox this is where side effects happen, so better keep them under control also trying out tracing, i should redo how i trace stuff in upub... --- upub/core/src/server/auth.rs | 5 +++-- upub/routes/src/activitypub/inbox.rs | 1 + upub/routes/src/activitypub/user/outbox.rs | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/upub/core/src/server/auth.rs b/upub/core/src/server/auth.rs index 1516bbc0..226dd964 100644 --- a/upub/core/src/server/auth.rs +++ b/upub/core/src/server/auth.rs @@ -10,6 +10,7 @@ use super::{fetcher::Fetcher, httpsign::HttpSignature}; pub enum Identity { Anonymous, Remote { + user: String, domain: String, internal: i64, }, @@ -124,11 +125,11 @@ where .verify(&user.public_key) { Ok(true) => { - // TODO can we avoid this extra db rountrip made on each server fetch? + let user = user.id; let domain = Context::server(&user_id); // TODO this will fail because we never fetch and insert into instance oops let internal = model::instance::Entity::domain_to_internal(&domain, ctx.db()).await?; - identity = Identity::Remote { domain, internal }; + identity = Identity::Remote { user, domain, internal }; }, Ok(false) => tracing::warn!("invalid signature: {http_signature:?}"), Err(e) => tracing::error!("error verifying signature: {e}"), diff --git a/upub/routes/src/activitypub/inbox.rs b/upub/routes/src/activitypub/inbox.rs index f3365f38..26135795 100644 --- a/upub/routes/src/activitypub/inbox.rs +++ b/upub/routes/src/activitypub/inbox.rs @@ -38,6 +38,7 @@ macro_rules! pretty_json { } +#[tracing::instrument(level = "info", skip(ctx), fields(activity = %activity))] pub async fn post( State(ctx): State, AuthIdentity(auth): AuthIdentity, diff --git a/upub/routes/src/activitypub/user/outbox.rs b/upub/routes/src/activitypub/user/outbox.rs index 62b3d6eb..9719eda7 100644 --- a/upub/routes/src/activitypub/user/outbox.rs +++ b/upub/routes/src/activitypub/user/outbox.rs @@ -37,6 +37,7 @@ pub async fn page( .await } +#[tracing::instrument(level = "info", skip(ctx), fields(activity = %activity))] pub async fn post( State(ctx): State, Path(id): Path, From c6628973cacbd765d93d01291cd06cfa07ccef11 Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:20:49 +0200 Subject: [PATCH 2/7] fix: better errors for debug getter --- upub/core/src/errors.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/upub/core/src/errors.rs b/upub/core/src/errors.rs index 6279a49a..874d158a 100644 --- a/upub/core/src/errors.rs +++ b/upub/core/src/errors.rs @@ -87,6 +87,7 @@ impl axum::response::IntoResponse for UpubError { // TODO it's kind of jank to hide this print down here, i should probably learn how spans work // in tracing and use the library's features but ehhhh tracing::debug!("emitting error response: {self:?}"); + let descr = self.to_string(); match self { UpubError::Redirect(to) => Redirect::to(&to).into_response(), UpubError::Status(status) => status.into_response(), @@ -94,7 +95,7 @@ impl axum::response::IntoResponse for UpubError { StatusCode::SERVICE_UNAVAILABLE, axum::Json(serde_json::json!({ "error": "database", - "description": format!("{e:#?}"), + "inner": format!("{e:#?}"), })) ).into_response(), UpubError::Reqwest(x) | UpubError::FetchError(x, _) => ( @@ -103,7 +104,8 @@ impl axum::response::IntoResponse for UpubError { "error": "request", "status": x.status().map(|s| s.to_string()).unwrap_or_default(), "url": x.url().map(|x| x.to_string()).unwrap_or_default(), - "description": format!("{x:#?}"), + "description": descr, + "inner": format!("{x:#?}"), })) ).into_response(), UpubError::Field(x) => ( @@ -111,7 +113,7 @@ impl axum::response::IntoResponse for UpubError { axum::Json(serde_json::json!({ "error": "field", "field": x.0.to_string(), - "description": format!("missing required field from request: '{}'", x.0), + "description": descr, })) ).into_response(), UpubError::Mismatch(expected, found) => ( @@ -120,14 +122,15 @@ impl axum::response::IntoResponse for UpubError { "error": "type", "expected": expected.as_ref().to_string(), "found": found.as_ref().to_string(), - "description": self.to_string(), + "description": descr, })) ).into_response(), - _ => ( + x => ( StatusCode::INTERNAL_SERVER_ERROR, axum::Json(serde_json::json!({ "error": "unknown", - "description": self.to_string(), + "description": descr, + "inner": format!("{x:#?}"), })) ).into_response(), } From b0f47de2780c188be10991b9f9a6230270c119dd Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:23:18 +0200 Subject: [PATCH 3/7] fix: sharkey wants trailing slash too --- upub/core/src/server/fetcher.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/upub/core/src/server/fetcher.rs b/upub/core/src/server/fetcher.rs index 1460aa5b..0bf41f91 100644 --- a/upub/core/src/server/fetcher.rs +++ b/upub/core/src/server/fetcher.rs @@ -131,7 +131,7 @@ impl Fetcher for Context { let document = Self::request( Method::GET, id, None, - &format!("https://{}", self.domain()), self.pkey(), self.domain(), + &format!("https://{}/", self.domain()), self.pkey(), self.domain(), ) .await? .json::() @@ -207,7 +207,8 @@ impl Fetcher for Context { }; if let Ok(res) = Self::request( - Method::GET, &format!("https://{domain}"), None, &format!("https://{}", self.domain()), self.pkey(), self.domain(), + Method::GET, &format!("https://{domain}"), None, + &format!("https://{}/", self.domain()), self.pkey(), self.domain(), ).await { if let Ok(actor) = res.json::().await { if let Some(name) = actor.name() { @@ -243,7 +244,7 @@ impl Fetcher for Context { if let Some(followers_url) = &document.followers().id() { let req = Self::request( Method::GET, followers_url, None, - &format!("https://{}", self.domain()), self.pkey(), self.domain(), + &format!("https://{}/", self.domain()), self.pkey(), self.domain(), ).await; if let Ok(res) = req { if let Ok(user_followers) = res.json::().await { @@ -255,9 +256,9 @@ impl Fetcher for Context { } if let Some(following_url) = &document.following().id() { - let req = Self::request( + let req = Self::request( Method::GET, following_url, None, - &format!("https://{}", self.domain()), self.pkey(), self.domain(), + &format!("https://{}/", self.domain()), self.pkey(), self.domain(), ).await; if let Ok(res) = req { if let Ok(user_following) = res.json::().await { From 6469dbe85e6caf33f4ed4b42d104385dad8de368 Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:54:22 +0200 Subject: [PATCH 4/7] feat(web): filter updated, more readable filter code --- web/src/components/activity.rs | 74 ++++++++++++++-------------------- web/src/config.rs | 24 +++++++++-- 2 files changed, 52 insertions(+), 46 deletions(-) diff --git a/web/src/components/activity.rs b/web/src/components/activity.rs index 406c97c6..9198a562 100644 --- a/web/src/components/activity.rs +++ b/web/src/components/activity.rs @@ -45,51 +45,39 @@ pub fn Item( let config = use_context::>().expect("missing config context"); let id = item.id().unwrap_or_default().to_string(); let sep = if sep { Some(view! {
}) } else { None }; - match item.object_type() { + if !replies && !config.get().filters.visible(&item) { + return None; + } + match item.object_type().unwrap_or(apb::ObjectType::Object) { // special case for placeholder activities - Some(apb::ObjectType::Note) | Some(apb::ObjectType::Document(_)) => (move || { - if !config.get().filters.replies && item.in_reply_to().id().is_some() { - None - } else if config.get().filters.orphans { - Some(view! { {sep.clone()} }) - } else { - None - } - }).into_view(), + apb::ObjectType::Note | apb::ObjectType::Document(_) => + Some(view! { {sep.clone()} }.into_view()), // everything else - Some(apb::ObjectType::Activity(t)) => (move || { - if config.get().filters.visible(apb::ObjectType::Activity(t)) { - let object_id = item.object().id().unwrap_or_default(); - if !replies && !config.get().filters.replies && CACHE.get(&object_id).map(|x| x.in_reply_to().id().is_some()).unwrap_or(false) { - None - } else { - let object = match t { - apb::ActivityType::Create | apb::ActivityType::Announce => - CACHE.get(&object_id).map(|obj| { - view! { } - }.into_view()), - apb::ActivityType::Follow => - CACHE.get(&object_id).map(|obj| { - view! { -
- - -
- } - }.into_view()), - _ => None, - }; - Some(view! { - - {object} - {sep.clone()} - }) - } - } else { - None - } - }).into_view(), + apb::ObjectType::Activity(t) => { + let object_id = item.object().id().unwrap_or_default(); + let object = match t { + apb::ActivityType::Create | apb::ActivityType::Announce => + CACHE.get(&object_id).map(|obj| { + view! { } + }.into_view()), + apb::ActivityType::Follow => + CACHE.get(&object_id).map(|obj| { + view! { +
+ + +
+ } + }.into_view()), + _ => None, + }; + Some(view! { + + {object} + {sep.clone()} + }.into_view()) + }, // should never happen - _ => view! {

type not implemented

}.into_view(), + t => Some(view! {

type not implemented : {t.as_ref().to_string()}

}.into_view()), } } diff --git a/web/src/config.rs b/web/src/config.rs index 68cf97ac..d5d44c27 100644 --- a/web/src/config.rs +++ b/web/src/config.rs @@ -36,21 +36,39 @@ pub struct FiltersConfig { #[serde_inline_default(true)] pub follows: bool, + #[serde_inline_default(false)] + pub updates: bool, + #[serde_inline_default(true)] pub orphans: bool, } impl FiltersConfig { - pub fn visible(&self, object_type: apb::ObjectType) -> bool { - match object_type { + pub fn visible(&self, item: &crate::Object) -> bool { + use apb::{Object, Activity}; + + let type_filter = match item.object_type().unwrap_or(apb::ObjectType::Object) { apb::ObjectType::Note | apb::ObjectType::Document(_) => self.orphans, apb::ObjectType::Activity(apb::ActivityType::Like | apb::ActivityType::EmojiReact) => self.likes, apb::ObjectType::Activity(apb::ActivityType::Create) => self.creates, apb::ObjectType::Activity(apb::ActivityType::Announce) => self.announces, + apb::ObjectType::Activity(apb::ActivityType::Update) => self.updates, apb::ObjectType::Activity( apb::ActivityType::Follow | apb::ActivityType::Accept(_) | apb::ActivityType::Reject(_) ) => self.follows, _ => true, - } + }; + let mut reply_filter = true; + + if + item.in_reply_to().id().is_some() || + item.object().get().map(|x| + x.in_reply_to().id().is_some() + ).unwrap_or(false) + { + reply_filter = self.replies; + }; + + type_filter && reply_filter } } From 5a57fd69b9a66ac457252448b22a5283e5063df4 Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:55:38 +0200 Subject: [PATCH 5/7] fix: remove excessive instruments, check actor before we were checking only for server match, now check whole uid match on inbox activities --- upub/routes/src/activitypub/inbox.rs | 14 ++++++-------- upub/routes/src/activitypub/user/outbox.rs | 1 - 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/upub/routes/src/activitypub/inbox.rs b/upub/routes/src/activitypub/inbox.rs index 26135795..6d26eab6 100644 --- a/upub/routes/src/activitypub/inbox.rs +++ b/upub/routes/src/activitypub/inbox.rs @@ -1,4 +1,4 @@ -use apb::{server::Inbox, Activity, ActivityType}; +use apb::{server::Inbox, Activity, ActivityType, Base}; use axum::{extract::{Query, State}, http::StatusCode, Json}; use sea_orm::{sea_query::IntoCondition, ColumnTrait}; use upub::{server::auth::{AuthIdentity, Identity}, Context}; @@ -38,13 +38,12 @@ macro_rules! pretty_json { } -#[tracing::instrument(level = "info", skip(ctx), fields(activity = %activity))] pub async fn post( State(ctx): State, AuthIdentity(auth): AuthIdentity, Json(activity): Json ) -> upub::Result<()> { - let Identity::Remote { domain: server, .. } = auth else { + let Identity::Remote { domain: server, user: uid, .. } = auth else { if activity.activity_type() == Some(ActivityType::Delete) { // this is spammy af, ignore them! // we basically received a delete for a user we can't fetch and verify, meaning remote @@ -62,15 +61,14 @@ pub async fn post( } }; - let Some(actor) = activity.actor().id() else { - return Err(upub::Error::bad_request()); - }; + let aid = activity.id().ok_or_else(|| upub::Error::field("id"))?.to_string(); + let actor = activity.actor().id().ok_or_else(|| upub::Error::field("actor"))?; - if server != Context::server(&actor) { + if uid != actor { return Err(upub::Error::unauthorized()); } - tracing::debug!("processing federated activity: '{}'", serde_json::to_string(&activity).unwrap_or_default()); + tracing::debug!("processing federated activity: '{:#}'", activity); // TODO we could process Links and bare Objects maybe, but probably out of AP spec? match activity.activity_type().ok_or_else(upub::Error::bad_request)? { diff --git a/upub/routes/src/activitypub/user/outbox.rs b/upub/routes/src/activitypub/user/outbox.rs index 9719eda7..62b3d6eb 100644 --- a/upub/routes/src/activitypub/user/outbox.rs +++ b/upub/routes/src/activitypub/user/outbox.rs @@ -37,7 +37,6 @@ pub async fn page( .await } -#[tracing::instrument(level = "info", skip(ctx), fields(activity = %activity))] pub async fn post( State(ctx): State, Path(id): Path, From 6129973b13d95621c1ab0535cecf95d24dc4cb22 Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:56:25 +0200 Subject: [PATCH 6/7] fix: gts uses a path but its not real??? --- upub/core/src/server/auth.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/upub/core/src/server/auth.rs b/upub/core/src/server/auth.rs index 226dd964..837e6a9c 100644 --- a/upub/core/src/server/auth.rs +++ b/upub/core/src/server/auth.rs @@ -115,6 +115,7 @@ where // TODO assert payload's digest is equal to signature's let user_id = http_signature.key_id + .replace("/main-key", "") // gotosocial whyyyyy .split('#') .next().ok_or(UpubError::bad_request())? .to_string(); From b6a17184eb581c67b6c41c6a73efa54ba4c27ad9 Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:56:41 +0200 Subject: [PATCH 7/7] chore(apb): track todo &str --- apb/src/node.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/apb/src/node.rs b/apb/src/node.rs index aa369d0d..a1312e45 100644 --- a/apb/src/node.rs +++ b/apb/src/node.rs @@ -99,6 +99,7 @@ impl Node { } /// returns id of object: url for link, id for object, None if empty or array + // TODO return Option<&str> and avoid inner clone pub fn id(&self) -> Option { match self { Node::Empty => None,