commit: 04f6b48ac1a76fe9c6c3fd573427d418bc152adf parent 4fbdd1c8a12fd2b3dc6b63ccbbaa7d4241fa778e Author: Ivan Tashkinov <ivantashkinov@gmail.com> Date: Sat, 31 Oct 2020 13:38:35 +0300 Auth subsystem refactoring and tweaks. Added proper OAuth skipping for SessionAuthenticationPlug. Integrated LegacyAuthenticationPlug into AuthenticationPlug. Adjusted tests & docs.Diffstat:
15 files changed, 97 insertions(+), 182 deletions(-)diff --git a/docs/dev.md b/docs/dev.md@@ -14,9 +14,9 @@ This document contains notes and guidelines for Pleroma developers. For `:api` pipeline routes, it'll be verified whether `OAuthScopesPlug` was called or explicitly skipped, and if it was not then auth information will be dropped for request. Then `EnsurePublicOrAuthenticatedPlug` will be called to ensure that either the instance is not private or user is authenticated (unless explicitly skipped). Such automated checks help to prevent human errors and result in higher security / privacy for users. -## [HTTP Basic Authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization) +## Non-OAuth authentication -* With HTTP Basic Auth, OAuth scopes check is _not_ performed for any action (since password is provided during the auth, requester is able to obtain a token with full permissions anyways). `Pleroma.Web.Plugs.AuthenticationPlug` and `Pleroma.Web.Plugs.LegacyAuthenticationPlug` both call `Pleroma.Web.Plugs.OAuthScopesPlug.skip_plug(conn)` when password is provided. +* With non-OAuth authentication ([HTTP Basic Authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization) or HTTP header- or params-provided auth), OAuth scopes check is _not_ performed for any action (since password is provided during the auth, requester is able to obtain a token with full permissions anyways); auth plugs invoke `Pleroma.Helpers.AuthHelper.skip_oauth(conn)` in this case. ## Auth-related configuration, OAuth consumer mode etc.diff --git a/lib/pleroma/helpers/auth_helper.ex b/lib/pleroma/helpers/auth_helper.ex@@ -0,0 +1,17 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/> +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Helpers.AuthHelper do + alias Pleroma.Web.Plugs.OAuthScopesPlug + + @doc """ + Skips OAuth permissions (scopes) checks, assigns nil `:token`. + Intended to be used with explicit authentication and only when OAuth token cannot be determined. + """ + def skip_oauth(conn) do + conn + |> Plug.Conn.assign(:token, nil) + |> OAuthScopesPlug.skip_plug() + end +enddiff --git a/lib/pleroma/web/plugs/admin_secret_authentication_plug.ex b/lib/pleroma/web/plugs/admin_secret_authentication_plug.ex@@ -5,8 +5,8 @@ defmodule Pleroma.Web.Plugs.AdminSecretAuthenticationPlug do import Plug.Conn + alias Pleroma.Helpers.AuthHelper alias Pleroma.User - alias Pleroma.Web.Plugs.OAuthScopesPlug alias Pleroma.Web.Plugs.RateLimiter def init(options) do @@ -51,7 +51,7 @@ defmodule Pleroma.Web.Plugs.AdminSecretAuthenticationPlug do defp assign_admin_user(conn) do conn |> assign(:user, %User{is_admin: true}) - |> OAuthScopesPlug.skip_plug() + |> AuthHelper.skip_oauth() end defp handle_bad_token(conn) dodiff --git a/lib/pleroma/web/plugs/authentication_plug.ex b/lib/pleroma/web/plugs/authentication_plug.ex@@ -3,6 +3,9 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.AuthenticationPlug do + @moduledoc "Password authentication plug." + + alias Pleroma.Helpers.AuthHelper alias Pleroma.User import Plug.Conn @@ -11,6 +14,30 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlug do def init(options), do: options + def call(%{assigns: %{user: %User{}}} = conn, _), do: conn + + def call( + %{ + assigns: %{ + auth_user: %{password_hash: password_hash} = auth_user, + auth_credentials: %{password: password} + } + } = conn, + _ + ) do + if checkpw(password, password_hash) do + {:ok, auth_user} = maybe_update_password(auth_user, password) + + conn + |> assign(:user, auth_user) + |> AuthHelper.skip_oauth() + else + conn + end + end + + def call(conn, _), do: conn + def checkpw(password, "$6" <> _ = password_hash) do :crypt.crypt(password, password_hash) == password_hash end @@ -40,40 +67,6 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlug do def maybe_update_password(user, _), do: {:ok, user} defp do_update_password(user, password) do - user - |> User.password_update_changeset(%{ - "password" => password, - "password_confirmation" => password - }) - |> Pleroma.Repo.update() + User.reset_password(user, %{password: password, password_confirmation: password}) end - - def call(%{assigns: %{user: %User{}}} = conn, _), do: conn - - def call( - %{ - assigns: %{ - auth_user: %{password_hash: password_hash} = auth_user, - auth_credentials: %{password: password} - } - } = conn, - _ - ) do - if checkpw(password, password_hash) do - {:ok, auth_user} = maybe_update_password(auth_user, password) - - conn - |> assign(:user, auth_user) - |> Pleroma.Web.Plugs.OAuthScopesPlug.skip_plug() - else - conn - end - end - - def call(%{assigns: %{auth_credentials: %{password: _}}} = conn, _) do - Pbkdf2.no_user_verify() - conn - end - - def call(conn, _), do: conn enddiff --git a/lib/pleroma/web/plugs/basic_auth_decoder_plug.ex b/lib/pleroma/web/plugs/basic_auth_decoder_plug.ex@@ -3,6 +3,12 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.BasicAuthDecoderPlug do + @moduledoc """ + Decodes HTTP Basic Auth information and assigns `:auth_credentials`. + + NOTE: no checks are performed at this step, auth_credentials/username could be easily faked. + """ + import Plug.Conn def init(options) dodiff --git a/lib/pleroma/web/plugs/ensure_user_key_plug.ex b/lib/pleroma/web/plugs/ensure_user_key_plug.ex@@ -5,6 +5,8 @@ defmodule Pleroma.Web.Plugs.EnsureUserKeyPlug do import Plug.Conn + @moduledoc "Ensures `conn.assigns.user` is initialized." + def init(opts) do opts end @@ -12,7 +14,6 @@ defmodule Pleroma.Web.Plugs.EnsureUserKeyPlug do def call(%{assigns: %{user: _}} = conn, _), do: conn def call(conn, _) do - conn - |> assign(:user, nil) + assign(conn, :user, nil) end enddiff --git a/lib/pleroma/web/plugs/legacy_authentication_plug.ex b/lib/pleroma/web/plugs/legacy_authentication_plug.ex@@ -1,41 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/> -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.Plugs.LegacyAuthenticationPlug do - import Plug.Conn - - alias Pleroma.User - - def init(options) do - options - end - - def call(%{assigns: %{user: %User{}}} = conn, _), do: conn - - def call( - %{ - assigns: %{ - auth_user: %{password_hash: "$6$" <> _ = password_hash} = auth_user, - auth_credentials: %{password: password} - } - } = conn, - _ - ) do - with ^password_hash <- :crypt.crypt(password, password_hash), - {:ok, user} <- - User.reset_password(auth_user, %{password: password, password_confirmation: password}) do - conn - |> assign(:auth_user, user) - |> assign(:user, user) - |> Pleroma.Web.Plugs.OAuthScopesPlug.skip_plug() - else - _ -> - conn - end - end - - def call(conn, _) do - conn - end -enddiff --git a/lib/pleroma/web/plugs/session_authentication_plug.ex b/lib/pleroma/web/plugs/session_authentication_plug.ex@@ -3,17 +3,27 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.SessionAuthenticationPlug do + @moduledoc """ + Authenticates user by session-stored `:user_id` and request-contained username. + Username can be provided via HTTP Basic Auth (the password is not checked and can be anything). + """ + import Plug.Conn + alias Pleroma.Helpers.AuthHelper + def init(options) do options end + def call(%{assigns: %{user: %Pleroma.User{}}} = conn, _), do: conn + def call(conn, _) do with saved_user_id <- get_session(conn, :user_id), %{auth_user: %{id: ^saved_user_id}} <- conn.assigns do conn |> assign(:user, conn.assigns.auth_user) + |> AuthHelper.skip_oauth() else _ -> conn enddiff --git a/lib/pleroma/web/plugs/set_user_session_id_plug.ex b/lib/pleroma/web/plugs/set_user_session_id_plug.ex@@ -11,8 +11,7 @@ defmodule Pleroma.Web.Plugs.SetUserSessionIdPlug do end def call(%{assigns: %{user: %User{id: id}}} = conn, _) do - conn - |> put_session(:user_id, id) + put_session(conn, :user_id, id) end def call(conn, _), do: conndiff --git a/lib/pleroma/web/plugs/user_fetcher_plug.ex b/lib/pleroma/web/plugs/user_fetcher_plug.ex@@ -3,6 +3,12 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.Plugs.UserFetcherPlug do + @moduledoc """ + Assigns `:auth_user` basing on `:auth_credentials`. + + NOTE: no checks are performed at this step, auth_credentials/username could be easily faked. + """ + alias Pleroma.User import Plug.Conndiff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex@@ -49,7 +49,6 @@ defmodule Pleroma.Web.Router do plug(Pleroma.Web.Plugs.BasicAuthDecoderPlug) plug(Pleroma.Web.Plugs.UserFetcherPlug) plug(Pleroma.Web.Plugs.SessionAuthenticationPlug) - plug(Pleroma.Web.Plugs.LegacyAuthenticationPlug) plug(Pleroma.Web.Plugs.AuthenticationPlug) enddiff --git a/test/pleroma/web/plugs/admin_secret_authentication_plug_test.exs b/test/pleroma/web/plugs/admin_secret_authentication_plug_test.exs@@ -49,6 +49,7 @@ defmodule Pleroma.Web.Plugs.AdminSecretAuthenticationPlugTest do |> AdminSecretAuthenticationPlug.call(%{}) assert conn.assigns[:user].is_admin + assert conn.assigns[:token] == nil assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) end @@ -69,6 +70,7 @@ defmodule Pleroma.Web.Plugs.AdminSecretAuthenticationPlugTest do |> AdminSecretAuthenticationPlug.call(%{}) assert conn.assigns[:user].is_admin + assert conn.assigns[:token] == nil assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) end enddiff --git a/test/pleroma/web/plugs/authentication_plug_test.exs b/test/pleroma/web/plugs/authentication_plug_test.exs@@ -48,6 +48,7 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlugTest do |> AuthenticationPlug.call(%{}) assert conn.assigns.user == conn.assigns.auth_user + assert conn.assigns.token == nil assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) end @@ -62,6 +63,7 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlugTest do |> AuthenticationPlug.call(%{}) assert conn.assigns.user.id == conn.assigns.auth_user.id + assert conn.assigns.token == nil assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) user = User.get_by_id(user.id) @@ -83,6 +85,7 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlugTest do |> AuthenticationPlug.call(%{}) assert conn.assigns.user.id == conn.assigns.auth_user.id + assert conn.assigns.token == nil assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) user = User.get_by_id(user.id)diff --git a/test/pleroma/web/plugs/legacy_authentication_plug_test.exs b/test/pleroma/web/plugs/legacy_authentication_plug_test.exs@@ -1,82 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/> -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.Plugs.LegacyAuthenticationPlugTest do - use Pleroma.Web.ConnCase - - import Pleroma.Factory - - alias Pleroma.User - alias Pleroma.Web.Plugs.LegacyAuthenticationPlug - alias Pleroma.Web.Plugs.OAuthScopesPlug - alias Pleroma.Web.Plugs.PlugHelper - - setup do - user = - insert(:user, - password: "password", - password_hash: - "$6$9psBWV8gxkGOZWBz$PmfCycChoxeJ3GgGzwvhlgacb9mUoZ.KUXNCssekER4SJ7bOK53uXrHNb2e4i8yPFgSKyzaW9CcmrDXWIEMtD1" - ) - - %{user: user} - end - - test "it does nothing if a user is assigned", %{conn: conn, user: user} do - conn = - conn - |> assign(:auth_credentials, %{username: "dude", password: "password"}) - |> assign(:auth_user, user) - |> assign(:user, %User{}) - - ret_conn = - conn - |> LegacyAuthenticationPlug.call(%{}) - - assert ret_conn == conn - end - - @tag :skip_on_mac - 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 - } do - conn = - conn - |> assign(:auth_credentials, %{username: "dude", password: "password"}) - |> assign(:auth_user, user) - - conn = LegacyAuthenticationPlug.call(conn, %{}) - - assert conn.assigns.user.id == user.id - assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) - end - - @tag :skip_on_mac - test "it does nothing if the password is wrong", %{ - conn: conn, - user: user - } do - conn = - conn - |> assign(:auth_credentials, %{username: "dude", password: "wrong_password"}) - |> assign(:auth_user, user) - - ret_conn = - conn - |> LegacyAuthenticationPlug.call(%{}) - - assert conn == ret_conn - end - - test "with no credentials or user it does nothing", %{conn: conn} do - ret_conn = - conn - |> LegacyAuthenticationPlug.call(%{}) - - assert ret_conn == conn - end -enddiff --git a/test/pleroma/web/plugs/session_authentication_plug_test.exs b/test/pleroma/web/plugs/session_authentication_plug_test.exs@@ -6,6 +6,8 @@ defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do use Pleroma.Web.ConnCase, async: true alias Pleroma.User + alias Pleroma.Web.Plugs.OAuthScopesPlug + alias Pleroma.Web.Plugs.PlugHelper alias Pleroma.Web.Plugs.SessionAuthenticationPlug setup %{conn: conn} do @@ -18,24 +20,20 @@ defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do conn = conn |> Plug.Session.call(Plug.Session.init(session_opts)) - |> fetch_session + |> fetch_session() |> assign(:auth_user, %User{id: 1}) %{conn: conn} end test "it does nothing if a user is assigned", %{conn: conn} do - conn = - conn - |> assign(:user, %User{}) - - ret_conn = - conn - |> SessionAuthenticationPlug.call(%{}) + conn = assign(conn, :user, %User{}) + ret_conn = SessionAuthenticationPlug.call(conn, %{}) assert ret_conn == conn end + # Scenario: requester has the cookie and knows the username (not necessarily knows the password) test "if the auth_user has the same id as the user_id in the session, it assigns the user", %{ conn: conn } do @@ -45,19 +43,23 @@ defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do |> SessionAuthenticationPlug.call(%{}) assert conn.assigns.user == conn.assigns.auth_user + assert conn.assigns.token == nil + assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug) end + # Scenario: requester has the cookie but doesn't know the username test "if the auth_user has a different id as the user_id in the session, it does nothing", %{ conn: conn } do - conn = - conn - |> put_session(:user_id, -1) - - ret_conn = - conn - |> SessionAuthenticationPlug.call(%{}) + conn = put_session(conn, :user_id, -1) + ret_conn = SessionAuthenticationPlug.call(conn, %{}) assert ret_conn == conn end + + test "if the session does not contain user_id, it does nothing", %{ + conn: conn + } do + assert conn == SessionAuthenticationPlug.call(conn, %{}) + end end