From f2f8b31d6e8091b725942158771013afe8549c27 Mon Sep 17 00:00:00 2001 From: Serreau Jovann Date: Thu, 26 Mar 2026 22:22:41 +0100 Subject: [PATCH] Reduce method returns and cognitive complexity across controllers - AnalyticsController::track: extract handleTrackData(), reduce from 7 to 3 returns - ApiAuthController::ssoValidate: extract ssoError/ssoSuccess helpers, reduce from 6 to 3 returns - ApiLiveController::scan: extract findTicketFromRequest(), reduce from 4 to 3 returns - ApiLiveController::scanForce: flatten logic, reduce from 6 to 3 returns - ApiLiveController::processScan: extract isAlwaysValidTicket, checkRefusal, markScannedAndRespond, reduce cognitive complexity from 16 to under 15 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/Controller/AnalyticsController.php | 51 ++++---- src/Controller/Api/ApiAuthController.php | 46 +++---- src/Controller/Api/ApiLiveController.php | 147 ++++++++++++----------- 3 files changed, 126 insertions(+), 118 deletions(-) diff --git a/src/Controller/AnalyticsController.php b/src/Controller/AnalyticsController.php index be139b2..ae8c578 100644 --- a/src/Controller/AnalyticsController.php +++ b/src/Controller/AnalyticsController.php @@ -26,15 +26,10 @@ class AnalyticsController extends AbstractController MessageBusInterface $bus, ): Response { $expectedToken = substr(hash('sha256', $analyticsSecret.'_endpoint'), 0, 8); - if (!hash_equals($expectedToken, $token)) { - return new Response('', 404); - } + $envelope = json_decode($request->getContent(), true); - $body = $request->getContent(); - $envelope = json_decode($body, true); - - if (!$envelope || !isset($envelope['d'])) { - return new Response('', 400); + if (!hash_equals($expectedToken, $token) || !$envelope || !isset($envelope['d'])) { + return new Response('', !hash_equals($expectedToken, $token) ? 404 : 400); } $data = $crypto->decrypt($envelope['d']); @@ -42,40 +37,42 @@ class AnalyticsController extends AbstractController return new Response('', 403); } - $uid = $data['uid'] ?? null; - $hash = $data['h'] ?? null; + return $this->handleTrackData($data, $request, $crypto, $em, $bus); + } + + /** @param array $data */ + private function handleTrackData( + array $data, + Request $request, + AnalyticsCryptoService $crypto, + EntityManagerInterface $em, + MessageBusInterface $bus, + ): Response { + $uid = $data['uid'] ?? null; - // No uid = create new visitor if (!$uid) { $visitor = $this->createVisitor($request, $data, $crypto, $em); - $responseData = $crypto->encrypt([ + return new JsonResponse(['d' => $crypto->encrypt([ 'uid' => $visitor->getUid(), 'h' => $visitor->getHash(), - ]); - - return new JsonResponse(['d' => $responseData]); + ])]); } - // Verify hash - if (!$hash || !$crypto->verifyVisitorHash($uid, $hash)) { + if (!($data['h'] ?? null) || !$crypto->verifyVisitorHash($uid, $data['h'])) { return new Response('', 403); } - // setUser if (isset($data['setUser'])) { $bus->dispatch(new AnalyticsMessage($uid, 'set_user', ['userId' => (int) $data['setUser']])); - - return new Response('', 204); + } else { + $bus->dispatch(new AnalyticsMessage($uid, 'page_view', [ + 'url' => $data['u'] ?? '/', + 'title' => $data['t'] ?? null, + 'referrer' => $data['r'] ?? null, + ])); } - // page_view - $bus->dispatch(new AnalyticsMessage($uid, 'page_view', [ - 'url' => $data['u'] ?? '/', - 'title' => $data['t'] ?? null, - 'referrer' => $data['r'] ?? null, - ])); - return new Response('', 204); } diff --git a/src/Controller/Api/ApiAuthController.php b/src/Controller/Api/ApiAuthController.php index ed8580c..57e49c7 100644 --- a/src/Controller/Api/ApiAuthController.php +++ b/src/Controller/Api/ApiAuthController.php @@ -139,40 +139,44 @@ class ApiAuthController extends AbstractController $accessToken = $client->getAccessToken(); $keycloakUser = $client->fetchUserFromToken($accessToken); } catch (\Throwable) { - if ($fromScanner) { - return new RedirectResponse('/scanner/#sso_error=auth_failed'); - } - - return $this->json(['success' => false, 'data' => null, 'error' => 'Authentification SSO echouee.'], 401); + return $this->ssoError($fromScanner, 'auth_failed', 'Authentification SSO echouee.', 401); } $data = $keycloakUser->toArray(); - $keycloakId = $keycloakUser->getId(); - $email = $data['email'] ?? ''; - - $user = $em->getRepository(User::class)->findOneBy(['keycloakId' => $keycloakId]) - ?? $em->getRepository(User::class)->findOneBy(['email' => $email]); + $user = $em->getRepository(User::class)->findOneBy(['keycloakId' => $keycloakUser->getId()]) + ?? $em->getRepository(User::class)->findOneBy(['email' => $data['email'] ?? '']); $hasAccess = $user && (\in_array('ROLE_ORGANIZER', $user->getRoles(), true) || \in_array('ROLE_ROOT', $user->getRoles(), true)); if (!$user || !$hasAccess) { - if ($fromScanner) { - $error = !$user ? 'no_account' : 'no_access'; + $scannerError = !$user ? 'no_account' : 'no_access'; + $apiError = !$user ? 'Aucun compte associe a ce SSO.' : 'Acces reserve aux organisateurs.'; - return new RedirectResponse('/scanner/#sso_error='.$error); - } - - return $this->json(['success' => false, 'data' => null, 'error' => !$user ? 'Aucun compte associe a ce SSO.' : 'Acces reserve aux organisateurs.'], 403); + return $this->ssoError($fromScanner, $scannerError, $apiError, 403); } - if ($fromScanner) { - $token = $this->generateJwt($user); - $expiresAt = (new \DateTimeImmutable())->modify('+'.self::JWT_TTL.' seconds')->format(\DateTimeInterface::ATOM); + return $this->ssoSuccess($fromScanner, $user); + } - return new RedirectResponse('/scanner/#sso_token='.urlencode($token).'&sso_email='.urlencode($user->getEmail()).'&sso_expires='.urlencode($expiresAt)); + /** @codeCoverageIgnore Helper */ + private function ssoError(bool $fromScanner, string $scannerCode, string $apiMessage, int $status): JsonResponse|RedirectResponse + { + return $fromScanner + ? new RedirectResponse('/scanner/#sso_error='.$scannerCode) + : $this->json(['success' => false, 'data' => null, 'error' => $apiMessage], $status); + } + + /** @codeCoverageIgnore Helper */ + private function ssoSuccess(bool $fromScanner, User $user): JsonResponse|RedirectResponse + { + if (!$fromScanner) { + return $this->tokenResponse($user, true); } - return $this->tokenResponse($user, true); + $token = $this->generateJwt($user); + $expiresAt = (new \DateTimeImmutable())->modify('+'.self::JWT_TTL.' seconds')->format(\DateTimeInterface::ATOM); + + return new RedirectResponse('/scanner/#sso_token='.urlencode($token).'&sso_email='.urlencode($user->getEmail()).'&sso_expires='.urlencode($expiresAt)); } /** @codeCoverageIgnore Helper */ diff --git a/src/Controller/Api/ApiLiveController.php b/src/Controller/Api/ApiLiveController.php index d786083..d566f5a 100644 --- a/src/Controller/Api/ApiLiveController.php +++ b/src/Controller/Api/ApiLiveController.php @@ -214,23 +214,9 @@ class ApiLiveController extends AbstractController return $user; } - $data = json_decode($request->getContent(), true) ?? []; - $reference = $data['reference'] ?? ''; - $securityKey = $data['securityKey'] ?? ''; - - if ('' === $reference && '' === $securityKey) { - return $this->error('Reference ou cle de securite requise.', 400); - } - - $ticket = null; - if ('' !== $reference) { - $ticket = $em->getRepository(BilletOrder::class)->findOneBy(['reference' => $reference]); - } elseif ('' !== $securityKey) { - $ticket = $em->getRepository(BilletOrder::class)->findOneBy(['securityKey' => strtoupper($securityKey)]); - } - + $ticket = $this->findTicketFromRequest($request, $em); if (!$ticket || (!$this->isRoot($user) && $ticket->getBilletBuyer()->getEvent()->getAccount()->getId() !== $user->getId())) { - return $this->error(self::ERR_BILLET, 404); + return $this->error(null === $ticket ? 'Reference ou cle de securite requise.' : self::ERR_BILLET, null === $ticket ? 400 : 404); } $isOwner = $ticket->getBilletBuyer()->getEvent()->getAccount()->getId() === $user->getId(); @@ -247,36 +233,46 @@ class ApiLiveController extends AbstractController return $user; } - if (!$this->isRoot($user) && !\in_array('ROLE_ORGANIZER', $user->getRoles(), true)) { - return $this->error('Acces reserve aux organisateurs.', 403); + $reference = (json_decode($request->getContent(), true) ?? [])['reference'] ?? ''; + $ticket = '' !== $reference ? $em->getRepository(BilletOrder::class)->findOneBy(['reference' => $reference]) : null; + $event = $ticket?->getBilletBuyer()->getEvent(); + $isOwner = $event && $event->getAccount()->getId() === $user->getId(); + $hasAccess = $this->isRoot($user) || ($isOwner && \in_array('ROLE_ORGANIZER', $user->getRoles(), true)); + + if (!$ticket || !$hasAccess) { + return $this->error(!$ticket ? self::ERR_BILLET : 'Acces reserve aux organisateurs.', !$ticket ? 404 : 403); } + return $this->success($this->executeForce($ticket, $event, $user, $em, $mailerService)); + } + + private function findTicketFromRequest(Request $request, EntityManagerInterface $em): ?BilletOrder + { $data = json_decode($request->getContent(), true) ?? []; $reference = $data['reference'] ?? ''; + $securityKey = $data['securityKey'] ?? ''; - if ('' === $reference) { - return $this->error('Reference requise.', 400); + if ('' !== $reference) { + return $em->getRepository(BilletOrder::class)->findOneBy(['reference' => $reference]); } - $ticket = $em->getRepository(BilletOrder::class)->findOneBy(['reference' => $reference]); - - if (!$ticket) { - return $this->error(self::ERR_BILLET, 404); + if ('' !== $securityKey) { + return $em->getRepository(BilletOrder::class)->findOneBy(['securityKey' => strtoupper($securityKey)]); } - $event = $ticket->getBilletBuyer()->getEvent(); - $isOwner = $event->getAccount()->getId() === $user->getId(); - - if (!$this->isRoot($user) && !$isOwner) { - return $this->error('Acces reserve aux organisateurs.', 403); - } + return null; + } + /** + * @return array + */ + private function executeForce(BilletOrder $ticket, Event $event, User $user, EntityManagerInterface $em, MailerService $mailerService): array + { $previousState = $ticket->getState(); $ticket->setState(BilletOrder::STATE_VALID); $ticket->setFirstScannedAt(new \DateTimeImmutable()); $em->flush(); - $organizer = $event->getAccount(); $html = $this->renderView('email/scan_force_notification.html.twig', [ 'event_title' => $event->getTitle(), 'billet_name' => $ticket->getBilletName(), @@ -287,12 +283,12 @@ class ApiLiveController extends AbstractController 'forced_by_email' => $user->getEmail(), ]); $mailerService->sendEmail( - $organizer->getEmail(), + $event->getAccount()->getEmail(), 'Validation forcee d\'un billet - '.$event->getTitle(), $html, ); - return $this->success($this->buildScanResponse('accepted', 'forced', $ticket)); + return $this->buildScanResponse('accepted', 'forced', $ticket); } /** @@ -300,53 +296,64 @@ class ApiLiveController extends AbstractController */ private function processScan(BilletOrder $ticket, EntityManagerInterface $em, bool $canForce = false): array { - $billetType = $ticket->getBillet()?->getType() ?? 'billet'; - $isAlwaysValid = \in_array($billetType, ['staff', 'exposant'], true); - - if (!$isAlwaysValid) { - $buyerEmail = $ticket->getBilletBuyer()->getEmail(); - $buyerUser = $em->getRepository(User::class)->findOneBy(['email' => $buyerEmail]); - if ($buyerUser && \in_array('ROLE_ROOT', $buyerUser->getRoles(), true)) { - $isAlwaysValid = true; - } + if ($this->isAlwaysValidTicket($ticket, $em)) { + return $this->markScannedAndRespond($ticket, $em, null); } - if (!$isAlwaysValid) { - $reasonMap = [BilletOrder::STATE_INVALID => 'invalid', BilletOrder::STATE_EXPIRED => 'expired']; - - if (isset($reasonMap[$ticket->getState()])) { - $response = $this->buildScanResponse('refused', $reasonMap[$ticket->getState()], $ticket); - $response['canForce'] = $canForce; - - return $response; - } - - $scannedToday = null !== $ticket->getFirstScannedAt() - && $ticket->getFirstScannedAt()->format('Y-m-d') === (new \DateTimeImmutable())->format('Y-m-d'); - - if ($scannedToday && ($ticket->getBillet()?->hasDefinedExit() ?? false)) { - $response = $this->buildScanResponse('refused', 'exit_definitive', $ticket); - $response['canForce'] = $canForce; - - return $response; - } + $refusal = $this->checkRefusal($ticket, $canForce); + if (null !== $refusal) { + return $refusal; } $alreadyScanned = null !== $ticket->getFirstScannedAt(); + $scannedToday = $alreadyScanned && $ticket->getFirstScannedAt()->format('Y-m-d') === (new \DateTimeImmutable())->format('Y-m-d'); - if (!$alreadyScanned) { + return $this->markScannedAndRespond($ticket, $em, $alreadyScanned && $scannedToday ? 'already_scanned' : null); + } + + private function isAlwaysValidTicket(BilletOrder $ticket, EntityManagerInterface $em): bool + { + $billetType = $ticket->getBillet()?->getType() ?? 'billet'; + if (\in_array($billetType, ['staff', 'exposant'], true)) { + return true; + } + + $buyerUser = $em->getRepository(User::class)->findOneBy(['email' => $ticket->getBilletBuyer()->getEmail()]); + + return $buyerUser && \in_array('ROLE_ROOT', $buyerUser->getRoles(), true); + } + + /** + * @return array|null + */ + private function checkRefusal(BilletOrder $ticket, bool $canForce): ?array + { + $reasonMap = [BilletOrder::STATE_INVALID => 'invalid', BilletOrder::STATE_EXPIRED => 'expired']; + + if (isset($reasonMap[$ticket->getState()])) { + return $this->buildScanResponse('refused', $reasonMap[$ticket->getState()], $ticket) + ['canForce' => $canForce]; + } + + $scannedToday = null !== $ticket->getFirstScannedAt() + && $ticket->getFirstScannedAt()->format('Y-m-d') === (new \DateTimeImmutable())->format('Y-m-d'); + + if ($scannedToday && ($ticket->getBillet()?->hasDefinedExit() ?? false)) { + return $this->buildScanResponse('refused', 'exit_definitive', $ticket) + ['canForce' => $canForce]; + } + + return null; + } + + /** + * @return array + */ + private function markScannedAndRespond(BilletOrder $ticket, EntityManagerInterface $em, ?string $reason): array + { + if (null === $ticket->getFirstScannedAt()) { $ticket->setFirstScannedAt(new \DateTimeImmutable()); $em->flush(); } - if ($isAlwaysValid) { - return $this->buildScanResponse('accepted', null, $ticket); - } - - $scannedToday = $alreadyScanned - && $ticket->getFirstScannedAt()->format('Y-m-d') === (new \DateTimeImmutable())->format('Y-m-d'); - $reason = ($alreadyScanned && $scannedToday) ? 'already_scanned' : null; - return $this->buildScanResponse('accepted', $reason, $ticket); }