| Numéro du ticket | Titre du ticket | |------------------|-----------------| | | | ## Description de la PR ## Modification du .env ## Check list - [x] Pas de régression - [x] TU/TI/TF rédigée - [x] TU/TI/TF OK - [ ] CHANGELOG modifié Co-authored-by: Matthieu <mtholot19@gmail.com> Reviewed-on: #8 Co-authored-by: tristan <tristan@yuno.malio.fr> Co-committed-by: tristan <tristan@yuno.malio.fr>
This commit was merged in pull request #8.
This commit is contained in:
@@ -10,9 +10,11 @@ use App\Module\Core\Domain\Entity\User;
|
||||
use App\Module\Core\Domain\Exception\LastAdminProtectionException;
|
||||
use App\Module\Core\Domain\Security\AdminHeadcountGuardInterface;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Doctrine\ORM\PersistentCollection;
|
||||
use LogicException;
|
||||
use Symfony\Bundle\SecurityBundle\Security;
|
||||
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
||||
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
|
||||
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
||||
|
||||
/**
|
||||
@@ -29,6 +31,21 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
|
||||
* - Dernier admin global : impossible de retirer `isAdmin` si c'est le
|
||||
* dernier administrateur de l'instance, meme par un tiers. Enforce via
|
||||
* AdminHeadcountGuardInterface.
|
||||
* - Permission sites.manage : si le payload mute la collection `sites`,
|
||||
* la permission `sites.manage` est requise en plus de `core.users.manage`.
|
||||
* - Coherence currentSite (ticket 2 module Sites) : apres persist des
|
||||
* sites autorises, si le `currentSite` n'est plus dans la collection,
|
||||
* il est repositionne automatiquement :
|
||||
* a) repasse a `null` s'il pointait vers un site retire ;
|
||||
* b) est auto-selectionne sur le premier site de `sites` s'il etait
|
||||
* null alors que la collection vient d'etre modifiee et n'est pas vide.
|
||||
* Un second flush est emis uniquement si la coherence a du etre corrigee.
|
||||
* La garde coherence est skippee si ni les sites ni le currentSite n'ont
|
||||
* change (evite le silent site-switch sur un PATCH ne touchant pas aux sites).
|
||||
*
|
||||
* Atomicite : persistProcessor->process() + ensureCurrentSiteConsistency() sont
|
||||
* executes dans une meme transaction wrapInTransaction pour eviter un etat
|
||||
* partiellement persiste en cas d'erreur entre les deux flush.
|
||||
*
|
||||
* @implements ProcessorInterface<User, User>
|
||||
*/
|
||||
@@ -80,6 +97,87 @@ final class UserRbacProcessor implements ProcessorInterface
|
||||
}
|
||||
}
|
||||
|
||||
return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||
// Detection de la mutation de la collection `sites` avant tout flush.
|
||||
// La collection est deja denormalisee dans $data quand process() est appele.
|
||||
// On utilise PersistentCollection::isDirty() pour savoir si l'ORM a detecte
|
||||
// une modification depuis le chargement initial (ajout/retrait d'elements).
|
||||
$sitesCollection = $data->getSites();
|
||||
$sitesWereMutated = $sitesCollection instanceof PersistentCollection
|
||||
&& $sitesCollection->isDirty();
|
||||
|
||||
// Capture de l'ID du currentSite avant persist pour la detection post-flush.
|
||||
$originalCurrentSiteId = $data->getCurrentSite()?->getId();
|
||||
|
||||
// Garde sites.manage : la modification de la collection de sites rattaches
|
||||
// a un user est une operation sensible qui requiert une permission distincte
|
||||
// de core.users.manage (evite le bypass de sites.manage via /rbac).
|
||||
if ($sitesWereMutated && !$this->security->isGranted('sites.manage')) {
|
||||
throw new AccessDeniedHttpException(
|
||||
'La modification des sites rattaches a un user requiert la permission sites.manage.'
|
||||
);
|
||||
}
|
||||
|
||||
// Persistance + correction de coherence currentSite dans une seule transaction.
|
||||
// wrapInTransaction rollback automatiquement sur toute exception et la re-lance,
|
||||
// ce qui preserve le comportement attendu pour BadRequestHttpException.
|
||||
$result = null;
|
||||
$this->entityManager->wrapInTransaction(function () use (
|
||||
$data,
|
||||
$operation,
|
||||
$uriVariables,
|
||||
$context,
|
||||
$sitesWereMutated,
|
||||
$originalCurrentSiteId,
|
||||
&$result,
|
||||
): void {
|
||||
$result = $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||
|
||||
// Garde coherence currentSite (ticket 2 module Sites).
|
||||
// Post-persist : le champ `sites` a ete applique par le persist processor.
|
||||
// On s'assure que `currentSite` pointe toujours vers un site present
|
||||
// dans la collection ou est recale automatiquement — mais UNIQUEMENT si
|
||||
// les sites ou le currentSite ont effectivement ete touches dans ce PATCH.
|
||||
$currentSiteChangedByPersist = $originalCurrentSiteId !== $data->getCurrentSite()?->getId();
|
||||
if ($sitesWereMutated || $currentSiteChangedByPersist) {
|
||||
$this->ensureCurrentSiteConsistency($data);
|
||||
}
|
||||
});
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Applique deux corrections post-persist sur `currentSite` :
|
||||
* - si l'actuel n'est plus dans `sites` apres update → repasse a null ;
|
||||
* - si null et `sites` non vide → auto-selectionne le premier site
|
||||
* (coherent avec le choix de ne jamais laisser un user rattache a
|
||||
* plusieurs sites sans contexte courant apres une mutation effective).
|
||||
*
|
||||
* N'emet un flush additionnel que si une correction a ete necessaire :
|
||||
* pas de cout DB sur la majorite des requetes /rbac qui ne touchent pas
|
||||
* aux sites.
|
||||
*
|
||||
* Cette methode ne doit etre appelee que si les sites ont reellement
|
||||
* ete mutes dans la requete courante (voir logique dans process()).
|
||||
*/
|
||||
private function ensureCurrentSiteConsistency(User $user): void
|
||||
{
|
||||
$currentSite = $user->getCurrentSite();
|
||||
$sites = $user->getSites();
|
||||
$changed = false;
|
||||
|
||||
if (null !== $currentSite && !$user->hasSite($currentSite)) {
|
||||
$user->setCurrentSite(null);
|
||||
$changed = true;
|
||||
}
|
||||
|
||||
if (null === $user->getCurrentSite() && !$sites->isEmpty()) {
|
||||
$user->setCurrentSite($sites->first() ?: null);
|
||||
$changed = true;
|
||||
}
|
||||
|
||||
if ($changed) {
|
||||
$this->entityManager->flush();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,26 +8,50 @@ use App\Module\Core\Domain\Entity\Role;
|
||||
use App\Module\Core\Domain\Entity\User;
|
||||
use App\Module\Core\Domain\Repository\RoleRepositoryInterface;
|
||||
use App\Module\Core\Domain\Security\SystemRoles;
|
||||
use App\Module\Sites\Domain\Entity\Site;
|
||||
use App\Module\Sites\Domain\Repository\SiteRepositoryInterface;
|
||||
use App\Module\Sites\Infrastructure\DataFixtures\SitesFixtures;
|
||||
use Doctrine\Bundle\FixturesBundle\Fixture;
|
||||
use Doctrine\Common\DataFixtures\DependentFixtureInterface;
|
||||
use Doctrine\Persistence\ObjectManager;
|
||||
use RuntimeException;
|
||||
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
|
||||
|
||||
/**
|
||||
* Fixtures de base du module Core : 3 utilisateurs (1 admin + 2 standards)
|
||||
* rattaches aux roles systeme RBAC seedes par la migration Version20260414150034.
|
||||
* rattaches aux roles systeme RBAC seedes par la migration Version20260414150034,
|
||||
* puis (ticket 2 module Sites) rattaches a au moins un site avec un currentSite
|
||||
* coherent.
|
||||
*
|
||||
* Note : le purger Doctrine execute avant load() supprime l'ensemble des
|
||||
* entites managees, ce qui inclut la table role. On re-seede donc les roles
|
||||
* systeme de maniere idempotente avant de rattacher les utilisateurs, afin
|
||||
* que le workflow "make db-reset && make fixtures" reste one-shot.
|
||||
*
|
||||
* Dependance explicite a SitesFixtures (ticket 2) : les 3 sites Chatellerault,
|
||||
* Saint-Jean et Pommevic doivent etre presents en base avant d'etre rattaches
|
||||
* aux users. L'inversion volontaire de l'ordre (AppFixtures ← SitesFixtures)
|
||||
* casse l'independance declaree au ticket 1 : c'est un couplage assume car
|
||||
* apres ticket 2 le modele metier exprime un besoin legitime de rattachement.
|
||||
*/
|
||||
class AppFixtures extends Fixture
|
||||
class AppFixtures extends Fixture implements DependentFixtureInterface
|
||||
{
|
||||
public function __construct(
|
||||
private readonly UserPasswordHasherInterface $passwordHasher,
|
||||
private readonly RoleRepositoryInterface $roleRepository,
|
||||
private readonly SiteRepositoryInterface $siteRepository,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* @return array<int, class-string>
|
||||
*/
|
||||
public function getDependencies(): array
|
||||
{
|
||||
// SitesFixtures doit tourner AVANT AppFixtures pour que les sites
|
||||
// soient disponibles au rattachement des users ci-dessous.
|
||||
return [SitesFixtures::class];
|
||||
}
|
||||
|
||||
public function load(ObjectManager $manager): void
|
||||
{
|
||||
$adminRole = $this->ensureSystemRole(
|
||||
@@ -43,23 +67,43 @@ class AppFixtures extends Fixture
|
||||
'Role de base sans permission specifique',
|
||||
);
|
||||
|
||||
// Recupere les 3 sites seedes par SitesFixtures. Si absents, c'est
|
||||
// une misconfiguration (fixture hors purge ou dependance ignoree) :
|
||||
// on fail fort avec un message explicite plutot que de continuer
|
||||
// avec des users orphelins de site.
|
||||
$chatellerault = $this->requireSite('Chatellerault');
|
||||
$saintJean = $this->requireSite('Saint-Jean');
|
||||
$pommevic = $this->requireSite('Pommevic');
|
||||
|
||||
$admin = new User();
|
||||
$admin->setUsername('admin');
|
||||
$admin->setIsAdmin(true);
|
||||
$admin->setPassword($this->passwordHasher->hashPassword($admin, 'admin'));
|
||||
$admin->addRbacRole($adminRole);
|
||||
// Admin rattache aux 3 sites pour faciliter le dev / les tests manuels.
|
||||
$admin->addSite($chatellerault);
|
||||
$admin->addSite($saintJean);
|
||||
$admin->addSite($pommevic);
|
||||
$admin->setCurrentSite($chatellerault);
|
||||
$manager->persist($admin);
|
||||
|
||||
$alice = new User();
|
||||
$alice->setUsername('alice');
|
||||
$alice->setPassword($this->passwordHasher->hashPassword($alice, 'alice'));
|
||||
$alice->addRbacRole($userRole);
|
||||
// Alice : un seul site, site courant = ce site.
|
||||
$alice->addSite($chatellerault);
|
||||
$alice->setCurrentSite($chatellerault);
|
||||
$manager->persist($alice);
|
||||
|
||||
$bob = new User();
|
||||
$bob->setUsername('bob');
|
||||
$bob->setPassword($this->passwordHasher->hashPassword($bob, 'bob'));
|
||||
$bob->addRbacRole($userRole);
|
||||
// Bob : site different de Alice, pour prouver le filtrage par site
|
||||
// dans les futurs tests (ticket 4 outillage SiteAware).
|
||||
$bob->addSite($saintJean);
|
||||
$bob->setCurrentSite($saintJean);
|
||||
$manager->persist($bob);
|
||||
|
||||
$manager->flush();
|
||||
@@ -90,4 +134,19 @@ class AppFixtures extends Fixture
|
||||
|
||||
return $role;
|
||||
}
|
||||
|
||||
private function requireSite(string $name): Site
|
||||
{
|
||||
$site = $this->siteRepository->findByName($name);
|
||||
|
||||
if (null === $site) {
|
||||
throw new RuntimeException(sprintf(
|
||||
'SitesFixtures doit avoir seede le site "%s" avant le chargement des users. '
|
||||
.'Verifier que SitesFixtures est bien en dependance de AppFixtures.',
|
||||
$name,
|
||||
));
|
||||
}
|
||||
|
||||
return $site;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user