From 32aa83f3a2a6ab14e36a3452708ea3be94ad4c43 Mon Sep 17 00:00:00 2001 From: Roger Braun Date: Thu, 30 Mar 2017 15:29:49 +0200 Subject: [PATCH] Short circuit user verification if cookie is present. --- lib/pleroma/plugs/authentication_plug.ex | 16 +++++-- test/plugs/authentication_plug_test.exs | 53 +++++++++++++++++++++--- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/plugs/authentication_plug.ex b/lib/pleroma/plugs/authentication_plug.ex index 90bd07b91..a3317f432 100644 --- a/lib/pleroma/plugs/authentication_plug.ex +++ b/lib/pleroma/plugs/authentication_plug.ex @@ -8,20 +8,28 @@ def init(options) do def call(conn, opts) do with {:ok, username, password} <- decode_header(conn), {:ok, user} <- opts[:fetcher].(username), - {:ok, verified_user} <- verify(user, password) + saved_user_id <- get_session(conn, :user_id), + {:ok, verified_user} <- verify(user, password, saved_user_id) do - conn |> assign(:user, verified_user) + conn + |> assign(:user, verified_user) + |> put_session(:user_id, verified_user.id) else _ -> conn |> halt_or_continue(opts) end end - defp verify(nil, _password) do + # 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 Comeonin.Pbkdf2.dummy_checkpw :error end - defp verify(user, password) do + defp verify(user, password, _user_id) do if Comeonin.Pbkdf2.checkpw(password, user.password_hash) do {:ok, user} else diff --git a/test/plugs/authentication_plug_test.exs b/test/plugs/authentication_plug_test.exs index 3f2f769e7..6f1568ec3 100644 --- a/test/plugs/authentication_plug_test.exs +++ b/test/plugs/authentication_plug_test.exs @@ -13,6 +13,12 @@ defp fetch_nil(_name) do password_hash: Comeonin.Pbkdf2.hashpwsalt("guy") } + @session_opts [ + store: :cookie, + key: "_test", + signing_salt: "cooldude" + ] + defp fetch_user(_name) do {:ok, @user} end @@ -23,14 +29,20 @@ defp basic_auth_enc(username, password) do describe "without an authorization header" do test "it halts the application" do - conn = build_conn() |> AuthenticationPlug.call(%{}) + 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 assigns a nil user if the 'optional' option is used" do - conn = build_conn() |> AuthenticationPlug.call(%{optional: true}) + conn = build_conn() + |> Plug.Session.call(Plug.Session.init(@session_opts)) + |> fetch_session + |> AuthenticationPlug.call(%{optional: true}) assert %{ user: nil } == conn.assigns end @@ -40,6 +52,8 @@ test "it assigns a nil user if the 'optional' option is used" 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 @@ -49,6 +63,8 @@ test "it halts the application" do 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 @@ -65,6 +81,8 @@ test "it halts the application" do conn = build_conn() + |> Plug.Session.call(Plug.Session.init(@session_opts)) + |> fetch_session |> put_req_header("authorization", header) |> AuthenticationPlug.call(opts) @@ -82,6 +100,8 @@ 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 |> put_req_header("authorization", header) |> AuthenticationPlug.call(opts) @@ -90,7 +110,7 @@ test "it assigns a nil user if the 'optional' option is used" do end describe "with a correct authorization header for an existing user" do - test "it assigns the user" do + test "it assigns the user", %{conn: conn} do opts = %{ optional: true, fetcher: &fetch_user/1 @@ -98,12 +118,35 @@ test "it assigns the user" do header = basic_auth_enc("dude", "guy") - conn = - build_conn() + 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 + 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