[#3053] No auth check in StaticFEController, even on non-federating instances. Adjust...
authorIvan Tashkinov <ivantashkinov@gmail.com>
Fri, 2 Oct 2020 19:18:02 +0000 (22:18 +0300)
committerIvan Tashkinov <ivantashkinov@gmail.com>
Fri, 2 Oct 2020 19:18:02 +0000 (22:18 +0300)
lib/pleroma/web/feed/tag_controller.ex
lib/pleroma/web/feed/user_controller.ex
lib/pleroma/web/router.ex
lib/pleroma/web/static_fe/static_fe_controller.ex
test/support/conn_case.ex
test/web/activity_pub/activity_pub_controller_test.exs
test/web/feed/user_controller_test.exs
test/web/ostatus/ostatus_controller_test.exs
test/web/static_fe/static_fe_controller_test.exs

index 93a8294b7b49ba88968e693e8121597eb1db6742..c348b32c2f74ffc4afab4d9505b630e46ccea3f5 100644 (file)
@@ -10,14 +10,14 @@ defmodule Pleroma.Web.Feed.TagController do
   alias Pleroma.Web.Feed.FeedView
 
   def feed(conn, params) do
-    unless Pleroma.Config.restrict_unauthenticated_access?(:activities, :local) do
+    unless Config.restrict_unauthenticated_access?(:activities, :local) do
       render_feed(conn, params)
     else
       render_error(conn, :not_found, "Not found")
     end
   end
 
-  def render_feed(conn, %{"tag" => raw_tag} = params) do
+  defp render_feed(conn, %{"tag" => raw_tag} = params) do
     {format, tag} = parse_tag(raw_tag)
 
     activities =
index 09ecdedb4c8700de2ff9ed5c4253c817f9d7fbc9..5fbcd82d753b2a2ef123e2e15e24a670debc31c2 100644 (file)
@@ -40,11 +40,11 @@ defmodule Pleroma.Web.Feed.UserController do
     end
   end
 
-  def render_feed(conn, %{"nickname" => nickname} = params) do
+  defp render_feed(conn, %{"nickname" => nickname} = params) do
     format = get_format(conn)
 
     format =
-      if format in ["rss", "atom"] do
+      if format in ["atom", "rss"] do
         format
       else
         "atom"
index 42a9db21da91cf9d902f686210c26cea15419779..e0e92549f9e30b8c918aca2744cebf204f78de95 100644 (file)
@@ -561,12 +561,17 @@ defmodule Pleroma.Web.Router do
     plug(Pleroma.Plugs.StaticFEPlug)
   end
 
+  pipeline :ostatus_no_html do
+    plug(:accepts, ["xml", "rss", "atom", "activity+json", "json"])
+  end
+
   pipeline :oembed do
     plug(:accepts, ["json", "xml"])
   end
 
   scope "/", Pleroma.Web do
-    pipe_through([:ostatus, :http_signature])
+    # Note: no authentication plugs, all endpoints below should only yield public objects
+    pipe_through(:ostatus)
 
     get("/objects/:uuid", OStatus.OStatusController, :object)
     get("/activities/:uuid", OStatus.OStatusController, :activity)
@@ -579,6 +584,10 @@ defmodule Pleroma.Web.Router do
 
     get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed)
     get("/users/:nickname", Feed.UserController, :feed_redirect, as: :user_feed)
+  end
+
+  scope "/", Pleroma.Web do
+    pipe_through(:ostatus_no_html)
 
     get("/tags/:tag", Feed.TagController, :feed, as: :tag_feed)
   end
index a7a891b133ea6e8fd28b7601cb7d31385803f7f0..b1c62f5b072fdf5a99d8171afb2868428b67e17a 100644 (file)
@@ -17,70 +17,9 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
   plug(:put_view, Pleroma.Web.StaticFE.StaticFEView)
   plug(:assign_id)
 
-  plug(Pleroma.Plugs.EnsureAuthenticatedPlug,
-    unless_func: &Pleroma.Web.FederatingPlug.federating?/1
-  )
-
   @page_keys ["max_id", "min_id", "limit", "since_id", "order"]
 
