From 5a57fd69b9a66ac457252448b22a5283e5063df4 Mon Sep 17 00:00:00 2001 From: alemi Date: Fri, 31 May 2024 15:55:38 +0200 Subject: [PATCH] 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 2613579..6d26eab 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 9719eda..62b3d6e 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,