From 9b225db7d86289fb9d9c51f62e6ec29f6c07f60d Mon Sep 17 00:00:00 2001
From: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Tue, 14 Jul 2020 11:58:41 +0300
Subject: [PATCH] [#1940] Applied rate limit for requests with bad
 `admin_token`. Added doc warnings on `admin_token` setting.

---
 config/description.exs                          |  6 ++++--
 docs/configuration/cheatsheet.md                |  2 ++
 .../plugs/admin_secret_authentication_plug.ex   | 17 +++++++++++++----
 .../admin_secret_authentication_plug_test.exs   |  9 +++++++++
 4 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/config/description.exs b/config/description.exs
index 84dcdb87e..8ec4b712f 100644
--- a/config/description.exs
+++ b/config/description.exs
@@ -2008,13 +2008,15 @@
     label: "Pleroma Admin Token",
     type: :group,
     description:
-      "Allows to set a token that can be used to authenticate with the admin api without using an actual user by giving it as the `admin_token` parameter",
+      "Allows to set a token that can be used to authenticate with the admin api without using an actual user by giving it as the `admin_token` parameter (risky; use HTTP Basic Auth or OAuth-based authentication if possible)",
     children: [
       %{
         key: :admin_token,
         type: :string,
         description: "Admin token",
-        suggestions: ["We recommend a secure random string or UUID"]
+        suggestions: [
+          "We recommend NOT setting the value do to increased security risk; if set, use a secure random long string or UUID (and change it as often as possible)"
+        ]
       }
     ]
   },
diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md
index f796330f1..24b162ce7 100644
--- a/docs/configuration/cheatsheet.md
+++ b/docs/configuration/cheatsheet.md
@@ -815,6 +815,8 @@ or
 curl -H "X-Admin-Token: somerandomtoken" "http://localhost:4000/api/pleroma/admin/users/invites"
 ```
 
+Warning: it's discouraged to use this feature because of the associated security risk: static / rarely changed instance-wide token is much weaker compared to email-password pair of a real admin user; consider using HTTP Basic Auth or OAuth-based authentication instead.
+
 ### :auth
 
 * `Pleroma.Web.Auth.PleromaAuthenticator`: default database authenticator.
diff --git a/lib/pleroma/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/plugs/admin_secret_authentication_plug.ex
index ff0328d4a..2e54df47a 100644
--- a/lib/pleroma/plugs/admin_secret_authentication_plug.ex
+++ b/lib/pleroma/plugs/admin_secret_authentication_plug.ex
@@ -5,15 +5,19 @@
 defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do
   import Plug.Conn
 
-  alias Pleroma.User
   alias Pleroma.Plugs.OAuthScopesPlug
+  alias Pleroma.Plugs.RateLimiter
+  alias Pleroma.User
 
   def init(options) do
     options
   end
 
   def secret_token do
-    Pleroma.Config.get(:admin_token)
+    case Pleroma.Config.get(:admin_token) do
+      blank when blank in [nil, ""] -> nil
+      token -> token
+    end
   end
 
   def call(%{assigns: %{user: %User{}}} = conn, _), do: conn
@@ -30,7 +34,7 @@ def authenticate(%{params: %{"admin_token" => admin_token}} = conn) do
     if admin_token == secret_token() do
       assign_admin_user(conn)
     else
-      conn
+      handle_bad_token(conn)
     end
   end
 
@@ -38,8 +42,9 @@ def authenticate(conn) do
     token = secret_token()
 
     case get_req_header(conn, "x-admin-token") do
+      blank when blank in [[], [""]] -> conn
       [^token] -> assign_admin_user(conn)
-      _ -> conn
+      _ -> handle_bad_token(conn)
     end
   end
 
@@ -48,4 +53,8 @@ defp assign_admin_user(conn) do
     |> assign(:user, %User{is_admin: true})
     |> OAuthScopesPlug.skip_plug()
   end
+
+  defp handle_bad_token(conn) do
+    RateLimiter.call(conn, name: :authentication)
+  end
 end
diff --git a/test/plugs/admin_secret_authentication_plug_test.exs b/test/plugs/admin_secret_authentication_plug_test.exs
index b541a7208..89df03c4b 100644
--- a/test/plugs/admin_secret_authentication_plug_test.exs
+++ b/test/plugs/admin_secret_authentication_plug_test.exs
@@ -4,11 +4,14 @@
 
 defmodule Pleroma.Plugs.AdminSecretAuthenticationPlugTest do
   use Pleroma.Web.ConnCase, async: true
+
+  import Mock
   import Pleroma.Factory
 
   alias Pleroma.Plugs.AdminSecretAuthenticationPlug
   alias Pleroma.Plugs.OAuthScopesPlug
   alias Pleroma.Plugs.PlugHelper
+  alias Pleroma.Plugs.RateLimiter
 
   test "does nothing if a user is assigned", %{conn: conn} do
     user = insert(:user)
@@ -27,6 +30,10 @@ test "does nothing if a user is assigned", %{conn: conn} do
   describe "when secret set it assigns an admin user" do
     setup do: clear_config([:admin_token])
 
+    setup_with_mocks([{RateLimiter, [:passthrough], []}]) do
+      :ok
+    end
+
     test "with `admin_token` query parameter", %{conn: conn} do
       Pleroma.Config.put(:admin_token, "password123")
 
@@ -35,6 +42,7 @@ test "with `admin_token` query parameter", %{conn: conn} do
         |> AdminSecretAuthenticationPlug.call(%{})
 
       refute conn.assigns[:user]
+      assert called(RateLimiter.call(conn, name: :authentication))
 
       conn =
         %{conn | params: %{"admin_token" => "password123"}}
@@ -53,6 +61,7 @@ test "with `x-admin-token` HTTP header", %{conn: conn} do
         |> AdminSecretAuthenticationPlug.call(%{})
 
       refute conn.assigns[:user]
+      assert called(RateLimiter.call(conn, name: :authentication))
 
       conn =
         conn