Refactoring of :if_func / :unless_func plug options (general availability). Added...
authorIvan Tashkinov <ivantashkinov@gmail.com>
Thu, 30 Apr 2020 15:19:51 +0000 (18:19 +0300)
committerIvan Tashkinov <ivantashkinov@gmail.com>
Thu, 30 Apr 2020 15:19:51 +0000 (18:19 +0300)
lib/pleroma/plugs/ensure_authenticated_plug.ex
lib/pleroma/plugs/federating_plug.ex
lib/pleroma/web/activity_pub/activity_pub_controller.ex
lib/pleroma/web/feed/user_controller.ex
lib/pleroma/web/ostatus/ostatus_controller.ex
lib/pleroma/web/static_fe/static_fe_controller.ex
lib/pleroma/web/web.ex
test/plugs/ensure_authenticated_plug_test.exs
test/web/plugs/plug_test.exs [new file with mode: 0644]

index 9c8f5597f72764ee9dd31dc6b701a9ce21854299..9d5176e2b66953a3fd50bb77fbc87b5d8cfd957a 100644 (file)
@@ -19,22 +19,7 @@ defmodule Pleroma.Plugs.EnsureAuthenticatedPlug do
     conn
   end
 
-  def perform(conn, options) do
-    perform =
-      cond do
-        options[:if_func] -> options[:if_func].()
-        options[:unless_func] -> !options[:unless_func].()
-        true -> true
-      end
-
-    if perform do
-      fail(conn)
-    else
-      conn
-    end
-  end
-
-  def fail(conn) do
+  def perform(conn, _) do
     conn
     |> render_error(:forbidden, "Invalid credentials.")
     |> halt()
index 7d947339fe1f0e7d00eb7e148ef5a33a7bdd8a9c..09038f3c6142f87b4e6c0e5174418bc20bdba6e2 100644 (file)
@@ -19,6 +19,9 @@ defmodule Pleroma.Web.FederatingPlug do
 
   def federating?, do: Pleroma.Config.get([:instance, :federating])
 
+  # Definition for the use in :if_func / :unless_func plug options
+  def federating?(_conn), do: federating?()
+
   defp fail(conn) do
     conn
     |> put_status(404)
