Auth subsystem refactoring and tweaks.

Added proper OAuth skipping for SessionAuthenticationPlug. Integrated LegacyAuthenticationPlug into AuthenticationPlug. Adjusted tests & docs.

4 files changed, 22 insertions, 97 deletions

diff --git a/test/pleroma/web/plugs/admin_secret_authentication_plug_test.exs b/test/pleroma/web/plugs/admin_secret_authentication_plug_test.exs
index 33394722a..23498badf 100644
--- a/test/pleroma/web/plugs/admin_secret_authentication_plug_test.exs
+++ b/test/pleroma/web/plugs/admin_secret_authentication_plug_test.exs
@@ -49,6 +49,7 @@ defmodule Pleroma.Web.Plugs.AdminSecretAuthenticationPlugTest do
         |> AdminSecretAuthenticationPlug.call(%{})
 
       assert conn.assigns[:user].is_admin
+      assert conn.assigns[:token] == nil
       assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
     end
 
@@ -69,6 +70,7 @@ defmodule Pleroma.Web.Plugs.AdminSecretAuthenticationPlugTest do
         |> AdminSecretAuthenticationPlug.call(%{})
 
       assert conn.assigns[:user].is_admin
+      assert conn.assigns[:token] == nil
       assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
     end
   end
diff --git a/test/pleroma/web/plugs/authentication_plug_test.exs b/test/pleroma/web/plugs/authentication_plug_test.exs
index af39352e2..3dedd38b2 100644
--- a/test/pleroma/web/plugs/authentication_plug_test.exs
+++ b/test/pleroma/web/plugs/authentication_plug_test.exs
@@ -48,6 +48,7 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlugTest do
       |> AuthenticationPlug.call(%{})
 
     assert conn.assigns.user == conn.assigns.auth_user
+    assert conn.assigns.token == nil
     assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
   end
 
@@ -62,6 +63,7 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlugTest do
       |> AuthenticationPlug.call(%{})
 
     assert conn.assigns.user.id == conn.assigns.auth_user.id
+    assert conn.assigns.token == nil
     assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
 
     user = User.get_by_id(user.id)
@@ -83,6 +85,7 @@ defmodule Pleroma.Web.Plugs.AuthenticationPlugTest do
       |> AuthenticationPlug.call(%{})
 
     assert conn.assigns.user.id == conn.assigns.auth_user.id
+    assert conn.assigns.token == nil
     assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
 
     user = User.get_by_id(user.id)
