399 lines
22 KiB
Markdown
399 lines
22 KiB
Markdown
# User Roles: Assignment, Approval and Revocation
|
||
|
||
Reference for how a user account comes into existence, how it gains and loses roles, and
|
||
how it gets blocked. Describes the behaviour as implemented — not a plan.
|
||
|
||
The governing rules, from which the rest follows:
|
||
|
||
> **BusPro is the source of truth for roles and hotel codes.** Both are synced on every
|
||
> login, so anything it no longer reports is withdrawn.
|
||
>
|
||
> **The CRM may nominate, but never grant, an administrative role.** Only a super admin
|
||
> turns a nomination into a privilege. The CRM's word alone is enough to take away access,
|
||
> never to hand it out - and an administrator's word alone is enough for neither.
|
||
|
||
Two corollaries that surprise people:
|
||
|
||
- **A role cannot be handed out by hand.** The edit form approves what BusPro claims; it
|
||
cannot add a role BusPro is silent about, nor remove one it reports. Same for hotel codes,
|
||
which are display-only there.
|
||
- **Every role stands on its own.** `ROLE_TEAMER` is not a base role others build on, and a
|
||
Reisemanager is not a superset of a Hausleitung - somebody claiming both is nominated for
|
||
both, and approved for each separately.
|
||
|
||
---
|
||
|
||
## Role catalogue
|
||
|
||
Defined in `User::ROLES` (`src/Entity/User.php`), these four are the only roles a human can
|
||
assign:
|
||
|
||
| Role | Label | Granted by | Revoked by | Hierarchy |
|
||
|------|-------|-----------|------------|-----------|
|
||
| `ROLE_ADMIN` | Admin | super admin, approving a CRM claim | the CRM, automatically | ⇒ `ROLE_ADMINISTRATIVE` |
|
||
| `ROLE_MANAGER` | Reisemanager | super admin, approving a CRM claim | the CRM, automatically | ⇒ `ROLE_ADMINISTRATIVE` |
|
||
| `ROLE_HOUSE_MANAGER` | Hausleitung | super admin, approving a CRM claim | the CRM, automatically | — |
|
||
| `ROLE_TEAMER` | Teamer | the CRM, automatically | the CRM, automatically | — |
|
||
|
||
`User::PENDING_ROLES` holds a marker for each of the three administrative roles, keyed by
|
||
the role it stands for:
|
||
|
||
| Marker | Meaning |
|
||
|--------|---------|
|
||
| `ROLE_ADMIN_PENDING` | the CRM claims this person is an admin, nobody has confirmed it |
|
||
| `ROLE_MANAGER_PENDING` | likewise for Reisemanager |
|
||
| `ROLE_HOUSE_MANAGER_PENDING` | likewise for Hausleitung |
|
||
|
||
Markers **grant nothing**. They appear in `getRoles()` and therefore in the security token,
|
||
but no `access_control` rule, `role_hierarchy` entry or voter references them. Their only
|
||
effects are cosmetic (rendered as "Admin (nicht freigeschaltet)") and organisational (they
|
||
put the user on the approval list). `ROLE_TEAMER` has no marker: it needs no approval.
|
||
|
||
Two further roles are synthesized by `User::getRoles()` and never stored: `ROLE_USER` for
|
||
everybody, and `ROLE_SUPER_ADMIN` when the separate `superAdmin` boolean column is set. A
|
||
validation callback (`User::validateSuperAdmin()`) refuses `superAdmin` without
|
||
`ROLE_ADMIN` alongside it — super admin is an elevation, never a standalone grant. The sync
|
||
enforces the same rule from the other side: revoking `ROLE_ADMIN` clears the flag, or the one
|
||
role that outranks every check in the application would outlive the role it depends on.
|
||
|
||
### Storage and accessors
|
||
|
||
Everything except `superAdmin` lives in the single `roles` JSON column. Three accessors
|
||
slice it, and picking the right one matters:
|
||
|
||
| Accessor | Returns |
|
||
|----------|---------|
|
||
| `getRoles()` | the column **plus** synthesized `ROLE_USER` / `ROLE_SUPER_ADMIN` — what Symfony authorises against |
|
||
| `getAssignedRoles()` | only the four real roles from the column — what the sync works on |
|
||
| `getPendingRoles()` | only the markers |
|
||
| `getNominatedRoles()` | the roles behind those markers, as `role => label` — what an approver acts on |
|
||
|
||
---
|
||
|
||
## Where roles come from: the CRM mapping
|
||
|
||
`ResponseParser::createCrmAttributesResponse()` walks the `selektionsmerkmale` of the
|
||
`SelektionCRM` response and sets four booleans plus the hotel codes:
|
||
|
||
| CRM attribute | Recognised by | Sets |
|
||
|---------------|---------------|------|
|
||
| admin | attribute id `%bpn_crm_id_admin%`, selected | `isAdmin` |
|
||
| Reisemanager | attribute id `%bpn_crm_id_manager%`, selected | `isManager` |
|
||
| teamer | attribute id `%bpn_crm_id_teamer%`, selected | `isTeamer` |
|
||
| Hausleitung | attribute id listed in `%bpn_crm_house_manager_ids%`, selected | `isHouseManager` + the hotel code that id maps to |
|
||
|
||
`bpn_crm_house_manager_ids` (`config/services.yaml`) maps a BusPro selection id to a hotel
|
||
code. Entries whose code is not a house in the `houses` parameter are kept commented out:
|
||
such a house manager could log in but would see no assignments and no dispositions at all.
|
||
|
||
`UserDataHandler::collectClaimedRoles()` turns those four booleans into plain role names -
|
||
what the CRM says. Two things are made of that list: `toPendingRoles()` produces one marker
|
||
per claimed administrative role (the roles are independent, so a Reisemanager who is also a
|
||
Hausleitung gets both), and `collectRoles()` produces the markers plus a real `ROLE_TEAMER`,
|
||
which is what a brand new account starts with.
|
||
|
||
### The shape of a BusPro response (important)
|
||
|
||
**BusPro always returns the full attribute tree.** Membership is expressed by the `auswahl`
|
||
flag on each `<selektion>`, so a role somebody does not hold arrives as `auswahl="False"`,
|
||
never as a missing element, and a person who holds nothing at all still receives every
|
||
group. Sample payloads for both cases live in `tests/Resources/crm_attributes_granted.xml`
|
||
and `crm_attributes_revoked.xml`, and `ResponseParserTest` asserts against them.
|
||
|
||
This is what makes the demotion path below safe: an **empty group set cannot occur in a real
|
||
response**, so it is a reliable signal that the response is degraded rather than that the
|
||
roles were revoked.
|
||
|
||
> Note what that check does *not* cover. The three ids are compared with `===`, and a wrong
|
||
> or unset one produces a perfectly well-formed response in which nobody holds anything —
|
||
> every user logging in would be demoted, one at a time. The attribute ids are therefore
|
||
> deployment-critical configuration, not a detail:
|
||
>
|
||
> | Parameter | Attribute |
|
||
> |-----------|-----------|
|
||
> | `APP_BPN_CRM_ID_ADMIN` | `Admin` |
|
||
> | `APP_BPN_CRM_ID_MANAGER` | `Manager` |
|
||
> | `APP_BPN_CRM_ID_TEAMER` | `E&P Teamer - allg. Merkmal` |
|
||
>
|
||
> Matching is by id and never by label, so `Preisrechner Admin` does not trip the admin flag.
|
||
>
|
||
> `bpn_crm_house_manager_ids` (`config/services.yaml`) is deployment-critical for the same
|
||
> reason, and more sharply so: since roles are synced, an id missing from that map does not
|
||
> merely fail to nominate a Hausleitung, it **revokes** the role from everyone holding it, one
|
||
> login at a time, with re-approval manual per user. Entries whose hotel code is not a house
|
||
> in the `houses` parameter are deliberately commented out there — a person holding only such
|
||
> a Hausleitung claims nothing at all and is blocked (see 4) rather than left with a role that
|
||
> shows them no data.
|
||
>
|
||
> `APP_BPN_DEFAULT_HOTEL_CODE` is a testing affordance: when set, **every** admin also
|
||
> becomes a house manager for that hotel. It must stay empty outside local development.
|
||
|
||
---
|
||
|
||
## Lifecycle
|
||
|
||
### 1. First login
|
||
|
||
`BpnAuthenticator::getOrCreateLocalUser()` → `UserDataHandler::createLocalUser()` writes
|
||
`collectRoles()` verbatim, together with the hotel codes from the Hausleitung attributes.
|
||
|
||
A CRM admin who is not also a teamer therefore starts with `['ROLE_ADMIN_PENDING']` and no
|
||
privileges at all: they can authenticate, but `UserChecker` refuses the session until a
|
||
super admin approves them.
|
||
|
||
### 2. Every subsequent login — the sync
|
||
|
||
`UserDataHandler::updateLocalUser()` refreshes name, email and the teamer record, and hands
|
||
the roles to **`syncRoles()`**, which is the whole policy in four steps:
|
||
|
||
1. **revoke** every granted role the CRM no longer claims. This is what makes BusPro the
|
||
source of truth, and it applies to `ROLE_TEAMER` as much as to the administrative roles.
|
||
2. **clear the super admin flag** when `ROLE_ADMIN` was among them — `ROLE_SUPER_ADMIN` is
|
||
synthesized from a separate column and would otherwise survive its own precondition.
|
||
3. **`refreshPendingRoles()`** recomputes the marker set from the current claims. A marker
|
||
whose real role is already granted is dropped — an approved role is never marked again.
|
||
It runs *after* the revocation, so a role just revoked is not immediately marked again.
|
||
4. **`grantTeamerRole()`** adds `ROLE_TEAMER` when the CRM claims it. Grant-only in itself;
|
||
withdrawing it is step 1's business.
|
||
|
||
**`syncHotelCodes()`** then replaces the hotel codes with the ones the CRM reports, so a
|
||
Hausleitung who moves house is not left seeing the old one.
|
||
|
||
Nothing on this path can raise a privilege: step 3 only ever produces markers.
|
||
|
||
The same two methods run on the MyE&P SSO path (`MyEpAuthenticator`), against the eligible
|
||
roles it reports — one policy, two identity sources.
|
||
|
||
### 3. Approval — turning a marker into a role
|
||
|
||
`/admin/system/user/edit/{id}`, `UserType`, gated by `UserVoter::EDIT`: super admin only,
|
||
never yourself, never while impersonating.
|
||
|
||
The page keeps three things apart, because they follow three different rules:
|
||
|
||
| Block | What it is |
|
||
|-------|-----------|
|
||
| **Aus BusPro** | information, not a form: granted roles, houses, last login. Synced on every login and editable nowhere in this application |
|
||
| **Freischaltung** | one action per nomination — a button, a confirmation dialog, its own route (`ApproveRoleController`). Only shown when the user carries a marker |
|
||
| **Account** | the actual form (`UserType`): super admin, block, block reasons |
|
||
|
||
None of this is a disabled form field. Roles and hotel codes are simply not fields, so there
|
||
is nothing to submit and nothing that looks editable but is not.
|
||
|
||
**Approval is its own act**, deliberately not a checkbox on the form: it grants a privilege,
|
||
so it is confirmed on its own, logged on its own, and cannot happen as a side effect of
|
||
saving an unrelated setting. `UserDataHandler::approveRole()` refuses any role the user has
|
||
no marker for, so a hand-crafted URL cannot grant one the CRM never claimed, and the check
|
||
runs again on submit to catch a sync that revoked the claim while the dialog was open.
|
||
|
||
A denial is not recorded anywhere: as long as the CRM keeps claiming the role, the
|
||
nomination is back on the next login.
|
||
|
||
**Super admin** is only offered to somebody who already holds `ROLE_ADMIN` — approve first,
|
||
elevate afterwards. The one exception is a flag that outlived its role, which stays editable
|
||
so the account can be saved at all while `User::validateSuperAdmin()` is violated; the sync
|
||
clears it (see 2), so it should never occur in practice.
|
||
|
||
The user list marks nominations with their own badge — it is the only place an approver would
|
||
look for them.
|
||
|
||
### 4. Losing everything — the block
|
||
|
||
Revocation of an *individual* role is step 1 of the sync above. This section is the stronger
|
||
case: the CRM claims **nothing at all**, which is not a demotion but an exit.
|
||
|
||
Evaluated on every login, in `BpnAuthenticator::getOrCreateLocalUser()`, before the local
|
||
user is even loaded. If `collectClaimedRoles()` is empty, the CRM grants this person nothing
|
||
in this application, so they are not a user of it:
|
||
|
||
| Situation | Outcome |
|
||
|-----------|---------|
|
||
| no local account | login refused, **no account created** |
|
||
| existing account | `disableForRevokedCrmRoles()` blocks it, and the user is still returned so `UserChecker` can state the reason |
|
||
| account already blocked | left completely alone — an existing block may be disciplinary and must never be overwritten |
|
||
|
||
`disableForRevokedCrmRoles()` sets `disabledAt`, a public `disabledReason` ("Für deinen
|
||
Account liegt in BusPro keine Berechtigung mehr vor.") and an internal one, and drops all
|
||
pending markers — they no longer reflect the CRM. **Granted roles are kept**, so the account
|
||
stays reviewable.
|
||
|
||
Two preconditions guard this branch, because "no roles" is otherwise indistinguishable from
|
||
"the CRM told us nothing". They now protect the sync as well: a degraded response that got
|
||
past them would not merely block one account, it would strip the roles of every user logging
|
||
in.
|
||
|
||
1. the response must be a `CrmAttributesResponse` — BusPro answers with a notification
|
||
record on its own errors;
|
||
2. it must carry at least one attribute group. Since a real response always carries the
|
||
full tree (see above), an empty group set means an empty payload or a changed schema —
|
||
which would otherwise block every user who logs in. That case refuses the single login,
|
||
logs a warning, and leaves the account untouched.
|
||
|
||
A misconfigured attribute id defeats both checks, as noted above. There is no signal inside
|
||
the response that distinguishes it from a genuine revocation.
|
||
|
||
**Regaining a CRM role does not unblock anything.** The next login skips the demotion branch,
|
||
refreshes markers as usual, and `UserChecker` still refuses on `disabledAt`. Unblocking is
|
||
always a human decision.
|
||
|
||
### 5. Blocking and unblocking by hand
|
||
|
||
Two surfaces, with deliberately different authority — the super admin rule is about
|
||
administrative users, teamers are an admin's business:
|
||
|
||
| Surface | Who | Notes |
|
||
|---------|-----|-------|
|
||
| `/admin/teamer/disable-user/{uuid}` and `/administrative/teamer/enable-user/{uuid}` | `ROLE_ADMIN` | teamers; public reason mandatory, internal optional |
|
||
| "Account gesperrt" checkbox on the user edit form | super admin (`UserVoter`) | everyone else; both reasons optional |
|
||
|
||
Both go through `User::setDisabled()`, which is a no-op when the state is unchanged — saving
|
||
an unrelated edit never resets the block timestamp — and clears both reasons on unblocking.
|
||
`UserType` additionally clears them in a `POST_SUBMIT` listener, so unticking the box without
|
||
emptying the prefilled textareas cannot leave a stale reason behind.
|
||
|
||
---
|
||
|
||
## The login gate
|
||
|
||
`UserChecker::checkPreAuth()` runs after BusPro has accepted the credentials, in this order:
|
||
|
||
| Condition | Message |
|
||
|-----------|---------|
|
||
| `disabledAt` set | `Dein Account wurde gesperrt: «disabledReason»` |
|
||
| no role from `User::ROLES`, but markers present | `Dein Account wurde noch nicht freigeschaltet.` |
|
||
| no role from `User::ROLES` at all | `Keine gültige Rolle zugewiesen.` |
|
||
|
||
Only then does `onAuthenticationSuccess()` stamp `lastLoginAt` and redirect to
|
||
`User::getDefaultRoute()` — admin, manager, house manager or teamer area, in that order of
|
||
precedence.
|
||
|
||
Note that a refused login never updates `lastLoginAt`, which is what the teamer list's
|
||
`includeInactive` filter keys on.
|
||
|
||
---
|
||
|
||
## Who is listed where
|
||
|
||
| List | Contains |
|
||
|------|----------|
|
||
| `/admin/system/user` (`UserRepository::getAdministrativeUsers()`) | holders of an administrative role, holders of a marker, and blocked accounts **without** a teamer record |
|
||
| teamer list (`TeamerRepository`) | teamers; blocked ones are badged, and stale ones need the `includeInactive` filter |
|
||
|
||
The blocked-accounts clause exists because a CRM demotion can leave a user with no role that
|
||
would list them — a candidate awaiting approval loses their markers and would otherwise
|
||
become invisible with no page to be unblocked from. Blocked teamers are excluded because
|
||
they already have one.
|
||
|
||
---
|
||
|
||
## Accountzustände
|
||
|
||
An account is in exactly one of three states. **Block and deletion are independent flags**,
|
||
so both can be set at once — a disciplinary block has to survive a deletion and the restore
|
||
that follows it.
|
||
|
||
| Zustand | Spalte | Anmeldung | Listen, Formulare, Mailings | Gesetzt von |
|
||
|---------|--------|-----------|------------------------------|-------------|
|
||
| aktiv | – | ja | enthalten | – |
|
||
| gesperrt | `user.disabled_at` | abgelehnt, Grund wird angezeigt | **weiterhin enthalten** | Admin (`DisableUserController`), CRM (`disableForRevokedCrmRoles()`) |
|
||
| gelöscht | `teamer.deleted_at` + `user.deleted_at` | abgelehnt | ausgeschlossen | ausschließlich `AccountDeletionHandler` |
|
||
|
||
A blocked teamer stays in the list on purpose: that list is the only place the block can be
|
||
lifted from. A deleted one is removed from every forward-looking process instead, which is
|
||
what makes the deletion legally meaningful.
|
||
|
||
### Why two columns
|
||
|
||
Both `Teamer` and `User` carry their own `deleted_at`. Teamers without a user exist, users
|
||
without a teamer exist, and there is no Doctrine SQL filter — every query filters by hand on
|
||
the alias it already has (`teamer` on the teamer side, `user` on the login and mail side). A
|
||
one-sided flag would force a join into roughly ten queries, each an opportunity to drop rows
|
||
through the wrong join type.
|
||
|
||
`src/Service/Teamer/AccountDeletionHandler.php` is the **only** place that writes either
|
||
flag, which is what keeps the two from drifting apart. Do not call `setDeleted()` on a Teamer
|
||
or User anywhere else.
|
||
|
||
### What a deletion excludes
|
||
|
||
`TeamerRepository::getListQuery()` (behind `includeDeleted`) and `getAutocompletionData()`,
|
||
`UserRepository::getAdministrativeUsers()` / `getUsersByRoleAndHotelCode()` /
|
||
`getAutocompletionData()`, `AvailabilityRepository::getListQuery()` (custom branch) and
|
||
`getSelectableForTeamer()`, every teamer-facing handler in `EmailNotificationSubscriber`, the
|
||
cron reminders in `UploadReminderService` and `DispositionReminderService`, new applications
|
||
(`ApplicationValidator`) and impersonation (`ImpersonationVoter`).
|
||
|
||
### What stays visible
|
||
|
||
Everything already recorded: dispositions, applications, uploads, feedback, and the contracts
|
||
and invoices rendered from them. `AssignmentRepository`, `DispositionRepository`,
|
||
`ApplicationRepository`, `UploadRepository`, `FeedbackRepository`, the dashboards and all of
|
||
`src/Service/Pdf/` deliberately do **not** filter. Deleted teamers are marked in the UI with
|
||
`templates/_partials/_teamer_deleted_badge.html.twig`; PDFs are left untouched so that a
|
||
document regenerated after a deletion is identical to the one issued before it.
|
||
|
||
### Login and CRM
|
||
|
||
`UserChecker` refuses a deleted account before it checks the block, so the deletion message
|
||
wins. `BpnAuthenticator::getOrCreateLocalUser()` returns a deleted user untouched before any
|
||
sync branch runs: nothing is written back, no role is granted or revoked, `lastLoginAt` is not
|
||
bumped. `UserDataHandler::findLocalUser()` still matches a deleted account **by email** —
|
||
excluding it there would make the caller take the person for unknown and create a second
|
||
account, resurrecting them under a new row.
|
||
|
||
Symfony's `ContextListener` does not re-run the user checker when restoring a session from
|
||
its cookie, so `DeletedUserSubscriber` ends the session of anyone deleted while logged in.
|
||
|
||
---
|
||
|
||
## Deliberate decisions and known edges
|
||
|
||
- **Markers grant nothing but are stored in `roles`.** Convenient (one column, one query),
|
||
but it means `getRoles()` contains strings that are not roles in any meaningful sense.
|
||
Always intersect against `User::ROLES` when asking "may this user do anything at all" —
|
||
`UserChecker` does exactly that.
|
||
- **`ROLE_TEAMER` is granted but never withdrawn automatically.** See 2. A strict mirror
|
||
would strip manual grants on the next login.
|
||
- **A denied nomination reappears.** See 3. There is no "rejected" state.
|
||
- **Sole-super-admin lockout.** A super admin who is not a teamer and loses their CRM admin
|
||
attribute is auto-blocked, and only another super admin can unblock them. With no second
|
||
super admin there is no route back through the UI.
|
||
- **`getUsersByRoleAndHotelCode()` matches granted roles only**, so notification recipients
|
||
(e.g. feedback reminders) never include people who are merely nominated. That is intended.
|
||
|
||
---
|
||
|
||
## File map
|
||
|
||
| Concern | File |
|
||
|---------|------|
|
||
| role constants, accessors, `setDisabled()` | `src/Entity/User.php` |
|
||
| CRM → roles mapping | `src/BusProNet/ResponseParser.php`, `src/BusProNet/Model/CrmAttributesResponse.php` |
|
||
| create / update / demote | `src/BusProNet/UserDataHandler.php` |
|
||
| login flow and the demotion branch | `src/Security/BpnAuthenticator.php` |
|
||
| login gate | `src/Security/UserChecker.php` |
|
||
| who may edit a user | `src/Security/Voter/UserVoter.php` |
|
||
| who may impersonate | `src/Security/Voter/ImpersonationVoter.php` |
|
||
| approval / block form | `src/Form/UserType.php`, `templates/admin/system/user/_form.html.twig` |
|
||
| teamer block / unblock | `src/Controller/Admin/Teamer/DisableUserController.php`, `src/Form/DisableUserType.php` |
|
||
| soft delete / restore | `src/Service/Teamer/AccountDeletionHandler.php`, `src/Entity/Traits/SoftDeletableEntity.php` |
|
||
| deletion by an admin | `src/Controller/Admin/Teamer/DeleteAccountController.php` |
|
||
| deletion by the teamer | `src/Controller/Teamer/Profile/DeleteAccountController.php`, `src/Form/DeleteAccountType.php` |
|
||
| session of a deleted user | `src/EventListener/DeletedUserSubscriber.php` |
|
||
| listing | `src/Repository/UserRepository.php` |
|
||
| hierarchy, firewall, impersonation | `config/packages/security.yaml` |
|
||
| CRM attribute ids | `config/services.yaml` |
|
||
|
||
## Tests
|
||
|
||
| File | Covers |
|
||
|------|--------|
|
||
| `tests/BusProNet/UserDataHandlerTest.php` | CRM → marker mapping, marker refresh, teamer role grant, demotion incl. the untouched disciplinary block |
|
||
| `tests/Security/BpnAuthenticatorTest.php` | the three demotion outcomes, including the empty-payload safeguard |
|
||
| `tests/Entity/UserTest.php` | `setDisabled()` block / unblock / no-op, block and deletion as independent states, OneToOne inverse-side sync |
|
||
| `tests/Service/Teamer/AccountDeletionHandlerTest.php` | deletion from either side, missing counterpart, idempotence, roles and block left untouched, restore |
|
||
| `tests/Security/UserCheckerTest.php` | deleted refused, deletion message wins over the block message |
|
||
| `tests/EventListener/EmailNotificationSubscriberTest.php` | no mail to a deleted teamer, per-teamer exclusion on an assignment call-off |
|
||
| `tests/Entity/Traits/SoftDeletableEntityTest.php` | nullable `deletedAt`, `setDeleted()` / `setRestored()` |
|
||
| `tests/Form/UserTypeTest.php` | approval clears the marker, block with reasons, unblock clears them even when the fields are still filled |
|