Merge branch 'fix/1659-rate-limiter' into 'develop'

remote_ip plug adds remote_ip_found flag

Closes #1659

See merge request pleroma/pleroma!2390

lib/pleroma/plugs/rate_limiter/rate_limiter.ex

@@ -110,20 +110,9 @@ defp handle(conn, action_settings) do
   end
 
   def disabled?(conn) do
-    localhost_or_socket =
+    if Map.has_key?(conn.assigns, :remote_ip_found),
-      case Config.get([Pleroma.Web.Endpoint, :http, :ip]) do
+      do: !conn.assigns.remote_ip_found,
-        {127, 0, 0, 1} -> true
+      else: false
-        {0, 0, 0, 0, 0, 0, 0, 1} -> true
-        {:local, _} -> true
-        _ -> false
-      end
-
-    remote_ip_not_found =
-      if Map.has_key?(conn.assigns, :remote_ip_found),
-        do: !conn.assigns.remote_ip_found,
-        else: false
-
-    localhost_or_socket and remote_ip_not_found
   end
 
   @inspect_bucket_not_found {:error, :not_found}

lib/pleroma/plugs/remote_ip.ex

@@ -7,8 +7,6 @@ defmodule Pleroma.Plugs.RemoteIp do
   This is a shim to call [`RemoteIp`](https://git.pleroma.social/pleroma/remote_ip) but with runtime configuration.
   """
 
-  import Plug.Conn
-
   @behaviour Plug
 
   @headers ~w[
@@ -28,12 +26,11 @@ defmodule Pleroma.Plugs.RemoteIp do
 
   def init(_), do: nil
 
-  def call(%{remote_ip: original_remote_ip} = conn, _) do
+  def call(conn, _) do
     config = Pleroma.Config.get(__MODULE__, [])
 
     if Keyword.get(config, :enabled, false) do
-      %{remote_ip: new_remote_ip} = conn = RemoteIp.call(conn, remote_ip_opts(config))
+      RemoteIp.call(conn, remote_ip_opts(config))
-      assign(conn, :remote_ip_found, original_remote_ip != new_remote_ip)
     else
       conn
     end

mix.exs

@@ -183,7 +183,7 @@ defp deps do
       {:flake_id, "~> 0.1.0"},
       {:remote_ip,
        git: "https://git.pleroma.social/pleroma/remote_ip.git",
-       ref: "825dc00aaba5a1b7c4202a532b696b595dd3bcb3"},
+       ref: "b647d0deecaa3acb140854fe4bda5b7e1dc6d1c8"},
       {:captcha,
        git: "https://git.pleroma.social/pleroma/elixir-libraries/elixir-captcha.git",
        ref: "e0f16822d578866e186a0974d65ad58cddc1e2ab"},

mix.lock

@@ -97,7 +97,7 @@
   "quack": {:hex, :quack, "0.1.1", "cca7b4da1a233757fdb44b3334fce80c94785b3ad5a602053b7a002b5a8967bf", [:mix], [{:poison, ">= 1.0.0", [hex: :poison, repo: "hexpm", optional: false]}, {:tesla, "~> 1.2.0", [hex: :tesla, repo: "hexpm", optional: false]}], "hexpm", "d736bfa7444112eb840027bb887832a0e403a4a3437f48028c3b29a2dbbd2543"},
   "ranch": {:hex, :ranch, "1.7.1", "6b1fab51b49196860b733a49c07604465a47bdb78aa10c1c16a3d199f7f8c881", [:rebar3], [], "hexpm", "451d8527787df716d99dc36162fca05934915db0b6141bbdac2ea8d3c7afc7d7"},
   "recon": {:hex, :recon, "2.5.0", "2f7fcbec2c35034bade2f9717f77059dc54eb4e929a3049ca7ba6775c0bd66cd", [:mix, :rebar3], [], "hexpm", "72f3840fedd94f06315c523f6cecf5b4827233bed7ae3fe135b2a0ebeab5e196"},
-  "remote_ip": {:git, "https://git.pleroma.social/pleroma/remote_ip.git", "825dc00aaba5a1b7c4202a532b696b595dd3bcb3", [ref: "825dc00aaba5a1b7c4202a532b696b595dd3bcb3"]},
+  "remote_ip": {:git, "https://git.pleroma.social/pleroma/remote_ip.git", "b647d0deecaa3acb140854fe4bda5b7e1dc6d1c8", [ref: "b647d0deecaa3acb140854fe4bda5b7e1dc6d1c8"]},
   "sleeplocks": {:hex, :sleeplocks, "1.1.1", "3d462a0639a6ef36cc75d6038b7393ae537ab394641beb59830a1b8271faeed3", [:rebar3], [], "hexpm", "84ee37aeff4d0d92b290fff986d6a95ac5eedf9b383fadfd1d88e9b84a1c02e1"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.5", "6eaf7ad16cb568bb01753dbbd7a95ff8b91c7979482b95f38443fe2c8852a79b", [:make, :mix, :rebar3], [], "hexpm", "13104d7897e38ed7f044c4de953a6c28597d1c952075eb2e328bc6d6f2bfc496"},
   "sweet_xml": {:hex, :sweet_xml, "0.6.6", "fc3e91ec5dd7c787b6195757fbcf0abc670cee1e4172687b45183032221b66b8", [:mix], [], "hexpm", "2e1ec458f892ffa81f9f8386e3f35a1af6db7a7a37748a64478f13163a1f3573"},

test/plugs/rate_limiter_test.exs

@@ -5,8 +5,10 @@
 defmodule Pleroma.Plugs.RateLimiterTest do
   use Pleroma.Web.ConnCase
 
+  alias Phoenix.ConnTest
   alias Pleroma.Config
   alias Pleroma.Plugs.RateLimiter
+  alias Plug.Conn
 
   import Pleroma.Factory
   import Pleroma.Tests.Helpers, only: [clear_config: 1, clear_config: 2]
@@ -36,8 +38,15 @@ test "config is required for plug to work" do
   end
 
   test "it is disabled if it remote ip plug is enabled but no remote ip is found" do
-    Config.put([Pleroma.Web.Endpoint, :http, :ip], {127, 0, 0, 1})
+    assert RateLimiter.disabled?(Conn.assign(build_conn(), :remote_ip_found, false))
-    assert RateLimiter.disabled?(Plug.Conn.assign(build_conn(), :remote_ip_found, false))
+  end
+
+  test "it is enabled if remote ip found" do
+    refute RateLimiter.disabled?(Conn.assign(build_conn(), :remote_ip_found, true))
+  end
+
+  test "it is enabled if remote_ip_found flag doesn't exist" do
+    refute RateLimiter.disabled?(build_conn())
   end
 
   test "it restricts based on config values" do
@@ -58,7 +67,7 @@ test "it restricts based on config values" do
     end
 
     conn = RateLimiter.call(conn, plug_opts)
-    assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
+    assert %{"error" => "Throttled"} = ConnTest.json_response(conn, :too_many_requests)
     assert conn.halted
 
     Process.sleep(50)
@@ -68,7 +77,7 @@ test "it restricts based on config values" do
     conn = RateLimiter.call(conn, plug_opts)
     assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
 
-    refute conn.status == Plug.Conn.Status.code(:too_many_requests)
+    refute conn.status == Conn.Status.code(:too_many_requests)
     refute conn.resp_body
     refute conn.halted
   end
@@ -98,7 +107,7 @@ test "`params` option allows different queries to be tracked independently" do
       plug_opts = RateLimiter.init(name: limiter_name, params: ["id"])
 
       conn = build_conn(:get, "/?id=1")
-      conn = Plug.Conn.fetch_query_params(conn)
+      conn = Conn.fetch_query_params(conn)
       conn_2 = build_conn(:get, "/?id=2")
 
       RateLimiter.call(conn, plug_opts)
@@ -119,7 +128,7 @@ test "it supports combination of options modifying bucket name" do
       id = "100"
 
       conn = build_conn(:get, "/?id=#{id}")
-      conn = Plug.Conn.fetch_query_params(conn)
+      conn = Conn.fetch_query_params(conn)
       conn_2 = build_conn(:get, "/?id=#{101}")
 
       RateLimiter.call(conn, plug_opts)
@@ -147,13 +156,13 @@ test "are restricted based on remote IP" do
 
       conn = RateLimiter.call(conn, plug_opts)
 
-      assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
+      assert %{"error" => "Throttled"} = ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
 
       conn_2 = RateLimiter.call(conn_2, plug_opts)
       assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts)
 
-      refute conn_2.status == Plug.Conn.Status.code(:too_many_requests)
+      refute conn_2.status == Conn.Status.code(:too_many_requests)
       refute conn_2.resp_body
       refute conn_2.halted
     end
@@ -187,7 +196,7 @@ test "can have limits separate from unauthenticated connections" do
 
       conn = RateLimiter.call(conn, plug_opts)
 
-      assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
+      assert %{"error" => "Throttled"} = ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
     end
 
@@ -210,12 +219,12 @@ test "different users are counted independently" do
       end
 
       conn = RateLimiter.call(conn, plug_opts)
-      assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
+      assert %{"error" => "Throttled"} = ConnTest.json_response(conn, :too_many_requests)
       assert conn.halted
 
       conn_2 = RateLimiter.call(conn_2, plug_opts)
       assert {1, 4} = RateLimiter.inspect_bucket(conn_2, limiter_name, plug_opts)
-      refute conn_2.status == Plug.Conn.Status.code(:too_many_requests)
+      refute conn_2.status == Conn.Status.code(:too_many_requests)
       refute conn_2.resp_body
       refute conn_2.halted
     end