diff --git a/CHANGELOG.md b/CHANGELOG.md index 434991cd..17e525a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Fixed bug where `Pow.Store.CredentialsCache` wasn't used due to how `Pow.Store.Base` macro worked * `Pow.Plug.Session` now stores a keyword list with metadata for the session rather than just the timestamp * `Pow.Phoenix.Router` now only filters routes that has equal number of bindings +* `Pow.Phoenix.Routes.user_not_authenticated_path/1` now only puts the `:request_path` param if the request is using "GET" method ## v1.0.13 (2019-08-25) diff --git a/lib/pow/phoenix/routes.ex b/lib/pow/phoenix/routes.ex index d8480dcd..979fcb55 100644 --- a/lib/pow/phoenix/routes.ex +++ b/lib/pow/phoenix/routes.ex @@ -92,10 +92,15 @@ defmodule Pow.Phoenix.Routes do redirect users back the the page they first attempted to visit. See `after_sign_in_path/1` for how `:request_path` is handled. + The `:request_path` will only be added if the request uses "GET" method. + See `Pow.Phoenix.SessionController` for more on how this value is handled. """ def user_not_authenticated_path(conn) do - session_path(conn, :new, request_path: Phoenix.Controller.current_path(conn)) + case conn.method do + "GET" -> session_path(conn, :new, request_path: Phoenix.Controller.current_path(conn)) + _method -> session_path(conn, :new) + end end @doc """ diff --git a/test/pow/phoenix/controllers/plug_error_handler_test.exs b/test/pow/phoenix/controllers/plug_error_handler_test.exs index 0cbdc416..2d4d7d62 100644 --- a/test/pow/phoenix/controllers/plug_error_handler_test.exs +++ b/test/pow/phoenix/controllers/plug_error_handler_test.exs @@ -13,13 +13,16 @@ defmodule Pow.Phoenix.PlugErrorHandlerTest do def user_already_authenticated(_conn), do: :already_authenticated end + defp prepare_conn(conn) do + conn + |> Conn.put_private(:pow_config, messages_backend: Messages) + |> Conn.put_private(:phoenix_flash, %{}) + |> Conn.put_private(:phoenix_router, Pow.Test.Phoenix.Router) + |> Conn.fetch_query_params() + end + setup do - conn = - ConnTest.build_conn() - |> Conn.put_private(:pow_config, messages_backend: Messages) - |> Conn.put_private(:phoenix_flash, %{}) - |> Conn.put_private(:phoenix_router, Pow.Test.Phoenix.Router) - |> Conn.fetch_query_params() + conn = prepare_conn(ConnTest.build_conn()) {:ok, conn: conn} end @@ -42,6 +45,26 @@ defmodule Pow.Phoenix.PlugErrorHandlerTest do assert ConnTest.get_flash(conn, :error) == "Existing error" end + test "call/2 :not_authenticated doesn't set request_path if not GET request" do + conn = + :post + |> ConnTest.build_conn("/", nil) + |> prepare_conn() + |> PlugErrorHandler.call(:not_authenticated) + + assert ConnTest.redirected_to(conn) == "/session/new" + assert ConnTest.get_flash(conn, :error) == :not_authenticated + + conn = + :delete + |> ConnTest.build_conn("/", nil) + |> prepare_conn() + |> PlugErrorHandler.call(:not_authenticated) + + assert ConnTest.redirected_to(conn) == "/session/new" + assert ConnTest.get_flash(conn, :error) == :not_authenticated + end + test "call/2 :already_authenticated", %{conn: conn} do conn = PlugErrorHandler.call(conn, :already_authenticated) diff --git a/test/support/phoenix/controller_assertions.ex b/test/support/phoenix/controller_assertions.ex index 7696b0be..54c26547 100644 --- a/test/support/phoenix/controller_assertions.ex +++ b/test/support/phoenix/controller_assertions.ex @@ -15,11 +15,17 @@ defmodule Pow.Test.Phoenix.ControllerAssertions do @spec assert_not_authenticated_redirect(Plug.Conn.t()) :: no_return defmacro assert_not_authenticated_redirect(conn) do - quote do - router = Module.concat([unquote(conn).private.phoenix_router, Helpers]) + quote bind_quoted: [conn: conn] do + router = Module.concat([conn.private.phoenix_router, Helpers]) + + expected_path = + case conn.method do + "GET" -> router.pow_session_path(conn, :new, request_path: Phoenix.Controller.current_path(conn)) + _any -> router.pow_session_path(conn, :new) + end - assert ConnTest.redirected_to(unquote(conn)) == router.pow_session_path(unquote(conn), :new, request_path: Phoenix.Controller.current_path(unquote(conn))) - assert ConnTest.get_flash(unquote(conn), :error) == Messages.user_not_authenticated(unquote(conn)) + assert ConnTest.redirected_to(conn) == expected_path + assert ConnTest.get_flash(conn, :error) == Messages.user_not_authenticated(conn) end end end