chore: reference for user roles and for worker operations

This commit is contained in:
Björn Fromme
2026-08-12 17:55:37 +02:00
parent 3b116add75
commit 44fab8bf6c
2 changed files with 472 additions and 0 deletions
+135
View File
@@ -0,0 +1,135 @@
# Operations: messenger workers and cron
Reference for how queued work actually gets executed in production. Describes the setup as
it is — not a plan.
The governing fact, from which most of the rest follows:
> **Nothing in this repository starts a worker.** Both workers are crontab entries on the
> host, outside version control. `deploy.php` only ever *stops* workers, and even that is
> largely ineffective (see below). If you change how a queue is consumed, change it here
> too, or the next person has to reverse-engineer it from the config.
---
## Transports
Defined in `config/packages/messenger.yaml`, all backed by the same Doctrine table
(`MESSENGER_TRANSPORT_DSN=doctrine://default?auto_setup=0`):
| Transport | `queue_name` | Carries | Consumed by |
|-----------|--------------|---------|-------------|
| `async` | `default` | all ordinary mail, chat and SMS notifications | the async cron worker |
| `mailing` | `mailing` | bulk teamer mailings only | the mailing cron worker |
| `failed` | `failed` | anything that exhausted its retries | nobody — inspected by hand |
| `sync` | — | dev and test only | the request itself |
The two live workers are isolated **by the `queue_name` column**, not by how they are
started: the Doctrine transport's `get()` filters on it, so neither worker can see the
other's messages. Ordinary mail therefore never queues behind a mailing of several hundred
recipients.
Overlapping runs of the same worker are harmless. `Connection::get()` selects
`FOR UPDATE SKIP LOCKED` inside a transaction and stamps `delivered_at` before committing,
so two consumers can never be handed the same row. No `flock` is needed; an overlap only
costs a second process.
## The cron entries
> **The async line below is a placeholder.** Read the real one off the host
> (`crontab -l` as `p704161`) and transcribe it verbatim, including any differences in
> flags, php path or logging — those differences are information.
```cron
# ordinary mail, notifications
*/15 * * * * cd /home/www/p704161/html/myep-team/current && /usr/local/bin/php bin/console messenger:consume async --time-limit=... --memory-limit=... --quiet >> /home/www/p704161/html/myep-team/shared/var/log/cron-messenger-async.log 2>&1
# bulk teamer mailings
*/15 * * * * cd /home/www/p704161/html/myep-team/current && /usr/local/bin/php bin/console messenger:consume mailing --time-limit=300 --memory-limit=128M --quiet >> /home/www/p704161/html/myep-team/shared/var/log/cron-messenger-mailing.log 2>&1
```
Why each part is the way it is:
- **`/usr/local/bin/php`, not bare `php`.** `deploy.php` sets `bin/php` to this path
explicitly for both hosts, because the host's default CLI is not the right one. Cron's
`PATH` is typically only `/usr/bin:/bin`. `vendor/` is rsynced from a developer machine
and there is no `deploy:vendors`, so the CLI must match the PHP the dependencies were
installed against (8.3).
- **`--time-limit=300`** on the mailing worker. An 800-recipient mailing needs roughly
200360 seconds of worker time — the fan-out renders 800 Twig bodies and inserts 800
rows, then the mails go out at about 35 per second over a reused SMTP connection. So a
mailing usually drains in one window and occasionally spills into the next; worst case
end to end is about half an hour. Raise it toward 840 for lower latency at the cost of an
effectively always-on process, lower it for a lighter footprint. Anything under 900
guarantees two runs cannot overlap.
- **Stopping mid-mailing is safe.** The time limit is only checked *between* messages
(`StopWorkerOnTimeLimitListener` listens on `WorkerRunningEvent`), so it never truncates
a send or a half-finished fan-out. Unhandled rows keep `delivered_at = NULL` and the next
run picks them up.
- **`--memory-limit` is a recycle trigger, not a safety net.** It too is only checked
between messages, so it cannot protect the fan-out, which is the memory peak. That is
down to the CLI `memory_limit` ini — keep it at 256M or more.
- **`--quiet` plus a redirect.** `messenger:consume` prints a banner on every run and
unredirected cron output is mailed to the crontab user, which would be 96 mails a day.
Nothing is lost: errors still reach `var/log/framework.prod.log` through the prod monolog
handler. Redirect into the **shared** `var/log`, which is in `shared_dirs` and survives
deploys — never anywhere under `var/cache`, which is per-release by design.
- **No `APP_ENV=prod` on the line.** `bin/console` loads Dotenv relative to the project
dir, and `APP_ENV` lives in the shared `.env.local`. A real environment variable takes
precedence over that file, so putting it in the crontab would create a second source of
truth and would need a different line for staging, which runs on the same host under the
same user.
## Deploys do not restart workers
`deploy.php` ends with `deploy:stop-workers`, which runs `messenger:stop-workers`. That
writes a restart timestamp into a cache pool under `%kernel.cache_dir%`, i.e. into the
**new release**. A worker that is already running was started from the previous release and
stays pinned to that release's `var/cache` for its whole life, so it never sees the flag.
**`--time-limit` is therefore what actually bounds a worker's life and what makes new code
take effect** — up to five minutes after a deploy for the mailing worker. This is not worth
"fixing" by moving `var/cache` into `shared_dirs`; that would break cache warming and
opcache invalidation far worse than the problem it solves.
Related: `keep_releases: 3`. A worker must not outlive three deploys, or it will be running
out of a directory that has been deleted. At a five-minute lifetime this cannot happen.
## When something goes wrong
```bash
bin/console messenger:stats # queue depths, including failed
bin/console messenger:failed:show # what failed and why
bin/console messenger:failed:show <id> -vv # the full exception
bin/console messenger:failed:retry # requeue, interactively
```
Nothing watches the `failed` transport, so a failed mailing stays silent until somebody
looks. Check it after any large mailing.
The scheduler's `email_on_failure` does not help here, and covers less than it appears to:
a task only counts as failed when its command exits non-zero, and `App\Command\CronCommand`
always returns `Command::SUCCESS` — even where it renders `$io->error()` for a failed log
flush. So none of the five services it orchestrates can currently raise an alert.
Three failure modes worth knowing because they are quiet rather than loud:
- **`.env.local` missing from the deploy path.** `APP_ENV` falls back to `dev` and
`MAILING_MAILER_DSN` to `null://null`. The worker then consumes every message and
delivers nothing, reporting success and recording no failure. The quietest failure in the
whole system.
- **No worker consuming `mailing`.** Mailings queue up indefinitely while the admin is told
they are on their way. `messenger:stats` shows it immediately.
- **Wrong PHP CLI.** A version mismatch against the rsynced `vendor/` fails at parse time,
inside a cron job nobody reads. This is why the log redirect exists.
## Scheduled tasks are a different mechanism
`config/packages/zenstruck_schedule.yaml` holds the daily jobs (`app:cron`,
`app:bpn-import`, `app:teamer-status`, …), driven by a separate `schedule:run` cron entry.
**Do not put `messenger:consume` in there.** The bundle runs tasks in-process and
sequentially, so a consume task holds the entire schedule for its duration — every later
task waits, and the next `schedule:run` starts concurrently because no task declares
`withoutOverlapping`. This was tried and reverted (`dc796849`, "remove messenger from
scheduler config to be run separately"). Workers belong in their own crontab lines.
+337
View File
@@ -0,0 +1,337 @@
# 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 rule, from which most of the rest follows:
> **The BusPro 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.
---
## Role catalogue
Defined in `User::ROLES` (`src/Entity/User.php`), these four are the only roles a human can
assign:
| Role | Label | Granted by | Hierarchy |
|------|-------|-----------|-----------|
| `ROLE_ADMIN` | Admin | super admin, manually | ⇒ `ROLE_ADMINISTRATIVE` |
| `ROLE_MANAGER` | Reisemanager | super admin, manually | ⇒ `ROLE_ADMINISTRATIVE` |
| `ROLE_HOUSE_MANAGER` | Hausleitung | super admin, manually | — |
| `ROLE_TEAMER` | Teamer | 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.
### 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 edit form binds to |
| `getPendingRoles()` | only the markers |
---
## 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 | label matching `/^Hausleitung ([A-Z0-9]+)$/`, selected | `isHouseManager` + one hotel code per match |
`UserDataHandler::collectPendingRoles()` turns those into markers — `isManager` wins over
`isHouseManager`, they are never both claimed, though the hotel codes of a Hausleitung are
imported eitherway. `collectRoles()` is that set plus a real `ROLE_TEAMER` when `isTeamer`.
### 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.
>
> `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 — the only automatic grant of anything
`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
`UserDataHandler::updateLocalUser()` refreshes name, email and the teamer record, and then:
- **`refreshPendingRoles()`** recomputes the marker set from the current CRM claims. A
marker whose real role is already granted is dropped — an approved role is never marked
again. Granted roles are kept untouched.
- **`grantTeamerRole()`** adds `ROLE_TEAMER` if the CRM reports a teamer and the user does
not have it yet. This is deliberately *grant-only*: the role is never withdrawn here,
because it may have been handed out manually and must survive a login. Losing the CRM
teamer attribute while holding no other role blocks the account anyway (see 4).
Administrative roles and hotel codes are imported at creation and are managed by hand
afterwards. Nothing on this path can raise a privilege.
### 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 `roles` field binds to `assignedRoles`, whose setter **replaces the whole column**.
Consequences to be aware of:
- Approving is "tick the real role and save". The marker disappears because the column is
rewritten from the ticked choices.
- Saving the form drops *every* marker, including ones you did not act on.
- A denial is not recorded anywhere. As long as the CRM keeps claiming the role, the marker
returns on that user's next login.
### 4. Revocation — demotion by the CRM
Evaluated on every login, in `BpnAuthenticator::getOrCreateLocalUser()`, before the local
user is even loaded. If `collectRoles()` 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":
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 |