Uh oh!
There was an error while loading. Please reload this page.
Add sign-in-as-user on an administrator's authority - #672
Merged
Conversation
An administrator supporting a customer had no way to see what that customer sees. The multi-account stack already had every primitive — add_authenticated_user/2 appends a session without a password — so this is the authority layer on top of it, not a new mechanism: MultiSession.impersonate/2, a Session controller action, and one route. The rules are deliberately role-based, not permission-based. can_access_admin_area?/1 is true for ANY permission holder, so a customer granted a single self-service permission would have qualified as an administrator under a permission check — and could then have borrowed another customer's account. Instead: the root account (never the active one, or an impersonated session could chain) must hold Owner or Admin; an Owner is never a target; and an Admin cannot take another Admin, while an Owner can, because there is nothing above it to escalate to. Every attempt is written to the activity feed as session.impersonated before the session changes. Verified against a running application on real accounts: admin → customer is allowed, admin → owner and admin → admin are refused, owner → admin and owner → customer are allowed, taking your own account is refused, and a customer holding a portal permission cannot impersonate anyone. Nine tests added; they are :integration and were not run here — this environment has no PostgreSQL for the core suite.
ddon pushed a commit
that referenced
this pull request
Jul 29, 2026
Impersonation (#672) documented "every attempt written to the activity feed before the session changes" and delivered neither half: the row was written only on success, only after the token had been minted, and alongside a second `session.account_added` row indistinguishable from a user adding an account of their own. Refusals — an Admin reaching for the Owner's account, an Admin reaching sideways, a non-staff user hitting the endpoint — left no trace at all, which is the entry a security review actually goes looking for. Refusals now write `session.impersonation_refused` with the deciding rule, carrying no target_uuid so they stay a feed entry rather than a message to the account they named; successes write one row, and it says impersonated. The controller resolved the uuid before checking authority, so its deliberately precise operator copy told every signed-in user which uuids were live accounts. `may_impersonate?/1` settles authority first and shares `staff?/1` with `authorize_impersonation/2` so the rules cannot drift. Both session actions reach the target's inbox via target_uuid but were claimed by no notification type and had no Render clause, so the customer's bell read "Session impersonated" and could not be switched off. Claimed by `security`, given recipient-facing copy. `already_intercepted` (#670) was independent of `skip_queue`, so a worker that set one and forgot the other handed its own dequeued job back to the queue. It can only mean "a worker is re-sending", so it now implies skip_queue. A generated username (#671) is put after `validate_username/2` has already run, so it reached the database with no uniqueness guard; two registrations racing on the same local part raised `Ecto.ConstraintError` out of `Repo.insert/1`. The constraint is re-attached to the generation branch. Reviews in dev_docs/pull_requests/2026/{670,671,672}-*/CLAUDE_REVIEW.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
An administrator supporting a customer has no way to see what that customer sees. Reproducing a "my order list is empty" report means either asking for their password or guessing.
What this is not
It is not a new session mechanism. The multi-account stack already has every primitive this needs —
add_authenticated_user/2appends a session for an already-authenticated user without a password, which is how the OAuth add-account callback works. This adds the authority layer on top:MultiSession.impersonate/2, one controller action, one route.The rules, and why they are role-based
Deliberately not
can_access_admin_area?/1. That predicate is true for any permission holder — a customer granted a single self-service permission (a client portal, say) satisfies it. Gating on it would have let one customer borrow another customer's account, which is the opposite of the point.Every attempt is written to the activity feed as
session.impersonated, with actor and target, before the session changes. An impersonation nobody can see afterwards is what would make this dangerous.The impersonated session joins the existing account switcher next to the administrator's own — they do not lose their own session, and returning is the switcher's existing "switch to" action.
Verification
Exercised against a running host application on real accounts (
MultiSession.impersonate/2directly, so the authority matrix is what is being tested, not the button)::target_is_owner:target_is_staff:self:not_allowedThat last row is the one that matters: it fails on the actor check, which a permission-based rule would have passed.
Nine tests added to
test/integration/users/multi_session_test.exscovering the same matrix plus the inactive / already-in-stack / anonymous cases. They are:integrationand were not executed here — the environment this was written in has no PostgreSQL for the core suite. Please run them with the suite.Note for the host
The action is behind the same
multi_session_enabledgate as the rest of the stack, so the button should only be offered where that setting is on. There is no UI in this PR: the endpoint isPOST /users/session/impersonate/:user_uuidwith an optionalreturn_to, so a host can put the button wherever its user list lives.