[#3053] No auth check in StaticFEController, even on non-federating instances. Adjusted tests.

lib/pleroma/web/feed/tag_controller.ex

@@ -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 =

lib/pleroma/web/feed/user_controller.ex

@@ -40,11 +40,11 @@ def feed(conn, params) 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"

lib/pleroma/web/router.ex

@@ -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

lib/pleroma/web/static_fe/static_fe_controller.ex

@@ -17,12 +17,95 @@ 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"]
 
+  @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),
+         true <- Visibility.is_public?(activity.object),
+         %User{} = user <- User.get_by_ap_id(activity.object.data["actor"]) do
+      meta = Metadata.build_tags(%{activity_id: notice_id, object: activity.object, user: user})
+
+      timeline =
+        activity.object.data["context"]
+        |> ActivityPub.fetch_activities_for_context(%{})
+        |> Enum.reverse()
+        |> Enum.map(&represent(&1, &1.object.id == activity.object.id))
+
+      render(conn, "conversation.html", %{activities: timeline, meta: meta})
+    else
+      %Activity{object: %Object{data: data}} ->
+        conn
+        |> put_status(:found)
+        |> redirect(external: data["url"] || data["external_url"] || data["id"])
+
+      _ ->
+        not_found(conn, "Post not found.")
+    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 ->
+        meta = Metadata.build_tags(%{user: user})
+
+        params =
+          params
+          |> Map.take(@page_keys)
+          |> Map.new(fn {k, v} -> {String.to_existing_atom(k), v} end)
+
+        timeline =
+          user
+          |> ActivityPub.fetch_user_activities(_reading_user = nil, params)
+          |> Enum.map(&represent/1)
+
+        prev_page_id =
+          (params["min_id"] || params["max_id"]) &&
+            List.first(timeline) && List.first(timeline).id
+
+        next_page_id = List.last(timeline) && List.last(timeline).id
+
+        render(conn, "profile.html", %{
+          user: User.sanitize_html(user),
+          timeline: timeline,
+          prev_page_id: prev_page_id,
+          next_page_id: next_page_id,
+          meta: meta
+        })
+
+      _ ->
+        not_found(conn, "User not found.")
+    end
+  end
+
+  def show(%{assigns: %{object_id: _}} = conn, _params) do
+    url = Helpers.url(conn) <> conn.request_path
+
+    case Activity.get_create_by_object_ap_id_with_object(url) do
+      %Activity{} = activity ->
+        to = Helpers.o_status_path(Pleroma.Web.Endpoint, :notice, activity)
+        redirect(conn, to: to)
+
+      _ ->
+        not_found(conn, "Post not found.")
+    end
+  end
+
+  def show(%{assigns: %{activity_id: _}} = conn, _params) do
+    url = Helpers.url(conn) <> conn.request_path
+
+    case Activity.get_by_ap_id(url) do
+      %Activity{} = activity ->
+        to = Helpers.o_status_path(Pleroma.Web.Endpoint, :notice, activity)
+        redirect(conn, to: to)
+
+      _ ->
+        not_found(conn, "Post not found.")
+    end
+  end
+
   defp get_title(%Object{data: %{"name" => name}}) when is_binary(name),
     do: name
 
@@ -81,91 +164,6 @@ defp represent(%Activity{object: %Object{data: data}} = activity, selected) do
     }
   end
 
