From f2945e01de9589283b0444093e060f95a0bdec85 Mon Sep 17 00:00:00 2001 From: rudy Date: Tue, 24 Sep 2024 11:34:29 +0200 Subject: [PATCH] issue-39 : code quality, typage manquant --- src/Controller/CustomErrorController.php | 3 ++- src/Controller/LocaleController.php | 3 ++- src/Controller/MainController.php | 15 ++++++++------- src/Controller/SecurityController.php | 2 +- src/Entity/User.php | 2 +- src/EventListener/LocaleSubscriber.php | 4 ++-- src/SQLLogin/SQLLoginRequest.php | 18 +++++++++--------- src/Security/Hasher/PasswordEncoder.php | 10 ++++------ src/Security/SQLLoginUserAuthenticator.php | 10 +++++----- src/Security/SQLLoginUserProvider.php | 4 ++-- src/Service/SQLLoginService.php | 6 ++++-- 11 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/Controller/CustomErrorController.php b/src/Controller/CustomErrorController.php index f1ce3e6..73e812c 100644 --- a/src/Controller/CustomErrorController.php +++ b/src/Controller/CustomErrorController.php @@ -4,12 +4,13 @@ namespace App\Controller; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\ErrorHandler\Exception\FlattenException; +use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; class CustomErrorController extends AbstractController { #[Route(path: '/error', name: 'custom_error_controller')] - public function show(FlattenException $exception) + public function show(FlattenException $exception): Response { $statusCode = $exception->getStatusCode(); $message = $exception->getMessage(); diff --git a/src/Controller/LocaleController.php b/src/Controller/LocaleController.php index 1541780..3ed5a31 100644 --- a/src/Controller/LocaleController.php +++ b/src/Controller/LocaleController.php @@ -4,6 +4,7 @@ namespace App\Controller; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; +use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Annotation\Route; @@ -17,7 +18,7 @@ class LocaleController extends AbstractController } #[Route(path: 'locale/{locale?}', name: 'locale_change')] - public function changeLocal(?string $locale, Request $request) + public function changeLocal(?string $locale, Request $request): RedirectResponse { if (empty($locale)) { $locale = $this->params->get('default_locale'); diff --git a/src/Controller/MainController.php b/src/Controller/MainController.php index 157eb21..30c84e4 100644 --- a/src/Controller/MainController.php +++ b/src/Controller/MainController.php @@ -8,18 +8,18 @@ use App\Hydra\HydraService; use App\SQLLogin\SQLLoginRequest; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; +use Symfony\Component\Finder\Exception\AccessDeniedException; +use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; -use Symfony\Component\HttpFoundation\RedirectResponse; -use Symfony\Component\Finder\Exception\AccessDeniedException; use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Symfony\Component\Routing\Annotation\Route; class MainController extends AbstractController { - public HydraService $hydra; - public Client $client; - public SessionInterface $session; + private HydraService $hydra; + private Client $client; + private SessionInterface $session; public function __construct(SessionInterface $session, HydraService $hydra, Client $client) { @@ -33,6 +33,7 @@ class MainController extends AbstractController { return $this->hydra->handleLoginRequest($request); } + /* * Route de Healthcheck (notament pour kubernetes) */ @@ -43,7 +44,7 @@ class MainController extends AbstractController } #[Route('/connect/login-accept', name: 'app_login_accept', methods: ['GET'])] - public function loginAccept(Request $request, SQLLoginRequest $sqlLoginRequest): RedirectResponse + public function loginAccept(SQLLoginRequest $sqlLoginRequest): RedirectResponse { $user = $this->getUser(); diff --git a/src/Controller/SecurityController.php b/src/Controller/SecurityController.php index 71fd3de..8ed8253 100644 --- a/src/Controller/SecurityController.php +++ b/src/Controller/SecurityController.php @@ -56,7 +56,7 @@ class SecurityController extends AbstractController } #[Route('/logout', name: 'logout')] - public function logout(Request $request) + public function logout(Request $request): void { } } diff --git a/src/Entity/User.php b/src/Entity/User.php index 2ae7689..92ffdaf 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -6,7 +6,7 @@ use Symfony\Component\Security\Core\User\UserInterface; class User implements UserInterface { - protected array $attributes = []; + private array $attributes = []; private string $login; private string $password; private bool $rememberMe; diff --git a/src/EventListener/LocaleSubscriber.php b/src/EventListener/LocaleSubscriber.php index 04bc7d2..c57fa24 100644 --- a/src/EventListener/LocaleSubscriber.php +++ b/src/EventListener/LocaleSubscriber.php @@ -16,7 +16,7 @@ class LocaleSubscriber implements EventSubscriberInterface $this->defaultLocale = $defaultLocale; } - public function onKernelRequest(RequestEvent $event) + public function onKernelRequest(RequestEvent $event): void { $request = $event->getRequest(); if (!$request->hasPreviousSession()) { @@ -32,7 +32,7 @@ class LocaleSubscriber implements EventSubscriberInterface } } - public static function getSubscribedEvents() + public static function getSubscribedEvents(): array { return [ KernelEvents::REQUEST => [['onKernelRequest', 20]], diff --git a/src/SQLLogin/SQLLoginRequest.php b/src/SQLLogin/SQLLoginRequest.php index 45abae2..e745213 100644 --- a/src/SQLLogin/SQLLoginRequest.php +++ b/src/SQLLogin/SQLLoginRequest.php @@ -14,10 +14,10 @@ class SQLLoginRequest public const TABLE_NAME = 'table_name'; public const SUBJECT_REWRITE_EXPRESSION = 'subject_rewrite_expression'; - protected array $config; - protected string $dsn; - protected string $user; - protected string $password; + private array $config; + private string $dsn; + private string $user; + private string $password; public function __construct(string $dsn, string $user, string $password, array $config = []) { @@ -72,7 +72,7 @@ class SQLLoginRequest return $this->config[self::SUBJECT_REWRITE_EXPRESSION]; } - public function getRequestScope() + public function getRequestScope(): string { $scope = ''; if (!$this->config[self::DATA_TO_FETCH]) { @@ -80,12 +80,12 @@ class SQLLoginRequest } foreach ($this->config[self::DATA_TO_FETCH] as $data) { - $scope .= $data . ','; + $scope .= $data.','; } // On enlève la dernière virgule $scope = substr($scope, 0, -1); - return 'SELECT ' . $scope . ' FROM ' . $this->getTableName() . ' WHERE ' . $this->getLoginColumnName() . ' = :' . $this->getLoginColumnName() . ';'; + return 'SELECT '.$scope.' FROM '.$this->getTableName().' WHERE '.$this->getLoginColumnName().' = :'.$this->getLoginColumnName().';'; } /** @@ -96,9 +96,9 @@ class SQLLoginRequest { $fields = $this->getPasswordColumnName(); if (!empty($this->getSaltColumnName())) { - $fields .= ', ' . $this->getSaltColumnName(); + $fields .= ', '.$this->getSaltColumnName(); } - return 'SELECT ' . $fields . ' FROM ' . $this->getTableName() . ' WHERE ' . $this->getLoginColumnName() . ' = :' . $this->getLoginColumnName() . ';'; + return 'SELECT '.$fields.' FROM '.$this->getTableName().' WHERE '.$this->getLoginColumnName().' = :'.$this->getLoginColumnName().';'; } } diff --git a/src/Security/Hasher/PasswordEncoder.php b/src/Security/Hasher/PasswordEncoder.php index 91ffa7b..19dd2f3 100644 --- a/src/Security/Hasher/PasswordEncoder.php +++ b/src/Security/Hasher/PasswordEncoder.php @@ -30,7 +30,7 @@ class PasswordEncoder implements LegacyPasswordHasherInterface /** * Pas utilisé, mais on doit le garder pour le implements */ - public function hash(string $plainPassword, string $salt = null): string + public function hash(string $plainPassword, ?string $salt = null): string { if ($this->isPasswordTooLong($plainPassword)) { throw new InvalidPasswordException(); @@ -39,7 +39,7 @@ class PasswordEncoder implements LegacyPasswordHasherInterface return hash($plainPassword.$salt, $this->hashAlgoLegacy[0]); } - public function verify(string $hashedPassword, string $plainPassword, string $salt = null): bool + public function verify(string $hashedPassword, string $plainPassword, ?string $salt = null): bool { if ('' === $plainPassword || $this->isPasswordTooLong($plainPassword)) { return false; @@ -76,10 +76,8 @@ class PasswordEncoder implements LegacyPasswordHasherInterface /** * Retourne la string à hasher en fonction du pattern indiqué - * - * @return string */ - protected function getPasswordToHash($plainTextPassword, $salt) + protected function getPasswordToHash(string $plainTextPassword, ?string $salt = null): string { $arrayRef = [ self::PASSWORD_PATTERN => $plainTextPassword, @@ -101,7 +99,7 @@ class PasswordEncoder implements LegacyPasswordHasherInterface return $completedPlainPassword; } - protected function compareSsha($hashPassword, $plainPassword) + protected function compareSsha(string $hashPassword, string $plainPassword): bool { $base_64_hash_with_salt = substr($hashPassword, 6); $hash_with_salt = base64_decode($base_64_hash_with_salt); diff --git a/src/Security/SQLLoginUserAuthenticator.php b/src/Security/SQLLoginUserAuthenticator.php index 239f94e..71376f3 100644 --- a/src/Security/SQLLoginUserAuthenticator.php +++ b/src/Security/SQLLoginUserAuthenticator.php @@ -31,7 +31,7 @@ class SQLLoginUserAuthenticator extends AbstractLoginFormAuthenticator public const ERROR_DATA_TO_FETCH_CONFIGURATION = 'error_data_to_fetch_configuration'; public const ERROR_SECURITY_PATTERN_CONFIGURATION = 'error_security_pattern_configuration'; - protected string $baseUrl; + private string $baseUrl; private SQLLoginService $sqlLoginService; private PasswordEncoder $passwordHasher; @@ -54,14 +54,14 @@ class SQLLoginUserAuthenticator extends AbstractLoginFormAuthenticator public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey): RedirectResponse { - return new RedirectResponse($this->baseUrl . '/connect/login-accept'); + return new RedirectResponse($this->baseUrl.'/connect/login-accept'); } public function onAuthenticationFailure(Request $request, AuthenticationException $exception): RedirectResponse { $request->getSession()->set(Security::AUTHENTICATION_ERROR, $exception); - return new RedirectResponse($this->baseUrl . '/login'); + return new RedirectResponse($this->baseUrl.'/login'); } public function authenticate(Request $request): SelfValidatingPassport @@ -86,7 +86,7 @@ class SQLLoginUserAuthenticator extends AbstractLoginFormAuthenticator } if (null === $remoteHashedPassword) { - $remoteHashedPassword = ""; + $remoteHashedPassword = ''; } try { @@ -122,6 +122,6 @@ class SQLLoginUserAuthenticator extends AbstractLoginFormAuthenticator protected function getLoginUrl(Request $request): string { - return $this->baseUrl . '/login'; + return $this->baseUrl.'/login'; } } diff --git a/src/Security/SQLLoginUserProvider.php b/src/Security/SQLLoginUserProvider.php index e5bef7d..41e7375 100644 --- a/src/Security/SQLLoginUserProvider.php +++ b/src/Security/SQLLoginUserProvider.php @@ -31,7 +31,7 @@ class SQLLoginUserProvider implements UserProviderInterface return $this->loadUserByIdentifier($username, null); } - public function refreshUser(UserInterface $user) + public function refreshUser(UserInterface $user): UserInterface|null { if (!$user instanceof User) { throw new UnsupportedUserException(sprintf('Invalid user class "%s".', get_class($user))); @@ -40,7 +40,7 @@ class SQLLoginUserProvider implements UserProviderInterface return $this->loadUserByIdentifier($user->getUserIdentifier(), $user); } - public function supportsClass(string $class) + public function supportsClass(string $class): bool { return User::class === $class || is_subclass_of($class, User::class); } diff --git a/src/Service/SQLLoginService.php b/src/Service/SQLLoginService.php index 46a23ab..1e59062 100644 --- a/src/Service/SQLLoginService.php +++ b/src/Service/SQLLoginService.php @@ -16,7 +16,7 @@ use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; class SQLLoginService extends AbstractController { - public SQLLoginRequest $sqlLoginRequest; + private SQLLoginRequest $sqlLoginRequest; public function __construct(SQLLoginRequest $sqlLoginRequest, private LoggerInterface $loggerInterface) { @@ -56,6 +56,7 @@ class SQLLoginService extends AbstractController if (false === $datas) { throw new Exception(sprintf('La requête sql "%s" a renvoyé un résultat incorrect.', $request)); } + return $datas; } @@ -83,13 +84,14 @@ class SQLLoginService extends AbstractController if (!$password) { throw new Exception('Une erreur est survenue lors de la récupération des données'); } + return [ $password[$this->sqlLoginRequest->getPasswordColumnName()], isset($password[$this->sqlLoginRequest->getSaltColumnName()]) ? $password[$this->sqlLoginRequest->getSaltColumnName()] : null, ]; } - public function getConnection(): PDO + private function getConnection(): PDO { // Appel du singleton $sqlLogin = SQLLoginConnect::getInstance();