diff --git a/lib/pleroma/helpers/auth_helper.ex b/lib/pleroma/helpers/auth_helper.ex
index 392fa7d5d..8f87b38be 100644
--- a/lib/pleroma/helpers/auth_helper.ex
+++ b/lib/pleroma/helpers/auth_helper.ex
@@ -20,20 +20,26 @@ def skip_oauth(conn) do
|> OAuthScopesPlug.skip_plug()
end
+ @doc "Drops authentication info from connection"
def drop_auth_info(conn) do
+ # To simplify debugging, setting a private variable on `conn` if auth info is dropped
conn
|> assign(:user, nil)
|> assign(:token, nil)
+ |> put_private(:authentication_ignored, true)
end
+ @doc "Gets OAuth token string from session"
def get_session_token(%Conn{} = conn) do
get_session(conn, @oauth_token_session_key)
end
+ @doc "Updates OAuth token string in session"
def put_session_token(%Conn{} = conn, token) when is_binary(token) do
put_session(conn, @oauth_token_session_key, token)
end
+ @doc "Deletes OAuth token string from session"
def delete_session_token(%Conn{} = conn) do
delete_session(conn, @oauth_token_session_key)
end
diff --git a/lib/pleroma/web.ex b/lib/pleroma/web.ex
index 6ed19d3dd..3ca20455d 100644
--- a/lib/pleroma/web.ex
+++ b/lib/pleroma/web.ex
@@ -20,6 +20,7 @@ defmodule Pleroma.Web do
below.
"""
+ alias Pleroma.Helpers.AuthHelper
alias Pleroma.Web.Plugs.EnsureAuthenticatedPlug
alias Pleroma.Web.Plugs.EnsurePublicOrAuthenticatedPlug
alias Pleroma.Web.Plugs.ExpectAuthenticatedCheckPlug
@@ -75,7 +76,7 @@ defp action(conn, params) do
defp maybe_drop_authentication_if_oauth_check_ignored(conn) do
if PlugHelper.plug_called?(conn, ExpectPublicOrAuthenticatedCheckPlug) and
not PlugHelper.plug_called_or_skipped?(conn, OAuthScopesPlug) do
- OAuthScopesPlug.drop_auth_info(conn)
+ AuthHelper.drop_auth_info(conn)
else
conn
end
diff --git a/lib/pleroma/web/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/web/plugs/admin_secret_authentication_plug.ex
index ff49801f4..ff851a874 100644
--- a/lib/pleroma/web/plugs/admin_secret_authentication_plug.ex
+++ b/lib/pleroma/web/plugs/admin_secret_authentication_plug.ex
@@ -13,13 +13,6 @@ def init(options) do
options
end
- def secret_token do
- 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
def call(conn, _) do
@@ -30,7 +23,7 @@ def call(conn, _) do
end
end
- def authenticate(%{params: %{"admin_token" => admin_token}} = conn) do
+ defp authenticate(%{params: %{"admin_token" => admin_token}} = conn) do
if admin_token == secret_token() do
assign_admin_user(conn)
else
@@ -38,7 +31,7 @@ def authenticate(%{params: %{"admin_token" => admin_token}} = conn) do
end
end
- def authenticate(conn) do
+ defp authenticate(conn) do
token = secret_token()
case get_req_header(conn, "x-admin-token") do
@@ -48,6 +41,13 @@ def authenticate(conn) do
end
end
+ defp secret_token do
+ case Pleroma.Config.get(:admin_token) do
+ blank when blank in [nil, ""] -> nil
+ token -> token
+ end
+ end
+
defp assign_admin_user(conn) do
conn
|> assign(:user, %User{is_admin: true})
diff --git a/lib/pleroma/web/plugs/ensure_user_key_plug.ex b/lib/pleroma/web/plugs/ensure_user_key_plug.ex
deleted file mode 100644
index 31608dbbf..000000000
--- a/lib/pleroma/web/plugs/ensure_user_key_plug.ex
+++ /dev/null
@@ -1,19 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2020 Pleroma Authors
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Web.Plugs.EnsureUserKeyPlug do
- import Plug.Conn
-
- @moduledoc "Ensures `conn.assigns.user` is initialized."
-
- def init(opts) do
- opts
- end
-
- def call(%{assigns: %{user: _}} = conn, _), do: conn
-
- def call(conn, _) do
- assign(conn, :user, nil)
- end
-end
diff --git a/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex b/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex
new file mode 100644
index 000000000..4253458b2
--- /dev/null
+++ b/lib/pleroma/web/plugs/ensure_user_token_assigns_plug.ex
@@ -0,0 +1,36 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug do
+ import Plug.Conn
+
+ alias Pleroma.Helpers.AuthHelper
+ alias Pleroma.User
+ alias Pleroma.Web.OAuth.Token
+
+ @moduledoc "Ensures presence and consistency of :user and :token assigns."
+
+ def init(opts) do
+ opts
+ end
+
+ def call(%{assigns: %{user: %User{id: user_id}} = assigns} = conn, _) do
+ with %Token{user_id: ^user_id} <- assigns[:token] do
+ conn
+ else
+ %Token{} ->
+ # A safety net for abnormal (unexpected) scenario: :token belongs to another user
+ AuthHelper.drop_auth_info(conn)
+
+ _ ->
+ assign(conn, :token, nil)
+ end
+ end
+
+ def call(conn, _) do
+ conn
+ |> assign(:user, nil)
+ |> assign(:token, nil)
+ end
+end
diff --git a/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex b/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex
index f44d4dee5..a0a0c5a9b 100644
--- a/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex
+++ b/lib/pleroma/web/plugs/mapped_signature_to_identity_plug.ex
@@ -3,6 +3,7 @@
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do
+ alias Pleroma.Helpers.AuthHelper
alias Pleroma.Signature
alias Pleroma.User
alias Pleroma.Web.ActivityPub.Utils
@@ -12,6 +13,47 @@ defmodule Pleroma.Web.Plugs.MappedSignatureToIdentityPlug do
def init(options), do: options
+ def call(%{assigns: %{user: %User{}}} = conn, _opts), do: conn
+
+ # if this has payload make sure it is signed by the same actor that made it
+ def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = conn, _opts) do
+ with actor_id <- Utils.get_ap_id(actor),
+ {:user, %User{} = user} <- {:user, user_from_key_id(conn)},
+ {:user_match, true} <- {:user_match, user.ap_id == actor_id} do
+ conn
+ |> assign(:user, user)
+ |> AuthHelper.skip_oauth()
+ else
+ {:user_match, false} ->
+ Logger.debug("Failed to map identity from signature (payload actor mismatch)")
+ Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{inspect(actor)}")
+ assign(conn, :valid_signature, false)
+
+ # remove me once testsuite uses mapped capabilities instead of what we do now
+ {:user, nil} ->
+ Logger.debug("Failed to map identity from signature (lookup failure)")
+ Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{actor}")
+ conn
+ end
+ end
+
+ # no payload, probably a signed fetch
+ def call(%{assigns: %{valid_signature: true}} = conn, _opts) do
+ with %User{} = user <- user_from_key_id(conn) do
+ conn
+ |> assign(:user, user)
+ |> AuthHelper.skip_oauth()
+ else
+ _ ->
+ Logger.debug("Failed to map identity from signature (no payload actor mismatch)")
+ Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}")
+ assign(conn, :valid_signature, false)
+ end
+ end
+
+ # no signature at all
+ def call(conn, _opts), do: conn
+
defp key_id_from_conn(conn) do
with %{"keyId" => key_id} <- HTTPSignatures.signature_for_conn(conn),
{:ok, ap_id} <- Signature.key_id_to_actor_id(key_id) do
@@ -31,41 +73,4 @@ defp user_from_key_id(conn) do
nil
end
end
-
- def call(%{assigns: %{user: _}} = conn, _opts), do: conn
-
- # if this has payload make sure it is signed by the same actor that made it
- def call(%{assigns: %{valid_signature: true}, params: %{"actor" => actor}} = conn, _opts) do
- with actor_id <- Utils.get_ap_id(actor),
- {:user, %User{} = user} <- {:user, user_from_key_id(conn)},
- {:user_match, true} <- {:user_match, user.ap_id == actor_id} do
- assign(conn, :user, user)
- else
- {:user_match, false} ->
- Logger.debug("Failed to map identity from signature (payload actor mismatch)")
- Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{inspect(actor)}")
- assign(conn, :valid_signature, false)
-
- # remove me once testsuite uses mapped capabilities instead of what we do now
- {:user, nil} ->
- Logger.debug("Failed to map identity from signature (lookup failure)")
- Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}, actor=#{actor}")
- conn
- end
- end
-
- # no payload, probably a signed fetch
- def call(%{assigns: %{valid_signature: true}} = conn, _opts) do
- with %User{} = user <- user_from_key_id(conn) do
- assign(conn, :user, user)
- else
- _ ->
- Logger.debug("Failed to map identity from signature (no payload actor mismatch)")
- Logger.debug("key_id=#{inspect(key_id_from_conn(conn))}")
- assign(conn, :valid_signature, false)
- end
- end
-
- # no signature at all
- def call(conn, _opts), do: conn
end
diff --git a/lib/pleroma/web/plugs/o_auth_scopes_plug.ex b/lib/pleroma/web/plugs/o_auth_scopes_plug.ex
index cfc30837c..e6d398b14 100644
--- a/lib/pleroma/web/plugs/o_auth_scopes_plug.ex
+++ b/lib/pleroma/web/plugs/o_auth_scopes_plug.ex
@@ -7,6 +7,7 @@ defmodule Pleroma.Web.Plugs.OAuthScopesPlug do
import Pleroma.Web.Gettext
alias Pleroma.Config
+ alias Pleroma.Helpers.AuthHelper
use Pleroma.Web, :plug
@@ -28,7 +29,7 @@ def perform(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
conn
options[:fallback] == :proceed_unauthenticated ->
- drop_auth_info(conn)
+ AuthHelper.drop_auth_info(conn)
true ->
missing_scopes = scopes -- matched_scopes
@@ -44,15 +45,6 @@ def perform(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
end
end
- @doc "Drops authentication info from connection"
- def drop_auth_info(conn) do
- # To simplify debugging, setting a private variable on `conn` if auth info is dropped
- conn
- |> put_private(:authentication_ignored, true)
- |> assign(:user, nil)
- |> assign(:token, nil)
- end
-
@doc "Keeps those of `scopes` which are descendants of `supported_scopes`"
def filter_descendants(scopes, supported_scopes) do
Enum.filter(
diff --git a/lib/pleroma/web/plugs/user_enabled_plug.ex b/lib/pleroma/web/plugs/user_enabled_plug.ex
index 291d1f568..4f1b163bd 100644
--- a/lib/pleroma/web/plugs/user_enabled_plug.ex
+++ b/lib/pleroma/web/plugs/user_enabled_plug.ex
@@ -11,12 +11,10 @@ def init(options) do
end
def call(%{assigns: %{user: %User{} = user}} = conn, _) do
- case User.account_status(user) do
- :active ->
- conn
-
- _ ->
- AuthHelper.drop_auth_info(conn)
+ if User.account_status(user) == :active do
+ conn
+ else
+ AuthHelper.drop_auth_info(conn)
end
end
diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
index b3462ba00..aefc9f0be 100644
--- a/lib/pleroma/web/router.ex
+++ b/lib/pleroma/web/router.ex
@@ -34,7 +34,7 @@ defmodule Pleroma.Web.Router do
plug(:fetch_session)
plug(Pleroma.Web.Plugs.OAuthPlug)
plug(Pleroma.Web.Plugs.UserEnabledPlug)
- plug(Pleroma.Web.Plugs.EnsureUserKeyPlug)
+ plug(Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug)
end
pipeline :expect_authentication do
@@ -55,7 +55,7 @@ defmodule Pleroma.Web.Router do
pipeline :after_auth do
plug(Pleroma.Web.Plugs.UserEnabledPlug)
plug(Pleroma.Web.Plugs.SetUserSessionIdPlug)
- plug(Pleroma.Web.Plugs.EnsureUserKeyPlug)
+ plug(Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug)
end
pipeline :base_api do
@@ -99,7 +99,7 @@ defmodule Pleroma.Web.Router do
pipeline :pleroma_html do
plug(:browser)
plug(:authenticate)
- plug(Pleroma.Web.Plugs.EnsureUserKeyPlug)
+ plug(Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug)
end
pipeline :well_known do
@@ -291,7 +291,6 @@ defmodule Pleroma.Web.Router do
post("/main/ostatus", UtilController, :remote_subscribe)
get("/ostatus_subscribe", RemoteFollowController, :follow)
-
post("/ostatus_subscribe", RemoteFollowController, :do_follow)
end
diff --git a/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs b/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
index c06ae55ca..e50d1425b 100644
--- a/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
+++ b/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
@@ -941,7 +941,6 @@ test "it resend emails for two users", %{conn: conn, admin: admin} do
describe "/api/pleroma/admin/stats" do
test "status visibility count", %{conn: conn} do
- admin = insert(:user, is_admin: true)
user = insert(:user)
CommonAPI.post(user, %{visibility: "public", status: "hey"})
CommonAPI.post(user, %{visibility: "unlisted", status: "hey"})
@@ -949,7 +948,6 @@ test "status visibility count", %{conn: conn} do
response =
conn
- |> assign(:user, admin)
|> get("/api/pleroma/admin/stats")
|> json_response(200)
@@ -958,7 +956,6 @@ test "status visibility count", %{conn: conn} do
end
test "by instance", %{conn: conn} do
- admin = insert(:user, is_admin: true)
user1 = insert(:user)
instance2 = "instance2.tld"
user2 = insert(:user, %{ap_id: "https://#{instance2}/@actor"})
@@ -969,7 +966,6 @@ test "by instance", %{conn: conn} do
response =
conn
- |> assign(:user, admin)
|> get("/api/pleroma/admin/stats", instance: instance2)
|> json_response(200)
diff --git a/test/pleroma/web/admin_api/controllers/config_controller_test.exs b/test/pleroma/web/admin_api/controllers/config_controller_test.exs
index 4e897455f..765a5a4b7 100644
--- a/test/pleroma/web/admin_api/controllers/config_controller_test.exs
+++ b/test/pleroma/web/admin_api/controllers/config_controller_test.exs
@@ -1415,11 +1415,7 @@ test "enables the welcome messages", %{conn: conn} do
describe "GET /api/pleroma/admin/config/descriptions" do
test "structure", %{conn: conn} do
- admin = insert(:user, is_admin: true)
-
- conn =
- assign(conn, :user, admin)
- |> get("/api/pleroma/admin/config/descriptions")
+ conn = get(conn, "/api/pleroma/admin/config/descriptions")
assert [child | _others] = json_response_and_validate_schema(conn, 200)
@@ -1437,11 +1433,7 @@ test "filters by database configuration whitelist", %{conn: conn} do
{:esshd}
])
- admin = insert(:user, is_admin: true)
-
- conn =
- assign(conn, :user, admin)
- |> get("/api/pleroma/admin/config/descriptions")
+ conn = get(conn, "/api/pleroma/admin/config/descriptions")
children = json_response_and_validate_schema(conn, 200)
diff --git a/test/pleroma/web/pleroma_api/controllers/chat_controller_test.exs b/test/pleroma/web/pleroma_api/controllers/chat_controller_test.exs
index c1e6a8cc5..a6c9d0c1b 100644
--- a/test/pleroma/web/pleroma_api/controllers/chat_controller_test.exs
+++ b/test/pleroma/web/pleroma_api/controllers/chat_controller_test.exs
@@ -264,9 +264,10 @@ test "it returns the messages for a given chat", %{conn: conn, user: user} do
assert length(result) == 3
# Trying to get the chat of a different user
+ other_user_chat = Chat.get(other_user.id, user.ap_id)
+
conn
- |> assign(:user, other_user)
- |> get("/api/v1/pleroma/chats/#{chat.id}/messages")
+ |> get("/api/v1/pleroma/chats/#{other_user_chat.id}/messages")
|> json_response_and_validate_schema(404)
end
end
diff --git a/test/pleroma/web/plugs/ensure_user_key_plug_test.exs b/test/pleroma/web/plugs/ensure_user_key_plug_test.exs
deleted file mode 100644
index f912ef755..000000000
--- a/test/pleroma/web/plugs/ensure_user_key_plug_test.exs
+++ /dev/null
@@ -1,29 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2020 Pleroma Authors
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Web.Plugs.EnsureUserKeyPlugTest do
- use Pleroma.Web.ConnCase, async: true
-
- alias Pleroma.Web.Plugs.EnsureUserKeyPlug
-
- test "if the conn has a user key set, it does nothing", %{conn: conn} do
- conn =
- conn
- |> assign(:user, 1)
-
- ret_conn =
- conn
- |> EnsureUserKeyPlug.call(%{})
-
- assert conn == ret_conn
- end
-
- test "if the conn has no key set, it sets it to nil", %{conn: conn} do
- conn =
- conn
- |> EnsureUserKeyPlug.call(%{})
-
- assert Map.has_key?(conn.assigns, :user)
- end
-end
diff --git a/test/pleroma/web/plugs/ensure_user_token_assigns_plug_test.exs b/test/pleroma/web/plugs/ensure_user_token_assigns_plug_test.exs
new file mode 100644
index 000000000..9592820c7
--- /dev/null
+++ b/test/pleroma/web/plugs/ensure_user_token_assigns_plug_test.exs
@@ -0,0 +1,69 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Web.Plugs.EnsureUserTokenAssignsPlugTest do
+ use Pleroma.Web.ConnCase, async: true
+
+ import Pleroma.Factory
+
+ alias Pleroma.Web.Plugs.EnsureUserTokenAssignsPlug
+
+ test "with :user assign set to a User record " <>
+ "and :token assign set to a Token belonging to this user, " <>
+ "it does nothing" do
+ %{conn: conn} = oauth_access(["read"])
+
+ ret_conn = EnsureUserTokenAssignsPlug.call(conn, %{})
+
+ assert conn == ret_conn
+ end
+
+ test "with :user assign set to a User record " <>
+ "but :token assign not set or not a Token, " <>
+ "it assigns :token to `nil`",
+ %{conn: conn} do
+ user = insert(:user)
+ conn = assign(conn, :user, user)
+
+ ret_conn = EnsureUserTokenAssignsPlug.call(conn, %{})
+
+ assert %{token: nil} = ret_conn.assigns
+
+ ret_conn2 =
+ conn
+ |> assign(:token, 1)
+ |> EnsureUserTokenAssignsPlug.call(%{})
+
+ assert %{token: nil} = ret_conn2.assigns
+ end
+
+ # Abnormal (unexpected) scenario
+ test "with :user assign set to a User record " <>
+ "but :token assign set to a Token NOT belonging to :user, " <>
+ "it drops auth info" do
+ %{conn: conn} = oauth_access(["read"])
+ other_user = insert(:user)
+
+ conn = assign(conn, :user, other_user)
+
+ ret_conn = EnsureUserTokenAssignsPlug.call(conn, %{})
+
+ assert %{user: nil, token: nil} = ret_conn.assigns
+ end
+
+ test "if :user assign is not set to a User record, it sets :user and :token to nil", %{
+ conn: conn
+ } do
+ ret_conn = EnsureUserTokenAssignsPlug.call(conn, %{})
+
+ assert %{user: nil, token: nil} = ret_conn.assigns
+
+ ret_conn2 =
+ conn
+ |> assign(:user, 1)
+ |> EnsureUserTokenAssignsPlug.call(%{})
+
+ assert %{user: nil, token: nil} = ret_conn2.assigns
+ end
+end