index d625530ecb575586986e22735544299733fe498a..a909516beea6405c302d8a8ed1d2d2ed77e8eb2a 100644 (file)
@@ -34,7 +34,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
 
   plug(
     EnsureAuthenticatedPlug,
-    [unless_func: &FederatingPlug.federating?/0] when action not in @federating_only_actions
+    [unless_func: &FederatingPlug.federating?/1] when action not in @federating_only_actions
   )
 
   plug(
index e27f859299dbc9f3e47dc83601d2bba6b9df7ee6..1b72e23dccd3948992ae52d0fd24a30bd9e098f2 100644 (file)
@@ -27,7 +27,7 @@ defmodule Pleroma.Web.Feed.UserController do
       when format in ["json", "activity+json"] do
     with %{halted: false} = conn <-
            Pleroma.Plugs.EnsureAuthenticatedPlug.call(conn,
-             unless_func: &Pleroma.Web.FederatingPlug.federating?/0
+             unless_func: &Pleroma.Web.FederatingPlug.federating?/1
            ) do
       ActivityPubController.call(conn, :user)
     end
index 6fd3cfce5ade97768e92ce8f88000d5a79ec8013..6971cd9f8c12ee05c00b82c240b07c34aa684da8 100644 (file)
@@ -17,7 +17,7 @@ defmodule Pleroma.Web.OStatus.OStatusController do
   alias Pleroma.Web.Router
 
   plug(Pleroma.Plugs.EnsureAuthenticatedPlug,
-    unless_func: &Pleroma.Web.FederatingPlug.federating?/0
+    unless_func: &Pleroma.Web.FederatingPlug.federating?/1
   )
 
   plug(
index 7a35238d7dd71657124d96a919485c7cd1c93c55..c3efb66513304ac4b254e6d6cc445419a06929f5 100644 (file)
@@ -18,7 +18,7 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
   plug(:assign_id)
 
   plug(Pleroma.Plugs.EnsureAuthenticatedPlug,
-    unless_func: &Pleroma.Web.FederatingPlug.federating?/0
+    unless_func: &Pleroma.Web.FederatingPlug.federating?/1
   )
 
   @page_keys ["max_id", "min_id", "limit", "since_id", "order"]
index 08e42a7e5398737cd14f1f8b9e7c80b1f8c60834..4f9281851dd5d43a2f3812d49cd89a7a509ef320 100644 (file)
@@ -200,11 +200,17 @@ defmodule Pleroma.Web do
 
       @impl Plug
       @doc """
-      If marked as skipped, returns `conn`, otherwise calls `perform/2`.
+      Before-plug hook that
+        * ensures the plug is not skipped
+        * processes `:if_func` / `:unless_func` functional pre-run conditions
+        * adds plug to the list of called plugs and calls `perform/2` if checks are passed
+
       Note: multiple invocations of the same plug (with different or same options) are allowed.
       """
       def call(%Plug.Conn{} = conn, options) do
-        if PlugHelper.plug_skipped?(conn, __MODULE__) do
+        if PlugHelper.plug_skipped?(conn, __MODULE__) ||
+             (options[:if_func] && !options[:if_func].(conn)) ||
+             (options[:unless_func] && options[:unless_func].(conn)) do
           conn
         else
           conn =
index 689fe757f5d00f2d712814dcca9a53a39cfbccb9..4e6142aabbe156483f2b9d85b2922226146f998a 100644 (file)
@@ -27,8 +27,8 @@ defmodule Pleroma.Plugs.EnsureAuthenticatedPlugTest do
   describe "with :if_func / :unless_func options" do
     setup do
       %{
-        true_fn: fn -> true end,
-        false_fn: fn -> false end
+        true_fn: fn _conn -> true end,
+        false_fn: fn _conn -> false end
       }
     end
 
diff --git a/test/web/plugs/plug_test.exs b/test/web/plugs/plug_test.exs
new file mode 100644 (file)
index 0000000..943e484
--- /dev/null
@@ -0,0 +1,91 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.PlugTest do
+  @moduledoc "Tests for the functionality added via `use Pleroma.Web, :plug`"
+
+  alias Pleroma.Plugs.ExpectAuthenticatedCheckPlug
+  alias Pleroma.Plugs.ExpectPublicOrAuthenticatedCheckPlug
+  alias Pleroma.Plugs.PlugHelper
+
+  import Mock
+
+  use Pleroma.Web.ConnCase
+
+  describe "when plug is skipped, " do
+    setup_with_mocks(
+      [
+        {ExpectPublicOrAuthenticatedCheckPlug, [:passthrough], []}
+      ],
+      %{conn: conn}
+    ) do
+      conn = ExpectPublicOrAuthenticatedCheckPlug.skip_plug(conn)
+      %{conn: conn}
+    end
+
+    test "it neither adds plug to called plugs list nor calls `perform/2`, " <>
+           "regardless of :if_func / :unless_func options",
+         %{conn: conn} do
+      for opts <- [%{}, %{if_func: fn _ -> true end}, %{unless_func: fn _ -> false end}] do
+        ret_conn = ExpectPublicOrAuthenticatedCheckPlug.call(conn, opts)
+
+        refute called(ExpectPublicOrAuthenticatedCheckPlug.perform(:_, :_))
+        refute PlugHelper.plug_called?(ret_conn, ExpectPublicOrAuthenticatedCheckPlug)
+      end
+    end
+  end
+
+  describe "when plug is NOT skipped, " do
+    setup_with_mocks([{ExpectAuthenticatedCheckPlug, [:passthrough], []}]) do
+      :ok
+    end
+
+    test "with no pre-run checks, adds plug to called plugs list and calls `perform/2`", %{
+      conn: conn
+    } do
+      ret_conn = ExpectAuthenticatedCheckPlug.call(conn, %{})
+
+      assert called(ExpectAuthenticatedCheckPlug.perform(ret_conn, :_))
+      assert PlugHelper.plug_called?(ret_conn, ExpectAuthenticatedCheckPlug)
+    end
+
+    test "when :if_func option is given, calls the plug only if provided function evals tru-ish",
+         %{conn: conn} do
+      ret_conn = ExpectAuthenticatedCheckPlug.call(conn, %{if_func: fn _ -> false end})
+
+      refute called(ExpectAuthenticatedCheckPlug.perform(:_, :_))
+      refute PlugHelper.plug_called?(ret_conn, ExpectAuthenticatedCheckPlug)
+
+      ret_conn = ExpectAuthenticatedCheckPlug.call(conn, %{if_func: fn _ -> true end})
+
+      assert called(ExpectAuthenticatedCheckPlug.perform(ret_conn, :_))
+      assert PlugHelper.plug_called?(ret_conn, ExpectAuthenticatedCheckPlug)
+    end
+
+    test "if :unless_func option is given, calls the plug only if provided function evals falsy",
+         %{conn: conn} do
+      ret_conn = ExpectAuthenticatedCheckPlug.call(conn, %{unless_func: fn _ -> true end})
+
+      refute called(ExpectAuthenticatedCheckPlug.perform(:_, :_))
+      refute PlugHelper.plug_called?(ret_conn, ExpectAuthenticatedCheckPlug)
+
+      ret_conn = ExpectAuthenticatedCheckPlug.call(conn, %{unless_func: fn _ -> false end})
+
+      assert called(ExpectAuthenticatedCheckPlug.perform(ret_conn, :_))
+      assert PlugHelper.plug_called?(ret_conn, ExpectAuthenticatedCheckPlug)
+    end
+
+    test "allows a plug to be called multiple times (even if it's in called plugs list)", %{
+      conn: conn
+    } do
+      conn = ExpectAuthenticatedCheckPlug.call(conn, %{an_option: :value1})
+      assert called(ExpectAuthenticatedCheckPlug.perform(conn, %{an_option: :value1}))
+
+      assert PlugHelper.plug_called?(conn, ExpectAuthenticatedCheckPlug)
+
+      conn = ExpectAuthenticatedCheckPlug.call(conn, %{an_option: :value2})
+      assert called(ExpectAuthenticatedCheckPlug.perform(conn, %{an_option: :value2}))
+    end
+  end
+end