frommeandClaude Opus 5
f080d05dd6
The login address was only trimmed, so the casing somebody happened to type at their very first login was frozen into the user row forever — createOrUpdateLocalUser() never wrote the address back. Everything downstream re-sends the stored address rather than the one just authenticated with, which also leaked that casing into the OAuth2 email claim and the log identities. Harmless in practice, since BusPro matches an address case-insensitively and so does the utf8mb4_unicode_ci column, but it left User out of step with the newsletter entities, which have always normalized. Fold the case once in authenticate(), which covers the BusPro calls, the lookup and a new account alike, and write the address back on every login so an account created before this converges instead of staying frozen. No backfill: a row nobody logs into again is matched case-insensitively either way. Co-Authored-By: Claude Opus 5 <[email protected]>
452 lines
17 KiB
PHP
452 lines
17 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
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\Message\RoleNominationMessage;
|
|
use App\Security\BpnAuthenticator;
|
|
use App\Security\Crypt;
|
|
use App\Security\EmployeeDomainMatcher;
|
|
use App\Security\Role;
|
|
use App\Service\ProfileCompletenessChecker;
|
|
use Doctrine\ORM\EntityManagerInterface;
|
|
use Doctrine\ORM\EntityRepository;
|
|
use PHPUnit\Framework\TestCase;
|
|
use Psr\Log\LoggerInterface;
|
|
use Symfony\Component\HttpFoundation\Request;
|
|
use Symfony\Component\Messenger\Envelope;
|
|
use Symfony\Component\Messenger\MessageBusInterface;
|
|
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
|
|
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
|
|
|
|
/**
|
|
* 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
|
|
{
|
|
/** @var object[] messages the authenticator dispatched during the login under test */
|
|
private array $dispatched = [];
|
|
|
|
protected function setUp(): void
|
|
{
|
|
$this->dispatched = [];
|
|
}
|
|
|
|
public function testNewAccountIsSeededFromTheCrm(): void
|
|
{
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::HOUSE_MANAGER, Role::TEAMER], ['SSL', 'SSL']),
|
|
null,
|
|
$persisted,
|
|
);
|
|
|
|
$user = $this->loadUser($authenticator);
|
|
|
|
self::assertSame($persisted, $user);
|
|
self::assertSame(['ROLE_USER', Role::TEAMER, Role::pending(Role::HOUSE_MANAGER)], $user->getRoles());
|
|
self::assertSame(['SSL'], $user->getHotelCodes());
|
|
}
|
|
|
|
public function testAdministrativeClaimIsOnlyANominationUntilItIsApproved(): void
|
|
{
|
|
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER]);
|
|
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::TEAMER, Role::HOUSE_MANAGER], []),
|
|
$existing,
|
|
$persisted,
|
|
);
|
|
|
|
$user = $this->loadUser($authenticator);
|
|
|
|
self::assertSame($existing, $persisted, 'a login must not create a second account');
|
|
self::assertSame(
|
|
['ROLE_USER', Role::TEAMER, Role::pending(Role::HOUSE_MANAGER)],
|
|
$user->getRoles(),
|
|
);
|
|
self::assertNotNull($user->getLastLoginAt(), 'the rest of the profile is still synced');
|
|
}
|
|
|
|
public function testTheLoginAddressIsNormalizedAndRewritesAStoredCasing(): void
|
|
{
|
|
// An account from before the address was normalized: it carries the casing of whatever
|
|
// its very first login happened to type.
|
|
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER]);
|
|
|
|
$persisted = null;
|
|
$persistedPassword = null;
|
|
$lookupCriteria = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::TEAMER], []),
|
|
$existing,
|
|
$persisted,
|
|
$persistedPassword,
|
|
null,
|
|
$lookupCriteria,
|
|
);
|
|
|
|
$user = $this->loadUser($authenticator, ' [email protected] ');
|
|
|
|
self::assertSame($existing, $persisted, 'a differently cased address is the same account');
|
|
self::assertSame(['email' => '[email protected]'], $lookupCriteria);
|
|
self::assertSame(
|
|
'[email protected]',
|
|
$user->getEmail(),
|
|
'the stored casing follows the login rather than staying frozen',
|
|
);
|
|
}
|
|
|
|
public function testApprovedRoleSurvivesTheNextLogin(): void
|
|
{
|
|
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER, Role::HOUSE_MANAGER]);
|
|
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::TEAMER, Role::HOUSE_MANAGER], []),
|
|
$existing,
|
|
$persisted,
|
|
);
|
|
|
|
self::assertSame(
|
|
['ROLE_USER', Role::TEAMER, Role::HOUSE_MANAGER],
|
|
$this->loadUser($authenticator)->getRoles(),
|
|
);
|
|
}
|
|
|
|
public function testRoleRevokedInBusProIsWithdrawnOnLogin(): void
|
|
{
|
|
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER, Role::HOUSE_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]);
|
|
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::TEAMER], []),
|
|
$existing,
|
|
$persisted,
|
|
);
|
|
|
|
// The case myep-team depends on: somebody becomes a Teamer after their account exists.
|
|
self::assertSame(['ROLE_USER', Role::TEAMER], $this->loadUser($authenticator)->getRoles());
|
|
}
|
|
|
|
public function testHotelCodesAreResyncedOnEveryLogin(): void
|
|
{
|
|
$existing = (new User('[email protected]'))
|
|
->setRoles([Role::HOUSE_MANAGER])
|
|
->setHotelCodes(['DKS', 'SSL'])
|
|
;
|
|
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::HOUSE_MANAGER], ['DKS']),
|
|
$existing,
|
|
$persisted,
|
|
);
|
|
|
|
self::assertSame(['DKS'], $this->loadUser($authenticator)->getHotelCodes());
|
|
}
|
|
|
|
public function testDegradedCrmResponseLeavesAnExistingAccountUntouched(): void
|
|
{
|
|
$existing = (new User('[email protected]'))
|
|
->setRoles([Role::TEAMER, Role::HOUSE_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::HOUSE_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());
|
|
}
|
|
|
|
public function testANewAccountIsOnlyRegisteredOnceItCarriesItsPassword(): void
|
|
{
|
|
$persisted = null;
|
|
$persistedPassword = null;
|
|
|
|
// A degraded payload is what makes syncFromCrm() log, and that log write reaches the
|
|
// database. An account registered before the profile is complete would be written out
|
|
// half-built, and the user table rejects it: password is NOT NULL.
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([], [], selectionGroups: []),
|
|
null,
|
|
$persisted,
|
|
$persistedPassword,
|
|
);
|
|
|
|
$this->loadUser($authenticator);
|
|
|
|
self::assertInstanceOf(User::class, $persisted);
|
|
self::assertSame('encrypted', $persistedPassword);
|
|
}
|
|
|
|
/**
|
|
* @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, ?array $selectionGroups = null): CrmAttributes
|
|
{
|
|
$attributes = new CrmAttributes();
|
|
$attributes->roles = $roles;
|
|
$attributes->hotelCodes = $hotelCodes;
|
|
$attributes->selectionGroups = $selectionGroups ?? [new CrmSelectionGroup()];
|
|
|
|
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());
|
|
}
|
|
|
|
public function testAStaffContactAddressOnTheBusProRecordDoesNotMakeTheLoginStaff(): void
|
|
{
|
|
$persisted = null;
|
|
$persistedPassword = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::ADMIN, Role::TEAMER], []),
|
|
null,
|
|
$persisted,
|
|
$persistedPassword,
|
|
'[email protected]',
|
|
);
|
|
|
|
$user = $this->loadUser($authenticator, '[email protected]');
|
|
|
|
// BusPro accepts any address on the record as a login and answers with the first contact
|
|
// address regardless of which one was used, so only the typed address may decide. Reading
|
|
// the response instead would hand ROLE_EMPLOYEE — and every EMPLOYEE_ONLY role with it —
|
|
// to anyone who can add a staff address to their own BusPro record.
|
|
self::assertSame(['ROLE_USER', Role::TEAMER], $user->getRoles());
|
|
self::assertSame([], $this->dispatched);
|
|
}
|
|
|
|
public function testEmployeeOnlyClaimFromAnotherDomainIsIgnored(): void
|
|
{
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator($this->crmAttributes([Role::ADMIN, Role::TEAMER], []), null, $persisted);
|
|
|
|
$user = $this->loadUser($authenticator, '[email protected]');
|
|
|
|
// Not even a nomination, so there is nothing for an administrator to be told about.
|
|
self::assertSame(['ROLE_USER', Role::TEAMER], $user->getRoles());
|
|
self::assertSame([], $this->dispatched);
|
|
}
|
|
|
|
public function testEmployeeOnlyRoleIsRevokedWhenTheAddressIsNotStaff(): void
|
|
{
|
|
$existing = (new User('[email protected]'))->setRoles([Role::TEAMER, Role::ADMIN, Role::HOUSE_MANAGER]);
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator(
|
|
$this->crmAttributes([Role::TEAMER, Role::ADMIN, Role::HOUSE_MANAGER], []),
|
|
$existing,
|
|
$persisted,
|
|
);
|
|
|
|
$user = $this->loadUser($authenticator, '[email protected]');
|
|
|
|
// Only the EMPLOYEE_ONLY role goes; a Hausleitung does not need a staff address.
|
|
self::assertSame(['ROLE_USER', Role::TEAMER, Role::HOUSE_MANAGER], $user->getRoles());
|
|
}
|
|
|
|
public function testANewNominationIsAnnouncedOnce(): void
|
|
{
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator($this->crmAttributes([Role::ADMIN], []), null, $persisted);
|
|
|
|
$user = $this->loadUser($authenticator, '[email protected]');
|
|
|
|
self::assertCount(1, $this->dispatched);
|
|
$message = $this->dispatched[0];
|
|
self::assertInstanceOf(RoleNominationMessage::class, $message);
|
|
// The role itself, not its marker: the marker is an internal bookkeeping detail.
|
|
self::assertSame([Role::ADMIN], $message->roles);
|
|
self::assertContains(Role::pending(Role::ADMIN), $user->getRoles());
|
|
}
|
|
|
|
public function testAStandingNominationIsNotAnnouncedAgain(): void
|
|
{
|
|
$existing = (new User('[email protected]'))->setRoles([Role::EMPLOYEE, Role::pending(Role::ADMIN)]);
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator($this->crmAttributes([Role::ADMIN], []), $existing, $persisted);
|
|
|
|
$this->loadUser($authenticator, '[email protected]');
|
|
|
|
// The nomination has not changed, so there is nothing new to tell an administrator about.
|
|
self::assertSame([], $this->dispatched);
|
|
}
|
|
|
|
public function testAnApprovedRoleIsNotAnnouncedAsANomination(): void
|
|
{
|
|
$existing = (new User('[email protected]'))->setRoles([Role::EMPLOYEE, Role::ADMIN]);
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator($this->crmAttributes([Role::ADMIN], []), $existing, $persisted);
|
|
|
|
$this->loadUser($authenticator, '[email protected]');
|
|
|
|
self::assertSame([], $this->dispatched);
|
|
}
|
|
|
|
public function testALoginWithoutANominationAnnouncesNothing(): void
|
|
{
|
|
$persisted = null;
|
|
$authenticator = $this->authenticator($this->crmAttributes([Role::TEAMER], []), null, $persisted);
|
|
|
|
$this->loadUser($authenticator);
|
|
|
|
self::assertSame([], $this->dispatched);
|
|
}
|
|
|
|
private function authenticator(
|
|
CrmAttributes $crmAttributes,
|
|
?User $existing,
|
|
?User &$persisted,
|
|
?string &$persistedPassword = null,
|
|
?string $contactEmail = null,
|
|
?array &$lookupCriteria = null,
|
|
): BpnAuthenticator {
|
|
$personalData = new PersonalData();
|
|
$personalData->personId = 42;
|
|
$personalData->addressId = 4711;
|
|
$personalData->communication->email = $contactEmail;
|
|
|
|
$apiClient = $this->createStub(ApiClient::class);
|
|
$apiClient->method('getPersonalData')->willReturn($personalData);
|
|
$apiClient->method('getCrmAttributes')->willReturn($crmAttributes);
|
|
|
|
$repository = $this->createStub(EntityRepository::class);
|
|
$repository
|
|
->method('findOneBy')
|
|
->willReturnCallback(static function (array $criteria) use ($existing, &$lookupCriteria): ?User {
|
|
$lookupCriteria = $criteria;
|
|
|
|
return $existing;
|
|
})
|
|
;
|
|
|
|
$entityManager = $this->createStub(EntityManagerInterface::class);
|
|
$entityManager->method('getRepository')->willReturn($repository);
|
|
$entityManager
|
|
->method('persist')
|
|
->willReturnCallback(static function (object $entity) use (&$persisted, &$persistedPassword): void {
|
|
$persisted = $entity;
|
|
// Snapshot rather than a reference: what matters is what the account looked like
|
|
// at the moment it was registered, not what it grew into afterwards.
|
|
$persistedPassword = $entity instanceof User ? $entity->getPassword() : null;
|
|
})
|
|
;
|
|
|
|
$crypt = $this->createStub(Crypt::class);
|
|
$crypt->method('encrypt')->willReturn('encrypted');
|
|
|
|
$completenessChecker = $this->createStub(ProfileCompletenessChecker::class);
|
|
$completenessChecker->method('isComplete')->willReturn(true);
|
|
|
|
$messageBus = $this->createStub(MessageBusInterface::class);
|
|
$messageBus
|
|
->method('dispatch')
|
|
->willReturnCallback(function (object $message): Envelope {
|
|
$this->dispatched[] = $message;
|
|
|
|
return new Envelope($message);
|
|
})
|
|
;
|
|
|
|
return new BpnAuthenticator(
|
|
$this->createStub(UrlGeneratorInterface::class),
|
|
$apiClient,
|
|
$entityManager,
|
|
$crypt,
|
|
$completenessChecker,
|
|
$this->createStub(LoggerInterface::class),
|
|
new EmployeeDomainMatcher(['ep-reisen.de']),
|
|
$messageBus,
|
|
);
|
|
}
|
|
|
|
private function loadUser(BpnAuthenticator $authenticator, string $email = '[email protected]'): User
|
|
{
|
|
$request = new Request();
|
|
$request->request->set('_username', $email);
|
|
$request->request->set('_password', 'secret');
|
|
|
|
$badge = $authenticator->authenticate($request)->getBadge(UserBadge::class);
|
|
self::assertInstanceOf(UserBadge::class, $badge);
|
|
|
|
$user = $badge->getUser();
|
|
self::assertInstanceOf(User::class, $user);
|
|
|
|
return $user;
|
|
}
|
|
}
|