From c854bff8f528b4f1560707ac3aa74f4a37044f52 Mon Sep 17 00:00:00 2001 From: Maksim Date: Wed, 1 May 2019 13:28:04 +0000 Subject: [PATCH] Refactored Pleroma.Web.Auth.Authenticator --- lib/pleroma/web/auth/authenticator.ex | 26 +++++++++++++++++ lib/pleroma/web/auth/ldap_authenticator.ex | 41 +++++++++++--------------- lib/pleroma/web/auth/pleroma_authenticator.ex | 15 ++++------ test/media_proxy_test.exs | 14 ++++----- test/web/auth/authenticator_test.exs | 42 +++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 41 deletions(-) create mode 100644 test/web/auth/authenticator_test.exs diff --git a/lib/pleroma/web/auth/authenticator.ex b/lib/pleroma/web/auth/authenticator.ex index b02f595dc..d4e0ffa80 100644 --- a/lib/pleroma/web/auth/authenticator.ex +++ b/lib/pleroma/web/auth/authenticator.ex @@ -42,4 +42,30 @@ defmodule Pleroma.Web.Auth.Authenticator do implementation().oauth_consumer_template() || Pleroma.Config.get([:auth, :oauth_consumer_template], "consumer.html") end + + @doc "Gets user by nickname or email for auth." + @spec fetch_user(String.t()) :: User.t() | nil + def fetch_user(name) do + User.get_by_nickname_or_email(name) + end + + # Gets name and password from conn + # + @spec fetch_credentials(Plug.Conn.t() | map()) :: + {:ok, {name :: any, password :: any}} | {:error, :invalid_credentials} + def fetch_credentials(%Plug.Conn{params: params} = _), + do: fetch_credentials(params) + + def fetch_credentials(params) do + case params do + %{"authorization" => %{"name" => name, "password" => password}} -> + {:ok, {name, password}} + + %{"grant_type" => "password", "username" => name, "password" => password} -> + {:ok, {name, password}} + + _ -> + {:error, :invalid_credentials} + end + end end diff --git a/lib/pleroma/web/auth/ldap_authenticator.ex b/lib/pleroma/web/auth/ldap_authenticator.ex index 363c99597..177c05636 100644 --- a/lib/pleroma/web/auth/ldap_authenticator.ex +++ b/lib/pleroma/web/auth/ldap_authenticator.ex @@ -7,6 +7,9 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do require Logger + import Pleroma.Web.Auth.Authenticator, + only: [fetch_credentials: 1, fetch_user: 1] + @behaviour Pleroma.Web.Auth.Authenticator @base Pleroma.Web.Auth.PleromaAuthenticator @@ -20,30 +23,20 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do defdelegate oauth_consumer_template, to: @base def get_user(%Plug.Conn{} = conn) do - if Pleroma.Config.get([:ldap, :enabled]) do - {name, password} = - case conn.params do - %{"authorization" => %{"name" => name, "password" => password}} -> - {name, password} - - %{"grant_type" => "password", "username" => name, "password" => password} -> - {name, password} - end - - case ldap_user(name, password) do - %User{} = user -> - {:ok, user} - - {:error, {:ldap_connection_error, _}} -> - # When LDAP is unavailable, try default authenticator - @base.get_user(conn) - - error -> - error - end + with {:ldap, true} <- {:ldap, Pleroma.Config.get([:ldap, :enabled])}, + {:ok, {name, password}} <- fetch_credentials(conn), + %User{} = user <- ldap_user(name, password) do + {:ok, user} else - # Fall back to default authenticator - @base.get_user(conn) + {:error, {:ldap_connection_error, _}} -> + # When LDAP is unavailable, try default authenticator + @base.get_user(conn) + + {:ldap, _} -> + @base.get_user(conn) + + error -> + error end end @@ -94,7 +87,7 @@ defmodule Pleroma.Web.Auth.LDAPAuthenticator do case :eldap.simple_bind(connection, "#{uid}=#{name},#{base}", password) do :ok -> - case User.get_by_nickname_or_email(name) do + case fetch_user(name) do %User{} = user -> user diff --git a/lib/pleroma/web/auth/pleroma_authenticator.ex b/lib/pleroma/web/auth/pleroma_authenticator.ex index d647f1e05..dd79cdcf7 100644 --- a/lib/pleroma/web/auth/pleroma_authenticator.ex +++ b/lib/pleroma/web/auth/pleroma_authenticator.ex @@ -8,19 +8,14 @@ defmodule Pleroma.Web.Auth.PleromaAuthenticator do alias Pleroma.Repo alias Pleroma.User + import Pleroma.Web.Auth.Authenticator, + only: [fetch_credentials: 1, fetch_user: 1] + @behaviour Pleroma.Web.Auth.Authenticator def get_user(%Plug.Conn{} = conn) do - {name, password} = - case conn.params do - %{"authorization" => %{"name" => name, "password" => password}} -> - {name, password} - - %{"grant_type" => "password", "username" => name, "password" => password} -> - {name, password} - end - - with {_, %User{} = user} <- {:user, User.get_by_nickname_or_email(name)}, + with {:ok, {name, password}} <- fetch_credentials(conn), + {_, %User{} = user} <- {:user, fetch_user(name)}, {_, true} <- {:checkpw, Pbkdf2.checkpw(password, user.password_hash)} do {:ok, user} else diff --git a/test/media_proxy_test.exs b/test/media_proxy_test.exs index a4331478e..0a02039a6 100644 --- a/test/media_proxy_test.exs +++ b/test/media_proxy_test.exs @@ -7,15 +7,15 @@ defmodule Pleroma.MediaProxyTest do import Pleroma.Web.MediaProxy alias Pleroma.Web.MediaProxy.MediaProxyController + setup do + enabled = Pleroma.Config.get([:media_proxy, :enabled]) + on_exit(fn -> Pleroma.Config.put([:media_proxy, :enabled], enabled) end) + :ok + end + describe "when enabled" do setup do - enabled = Pleroma.Config.get([:media_proxy, :enabled]) - - unless enabled do - Pleroma.Config.put([:media_proxy, :enabled], true) - on_exit(fn -> Pleroma.Config.put([:media_proxy, :enabled], enabled) end) - end - + Pleroma.Config.put([:media_proxy, :enabled], true) :ok end diff --git a/test/web/auth/authenticator_test.exs b/test/web/auth/authenticator_test.exs new file mode 100644 index 000000000..fea5c8209 --- /dev/null +++ b/test/web/auth/authenticator_test.exs @@ -0,0 +1,42 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.Auth.AuthenticatorTest do + use Pleroma.Web.ConnCase + + alias Pleroma.Web.Auth.Authenticator + import Pleroma.Factory + + describe "fetch_user/1" do + test "returns user by name" do + user = insert(:user) + assert Authenticator.fetch_user(user.nickname) == user + end + + test "returns user by email" do + user = insert(:user) + assert Authenticator.fetch_user(user.email) == user + end + + test "returns nil" do + assert Authenticator.fetch_user("email") == nil + end + end + + describe "fetch_credentials/1" do + test "returns name and password from authorization params" do + params = %{"authorization" => %{"name" => "test", "password" => "test-pass"}} + assert Authenticator.fetch_credentials(params) == {:ok, {"test", "test-pass"}} + end + + test "returns name and password with grant_type 'password'" do + params = %{"grant_type" => "password", "username" => "test", "password" => "test-pass"} + assert Authenticator.fetch_credentials(params) == {:ok, {"test", "test-pass"}} + end + + test "returns error" do + assert Authenticator.fetch_credentials(%{}) == {:error, :invalid_credentials} + end + end +end