diff --git a/CHANGELOG.md b/CHANGELOG.md index 68b65f23..3a71601c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/extensions/persistent_session/plug/cookie.ex b/lib/extensions/persistent_session/plug/cookie.ex index 6b302550..a63a656b 100644 --- a/lib/extensions/persistent_session/plug/cookie.ex +++ b/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!() diff --git a/test/extensions/persistent_session/phoenix/controllers/controller_callbacks_test.exs b/test/extensions/persistent_session/phoenix/controllers/controller_callbacks_test.exs index 63aad2a8..d18a3dae 100644 --- a/test/extensions/persistent_session/phoenix/controllers/controller_callbacks_test.exs +++ b/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 diff --git a/test/extensions/persistent_session/plug/cookie_test.exs b/test/extensions/persistent_session/plug/cookie_test.exs index fbee7f5d..ef260a6a 100644 --- a/test/extensions/persistent_session/plug/cookie_test.exs +++ b/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