aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIvan Tashkinov <ivantashkinov@gmail.com>2020-07-14 11:58:41 +0300
committerIvan Tashkinov <ivantashkinov@gmail.com>2020-07-14 11:58:41 +0300
commit9b225db7d86289fb9d9c51f62e6ec29f6c07f60d (patch)
treed69a32b483f39e0b02419ce45b8a6b578666172a
parentcf3f8cb72a46f0c8c798d4022cff442fae4ab401 (diff)
downloadpleroma-9b225db7d86289fb9d9c51f62e6ec29f6c07f60d.tar.gz
[#1940] Applied rate limit for requests with bad `admin_token`. Added doc warnings on `admin_token` setting.
-rw-r--r--config/description.exs6
-rw-r--r--docs/configuration/cheatsheet.md2
-rw-r--r--lib/pleroma/plugs/admin_secret_authentication_plug.ex17
-rw-r--r--test/plugs/admin_secret_authentication_plug_test.exs9
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 @@ config :pleroma, :config_description, [
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 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug do
if admin_token == secret_token() do
assign_admin_user(conn)
else
- conn
+ handle_bad_token(conn)
end
end
@@ -38,8 +42,9 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug 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 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlug 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 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlugTest 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 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlugTest do
|> AdminSecretAuthenticationPlug.call(%{})
refute conn.assigns[:user]
+ assert called(RateLimiter.call(conn, name: :authentication))
conn =
%{conn | params: %{"admin_token" => "password123"}}
@@ -53,6 +61,7 @@ defmodule Pleroma.Plugs.AdminSecretAuthenticationPlugTest do
|> AdminSecretAuthenticationPlug.call(%{})
refute conn.assigns[:user]
+ assert called(RateLimiter.call(conn, name: :authentication))
conn =
conn