feat: derive ROLE_EMPLOYEE from the account's email domain

config/services.yaml

@@ -31,6 +31,12 @@ parameters:
         10321389: 'Reisen-Alert Stubaital'
         10554990: 'Reisen-Alert Ski & Boarderweek'
 
+    # Email domains whose accounts are members of staff. BusPro has no CRM selection expressing
+    # "works here", so ROLE_EMPLOYEE is derived from the address instead. Matched exactly on the
+    # domain part, never as a suffix.
+    employee_email_domains:
+        - 'ep-reisen.de'
+
     # BusProNet "Hausleitung {CODE}" CRM selections, by selection id.
     # DEPLOYMENT-CRITICAL: roles and hotel codes are synced on every login, so an id missing
     # here does not merely fail to nominate a Hausleitung — it revokes the role and the hotel
@@ -299,6 +305,10 @@ services:
         arguments:
             $houseManagerIds: '%bpn_crm_house_manager_ids%'
 
+    App\Security\EmployeeDomainMatcher:
+        arguments:
+            $domains: '%employee_email_domains%'
+
     App\Service\DomainConfigProvider:
         arguments:
             $domainConfig: '%domain_config%'

src/Security/BpnAuthenticator.php

@@ -34,8 +34,10 @@ use Symfony\Component\Security\Http\Util\TargetPathTrait;
  *
  * The password is kept, RSA-encrypted, because every later BPN call needs it again.
  *
- * BusPro owns the whole role set and the hotel codes: both are synced on every login, in both
+ * BusPro owns the whole role set bar one, and the hotel codes: both are synced on every login, in
- * directions, so anything the CRM no longer reports is withdrawn here. What the CRM claims is
+ * both directions, so anything the CRM no longer reports is withdrawn here. The exception is
+ * ROLE_EMPLOYEE, which BusPro has no selection for and which is derived from the account's email
+ * domain — passed to Role::sync() as a claim, so it is granted and revoked by the same machinery. What the CRM claims is
  * not automatically granted, though — Role::sync() turns an administrative claim into a
  * nomination that an administrator has to approve in /admin/user, because BusPro backend users
  * can edit their own CRM selections and would otherwise make themselves administrators.
@@ -51,6 +53,7 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent
         private readonly Crypt $crypt,
         private readonly ProfileCompletenessChecker $completenessChecker,
         private readonly LoggerInterface $authLogger,
+        private readonly EmployeeDomainMatcher $employeeDomainMatcher,
     ) {
     }
 
@@ -152,8 +155,17 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent
             }
         }
 
+        $claimedRoles = $crmAttributes->roles;
+
+        // Not a CRM claim: BusPro has no selection for it, so the account's own address decides.
+        // Passing it in as a claim rather than setting it afterwards is what makes it revocable —
+        // Role::sync() strips every stored role the claim set does not contain.
+        if ($this->employeeDomainMatcher->isEmployee($user->getEmail())) {
+            $claimedRoles[] = Role::EMPLOYEE;
+        }
+
         $user
-            ->setRoles(Role::sync($previousRoles, $crmAttributes->roles))
+            ->setRoles(Role::sync($previousRoles, $claimedRoles))
             ->setHotelCodes(array_values(array_unique($crmAttributes->hotelCodes)))
         ;
 

src/Security/EmployeeDomainMatcher.php

@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Security;
+
+/**
+ * Decides whether an account belongs to a member of staff, by its email domain.
+ *
+ * BusPro has no CRM selection expressing "works here", so the email address is the only signal
+ * available. The domains are configuration (%employee_email_domains%) rather than a constant
+ * because they are deployment-specific, in the same way the brand hosts and the CRM selection ids
+ * are.
+ *
+ * Matching is exact on the domain part and never on a suffix: "mail.ep-reisen.de" and
+ * "notep-reisen.de" are not "ep-reisen.de". A suffix match here would hand ROLE_EMPLOYEE to
+ * anybody able to register a domain ending in the configured one.
+ */
+final class EmployeeDomainMatcher
+{
+    /**
+     * @var string[]
+     */
+    private readonly array $domains;
+
+    /**
+     * @param string[] $domains
+     */
+    public function __construct(array $domains)
+    {
+        $this->domains = array_values(array_filter(array_map(
+            static fn (string $domain): string => strtolower(trim($domain, " \t\n\r\0\x0B.@")),
+            $domains,
+        )));
+    }
+
+    /**
+     * A malformed or missing identifier is simply not an employee. It must not throw: this runs
+     * inside the authentication path, where an exception would turn a bad address into a failed
+     * login rather than a login without the role.
+     */
+    public function isEmployee(?string $email): bool
+    {
+        if (null === $email || [] === $this->domains) {
+            return false;
+        }
+
+        $position = strrpos($email, '@');
+
+        if (false === $position) {
+            return false;
+        }
+
+        $domain = strtolower(substr($email, $position + 1));
+
+        return \in_array($domain, $this->domains, true);
+    }
+}

