commit: 07e7c80bc9e919cd92ca9dda1e21384142e5bd77
parent: a716543267469642b326dc5061528d8307ea6633
Author: lain <lain@soykaf.club>
Date: Wed, 6 May 2020 09:14:05 +0000
Merge branch 'plug-if-unless-func-options-refactoring' into 'develop'
Refactoring of :if_func / :unless_func plug options
See merge request pleroma/pleroma!2446
Diffstat:
9 files changed, 109 insertions(+), 24 deletions(-)
diff --git a/lib/pleroma/plugs/ensure_authenticated_plug.ex b/lib/pleroma/plugs/ensure_authenticated_plug.ex
@@ -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()
diff --git a/lib/pleroma/plugs/federating_plug.ex b/lib/pleroma/plugs/federating_plug.ex
@@ -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)
diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@@ -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
)
# Note: :following and :followers must be served even without authentication (as via :api)
diff --git a/lib/pleroma/web/feed/user_controller.ex b/lib/pleroma/web/feed/user_controller.ex
@@ -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
diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex
@@ -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(
diff --git a/lib/pleroma/web/static_fe/static_fe_controller.ex b/lib/pleroma/web/static_fe/static_fe_controller.ex
@@ -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"]
diff --git a/lib/pleroma/web/web.ex b/lib/pleroma/web/web.ex
@@ -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 =
diff --git a/test/plugs/ensure_authenticated_plug_test.exs b/test/plugs/ensure_authenticated_plug_test.exs
@@ -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
@@ -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
