Only allow authenticated users to fetch remote objects (#2493)

* Only allow authenticated users to fetch remote objects

* try to fix api tests

diff --git a/api_tests/src/comment.spec.ts b/api_tests/src/comment.spec.ts
index 47e6dba2d86bbfd6e8bc2325c9636c6b525dba07..ca9cc3da5c5af9ee5e6a32ba5290008183137de5 100644 (file)
--- 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 75ec9f29f8c0e05292417af8ba005ecbd9ae2510..54fc16669d2e66572db31de619220ecde02422bb 100644 (file)
--- 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 fe7e6c25cbe6c25caeb8c4ebbabaf640f6433cc6..edf85ebd2537ac6794edd44b7d616388b86f6f48 100644 (file)
--- 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 1c0c01a8d05f8159287fe14d6d19722967db8ec3..7286ea5af2f70658aa745f06f7f7368eac2f12a6 100644 (file)
--- 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 fabcc1add78a3c6068d903e536c5c25a9cdbbf6e..3dd66e581d30c59d25c4431bce65f1cf305ae60a 100644 (file)
--- 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 1048101d60ea38b39cf93b0874f8f887ec993176..57e2a5cea9e3f3299a18a4d751b98c4d62db01c1 100644 (file)
--- 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 193dd686567ef92f01c86f90f608a0b5f4f591f8..edb8a759b02acbd88e811c7a6ff4231a247822ee 100644 (file)
--- 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 00fbf67fe8795ce71ca3eaac889664d95574ca4d..3f7eeeab152219f99de358af569bc0102a1dda13 100644 (file)
--- 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 80dd3bc6ebc8129dc961a6bdc07e778b6b0da84e..428c8d59a41c8ebffb40b722a989180f8638f3ca 100644 (file)
--- 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 af1f4d1a9fe419306d3f40a0260c22f65ca90047..aa10658b394cb46451bb0590d643d097c12f73d8 100644 (file)
--- 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()?);