chore: reference for user roles and for worker operations

.env

@@ -93,5 +93,7 @@ [email protected]
 
 FEATURE_SANITIZE_UPLOADS=false
 FEATURE_STAMP_INVOICES=false
+FEATURE_DATEV_EMAIL=false
+FEATURE_MYEP_LOGIN=false
 
 UPLOAD_REPLACEMENT_FILE=assets/pdf/chicken.pdf

docs/operations.md

@@ -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
+  200–360 seconds of worker time — the fan-out renders 800 Twig bodies and inserts 800
+  rows, then the mails go out at about 3–5 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.

docs/user-roles.md

@@ -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 |

src/BusProNet/UserDataHandler.php

@@ -11,6 +11,13 @@ use App\Entity\User;
 use Doctrine\ORM\EntityManagerInterface;
 use Psr\Log\LoggerInterface;
 
+/**
+ * Owns the role policy for local user accounts, whatever identity source reports them.
+ * The importing of BusPro profile data is specific to the CRM, but the role rules -
+ * toPendingRoles(), refreshPendingRoles() and grantTeamerRole() - are deliberately
+ * shared with the MyE&P SSO login (see App\Security\MyEpAuthenticator), so that no
+ * identity source can grant an administrative role this application would not.
+ */
 class UserDataHandler
 {
     public function __construct(
@@ -45,15 +52,45 @@ class UserDataHandler
      */
     public function collectPendingRoles(CrmAttributesResponse $crmAttributes): array
     {
-        $roles = [];
+        $claimedRoles = [];
 
         if ($crmAttributes->isAdmin()) {
-            $roles[] = User::PENDING_ROLES['ROLE_ADMIN'];
+            $claimedRoles[] = 'ROLE_ADMIN';
         }
 
         if ($crmAttributes->isManager()) {
+            $claimedRoles[] = 'ROLE_MANAGER';
+        }
+
+        if ($crmAttributes->isHouseManager()) {
+            $claimedRoles[] = 'ROLE_HOUSE_MANAGER';
+        }
+
+        return $this->toPendingRoles($claimedRoles);
+    }
+
+    /**
+     * Maps a list of claimed roles to the pending markers for the administrative ones
+     * among them. This is the same policy collectPendingRoles() applies to the CRM
+     * attributes, expressed over plain role names so that any identity source can use
+     * it. ROLE_TEAMER has no marker and is dropped here: it needs no approval.
+     *
+     * @param string[] $claimedRoles
+     *
+     * @return string[]
+     */
+    public function toPendingRoles(array $claimedRoles): array
+    {
+        $roles = [];
+
+        if (true === in_array('ROLE_ADMIN', $claimedRoles, true)) {
+            $roles[] = User::PENDING_ROLES['ROLE_ADMIN'];
+        }
+
+        // a manager outranks a house manager, so only the higher marker is kept
+        if (true === in_array('ROLE_MANAGER', $claimedRoles, true)) {
             $roles[] = User::PENDING_ROLES['ROLE_MANAGER'];
-        } elseif ($crmAttributes->isHouseManager()) {
+        } elseif (true === in_array('ROLE_HOUSE_MANAGER', $claimedRoles, true)) {
             $roles[] = User::PENDING_ROLES['ROLE_HOUSE_MANAGER'];
         }
 
@@ -279,8 +316,10 @@ class UserDataHandler
      *
      * It is never withdrawn here. Losing the CRM attribute while holding no other role
      * blocks the account anyway, and a role handed out manually must survive a login.
+     *
+     * Does not flush; the caller decides when to.
      */
-    private function grantTeamerRole(User $user): void
+    public function grantTeamerRole(User $user): void
     {
         $grantedRoles = $user->getAssignedRoles();
 
@@ -302,9 +341,11 @@ class UserDataHandler
      * super admin can turn one into an actual role, and an already granted role is never
      * marked as pending again.
      *
-     * @param string[] $claimedRoles
+     * Does not flush; the caller decides when to.
+     *
+     * @param string[] $claimedRoles pending markers as returned by toPendingRoles()
      */
-    private function refreshPendingRoles(User $user, array $claimedRoles): void
+    public function refreshPendingRoles(User $user, array $claimedRoles): void
     {
         // an approved role needs no marker anymore
         $grantedRoles = $user->getAssignedRoles();

src/Controller/Security/OAuth2Controller.php

@@ -3,6 +3,7 @@
 namespace App\Controller\Security;
 
 use App\Security\OAuth2\MyEpClient;
+use Flagception\Manager\FeatureManagerInterface;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -10,13 +11,17 @@ use Symfony\Component\Routing\Attribute\Route;
 
 class OAuth2Controller extends AbstractController
 {
-    public function __construct(private readonly MyEpClient $client)
-    {
+    public function __construct(
+        private readonly MyEpClient $client,
+        private readonly FeatureManagerInterface $featureManager,
+    ) {
     }
 
     #[Route('/myep-auth/init', name: 'app_myep_auth_init')]
     public function init(Request $request): Response
     {
+        $this->denyUnlessFeatureIsActive();
+
         $provider = $this->client->getProvider();
         $url = $provider->getAuthorizationUrl();
         $state = $provider->getState();
@@ -28,5 +33,19 @@ class OAuth2Controller extends AbstractController
     #[Route('/myep-auth/check', name: 'app_myep_auth_check')]
     public function check(): void
     {
+        // only reached when the authenticator did not handle the request, which with the
+        // feature switched off it never does
+        $this->denyUnlessFeatureIsActive();
+    }
+
+    /**
+     * The login is not merely hidden while the feature is off: the callback creates and
+     * updates local accounts, so the routes must not be reachable by direct URL either.
+     */
+    private function denyUnlessFeatureIsActive(): void
+    {
+        if (false === $this->featureManager->isActive('myep_login')) {
+            throw $this->createNotFoundException();
+        }
     }
 }

src/Entity/Teamer.php

@@ -250,11 +250,11 @@ class Teamer implements TimestampableEntityInterface, SoftDeletableEntityInterfa
             ->setGender($userinfo['profile']['gender'] ?? null)
         ;
 
-        if (null !== $userinfo['profile']['date_of_birth'] ?? null) {
+        if (true === isset($userinfo['profile']['date_of_birth'])) {
             try {
-                $dateOfBirth = new \DateTimeImmutable(['profile']['date_of_birth']);
+                $dateOfBirth = new \DateTimeImmutable((string) $userinfo['profile']['date_of_birth']);
                 $instance->setDateOfBirth($dateOfBirth);
-            } catch (\Exception $e) {
+            } catch (\Exception|\TypeError $e) {
             }
         }
 

src/Security/MyEpAuthenticator.php

@@ -5,9 +5,11 @@ namespace App\Security;
 use App\BusProNet\UserDataHandler;
 use App\Entity\Teamer;
 use App\Entity\User;
+use App\RequiredTeamerCheck\RequiredTeamerCheckRegistry;
 use App\Security\OAuth2\AuthorizationRequestException;
 use App\Security\OAuth2\MyEpClient;
 use Doctrine\ORM\EntityManagerInterface;
+use Flagception\Manager\FeatureManagerInterface;
 use League\OAuth2\Client\Provider\Exception\IdentityProviderException;
 use Psr\Http\Client\ClientExceptionInterface;
 use Psr\Log\LoggerInterface;
@@ -26,22 +28,35 @@ use Symfony\Component\Security\Http\Util\TargetPathTrait;
 
 class MyEpAuthenticator extends AbstractAuthenticator
 {
-    private const ELIGIBLE_ROLES = ['ROLE_ADMIN', 'ROLE_TEAMER', 'ROLE_MANAGER', 'ROLE_HOUSE_MANAGER'];
-
     use TargetPathTrait;
 
+    /**
+     * The roles that entitle someone to log in here at all. Anything else MyE&P reports
+     * is dropped rather than stored, so that no role this application assigns a meaning
+     * to can be set from the outside.
+     */
+    private const ELIGIBLE_ROLES = ['ROLE_ADMIN', 'ROLE_TEAMER', 'ROLE_MANAGER', 'ROLE_HOUSE_MANAGER'];
+
     public function __construct(
         private readonly MyEpClient $client,
         private readonly UserDataHandler $userDataHandler,
         private readonly EntityManagerInterface $entityManager,
         private readonly UrlGeneratorInterface $urlGenerator,
         private readonly LoggerInterface $logger,
+        private readonly FeatureManagerInterface $featureManager,
+        private readonly RequiredTeamerCheckRegistry $requiredTeamerCheckRegistry,
     ) {
     }
 
     public function supports(Request $request): ?bool
     {
-        return 'app_myep_auth_check' === $request->attributes->get('_route');
+        if ('app_myep_auth_check' !== $request->attributes->get('_route')) {
+            return false;
+        }
+
+        // the controller guards the route as well, but the authenticator must not fire
+        // on a disabled feature either: it is what creates and updates accounts
+        return $this->featureManager->isActive('myep_login');
     }
 
     public function authenticate(Request $request): Passport
@@ -58,30 +73,44 @@ class MyEpAuthenticator extends AbstractAuthenticator
         try {
             $provider = $this->client->getProvider();
             $url = $provider->getResourceOwnerDetailsUrl($accessToken);
-            $request = $provider->getAuthenticatedRequest('GET', $url, $accessToken, [
+            $userinfoRequest = $provider->getAuthenticatedRequest('GET', $url, $accessToken, [
                 'headers' => [
                     'Accept' => 'application/json',
                     'Content-Type' => 'application/json',
                 ],
             ]);
-            $response = $provider->getHttpClient()->sendRequest($request);
+            $response = $provider->getHttpClient()->sendRequest($userinfoRequest);
         } catch (ClientExceptionInterface $e) {
             $this->logger->error('Login via MyE&P failed due to unexpected userinfo response');
             throw new CustomUserMessageAuthenticationException('Login via MyE&P nicht möglich');
         }
-        $userinfo = json_decode($response->getBody(), true);
-        $username = $userinfo['email'] ?? null;
 
-        if (null === $username) {
+        $userinfo = json_decode($response->getBody(), true);
+
+        if (false === is_array($userinfo)) {
+            $this->logger->error('Login via MyE&P failed due to unreadable userinfo payload');
+            throw new CustomUserMessageAuthenticationException('Login via MyE&P nicht möglich');
+        }
+
+        if (null === ($userinfo['email'] ?? null)) {
             $this->logger->error('Login via MyE&P failed due to missing username claim');
             throw new CustomUserMessageAuthenticationException('Login via MyE&P nicht möglich');
         }
 
-        if (null !== $this->createOrUpdateUserFromUserinfo($userinfo)) {
-            return new SelfValidatingPassport(new UserBadge($username));
+        $user = $this->createOrUpdateUserFromUserinfo($userinfo);
+
+        if (null === $user) {
+            throw new CustomUserMessageAuthenticationException('Login via MyE&P nicht möglich');
         }
 
-        throw new CustomUserMessageAuthenticationException('Login via MyE&P nicht möglich');
+        // The user is resolved here rather than by the user provider: the account is
+        // matched on the BusPro ids, which the provider cannot do, and handing it an
+        // email would risk loading a different row than the one just written. The
+        // UserChecker still runs and still refuses deleted, disabled and unapproved
+        // accounts.
+        return new SelfValidatingPassport(
+            new UserBadge($user->getUserIdentifier(), static fn (): User => $user),
+        );
     }
 
     public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
@@ -89,13 +118,26 @@ class MyEpAuthenticator extends AbstractAuthenticator
         /** @var User $user */
         $user = $token->getUser();
 
+        // recorded here rather than while importing, so that only a login that actually
+        // passed the UserChecker is counted as one
+        $user->setLastLoginAt(new \DateTimeImmutable());
+        $this->entityManager->flush();
+
         $this->logger->info('Logged in');
 
         if ($targetPath = $this->getTargetPath($request->getSession(), $firewallName)) {
             return new RedirectResponse($targetPath);
         }
 
-        $url = $this->urlGenerator->generate($user->getDefaultRoute());
+        $route = $user->getDefaultRoute();
+        if ('app_teamer_index' === $route) {
+            $check = $this->requiredTeamerCheckRegistry->getFirstUnresolvedCheck($user);
+            if (null !== $check) {
+                $route = $check->getRouteName();
+            }
+        }
+
+        $url = $this->urlGenerator->generate($route);
 
         return new RedirectResponse($url);
     }
@@ -111,57 +153,185 @@ class MyEpAuthenticator extends AbstractAuthenticator
 
     private function createOrUpdateUserFromUserinfo(array $userinfo): ?User
     {
-        // User is expected to have at least one role
-        if (false === isset($userinfo['roles']) || 0 === count($userinfo['roles'])) {
-            // Claim keys only, the payload itself carries the full profile
-            $this->logger->warning('Login via MyE&P failed due to missing roles claim', [
-                'claims' => array_keys($userinfo),
-            ]);
+        $claimedRoles = $this->collectClaimedRoles($userinfo);
 
+        if ([] === $claimedRoles) {
             return null;
         }
 
-        // User is expected to have at least one of the roles teamer, manager, house manager or admin
-        if ([] === array_intersect(self::ELIGIBLE_ROLES, $userinfo['roles'])) {
-            $this->logger->warning('Login via MyE&P failed due to lack of an eligible role', [
-                'roles' => $userinfo['roles'],
-                'eligible_roles' => self::ELIGIBLE_ROLES,
+        $isTeamer = in_array('ROLE_TEAMER', $claimedRoles, true);
+        $pendingRoles = $this->userDataHandler->toPendingRoles($claimedRoles);
+
+        $user = $this->findLocalUser($userinfo);
+
+        // A deleted account is excluded from every process, and MyE&P must not be able to
+        // undo that: no data is written back, no role is granted or revoked, not even
+        // lastLoginAt is bumped. It is returned untouched so the UserChecker can refuse
+        // the login and say why. Only an admin restores it.
+        if (true === $user?->isDeleted()) {
+            $this->logger->info('Skip MyE&P sync for deleted user', [
+                'user_id' => $user->getId(),
+                'user_email' => $userinfo['email'],
             ]);
 
-            return null;
+            return $user;
         }
 
-        // Check if user is already present in local database
-        $user = $this
-            ->entityManager
-            ->getRepository(User::class)
-            ->findOneBy(['email' => $userinfo['email']])
-        ;
-
-        // Update existing user's roles and teamer data and return it
+        // Update existing user, leaving granted roles and hotel codes alone: they are
+        // imported once on creation and managed manually afterwards. Only the
+        // privilege-free markers and ROLE_TEAMER track MyE&P on every login.
         if (null !== $user) {
-            $user
-                ->setRoles($userinfo['roles'])
-                ->setHotelCodes($userinfo['profile']['hotel_codes'])
-                ->setLastLoginAt(new \DateTimeImmutable('now'))
-            ;
+            $this->refreshBusProIds($user, $userinfo);
+            $this->userDataHandler->refreshPendingRoles($user, $pendingRoles);
+
+            if (true === $isTeamer) {
+                $this->userDataHandler->grantTeamerRole($user);
+            }
 
             $this->entityManager->flush();
 
             return $user;
         }
 
+        return $this->createLocalUser($userinfo, $pendingRoles, $isTeamer);
+    }
+
+    /**
+     * The roles MyE&P grants this person in this application.
+     *
+     * The remote list is filtered down to the eligible roles rather than trusted as it
+     * stands: anything else the identity provider reports is meaningless here at best,
+     * and at worst a role this application assigns a meaning to - ROLE_ADMINISTRATIVE
+     * from the role hierarchy, say - which must never be settable from the outside.
+     *
+     * @return string[]
+     */
+    private function collectClaimedRoles(array $userinfo): array
+    {
+        $roles = $userinfo['roles'] ?? [];
+
+        // User is expected to have at least one role
+        if (false === is_array($roles) || [] === $roles) {
+            // Claim keys only, the payload itself carries the full profile
+            $this->logger->warning('Login via MyE&P failed due to missing roles claim', [
+                'claims' => array_keys($userinfo),
+            ]);
+
+            return [];
+        }
+
+        // User is expected to have at least one of the roles teamer, manager, house manager or admin
+        $claimedRoles = array_values(array_intersect(self::ELIGIBLE_ROLES, $roles));
+
+        if ([] === $claimedRoles) {
+            $this->logger->warning('Login via MyE&P failed due to lack of an eligible role', [
+                'roles' => $roles,
+                'eligible_roles' => self::ELIGIBLE_ROLES,
+            ]);
+        }
+
+        return $claimedRoles;
+    }
+
+    /**
+     * Matches the local account on the BusPro ids first and falls back to the email, the
+     * same precedence UserDataHandler::findLocalUser() applies to a BusPro login, so that
+     * the two login paths cannot create two accounts for the same person.
+     */
+    private function findLocalUser(array $userinfo): ?User
+    {
+        $repository = $this->entityManager->getRepository(User::class);
+
+        $addressId = $userinfo['address_id'] ?? null;
+        $personId = $userinfo['person_id'] ?? null;
+
+        if (null !== $addressId && null !== $personId) {
+            $user = $repository->findOneBy([
+                'busProAddressId' => $addressId,
+                'busProPersonId' => $personId,
+            ]);
+
+            if (null !== $user) {
+                return $user;
+            }
+        }
+
+        $users = $repository->findBy(['email' => $userinfo['email']]);
+
+        if (1 !== count($users)) {
+            if (1 < count($users)) {
+                $this->logger->warning('Unable to match local user by email: multiple users found', [
+                    'email' => $userinfo['email'],
+                    'count' => count($users),
+                ]);
+            }
+
+            return null;
+        }
+
+        return $users[0];
+    }
+
+    /**
+     * Backfills the BusPro ids on an account matched by email, so the next login matches
+     * on the ids instead. Never called for a deleted user, who must not be written to.
+     */
+    private function refreshBusProIds(User $user, array $userinfo): void
+    {
+        $addressId = $userinfo['address_id'] ?? null;
+        $personId = $userinfo['person_id'] ?? null;
+
+        if (null === $addressId || null === $personId) {
+            $this->logger->warning('Skip BusPro ID refresh: userinfo missing IDs', [
+                'user_id' => $user->getId(),
+                'user_email' => $user->getEmail(),
+                'bus_pro_address_id' => $addressId,
+                'bus_pro_person_id' => $personId,
+            ]);
+
+            return;
+        }
+
+        $user
+            ->setBusProAddressId((int) $addressId)
+            ->setBusProPersonId((int) $personId)
+        ;
+    }
+
+    /**
+     * @param string[] $pendingRoles
+     */
+    private function createLocalUser(array $userinfo, array $pendingRoles, bool $isTeamer): ?User
+    {
+        $addressId = $userinfo['address_id'] ?? null;
+        $personId = $userinfo['person_id'] ?? null;
+
+        // Both are required columns and there is nothing sensible to fall back to, so an
+        // account is not created at all rather than half-created from a partial payload
+        if (null === $addressId || null === $personId) {
+            $this->logger->warning('Login via MyE&P failed due to missing BusPro ID claims', [
+                'email' => $userinfo['email'],
+                'claims' => array_keys($userinfo),
+            ]);
+
+            return null;
+        }
+
+        $profile = is_array($userinfo['profile'] ?? null) ? $userinfo['profile'] : [];
+        $hotelCodes = is_array($profile['hotel_codes'] ?? null) ? $profile['hotel_codes'] : [];
+
         $user = new User();
         $user
-            ->setFirstName($userinfo['profile']['first_name'])
-            ->setLastName($userinfo['profile']['last_name'])
-            ->setEmail($userinfo['profile']['communication']['email'])
-            ->setBusProPersonId($userinfo['id'])
-            ->setHotelCodes($userinfo['profile']['hotel_codes'])
-            ->setRoles($userinfo['roles'])
+            ->setFirstName($profile['first_name'] ?? null)
+            ->setLastName($profile['last_name'] ?? null)
+            ->setEmail((string) $userinfo['email'])
+            ->setBusProAddressId((int) $addressId)
+            ->setBusProPersonId((int) $personId)
+            ->setHotelCodes($hotelCodes)
+            ->setRoles([...$pendingRoles, ...($isTeamer ? ['ROLE_TEAMER'] : [])])
         ;
 
-        if (true === in_array('ROLE_TEAMER', $user->getRoles(), true)) {
+        if (true === $isTeamer) {
             $teamer = Teamer::fromUserinfo($userinfo);
             $user->setTeamer($teamer);
 
@@ -176,6 +346,11 @@ class MyEpAuthenticator extends AbstractAuthenticator
         $this->entityManager->persist($user);
         $this->entityManager->flush();
 
+        $this->logger->info('Create user', [
+            'user_id' => $user->getId(),
+            'user_email' => $user->getEmail(),
+        ]);
+
         return $user;
     }
 }

tests/BusProNet/UserDataHandlerTest.php

@@ -83,6 +83,59 @@ class UserDataHandlerTest extends TestCase
         ];
     }
 
+    /**
+     * The role policy has to be identical whichever identity source claims the role, so
+     * toPendingRoles() must agree with collectPendingRoles() on equivalent input.
+     *
+     * @dataProvider toPendingRolesProvider
+     */
+    public function testToPendingRolesMatchesTheCrmPolicy(array $claimedRoles, CrmAttributesResponse $crmAttributes, array $expectedRoles): void
+    {
+        $handler = new UserDataHandler($this->entityManager, $this->logger);
+
+        $this->assertSame($expectedRoles, $handler->toPendingRoles($claimedRoles));
+        $this->assertSame($expectedRoles, $handler->collectPendingRoles($crmAttributes));
+    }
+
+    public static function toPendingRolesProvider(): iterable
+    {
+        yield 'admin' => [
+            ['ROLE_ADMIN'],
+            (new CrmAttributesResponse())->setAdmin(true),
+            [User::PENDING_ROLES['ROLE_ADMIN']],
+        ];
+
+        yield 'manager takes precedence over house manager' => [
+            ['ROLE_HOUSE_MANAGER', 'ROLE_MANAGER'],
+            (new CrmAttributesResponse())->setManager(true)->setHouseManager(true),
+            [User::PENDING_ROLES['ROLE_MANAGER']],
+        ];
+
+        yield 'house manager' => [
+            ['ROLE_HOUSE_MANAGER'],
+            (new CrmAttributesResponse())->setHouseManager(true),
+            [User::PENDING_ROLES['ROLE_HOUSE_MANAGER']],
+        ];
+
+        yield 'admin and house manager' => [
+            ['ROLE_ADMIN', 'ROLE_HOUSE_MANAGER'],
+            (new CrmAttributesResponse())->setAdmin(true)->setHouseManager(true),
+            [User::PENDING_ROLES['ROLE_ADMIN'], User::PENDING_ROLES['ROLE_HOUSE_MANAGER']],
+        ];
+
+        yield 'teamer has no marker' => [
+            ['ROLE_TEAMER'],
+            (new CrmAttributesResponse())->setTeamer(true),
+            [],
+        ];
+
+        yield 'nothing claimed' => [
+            [],
+            new CrmAttributesResponse(),
+            [],
+        ];
+    }
+
     public function testUpdateLocalUserSyncsUserAndTeamerDataFromBusPro(): void
     {
         $user = (new User())

tests/Security/MyEpAuthenticatorTest.php

@@ -0,0 +1,394 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Security;
+
+use App\BusProNet\UserDataHandler;
+use App\Entity\User;
+use App\RequiredTeamerCheck\RequiredTeamerCheckRegistry;
+use App\Security\MyEpAuthenticator;
+use App\Security\OAuth2\MyEpClient;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\Persistence\ObjectRepository;
+use Flagception\Manager\FeatureManagerInterface;
+use GuzzleHttp\Psr7\Request as Psr7Request;
+use GuzzleHttp\Psr7\Response as Psr7Response;
+use League\OAuth2\Client\Provider\AbstractProvider;
+use League\OAuth2\Client\Token\AccessToken;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Psr\Http\Client\ClientInterface;
+use Psr\Log\LoggerInterface;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Session\Session;
+use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
+use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
+use Symfony\Component\Security\Core\Exception\CustomUserMessageAuthenticationException;
+use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
+
+class MyEpAuthenticatorTest extends TestCase
+{
+    private EntityManagerInterface&MockObject $entityManager;
+    private ObjectRepository&MockObject $repository;
+
+    /** @var User[] */
+    private array $persisted = [];
+
+    protected function setUp(): void
+    {
+        $this->persisted = [];
+        $this->repository = $this->createMock(ObjectRepository::class);
+        $this->entityManager = $this->createMock(EntityManagerInterface::class);
+        $this->entityManager->method('getRepository')->willReturn($this->repository);
+        $this->entityManager
+            ->method('persist')
+            ->willReturnCallback(function (object $entity): void {
+                if ($entity instanceof User) {
+                    $this->persisted[] = $entity;
+                }
+            })
+        ;
+    }
+
+    /**
+     * The core of this test case: MyE&P may report an administrative role, but it may
+     * not grant one. Only a super admin turns the marker into the real role.
+     */
+    public function testAdministrativeRolesAreImportedAsPendingMarkersOnly(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $user = $this->loadUser($this->createUserinfo(['ROLE_ADMIN', 'ROLE_TEAMER']));
+
+        $this->assertSame([User::PENDING_ROLES['ROLE_ADMIN']], $user->getPendingRoles());
+        $this->assertSame(['ROLE_TEAMER'], $user->getAssignedRoles());
+        $this->assertNotContains('ROLE_ADMIN', $user->getRoles());
+        $this->assertContains('ROLE_TEAMER', $user->getRoles());
+    }
+
+    public function testUnknownRolesAreNeverStored(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        // ROLE_ADMINISTRATIVE is granted by the role hierarchy and must not be settable
+        // from the outside, ROLE_KUNDE means nothing here
+        $userinfo = $this->createUserinfo(['ROLE_TEAMER', 'ROLE_ADMINISTRATIVE', 'ROLE_KUNDE']);
+
+        $user = $this->loadUser($userinfo);
+
+        $this->assertSame(['ROLE_USER', 'ROLE_TEAMER'], $user->getRoles());
+    }
+
+    public function testLoginIsRefusedWithoutAnEligibleRole(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $this->expectException(CustomUserMessageAuthenticationException::class);
+
+        try {
+            $this->loadUser($this->createUserinfo(['ROLE_KUNDE']));
+        } finally {
+            $this->assertSame([], $this->persisted, 'no account may be created');
+        }
+    }
+
+    public function testLoginIsRefusedWithoutAnyRoleClaim(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $userinfo = $this->createUserinfo([]);
+        unset($userinfo['roles']);
+
+        $this->expectException(CustomUserMessageAuthenticationException::class);
+
+        $this->loadUser($userinfo);
+    }
+
+    /**
+     * A super admin's manual demotion has to survive the user's next SSO login.
+     */
+    public function testExistingGrantedRolesAreNeverOverwritten(): void
+    {
+        $user = (new User())->setEmail('[email protected]')->setRoles(['ROLE_TEAMER']);
+        $this->repository->method('findOneBy')->willReturn($user);
+
+        $this->loadUser($this->createUserinfo(['ROLE_ADMIN', 'ROLE_MANAGER', 'ROLE_TEAMER']));
+
+        $this->assertSame(['ROLE_TEAMER'], $user->getAssignedRoles());
+        $this->assertSame(
+            [User::PENDING_ROLES['ROLE_ADMIN'], User::PENDING_ROLES['ROLE_MANAGER']],
+            $user->getPendingRoles(),
+        );
+        $this->assertNotContains('ROLE_ADMIN', $user->getRoles());
+    }
+
+    /**
+     * An already approved role must not be demoted back to a marker on the next login.
+     */
+    public function testAnAlreadyGrantedAdministrativeRoleIsKept(): void
+    {
+        $user = (new User())->setEmail('[email protected]')->setRoles(['ROLE_ADMIN', 'ROLE_TEAMER']);
+        $this->repository->method('findOneBy')->willReturn($user);
+
+        $this->loadUser($this->createUserinfo(['ROLE_ADMIN', 'ROLE_TEAMER']));
+
+        $this->assertSame(['ROLE_ADMIN', 'ROLE_TEAMER'], $user->getAssignedRoles());
+        $this->assertSame([], $user->getPendingRoles());
+    }
+
+    public function testTeamerRoleIsGrantedToAnExistingUser(): void
+    {
+        $user = (new User())->setEmail('[email protected]')->setRoles([User::PENDING_ROLES['ROLE_MANAGER']]);
+        $this->repository->method('findOneBy')->willReturn($user);
+
+        $this->loadUser($this->createUserinfo(['ROLE_TEAMER', 'ROLE_MANAGER']));
+
+        $this->assertContains('ROLE_TEAMER', $user->getAssignedRoles());
+        $this->assertSame([User::PENDING_ROLES['ROLE_MANAGER']], $user->getPendingRoles());
+    }
+
+    /**
+     * MyE&P must not be able to undo a deletion, in any direction.
+     */
+    public function testDeletedUserIsReturnedWithoutAnySync(): void
+    {
+        $user = (new User())->setEmail('[email protected]')->setRoles(['ROLE_TEAMER']);
+        $user->setHotelCodes(['OLD']);
+        $user->setDeleted();
+
+        $this->repository->method('findOneBy')->willReturn($user);
+        $this->entityManager->expects($this->never())->method('flush');
+
+        $userinfo = $this->createUserinfo(['ROLE_ADMIN', 'ROLE_TEAMER']);
+        $userinfo['profile']['hotel_codes'] = ['NEW'];
+
+        // returned rather than refused, so the UserChecker can explain the deletion
+        $this->assertSame($user, $this->loadUser($userinfo));
+        $this->assertSame(['ROLE_TEAMER'], $user->getAssignedRoles());
+        $this->assertSame([], $user->getPendingRoles());
+        $this->assertSame(['OLD'], $user->getHotelCodes());
+        $this->assertNull($user->getLastLoginAt());
+    }
+
+    public function testNewUserIsCreatedWithBothBusProIds(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $user = $this->loadUser($this->createUserinfo(['ROLE_TEAMER']));
+
+        $this->assertSame(6789, $user->getBusProAddressId());
+        $this->assertSame(12345, $user->getBusProPersonId());
+        $this->assertSame('[email protected]', $user->getEmail());
+        $this->assertSame(['HOTEL'], $user->getHotelCodes());
+        $this->assertNotNull($user->getTeamer());
+    }
+
+    public function testNoAccountIsCreatedWithoutTheBusProIdClaims(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $userinfo = $this->createUserinfo(['ROLE_TEAMER']);
+        unset($userinfo['person_id'], $userinfo['address_id']);
+
+        $this->expectException(CustomUserMessageAuthenticationException::class);
+
+        try {
+            $this->loadUser($userinfo);
+        } finally {
+            $this->assertSame([], $this->persisted);
+        }
+    }
+
+    /**
+     * The account a BusPro login created is matched on the id pair, so the two login
+     * paths cannot end up with two rows for the same person.
+     */
+    public function testExistingUserIsMatchedOnTheBusProIdsRatherThanTheEmail(): void
+    {
+        $user = (new User())->setEmail('[email protected]')->setRoles(['ROLE_TEAMER']);
+
+        $this->repository
+            ->expects($this->once())
+            ->method('findOneBy')
+            ->with(['busProAddressId' => 6789, 'busProPersonId' => 12345])
+            ->willReturn($user)
+        ;
+        $this->repository->expects($this->never())->method('findBy');
+
+        $this->assertSame($user, $this->loadUser($this->createUserinfo(['ROLE_TEAMER'])));
+        $this->assertSame([], $this->persisted);
+    }
+
+    public function testUserMatchedByEmailHasItsBusProIdsBackfilled(): void
+    {
+        $user = (new User())->setEmail('[email protected]')->setRoles(['ROLE_TEAMER']);
+
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([$user]);
+
+        $this->loadUser($this->createUserinfo(['ROLE_TEAMER']));
+
+        $this->assertSame(6789, $user->getBusProAddressId());
+        $this->assertSame(12345, $user->getBusProPersonId());
+    }
+
+    public function testAmbiguousEmailNeverCreatesOrPicksAnAccount(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([(new User())->setEmail('[email protected]'), (new User())->setEmail('[email protected]')]);
+
+        $userinfo = $this->createUserinfo(['ROLE_TEAMER']);
+        unset($userinfo['person_id'], $userinfo['address_id']);
+
+        $this->expectException(CustomUserMessageAuthenticationException::class);
+
+        try {
+            $this->loadUser($userinfo);
+        } finally {
+            $this->assertSame([], $this->persisted);
+        }
+    }
+
+    /**
+     * A partial payload must refuse the login at worst, never fatal.
+     */
+    public function testPartialProfileDoesNotFail(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $userinfo = $this->createUserinfo(['ROLE_TEAMER']);
+        unset($userinfo['profile']);
+
+        $user = $this->loadUser($userinfo);
+
+        $this->assertSame(['ROLE_USER', 'ROLE_TEAMER'], $user->getRoles());
+        $this->assertSame([], $user->getHotelCodes());
+    }
+
+    public function testDateOfBirthIsReadFromTheProfile(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $userinfo = $this->createUserinfo(['ROLE_TEAMER']);
+        $userinfo['profile']['date_of_birth'] = '1980-04-01';
+
+        $user = $this->loadUser($userinfo);
+
+        $this->assertSame('1980-04-01', $user->getTeamer()?->getDateOfBirth()?->format('Y-m-d'));
+    }
+
+    public function testUnparsableDateOfBirthIsIgnoredRatherThanFatal(): void
+    {
+        $this->repository->method('findOneBy')->willReturn(null);
+        $this->repository->method('findBy')->willReturn([]);
+
+        $userinfo = $this->createUserinfo(['ROLE_TEAMER']);
+        $userinfo['profile']['date_of_birth'] = 'not a date';
+
+        $user = $this->loadUser($userinfo);
+
+        $this->assertNull($user->getTeamer()?->getDateOfBirth());
+    }
+
+    public function testAuthenticatorDoesNotSupportTheRouteWhileTheFeatureIsOff(): void
+    {
+        $authenticator = $this->createAuthenticator([], featureActive: false);
+
+        $request = new Request();
+        $request->attributes->set('_route', 'app_myep_auth_check');
+
+        $this->assertFalse($authenticator->supports($request));
+    }
+
+    public function testAuthenticatorSupportsTheRouteWhileTheFeatureIsOn(): void
+    {
+        $authenticator = $this->createAuthenticator([]);
+
+        $request = new Request();
+        $request->attributes->set('_route', 'app_myep_auth_check');
+
+        $this->assertTrue($authenticator->supports($request));
+    }
+
+    /**
+     * @param string[] $roles
+     */
+    private function createUserinfo(array $roles): array
+    {
+        return [
+            'email' => '[email protected]',
+            'person_id' => 12345,
+            'address_id' => 6789,
+            'roles' => $roles,
+            'profile' => [
+                'first_name' => 'Erika',
+                'last_name' => 'Musterfrau',
+                'hotel_codes' => ['HOTEL'],
+                'communication' => [
+                    'email' => '[email protected]',
+                ],
+            ],
+        ];
+    }
+
+    private function loadUser(array $userinfo): User
+    {
+        $authenticator = $this->createAuthenticator($userinfo);
+
+        $request = new Request(query: ['code' => 'code', 'state' => 'state']);
+        $request->setSession(new Session(new MockArraySessionStorage()));
+
+        $passport = $authenticator->authenticate($request);
+
+        /** @var UserBadge $badge */
+        $badge = $passport->getBadge(UserBadge::class);
+
+        /** @var User $user */
+        $user = $badge->getUser();
+
+        return $user;
+    }
+
+    private function createAuthenticator(array $userinfo, bool $featureActive = true): MyEpAuthenticator
+    {
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient
+            ->method('sendRequest')
+            ->willReturn(new Psr7Response(200, [], json_encode($userinfo)))
+        ;
+
+        $provider = $this->createMock(AbstractProvider::class);
+        $provider->method('getResourceOwnerDetailsUrl')->willReturn('https://my.example.com/api/userinfo');
+        $provider
+            ->method('getAuthenticatedRequest')
+            ->willReturn(new Psr7Request('GET', 'https://my.example.com/api/userinfo'))
+        ;
+        $provider->method('getHttpClient')->willReturn($httpClient);
+
+        $client = $this->createMock(MyEpClient::class);
+        $client->method('fetchAccessToken')->willReturn(new AccessToken(['access_token' => 'token']));
+        $client->method('getProvider')->willReturn($provider);
+
+        $featureManager = $this->createMock(FeatureManagerInterface::class);
+        $featureManager->method('isActive')->willReturn($featureActive);
+
+        return new MyEpAuthenticator(
+            $client,
+            new UserDataHandler($this->entityManager, $this->createMock(LoggerInterface::class)),
+            $this->entityManager,
+            $this->createMock(UrlGeneratorInterface::class),
+            $this->createMock(LoggerInterface::class),
+            $featureManager,
+            $this->createMock(RequiredTeamerCheckRegistry::class),
+        );
+    }
+}