rate limiter: disable based on if remote ip was found, not on if the plug was enabled

The current rate limiter disable logic won't trigger when the remote ip
is not forwarded, only when the remoteip plug is not enabled, which is
not the case on most instances since it's enabled by default. This
changes the behavior to warn and disable  when the remote ip was not forwarded,
even if the RemoteIP plug is enabled.

Also closes #1620

2 files changed, 28 insertions, 52 deletions

diff --git a/test/plugs/rate_limiter_test.exs b/test/plugs/rate_limiter_test.exs
index 8023271e4..81e2009c8 100644
--- a/test/plugs/rate_limiter_test.exs
+++ b/test/plugs/rate_limiter_test.exs
@@ -3,8 +3,7 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Plugs.RateLimiterTest do
-  use ExUnit.Case, async: true
-  use Plug.Test
+  use Pleroma.Web.ConnCase
 
   alias Pleroma.Config
   alias Pleroma.Plugs.RateLimiter
@@ -36,63 +35,44 @@ defmodule Pleroma.Plugs.RateLimiterTest do
                |> RateLimiter.init()
                |> RateLimiter.action_settings()
     end
+  end
 
-    test "it is disabled for localhost" do
-      Config.put([:rate_limit, @limiter_name], {1, 1})
-      Config.put([Pleroma.Web.Endpoint, :http, :ip], {127, 0, 0, 1})
-      Config.put([Pleroma.Plugs.RemoteIp, :enabled], false)
-
-      assert RateLimiter.disabled?() == true
-    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?(Plug.Conn.assign(build_conn(), :remote_ip_found, false))
+  end
 
-    test "it is disabled for socket" do
-      Config.put([:rate_limit, @limiter_name], {1, 1})
-      Config.put([Pleroma.Web.Endpoint, :http, :ip], {:local, "/path/to/pleroma.sock"})
-      Config.put([Pleroma.Plugs.RemoteIp, :enabled], false)
+  test "it restricts based on config values" do
+    limiter_name = :test_plug_opts
+    scale = 80
+    limit = 5
 
-      assert RateLimiter.disabled?() == true
-    end
+    Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
+    Config.put([:rate_limit, limiter_name], {scale, limit})
 
-    test "it is enabled for socket when remote ip is enabled" do
-      Config.put([:rate_limit, @limiter_name], {1, 1})
-      Config.put([Pleroma.Web.Endpoint, :http, :ip], {:local, "/path/to/pleroma.sock"})
-      Config.put([Pleroma.Plugs.RemoteIp, :enabled], true)
+    plug_opts = RateLimiter.init(name: limiter_name)
+    conn = conn(:get, "/")
 
-      assert RateLimiter.disabled?() == false
+    for i <- 1..5 do
+      conn = RateLimiter.call(conn, plug_opts)
+      assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
+      Process.sleep(10)
     end
 
-    test "it restricts based on config values" do
-      limiter_name = :test_plug_opts
-      scale = 80
-      limit = 5
-
-      Config.put([Pleroma.Web.Endpoint, :http, :ip], {8, 8, 8, 8})
-      Config.put([:rate_limit, limiter_name], {scale, limit})
-
-      plug_opts = RateLimiter.init(name: limiter_name)
-      conn = conn(:get, "/")
-
-      for i <- 1..5 do
-        conn = RateLimiter.call(conn, plug_opts)
-        assert {^i, _} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
-        Process.sleep(10)
-      end
+    conn = RateLimiter.call(conn, plug_opts)
+    assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
+    assert conn.halted
 
-      conn = RateLimiter.call(conn, plug_opts)
-      assert %{"error" => "Throttled"} = Phoenix.ConnTest.json_response(conn, :too_many_requests)
-      assert conn.halted
+    Process.sleep(50)
 
-      Process.sleep(50)
+    conn = conn(:get, "/")
 
-      conn = conn(:get, "/")
+    conn = RateLimiter.call(conn, plug_opts)
+    assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts)
 
-      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.resp_body
-      refute conn.halted
-    end
+    refute conn.status == Plug.Conn.Status.code(:too_many_requests)
+    refute conn.resp_body
+    refute conn.halted
   end
 
   describe "options" do
diff --git a/test/web/mastodon_api/controllers/account_controller_test.exs b/test/web/mastodon_api/controllers/account_controller_test.exs
index 7f7d8cea3..7efccd9c4 100644
--- a/test/web/mastodon_api/controllers/account_controller_test.exs
+++ b/test/web/mastodon_api/controllers/account_controller_test.exs
@@ -756,10 +756,6 @@ defmodule Pleroma.Web.MastodonAPI.AccountControllerTest do
   end
 
   describe "create account by app / rate limit" do
-    clear_config([Pleroma.Plugs.RemoteIp, :enabled]) do
-      Pleroma.Config.put([Pleroma.Plugs.RemoteIp, :enabled], true)
-    end
-
     clear_config([:rate_limit, :app_account_creation]) do
       Pleroma.Config.put([:rate_limit, :app_account_creation], {10_000, 2})
     end