From 3cee31eeb033f4e068e3037a55ff6da1d7a447f3 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Mon, 1 Jan 2018 23:14:20 +0100 Subject: [PATCH 1/8] Make CAS parse XML response instead of JSON. --- .../ExternalLogin/CAS/OAuthLoginService.php | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php index 77fe4d381..67f759793 100644 --- a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php +++ b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php @@ -121,7 +121,27 @@ private function validateTicket(string $ticket, string $clientUrl) { if ($res->getStatusCode() === 200) { // the response should be 200 even if the ticket is invalid try { - $data = Json::decode($res->getBody(), Json::FORCE_ARRAY); + $body = $res->getBody(); + + libxml_use_internal_errors(true); + $xml = simplexml_load_string($body); + $err = libxml_get_errors(); + if ($err) { + throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); + } + + // If namespace selection is required ... + $namespaces = $root->getDocNamespaces(); + if ($namespaces) { + $namespace = empty($namespaces['cas']) ? reset($namespaces) : $namespaces['cas']; + $xml = simplexml_load_string($body, "SimpleXMLElement", 0, $namespace); + $err = libxml_get_errors(); + if ($err) { + throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); + } + } + + $data = (array)$xml; } catch (JsonException $e) { throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); } @@ -141,7 +161,7 @@ private function validateTicket(string $ticket, string $clientUrl) { private function getValidationUrl($ticket, $clientUrl) { $service = urlencode($clientUrl); $ticket = urlencode($ticket); - return "{$this->casHttpBaseUri}serviceValidate?service={$service}&ticket={$ticket}&format=json"; + return "{$this->casHttpBaseUri}serviceValidate?service={$service}&ticket={$ticket}&format=xml"; } /** @@ -154,7 +174,7 @@ private function getValidationUrl($ticket, $clientUrl) { */ private function getUserData($ticket, $data): UserData { try { - $info = Arrays::get($data, ["serviceResponse", "authenticationSuccess", "attributes"]); + $info = Arrays::get($data, [/*"serviceResponse",*/ "authenticationSuccess", "attributes"]); } catch (InvalidArgumentException $e) { throw new WrongCredentialsException("The ticket '$ticket' is not valid and does not belong to a CUNI student or staff or it was already used."); } From 97a8dc6d89d8468e4b103eefa00d02e0d11ef2f1 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Mon, 1 Jan 2018 23:27:25 +0100 Subject: [PATCH 2/8] Fixing a bug. --- app/helpers/ExternalLogin/CAS/OAuthLoginService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php index 67f759793..be7afda37 100644 --- a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php +++ b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php @@ -131,7 +131,7 @@ private function validateTicket(string $ticket, string $clientUrl) { } // If namespace selection is required ... - $namespaces = $root->getDocNamespaces(); + $namespaces = $xml->getDocNamespaces(); if ($namespaces) { $namespace = empty($namespaces['cas']) ? reset($namespaces) : $namespaces['cas']; $xml = simplexml_load_string($body, "SimpleXMLElement", 0, $namespace); From 7e7b1d77b24946b5f2928bcc9a4e3f44e3872594 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Mon, 1 Jan 2018 23:39:42 +0100 Subject: [PATCH 3/8] Fixing a bug. --- app/helpers/ExternalLogin/CAS/OAuthLoginService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php index be7afda37..fb6ac18ad 100644 --- a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php +++ b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php @@ -141,7 +141,8 @@ private function validateTicket(string $ticket, string $clientUrl) { } } - $data = (array)$xml; + // A trick that utilizes JSON serialization of SimpleXML objects to convert the XML into an array. + $data = JSON::Decode(JSON::Encode($xml), JSON::FORCE_ARRAY); } catch (JsonException $e) { throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); } From 6e58eabb7804e4b5a2f1ba4585f46f3c284066ce Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Mon, 1 Jan 2018 23:45:17 +0100 Subject: [PATCH 4/8] Fixing a bug. --- app/helpers/ExternalLogin/CAS/OAuthLoginService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php index fb6ac18ad..68dc70153 100644 --- a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php +++ b/app/helpers/ExternalLogin/CAS/OAuthLoginService.php @@ -121,7 +121,7 @@ private function validateTicket(string $ticket, string $clientUrl) { if ($res->getStatusCode() === 200) { // the response should be 200 even if the ticket is invalid try { - $body = $res->getBody(); + $body = (string)$res->getBody(); libxml_use_internal_errors(true); $xml = simplexml_load_string($body); @@ -162,7 +162,7 @@ private function validateTicket(string $ticket, string $clientUrl) { private function getValidationUrl($ticket, $clientUrl) { $service = urlencode($clientUrl); $ticket = urlencode($ticket); - return "{$this->casHttpBaseUri}serviceValidate?service={$service}&ticket={$ticket}&format=xml"; + return "{$this->casHttpBaseUri}p3/serviceValidate?service={$service}&ticket={$ticket}&format=xml"; } /** From b71272fec161e7f8f67db9991cbf1c98dff2e8bf Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Mon, 1 Jan 2018 23:50:09 +0100 Subject: [PATCH 5/8] Rename OAuth to CAS. --- app/config/config.local.neon.example | 2 +- app/config/config.neon | 8 ++++---- .../CAS/{OAuthLoginService.php => CASLoginService.php} | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) rename app/helpers/ExternalLogin/CAS/{OAuthLoginService.php => CASLoginService.php} (98%) diff --git a/app/config/config.local.neon.example b/app/config/config.local.neon.example index efff6212c..8d89a9285 100644 --- a/app/config/config.local.neon.example +++ b/app/config/config.local.neon.example @@ -27,7 +27,7 @@ parameters: options: ldap: hostname: "ldap.cuni.cz" - oauth: + cas: baseUri: "https://idp.cuni.cz/cas/" sis: diff --git a/app/config/config.neon b/app/config/config.neon index 532bcc5f3..b03881c4d 100644 --- a/app/config/config.neon +++ b/app/config/config.neon @@ -59,10 +59,10 @@ parameters: #port: 389 #security: SSL bindName: "cunipersonalid" - oauth: + cas: baseUri: "https://idp.cuni.cz/cas/" fields: - oauth: + cas: ukco: "cunipersonalid" email: "mail" firstName: "givenname" @@ -241,13 +241,13 @@ services: # external login services - App\Helpers\ExternalLogin\CAS\LDAPLoginService(%CAS.serviceId%, %CAS.options.ldap%, %CAS.fields.ldap%) - - App\Helpers\ExternalLogin\CAS\OAuthLoginService(%CAS.serviceId%, %CAS.options.oauth%, %CAS.fields.oauth%) + - App\Helpers\ExternalLogin\CAS\CASLoginService(%CAS.serviceId%, %CAS.options.cas%, %CAS.fields.cas%) - App\Helpers\ExternalLogin\ExternalServiceAuthenticator( @App\Model\Repository\ExternalLogins, @App\Model\Repository\Users, @App\Model\Repository\Logins, @App\Helpers\ExternalLogin\CAS\LDAPLoginService, - @App\Helpers\ExternalLogin\CAS\OAuthLoginService, + @App\Helpers\ExternalLogin\CAS\CASLoginService, ) # config objects diff --git a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php b/app/helpers/ExternalLogin/CAS/CASLoginService.php similarity index 98% rename from app/helpers/ExternalLogin/CAS/OAuthLoginService.php rename to app/helpers/ExternalLogin/CAS/CASLoginService.php index 68dc70153..adb669bc6 100644 --- a/app/helpers/ExternalLogin/CAS/OAuthLoginService.php +++ b/app/helpers/ExternalLogin/CAS/CASLoginService.php @@ -29,7 +29,7 @@ * This is hard to test on a local server, as the CAS will only reveal the sensitive * personal information to computers in the CUNI network. */ -class OAuthLoginService implements IExternalLoginService { +class CASLoginService implements IExternalLoginService { /** @var string Unique identifier of this login service, for example "cas-uk" */ private $serviceId; @@ -41,9 +41,9 @@ class OAuthLoginService implements IExternalLoginService { public function getServiceId(): string { return $this->serviceId; } /** - * @return string The OAuth authentication + * @return string The CAS authentication */ - public function getType(): string { return "oauth"; } + public function getType(): string { return "cas"; } /** @var string Name of JSON field containing user's UKCO */ private $ukcoField; From 43dc14669e2c4e43342a226513eebd3995f38555 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Tue, 2 Jan 2018 13:17:46 +0100 Subject: [PATCH 6/8] Completing CAS login modifications and refactoring. --- .../ExternalLogin/CAS/CASLoginService.php | 53 +++++++++++++------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/app/helpers/ExternalLogin/CAS/CASLoginService.php b/app/helpers/ExternalLogin/CAS/CASLoginService.php index adb669bc6..28f0f5995 100644 --- a/app/helpers/ExternalLogin/CAS/CASLoginService.php +++ b/app/helpers/ExternalLogin/CAS/CASLoginService.php @@ -9,12 +9,13 @@ use App\Exceptions\CASMissingInfoException; use App\Model\Entity\User; -use GuzzleHttp\Psr7\Request; use Nette\InvalidArgumentException; use Nette\Utils\Arrays; use Nette\Utils\Json; use Nette\Utils\JsonException; +use Tracy\ILogger; +use GuzzleHttp\Psr7\Request; use GuzzleHttp\Client; @@ -66,13 +67,18 @@ public function getType(): string { return "cas"; } /** @var string The base URI for the validation of login tickets */ private $casHttpBaseUri; + /** + * @var ILogger + */ + private $logger; + /** * Constructor * @param string $serviceId Identifier of this login service, must be unique * @param array $options * @param array $fields */ - public function __construct(string $serviceId, array $options, array $fields) { + public function __construct(string $serviceId, array $options, array $fields, ILogger $logger) { $this->serviceId = $serviceId; // The field names of user's information stored in the CAS LDAP @@ -85,6 +91,7 @@ public function __construct(string $serviceId, array $options, array $fields) { // The CAS HTTP validation endpoint $this->casHttpBaseUri = Arrays::get($options, "baseUri", "https://idp.cuni.cz/cas/"); + $this->logger = $logger; } /** @@ -106,6 +113,29 @@ public function getUser($credentials): UserData { return $this->getUserData($ticket, $info); } + /** + * Internal XML parsing routine for ticket response. + * @param string $body String representation of the response body. + * @param string $namespace XML namespace URI, if detected. + * @return SimpleXMLObject representing the response body. + * @throws WrongCredentialsException If the XML could not have been parsed. + */ + private function parseXMLBody(string $body, string $namespace = '') + { + libxml_use_internal_errors(true); + $xml = simplexml_load_string($body, 'SimpleXMLElement', 0, $namespace); + $err = libxml_get_errors(); + if ($err) { + $this->logger->log("CAS Ticket validation returned following response:\n$body", ILogger::DEBUG); + foreach ($err as $e) { + // Internal XML errors are logges as warnings + $this->logger->log($e, ILogger::WARNING); + } + throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); + } + return $xml; + } + /** * @param string $ticket * @param string $clientUrl @@ -123,22 +153,12 @@ private function validateTicket(string $ticket, string $clientUrl) { try { $body = (string)$res->getBody(); - libxml_use_internal_errors(true); - $xml = simplexml_load_string($body); - $err = libxml_get_errors(); - if ($err) { - throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); - } - - // If namespace selection is required ... + // Parse XML (twice, if necessary, to get right namespace) ... + $xml = $this->parseXMLBody($body); $namespaces = $xml->getDocNamespaces(); if ($namespaces) { $namespace = empty($namespaces['cas']) ? reset($namespaces) : $namespaces['cas']; - $xml = simplexml_load_string($body, "SimpleXMLElement", 0, $namespace); - $err = libxml_get_errors(); - if ($err) { - throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); - } + $xml = $this->parseXMLBody($body, $namespace); } // A trick that utilizes JSON serialization of SimpleXML objects to convert the XML into an array. @@ -175,8 +195,9 @@ private function getValidationUrl($ticket, $clientUrl) { */ private function getUserData($ticket, $data): UserData { try { - $info = Arrays::get($data, [/*"serviceResponse",*/ "authenticationSuccess", "attributes"]); + $info = Arrays::get($data, ["authenticationSuccess", "attributes"]); } catch (InvalidArgumentException $e) { + $this->logger->log("Ticket validation did not return successful response with attributes:\n" . var_export($data, true), ILogger::ERROR); throw new WrongCredentialsException("The ticket '$ticket' is not valid and does not belong to a CUNI student or staff or it was already used."); } From 5abb45e69ae8358d3f023f3385f6559dc3e2eb54 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Tue, 2 Jan 2018 13:22:20 +0100 Subject: [PATCH 7/8] Final polishing. --- app/helpers/ExternalLogin/CAS/CASLoginService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/ExternalLogin/CAS/CASLoginService.php b/app/helpers/ExternalLogin/CAS/CASLoginService.php index 28f0f5995..3e0dadbb8 100644 --- a/app/helpers/ExternalLogin/CAS/CASLoginService.php +++ b/app/helpers/ExternalLogin/CAS/CASLoginService.php @@ -162,7 +162,7 @@ private function validateTicket(string $ticket, string $clientUrl) { } // A trick that utilizes JSON serialization of SimpleXML objects to convert the XML into an array. - $data = JSON::Decode(JSON::Encode($xml), JSON::FORCE_ARRAY); + $data = JSON::Decode(JSON::Encode((array)$xml), JSON::FORCE_ARRAY); } catch (JsonException $e) { throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); } From 12ee1224b65ace60e18d4330230172d72da970f9 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Tue, 2 Jan 2018 15:46:43 +0100 Subject: [PATCH 8/8] Fixing static analysis errors. --- app/helpers/ExternalLogin/CAS/CASLoginService.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/helpers/ExternalLogin/CAS/CASLoginService.php b/app/helpers/ExternalLogin/CAS/CASLoginService.php index 3e0dadbb8..4bd82b9ff 100644 --- a/app/helpers/ExternalLogin/CAS/CASLoginService.php +++ b/app/helpers/ExternalLogin/CAS/CASLoginService.php @@ -115,12 +115,13 @@ public function getUser($credentials): UserData { /** * Internal XML parsing routine for ticket response. + * @param string $ticket * @param string $body String representation of the response body. * @param string $namespace XML namespace URI, if detected. - * @return SimpleXMLObject representing the response body. + * @return \SimpleXMLElement representing the response body. * @throws WrongCredentialsException If the XML could not have been parsed. */ - private function parseXMLBody(string $body, string $namespace = '') + private function parseXMLBody(string $ticket, string $body, string $namespace = '') { libxml_use_internal_errors(true); $xml = simplexml_load_string($body, 'SimpleXMLElement', 0, $namespace); @@ -154,15 +155,15 @@ private function validateTicket(string $ticket, string $clientUrl) { $body = (string)$res->getBody(); // Parse XML (twice, if necessary, to get right namespace) ... - $xml = $this->parseXMLBody($body); + $xml = $this->parseXMLBody($ticket, $body); $namespaces = $xml->getDocNamespaces(); if ($namespaces) { $namespace = empty($namespaces['cas']) ? reset($namespaces) : $namespaces['cas']; - $xml = $this->parseXMLBody($body, $namespace); + $xml = $this->parseXMLBody($ticket, $body, $namespace); } // A trick that utilizes JSON serialization of SimpleXML objects to convert the XML into an array. - $data = JSON::Decode(JSON::Encode((array)$xml), JSON::FORCE_ARRAY); + $data = JSON::decode(JSON::encode((array)$xml), JSON::FORCE_ARRAY); } catch (JsonException $e) { throw new WrongCredentialsException("The ticket '$ticket' cannot be validated as the response from the server is corrupted or incomplete."); }