src/Security/Role.php

@@ -16,6 +16,13 @@ namespace App\Security;
  * nothing until an administrator approves it in /admin/user. The CRM's word alone is enough to
  * take a role away, never to hand it out, and an administrator's word alone is enough for
  * neither.
+ *
+ * One role is not claimed by the CRM at all: ROLE_EMPLOYEE is derived from the account's own
+ * email domain, because BusPro has no selection expressing "works here". It is passed to sync()
+ * as a claim alongside the CRM's, so it is granted and revoked by exactly the same machinery.
+ * That widens a claim from "what the CRM reports" to "what the CRM reports plus what the account
+ * itself implies", and nothing more: ROLE_EMPLOYEE is not administrative, so it cannot reach the
+ * nomination path, and the CRM remains the only source for every role that grants privileges.
  */
 final class Role
 {
@@ -32,6 +39,7 @@ final class Role
     public const HOUSE_MANAGER = 'ROLE_HOUSE_MANAGER';
     public const GROUPS_ADMIN = 'ROLE_GROUPS_ADMIN';
     public const GROUPS_MANAGER = 'ROLE_GROUPS_MANAGER';
+    public const EMPLOYEE = 'ROLE_EMPLOYEE';
 
     /**
      * Appended to an administrative role to mark it as claimed by the CRM but not yet approved.
@@ -55,17 +63,21 @@ final class Role
         self::HOUSE_MANAGER,
         self::GROUPS_ADMIN,
         self::GROUPS_MANAGER,
+        self::EMPLOYEE,
     ];
 
     /**
-     * Roles the CRM grants outright. ROLE_CUSTOMER is never claimed by BusPro — it is the
+     * Roles granted outright, without an approval step. ROLE_CUSTOMER is never claimed by BusPro —
-     * fallback for an account left without any effective role, and exclusive with the others.
+     * it is the fallback for an account left without any effective role, and exclusive with the
+     * others. ROLE_EMPLOYEE is not claimed by BusPro either: it is derived from the account's own
+     * email domain (see EmployeeDomainMatcher) and, being effective, displaces that fallback.
      *
      * @var string[]
      */
     public const UNCONDITIONAL = [
         self::TEAMER,
         self::CUSTOMER,
+        self::EMPLOYEE,
     ];
 
     /**
@@ -180,7 +192,8 @@ final class Role
      * Nothing here can raise a privilege: step 3 only ever produces markers.
      *
      * @param string[] $storedRoles
-     * @param string[] $claimedRoles what the CRM reports
+     * @param string[] $claimedRoles what the CRM reports, plus the roles derived from the account
+     *                               itself (ROLE_EMPLOYEE); anything outside self::ALL is ignored
      *
      * @return string[]
      */
@@ -274,6 +287,7 @@ final class Role
             self::HOUSE_MANAGER => 'Hausleitung',
             self::GROUPS_ADMIN => 'Preisrechner Admin',
             self::GROUPS_MANAGER => 'Preisrechner',
+            self::EMPLOYEE => 'Mitarbeiter:in',
         ];
 
         foreach (self::ADMINISTRATIVE as $role) {

tests/Security/BpnAuthenticatorTest.php

@@ -11,6 +11,7 @@ use App\BusProNet\Model\PersonalData;
 use App\Entity\User;
 use App\Security\BpnAuthenticator;
 use App\Security\Crypt;
+use App\Security\EmployeeDomainMatcher;
 use App\Security\Role;
 use App\Service\ProfileCompletenessChecker;
 use Doctrine\ORM\EntityManagerInterface;
@@ -196,6 +197,38 @@ class BpnAuthenticatorTest extends TestCase
         return $attributes;
     }
 
+    public function testStaffEmailDomainGrantsTheEmployeeRole(): void
+    {
+        $persisted = null;
+        $authenticator = $this->authenticator($this->crmAttributes([], []), null, $persisted);
+
+        $user = $this->loadUser($authenticator, '[email protected]');
+
+        // Effective, so it displaces the customer fallback the same account would get otherwise.
+        self::assertSame(['ROLE_USER', Role::EMPLOYEE], $user->getRoles());
+    }
+
+    public function testAnotherEmailDomainStillFallsBackToCustomer(): void
+    {
+        $persisted = null;
+        $authenticator = $this->authenticator($this->crmAttributes([], []), null, $persisted);
+
+        $user = $this->loadUser($authenticator, '[email protected]');
+
+        self::assertSame(['ROLE_USER', Role::CUSTOMER], $user->getRoles());
+    }
+
+    public function testEmployeeRoleIsWithdrawnWhenTheAddressIsNoLongerStaff(): void
+    {
+        $existing = (new User('[email protected]'))->setRoles([Role::EMPLOYEE]);
+        $persisted = null;
+        $authenticator = $this->authenticator($this->crmAttributes([], []), $existing, $persisted);
+
+        $user = $this->loadUser($authenticator, '[email protected]');
+
+        self::assertSame(['ROLE_USER', Role::CUSTOMER], $user->getRoles());
+    }
+
     private function authenticator(
         CrmAttributes $crmAttributes,
         ?User $existing,
@@ -238,13 +271,14 @@ class BpnAuthenticatorTest extends TestCase
             $crypt,
             $completenessChecker,
             $this->createStub(LoggerInterface::class),
+            new EmployeeDomainMatcher(['ep-reisen.de']),
         );
     }
 
