Skip to content

Commit

Permalink
Refactor active session invalidation logic
Browse files Browse the repository at this point in the history
  • Loading branch information
danschultzer committed Sep 22, 2020
1 parent 18c6002 commit 4a3e450
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 42 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
31 changes: 29 additions & 2 deletions lib/pow/plug/session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.
Expand Down
25 changes: 0 additions & 25 deletions lib/pow/store/credentials_cache.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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"
Expand Down
32 changes: 32 additions & 0 deletions test/pow/plug/session_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 0 additions & 15 deletions test/pow/store/credentials_cache_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 4a3e450

Please sign in to comment.