-  defp get_title(%Object{data: %{"name" => name}}) when is_binary(name),
-    do: name
-
-  defp get_title(%Object{data: %{"summary" => summary}}) when is_binary(summary),
-    do: summary
-
-  defp get_title(_), do: nil
-
-  defp not_found(conn, message) do
-    conn
-    |> put_status(404)
-    |> render("error.html", %{message: message, meta: ""})
-  end
-
-  defp get_counts(%Activity{} = activity) do
-    %Object{data: data} = Object.normalize(activity)
-
-    %{
-      likes: data["like_count"] || 0,
-      replies: data["repliesCount"] || 0,
-      announces: data["announcement_count"] || 0
-    }
-  end
-
-  defp represent(%Activity{} = activity), do: represent(activity, false)
-
-  defp represent(%Activity{object: %Object{data: data}} = activity, selected) do
-    {:ok, user} = User.get_or_fetch(activity.object.data["actor"])
-
-    link =
-      case user.local do
-        true -> Helpers.o_status_url(Pleroma.Web.Endpoint, :notice, activity)
-        _ -> data["url"] || data["external_url"] || data["id"]
-      end
-
-    content =
-      if data["content"] do
-        data["content"]
-        |> Pleroma.HTML.filter_tags()
-        |> Pleroma.Emoji.Formatter.emojify(Map.get(data, "emoji", %{}))
-      else
-        nil
-      end
-
-    %{
-      user: User.sanitize_html(user),
-      title: get_title(activity.object),
-      content: content,
-      attachment: data["attachment"],
-      link: link,
-      published: data["published"],
-      sensitive: data["sensitive"],
-      selected: selected,
-      counts: get_counts(activity),
-      id: activity.id
-    }
-  end
-
+  @doc "Renders requested local public activity"
   def show(%{assigns: %{notice_id: notice_id}} = conn, _params) do
     with %Activity{local: true} = activity <-
            Activity.get_by_id_with_object(notice_id),
@@ -106,6 +45,7 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
     end
   end
 
+  @doc "Renders public activities of requested user"
   def show(%{assigns: %{username_or_id: username_or_id}} = conn, params) do
     case User.get_cached_by_nickname_or_id(username_or_id) do
       %User{} = user ->
@@ -118,7 +58,7 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
 
         timeline =
           user
-          |> ActivityPub.fetch_user_activities(nil, params)
+          |> ActivityPub.fetch_user_activities(_reading_user = nil, params)
           |> Enum.map(&represent/1)
 
         prev_page_id =
@@ -166,6 +106,64 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
     end
   end
 
+  defp get_title(%Object{data: %{"name" => name}}) when is_binary(name),
+    do: name
+
+  defp get_title(%Object{data: %{"summary" => summary}}) when is_binary(summary),
+    do: summary
+
+  defp get_title(_), do: nil
+
+  defp not_found(conn, message) do
+    conn
+    |> put_status(404)
+    |> render("error.html", %{message: message, meta: ""})
+  end
+
+  defp get_counts(%Activity{} = activity) do
+    %Object{data: data} = Object.normalize(activity)
+
+    %{
+      likes: data["like_count"] || 0,
+      replies: data["repliesCount"] || 0,
+      announces: data["announcement_count"] || 0
+    }
+  end
+
+  defp represent(%Activity{} = activity), do: represent(activity, false)
+
+  defp represent(%Activity{object: %Object{data: data}} = activity, selected) do
+    {:ok, user} = User.get_or_fetch(activity.object.data["actor"])
+
+    link =
+      case user.local do
+        true -> Helpers.o_status_url(Pleroma.Web.Endpoint, :notice, activity)
+        _ -> data["url"] || data["external_url"] || data["id"]
+      end
+
+    content =
+      if data["content"] do
+        data["content"]
+        |> Pleroma.HTML.filter_tags()
+        |> Pleroma.Emoji.Formatter.emojify(Map.get(data, "emoji", %{}))
+      else
+        nil
+      end
+
+    %{
+      user: User.sanitize_html(user),
+      title: get_title(activity.object),
+      content: content,
+      attachment: data["attachment"],
+      link: link,
+      published: data["published"],
+      sensitive: data["sensitive"],
+      selected: selected,
+      counts: get_counts(activity),
+      id: activity.id
+    }
+  end
+
   defp assign_id(%{path_info: ["notice", notice_id]} = conn, _opts),
     do: assign(conn, :notice_id, notice_id)
 
index 7ef6812589b1f9c0e364bff2aae1b9a328f05a4e..7b28d70e70a398b86177d5ae0702e22298d460b3 100644 (file)
@@ -111,28 +111,6 @@ defmodule Pleroma.Web.ConnCase do
       defp json_response_and_validate_schema(conn, _status) do
         flunk("Response schema not found for #{conn.method} #{conn.request_path} #{conn.status}")
       end
-
-      defp ensure_federating_or_authenticated(conn, url, user) do
-        initial_setting = Config.get([:instance, :federating])
-        on_exit(fn -> Config.put([:instance, :federating], initial_setting) end)
-
-        Config.put([:instance, :federating], false)
-
-        conn
-        |> get(url)
-        |> response(403)
-
-        conn
-        |> assign(:user, user)
-        |> get(url)
-        |> response(200)
-
-        Config.put([:instance, :federating], true)
-
-        conn
-        |> get(url)
-        |> response(200)
-      end
     end
   end
 
index 0517571f28f7cde9cbfa7bc36cb9fe274da313e9..ab57b65237313e2506636338bcda43a1584ec498 100644 (file)
@@ -33,6 +33,25 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
 
   setup do: clear_config([:instance, :federating], true)
 
+  defp ensure_federating_or_authenticated(conn, url, user) do
+    Config.put([:instance, :federating], false)
+
+    conn
+    |> get(url)
+    |> response(403)
+
+    conn
+    |> assign(:user, user)
+    |> get(url)
+    |> response(200)
+
+    Config.put([:instance, :federating], true)
+
+    conn
+    |> get(url)
+    |> response(200)
+  end
+
   describe "/relay" do
     setup do: clear_config([:instance, :allow_relay])
 