-    private function loadUser(BpnAuthenticator $authenticator): User
+    private function loadUser(BpnAuthenticator $authenticator, string $email = '[email protected]'): User
     {
         $request = new Request();
-        $request->request->set('_username', '[email protected]');
+        $request->request->set('_username', $email);
         $request->request->set('_password', 'secret');
 
         $badge = $authenticator->authenticate($request)->getBadge(UserBadge::class);

tests/Security/EmployeeDomainMatcherTest.php

@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Security;
+
+use App\Security\EmployeeDomainMatcher;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * The domain is matched exactly, never as a suffix: a suffix match would hand ROLE_EMPLOYEE to
+ * anybody able to register a domain ending in the configured one.
+ */
+class EmployeeDomainMatcherTest extends TestCase
+{
+    #[DataProvider('addresses')]
+    public function testRecognisesAnEmployeeAddress(?string $email, bool $expected): void
+    {
+        $matcher = new EmployeeDomainMatcher(['ep-reisen.de']);
+
+        self::assertSame($expected, $matcher->isEmployee($email));
+    }
+
+    /**
+     * @return iterable<string, array{0: ?string, 1: bool}>
+     */
+    public static function addresses(): iterable
+    {
+        yield 'the configured domain' => ['[email protected]', true];
+        yield 'mixed case is still the same domain' => ['[email protected]', true];
+        yield 'plus addressing does not touch the domain' => ['[email protected]', true];
+        yield 'an at sign in the local part' => ['"odd@name"@ep-reisen.de', true];
+
+        yield 'another domain' => ['[email protected]', false];
+        yield 'a subdomain is not the domain' => ['[email protected]', false];
+        yield 'a domain merely ending in it' => ['[email protected]', false];
+        yield 'the domain as a prefix' => ['[email protected]', false];
+        yield 'no at sign at all' => ['ep-reisen.de', false];
+        yield 'empty' => ['', false];
+        yield 'null' => [null, false];
+    }
+
+    public function testWithoutConfiguredDomainsNobodyIsAnEmployee(): void
+    {
+        self::assertFalse((new EmployeeDomainMatcher([]))->isEmployee('[email protected]'));
+    }
+
+    public function testConfiguredDomainsAreNormalised(): void
+    {
+        $matcher = new EmployeeDomainMatcher([' EP-Reisen.DE ', '@example.org']);
+
+        self::assertTrue($matcher->isEmployee('[email protected]'));
+        self::assertTrue($matcher->isEmployee('[email protected]'));
+    }
+}

tests/Security/RoleTest.php

@@ -93,6 +93,35 @@ class RoleTest extends TestCase
         self::assertSame([Role::ADMIN => 'Administration'], Role::nominatedFrom($roles));
     }
 
+    public function testEmployeeIsGrantedOutrightAndDisplacesTheCustomerFallback(): void
+    {
+        // The claim does not come from the CRM, but it travels the same path as one.
+        self::assertSame([Role::EMPLOYEE], Role::sync([], [Role::EMPLOYEE]));
+    }
+
+    public function testEmployeeIsRevokedOnceItIsNoLongerClaimed(): void
+    {
+        // Somebody whose address left the staff domain: no claim, so the role goes, and with no
+        // effective role left the fallback returns.
+        self::assertSame([Role::CUSTOMER], Role::sync([Role::EMPLOYEE], []));
+    }
+
+    public function testEmployeeDoesNotShortCircuitTheNominationOfAnAdministrativeRole(): void
+    {
+        self::assertSame(
+            [Role::EMPLOYEE, Role::pending(Role::ADMIN)],
+            Role::sync([], [Role::EMPLOYEE, Role::ADMIN]),
+        );
+    }
+
+    public function testEmployeeAndTeamerCoexist(): void
+    {
+        self::assertSame(
+            [Role::TEAMER, Role::EMPLOYEE],
+            Role::sync([], [Role::TEAMER, Role::EMPLOYEE]),
+        );
+    }
+
     public function testEveryRoleAndNominationHasALabel(): void
     {
         $labels = Role::labels();