OAuth: refactor, add CookieAuthPlug

lib/pleroma/web/plugs/cookie_auth_plug.ex

@@ -0,0 +1,28 @@
+# 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.CookieAuthPlug do
+  alias Pleroma.User
+  import Plug.Conn
+
+  def init(opts) do
+    opts
+  end
+
+  # If the user is already assigned (by a bearer token, probably), skip ahead.
+  def call(%{assigns: %{user: _}} = conn, _), do: conn
+
+  # Authenticate with a session cookie, if available.
+  # For staticly-rendered pages (like the OAuth form)
+  # this is the only way it can authenticate.
+  def call(conn, _) do
+    with user_id <- get_session(conn, :user_id),
+         true <- is_binary(user_id),
+         %User{} = user <- User.get_by_id(user_id) do
+      assign(conn, :user, user)
+    else
+      _ -> conn
+    end
+  end
+end

lib/pleroma/web/plugs/ensure_user_key_plug.ex

@@ -3,7 +3,6 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.Plugs.EnsureUserKeyPlug do
-  alias Pleroma.User
   import Plug.Conn
 
   def init(opts) do
@@ -13,12 +12,7 @@ defmodule Pleroma.Web.Plugs.EnsureUserKeyPlug do
   def call(%{assigns: %{user: _}} = conn, _), do: conn
 
   def call(conn, _) do
-    with user_id <- get_session(conn, :user_id),
-         true <- is_binary(user_id),
-         %User{} = user <- User.get_by_id(user_id) do
-      assign(conn, :user, user)
-    else
-      _ -> assign(conn, :user, nil)
-    end
+    conn
+    |> assign(:user, nil)
   end
 end

lib/pleroma/web/router.ex

@@ -33,7 +33,9 @@ defmodule Pleroma.Web.Router do
   pipeline :oauth do
     plug(:fetch_session)
     plug(Pleroma.Web.Plugs.OAuthPlug)
+    plug(Pleroma.Web.Plugs.CookieAuthPlug)
     plug(Pleroma.Web.Plugs.UserEnabledPlug)
+    plug(Pleroma.Web.Plugs.EnsureUserKeyPlug)
   end
 
   pipeline :expect_authentication do
@@ -317,7 +319,7 @@ defmodule Pleroma.Web.Router do
 
   scope "/oauth", Pleroma.Web.OAuth do
     scope [] do
-      pipe_through([:oauth, :after_auth])
+      pipe_through(:oauth)
       get("/authorize", OAuthController, :authorize)
       post("/authorize", OAuthController, :create_authorization)
     end

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -1414,11 +1414,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
 
   describe "Additional ActivityPub C2S endpoints" do
     test "GET /api/ap/whoami", %{conn: conn} do
-      # Test the 403 first because a user cookie gets set below
-      conn
-      |> get("/api/ap/whoami")
-      |> json_response(403)
-
       user = insert(:user)
 
       conn =
@@ -1429,6 +1424,10 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       user = User.get_cached_by_id(user.id)
 
       assert UserView.render("user.json", %{user: user}) == json_response(conn, 200)
+
+      conn
+      |> get("/api/ap/whoami")
+      |> json_response(403)
     end
 
     setup do: clear_config([:media_proxy])

test/pleroma/web/plugs/cookie_auth_plug_test.exs

@@ -0,0 +1,48 @@
+# 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.CookieAuthPlugTest do
+  use Pleroma.Web.ConnCase, async: true
+  alias Pleroma.Web.Plugs.CookieAuthPlug
+  import Pleroma.Factory
+
+  @session_opts [
+    store: :cookie,
+    key: "_test",
+    signing_salt: "cooldude"
+  ]
+
+  setup %{conn: conn} do
+    conn =
+      conn
+      |> Plug.Session.call(Plug.Session.init(@session_opts))
+      |> fetch_session()
+
+    %{conn: conn}
+  end
+
+  test "if the conn has a user key set, it does nothing", %{conn: conn} do
+    conn = assign(conn, :user, 1)
+    result = CookieAuthPlug.call(conn, %{})
+
+    assert result == conn
+  end
+
+  test "if the session has a user_id, it sets the user", %{conn: conn} do
+    user = insert(:user)
+
+    conn =
+      conn
+      |> put_session(:user_id, user.id)
+      |> CookieAuthPlug.call(%{})
+
+    assert conn.assigns[:user] == user
+  end
+
+  test "if the conn has no key set, it does nothing", %{conn: conn} do
+    result = CookieAuthPlug.call(conn, %{})
+
+    assert result == conn
+  end
+end

test/pleroma/web/plugs/ensure_user_key_plug_test.exs

@@ -4,23 +4,8 @@
 
 defmodule Pleroma.Web.Plugs.EnsureUserKeyPlugTest do
   use Pleroma.Web.ConnCase, async: true
+
   alias Pleroma.Web.Plugs.EnsureUserKeyPlug
-  import Pleroma.Factory
-
-  @session_opts [
-    store: :cookie,
-    key: "_test",
-    signing_salt: "cooldude"
-  ]
-
-  setup %{conn: conn} do
-    conn =
-      conn
-      |> Plug.Session.call(Plug.Session.init(@session_opts))
-      |> fetch_session()
-
-    %{conn: conn}
-  end
 
   test "if the conn has a user key set, it does nothing", %{conn: conn} do
     conn =
@@ -34,17 +19,6 @@ defmodule Pleroma.Web.Plugs.EnsureUserKeyPlugTest do
     assert conn == ret_conn
   end
 
-  test "if the session has a user_id, it sets the user", %{conn: conn} do
-    user = insert(:user)
-
-    conn =
-      conn
-      |> put_session(:user_id, user.id)
-      |> EnsureUserKeyPlug.call(%{})
-
-    assert conn.assigns[:user] == user
-  end
-
   test "if the conn has no key set, it sets it to nil", %{conn: conn} do
     conn =
       conn