diff --git a/composer.json b/composer.json index 8dd6294..5f0ff57 100644 --- a/composer.json +++ b/composer.json @@ -57,7 +57,8 @@ "scripts": { "verify": [ "vendor/bin/phpcs -p", - "vendor/bin/phpunit --no-coverage --testdox" + "vendor/bin/phpunit --no-coverage --testdox", + "vendor/bin/phpstan --memory-limit=256M analyze -c phpstan.neon" ], "tests": [ "vendor/bin/phpunit --no-coverage" diff --git a/src/Controller/Traits/TicketValidatorTrait.php b/src/Controller/Traits/TicketValidatorTrait.php index 5956875..23cd9e0 100644 --- a/src/Controller/Traits/TicketValidatorTrait.php +++ b/src/Controller/Traits/TicketValidatorTrait.php @@ -74,21 +74,7 @@ public function validate( $message = ''; // Below, we do not have a ticket or the ticket does not meet the very basic criteria that allow // any further handling - if (empty($serviceTicket)) { - // No ticket - $message = 'Ticket ' . var_export($ticket, true) . ' not recognized'; - $failed = true; - } elseif ($method === 'proxyValidate' && !$this->ticketFactory->isProxyTicket($serviceTicket)) { - // proxyValidate but not a proxy ticket - $message = 'Ticket ' . var_export($ticket, true) . ' is not a proxy ticket.'; - $failed = true; - } elseif ($method === 'serviceValidate' && !$this->ticketFactory->isServiceTicket($serviceTicket)) { - // serviceValidate but not a service ticket - $message = 'Ticket ' . var_export($ticket, true) . ' is not a service ticket.'; - $failed = true; - } - - if ($failed) { + if ($message = $this->validateServiceTicket($serviceTicket, $ticket, $method)) { $finalMessage = 'casserver:validate: ' . $message; Logger::error(__METHOD__ . '::' . $finalMessage); @@ -185,4 +171,27 @@ public function validate( Response::HTTP_OK, ); } + + /** + * @param array{'id': string}|null $serviceTicket + * + * @return ?string Message on failure, null on success + */ + private function validateServiceTicket(?array $serviceTicket, string $ticket, string $method): ?string + { + if (empty($serviceTicket)) { + return 'Ticket ' . var_export($ticket, true) . ' not recognized'; + } + + $isServiceTicket = $this->ticketFactory->isServiceTicket($serviceTicket); + if ($method === 'serviceValidate' && !$isServiceTicket) { + return 'Ticket ' . var_export($ticket, true) . ' is not a service ticket.'; + } + + if ($method === 'proxyValidate' && !$isServiceTicket && !$this->ticketFactory->isProxyTicket($serviceTicket)) { + return 'Ticket ' . var_export($ticket, true) . ' is not a proxy ticket.'; + } + + return null; + } } diff --git a/tests/src/Controller/Cas20ControllerTest.php b/tests/src/Controller/Cas20ControllerTest.php index 4842ad1..1dcdf76 100644 --- a/tests/src/Controller/Cas20ControllerTest.php +++ b/tests/src/Controller/Cas20ControllerTest.php @@ -565,14 +565,6 @@ public static function validateOnDifferentQueryParameterCombinationsProxyValidat "Ticket 'PT-{$sessionId}' has expired", 'PT-' . $sessionId, ], - 'Returns Bad Request on Ticket is A Service Ticket' => [ - [ - 'ticket' => 'ST-' . $sessionId, - 'service' => 'https://myservice.com/abcd', - ], - "Ticket 'ST-{$sessionId}' is not a proxy ticket.", - 'ST-' . $sessionId, - ], 'Returns Bad Request on Ticket Issued By Single SignOn Session' => [ [ 'ticket' => 'PT-' . $sessionId, @@ -583,7 +575,7 @@ public static function validateOnDifferentQueryParameterCombinationsProxyValidat 'PT-' . $sessionId, 9999999999, ], - 'Returns Success' => [ + 'Returns Success with Proxy Ticket' => [ [ 'ticket' => 'PT-' . $sessionId, 'service' => 'https://myservice.com/abcd', @@ -592,6 +584,15 @@ public static function validateOnDifferentQueryParameterCombinationsProxyValidat 'PT-' . $sessionId, 9999999999, ], + 'Returns Success with Service Ticket' => [ + [ + 'ticket' => 'ST-' . $sessionId, + 'service' => 'https://myservice.com/abcd', + ], + 'username@google.com', + 'ST-' . $sessionId, + 9999999999, + ], ]; }