Simplify AuthenticationPlug
authorlain <lain@soykaf.club>
Wed, 5 Sep 2018 16:53:38 +0000 (18:53 +0200)
committerlain <lain@soykaf.club>
Wed, 5 Sep 2018 16:53:38 +0000 (18:53 +0200)
lib/pleroma/plugs/authentication_plug.ex
test/plugs/authentication_plug_test.exs

index ffecb403d6bf246ea528888420c2f3ca33544d95..8706b32cd6b415a382234794b85b817fd3af8758 100644 (file)
@@ -9,71 +9,32 @@ defmodule Pleroma.Plugs.AuthenticationPlug 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
index fd58d6ab470455caf9c1a5b5c1edec44853dad77..061fa0cac384f7c04ea18b17610d514c8b3b832f 100644 (file)
@@ -4,224 +4,50 @@ defmodule Pleroma.Plugs.AuthenticationPlugTest do
   alias Pleroma.Plugs.AuthenticationPlug
   alias Pleroma.User
 
-  defp fetch_nil(_name) do
-    {:ok, nil}
-  end
-
-  @user %User{
-    id: 1,
-    name: "dude",
-    password_hash: Comeonin.Pbkdf2.hashpwsalt("guy")
-  }
-
-  @deactivated %User{
-    id: 1,
-    name: "dude",
-    password_hash: Comeonin.Pbkdf2.hashpwsalt("guy"),
-    info: %{"deactivated" => true}
-  }
-
-  @legacy %User{
-    id: 1,
-    name: "dude",
-    password_hash:
-      "$6$9psBWV8gxkGOZWBz$PmfCycChoxeJ3GgGzwvhlgacb9mUoZ.KUXNCssekER4SJ7bOK53uXrHNb2e4i8yPFgSKyzaW9CcmrDXWIEMtD1"
-  }
-
-  @session_opts [
-    store: :cookie,
-    key: "_test",
-    signing_salt: "cooldude"
-  ]
+  setup %{conn: conn} do
+    user = %User{
+      id: 1,
+      name: "dude",
+      password_hash: Comeonin.Pbkdf2.hashpwsalt("guy")
+    }
 
-  defp fetch_user(_name) do
-    {:ok, @user}
-  end
+    conn =
+      conn
+      |> assign(:auth_user, user)
 
-  defp basic_auth_enc(username, password) do
-    "Basic " <> Base.encode64("#{username}:#{password}")
+    %{user: user, conn: conn}
   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(%{})
-
-      assert conn.status == 403
-      assert conn.halted == true
-    end
+  test "it does nothing if a user is assigned", %{conn: conn} do
+    conn =
+      conn
+      |> assign(:user, %User{})
 
-    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})
+    ret_conn =
+      conn
+      |> AuthenticationPlug.call(%{})
 
-      assert %{user: nil} == conn.assigns
-    end
+    assert ret_conn == conn
   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)
+  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 %{user: @user} == conn.assigns
-      assert get_session(conn, :user_id) == @user.id
-      assert conn.halted == false
-    end
+    assert conn.assigns.user == conn.assigns.auth_user
   end
 
-  describe "with an assigned user" do
-    test "it does nothing, returning the incoming conn", %{conn: conn} do
-      conn =
-        conn
-        |> assign(:user, @user)
+  test "with a wrong password in the credentials, it does nothing", %{conn: conn} do
+    conn =
+      conn
+      |> assign(:auth_credentials, %{password: "wrong"})
 
-      conn_result = AuthenticationPlug.call(conn, %{})
+    ret_conn =
+      conn
+      |> AuthenticationPlug.call(%{})
 
-      assert conn == conn_result
-    end
+    assert conn == ret_conn
   end
 end