EP/myep-team
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: rename ROLE_ADMIN to ROLE_TEAM_ADMIN since being too broad

Browse Source
This commit is contained in:
fromme
2026-09-16 17:20:45 +02:00
parent 39a343911a
commit d4908b6ab9
75 changed files with 212 additions and 180 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/user-roles.md
+44 -12
View File
@@ -30,17 +30,35 @@ assign:
| Role | Label | Granted by | Revoked by | Hierarchy |
|------|-------|-----------|------------|-----------|
| `ROLE_ADMIN` | Admin | super admin, approving a CRM claim | the CRM, automatically | ⇒ `ROLE_ADMINISTRATIVE` |
| `ROLE_TEAM_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 | — |
### A note on the `TEAM_` prefix
`ROLE_TEAM_ADMIN`, `ROLE_TEAM_ADMIN_PENDING` and `ROLE_TEAM_SUPER_ADMIN` were renamed from
`ROLE_ADMIN`, `ROLE_ADMIN_PENDING` and `ROLE_SUPER_ADMIN`. The MyE&P identity provider is
shared with a sibling portal that uses `ROLE_ADMIN` for a different privilege, so the plain
name was ambiguous across the estate; the prefix makes it unambiguously *this* application's
admin. The German label is unchanged — it still reads "Admin" everywhere in the UI.
Two consequences worth remembering:
- **`ROLE_ADMINISTRATIVE` is a different role and was not renamed.** It is granted only by
the hierarchy, never stored, and it shares the old `ROLE_ADMIN` prefix — so any
search-and-replace over role names must match on a word boundary
(`ROLE_TEAM_ADMIN(?![A-Z_])`) or it will corrupt ~100 call sites silently.
- **`ELIGIBLE_ROLES` in `MyEpAuthenticator` is a wire contract**, not an internal name: it is
compared directly against the IdP's `roles` claim. It only works while MyE&P emits
`ROLE_TEAM_ADMIN`, so the two sides have to move together.
`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_TEAM_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 |
@@ -50,10 +68,10 @@ effects are cosmetic (rendered as "Admin (nicht freigeschaltet)") and organisati
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
everybody, and `ROLE_TEAM_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_TEAM_ADMIN` alongside it — super admin is an elevation, never a standalone grant. The sync
enforces the same rule from the other side: revoking `ROLE_TEAM_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
@@ -63,7 +81,7 @@ slice it, and picking the right one matters:
| Accessor | Returns |
|----------|---------|
| `getRoles()` | the column **plus** synthesized `ROLE_USER` / `ROLE_SUPER_ADMIN` — what Symfony authorises against |
| `getRoles()` | the column **plus** synthesized `ROLE_USER` / `ROLE_TEAM_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 |
@@ -77,7 +95,7 @@ slice it, and picking the right one matters:
| CRM attribute | Recognised by | Sets |
|---------------|---------------|------|
| admin | attribute id `%bpn_crm_id_admin%`, selected | `isAdmin` |
| admin | attribute id `%bpn_crm_id_team_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 |
@@ -111,12 +129,26 @@ roles were revoked.
>
> | Parameter | Attribute |
> |-----------|-----------|
> | `APP_BPN_CRM_ID_ADMIN` | `Admin` |
> | `APP_BPN_CRM_ID_TEAM_ADMIN` | the team-admin selection — **not** the old portal-wide `Admin` (1292) |
> | `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_CRM_ID_TEAM_ADMIN` was renamed from `APP_BPN_CRM_ID_ADMIN` with the
> `ROLE_TEAM_ADMIN` rename, and its **value has to change too**. The old value 1292 is the
> portal-wide admin selection, which still means `ROLE_ADMIN` in the sibling portal — keeping
> it would have left this app granting its admin off the very selection the rename was meant
> to stop sharing. It ships as `0` — a valid int that matches no attribute, so the container
> boots but nobody is granted the role — and must be set to the new selection's id.
>
> **This is a hard cutover.** `revokeUnclaimedRoles()` withdraws any granted role the CRM no
> longer claims, and `revokeSuperAdminWithoutRoleAdmin()` takes the super admin flag down with
> `ROLE_TEAM_ADMIN`. So the new selection must exist **and already be assigned to every admin**
> in BusPro before this is deployed; otherwise each of them is demoted on their next login and
> needs a super admin to re-approve. Admins who also hold `ROLE_TEAMER` degrade to teamer
> access; an admin without it is blocked outright by `disableForRevokedCrmRoles()`.
>
> `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
@@ -137,7 +169,7 @@ roles were revoked.
`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
A CRM admin who is not also a teamer therefore starts with `['ROLE_TEAM_ADMIN_PENDING']` and no
privileges at all: they can authenticate, but `UserChecker` refuses the session until a
super admin approves them.
@@ -148,7 +180,7 @@ 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
2. **clear the super admin flag** when `ROLE_TEAM_ADMIN` was among them — `ROLE_TEAM_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.
@@ -189,7 +221,7 @@ runs again on submit to catch a sync that revoked the claim while the dialog was
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,
**Super admin** is only offered to somebody who already holds `ROLE_TEAM_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.
@@ -243,7 +275,7 @@ 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 |
| `/admin/teamer/disable-user/{uuid}` and `/administrative/teamer/enable-user/{uuid}` | `ROLE_TEAM_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