commit: f1843db41d27c695b4fdb92b13fe6642102a35bc
parent: e303d80dcb35b0d63933a1cebc06caa07ec6eed1
Author: rinpatch <rinpatch@sdf.org>
Date: Fri, 17 Apr 2020 20:47:43 +0000
Merge branch '1682-basic-auth-permissions-fix' into 'develop'
[#1682] Fixed Basic Auth permissions issue
Closes #1682
See merge request pleroma/pleroma!2401
Diffstat:
9 files changed, 101 insertions(+), 23 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
## [unreleased-patch]
### Fixed
- Logger configuration through AdminFE
+- HTTP Basic Authentication permissions issue
### Added
<details>
diff --git a/lib/pleroma/plugs/authentication_plug.ex b/lib/pleroma/plugs/authentication_plug.ex
@@ -4,8 +4,11 @@
defmodule Pleroma.Plugs.AuthenticationPlug do
alias Comeonin.Pbkdf2
- import Plug.Conn
+ alias Pleroma.Plugs.OAuthScopesPlug
alias Pleroma.User
+
+ import Plug.Conn
+
require Logger
def init(options), do: options
@@ -37,6 +40,7 @@ defmodule Pleroma.Plugs.AuthenticationPlug do
if Pbkdf2.checkpw(password, password_hash) do
conn
|> assign(:user, auth_user)
+ |> OAuthScopesPlug.skip_plug()
else
conn
end
diff --git a/lib/pleroma/plugs/legacy_authentication_plug.ex b/lib/pleroma/plugs/legacy_authentication_plug.ex
@@ -4,6 +4,8 @@
defmodule Pleroma.Plugs.LegacyAuthenticationPlug do
import Plug.Conn
+
+ alias Pleroma.Plugs.OAuthScopesPlug
alias Pleroma.User
def init(options) do
@@ -27,6 +29,7 @@ defmodule Pleroma.Plugs.LegacyAuthenticationPlug do
conn
|> assign(:auth_user, user)
|> assign(:user, user)
+ |> OAuthScopesPlug.skip_plug()
else
_ ->
conn
diff --git a/lib/pleroma/plugs/plug_helper.ex b/lib/pleroma/plugs/plug_helper.ex
@@ -5,30 +5,32 @@
defmodule Pleroma.Plugs.PlugHelper do
@moduledoc "Pleroma Plug helper"
- def append_to_called_plugs(conn, plug_module) do
- append_to_private_list(conn, :called_plugs, plug_module)
- end
+ @called_plugs_list_id :called_plugs
+ def called_plugs_list_id, do: @called_plugs_list_id
- def append_to_skipped_plugs(conn, plug_module) do
- append_to_private_list(conn, :skipped_plugs, plug_module)
- end
+ @skipped_plugs_list_id :skipped_plugs
+ def skipped_plugs_list_id, do: @skipped_plugs_list_id
+ @doc "Returns `true` if specified plug was called."
def plug_called?(conn, plug_module) do
- contained_in_private_list?(conn, :called_plugs, plug_module)
+ contained_in_private_list?(conn, @called_plugs_list_id, plug_module)
end
+ @doc "Returns `true` if specified plug was explicitly marked as skipped."
def plug_skipped?(conn, plug_module) do
- contained_in_private_list?(conn, :skipped_plugs, plug_module)
+ contained_in_private_list?(conn, @skipped_plugs_list_id, plug_module)
end
+ @doc "Returns `true` if specified plug was either called or explicitly marked as skipped."
def plug_called_or_skipped?(conn, plug_module) do
plug_called?(conn, plug_module) || plug_skipped?(conn, plug_module)
end
- defp append_to_private_list(conn, private_variable, value) do
- list = conn.private[private_variable] || []
+ # Appends plug to known list (skipped, called). Intended to be used from within plug code only.
+ def append_to_private_list(conn, list_id, value) do
+ list = conn.private[list_id] || []
modified_list = Enum.uniq(list ++ [value])
- Plug.Conn.put_private(conn, private_variable, modified_list)
+ Plug.Conn.put_private(conn, list_id, modified_list)
end
defp contained_in_private_list?(conn, private_variable, value) do
diff --git a/lib/pleroma/web/web.ex b/lib/pleroma/web/web.ex
@@ -40,17 +40,22 @@ defmodule Pleroma.Web do
# Marks a plug intentionally skipped and blocks its execution if it's present in plugs chain
defp skip_plug(conn, plug_module) do
try do
- plug_module.ensure_skippable()
+ plug_module.skip_plug(conn)
rescue
UndefinedFunctionError ->
raise "#{plug_module} is not skippable. Append `use Pleroma.Web, :plug` to its code."
end
-
- PlugHelper.append_to_skipped_plugs(conn, plug_module)
end
- # Here we can apply before-action hooks (e.g. verify whether auth checks were preformed)
+ # Executed just before actual controller action, invokes before-action hooks (callbacks)
defp action(conn, params) do
+ with %Plug.Conn{halted: false} <- maybe_halt_on_missing_oauth_scopes_check(conn) do
+ super(conn, params)
+ end
+ end
+
+ # Halts if authenticated API action neither performs nor explicitly skips OAuth scopes check
+ defp maybe_halt_on_missing_oauth_scopes_check(conn) do
if Pleroma.Plugs.AuthExpectedPlug.auth_expected?(conn) &&
not PlugHelper.plug_called_or_skipped?(conn, Pleroma.Plugs.OAuthScopesPlug) do
conn
@@ -60,7 +65,7 @@ defmodule Pleroma.Web do
)
|> halt()
else
- super(conn, params)
+ conn
end
end
end
@@ -129,7 +134,16 @@ defmodule Pleroma.Web do
quote do
alias Pleroma.Plugs.PlugHelper
- def ensure_skippable, do: :noop
+ @doc """
+ Marks a plug intentionally skipped and blocks its execution if it's present in plugs chain.
+ """
+ def skip_plug(conn) do
+ PlugHelper.append_to_private_list(
+ conn,
+ PlugHelper.skipped_plugs_list_id(),
+ __MODULE__
+ )
+ end
@impl Plug
@doc "If marked as skipped, returns `conn`, and calls `perform/2` otherwise."
@@ -138,7 +152,7 @@ defmodule Pleroma.Web do
conn
else
conn
- |> PlugHelper.append_to_called_plugs(__MODULE__)
+ |> PlugHelper.append_to_private_list(PlugHelper.called_plugs_list_id(), __MODULE__)
|> perform(options)
end
end
diff --git a/test/plugs/authentication_plug_test.exs b/test/plugs/authentication_plug_test.exs
@@ -6,6 +6,8 @@ defmodule Pleroma.Plugs.AuthenticationPlugTest do
use Pleroma.Web.ConnCase, async: true
alias Pleroma.Plugs.AuthenticationPlug
+ alias Pleroma.Plugs.OAuthScopesPlug
+ alias Pleroma.Plugs.PlugHelper
alias Pleroma.User
import ExUnit.CaptureLog
@@ -36,13 +38,16 @@ defmodule Pleroma.Plugs.AuthenticationPlugTest do
assert ret_conn == conn
end
- test "with a correct password in the credentials, it assigns the auth_user", %{conn: conn} do
+ test "with a correct password in the credentials, " <>
+ "it assigns the auth_user and marks OAuthScopesPlug as skipped",
+ %{conn: conn} do
conn =
conn
|> assign(:auth_credentials, %{password: "guy"})
|> AuthenticationPlug.call(%{})
assert conn.assigns.user == conn.assigns.auth_user
+ assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
end
test "with a wrong password in the credentials, it does nothing", %{conn: conn} do
diff --git a/test/plugs/legacy_authentication_plug_test.exs b/test/plugs/legacy_authentication_plug_test.exs
@@ -8,6 +8,8 @@ defmodule Pleroma.Plugs.LegacyAuthenticationPlugTest do
import Pleroma.Factory
alias Pleroma.Plugs.LegacyAuthenticationPlug
+ alias Pleroma.Plugs.OAuthScopesPlug
+ alias Pleroma.Plugs.PlugHelper
alias Pleroma.User
setup do
@@ -36,7 +38,8 @@ defmodule Pleroma.Plugs.LegacyAuthenticationPlugTest do
end
@tag :skip_on_mac
- test "it authenticates the auth_user if present and password is correct and resets the password",
+ test "if `auth_user` is present and password is correct, " <>
+ "it authenticates the user, resets the password, marks OAuthScopesPlug as skipped",
%{
conn: conn,
user: user
@@ -49,6 +52,7 @@ defmodule Pleroma.Plugs.LegacyAuthenticationPlugTest do
conn = LegacyAuthenticationPlug.call(conn, %{})
assert conn.assigns.user.id == user.id
+ assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
end
@tag :skip_on_mac
diff --git a/test/plugs/oauth_scopes_plug_test.exs b/test/plugs/oauth_scopes_plug_test.exs
@@ -7,7 +7,6 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
alias Pleroma.Plugs.OAuthScopesPlug
- alias Pleroma.Plugs.PlugHelper
alias Pleroma.Repo
import Mock
@@ -21,7 +20,7 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
with_mock OAuthScopesPlug, [:passthrough], perform: &passthrough([&1, &2]) do
conn =
conn
- |> PlugHelper.append_to_skipped_plugs(OAuthScopesPlug)
+ |> OAuthScopesPlug.skip_plug()
|> OAuthScopesPlug.call(%{scopes: ["random_scope"]})
refute called(OAuthScopesPlug.perform(:_, :_))
diff --git a/test/web/auth/basic_auth_test.exs b/test/web/auth/basic_auth_test.exs
@@ -0,0 +1,46 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Auth.BasicAuthTest do
+ use Pleroma.Web.ConnCase
+
+ import Pleroma.Factory
+
+ test "with HTTP Basic Auth used, grants access to OAuth scope-restricted endpoints", %{
+ conn: conn
+ } do
+ user = insert(:user)
+ assert Comeonin.Pbkdf2.checkpw("test", user.password_hash)
+
+ basic_auth_contents =
+ (URI.encode_www_form(user.nickname) <> ":" <> URI.encode_www_form("test"))
+ |> Base.encode64()
+
+ # Succeeds with HTTP Basic Auth
+ response =
+ conn
+ |> put_req_header("authorization", "Basic " <> basic_auth_contents)
+ |> get("/api/v1/accounts/verify_credentials")
+ |> json_response(200)
+
+ user_nickname = user.nickname
+ assert %{"username" => ^user_nickname} = response
+
+ # Succeeds with a properly scoped OAuth token
+ valid_token = insert(:oauth_token, scopes: ["read:accounts"])
+
+ conn
+ |> put_req_header("authorization", "Bearer #{valid_token.token}")
+ |> get("/api/v1/accounts/verify_credentials")
+ |> json_response(200)
+
+ # Fails with a wrong-scoped OAuth token (proof of restriction)
+ invalid_token = insert(:oauth_token, scopes: ["read:something"])
+
+ conn
+ |> put_req_header("authorization", "Bearer #{invalid_token.token}")
+ |> get("/api/v1/accounts/verify_credentials")
+ |> json_response(403)
+ end
+end