diff --git a/test/pleroma/web/plugs/legacy_authentication_plug_test.exs b/test/pleroma/web/plugs/legacy_authentication_plug_test.exs
deleted file mode 100644
index 2016a31a8..000000000
--- a/test/pleroma/web/plugs/legacy_authentication_plug_test.exs
+++ /dev/null
@@ -1,82 +0,0 @@
-# Pleroma: A lightweight social networking server
-# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
-# SPDX-License-Identifier: AGPL-3.0-only
-
-defmodule Pleroma.Web.Plugs.LegacyAuthenticationPlugTest do
-  use Pleroma.Web.ConnCase
-
-  import Pleroma.Factory
-
-  alias Pleroma.User
-  alias Pleroma.Web.Plugs.LegacyAuthenticationPlug
-  alias Pleroma.Web.Plugs.OAuthScopesPlug
-  alias Pleroma.Web.Plugs.PlugHelper
-
-  setup do
-    user =
-      insert(:user,
-        password: "password",
-        password_hash:
-          "$6$9psBWV8gxkGOZWBz$PmfCycChoxeJ3GgGzwvhlgacb9mUoZ.KUXNCssekER4SJ7bOK53uXrHNb2e4i8yPFgSKyzaW9CcmrDXWIEMtD1"
-      )
-
-    %{user: user}
-  end
-
-  test "it does nothing if a user is assigned", %{conn: conn, user: user} do
-    conn =
-      conn
-      |> assign(:auth_credentials, %{username: "dude", password: "password"})
-      |> assign(:auth_user, user)
-      |> assign(:user, %User{})
-
-    ret_conn =
-      conn
-      |> LegacyAuthenticationPlug.call(%{})
-
-    assert ret_conn == conn
-  end
-
-  @tag :skip_on_mac
-  test "if `auth_user` is present and password is correct, " <>
-         "it authenticates the user, resets the password, marks OAuthScopesPlug as skipped",
-       %{
-         conn: conn,
-         user: user
-       } do
-    conn =
-      conn
-      |> assign(:auth_credentials, %{username: "dude", password: "password"})
-      |> assign(:auth_user, user)
-
-    conn = LegacyAuthenticationPlug.call(conn, %{})
-
-    assert conn.assigns.user.id == user.id
-    assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
-  end
-
-  @tag :skip_on_mac
-  test "it does nothing if the password is wrong", %{
-    conn: conn,
-    user: user
-  } do
-    conn =
-      conn
-      |> assign(:auth_credentials, %{username: "dude", password: "wrong_password"})
-      |> assign(:auth_user, user)
-
-    ret_conn =
-      conn
-      |> LegacyAuthenticationPlug.call(%{})
-
-    assert conn == ret_conn
-  end
-
-  test "with no credentials or user it does nothing", %{conn: conn} do
-    ret_conn =
-      conn
-      |> LegacyAuthenticationPlug.call(%{})
-
-    assert ret_conn == conn
-  end
-end
diff --git a/test/pleroma/web/plugs/session_authentication_plug_test.exs b/test/pleroma/web/plugs/session_authentication_plug_test.exs
index 2b4d5bc0c..d027331a9 100644
--- a/test/pleroma/web/plugs/session_authentication_plug_test.exs
+++ b/test/pleroma/web/plugs/session_authentication_plug_test.exs
@@ -6,6 +6,8 @@ defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do
   use Pleroma.Web.ConnCase, async: true
 
   alias Pleroma.User
+  alias Pleroma.Web.Plugs.OAuthScopesPlug
+  alias Pleroma.Web.Plugs.PlugHelper
   alias Pleroma.Web.Plugs.SessionAuthenticationPlug
 
   setup %{conn: conn} do
@@ -18,24 +20,20 @@ defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do
     conn =
       conn
       |> Plug.Session.call(Plug.Session.init(session_opts))
-      |> fetch_session
+      |> fetch_session()
       |> assign(:auth_user, %User{id: 1})
 
     %{conn: conn}
   end
 
   test "it does nothing if a user is assigned", %{conn: conn} do
-    conn =
-      conn
-      |> assign(:user, %User{})
-
-    ret_conn =
-      conn
-      |> SessionAuthenticationPlug.call(%{})
+    conn = assign(conn, :user, %User{})
+    ret_conn = SessionAuthenticationPlug.call(conn, %{})
 
     assert ret_conn == conn
   end
 
+  # Scenario: requester has the cookie and knows the username (not necessarily knows the password)
   test "if the auth_user has the same id as the user_id in the session, it assigns the user", %{
     conn: conn
   } do
@@ -45,19 +43,23 @@ defmodule Pleroma.Web.Plugs.SessionAuthenticationPlugTest do
       |> SessionAuthenticationPlug.call(%{})
 
     assert conn.assigns.user == conn.assigns.auth_user
+    assert conn.assigns.token == nil
+    assert PlugHelper.plug_skipped?(conn, OAuthScopesPlug)
   end
 
+  # Scenario: requester has the cookie but doesn't know the username
   test "if the auth_user has a different id as the user_id in the session, it does nothing", %{
     conn: conn
   } do
-    conn =
-      conn
-      |> put_session(:user_id, -1)
-
-    ret_conn =
-      conn
-      |> SessionAuthenticationPlug.call(%{})
+    conn = put_session(conn, :user_id, -1)
+    ret_conn = SessionAuthenticationPlug.call(conn, %{})
 
     assert ret_conn == conn
   end
+
+  test "if the session does not contain user_id, it does nothing", %{
+    conn: conn
+  } do
+    assert conn == SessionAuthenticationPlug.call(conn, %{})
+  end
 end