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

[Sécurité] [BO] Améliorations du FileVoter #2696

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

numew
Copy link
Collaborator

@numew numew commented Jun 14, 2024

Ticket

#2679

Description

  • Remplacement du voter FILE_CREATE par l'existant SIGN_EDIT qui est plus complet (et logiquement si l'on peux éditer les info d'un signalement on peux aussi y ajouter des documents)
  • Remplacement du voter FILE_VIEW par l'existant SIGN_VIEW qui est plus complet (et logiquement si l'on peux consulter un signalement on peux voir ses documents)
  • Ajout d'un contrôle afin de bloquer les signalements archivé directement dans le voter SIGN_VIEW
  • Remplacement du voter INTERVENTION_EDIT_VISITE par l'existant SIGN_ADD_VISITE qui fait les mêmes contrôles
  • Réorganisation du FileVoter
  • Dans le SignalementVisitesController ajout d'un contrôle de cohérence entre le signalement et l'affectation + retrait d'un appel au voter SIGN_VIEW inutile (tous les contrôle étant déja correct dans SIGN_ADD_VISITE
  • Ajout de tests

Tests

  • Vérifier que l'ajout de document (coté BO) sur un signalement fonctionne correctement
  • Vérifier que l'ajout de visite (coté BO) sur un signalement fonctionne correctement

@numew numew marked this pull request as ready for review June 14, 2024 15:28
@numew numew changed the title [WIP] [Sécurité] [BO] Améliorations du FileVoter [Sécurité] [BO] Améliorations du FileVoter Jun 14, 2024
@numew numew force-pushed the feature/2645-secure-role-and-more branch from 82d237c to 07ab159 Compare June 14, 2024 15:44
@numew numew changed the base branch from feature/2645-secure-role-and-more to develop June 17, 2024 09:06
@numew numew force-pushed the feature/2679-revamp-file-voter branch from 4f370f5 to 40cc8db Compare June 17, 2024 11:32
@emilschn
Copy link
Collaborator

Retour de test :
Sur le 13, j'ai pris un signalement avec admin-territoire que j'ai affecté à Partenaire 13-01, 02 et 03.
Je me connecte en tant que Partenaire 13-02, et l'accès au signalement est refusé (Access denied)
Ce n'est pas le cas sur develop.
De mon analyse, c'est que dans le SignalementVoter, tu as restreint l'accès si l'affectation est acceptée. Je pense qu'il ne faut pas restreindre en fonction du statut de l'affectation.

C'est le seul truc que j'ai remarqué pour l'instant.

@numew
Copy link
Collaborator Author

numew commented Jun 19, 2024

Retour de test : Sur le 13, j'ai pris un signalement avec admin-territoire que j'ai affecté à Partenaire 13-01, 02 et 03. Je me connecte en tant que Partenaire 13-02, et l'accès au signalement est refusé (Access denied) Ce n'est pas le cas sur develop. De mon analyse, c'est que dans le SignalementVoter, tu as restreint l'accès si l'affectation est acceptée. Je pense qu'il ne faut pas restreindre en fonction du statut de l'affectation.

C'est le seul truc que j'ai remarqué pour l'instant.

ok pour ce point

Copy link

sonarcloud bot commented Jun 19, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
3.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.

Un commentaire, mais tests OK

@@ -45,7 +45,7 @@ security:
# https://symfony.com/doc/current/security.html#the-firewall

# https://symfony.com/doc/current/security/impersonating_user.html
# switch_user: true
#switch_user: {role: ROLE_ADMIN}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@numew je ne comprends pas ça.

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 pour moi

Copy link
Collaborator

@emilschn emilschn left a comment

Choose a reason for hiding this comment

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

Un retour de test !

return $signalement->getAffectations()->filter(function (Affectation $affectation) use ($user) {
return $affectation->getPartner()->getId() === $user->getPartner()->getId();
})->count() > 0 || $user->isTerritoryAdmin() && $user->getTerritory() === $signalement->getTerritory();
}

public function canAddVisite(Signalement $signalement, User $user): bool
{
if (Signalement::STATUS_ACTIVE !== $signalement->getStatut() && Signalement::STATUS_NEED_PARTNER_RESPONSE !== $signalement->getStatut()) {
if (!\in_array($signalement->getStatut(), [Signalement::STATUS_ACTIVE, Signalement::STATUS_NEED_PARTNER_RESPONSE])) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

il faudrait aussi que l'affectation soit acceptée, non ?
en tant qu'admin territoire, même si je n'ai pas accepté, je peux ajouter des fichiers et des visites

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oui, alors j'ai aussi remarqué ca hier en validant la PR #2732 mais du coup ca semble être le fonctionnement actuel, on a fermé les yeux avec hélène....

Copy link
Collaborator

Choose a reason for hiding this comment

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

c'est pas l'occasion de le faire dans cette PR spécifique ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Tout est finalement ok, erreur de ma part !

@emilschn emilschn merged commit 40a83c3 into develop Jun 27, 2024
2 of 3 checks passed
@sfinx13 sfinx13 deleted the feature/2679-revamp-file-voter branch July 10, 2024 18:11
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.

3 participants