From b0c318566342740c5c97dd4b4d341fc2902607e0 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 28 Oct 2022 13:38:22 +0000 Subject: [PATCH 1/4] Make verify apub url function async (#2514) * Make verify apub url function async * cleanup * use dep from crates.io * dont use unwrap --- Cargo.lock | 11 ++++-- Cargo.toml | 2 +- crates/api/Cargo.toml | 2 +- crates/api_crud/Cargo.toml | 2 +- crates/apub/Cargo.toml | 2 +- .../apub/src/activities/block/block_user.rs | 6 ---- .../src/activities/block/undo_block_user.rs | 35 ++++++++----------- .../apub/src/activities/community/add_mod.rs | 6 ---- .../apub/src/activities/community/announce.rs | 9 +---- .../src/activities/community/remove_mod.rs | 6 ---- .../apub/src/activities/community/report.rs | 6 ---- .../apub/src/activities/community/update.rs | 5 --- .../activities/create_or_update/comment.rs | 6 ---- .../src/activities/create_or_update/post.rs | 5 --- .../create_or_update/private_message.rs | 6 ---- crates/apub/src/activities/deletion/delete.rs | 5 --- .../src/activities/deletion/delete_user.rs | 7 +--- .../src/activities/deletion/undo_delete.rs | 5 --- .../apub/src/activities/following/accept.rs | 6 ---- .../apub/src/activities/following/follow.rs | 5 --- .../src/activities/following/undo_follow.rs | 5 --- .../apub/src/activities/voting/undo_vote.rs | 5 --- crates/apub/src/activities/voting/vote.rs | 5 --- crates/apub/src/lib.rs | 20 +++++++++-- crates/db_schema/Cargo.toml | 2 +- crates/utils/translations | 2 +- 26 files changed, 49 insertions(+), 127 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e726ae25e..20e6c85be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,9 +4,9 @@ version = 3 [[package]] name = "activitypub_federation" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "690ed975ab70b883b4f0776f60fd7f23a7484a49f4257e7672e64d0990e95771" +checksum = "dd9ae511df7135c271dca3ef3751f5528891c965e47d8d7a70fed9d2f1e5b6b1" dependencies = [ "activitypub_federation_derive", "actix-web", @@ -16,6 +16,7 @@ dependencies = [ "base64", "chrono", "derive_builder 0.11.2", + "dyn-clone", "http", "http-signature-normalization-actix", "http-signature-normalization-reqwest", @@ -1219,6 +1220,12 @@ dependencies = [ "syn 1.0.100", ] +[[package]] +name = "dyn-clone" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f94fa09c2aeea5b8839e414b7b841bf429fd25b9c522116ac97ee87856d88b2" + [[package]] name = "either" version = "1.8.0" diff --git a/Cargo.toml b/Cargo.toml index 58fb33257..82962d16f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ lemmy_db_schema = { version = "=0.16.5", path = "./crates/db_schema" } lemmy_api_common = { version = "=0.16.5", path = "crates/api_common" } lemmy_websocket = { version = "=0.16.5", path = "./crates/websocket" } lemmy_routes = { version = "=0.16.5", path = "./crates/routes" } -activitypub_federation = "0.2.0" +activitypub_federation = "0.2.3" diesel = "2.0.0" diesel_migrations = "2.0.0" serde = { version = "1.0.145", features = ["derive"] } diff --git a/crates/api/Cargo.toml b/crates/api/Cargo.toml index 935a4788b..09136b43d 100644 --- a/crates/api/Cargo.toml +++ b/crates/api/Cargo.toml @@ -22,7 +22,7 @@ lemmy_db_views_moderator = { version = "=0.16.5", path = "../db_views_moderator" lemmy_db_views_actor = { version = "=0.16.5", path = "../db_views_actor", features = ["full"] } lemmy_api_common = { version = "=0.16.5", path = "../api_common", features = ["full"] } lemmy_websocket = { version = "=0.16.5", path = "../websocket" } -activitypub_federation = { version = "0.2.2" } +activitypub_federation = "0.2.3" diesel = "2.0.0" bcrypt = "0.13.0" chrono = { version = "0.4.22", features = ["serde"], default-features = false } diff --git a/crates/api_crud/Cargo.toml b/crates/api_crud/Cargo.toml index 161e57c09..6608f22cb 100644 --- a/crates/api_crud/Cargo.toml +++ b/crates/api_crud/Cargo.toml @@ -16,7 +16,7 @@ lemmy_db_views = { version = "=0.16.5", path = "../db_views", features = ["full" lemmy_db_views_actor = { version = "=0.16.5", path = "../db_views_actor", features = ["full"] } lemmy_api_common = { version = "=0.16.5", path = "../api_common", features = ["full"] } lemmy_websocket = { version = "=0.16.5", path = "../websocket" } -activitypub_federation = { version = "0.2.2" } +activitypub_federation = "0.2.3" bcrypt = "0.13.0" serde_json = { version = "1.0.85", features = ["preserve_order"] } serde = { version = "1.0.145", features = ["derive"] } diff --git a/crates/apub/Cargo.toml b/crates/apub/Cargo.toml index 22e6a52b5..81967ee91 100644 --- a/crates/apub/Cargo.toml +++ b/crates/apub/Cargo.toml @@ -20,7 +20,7 @@ lemmy_db_views = { version = "=0.16.5", path = "../db_views", features = ["full" lemmy_db_views_actor = { version = "=0.16.5", path = "../db_views_actor", features = ["full"] } lemmy_api_common = { version = "=0.16.5", path = "../api_common", features = ["full"] } lemmy_websocket = { version = "=0.16.5", path = "../websocket" } -activitypub_federation = { version = "0.2.2" } +activitypub_federation = "0.2.3" diesel = "2.0.0" activitystreams-kinds = "0.2.1" chrono = { version = "0.4.22", features = ["serde"], default-features = false } diff --git a/crates/apub/src/activities/block/block_user.rs b/crates/apub/src/activities/block/block_user.rs index 3e574232d..bef3b6a28 100644 --- a/crates/apub/src/activities/block/block_user.rs +++ b/crates/apub/src/activities/block/block_user.rs @@ -9,8 +9,6 @@ use crate::{ verify_person_in_community, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, instance::remote_instance_inboxes, person::ApubPerson}, protocol::activities::block::block_user::BlockUser, @@ -125,10 +123,6 @@ impl ActivityHandler for BlockUser { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_is_public(&self.to, &self.cc)?; match self .target diff --git a/crates/apub/src/activities/block/undo_block_user.rs b/crates/apub/src/activities/block/undo_block_user.rs index b93d0aa3a..6de1d3a17 100644 --- a/crates/apub/src/activities/block/undo_block_user.rs +++ b/crates/apub/src/activities/block/undo_block_user.rs @@ -1,19 +1,3 @@ -use crate::{ - activities::{ - block::{generate_cc, SiteOrCommunity}, - community::{announce::GetCommunity, send_activity_in_community}, - generate_activity_id, - send_lemmy_activity, - verify_is_public, - }, - activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, - local_instance, - objects::{community::ApubCommunity, instance::remote_instance_inboxes, person::ApubPerson}, - protocol::activities::block::{block_user::BlockUser, undo_block_user::UndoBlockUser}, - ActorType, -}; use activitypub_federation::{ core::object_id::ObjectId, data::Data, @@ -34,6 +18,21 @@ use lemmy_utils::error::LemmyError; use lemmy_websocket::LemmyContext; use url::Url; +use crate::{ + activities::{ + block::{generate_cc, SiteOrCommunity}, + community::{announce::GetCommunity, send_activity_in_community}, + generate_activity_id, + send_lemmy_activity, + verify_is_public, + }, + activity_lists::AnnouncableActivities, + local_instance, + objects::{community::ApubCommunity, instance::remote_instance_inboxes, person::ApubPerson}, + protocol::activities::block::{block_user::BlockUser, undo_block_user::UndoBlockUser}, + ActorType, +}; + impl UndoBlockUser { #[tracing::instrument(skip_all)] pub async fn send( @@ -92,10 +91,6 @@ impl ActivityHandler for UndoBlockUser { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_is_public(&self.to, &self.cc)?; verify_domains_match(self.actor.inner(), self.object.actor.inner())?; self.object.verify(context, request_counter).await?; diff --git a/crates/apub/src/activities/community/add_mod.rs b/crates/apub/src/activities/community/add_mod.rs index 86ee91666..4dbd463b7 100644 --- a/crates/apub/src/activities/community/add_mod.rs +++ b/crates/apub/src/activities/community/add_mod.rs @@ -12,8 +12,6 @@ use crate::{ verify_person_in_community, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, generate_moderators_url, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, @@ -86,10 +84,6 @@ impl ActivityHandler for AddMod { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_is_public(&self.to, &self.cc)?; let community = self.get_community(context, request_counter).await?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; diff --git a/crates/apub/src/activities/community/announce.rs b/crates/apub/src/activities/community/announce.rs index 2886c2bf0..e5419680e 100644 --- a/crates/apub/src/activities/community/announce.rs +++ b/crates/apub/src/activities/community/announce.rs @@ -1,8 +1,6 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity, verify_is_public}, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, insert_activity, objects::community::ApubCommunity, protocol::{ @@ -13,7 +11,6 @@ use crate::{ }; use activitypub_federation::{core::object_id::ObjectId, data::Data, traits::ActivityHandler}; use activitystreams_kinds::{activity::AnnounceType, public}; -use lemmy_api_common::utils::blocking; use lemmy_utils::error::LemmyError; use lemmy_websocket::LemmyContext; use tracing::debug; @@ -87,13 +84,9 @@ impl ActivityHandler for AnnounceActivity { #[tracing::instrument(skip_all)] async fn verify( &self, - context: &Data, + _context: &Data, _request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_is_public(&self.to, &self.cc)?; Ok(()) } diff --git a/crates/apub/src/activities/community/remove_mod.rs b/crates/apub/src/activities/community/remove_mod.rs index 3dca02397..ede6a0089 100644 --- a/crates/apub/src/activities/community/remove_mod.rs +++ b/crates/apub/src/activities/community/remove_mod.rs @@ -12,8 +12,6 @@ use crate::{ verify_person_in_community, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, generate_moderators_url, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, @@ -86,10 +84,6 @@ impl ActivityHandler for RemoveMod { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_is_public(&self.to, &self.cc)?; let community = self.get_community(context, request_counter).await?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; diff --git a/crates/apub/src/activities/community/report.rs b/crates/apub/src/activities/community/report.rs index de3fb3566..c3b50abfe 100644 --- a/crates/apub/src/activities/community/report.rs +++ b/crates/apub/src/activities/community/report.rs @@ -1,7 +1,5 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity, verify_person_in_community}, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::community::report::Report, @@ -76,10 +74,6 @@ impl ActivityHandler for Report { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - let community = self.to[0] .dereference(context, local_instance(context), request_counter) .await?; diff --git a/crates/apub/src/activities/community/update.rs b/crates/apub/src/activities/community/update.rs index b444f4f5e..def4d7a4e 100644 --- a/crates/apub/src/activities/community/update.rs +++ b/crates/apub/src/activities/community/update.rs @@ -7,8 +7,6 @@ use crate::{ verify_person_in_community, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::community::update::UpdateCommunity, @@ -71,9 +69,6 @@ impl ActivityHandler for UpdateCommunity { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; verify_is_public(&self.to, &self.cc)?; let community = self.get_community(context, request_counter).await?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; diff --git a/crates/apub/src/activities/create_or_update/comment.rs b/crates/apub/src/activities/create_or_update/comment.rs index 3e0de366f..dad6ada9b 100644 --- a/crates/apub/src/activities/create_or_update/comment.rs +++ b/crates/apub/src/activities/create_or_update/comment.rs @@ -8,8 +8,6 @@ use crate::{ verify_person_in_community, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, local_instance, mentions::MentionOrValue, objects::{comment::ApubComment, community::ApubCommunity, person::ApubPerson}, @@ -117,10 +115,6 @@ impl ActivityHandler for CreateOrUpdateComment { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_is_public(&self.to, &self.cc)?; let post = self.object.get_parents(context, request_counter).await?.0; let community = self.get_community(context, request_counter).await?; diff --git a/crates/apub/src/activities/create_or_update/post.rs b/crates/apub/src/activities/create_or_update/post.rs index 1f997f72f..d9d7b8545 100644 --- a/crates/apub/src/activities/create_or_update/post.rs +++ b/crates/apub/src/activities/create_or_update/post.rs @@ -8,8 +8,6 @@ use crate::{ verify_person_in_community, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, objects::{community::ApubCommunity, person::ApubPerson, post::ApubPost}, protocol::activities::{create_or_update::post::CreateOrUpdatePost, CreateOrUpdateType}, ActorType, @@ -95,9 +93,6 @@ impl ActivityHandler for CreateOrUpdatePost { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; verify_is_public(&self.to, &self.cc)?; let community = self.get_community(context, request_counter).await?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; diff --git a/crates/apub/src/activities/create_or_update/private_message.rs b/crates/apub/src/activities/create_or_update/private_message.rs index 20310bc41..9ad547691 100644 --- a/crates/apub/src/activities/create_or_update/private_message.rs +++ b/crates/apub/src/activities/create_or_update/private_message.rs @@ -1,7 +1,5 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity, verify_person}, - check_apub_id_valid, - fetch_local_site_data, objects::{person::ApubPerson, private_message::ApubPrivateMessage}, protocol::activities::{ create_or_update::private_message::CreateOrUpdatePrivateMessage, @@ -71,10 +69,6 @@ impl ActivityHandler for CreateOrUpdatePrivateMessage { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_person(&self.actor, context, request_counter).await?; verify_domains_match(self.actor.inner(), self.object.id.inner())?; verify_domains_match(self.to[0].inner(), self.object.to[0].inner())?; diff --git a/crates/apub/src/activities/deletion/delete.rs b/crates/apub/src/activities/deletion/delete.rs index 95024c475..ca93b89f0 100644 --- a/crates/apub/src/activities/deletion/delete.rs +++ b/crates/apub/src/activities/deletion/delete.rs @@ -4,8 +4,6 @@ use crate::{ deletion::{receive_delete_action, verify_delete_activity, DeletableObjects}, generate_activity_id, }, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::{activities::deletion::delete::Delete, IdOrNestedObject}, @@ -57,9 +55,6 @@ impl ActivityHandler for Delete { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; verify_delete_activity(self, self.summary.is_some(), context, request_counter).await?; Ok(()) } diff --git a/crates/apub/src/activities/deletion/delete_user.rs b/crates/apub/src/activities/deletion/delete_user.rs index 5fb453eef..9570d85d2 100644 --- a/crates/apub/src/activities/deletion/delete_user.rs +++ b/crates/apub/src/activities/deletion/delete_user.rs @@ -1,7 +1,5 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity, verify_is_public, verify_person}, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{instance::remote_instance_inboxes, person::ApubPerson}, protocol::activities::deletion::delete_user::DeleteUser, @@ -13,7 +11,7 @@ use activitypub_federation::{ utils::verify_urls_match, }; use activitystreams_kinds::{activity::DeleteType, public}; -use lemmy_api_common::utils::{blocking, delete_user_account}; +use lemmy_api_common::utils::delete_user_account; use lemmy_utils::error::LemmyError; use lemmy_websocket::LemmyContext; use url::Url; @@ -38,9 +36,6 @@ impl ActivityHandler for DeleteUser { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; verify_is_public(&self.to, &[])?; verify_person(&self.actor, context, request_counter).await?; verify_urls_match(self.actor.inner(), self.object.inner())?; diff --git a/crates/apub/src/activities/deletion/undo_delete.rs b/crates/apub/src/activities/deletion/undo_delete.rs index f73c780c2..1576e944c 100644 --- a/crates/apub/src/activities/deletion/undo_delete.rs +++ b/crates/apub/src/activities/deletion/undo_delete.rs @@ -4,8 +4,6 @@ use crate::{ deletion::{receive_delete_action, verify_delete_activity, DeletableObjects}, generate_activity_id, }, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::deletion::{delete::Delete, undo_delete::UndoDelete}, @@ -56,9 +54,6 @@ impl ActivityHandler for UndoDelete { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; self.object.verify(context, request_counter).await?; verify_delete_activity( &self.object, diff --git a/crates/apub/src/activities/following/accept.rs b/crates/apub/src/activities/following/accept.rs index c4bbbb1bb..f964da086 100644 --- a/crates/apub/src/activities/following/accept.rs +++ b/crates/apub/src/activities/following/accept.rs @@ -1,7 +1,5 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity}, - check_apub_id_valid, - fetch_local_site_data, local_instance, protocol::activities::following::{accept::AcceptFollowCommunity, follow::FollowCommunity}, ActorType, @@ -69,10 +67,6 @@ impl ActivityHandler for AcceptFollowCommunity { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; - verify_urls_match(self.actor.inner(), self.object.object.inner())?; self.object.verify(context, request_counter).await?; Ok(()) diff --git a/crates/apub/src/activities/following/follow.rs b/crates/apub/src/activities/following/follow.rs index 512c074cd..b0ae72c71 100644 --- a/crates/apub/src/activities/following/follow.rs +++ b/crates/apub/src/activities/following/follow.rs @@ -5,8 +5,6 @@ use crate::{ verify_person, verify_person_in_community, }, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::following::{accept::AcceptFollowCommunity, follow::FollowCommunity}, @@ -86,9 +84,6 @@ impl ActivityHandler for FollowCommunity { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; verify_person(&self.actor, context, request_counter).await?; let community = self .object diff --git a/crates/apub/src/activities/following/undo_follow.rs b/crates/apub/src/activities/following/undo_follow.rs index b37e21fb2..94b0b68ec 100644 --- a/crates/apub/src/activities/following/undo_follow.rs +++ b/crates/apub/src/activities/following/undo_follow.rs @@ -1,7 +1,5 @@ use crate::{ activities::{generate_activity_id, send_lemmy_activity, verify_person}, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::following::{follow::FollowCommunity, undo_follow::UndoFollowCommunity}, @@ -65,9 +63,6 @@ impl ActivityHandler for UndoFollowCommunity { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; verify_urls_match(self.actor.inner(), self.object.actor.inner())?; verify_person(&self.actor, context, request_counter).await?; self.object.verify(context, request_counter).await?; diff --git a/crates/apub/src/activities/voting/undo_vote.rs b/crates/apub/src/activities/voting/undo_vote.rs index 80e319bf0..f6fc36a2d 100644 --- a/crates/apub/src/activities/voting/undo_vote.rs +++ b/crates/apub/src/activities/voting/undo_vote.rs @@ -6,8 +6,6 @@ use crate::{ voting::{undo_vote_comment, undo_vote_post}, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::voting::{ @@ -84,9 +82,6 @@ impl ActivityHandler for UndoVote { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; let community = self.get_community(context, request_counter).await?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; verify_urls_match(self.actor.inner(), self.object.actor.inner())?; diff --git a/crates/apub/src/activities/voting/vote.rs b/crates/apub/src/activities/voting/vote.rs index 7d07fe8cd..e33c95676 100644 --- a/crates/apub/src/activities/voting/vote.rs +++ b/crates/apub/src/activities/voting/vote.rs @@ -6,8 +6,6 @@ use crate::{ voting::{vote_comment, vote_post}, }, activity_lists::AnnouncableActivities, - check_apub_id_valid, - fetch_local_site_data, local_instance, objects::{community::ApubCommunity, person::ApubPerson}, protocol::activities::voting::vote::{Vote, VoteType}, @@ -83,9 +81,6 @@ impl ActivityHandler for Vote { context: &Data, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let local_site_data = blocking(context.pool(), fetch_local_site_data).await??; - check_apub_id_valid(self.id(), &local_site_data, context.settings()) - .map_err(LemmyError::from_message)?; let community = self.get_community(context, request_counter).await?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; let enable_downvotes = blocking(context.pool(), LocalSite::read) diff --git a/crates/apub/src/lib.rs b/crates/apub/src/lib.rs index 471a7564f..8c7c042c6 100644 --- a/crates/apub/src/lib.rs +++ b/crates/apub/src/lib.rs @@ -4,8 +4,10 @@ use activitypub_federation::{ traits::{Actor, ApubObject}, InstanceSettings, LocalInstance, + UrlVerifier, }; use anyhow::Context; +use async_trait::async_trait; use diesel::PgConnection; use lemmy_api_common::utils::blocking; use lemmy_db_schema::{ @@ -59,9 +61,8 @@ fn local_instance(context: &LemmyContext) -> &'static LocalInstance { .http_fetch_retry_limit(http_fetch_retry_limit) .worker_count(worker_count) .debug(federation_debug) - // TODO No idea why, but you can't pass context.settings() to the verify_url_function closure - // without the value getting captured. .http_signature_compat(true) + .url_verifier(Box::new(VerifyUrlData(context.clone()))) .build() .expect("configure federation"); LocalInstance::new( @@ -72,6 +73,20 @@ fn local_instance(context: &LemmyContext) -> &'static LocalInstance { }) } +#[derive(Clone)] +struct VerifyUrlData(LemmyContext); + +#[async_trait] +impl UrlVerifier for VerifyUrlData { + async fn verify(&self, url: &Url) -> Result<(), &'static str> { + let local_site_data = blocking(self.0.pool(), fetch_local_site_data) + .await + .expect("read local site data") + .expect("read local site data"); + check_apub_id_valid(url, &local_site_data, self.0.settings()) + } +} + /// Checks if the ID is allowed for sending or receiving. /// /// In particular, it checks for: @@ -83,7 +98,6 @@ fn local_instance(context: &LemmyContext) -> &'static LocalInstance { /// `use_strict_allowlist` should be true only when parsing a remote community, or when parsing a /// post/comment in a local community. #[tracing::instrument(skip(settings, local_site_data))] -// TODO This function needs to be called by incoming activities fn check_apub_id_valid( apub_id: &Url, local_site_data: &LocalSiteData, diff --git a/crates/db_schema/Cargo.toml b/crates/db_schema/Cargo.toml index 155d84406..5808f81c9 100644 --- a/crates/db_schema/Cargo.toml +++ b/crates/db_schema/Cargo.toml @@ -24,7 +24,7 @@ url = { version = "2.3.1", features = ["serde"] } strum = "0.24.1" strum_macros = "0.24.3" serde_json = { version = "1.0.85", features = ["preserve_order"], optional = true } -activitypub_federation = { version = "0.2.2", optional = true } +activitypub_federation = { version = "0.2.3", optional = true } lemmy_utils = { version = "=0.16.5", path = "../utils", optional = true } bcrypt = { version = "0.13.0", optional = true } diesel = { version = "2.0.0", features = ["postgres","chrono","r2d2","serde_json"], optional = true } diff --git a/crates/utils/translations b/crates/utils/translations index 454debaed..f5d6f0eab 160000 --- a/crates/utils/translations +++ b/crates/utils/translations @@ -1 +1 @@ -Subproject commit 454debaede4cc932ac15fea9bf620cf1daf1ae4c +Subproject commit f5d6f0eabafd559417bf8f203fd655f7858bffcf From 3b0be52e673a4ac851c8df688a29e1fe828fb7a4 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 28 Oct 2022 14:34:40 +0000 Subject: [PATCH 2/4] Image improvements (#2513) * Image improvements * remove rate limits --- crates/routes/src/images.rs | 8 +++++--- docker/dev/lemmy.hjson | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index 3eb9bfd4e..cb7187d55 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -45,7 +45,7 @@ struct Images { #[derive(Deserialize)] struct PictrsParams { format: Option, - thumbnail: Option, + thumbnail: Option, } #[derive(Deserialize)] @@ -130,8 +130,10 @@ async fn full_res( let url = if params.format.is_none() && params.thumbnail.is_none() { format!("{}image/original/{}", pictrs_config.url, name,) } else { - // Use jpg as a default when none is given - let format = params.format.unwrap_or_else(|| "jpg".to_string()); + // Take file type from name, or jpg if nothing is given + let format = params + .format + .unwrap_or_else(|| name.split('.').last().unwrap_or("jpg").to_string()); let mut url = format!("{}image/process.{}?src={}", pictrs_config.url, format, name,); diff --git a/docker/dev/lemmy.hjson b/docker/dev/lemmy.hjson index 23f405204..2bd0675a9 100644 --- a/docker/dev/lemmy.hjson +++ b/docker/dev/lemmy.hjson @@ -10,6 +10,9 @@ admin_password: "lemmylemmy" site_name: "lemmy-dev" } + database: { + host: postgres + } database: { host: "postgres" From 6aa9bdebaecaeb3f17008c6137b421d2dbf05949 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 28 Oct 2022 14:42:05 +0000 Subject: [PATCH 3/4] Fix limit_languages to operate on correct instance (fixes #2496) (#2518) * Fix limit_languages to operate on correct instance (fixes #2496) * cargo fmt --- crates/api_crud/src/site/update.rs | 5 ++-- crates/apub/src/objects/instance.rs | 2 +- crates/db_schema/src/impls/actor_language.rs | 24 ++++++++++---------- crates/db_schema/src/impls/site.rs | 2 +- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 47915e434..2f0503952 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -76,7 +76,8 @@ impl PerformCrud for EditSite { let site_id = local_site.site_id; if let Some(discussion_languages) = data.discussion_languages.clone() { blocking(context.pool(), move |conn| { - SiteLanguage::update(conn, discussion_languages.clone(), site_id) + let site = Site::read(conn, site_id)?; + SiteLanguage::update(conn, discussion_languages, &site) }) .await??; } @@ -92,7 +93,7 @@ impl PerformCrud for EditSite { .build(); blocking(context.pool(), move |conn| { - Site::update(conn, site_id, &site_form) + Site::update(conn, local_site.site_id, &site_form) }) .await // Ignore errors for all these, so as to not throw errors if no update occurs diff --git a/crates/apub/src/objects/instance.rs b/crates/apub/src/objects/instance.rs index 521de64f3..de4839ada 100644 --- a/crates/apub/src/objects/instance.rs +++ b/crates/apub/src/objects/instance.rs @@ -159,7 +159,7 @@ impl ApubObject for ApubSite { let site = blocking(data.pool(), move |conn| { let site = Site::create(conn, &site_form)?; - SiteLanguage::update(conn, languages, site.id)?; + SiteLanguage::update(conn, languages, &site)?; Ok::(site) }) .await??; diff --git a/crates/db_schema/src/impls/actor_language.rs b/crates/db_schema/src/impls/actor_language.rs index 1de5c6aa6..6525a82d8 100644 --- a/crates/db_schema/src/impls/actor_language.rs +++ b/crates/db_schema/src/impls/actor_language.rs @@ -1,7 +1,7 @@ use crate::{ diesel::JoinOnDsl, - newtypes::{CommunityId, LanguageId, LocalUserId, SiteId}, - source::{actor_language::*, language::Language}, + newtypes::{CommunityId, InstanceId, LanguageId, LocalUserId, SiteId}, + source::{actor_language::*, language::Language, site::Site}, }; use diesel::{ delete, @@ -81,17 +81,17 @@ impl SiteLanguage { pub fn update( conn: &mut PgConnection, language_ids: Vec, - for_site_id: SiteId, + site: &Site, ) -> Result<(), Error> { conn.build_transaction().read_write().run(|conn| { use crate::schema::site_language::dsl::*; // Clear the current languages - delete(site_language.filter(site_id.eq(for_site_id))).execute(conn)?; + delete(site_language.filter(site_id.eq(site.id))).execute(conn)?; let lang_ids = convert_update_languages(conn, language_ids)?; for l in lang_ids { let form = SiteLanguageForm { - site_id: for_site_id, + site_id: site.id, language_id: l, }; insert_into(site_language) @@ -99,7 +99,7 @@ impl SiteLanguage { .get_result::(conn)?; } - CommunityLanguage::limit_languages(conn)?; + CommunityLanguage::limit_languages(conn, site.instance_id)?; Ok(()) }) @@ -136,7 +136,7 @@ impl CommunityLanguage { /// also part of site languages. This is because post/comment language is only checked against /// community language, and it shouldnt be possible to post content in languages which are not /// allowed by local site. - fn limit_languages(conn: &mut PgConnection) -> Result<(), Error> { + fn limit_languages(conn: &mut PgConnection, for_instance_id: InstanceId) -> Result<(), Error> { use crate::schema::{ community::dsl as c, community_language::dsl as cl, @@ -145,7 +145,7 @@ impl CommunityLanguage { let community_languages: Vec = cl::community_language .left_outer_join(sl::site_language.on(cl::language_id.eq(sl::language_id))) .inner_join(c::community) - .filter(c::local) + .filter(c::instance_id.eq(for_instance_id)) .filter(sl::language_id.is_null()) .select(cl::language_id) .get_results(conn)?; @@ -343,7 +343,7 @@ mod tests { assert_eq!(184, site_languages1.len()); let test_langs = test_langs1(conn); - SiteLanguage::update(conn, test_langs.clone(), site.id).unwrap(); + SiteLanguage::update(conn, test_langs.clone(), &site).unwrap(); let site_languages2 = SiteLanguage::read_local(conn).unwrap(); // after update, site only has new languages @@ -361,7 +361,7 @@ mod tests { let (site, instance) = create_test_site(conn); let test_langs = test_langs1(conn); - SiteLanguage::update(conn, test_langs.clone(), site.id).unwrap(); + SiteLanguage::update(conn, test_langs.clone(), &site).unwrap(); let person_form = PersonInsertForm::builder() .name("my test person".to_string()) @@ -399,7 +399,7 @@ mod tests { let conn = &mut establish_unpooled_connection(); let (site, instance) = create_test_site(conn); let test_langs = test_langs1(conn); - SiteLanguage::update(conn, test_langs.clone(), site.id).unwrap(); + SiteLanguage::update(conn, test_langs.clone(), &site).unwrap(); let read_site_langs = SiteLanguage::read(conn, site.id).unwrap(); assert_eq!(test_langs, read_site_langs); @@ -431,7 +431,7 @@ mod tests { // limit site languages to en, fi. after this, community languages should be updated to // intersection of old languages (en, fr, ru) and (en, fi), which is only fi. - SiteLanguage::update(conn, vec![test_langs[0], test_langs2[0]], site.id).unwrap(); + SiteLanguage::update(conn, vec![test_langs[0], test_langs2[0]], &site).unwrap(); let community_langs2 = CommunityLanguage::read(conn, community.id).unwrap(); assert_eq!(vec![test_langs[0]], community_langs2); diff --git a/crates/db_schema/src/impls/site.rs b/crates/db_schema/src/impls/site.rs index bea9a21c5..de53c19d0 100644 --- a/crates/db_schema/src/impls/site.rs +++ b/crates/db_schema/src/impls/site.rs @@ -26,7 +26,7 @@ impl Crud for Site { .get_result::(conn)?; // initialize with all languages - SiteLanguage::update(conn, vec![], site_.id)?; + SiteLanguage::update(conn, vec![], &site_)?; Ok(site_) } From 7aa6d6b3e1f5b102c69e8f3ce726e1e48778a852 Mon Sep 17 00:00:00 2001 From: sam365724 <111515092+sam365724@users.noreply.github.com> Date: Fri, 28 Oct 2022 16:43:33 +0200 Subject: [PATCH 4/4] Fix 2455: Check auth for pictrs when instance is private. (#2477) * Fix 2455: Check auth for pictrs when instance is private. * Update, no utils function, use of existing get_local_user_view_from_jwt * rustup toolchain install nightly (ftw) --- crates/routes/src/images.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index cb7187d55..78b5a8545 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -11,6 +11,8 @@ use actix_web::{ HttpResponse, }; use futures::stream::{Stream, StreamExt}; +use lemmy_api_common::utils::{blocking, get_local_user_view_from_jwt}; +use lemmy_db_schema::source::site::Site; use lemmy_utils::{claims::Claims, rate_limit::RateLimit, REQWEST_TIMEOUT}; use lemmy_websocket::LemmyContext; use reqwest::Body; @@ -123,6 +125,22 @@ async fn full_res( client: web::Data, context: web::Data, ) -> Result { + // block access to images if instance is private and unauthorized, public + let site = blocking(context.pool(), Site::read_local_site).await?; + // The site might not be set up yet + if let Ok(site) = site { + if site.private_instance { + let jwt = req + .cookie("jwt") + .expect("No auth header for picture access"); + if get_local_user_view_from_jwt(jwt.value(), context.pool(), context.secret()) + .await + .is_err() + { + return Ok(HttpResponse::Unauthorized().finish()); + }; + } + } let name = &filename.into_inner(); // If there are no query params, the URL is original