-  def show(%{assigns: %{notice_id: notice_id}} = conn, _params) do
-    with %Activity{local: true} = activity <-
-           Activity.get_by_id_with_object(notice_id),
-         true <- Visibility.is_public?(activity.object),
-         %User{} = user <- User.get_by_ap_id(activity.object.data["actor"]) do
-      meta = Metadata.build_tags(%{activity_id: notice_id, object: activity.object, user: user})
-
-      timeline =
-        activity.object.data["context"]
-        |> ActivityPub.fetch_activities_for_context(%{})
-        |> Enum.reverse()
-        |> Enum.map(&represent(&1, &1.object.id == activity.object.id))
-
-      render(conn, "conversation.html", %{activities: timeline, meta: meta})
-    else
-      %Activity{object: %Object{data: data}} ->
-        conn
-        |> put_status(:found)
-        |> redirect(external: data["url"] || data["external_url"] || data["id"])
-
-      _ ->
-        not_found(conn, "Post not found.")
-    end
-  end
-
-  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 ->
-        meta = Metadata.build_tags(%{user: user})
-
-        params =
-          params
-          |> Map.take(@page_keys)
-          |> Map.new(fn {k, v} -> {String.to_existing_atom(k), v} end)
-
-        timeline =
-          user
-          |> ActivityPub.fetch_user_activities(nil, params)
-          |> Enum.map(&represent/1)
-
-        prev_page_id =
-          (params["min_id"] || params["max_id"]) &&
-            List.first(timeline) && List.first(timeline).id
-
-        next_page_id = List.last(timeline) && List.last(timeline).id
-
-        render(conn, "profile.html", %{
-          user: User.sanitize_html(user),
-          timeline: timeline,
-          prev_page_id: prev_page_id,
-          next_page_id: next_page_id,
-          meta: meta
-        })
-
-      _ ->
-        not_found(conn, "User not found.")
-    end
-  end
-
-  def show(%{assigns: %{object_id: _}} = conn, _params) do
-    url = Helpers.url(conn) <> conn.request_path
-
-    case Activity.get_create_by_object_ap_id_with_object(url) do
-      %Activity{} = activity ->
-        to = Helpers.o_status_path(Pleroma.Web.Endpoint, :notice, activity)
-        redirect(conn, to: to)
-
-      _ ->
-        not_found(conn, "Post not found.")
-    end
-  end
-
-  def show(%{assigns: %{activity_id: _}} = conn, _params) do
-    url = Helpers.url(conn) <> conn.request_path
-
-    case Activity.get_by_ap_id(url) do
-      %Activity{} = activity ->
-        to = Helpers.o_status_path(Pleroma.Web.Endpoint, :notice, activity)
-        redirect(conn, to: to)
-
-      _ ->
-        not_found(conn, "Post not found.")
-    end
-  end
-
   defp assign_id(%{path_info: ["notice", notice_id]} = conn, _opts),
     do: assign(conn, :notice_id, notice_id)
 

test/support/conn_case.ex

@@ -111,28 +111,6 @@ defp json_response_and_validate_schema(
       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
 

test/web/activity_pub/activity_pub_controller_test.exs

@@ -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])
 

test/web/feed/user_controller_test.exs

@@ -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 @@ test "returns 404 when the user is remote", %{conn: conn} 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

test/web/ostatus/ostatus_controller_test.exs

@@ -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 @@ test "404s a non-existing notice", %{conn: conn} 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")
+      conn
-
+      |> put_req_header("accept", "text/html")
-      ensure_federating_or_authenticated(conn, "/notice/#{note_activity.id}", user)
+      |> get("/notice/#{note_activity.id}")
+      |> response(200)
     end
   end
 
@@ -325,14 +325,16 @@ test "404s when attachment isn't audio or video", %{conn: conn} 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)
+      clear_config([:instance, :federating], false)
-      conn = put_req_header(conn, "accept", "text/html")
 
-      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

test/web/static_fe/static_fe_controller_test.exs

@@ -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 @@ test "pagination, page 2", %{conn: conn, user: user} do
       refute html =~ ">test29<"
     end
 
-    test "it requires authentication if instance is NOT federating", %{conn: conn, user: user} do
+    test "does not require authentication on non-federating instances", %{
-      ensure_federating_or_authenticated(conn, "/users/#{user.nickname}", user)
+      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 @@ test "302 for remote cached status", %{conn: conn, user: user} 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