From 32465b9939718f7bc6604594e0404340c3e02cc9 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Sep 2018 18:53:38 +0200 Subject: [PATCH] Simplify AuthenticationPlug --- lib/pleroma/plugs/authentication_plug.ex | 79 ++------ test/plugs/authentication_plug_test.exs | 244 ++++------------------- 2 files changed, 55 insertions(+), 268 deletions(-) diff --git a/lib/pleroma/plugs/authentication_plug.ex b/lib/pleroma/plugs/authentication_plug.ex index ffecb403d..8706b32cd 100644 --- a/lib/pleroma/plugs/authentication_plug.ex +++ b/lib/pleroma/plugs/authentication_plug.ex @@ -9,71 +9,32 @@ def init(options) do def call(%{assigns: %{user: %User{}}} = conn, _), do: conn - def call(conn, opts) do - with {:ok, username, password} <- decode_header(conn), - {:ok, user} <- opts[:fetcher].(username), - false <- !!user.info["deactivated"], - saved_user_id <- get_session(conn, :user_id), - legacy_password <- String.starts_with?(user.password_hash, "$6$"), - update_legacy_password <- - !(Map.has_key?(opts, :update_legacy_password) && opts[:update_legacy_password] == false), - {:ok, verified_user} <- verify(user, password, saved_user_id) do - if legacy_password and update_legacy_password do - User.reset_password(verified_user, %{ - :password => password, - :password_confirmation => password - }) - end - + def call( + %{ + assigns: %{ + auth_user: %{password_hash: password_hash} = auth_user, + auth_credentials: %{password: password} + } + } = conn, + _ + ) do + if Pbkdf2.checkpw(password, password_hash) do conn - |> assign(:user, verified_user) - |> put_session(:user_id, verified_user.id) + |> assign(:user, auth_user) else - _ -> conn |> halt_or_continue(opts) + conn end end - # Short-circuit if we have a cookie with the id for the given user. - defp verify(%{id: id} = user, _password, id) do - {:ok, user} - end - - defp verify(nil, _password, _user_id) do + def call( + %{ + assigns: %{ + auth_credentials: %{password: password} + } + } = conn, + _ + ) do Pbkdf2.dummy_checkpw() - :error - end - - defp verify(user, password, _user_id) do - valid = - if String.starts_with?(user.password_hash, "$6$") do - :crypt.crypt(password, user.password_hash) == user.password_hash - else - Pbkdf2.checkpw(password, user.password_hash) - end - - if valid do - {:ok, user} - else - :error - end - end - - defp decode_header(conn) do - with ["Basic " <> header] <- get_req_header(conn, "authorization"), - {:ok, userinfo} <- Base.decode64(header), - [username, password] <- String.split(userinfo, ":", parts: 2) do - {:ok, username, password} - end - end - - defp halt_or_continue(conn, %{optional: true}) do - conn |> assign(:user, nil) - end - - defp halt_or_continue(conn, _) do conn - |> put_resp_content_type("application/json") - |> send_resp(403, Jason.encode!(%{error: "Invalid credentials."})) - |> halt end end diff --git a/test/plugs/authentication_plug_test.exs b/test/plugs/authentication_plug_test.exs index fd58d6ab4..061fa0cac 100644 --- a/test/plugs/authentication_plug_test.exs +++ b/test/plugs/authentication_plug_test.exs @@ -4,224 +4,50 @@ defmodule Pleroma.Plugs.AuthenticationPlugTest do alias Pleroma.Plugs.AuthenticationPlug alias Pleroma.User - defp fetch_nil(_name) do - {:ok, nil} + setup %{conn: conn} do + user = %User{ + id: 1, + name: "dude", + password_hash: Comeonin.Pbkdf2.hashpwsalt("guy") + } + + conn = + conn + |> assign(:auth_user, user) + + %{user: user, conn: conn} end - @user %User{ - id: 1, - name: "dude", - password_hash: Comeonin.Pbkdf2.hashpwsalt("guy") - } + test "it does nothing if a user is assigned", %{conn: conn} do + conn = + conn + |> assign(:user, %User{}) - @deactivated %User{ - id: 1, - name: "dude", - password_hash: Comeonin.Pbkdf2.hashpwsalt("guy"), - info: %{"deactivated" => true} - } + ret_conn = + conn + |> AuthenticationPlug.call(%{}) - @legacy %User{ - id: 1, - name: "dude", - password_hash: - "$6$9psBWV8gxkGOZWBz$PmfCycChoxeJ3GgGzwvhlgacb9mUoZ.KUXNCssekER4SJ7bOK53uXrHNb2e4i8yPFgSKyzaW9CcmrDXWIEMtD1" - } - - @session_opts [ - store: :cookie, - key: "_test", - signing_salt: "cooldude" - ] - - defp fetch_user(_name) do - {:ok, @user} + assert ret_conn == conn end - defp basic_auth_enc(username, password) do - "Basic " <> Base.encode64("#{username}:#{password}") + test "with a correct password in the credentials, it assigns the auth_user", %{conn: conn} do + conn = + conn + |> assign(:auth_credentials, %{password: "guy"}) + |> AuthenticationPlug.call(%{}) + + assert conn.assigns.user == conn.assigns.auth_user end - describe "without an authorization header" do - test "it halts the application" do - conn = - build_conn() - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> AuthenticationPlug.call(%{}) + test "with a wrong password in the credentials, it does nothing", %{conn: conn} do + conn = + conn + |> assign(:auth_credentials, %{password: "wrong"}) - assert conn.status == 403 - assert conn.halted == true - end + ret_conn = + conn + |> AuthenticationPlug.call(%{}) - test "it assigns a nil user if the 'optional' option is used" do - conn = - build_conn() - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> AuthenticationPlug.call(%{optional: true}) - - assert %{user: nil} == conn.assigns - end - end - - describe "with an authorization header for a nonexisting user" do - test "it halts the application" do - conn = - build_conn() - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> AuthenticationPlug.call(%{fetcher: &fetch_nil/1}) - - assert conn.status == 403 - assert conn.halted == true - end - - test "it assigns a nil user if the 'optional' option is used" do - conn = - build_conn() - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> AuthenticationPlug.call(%{optional: true, fetcher: &fetch_nil/1}) - - assert %{user: nil} == conn.assigns - end - end - - describe "with an incorrect authorization header for a enxisting user" do - test "it halts the application" do - opts = %{ - fetcher: &fetch_user/1 - } - - header = basic_auth_enc("dude", "man") - - conn = - build_conn() - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> put_req_header("authorization", header) - |> AuthenticationPlug.call(opts) - - assert conn.status == 403 - assert conn.halted == true - end - - test "it assigns a nil user if the 'optional' option is used" do - opts = %{ - optional: true, - fetcher: &fetch_user/1 - } - - header = basic_auth_enc("dude", "man") - - conn = - build_conn() - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> put_req_header("authorization", header) - |> AuthenticationPlug.call(opts) - - assert %{user: nil} == conn.assigns - end - end - - describe "with a correct authorization header for an existing user" do - test "it assigns the user", %{conn: conn} do - opts = %{ - optional: true, - fetcher: &fetch_user/1 - } - - header = basic_auth_enc("dude", "guy") - - conn = - conn - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> put_req_header("authorization", header) - |> AuthenticationPlug.call(opts) - - assert %{user: @user} == conn.assigns - assert get_session(conn, :user_id) == @user.id - assert conn.halted == false - end - - test "it assigns legacy user", %{conn: conn} do - opts = %{ - optional: true, - fetcher: fn _ -> {:ok, @legacy} end, - update_legacy_password: false - } - - header = basic_auth_enc("dude", "password") - - conn = - conn - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> put_req_header("authorization", header) - |> AuthenticationPlug.call(opts) - - assert %{user: @legacy} == conn.assigns - assert get_session(conn, :user_id) == @legacy.id - assert conn.halted == false - end - end - - describe "with a correct authorization header for an deactiviated user" do - test "it halts the appication", %{conn: conn} do - opts = %{ - optional: false, - fetcher: fn _ -> @deactivated end - } - - header = basic_auth_enc("dude", "guy") - - conn = - conn - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> put_req_header("authorization", header) - |> AuthenticationPlug.call(opts) - - assert conn.status == 403 - assert conn.halted == true - end - end - - describe "with a user_id in the session for an existing user" do - test "it assigns the user", %{conn: conn} do - opts = %{ - optional: true, - fetcher: &fetch_user/1 - } - - header = basic_auth_enc("dude", "THIS IS WRONG") - - conn = - conn - |> Plug.Session.call(Plug.Session.init(@session_opts)) - |> fetch_session - |> put_session(:user_id, @user.id) - |> put_req_header("authorization", header) - |> AuthenticationPlug.call(opts) - - assert %{user: @user} == conn.assigns - assert get_session(conn, :user_id) == @user.id - assert conn.halted == false - end - end - - describe "with an assigned user" do - test "it does nothing, returning the incoming conn", %{conn: conn} do - conn = - conn - |> assign(:user, @user) - - conn_result = AuthenticationPlug.call(conn, %{}) - - assert conn == conn_result - end + assert conn == ret_conn end end