diff --git a/CHANGELOG.md b/CHANGELOG.md index dec1f863..354dda6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ * [`Pow.Operations`] Added `Pow.Operations.reload/2` to reload structs * [`PowPersistentSession.Store.PersistentSessionCache`] Update `PowPersistentSession.Store.PersistentSessionCache.get/2` to reload the user using `Pow.Operations.reload/2` * [`Pow.Store.CredentialsCache`] Now support `reload: true` configuration so once fetched from the cache the user object will be reloaded through the context module +* [`Pow.Store.CredentialsCache`] No longer invalidates active sessions with the same fingerprint when `Pow.Store.CredentialsCache.put/2` is called +* [`Pow.Plug.Session`] Will invalidate any other sessions with the same `:fingerprint` if any is set in session metadata when `Pow.Plug.Session.create/3` is called ## v1.0.21 (2020-09-13) diff --git a/lib/pow/plug/session.ex b/lib/pow/plug/session.ex index 365d1f29..f04f1774 100644 --- a/lib/pow/plug/session.ex +++ b/lib/pow/plug/session.ex @@ -162,6 +162,9 @@ defmodule Pow.Plug.Session do will always be overridden. If no `:fingerprint` exists in the metadata a random UUID value will be generated as its value. + Any existing active sessions with the same `:fingerprint` in metadata will + be deleted. + The session id will be signed for public consumption with `Pow.Plug.sign_token/4`. @@ -193,17 +196,41 @@ defmodule Pow.Plug.Session do defp gen_fingerprint(), do: UUID.generate() - defp before_send_create(conn, value, config) do + defp before_send_create(conn, {user, metadata}, config) do {store, store_config} = store(config) session_id = gen_session_id(config) register_before_send(conn, fn conn -> - store.put(store_config, session_id, value) + delete_user_sessions_with_fingerprint(store, store_config, user, metadata) + + store.put(store_config, session_id, {user, metadata}) client_store_put(conn, session_id, config) end) end + defp delete_user_sessions_with_fingerprint(store, store_config, user, metadata) do + case Keyword.get(metadata, :fingerprint) do + nil -> :ok + fingerprint -> do_delete_user_sessions_with_fingerprint(store, store_config, user, fingerprint) + end + end + + defp do_delete_user_sessions_with_fingerprint(store, store_config, user, fingerprint) do + store_config + |> store.sessions(user) + |> Enum.map(& {&1, store.get(store_config, &1)}) + |> Enum.each(fn + {session_id, {_user, metadata}} -> + with ^fingerprint <- Keyword.get(metadata, :fingerprint) do + store.delete(store_config, session_id) + end + + {_session_id, :not_found} -> + :ok + end) + end + @doc """ Delete an existing session in the credentials cache. diff --git a/lib/pow/store/credentials_cache.ex b/lib/pow/store/credentials_cache.ex index daf1164b..172a9391 100644 --- a/lib/pow/store/credentials_cache.ex +++ b/lib/pow/store/credentials_cache.ex @@ -94,9 +94,6 @@ defmodule Pow.Store.CredentialsCache do - `{session_id, {[user_struct, :user, user_id], metadata}}` - `{[user_struct, :user, user_id], user}` - `{[user_struct, :user, user_id, :session, session_id], inserted_at}` - - If metadata has `:fingerprint` any active sessions for the user with the same - `:fingerprint` in metadata will be deleted. """ @spec put(Base.config(), binary(), {map(), list()}) :: :ok def put(config, session_id, {user, metadata}) do @@ -109,8 +106,6 @@ defmodule Pow.Store.CredentialsCache do {session_key, :os.system_time(:millisecond)} ] - delete_user_sessions_with_fingerprint(config, user, metadata) - Base.put(config, backend_config(config), records) end @@ -200,26 +195,6 @@ defmodule Pow.Store.CredentialsCache do end end - defp delete_user_sessions_with_fingerprint(config, user, metadata) do - case Keyword.get(metadata, :fingerprint) do - nil -> :ok - fingerprint -> do_delete_user_sessions_with_fingerprint(config, user, fingerprint) - end - end - - defp do_delete_user_sessions_with_fingerprint(config, user, fingerprint) do - backend_config = backend_config(config) - - config - |> sessions(user) - |> Enum.each(fn session_id -> - with {_user_key, metadata} when is_list(metadata) <- Base.get(config, backend_config, session_id), - ^fingerprint <- Keyword.get(metadata, :fingerprint) do - delete(config, session_id) - end - end) - end - # TODO: Remove by 1.1.0 @doc false @deprecated "Use `users/2` or `sessions/2` instead" diff --git a/test/pow/plug/session_test.exs b/test/pow/plug/session_test.exs index 0a45f0c1..177538b5 100644 --- a/test/pow/plug/session_test.exs +++ b/test/pow/plug/session_test.exs @@ -145,6 +145,8 @@ defmodule Pow.Plug.SessionTest do @timeout :timer.seconds(5) + defdelegate sessions(config, user), to: CredentialsCache + defdelegate put(config, session_id, record_or_records), to: CredentialsCache defdelegate get(config, session_id), to: CredentialsCache @@ -346,6 +348,36 @@ defmodule Pow.Plug.SessionTest do assert {:ok, decoded_session_id} = Plug.verify_token(conn, Atom.to_string(Session), session_id) assert String.starts_with?(decoded_session_id, "test_app_") end + + test "invalidates sessions with identical fingerprint", %{conn: new_conn} do + conn = + new_conn + |> init_plug() + |> run_do_create(@user) + + assert session_id = get_session_id(conn) + assert {@user, metadata} = get_from_cache(session_id) + assert fingerprint = metadata[:fingerprint] + + timestamp = :os.system_time(:millisecond) + other_session_id = store_in_cache("token", {@user, inserted_at: timestamp, fingerprint: fingerprint}) + + assert Enum.count(CredentialsCache.sessions(@store_config, @user)) == 2 + + conn = + new_conn + |> init_plug() + |> Conn.put_private(:pow_session_metadata, fingerprint: fingerprint) + |> run_do_create(@user) + + assert new_session_id = get_session_id(conn) + assert {@user, new_metadata} = get_from_cache(new_session_id) + assert metadata[:fingerprint] == new_metadata[:fingerprint] + + assert Enum.count(CredentialsCache.sessions(@store_config, @user)) == 1 + assert get_from_cache(session_id) == :not_found + assert get_from_cache(other_session_id) == :not_found + end end test "delete/2 removes session id", %{conn: new_conn} do diff --git a/test/pow/store/credentials_cache_test.exs b/test/pow/store/credentials_cache_test.exs index 3c96ac91..519b991a 100644 --- a/test/pow/store/credentials_cache_test.exs +++ b/test/pow/store/credentials_cache_test.exs @@ -82,21 +82,6 @@ defmodule Pow.Store.CredentialsCacheTest do end end - test "put/3 invalidates sessions with identical fingerprint" do - user = %User{id: 1} - - CredentialsCache.put(@config, "key_1", {user, fingerprint: 1}) - CredentialsCache.put(@config, "key_2", {user, fingerprint: 2}) - - assert CredentialsCache.get(@config, "key_1") == {user, fingerprint: 1} - - CredentialsCache.put(@config, "key_3", {user, fingerprint: 1}) - - assert CredentialsCache.get(@config, "key_1") == :not_found - assert CredentialsCache.get(@config, "key_2") == {user, fingerprint: 2} - assert CredentialsCache.get(@config, "key_3") == {user, fingerprint: 1} - end - defmodule CompositePrimaryFieldsUser do use Ecto.Schema