Don't delete or expire cookies

CHANGELOG.md

@@ -19,6 +19,7 @@
 * [`PowEmailConfirmation.Phoenix.ControllerCallbacks`] Now prevents user enumeration attack for `PowInvitation.Phoenix.InvitationController.create/2`
 * [`PowPersistentSession.Plug.Cookie`] Changed default cookie name to `persistent_session`
 * [`PowPersistentSession.Plug.Cookie`] Removed renewal of cookie as the token will always expire
+* [`PowPersistentSession.Plug.Cookie`] No longer expires invalid cookies
 
 ## Removed
 

lib/extensions/persistent_session/plug/cookie.ex

@@ -40,10 +40,6 @@ defmodule PowPersistentSession.Plug.Cookie do
       `[max_age: max_age, path: "/"]` where `:max_age` is the value defined in
       `:persistent_session_ttl`.
 
-    * `:persistent_session_cookie_expiration_timeout` - integer value in
-      seconds for how much time should go by before cookie should expire after
-      the token is fetched in `authenticate/2`. Defaults to 10.
-
   ## Custom metadata
 
   You can assign a private `:pow_persistent_session_metadata` key in the conn
@@ -74,7 +70,6 @@ defmodule PowPersistentSession.Plug.Cookie do
   alias Pow.{Config, Operations, Plug, UUID}
 
   @cookie_key "persistent_session"
-  @cookie_expiration_timeout 10
 
   @doc """
   Sets a persistent session cookie with an auto generated token.
@@ -138,7 +133,7 @@ defmodule PowPersistentSession.Plug.Cookie do
   end
 
   @doc """
-  Expires the persistent session cookie.
+  Expires the persistent session.
 
   If a persistent session cookie exists it'll be updated to expire immediately,
   and the token in the persistent session cache will be deleted.
@@ -165,12 +160,7 @@ defmodule PowPersistentSession.Plug.Cookie do
   end
 
   defp delete_cookie(conn, cookie_key, config) do
-    opts =
-      config
-      |> cookie_opts()
-      |> Keyword.put(:max_age, -1)
-
-    Conn.put_resp_cookie(conn, cookie_key, "", opts)
+    Conn.delete_resp_cookie(conn, cookie_key, cookie_opts(config))
   end
 
   @doc """
@@ -179,15 +169,9 @@ defmodule PowPersistentSession.Plug.Cookie do
   If a persistent session cookie exists, it'll fetch the credentials from the
   persistent session cache.
 
-  After the value is fetched from the cookie, it'll be updated to expire after
-  the value of `:persistent_session_cookie_expiration_timeout` so invalid
-  cookies will be deleted eventually. This timeout prevents immediate deletion
-  of the cookie so in case of multiple simultaneous requests, the cache has
-  time to update the value.
-
   If credentials was fetched successfully, the token in the cache is deleted, a
   new session is created, and `create/2` is called to create a new persistent
-  session cookie. This will override any expiring cookie.
+  session cookie.
 
   If a `:session_metadata` keyword list is fetched from the persistent session
   metadata, all the values will be merged into the private
@@ -210,16 +194,15 @@ defmodule PowPersistentSession.Plug.Cookie do
 
     case conn.req_cookies[cookie_key] do
       nil    -> conn
-      key_id -> do_authenticate(conn, cookie_key, key_id, config)
+      key_id -> do_authenticate(conn, key_id, config)
     end
   end
   defp maybe_authenticate(conn, _user, _config), do: conn
 
-  defp do_authenticate(conn, cookie_key, key_id, config) do
+  defp do_authenticate(conn, key_id, config) do
     {store, store_config} = store(config)
     res                   = store.get(store_config, key_id)
     plug                  = Plug.get_plug(config)
-    conn                  = expire_cookie(conn, cookie_key, key_id, config)
 
     case res do
       :not_found ->
@@ -232,17 +215,6 @@ defmodule PowPersistentSession.Plug.Cookie do
     end
   end
 
-  defp expire_cookie(conn, cookie_key, key_id, config) do
-    max_age = Config.get(config, :persistent_session_cookie_expiration_timeout, @cookie_expiration_timeout)
-    opts    =
-      config
-      |> cookie_opts()
-      |> Keyword.put(:max_age, max_age)
-
-    Conn.put_resp_cookie(conn, cookie_key, key_id, opts)
-  end
-
-
   defp fetch_and_auth_user(conn, {clauses, metadata}, plug, config) do
     clauses
     |> filter_invalid!()

test/extensions/persistent_session/phoenix/controllers/controller_callbacks_test.exs

@@ -34,10 +34,12 @@ defmodule PowPersistentSession.Phoenix.ControllerCallbacksTest do
   describe "Pow.Phoenix.SessionController.delete/2" do
     test "generates cookie", %{conn: conn, ets: ets} do
       conn = post conn, Routes.pow_session_path(conn, :create, %{"user" => @valid_params})
-      %{value: id} = conn.resp_cookies[@cookie_key]
+
+      assert %{value: id} = conn.resp_cookies[@cookie_key]
+
       conn = delete conn, Routes.pow_session_path(conn, :delete)
 
-      assert %{max_age: -1, path: "/", value: ""} = conn.resp_cookies[@cookie_key]
+      refute conn.resp_cookies[@cookie_key]
       assert PersistentSessionCache.get([backend: ets], id) == :not_found
     end
   end

test/extensions/persistent_session/plug/cookie_test.exs

@@ -129,7 +129,7 @@ defmodule PowPersistentSession.Plug.CookieTest do
       |> Cookie.call(Cookie.init([]))
 
     refute Plug.current_user(conn)
-    assert conn.resp_cookies[@cookie_key] == %{max_age: 10, path: "/", value: "test"}
+    refute conn.resp_cookies[@cookie_key]
     assert PersistentSessionCache.get([backend: ets], id) == :not_found
   end
 
@@ -140,26 +140,7 @@ defmodule PowPersistentSession.Plug.CookieTest do
       |> Cookie.call(Cookie.init([]))
 
     refute Plug.current_user(conn)
-    assert conn.resp_cookies[@cookie_key] == %{max_age: 10, path: "/", value: "test"}
-  end
-
-  test "call/2 when persistent session cache doesn't have credentials with custom cookie options", %{conn: conn, config: config} do
-    config = Keyword.merge(config, persistent_session_cookie_opts: @custom_cookie_opts, persistent_session_cookie_expiration_timeout: 20)
-    conn   =
-      conn
-      |> persistent_cookie(@cookie_key, "test")
-      |> Cookie.call(Cookie.init(config))
-
-    refute Plug.current_user(conn)
-    assert conn.resp_cookies[@cookie_key] == %{
-      domain: "domain.com",
-      extra: "SameSite=Lax",
-      http_only: false,
-      max_age: 20,
-      path: "/path",
-      secure: true,
-      value: "test"
-    }
+    refute conn.resp_cookies[@cookie_key]
   end
 
   test "call/2 with invalid stored clauses", %{conn: conn, ets: ets} do
@@ -265,7 +246,7 @@ defmodule PowPersistentSession.Plug.CookieTest do
       |> Cookie.delete(config)
 
     refute Plug.current_user(conn)
-    assert conn.resp_cookies[@cookie_key] == %{max_age: -1, path: "/", value: ""}
+    refute conn.resp_cookies[@cookie_key]
     assert PersistentSessionCache.get([backend: ets], id) == :not_found
   end
 
@@ -278,15 +259,7 @@ defmodule PowPersistentSession.Plug.CookieTest do
       |> Cookie.delete(config)
 
     refute Plug.current_user(conn)
-    assert conn.resp_cookies[@cookie_key] == %{
-      domain: "domain.com",
-      extra: "SameSite=Lax",
-      http_only: false,
-      max_age: -1,
-      path: "/path",
-      secure: true,
-      value: ""
-    }
+    refute conn.resp_cookies[@cookie_key]
     assert PersistentSessionCache.get([backend: ets], id) == :not_found
   end
 end