index 9a5610baa9bd78ae47cbfe3ef8e232e81dad7030..7383e82cc9c873575fcf5f64ef4baeaa8851ac34 100644 (file)
@@ -13,7 +13,7 @@ defmodule Pleroma.Web.Feed.UserControllerTest do
   alias Pleroma.User
   alias Pleroma.Web.CommonAPI
 
-  setup do: clear_config([:instance, :federating], true)
+  setup do: clear_config([:static_fe, :enabled], false)
 
   describe "feed" do
     setup do: clear_config([:feed])
@@ -192,6 +192,16 @@ defmodule Pleroma.Web.Feed.UserControllerTest do
              |> get(user_feed_path(conn, :feed, user.nickname))
              |> response(404)
     end
+
+    test "does not require authentication on non-federating instances", %{conn: conn} do
+      clear_config([:instance, :federating], false)
+      user = insert(:user)
+
+      conn
+      |> put_req_header("accept", "application/rss+xml")
+      |> get("/users/#{user.nickname}/feed.rss")
+      |> response(200)
+    end
   end
 
   # Note: see ActivityPubControllerTest for JSON format tests
index ee498f4b555ea10022ac07dbe91f2f6713f88900..65b2c22dbe45bb16ac9a5335f5a3f5ae8675603d 100644 (file)
@@ -7,7 +7,6 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
 
   import Pleroma.Factory
 
-  alias Pleroma.Config
   alias Pleroma.Object
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
@@ -21,7 +20,7 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
     :ok
   end
 
-  setup do: clear_config([:instance, :federating], true)
+  setup do: clear_config([:static_fe, :enabled], false)
 
   describe "Mastodon compatibility routes" do
     setup %{conn: conn} do
@@ -215,15 +214,16 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
       assert response(conn, 404)
     end
 
-    test "it requires authentication if instance is NOT federating", %{
+    test "does not require authentication on non-federating instances", %{
       conn: conn
     } do
-      user = insert(:user)
+      clear_config([:instance, :federating], false)
       note_activity = insert(:note_activity)
 
-      conn = put_req_header(conn, "accept", "text/html")
-
-      ensure_federating_or_authenticated(conn, "/notice/#{note_activity.id}", user)
+      conn
+      |> put_req_header("accept", "text/html")
+      |> get("/notice/#{note_activity.id}")
+      |> response(200)
     end
   end
 
@@ -325,14 +325,16 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do
       |> response(404)
     end
 
-    test "it requires authentication if instance is NOT federating", %{
+    test "does not require authentication on non-federating instances", %{
       conn: conn,
       note_activity: note_activity
     } do
-      user = insert(:user)
-      conn = put_req_header(conn, "accept", "text/html")
+      clear_config([:instance, :federating], false)
 
-      ensure_federating_or_authenticated(conn, "/notice/#{note_activity.id}/embed_player", user)
+      conn
+      |> put_req_header("accept", "text/html")
+      |> get("/notice/#{note_activity.id}/embed_player")
+      |> response(200)
     end
   end
 end
index 1598bf6752ed2f19e6214e7137462b2ec80893f0..bab0b0a7b74d32302a8701c866cadff483e78eae 100644 (file)
@@ -2,14 +2,12 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
   use Pleroma.Web.ConnCase
 
   alias Pleroma.Activity
-  alias Pleroma.Config
   alias Pleroma.Web.ActivityPub.Transmogrifier
   alias Pleroma.Web.CommonAPI
 
   import Pleroma.Factory
 
   setup_all do: clear_config([:static_fe, :enabled], true)
-  setup do: clear_config([:instance, :federating], true)
 
   setup %{conn: conn} do
     conn = put_req_header(conn, "accept", "text/html")
@@ -70,8 +68,15 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
       refute html =~ ">test29<"
     end
 
-    test "it requires authentication if instance is NOT federating", %{conn: conn, user: user} do
-      ensure_federating_or_authenticated(conn, "/users/#{user.nickname}", user)
+    test "does not require authentication on non-federating instances", %{
+      conn: conn,
+      user: user
+    } do
+      clear_config([:instance, :federating], false)
+
+      conn = get(conn, "/users/#{user.nickname}")
+
+      assert html_response(conn, 200) =~ user.nickname
     end
   end
 
@@ -183,10 +188,17 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
       assert html_response(conn, 302) =~ "redirected"
     end
 
-    test "it requires authentication if instance is NOT federating", %{conn: conn, user: user} do
+    test "does not require authentication on non-federating instances", %{
+      conn: conn,
+      user: user
+    } do
+      clear_config([:instance, :federating], false)
+
       {:ok, activity} = CommonAPI.post(user, %{status: "testing a thing!"})
 
-      ensure_federating_or_authenticated(conn, "/notice/#{activity.id}", user)
+      conn = get(conn, "/notice/#{activity.id}")
+
+      assert html_response(conn, 200) =~ "testing a thing!"
     end
   end
 end