From 3d7a2d28b3cb6b2d737c1e125c37ac2281c4ec12 Mon Sep 17 00:00:00 2001
From: Dan Schultzer <dan@dreamconception.com>
Date: Wed, 16 Oct 2019 12:43:42 -0700
Subject: [PATCH] Only permit GET method for request_path param

---
 CHANGELOG.md                                  |  1 +
 lib/pow/phoenix/routes.ex                     |  7 +++-
 .../controllers/plug_error_handler_test.exs   | 35 +++++++++++++++----
 test/support/phoenix/controller_assertions.ex | 14 +++++---
 4 files changed, 46 insertions(+), 11 deletions(-)

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