feat: streamlined role-revocation logic

This commit is contained in:
Björn Fromme
2026-08-10 16:24:41 +02:00
parent b39a78da82
commit fd5d478a5c
14 changed files with 426 additions and 22 deletions
@@ -87,12 +87,13 @@ class CrmAttributesResponse
{
$crmSelections = [];
foreach ($this->getAttributeGroups() as $group) {
// a response the CRM sent no attributes in still has to flatten to an empty set
foreach ($this->getAttributeGroups() ?? [] as $group) {
/** @var CrmAttributeGroup $group */
if (false === isset($crmSelections[$group->getLabel()])) {
$crmSelections[$group->getLabel()] = [];
}
foreach ($group->getAttributes() as $attribute) {
foreach ($group->getAttributes() ?? [] as $attribute) {
/** @var CrmAttribute $attribute */
if (false === $attribute->isSelected()) {
continue;
+42 -8
View File
@@ -165,10 +165,11 @@ class UserDataHandler
/**
* Updates an existing user from BusPro data.
*
* Roles and hotel codes are imported once on user creation only and are managed
* manually afterwards, so they are intentionally left untouched here. The only
* exception are the privilege-free pending markers, which keep tracking the
* administrative roles claimed in the CRM.
* Administrative roles and hotel codes are imported once on user creation only and are
* managed manually afterwards, so they are intentionally left untouched here. The
* exceptions are the privilege-free pending markers, which keep tracking the
* administrative roles claimed in the CRM, and ROLE_TEAMER, which needs no approval
* and is granted to whoever the CRM reports as a teamer.
*
* @param string[] $claimedRoles pending markers as returned by collectPendingRoles()
*/
@@ -188,6 +189,8 @@ class UserDataHandler
$this->refreshPendingRoles($user, $claimedRoles);
if (true === $isTeamer) {
$this->grantTeamerRole($user);
$address = Address::fromApiResponse($profileResponse);
$communication = Communication::fromApiResponse($profileResponse);
if (null === $user->getTeamer()) {
@@ -221,15 +224,19 @@ class UserDataHandler
/**
* Blocks a user the CRM no longer grants anything in this application.
*
* The granted roles are deliberately kept: they stay visible for review and are what
* makes the user reappear in the administrative list, where a super admin can unblock
* them. Only the privilege-free markers are dropped, as they no longer reflect the CRM.
* The granted roles are deliberately kept: they stay visible for review. Only the
* privilege-free markers are dropped, as they no longer reflect the CRM. What keeps the
* user reachable is the block itself, not the roles: a teamer stays in the teamer list,
* everyone else is listed by getAdministrativeUsers() whatever roles are left.
* Regaining a CRM role does not unblock the account, that is a manual decision.
*/
public function disableForRevokedCrmRoles(User $user): void
{
// an existing block may be a disciplinary one and must never be overwritten
// an existing block may be a disciplinary one and must never be overwritten, but
// findLocalUser() may have refreshed the BusPro ids and nothing else flushes here
if (true === $user->isDisabled()) {
$this->entityManager->flush();
return;
}
@@ -249,6 +256,33 @@ class UserDataHandler
]);
}
/**
* Grants ROLE_TEAMER to a user the CRM reports as a teamer.
*
* Unlike the administrative roles this one needs no approval, so it may be granted on
* any login rather than on creation only: it carries no privileges beyond the teamer
* area, and a teamer without it would be left with a teamer record they cannot reach,
* or locked out entirely for holding no assignable role at all.
*
* 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.
*/
private function grantTeamerRole(User $user): void
{
$grantedRoles = $user->getAssignedRoles();
if (true === in_array('ROLE_TEAMER', $grantedRoles, true)) {
return;
}
$user->setRoles([...$grantedRoles, 'ROLE_TEAMER', ...$user->getPendingRoles()]);
$this->logger->info('Grant teamer role', [
'user_id' => $user->getId(),
'user_email' => $user->getEmail(),
]);
}
/**
* Keeps the pending markers in sync with the administrative roles claimed in the CRM.
* The markers grant no privileges, so tracking them on every login is safe: only a
@@ -29,7 +29,7 @@ class DisableUserController extends AbstractController
$form = $this->createForm(DisableUserType::class, $user, ['hx_post' => $request->getUri()]);
$form->handleRequest($request);
if ($form->isSubmitted() && $form->isValid()) {
$user->setDisabledAt(new \DateTimeImmutable());
$user->setDisabled(true);
$this->entityManager->flush();
@@ -56,11 +56,7 @@ class DisableUserController extends AbstractController
$user = $teamer->getUser();
if (true === $request->isMethod(Request::METHOD_POST)) {
$user
->setDisabledAt(null)
->setDisabledReason(null)
->setDisabledReasonInternal(null)
;
$user->setDisabled(false);
$this->entityManager->flush();
+25 -2
View File
@@ -33,8 +33,6 @@ class UserType extends AbstractType
'required' => false,
'help' => 'Setzt die Rolle Admin voraus.',
])
// must stay ahead of the reason: properties are written in field order and
// unblocking clears the reasons
->add('disabled', CheckboxType::class, [
'label' => 'Account gesperrt',
'required' => false,
@@ -49,8 +47,33 @@ class UserType extends AbstractType
'data-action' => 'textarea-autosize#resize',
],
])
->add('disabledReasonInternal', TextareaType::class, [
'label' => 'Begründung intern',
'required' => false,
'help' => 'Wird der Benutzer:in nicht angezeigt.',
'attr' => [
'data-controller' => 'textarea-autosize',
'data-action' => 'textarea-autosize#resize',
],
])
;
// the reasons only ever describe a block, so an unblocked account carries none.
// Done after mapping instead of relying on the field order, as the submitted text
// would otherwise be written back over the reset done by setDisabled()
$builder->addEventListener(FormEvents::POST_SUBMIT, static function (FormEvent $event): void {
$user = $event->getData();
if (false === $user instanceof User || true === $user->isDisabled()) {
return;
}
$user
->setDisabledReason(null)
->setDisabledReasonInternal(null)
;
});
// hotel codes already assigned to the user may predate the catalog, so they are
// added as choices to keep them selectable instead of failing
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event): void {
+10
View File
@@ -25,6 +25,11 @@ class UserRepository extends ServiceEntityRepository
}
/**
* Users holding an administrative role or awaiting approval for one, plus blocked
* accounts without a teamer. A user demoted by the CRM may keep no role that would list
* them, and a block is only ever lifted from a list: teamers are unblocked from theirs,
* everyone else has none but this one.
*
* @return User[]
*/
public function getAdministrativeUsers(): array
@@ -41,6 +46,11 @@ class UserRepository extends ServiceEntityRepository
;
}
$qb->orWhere($qb->expr()->andX(
$qb->expr()->isNotNull('u.disabledAt'),
$qb->expr()->isNull('u.teamer'),
));
return $qb
->orderBy('u.lastName', 'ASC')
->getQuery()
+19 -1
View File
@@ -112,12 +112,16 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent
): ?User {
// Fetch CRM attributes, early return in case of an API error
try {
/** @var CrmAttributesResponse $crmAttributes */
$crmAttributes = $this->apiClient->getCrmAttributes($email, $password);
} catch (ApiClientException $e) {
return null;
}
// BusPro answers with a notification record instead of the data on its own errors
if (false === $crmAttributes instanceof CrmAttributesResponse) {
return null;
}
// Flatten selected CRM attributes
$crmSelections = $crmAttributes->toArray();
@@ -140,6 +144,20 @@ class BpnAuthenticator extends AbstractLoginFormAuthenticator implements Authent
// are no user of it: never create an account, block an existing one. Returning
// the blocked user lets the UserChecker explain why the login was refused.
if ([] === $claimedRoles) {
// A response without any attribute group carries no roles either, so it looks
// exactly like a revocation while it really means the CRM told us nothing:
// an empty payload, a changed schema, a misconfigured attribute id. Blocking
// on that would lock out every user logging in, so refuse this single login
// instead and leave the account alone.
if ([] === ($crmAttributes->getAttributeGroups() ?? [])) {
$this->logger->warning('Skip demotion: CRM attributes response carries no attribute groups', [
'user_id' => $user?->getId(),
'user_email' => $email,
]);
return null;
}
if (null === $user) {
return null;
}
+1 -1
View File
@@ -59,7 +59,7 @@ class FeedbackReminderService
$hotelManagers = $this
->userRepository
->getUsersByRoleAndHotelCode('ROLE_HOTEL_MANAGER', $hotelBaseCodes)
->getUsersByRoleAndHotelCode('ROLE_HOUSE_MANAGER', $hotelBaseCodes)
;
if (0 === count($hotelManagers)) {