From a004c4b1e250ddb489bcfdf044aa739c3d02234c Mon Sep 17 00:00:00 2001 From: D Vargas Date: Fri, 15 Nov 2019 17:44:17 +0100 Subject: [PATCH 1/9] OPENEUROPA-2502: Add param groups to validation path. --- src/Event/EuLoginEventSubscriber.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Event/EuLoginEventSubscriber.php b/src/Event/EuLoginEventSubscriber.php index 1a354c7..7a15580 100644 --- a/src/Event/EuLoginEventSubscriber.php +++ b/src/Event/EuLoginEventSubscriber.php @@ -145,6 +145,7 @@ public function alterValidationPath(CasPreValidateEvent $event): void { 'assuranceLevel' => $config->get('assurance_level'), 'ticketTypes' => $config->get('ticket_types'), 'userDetails' => 'true', + 'groups' => '*', ]; $event->addParameters($params); } From 61d38eea59bcf95915e575d1162d8337ea313cda Mon Sep 17 00:00:00 2001 From: D Vargas Date: Mon, 18 Nov 2019 15:26:40 +0100 Subject: [PATCH 2/9] OPENEUROPA-2502: Fix XML parsing for groups. --- src/CasProcessor.php | 7 ++++++- tests/fixtures/responses/response.xml | 30 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/responses/response.xml diff --git a/src/CasProcessor.php b/src/CasProcessor.php index 6e87c91..b025e03 100644 --- a/src/CasProcessor.php +++ b/src/CasProcessor.php @@ -87,7 +87,12 @@ private static function parseAttributes(\DOMElement $node): array { else { $value = $child->nodeValue; } - $attributes[$name] = $value; + if ($name === 'group') { + $attributes[$name][] = $value; + } + else { + $attributes[$name] = $value; + } } return $attributes; } diff --git a/tests/fixtures/responses/response.xml b/tests/fixtures/responses/response.xml new file mode 100644 index 0000000..d070681 --- /dev/null +++ b/tests/fixtures/responses/response.xml @@ -0,0 +1,30 @@ + + + + chucknorris + DIGIT.A.3.001 + texasranger@chucknorris.com.eu + f + Chuck + NORRIS + eu.europa.ec + chucknorris + 1 + en + 40 + chucknorris + + INTERNET + DG_DIGIT + LIVENEWS + TEXAS_RANGER + + BASIC + + chucknorris + + 2019-11-18T11:37:04.557+01:00 + true + SERVICE + + From d4266a1d0531477c09a8cb01fd995a937dc1bac7 Mon Sep 17 00:00:00 2001 From: upchuk Date: Fri, 6 Dec 2019 11:50:04 +0100 Subject: [PATCH 3/9] OPENEUROPA-2502: Recursevely parse multi-level CAS attributes. --- src/CasProcessor.php | 45 ++++++++++++++++++++++----- tests/fixtures/responses/response.xml | 30 ------------------ 2 files changed, 37 insertions(+), 38 deletions(-) delete mode 100644 tests/fixtures/responses/response.xml diff --git a/src/CasProcessor.php b/src/CasProcessor.php index b025e03..77f236c 100644 --- a/src/CasProcessor.php +++ b/src/CasProcessor.php @@ -72,31 +72,60 @@ public static function processValidationResponseAttributes(string $source): arra * * @param \DOMElement $node * An XML element containing attributes. + * @param bool $sublevel + * Whether the method is called in a recursive loop. * * @return array * An associative array of attributes. */ - private static function parseAttributes(\DOMElement $node): array { + private static function parseAttributes(\DOMElement $node, bool $sublevel = FALSE): array { $attributes = []; // @var \DOMElement $child - foreach ($node->childNodes as $child) { + foreach ($node->childNodes as $key => $child) { $name = $child->localName; + // If the child has sub-levels, recursively parse the attributes + // underneath. if ($child->hasAttribute('number')) { - $value = CasProcessor::parseAttributes($child); + $value = CasProcessor::parseAttributes($child, TRUE); } else { $value = $child->nodeValue; } - if ($name === 'group') { - $attributes[$name][] = $value; - } - else { - $attributes[$name] = $value; + + if ($sublevel) { + // If the sublevel children are keyed by the same key, we cannot make + // it an associated array so we have to key numerically. + $associative = CasProcessor::isAssociative($node); + $sublevel_name = $associative ? $name : $key; + $attributes[$sublevel_name] = $value; + continue; } + + $attributes[$name] = $value; } return $attributes; } + /** + * Checks whether the node children can be represented as an associated array. + * + * @param \DOMElement $node + * The node element. + * + * @return bool + * Whether the node children should be mapped to an associated array. + */ + protected static function isAssociative(\DOMElement $node): bool { + $names = []; + foreach ($node->childNodes as $key => $child) { + $names[] = $child->localName; + } + + // We consider it as associative if we have only one name value or if the + // name values are different. + return count($names) < 2 || count(array_unique($names)) > 1; + } + /** * Check whether the validation response is valid or not. * diff --git a/tests/fixtures/responses/response.xml b/tests/fixtures/responses/response.xml deleted file mode 100644 index d070681..0000000 --- a/tests/fixtures/responses/response.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - chucknorris - DIGIT.A.3.001 - texasranger@chucknorris.com.eu - f - Chuck - NORRIS - eu.europa.ec - chucknorris - 1 - en - 40 - chucknorris - - INTERNET - DG_DIGIT - LIVENEWS - TEXAS_RANGER - - BASIC - - chucknorris - - 2019-11-18T11:37:04.557+01:00 - true - SERVICE - - From f0f24db21810cb9c3976634a2eb324e3dc27b9d2 Mon Sep 17 00:00:00 2001 From: D Vargas Date: Tue, 10 Dec 2019 16:26:25 +0100 Subject: [PATCH 4/9] OPENEUROPA-2502: Improve isAssociative logic in CasProcessor. --- src/CasProcessor.php | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/CasProcessor.php b/src/CasProcessor.php index 77f236c..ca8a71a 100644 --- a/src/CasProcessor.php +++ b/src/CasProcessor.php @@ -72,13 +72,18 @@ public static function processValidationResponseAttributes(string $source): arra * * @param \DOMElement $node * An XML element containing attributes. - * @param bool $sublevel - * Whether the method is called in a recursive loop. + * @param bool $toplevel + * Whether the method is called from out of the recursive loop. * * @return array - * An associative array of attributes. + * An array of attributes. */ - private static function parseAttributes(\DOMElement $node, bool $sublevel = FALSE): array { + private static function parseAttributes(\DOMElement $node, bool $toplevel = TRUE): array { + + // Check if we can return an associative array or if + // we must use numeric keys. + $associative = $toplevel || CasProcessor::isAssociative($node); + $attributes = []; // @var \DOMElement $child foreach ($node->childNodes as $key => $child) { @@ -86,28 +91,27 @@ private static function parseAttributes(\DOMElement $node, bool $sublevel = FALS // If the child has sub-levels, recursively parse the attributes // underneath. if ($child->hasAttribute('number')) { - $value = CasProcessor::parseAttributes($child, TRUE); + $value = CasProcessor::parseAttributes($child, FALSE); } else { $value = $child->nodeValue; } - if ($sublevel) { - // If the sublevel children are keyed by the same key, we cannot make - // it an associated array so we have to key numerically. - $associative = CasProcessor::isAssociative($node); - $sublevel_name = $associative ? $name : $key; - $attributes[$sublevel_name] = $value; - continue; + if ($associative) { + $attributes[$name] = $value; + } + else { + $attributes[] = $value; } - $attributes[$name] = $value; } return $attributes; } /** - * Checks whether the node children can be represented as an associated array. + * Checks if the node children can be represented as an associative array. + * + * Array can be associative if it will get different names for all keys. * * @param \DOMElement $node * The node element. @@ -116,14 +120,11 @@ private static function parseAttributes(\DOMElement $node, bool $sublevel = FALS * Whether the node children should be mapped to an associated array. */ protected static function isAssociative(\DOMElement $node): bool { - $names = []; - foreach ($node->childNodes as $key => $child) { - $names[] = $child->localName; + $names = $counter = []; + foreach ($node->childNodes as $child) { + $names[$child->localName] = $counter[] = $child->localName; } - - // We consider it as associative if we have only one name value or if the - // name values are different. - return count($names) < 2 || count(array_unique($names)) > 1; + return count($names) === count($counter); } /** From f726d6c18c3597df837eb03415fa9c081b4f7124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hern=C3=A2ni?= Date: Thu, 12 Dec 2019 14:47:14 +0000 Subject: [PATCH 5/9] OPENEUROPA-2502: Add new event subscriber. --- oe_authentication.services.yml | 7 +- src/Event/EuLoginEventSubscriber.php | 7 +- src/Event/MessengerEuLoginEventSubscriber.php | 75 +++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 src/Event/MessengerEuLoginEventSubscriber.php diff --git a/oe_authentication.services.yml b/oe_authentication.services.yml index a003bb9..9fdeb39 100644 --- a/oe_authentication.services.yml +++ b/oe_authentication.services.yml @@ -12,4 +12,9 @@ services: class: \Drupal\oe_authentication\Event\EuLoginEventSubscriber tags: - { name: event_subscriber } - arguments: ['@config.factory', '@messenger'] + arguments: ['@config.factory'] + oe_authentication.messenger.event_subscriber: + class: \Drupal\oe_authentication\Event\MessengerEuLoginEventSubscriber + tags: + - { name: event_subscriber } + arguments: ['@messenger'] \ No newline at end of file diff --git a/src/Event/EuLoginEventSubscriber.php b/src/Event/EuLoginEventSubscriber.php index 7a15580..b5def25 100644 --- a/src/Event/EuLoginEventSubscriber.php +++ b/src/Event/EuLoginEventSubscriber.php @@ -10,7 +10,6 @@ use Drupal\cas\Event\CasPreValidateEvent; use Drupal\cas\Service\CasHelper; use Drupal\Core\Config\ConfigFactoryInterface; -use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -45,12 +44,9 @@ class EuLoginEventSubscriber implements EventSubscriberInterface { * * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory * The config factory. - * @param \Drupal\Core\Messenger\MessengerInterface $messenger - * The messenger. */ - public function __construct(ConfigFactoryInterface $configFactory, MessengerInterface $messenger) { + public function __construct(ConfigFactoryInterface $configFactory) { $this->configFactory = $configFactory; - $this->messenger = $messenger; } /** @@ -111,7 +107,6 @@ public function processUserProperties(CasPreRegisterEvent $event): void { $user_settings = $this->configFactory->get('user.settings'); if ($user_settings->get('register') === USER_REGISTER_VISITORS_ADMINISTRATIVE_APPROVAL) { $event->setPropertyValue('status', 0); - $this->messenger->addStatus($this->t('Thank you for applying for an account. Your account is currently pending approval by the site administrator.')); } } diff --git a/src/Event/MessengerEuLoginEventSubscriber.php b/src/Event/MessengerEuLoginEventSubscriber.php new file mode 100644 index 0000000..e1b4d2c --- /dev/null +++ b/src/Event/MessengerEuLoginEventSubscriber.php @@ -0,0 +1,75 @@ +messenger = $messenger; + } + + /** + * Returns an array of event names this subscriber wants to listen to. + * + * @return array + * The event names to listen to. + */ + public static function getSubscribedEvents(): array { + $events = []; + $events[CasHelper::EVENT_PRE_REGISTER] = ['showUserMessage', -100]; + return $events; + } + + /** + * Show user message about its activation status. + * + * @param \Drupal\cas\Event\CasPreRegisterEvent $event + * The triggered event. + */ + public function showUserMessage(CasPreRegisterEvent $event): void { + + $properties = $event->getPropertyValues(); + + if (!isset($properties['status'])) { + return; + } + + if ($properties['status']) { + $this->messenger->addStatus($this->t('Thank you for applying for an account. Your account is ready to be used.')); + } + else { + $this->messenger->addStatus($this->t('Thank you for applying for an account. Your account is currently pending approval by the site administrator.')); + } + + } + +} From 5864939846802e747a41c6751c1f3507831ea146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hern=C3=A2ni?= Date: Thu, 12 Dec 2019 14:55:49 +0000 Subject: [PATCH 6/9] OPENEUROPA-2502: Change logic for messages. --- src/Event/MessengerEuLoginEventSubscriber.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Event/MessengerEuLoginEventSubscriber.php b/src/Event/MessengerEuLoginEventSubscriber.php index e1b4d2c..13080e1 100644 --- a/src/Event/MessengerEuLoginEventSubscriber.php +++ b/src/Event/MessengerEuLoginEventSubscriber.php @@ -63,10 +63,7 @@ public function showUserMessage(CasPreRegisterEvent $event): void { return; } - if ($properties['status']) { - $this->messenger->addStatus($this->t('Thank you for applying for an account. Your account is ready to be used.')); - } - else { + if (!$properties['status']) { $this->messenger->addStatus($this->t('Thank you for applying for an account. Your account is currently pending approval by the site administrator.')); } From 1bd30e6556380e1ec24ded355e6a75ff7e5490df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hern=C3=A2ni?= Date: Thu, 12 Dec 2019 16:22:03 +0000 Subject: [PATCH 7/9] OPENEUROPA-2502: Fix typo. --- src/Event/MessengerEuLoginEventSubscriber.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Event/MessengerEuLoginEventSubscriber.php b/src/Event/MessengerEuLoginEventSubscriber.php index 13080e1..8aeaf4d 100644 --- a/src/Event/MessengerEuLoginEventSubscriber.php +++ b/src/Event/MessengerEuLoginEventSubscriber.php @@ -13,8 +13,8 @@ /** * Event subscriber for CAS module events. * - * The class subscribes to the events provided by the CAS module shows messages - * accordingly. + * The class subscribes to the events provided by the CAS module and shows + * messages accordingly. */ class MessengerEuLoginEventSubscriber implements EventSubscriberInterface { From cacabc54a0634702ee30740fc80948b9741ceda5 Mon Sep 17 00:00:00 2001 From: Francesco Sardara Date: Thu, 12 Dec 2019 17:43:00 +0100 Subject: [PATCH 8/9] OPENEUROPA-2502: Use static:: instead of class name to invoke its own static methods. --- src/CasProcessor.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/CasProcessor.php b/src/CasProcessor.php index ca8a71a..ea1af15 100644 --- a/src/CasProcessor.php +++ b/src/CasProcessor.php @@ -49,7 +49,7 @@ public static function convertCasAttributesToFieldValues(array $attributes): arr * An array containing the parsed attributes. */ public static function processValidationResponseAttributes(string $source): array { - if (!CasProcessor::isValidResponse($source)) { + if (!static::isValidResponse($source)) { throw new \InvalidArgumentException(); } // Load cas attributes. @@ -63,7 +63,7 @@ public static function processValidationResponseAttributes(string $source): arra $success_element = $success_elements->item(0); // Parse the attributes coming from Eu Login // and add them to the default ones. - $eulogin_attributes = CasProcessor::parseAttributes($success_element); + $eulogin_attributes = static::parseAttributes($success_element); return $eulogin_attributes; } @@ -78,11 +78,10 @@ public static function processValidationResponseAttributes(string $source): arra * @return array * An array of attributes. */ - private static function parseAttributes(\DOMElement $node, bool $toplevel = TRUE): array { - + protected static function parseAttributes(\DOMElement $node, bool $toplevel = TRUE): array { // Check if we can return an associative array or if // we must use numeric keys. - $associative = $toplevel || CasProcessor::isAssociative($node); + $associative = $toplevel || static::isAssociative($node); $attributes = []; // @var \DOMElement $child @@ -91,7 +90,7 @@ private static function parseAttributes(\DOMElement $node, bool $toplevel = TRUE // If the child has sub-levels, recursively parse the attributes // underneath. if ($child->hasAttribute('number')) { - $value = CasProcessor::parseAttributes($child, FALSE); + $value = static::parseAttributes($child, FALSE); } else { $value = $child->nodeValue; From 3316894e0700fd7f13e06487854d9a2eb953ef28 Mon Sep 17 00:00:00 2001 From: Francesco Sardara Date: Thu, 12 Dec 2019 17:43:12 +0100 Subject: [PATCH 9/9] OPENEUROPA-2502: Minor coding standards fixes. --- oe_authentication.services.yml | 2 +- src/CasProcessor.php | 8 ++++---- src/Event/MessengerEuLoginEventSubscriber.php | 8 +++----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/oe_authentication.services.yml b/oe_authentication.services.yml index 9fdeb39..1c84676 100644 --- a/oe_authentication.services.yml +++ b/oe_authentication.services.yml @@ -17,4 +17,4 @@ services: class: \Drupal\oe_authentication\Event\MessengerEuLoginEventSubscriber tags: - { name: event_subscriber } - arguments: ['@messenger'] \ No newline at end of file + arguments: ['@messenger'] diff --git a/src/CasProcessor.php b/src/CasProcessor.php index ea1af15..e83e02b 100644 --- a/src/CasProcessor.php +++ b/src/CasProcessor.php @@ -55,9 +55,9 @@ public static function processValidationResponseAttributes(string $source): arra // Load cas attributes. $dom = new \DOMDocument(); $dom->preserveWhiteSpace = FALSE; - $dom->encoding = "utf-8"; + $dom->encoding = 'utf-8'; @$dom->loadXML($source); - $success_elements = $dom->getElementsByTagName("authenticationSuccess"); + $success_elements = $dom->getElementsByTagName('authenticationSuccess'); // There should only be one success element, grab it and extract username. $success_element = $success_elements->item(0); @@ -138,7 +138,7 @@ protected static function isAssociative(\DOMElement $node): bool { public static function isValidResponse(string $response) { $dom = new \DOMDocument(); $dom->preserveWhiteSpace = FALSE; - $dom->encoding = "utf-8"; + $dom->encoding = 'utf-8'; // Suppress errors from this function, as we intend to allow other // event subscribers to work on the data. @@ -146,7 +146,7 @@ public static function isValidResponse(string $response) { return FALSE; } - $success_elements = $dom->getElementsByTagName("authenticationSuccess"); + $success_elements = $dom->getElementsByTagName('authenticationSuccess'); return $success_elements->length !== 0; } diff --git a/src/Event/MessengerEuLoginEventSubscriber.php b/src/Event/MessengerEuLoginEventSubscriber.php index 8aeaf4d..cda471e 100644 --- a/src/Event/MessengerEuLoginEventSubscriber.php +++ b/src/Event/MessengerEuLoginEventSubscriber.php @@ -21,17 +21,17 @@ class MessengerEuLoginEventSubscriber implements EventSubscriberInterface { use StringTranslationTrait; /** - * Stores a Messenger object. + * The messenger service. * * @var \Drupal\Core\Messenger\MessengerInterface */ protected $messenger; /** - * Constructors the MessengerEuLoginEventSubscriber. + * Constructs the MessengerEuLoginEventSubscriber. * * @param \Drupal\Core\Messenger\MessengerInterface $messenger - * The messenger. + * The messenger service. */ public function __construct(MessengerInterface $messenger) { $this->messenger = $messenger; @@ -56,7 +56,6 @@ public static function getSubscribedEvents(): array { * The triggered event. */ public function showUserMessage(CasPreRegisterEvent $event): void { - $properties = $event->getPropertyValues(); if (!isset($properties['status'])) { @@ -66,7 +65,6 @@ public function showUserMessage(CasPreRegisterEvent $event): void { if (!$properties['status']) { $this->messenger->addStatus($this->t('Thank you for applying for an account. Your account is currently pending approval by the site administrator.')); } - } }