diff --git a/lib/pleroma/web/plugs/cookie_auth_plug.ex b/lib/pleroma/web/plugs/cookie_auth_plug.ex new file mode 100644 index 000000000..dd5153cd4 --- /dev/null +++ b/lib/pleroma/web/plugs/cookie_auth_plug.ex @@ -0,0 +1,28 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# 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 diff --git a/lib/pleroma/web/plugs/ensure_user_key_plug.ex b/lib/pleroma/web/plugs/ensure_user_key_plug.ex index 537ea8562..70d3091f0 100644 --- a/lib/pleroma/web/plugs/ensure_user_key_plug.ex +++ b/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 @@ def init(opts) 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 diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index d19519186..768d35528 100644 --- a/lib/pleroma/web/router.ex +++ b/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 diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 0bb21b0a8..b696a24f4 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -1414,11 +1414,6 @@ test "it tracks a signed activity fetch when the json is cached", %{conn: conn} 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 @@ test "GET /api/ap/whoami", %{conn: conn} 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]) diff --git a/test/pleroma/web/plugs/cookie_auth_plug_test.exs b/test/pleroma/web/plugs/cookie_auth_plug_test.exs new file mode 100644 index 000000000..12ca11c1d --- /dev/null +++ b/test/pleroma/web/plugs/cookie_auth_plug_test.exs @@ -0,0 +1,48 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# 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 diff --git a/test/pleroma/web/plugs/ensure_user_key_plug_test.exs b/test/pleroma/web/plugs/ensure_user_key_plug_test.exs index 229110f25..f912ef755 100644 --- a/test/pleroma/web/plugs/ensure_user_key_plug_test.exs +++ b/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 @@ test "if the conn has a user key set, it does nothing", %{conn: conn} 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