Merge branch 'support/oauth' into 'develop'

Refactored Pleroma.Web.Auth.Authenticator

See merge request pleroma/pleroma!1108

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

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
+    with {:ldap, true} <- {:ldap, Pleroma.Config.get([:ldap, :enabled])},
-      {name, password} =
+         {:ok, {name, password}} <- fetch_credentials(conn),
-        case conn.params do
+         %User{} = user <- ldap_user(name, password) do
-          %{"authorization" => %{"name" => name, "password" => password}} ->
+      {:ok, user}
-            {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
     else
-      # Fall back to default authenticator
+      {:error, {:ldap_connection_error, _}} ->
-      @base.get_user(conn)
+        # 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
 

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} =
+    with {:ok, {name, password}} <- fetch_credentials(conn),
-      case conn.params do
+         {_, %User{} = user} <- {:user, fetch_user(name)},
-        %{"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)},
          {_, true} <- {:checkpw, Pbkdf2.checkpw(password, user.password_hash)} do
       {:ok, user}
     else

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])
+      Pleroma.Config.put([:media_proxy, :enabled], true)
-
-      unless enabled do
-        Pleroma.Config.put([:media_proxy, :enabled], true)
-        on_exit(fn -> Pleroma.Config.put([:media_proxy, :enabled], enabled) end)
-      end
-
       :ok
     end
 

test/web/auth/authenticator_test.exs

@@ -0,0 +1,42 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2019 Pleroma Authors <https://pleroma.social/>
+# 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