From 6c3e984ad16ac07049b74dcbb8b386c87ce2fde5 Mon Sep 17 00:00:00 2001
From: Nutomic <me@nutomic.com>
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 47e6dba2..ca9cc3da 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 75ec9f29..54fc1666 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 fe7e6c25..edf85ebd 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 1c0c01a8..7286ea5a 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 fabcc1ad..3dd66e58 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 1048101d..57e2a5ce 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 193dd686..edb8a759 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::<Actor>(identifier, context, &mut 0).await?;
+      let id = webfinger_resolve_actor::<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 00fbf67f..3f7eeeab 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<SearchableObjects, LemmyError> {
   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::<ApubPerson>(identifier, context, request_counter).await?;
-          Ok(SearchableObjects::Person(
-            ObjectId::new(id)
-              .dereference(context, instance, request_counter)
-              .await?,
-          ))
+          webfinger_resolve_actor::<ApubPerson>(identifier, local_only, context, request_counter)
+            .await?
         }
         Some('!') => {
-          let id =
-            webfinger_resolve_actor::<ApubCommunity>(identifier, context, request_counter).await?;
-          Ok(SearchableObjects::Community(
-            ObjectId::new(id)
-              .dereference(context, instance, request_counter)
-              .await?,
-          ))
+          webfinger_resolve_actor::<ApubCommunity>(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 80dd3bc6..428c8d59 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<Kind>(
   identifier: &str,
+  local_only: bool,
   context: &LemmyContext,
   request_counter: &mut i32,
 ) -> Result<DbUrl, LemmyError>
@@ -68,9 +69,14 @@ where
     .filter_map(|l| l.href.clone())
     .collect();
   for l in links {
-    let object = ObjectId::<Kind>::new(l)
-      .dereference(context, local_instance(context), request_counter)
-      .await;
+    let object_id = ObjectId::<Kind>::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 af1f4d1a..aa10658b 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::<Vec<MentionData>>();
 
   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::<ApubPerson>(&identifier, context, request_counter).await;
+      webfinger_resolve_actor::<ApubPerson>(&identifier, true, context, request_counter).await;
     if let Ok(actor_id) = actor_id {
       let actor_id: ObjectId<ApubPerson> = ObjectId::new(actor_id);
       addressed_ccs.push(actor_id.to_string().parse()?);
-- 
2.44.1