From 450f5b05156f48d9812ef1548e945ab83c062004 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= <fromme@dreipunktnull.com>
Date: Wed, 12 Aug 2026 17:54:18 +0200
Subject: [PATCH] feat: share the role policy across identity sources

---
 src/BusProNet/UserDataHandler.php       | 53 ++++++++++++++++++++++---
 tests/BusProNet/UserDataHandlerTest.php | 53 +++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/src/BusProNet/UserDataHandler.php b/src/BusProNet/UserDataHandler.php
index fdbe706..25480c9 100644
--- a/src/BusProNet/UserDataHandler.php
+++ b/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();
diff --git a/tests/BusProNet/UserDataHandlerTest.php b/tests/BusProNet/UserDataHandlerTest.php
index 08ea8ea..e0e8aa9 100644
--- a/tests/BusProNet/UserDataHandlerTest.php
+++ b/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())