From ecf4e5184906fd5e31fda243d34ffd6b292a5136 Mon Sep 17 00:00:00 2001
From: Tusooa Zhu <tusooa@kazv.moe>
Date: Thu, 5 May 2022 19:20:32 -0400
Subject: [PATCH] Skip cache when /objects or /activities is authenticated

Ref: fix-local-public
---
 .../activity_pub/activity_pub_controller.ex   | 11 +++++++++
 lib/pleroma/web/plugs/cache.ex                | 21 +++++++++-------
 .../activity_pub_controller_test.exs          | 24 +++++++++++++++++++
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
index 20f8bbc2d..b8f63d69d 100644
--- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@@ -84,6 +84,7 @@ def object(%{assigns: assigns} = conn, _) do
          user <- Map.get(assigns, :user, nil),
          {_, true} <- {:visible?, Visibility.visible_for_user?(object, user)} do
       conn
+      |> maybe_skip_cache(user)
       |> assign(:tracking_fun_data, object.id)
       |> set_cache_ttl_for(object)
       |> put_resp_content_type("application/activity+json")
@@ -112,6 +113,7 @@ def activity(%{assigns: assigns} = conn, _) do
          user <- Map.get(assigns, :user, nil),
          {_, true} <- {:visible?, Visibility.visible_for_user?(activity, user)} do
       conn
+      |> maybe_skip_cache(user)
       |> maybe_set_tracking_data(activity)
       |> set_cache_ttl_for(activity)
       |> put_resp_content_type("application/activity+json")
@@ -151,6 +153,15 @@ defp set_cache_ttl_for(conn, entity) do
     assign(conn, :cache_ttl, ttl)
   end
 
+  def maybe_skip_cache(conn, user) do
+    if user do
+      conn
+      |> assign(:skip_cache, true)
+    else
+      conn
+    end
+  end
+
   # GET /relay/following
   def relay_following(conn, _params) do
     with %{halted: false} = conn <- FederatingPlug.call(conn, []) do
diff --git a/lib/pleroma/web/plugs/cache.ex b/lib/pleroma/web/plugs/cache.ex
index aaff36407..667477857 100644
--- a/lib/pleroma/web/plugs/cache.ex
+++ b/lib/pleroma/web/plugs/cache.ex
@@ -97,20 +97,23 @@ defp cache_resp(conn, opts) do
         key = cache_key(conn, opts)
         content_type = content_type(conn)
 
+        should_cache = not Map.get(conn.assigns, :skip_cache, false)
+
         conn =
-          cond do
-            Map.get(conn.assigns, :skip_cache, false) ->
-              conn
-
-            !opts[:tracking_fun] ->
+          unless opts[:tracking_fun] do
+            if should_cache do
               @cachex.put(:web_resp_cache, key, {content_type, body}, ttl: ttl)
-              conn
+            end
 
-            true ->
-              tracking_fun_data = Map.get(conn.assigns, :tracking_fun_data, nil)
+            conn
+          else
+            tracking_fun_data = Map.get(conn.assigns, :tracking_fun_data, nil)
+
+            if should_cache do
               @cachex.put(:web_resp_cache, key, {content_type, body, tracking_fun_data}, ttl: ttl)
+            end
 
-              opts.tracking_fun.(conn, tracking_fun_data)
+            opts.tracking_fun.(conn, tracking_fun_data)
           end
 
         put_resp_header(conn, "x-cache", "MISS from Pleroma")
diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs
index 1923ec479..1c5c40e84 100644
--- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs
@@ -291,6 +291,30 @@ test "it returns a json representation of the object with accept application/ld+
       assert json_response(conn, 200) == ObjectView.render("object.json", %{object: note})
     end
 
+    test "does not cache authenticated response", %{conn: conn} do
+      user = insert(:user)
+      reader = insert(:user)
+
+      {:ok, post} =
+        CommonAPI.post(user, %{status: "test @#{reader.nickname}", visibility: "local"})
+
+      object = Object.normalize(post, fetch: false)
+      uuid = String.split(object.data["id"], "/") |> List.last()
+
+      assert response =
+               conn
+               |> assign(:user, reader)
+               |> put_req_header("accept", "application/activity+json")
+               |> get("/objects/#{uuid}")
+
+      json_response(response, 200)
+
+      conn
+      |> put_req_header("accept", "application/activity+json")
+      |> get("/objects/#{uuid}")
+      |> json_response(404)
+    end
+
     test "it returns 404 for non-public messages", %{conn: conn} do
       note = insert(:direct_note)
       uuid = String.split(note.data["id"], "/") |> List.last()