Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Securité] Revoir les actions sur les utilisateurs et partenaires #2297

Merged
merged 12 commits into from
Mar 5, 2024

Conversation

numew
Copy link
Collaborator

@numew numew commented Feb 27, 2024

Ticket

#2247

Description

  • Lors de la création/modification d'un utilisateur le role est contrôlé (impossible de modifier un role supérieur au sien / impossible d'attribuer un role supérieur au sien)
  • Lors du transfert d'un utilisateur contrôle du partenaire cible (doit être dans le même territoire que l'utilisateur - hors super admin)
  • Simplifications et corrections diverses sur les Voters User, Partner et leurs utilisations

Tests

  • Vérifier que l'ajout/modification de partenaire fonctionne correctement
  • Vérifier que l'ajout/modification/transfert/suppression d'utilisateur fonctionne correctement

@hmeneuvrier
Copy link
Collaborator

hmeneuvrier commented Feb 28, 2024

Test réactivation utilisateur OK
Test transfert utilisateur OK

Test en admin partenaire :
Je veux éditer un admin territoire, je peux lui mettre un rôle d'admin partenaire ou d'utilisateur. (ensuite quand je le réédites, je ne peux pas le remettre admin territoire)

image

assets/controllers/form_partner.js Show resolved Hide resolved
$this->denyAccessUnlessGranted('USER_CREATE', $partner);
$data = $request->get('user_create');
if (!$this->isCsrfTokenValid('partner_user_create', $request->request->get('_token'))) {
$this->addFlash('error', 'Token invalide.');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mettre un message d'erreur plus explicite et dire quoi faire

Une erreur est survenue, merci d'actualiser la page et réessayer ?

src/Controller/Back/PartnerController.php Outdated Show resolved Hide resolved
$this->addFlash('success', 'L\'utilisateur a bien été supprimé.');
$userId = $request->request->get('user_id');
if (!$this->isCsrfTokenValid('partner_user_delete', $request->request->get('_token'))) {
$this->addFlash('error', 'Token invalide.');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Une erreur est survenue, merci d'actualiser la page et réessayer ?

src/Controller/Back/PartnerController.php Outdated Show resolved Hide resolved
src/Security/Voter/UserVoter.php Show resolved Hide resolved
@numew numew marked this pull request as draft February 29, 2024 11:11
@numew numew marked this pull request as ready for review March 1, 2024 13:52
@hmeneuvrier
Copy link
Collaborator

Un dernier petit souci @numew

Je suis resp territoire, j'ai un super admin dans un partenaire (ce qui n'est pas sensé arriver, mais qui est possible techniquement), je ne peux plus l'éditer (cool), mais je peux toujours le transférer et le supprimer :
image

src/Security/Voter/UserVoter.php Show resolved Hide resolved
src/Security/Voter/UserVoter.php Show resolved Hide resolved
Copy link

sonarcloud bot commented Mar 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
10.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests ok, une dernière petite remarque sur le code

src/Security/Voter/PartnerVoter.php Show resolved Hide resolved
templates/back/partner/view.html.twig Show resolved Hide resolved
src/Security/Voter/UserVoter.php Show resolved Hide resolved
src/Security/Voter/UserVoter.php Show resolved Hide resolved
Copy link
Collaborator

@hmeneuvrier hmeneuvrier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK tests et lecture

Copy link
Collaborator

@sfinx13 sfinx13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lecture et test OK

@sfinx13 sfinx13 merged commit 7175872 into develop Mar 5, 2024
2 of 3 checks passed
@sfinx13 sfinx13 deleted the bugfix/2247-control-user-role branch March 11, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants