-
Notifications
You must be signed in to change notification settings - Fork 0
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é] Historisation des connexions à la plateforme #2742
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça marche très bien, mais quelques questions
src/EventListener/LoginListener.php
Outdated
user: $user | ||
); | ||
} | ||
|
||
$this->requestStack->getSession()->set('_security.territory', $user->getTerritory()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Juste pour éviter de prêter à confusion utiliser une seule ressource pour récupérer la requête. Vu qu'elle est dispo dans l'event je propose de s'appuyer uniquement sur $event->getRequest()
et supprimer l'utilisateur l'utilisation de $requestStack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pas tout à fait sûr de comprendre.
Est-ce que ce que j'ai modifié correspond à ce que tu proposes ?
(et dans ce cas, tu voulais peut-être dire "utilisation" plutôt que "utilisateur" ?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ça correspond bien mais à la ligne 39 je vois qu'il y'a le service $requestStack
qui est injecté ça fait redondant avec $event->getRequest()
Remplacer
$this->requestStack->getSession()->set('_security.territory', $user->getTerritory());
par
$request = $event->getRequest();
if (self::CHECK_2FA_PATH !== $request->getPathInfo()) {
// .....
}
$request->getSession()->set('_security.territory', $user->getTerritory());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu as bien appliqué la modif donc c'est tout bon
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relecture et retests OK pour moi
There was a problem hiding this 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
Ticket
#728
Description
Historisation de l'ensemble des accès à la plateforme
Changements apportés
Pré-requis
make execute-migration name=Version20240624153853 direction=up
Tests
history_entry