graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix impersonation crash when session IDs are nil

Browse source 
Critical bug fix: fetch_current_scope_for_user and mount_current_scope
were calling Accounts.get_user(nil) when impersonating session flag was
true but superuser_id or target_user_id were nil.

This caused FunctionClauseError crashes for all logged-out users if they
had stale impersonation session data.

Changes:
- Check if both superuser_id and target_user_id exist before calling get_user
- Clear invalid impersonation state if IDs are missing
- Apply fix to both fetch_current_scope_for_user (controllers) and
  mount_current_scope (LiveViews)

This ensures graceful handling of corrupted/partial session state.
This commit is contained in:
Graham McIntire 2026-01-06 13:32:17 -06:00
parent 7df6e8b3b4
commit e02c2d2b8f
No known key found for this signature in database
1 changed files with 30 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
lib/towerops_web/user_auth.ex
View file

@ -74,17 +74,27 @@ defmodule ToweropsWeb.UserAuth do
superuser_id = get_session(conn, :superuser_id)
target_user_id = get_session(conn, :target_user_id)
with superuser when not is_nil(superuser) <- Accounts.get_user(superuser_id),
target_user when not is_nil(target_user) <- Accounts.get_user(target_user_id) do
assign(conn, :current_scope, Scope.for_impersonation(superuser, target_user))
# Validate we have both IDs before attempting to fetch users
if superuser_id && target_user_id do
with superuser when not is_nil(superuser) <- Accounts.get_user(superuser_id),
target_user when not is_nil(target_user) <- Accounts.get_user(target_user_id) do
assign(conn, :current_scope, Scope.for_impersonation(superuser, target_user))
else
_ ->
# Impersonation invalid, clear it
conn
|> delete_session(:superuser_id)
|> delete_session(:target_user_id)
|> delete_session(:impersonating)
|> assign(:current_scope, Scope.for_user(nil))
end
else
_ ->
# Impersonation invalid, clear it
conn
|> delete_session(:superuser_id)
|> delete_session(:target_user_id)
|> delete_session(:impersonating)
|> assign(:current_scope, Scope.for_user(nil))
# Missing IDs, clear invalid impersonation state
conn
|> delete_session(:superuser_id)
|> delete_session(:target_user_id)
|> delete_session(:impersonating)
|> assign(:current_scope, Scope.for_user(nil))
end
else
# Normal authentication flow
@ -411,11 +421,17 @@ defmodule ToweropsWeb.UserAuth do
superuser_id = session["superuser_id"]
target_user_id = session["target_user_id"]
with superuser when not is_nil(superuser) <- Accounts.get_user(superuser_id),
target_user when not is_nil(target_user) <- Accounts.get_user(target_user_id) do
Scope.for_impersonation(superuser, target_user)
# Validate we have both IDs before attempting to fetch users
if superuser_id && target_user_id do
with superuser when not is_nil(superuser) <- Accounts.get_user(superuser_id),
target_user when not is_nil(target_user) <- Accounts.get_user(target_user_id) do
Scope.for_impersonation(superuser, target_user)
else
_ -> Scope.for_user(nil)
end
else
_ -> Scope.for_user(nil)
# Missing IDs, return nil user
Scope.for_user(nil)
end
else
# Normal authentication flow