/users/convert lists the accounts a conversion can be started from. For
the promotion direction those are clients, and the query asked only for
the type:
User::query()->where('type', UserType::Client)
The write beside it does not. AccountConversion::guardToStaff() ends with
abort_unless($this->library->canAssignClient($actor, $target), 404);
and says why: a promotion is the most far-reaching thing that can be done
to a client, so reaching one outside the actor's roster "through this door
and no other is not a rule, it is a gap".
The gap was on the way in. A client-scoped staff member holding
manage_users and edit_users was refused the promotion with a 404 -- the
refusal that is careful not to distinguish a stranger from an account that
is not there -- and then shown that same person's name, email, role,
status and consequence counts in the list the refusal came from,
searchable by name or address and paginated to the end.
StaffLibraryScope::clients() is canAssignClient()'s listing half, written
for this: "so a screen narrows by the same rule its buttons are guarded
with rather than restating it -- which is how ClientsController came to
list every client on the installation, name and email, to a viewer who
could reach nothing of theirs." The picker twenty lines below already went
through the same boundary via assignableClientIds().
Only the client direction is narrowed. The staff direction is left exactly
as it was: whoever may demote a staff member may see the staff roster, and
what limits a demotion is guardTarget() on the write, not the listing.
Tests: the listing half added to AccountConversionScopeTest, which until
now covered only the refusals. Two of the five fail against the unfixed
controller -- the stranger's address in the list, and reaching it by exact
search. The other three pin what must not change: the actor still sees
their own client, unscoped staff still see everybody, and the demotion
list still lists staff.
Every route that binds one client — the edit screen, the update, the
delete, the second-factor reset, the file browser — asks whether this
staff member may manage that client. POST users/convert/{user} binds one
too, and asks nothing about it.
AccountConversion::guardToStaff() says why it skips
StaffAccounts::guardTarget, and the reason is sound as far as it goes:
guardTarget asks "could the actor have granted the target's role", which
is meaningless of a client, and what limits a promotion is the role being
*granted* — enforced by the controller validating role_id against
assignableRoleIds(). That answers the question about the role. Nothing
answers the one about the target.
So a client-scoped staff member holding manage_users, edit_users and
edit_clients could promote any client on the installation. It is the
most far-reaching thing that can be done to a client account: the portal
access goes, the assignments that made them somebody's client go inert,
and they come out holding whatever staff role the actor picked from
their own list. The client is never told.
guardToStaff() now asks StaffLibraryScope::canAssignClient — the same
predicate ResolvesShareTargets uses to decide who a file may be shared
with, and true by construction for unscoped staff, so the ordinary
administrator path is untouched. 404 rather than 403, matching both the
isClient() check the controller makes on the way in and the answer the
clients routes give: a client this staff member may not manage should
not be distinguishable from one that is not there.
guardToClient() is unchanged. Its target is a staff account, guardTarget
is the right question to ask about one, and it was already being asked.
The account list on the converter screen is deliberately left as it is.
It runs under can:edit_users and shows every account of the chosen
direction, the same way every other staff surface that lists clients
shows all of them; narrowing a listing is a product decision, not this
fix. What changes is that the button on the row now refuses rather than
going through.
denkfabrik-li