feat: align role assignment logic with myep-team
This commit is contained in:
Björn Fromme
@@ -6,11 +6,11 @@ namespace App\Tests\Security;
|
||||
|
||||
use App\BusProNet\ApiClient;
|
||||
use App\BusProNet\Model\CrmAttributes;
|
||||
use App\BusProNet\Model\CrmSelectionGroup;
|
||||
use App\BusProNet\Model\PersonalData;
|
||||
use App\Entity\User;
|
||||
use App\Security\BpnAuthenticator;
|
||||
use App\Security\Crypt;
|
||||
use App\Security\DefaultRouteResolver;
|
||||
use App\Security\Role;
|
||||
use App\Service\ProfileCompletenessChecker;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
@@ -22,16 +22,16 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
|
||||
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
|
||||
|
||||
/**
|
||||
* Covers who may grant roles: BusPro backend users can edit their own CRM selections, so the
|
||||
* import must not be a channel for privilege escalation.
|
||||
* Covers what a login does to an account: BusPro owns the roles and the hotel codes, but a
|
||||
* CRM claim must never grant an administrative role on its own.
|
||||
*/
|
||||
class BpnAuthenticatorTest extends TestCase
|
||||
{
|
||||
public function testNewAccountIsSeededWithTheImportableRolesOnly(): void
|
||||
public function testNewAccountIsSeededFromTheCrm(): void
|
||||
{
|
||||
$persisted = null;
|
||||
$authenticator = $this->authenticator(
|
||||
$this->crmAttributes([Role::ADMIN, Role::TEAMER, Role::GROUPS_ADMIN], ['SSL', 'SSL']),
|
||||
$this->crmAttributes([Role::ADMIN, Role::TEAMER], ['SSL', 'SSL']),
|
||||
null,
|
||||
$persisted,
|
||||
);
|
||||
@@ -39,20 +39,17 @@ class BpnAuthenticatorTest extends TestCase
|
||||
$user = $this->loadUser($authenticator);
|
||||
|
||||
self::assertSame($persisted, $user);
|
||||
self::assertSame(['ROLE_USER', Role::TEAMER], $user->getRoles());
|
||||
self::assertSame(['ROLE_USER', Role::TEAMER, Role::pending(Role::ADMIN)], $user->getRoles());
|
||||
self::assertSame(['SSL'], $user->getHotelCodes());
|
||||
}
|
||||
|
||||
public function testExistingAccountKeepsThePrivilegedRolesAnAdministratorAssigned(): void
|
||||
public function testAdministrativeClaimIsOnlyANominationUntilItIsApproved(): void
|
||||
{
|
||||
$existing = (new User('[email protected]'))
|
||||
->setRoles([Role::TEAMER, Role::GROUPS_MANAGER])
|
||||
->setHotelCodes(['DKS'])
|
||||
;
|
||||
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER]);
|
||||
|
||||
$persisted = null;
|
||||
$authenticator = $this->authenticator(
|
||||
$this->crmAttributes([Role::ADMIN, Role::CUSTOMER], ['SSL']),
|
||||
$this->crmAttributes([Role::TEAMER, Role::GROUPS_ADMIN], []),
|
||||
$existing,
|
||||
$persisted,
|
||||
);
|
||||
@@ -60,13 +57,42 @@ class BpnAuthenticatorTest extends TestCase
|
||||
$user = $this->loadUser($authenticator);
|
||||
|
||||
self::assertNull($persisted, 'an existing account must not be persisted again');
|
||||
// ROLE_TEAMER is gone with its CRM selection, ROLE_ADMIN is still not honoured, and the
|
||||
// administrator-granted ROLE_GROUPS_MANAGER survives.
|
||||
self::assertSame(['ROLE_USER', Role::CUSTOMER, Role::GROUPS_MANAGER], $user->getRoles());
|
||||
self::assertSame(['DKS'], $user->getHotelCodes(), 'hotel codes stay administrator-managed');
|
||||
self::assertSame(
|
||||
['ROLE_USER', Role::TEAMER, Role::pending(Role::GROUPS_ADMIN)],
|
||||
$user->getRoles(),
|
||||
);
|
||||
self::assertNotNull($user->getLastLoginAt(), 'the rest of the profile is still synced');
|
||||
}
|
||||
|
||||
public function testApprovedRoleSurvivesTheNextLogin(): void
|
||||
{
|
||||
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER, Role::GROUPS_MANAGER]);
|
||||
|
||||
$persisted = null;
|
||||
$authenticator = $this->authenticator(
|
||||
$this->crmAttributes([Role::TEAMER, Role::GROUPS_MANAGER], []),
|
||||
$existing,
|
||||
$persisted,
|
||||
);
|
||||
|
||||
self::assertSame(
|
||||
['ROLE_USER', Role::TEAMER, Role::GROUPS_MANAGER],
|
||||
$this->loadUser($authenticator)->getRoles(),
|
||||
);
|
||||
}
|
||||
|
||||
public function testRoleRevokedInBusProIsWithdrawnOnLogin(): void
|
||||
{
|
||||
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER, Role::GROUPS_MANAGER]);
|
||||
|
||||
$persisted = null;
|
||||
$authenticator = $this->authenticator($this->crmAttributes([], []), $existing, $persisted);
|
||||
|
||||
// Nothing is claimed any more, so nothing is held — and an account without an effective
|
||||
// role is a customer.
|
||||
self::assertSame(['ROLE_USER', Role::CUSTOMER], $this->loadUser($authenticator)->getRoles());
|
||||
}
|
||||
|
||||
public function testRoleGainedInBusProIsGrantedOnLogin(): void
|
||||
{
|
||||
$existing = (new User('[email protected]'))->setRoles([Role::CUSTOMER]);
|
||||
@@ -82,29 +108,69 @@ class BpnAuthenticatorTest extends TestCase
|
||||
self::assertSame(['ROLE_USER', Role::TEAMER], $this->loadUser($authenticator)->getRoles());
|
||||
}
|
||||
|
||||
public function testAccountWithoutAnyRoleIsHealedOnLogin(): void
|
||||
public function testHotelCodesAreResyncedOnEveryLogin(): void
|
||||
{
|
||||
$existing = new User('teamer@example.org');
|
||||
$existing = (new User('house@example.org'))
|
||||
->setRoles([Role::HOUSE_MANAGER])
|
||||
->setHotelCodes(['DKS', 'SSL'])
|
||||
;
|
||||
|
||||
$persisted = null;
|
||||
$authenticator = $this->authenticator(
|
||||
$this->crmAttributes([Role::TEAMER], []),
|
||||
$this->crmAttributes([Role::HOUSE_MANAGER], ['DKS']),
|
||||
$existing,
|
||||
$persisted,
|
||||
);
|
||||
|
||||
self::assertSame(['ROLE_USER', Role::TEAMER], $this->loadUser($authenticator)->getRoles());
|
||||
self::assertSame(['DKS'], $this->loadUser($authenticator)->getHotelCodes());
|
||||
}
|
||||
|
||||
public function testDegradedCrmResponseLeavesAnExistingAccountUntouched(): void
|
||||
{
|
||||
$existing = (new User('[email protected]'))
|
||||
->setRoles([Role::TEAMER, Role::GROUPS_MANAGER])
|
||||
->setHotelCodes(['DKS'])
|
||||
;
|
||||
|
||||
$persisted = null;
|
||||
// No selection groups at all: BusPro always answers with the full attribute tree, so
|
||||
// this is a degraded payload and not a revocation of everything.
|
||||
$authenticator = $this->authenticator(
|
||||
$this->crmAttributes([], [], selectionGroups: []),
|
||||
$existing,
|
||||
$persisted,
|
||||
);
|
||||
|
||||
$user = $this->loadUser($authenticator);
|
||||
|
||||
self::assertSame(['ROLE_USER', Role::TEAMER, Role::GROUPS_MANAGER], $user->getRoles());
|
||||
self::assertSame(['DKS'], $user->getHotelCodes());
|
||||
}
|
||||
|
||||
public function testDegradedCrmResponseStillGivesANewAccountTheFallbackRole(): void
|
||||
{
|
||||
$persisted = null;
|
||||
$authenticator = $this->authenticator(
|
||||
$this->crmAttributes([], [], selectionGroups: []),
|
||||
null,
|
||||
$persisted,
|
||||
);
|
||||
|
||||
self::assertSame(['ROLE_USER', Role::CUSTOMER], $this->loadUser($authenticator)->getRoles());
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string[] $roles
|
||||
* @param string[] $hotelCodes
|
||||
* @param string[] $roles
|
||||
* @param string[] $hotelCodes
|
||||
* @param CrmSelectionGroup[] $selectionGroups only their presence matters here — an empty
|
||||
* set is what marks a response as degraded
|
||||
*/
|
||||
private function crmAttributes(array $roles, array $hotelCodes): CrmAttributes
|
||||
private function crmAttributes(array $roles, array $hotelCodes, ?array $selectionGroups = null): CrmAttributes
|
||||
{
|
||||
$attributes = new CrmAttributes();
|
||||
$attributes->roles = $roles;
|
||||
$attributes->hotelCodes = $hotelCodes;
|
||||
$attributes->selectionGroups = $selectionGroups ?? [new CrmSelectionGroup()];
|
||||
|
||||
return $attributes;
|
||||
}
|
||||
@@ -144,7 +210,6 @@ class BpnAuthenticatorTest extends TestCase
|
||||
$crypt,
|
||||
$completenessChecker,
|
||||
$this->createMock(LoggerInterface::class),
|
||||
$this->createMock(DefaultRouteResolver::class),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+73
-33
@@ -7,62 +7,102 @@ namespace App\Tests\Security;
|
||||
use App\Security\Role;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
/**
|
||||
* Covers the role policy: BusPro backend users can edit their own CRM selections, so a claim
|
||||
* must never grant an administrative role on its own.
|
||||
*/
|
||||
class RoleTest extends TestCase
|
||||
{
|
||||
public function testPrivilegedRolesAreNeverImported(): void
|
||||
public function testAdministrativeClaimOnlyProducesANomination(): void
|
||||
{
|
||||
$roles = Role::filterImportable([
|
||||
Role::TEAMER,
|
||||
Role::ADMIN,
|
||||
Role::GROUPS_ADMIN,
|
||||
Role::GROUPS_MANAGER,
|
||||
Role::HOUSE_MANAGER,
|
||||
]);
|
||||
$roles = Role::sync([], [Role::ADMIN, Role::GROUPS_ADMIN, Role::TEAMER]);
|
||||
|
||||
self::assertSame([Role::TEAMER, Role::HOUSE_MANAGER], $roles);
|
||||
self::assertSame(
|
||||
[Role::TEAMER, Role::pending(Role::ADMIN), Role::pending(Role::GROUPS_ADMIN)],
|
||||
$roles,
|
||||
);
|
||||
self::assertSame([Role::TEAMER], Role::effectiveOnly($roles));
|
||||
}
|
||||
|
||||
public function testResultIsADedupedList(): void
|
||||
public function testApprovedRoleSurvivesTheNextSyncAndIsNotMarkedAgain(): void
|
||||
{
|
||||
// CrmAttributesResponseParser applies array_unique(), which preserves keys — a
|
||||
// non-list would be persisted as a JSON object instead of an array.
|
||||
$roles = Role::filterImportable([0 => Role::ADMIN, 2 => Role::TEAMER, 5 => Role::TEAMER]);
|
||||
$roles = Role::sync([Role::TEAMER, Role::GROUPS_ADMIN], [Role::GROUPS_ADMIN, Role::TEAMER]);
|
||||
|
||||
self::assertSame([Role::TEAMER, Role::GROUPS_ADMIN], $roles);
|
||||
}
|
||||
|
||||
public function testRoleTheCrmNoLongerClaimsIsRevoked(): void
|
||||
{
|
||||
// Both halves go: BusPro is the source of truth for the granted role as much as for
|
||||
// the nomination.
|
||||
$roles = Role::sync([Role::TEAMER, Role::ADMIN, Role::pending(Role::MANAGER)], [Role::TEAMER]);
|
||||
|
||||
self::assertSame([Role::TEAMER], $roles);
|
||||
self::assertSame(array_keys($roles), range(0, \count($roles) - 1));
|
||||
}
|
||||
|
||||
public function testAccountWithOnlyPrivilegedRolesFallsBackToCustomer(): void
|
||||
public function testRevokedRoleIsNotImmediatelyNominatedAgain(): void
|
||||
{
|
||||
self::assertSame([Role::CUSTOMER], Role::filterImportable([Role::ADMIN]));
|
||||
self::assertSame([Role::CUSTOMER], Role::filterImportable([]));
|
||||
self::assertSame([Role::CUSTOMER], Role::sync([Role::ADMIN], []));
|
||||
}
|
||||
|
||||
public function testAssignedOnlyDropsTheImplicitRoleUser(): void
|
||||
public function testAccountWithoutAnEffectiveRoleFallsBackToCustomer(): void
|
||||
{
|
||||
$roles = Role::assignedOnly([Role::USER, Role::TEAMER, Role::GROUPS_ADMIN]);
|
||||
|
||||
self::assertSame([Role::TEAMER, Role::GROUPS_ADMIN], $roles);
|
||||
// The value is JSON-encoded into the userinfo response and must not become an object.
|
||||
self::assertSame(array_keys($roles), range(0, \count($roles) - 1));
|
||||
// The nomination stays visible — it is what an approver acts on — but grants nothing,
|
||||
// so the account is a customer in the meantime.
|
||||
self::assertSame(
|
||||
[Role::pending(Role::ADMIN), Role::CUSTOMER],
|
||||
Role::sync([], [Role::ADMIN]),
|
||||
);
|
||||
self::assertSame([Role::CUSTOMER], Role::sync([], []));
|
||||
}
|
||||
|
||||
public function testCombineKeepsEachHalfInItsOwnLane(): void
|
||||
public function testCustomerIsAFallbackAndNotABaseline(): void
|
||||
{
|
||||
$roles = Role::combine([Role::TEAMER, Role::ADMIN], [Role::GROUPS_ADMIN, Role::TEAMER]);
|
||||
|
||||
// The ADMIN from the synced half and the TEAMER from the privileged half are discarded.
|
||||
self::assertSame([Role::TEAMER, Role::GROUPS_ADMIN], $roles);
|
||||
self::assertSame(array_keys($roles), range(0, \count($roles) - 1));
|
||||
self::assertSame([Role::TEAMER], Role::sync([Role::CUSTOMER], [Role::TEAMER]));
|
||||
}
|
||||
|
||||
public function testCombineDropsTheImplicitRoleUser(): void
|
||||
public function testUnknownClaimsAndTheImplicitRoleUserAreIgnored(): void
|
||||
{
|
||||
self::assertSame([Role::TEAMER], Role::combine([Role::USER, Role::TEAMER], []));
|
||||
self::assertSame(
|
||||
[Role::TEAMER],
|
||||
Role::sync([Role::USER, Role::TEAMER], [Role::TEAMER, 'ROLE_SOMETHING_ELSE']),
|
||||
);
|
||||
}
|
||||
|
||||
public function testEveryRoleHasALabel(): void
|
||||
public function testApprovalTurnsTheNominationIntoTheRole(): void
|
||||
{
|
||||
self::assertSame(Role::ALL, array_keys(Role::labels()));
|
||||
$roles = Role::approve([Role::pending(Role::ADMIN), Role::CUSTOMER], Role::ADMIN);
|
||||
|
||||
// The customer fallback goes with it: the account now holds an effective role.
|
||||
self::assertSame([Role::ADMIN], $roles);
|
||||
}
|
||||
|
||||
public function testApprovingARoleWithoutANominationIsRefused(): void
|
||||
{
|
||||
$this->expectException(\InvalidArgumentException::class);
|
||||
|
||||
Role::approve([Role::TEAMER], Role::ADMIN);
|
||||
}
|
||||
|
||||
public function testEffectiveRolesExcludeNominationsAndTheImplicitRoleUser(): void
|
||||
{
|
||||
$roles = [Role::USER, Role::TEAMER, Role::pending(Role::ADMIN)];
|
||||
|
||||
self::assertSame([Role::TEAMER], Role::effectiveOnly($roles));
|
||||
self::assertSame([Role::pending(Role::ADMIN)], Role::pendingOnly($roles));
|
||||
self::assertSame([Role::ADMIN => 'Administration'], Role::nominatedFrom($roles));
|
||||
}
|
||||
|
||||
public function testEveryRoleAndNominationHasALabel(): void
|
||||
{
|
||||
$labels = Role::labels();
|
||||
|
||||
foreach (Role::ALL as $role) {
|
||||
self::assertArrayHasKey($role, $labels);
|
||||
}
|
||||
|
||||
foreach (Role::ADMINISTRATIVE as $role) {
|
||||
self::assertArrayHasKey(Role::pending($role), $labels);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user