From 6c3e984ad16ac07049b74dcbb8b386c87ce2fde5 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Thu, 13 Oct 2022 16:30:31 +0000 Subject: [PATCH] Only allow authenticated users to fetch remote objects (#2493) * Only allow authenticated users to fetch remote objects * try to fix api tests --- api_tests/src/comment.spec.ts | 5 ++- api_tests/src/follow.spec.ts | 4 +- api_tests/src/post.spec.ts | 6 ++- api_tests/src/shared.ts | 4 +- api_tests/src/user.spec.ts | 5 +++ crates/api/src/site/resolve_object.rs | 4 +- crates/apub/src/fetcher/mod.rs | 2 +- crates/apub/src/fetcher/search.rs | 56 ++++++++++++--------------- crates/apub/src/fetcher/webfinger.rs | 12 ++++-- crates/apub/src/mentions.rs | 3 +- 10 files changed, 53 insertions(+), 48 deletions(-) diff --git a/api_tests/src/comment.spec.ts b/api_tests/src/comment.spec.ts index 47e6dba2d..ca9cc3da5 100644 --- a/api_tests/src/comment.spec.ts +++ b/api_tests/src/comment.spec.ts @@ -30,6 +30,7 @@ import { unfollows, getComments, getCommentParentId, + resolveCommunity, } from './shared'; let postRes: PostResponse; @@ -293,8 +294,8 @@ test('Comment Search', async () => { test('A and G subscribe to B (center) A posts, G mentions B, it gets announced to A', async () => { // Create a local post - let alphaCommunity = await createCommunity(alpha, "main"); - let alphaPost = await createPost(alpha, alphaCommunity.community_view.community.id); + let alphaCommunity = (await resolveCommunity(alpha, "!main@lemmy-alpha:8541")).community.unwrap(); + let alphaPost = await createPost(alpha, alphaCommunity.community.id); expect(alphaPost.post_view.community.local).toBe(true); // Make sure gamma sees it diff --git a/api_tests/src/follow.spec.ts b/api_tests/src/follow.spec.ts index 75ec9f29f..54fc16669 100644 --- a/api_tests/src/follow.spec.ts +++ b/api_tests/src/follow.spec.ts @@ -37,7 +37,7 @@ test('Follow federated community', async () => { c => c.community.local == false ).community.id; expect(remoteCommunityId).toBeDefined(); - expect(site.my_user.unwrap().follows.length).toBe(1); + expect(site.my_user.unwrap().follows.length).toBe(2); // Test an unfollow let unfollow = await followCommunity(alpha, false, remoteCommunityId); @@ -45,5 +45,5 @@ test('Follow federated community', async () => { // Make sure you are unsubbed locally let siteUnfollowCheck = await getSite(alpha); - expect(siteUnfollowCheck.my_user.unwrap().follows.length).toBe(0); + expect(siteUnfollowCheck.my_user.unwrap().follows.length).toBe(1); }); diff --git a/api_tests/src/post.spec.ts b/api_tests/src/post.spec.ts index fe7e6c25c..edf85ebd2 100644 --- a/api_tests/src/post.spec.ts +++ b/api_tests/src/post.spec.ts @@ -32,7 +32,8 @@ import { registerUser, API, getSite, - unfollows + unfollows, + resolveCommunity } from './shared'; let betaCommunity: CommunityView; @@ -226,7 +227,8 @@ test('Delete a post', async () => { }); test('Remove a post from admin and community on different instance', async () => { - let postRes = await createPost(gamma, betaCommunity.community.id); + let gammaCommunity = await resolveCommunity(gamma, betaCommunity.community.actor_id); + let postRes = await createPost(gamma, gammaCommunity.community.unwrap().community.id); let alphaPost = (await resolvePost(alpha, postRes.post_view.post)).post.unwrap(); let removedPost = await removePost(alpha, true, alphaPost.post); diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 1c0c01a8d..7286ea5af 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -174,9 +174,9 @@ export async function setupLogins() { editSiteForm.auth = epsilon.auth.unwrap(); await epsilon.client.editSite(editSiteForm); - // Create the main beta community, follow it + // Create the main alpha/beta communities + await createCommunity(alpha, "main"); await createCommunity(beta, "main"); - await followBeta(beta); } export async function createPost( diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index fabcc1add..3dd66e581 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -19,8 +19,13 @@ import { API, resolveComment, saveUserSettingsFederated, + setupLogins, } from './shared'; +beforeAll(async () => { + await setupLogins(); +}); + let apShortname: string; function assertUserFederation(userOne: PersonViewSafe, userTwo: PersonViewSafe) { diff --git a/crates/api/src/site/resolve_object.rs b/crates/api/src/site/resolve_object.rs index 1048101d6..57e2a5cea 100644 --- a/crates/api/src/site/resolve_object.rs +++ b/crates/api/src/site/resolve_object.rs @@ -5,7 +5,7 @@ use lemmy_api_common::{ site::{ResolveObject, ResolveObjectResponse}, utils::{blocking, check_private_instance, get_local_user_view_from_jwt_opt}, }; -use lemmy_apub::fetcher::search::{search_by_apub_id, SearchableObjects}; +use lemmy_apub::fetcher::search::{search_query_to_object_id, SearchableObjects}; use lemmy_db_schema::{newtypes::PersonId, utils::DbPool}; use lemmy_db_views::structs::{CommentView, PostView}; use lemmy_db_views_actor::structs::{CommunityView, PersonViewSafe}; @@ -27,7 +27,7 @@ impl Perform for ResolveObject { .await?; check_private_instance(&local_user_view, context.pool()).await?; - let res = search_by_apub_id(&self.q, context) + let res = search_query_to_object_id(&self.q, local_user_view.is_none(), context) .await .map_err(|e| e.with_message("couldnt_find_object"))?; convert_response(res, local_user_view.map(|l| l.person.id), context.pool()) diff --git a/crates/apub/src/fetcher/mod.rs b/crates/apub/src/fetcher/mod.rs index 193dd6865..edb8a759b 100644 --- a/crates/apub/src/fetcher/mod.rs +++ b/crates/apub/src/fetcher/mod.rs @@ -45,7 +45,7 @@ where Ok(actor?) } else { // Fetch the actor from its home instance using webfinger - let id = webfinger_resolve_actor::(identifier, context, &mut 0).await?; + let id = webfinger_resolve_actor::(identifier, true, context, &mut 0).await?; let actor: DbActor = blocking(context.pool(), move |conn| { DbActor::read_from_apub_id(conn, &id) }) diff --git a/crates/apub/src/fetcher/search.rs b/crates/apub/src/fetcher/search.rs index 00fbf67fe..3f7eeeab1 100644 --- a/crates/apub/src/fetcher/search.rs +++ b/crates/apub/src/fetcher/search.rs @@ -11,52 +11,44 @@ use lemmy_websocket::LemmyContext; use serde::Deserialize; use url::Url; -/// Attempt to parse the query as URL, and fetch an ActivityPub object from it. -/// -/// Some working examples for use with the `docker/federation/` setup: -/// http://lemmy_alpha:8541/c/main, or !main@lemmy_alpha:8541 -/// http://lemmy_beta:8551/u/lemmy_alpha, or @lemmy_beta@lemmy_beta:8551 -/// http://lemmy_gamma:8561/post/3 -/// http://lemmy_delta:8571/comment/2 +/// Converts search query to object id. The query can either be an URL, which will be treated as +/// ObjectId directly, or a webfinger identifier (@user@example.com or !community@example.com) +/// which gets resolved to an URL. #[tracing::instrument(skip_all)] -pub async fn search_by_apub_id( +pub async fn search_query_to_object_id( query: &str, + local_only: bool, context: &LemmyContext, ) -> Result { let request_counter = &mut 0; - let instance = local_instance(context); - match Url::parse(query) { - Ok(url) => { - ObjectId::new(url) - .dereference(context, instance, request_counter) - .await - } + let object_id = match Url::parse(query) { + // its already an url, just go with it + Ok(url) => ObjectId::new(url), Err(_) => { + // not an url, try to resolve via webfinger let mut chars = query.chars(); let kind = chars.next(); let identifier = chars.as_str(); - match kind { + let id = match kind { Some('@') => { - let id = - webfinger_resolve_actor::(identifier, context, request_counter).await?; - Ok(SearchableObjects::Person( - ObjectId::new(id) - .dereference(context, instance, request_counter) - .await?, - )) + webfinger_resolve_actor::(identifier, local_only, context, request_counter) + .await? } Some('!') => { - let id = - webfinger_resolve_actor::(identifier, context, request_counter).await?; - Ok(SearchableObjects::Community( - ObjectId::new(id) - .dereference(context, instance, request_counter) - .await?, - )) + webfinger_resolve_actor::(identifier, local_only, context, request_counter) + .await? } - _ => Err(LemmyError::from_message("invalid query")), - } + _ => return Err(LemmyError::from_message("invalid query")), + }; + ObjectId::new(id) } + }; + if local_only { + object_id.dereference_local(context).await + } else { + object_id + .dereference(context, local_instance(context), request_counter) + .await } } diff --git a/crates/apub/src/fetcher/webfinger.rs b/crates/apub/src/fetcher/webfinger.rs index 80dd3bc6e..428c8d59a 100644 --- a/crates/apub/src/fetcher/webfinger.rs +++ b/crates/apub/src/fetcher/webfinger.rs @@ -28,6 +28,7 @@ pub struct WebfingerResponse { #[tracing::instrument(skip_all)] pub(crate) async fn webfinger_resolve_actor( identifier: &str, + local_only: bool, context: &LemmyContext, request_counter: &mut i32, ) -> Result @@ -68,9 +69,14 @@ where .filter_map(|l| l.href.clone()) .collect(); for l in links { - let object = ObjectId::::new(l) - .dereference(context, local_instance(context), request_counter) - .await; + let object_id = ObjectId::::new(l); + let object = if local_only { + object_id.dereference_local(context).await + } else { + object_id + .dereference(context, local_instance(context), request_counter) + .await + }; if object.is_ok() { return object.map(|o| o.actor_id().into()); } diff --git a/crates/apub/src/mentions.rs b/crates/apub/src/mentions.rs index af1f4d1a9..aa10658b3 100644 --- a/crates/apub/src/mentions.rs +++ b/crates/apub/src/mentions.rs @@ -73,10 +73,9 @@ pub async fn collect_non_local_mentions( .collect::>(); for mention in &mentions { - // TODO should it be fetching it every time? let identifier = format!("{}@{}", mention.name, mention.domain); let actor_id = - webfinger_resolve_actor::(&identifier, context, request_counter).await; + webfinger_resolve_actor::(&identifier, true, context, request_counter).await; if let Ok(actor_id) = actor_id { let actor_id: ObjectId = ObjectId::new(actor_id); addressed_ccs.push(actor_id.to_string().parse()?);