Merge pull request #253 from dBucik/fixes

Fixes

Author: Dominik František Bučík

config-templates/processFilterConfigurations-example.md

@@ -166,7 +166,7 @@ Example how to enable filter AttributeMap:
 Filter is intended to extract an attribute specified by set of keys forming the chain of keys in the `$request` variable into the configured destination attribute.
 
 Configuration options:
-* `attr_name`: specifies attribute name, into which the extracted value will be stored
+* `destination_attr_name`: specifies attribute name, into which the extracted value will be stored
 * `request_keys`: string, which contains a semicolon (`;`) separated chain of keys that are examined in the state. Numeric keys are automatically treated as array indexes. For instance, value `'saml:AuthenticatingAuthority;0'` will be treated as code `$request['saml:AuthenticatingAuthority'][0]`. In case of this value being empty, exception is thrown. Otherwise, extracted value is stored into the configured destination attribute.
 * `fail_on_nonexisting_keys`: `true` or `false`, specifies if in case of missing key in the request variable the filter should terminate with an exception or not
 * `default_value`: array, which will be set as default value, if the configured keys did not lead to value
@@ -175,10 +175,10 @@ Configuration options:
 // EXTRACT AUTHENTICATING ENTITY INTO authenticating_idp attribute
 1 => [ 
     'class' => 'perun:ExtractRequestAttribute',
-    'attr_name' => 'authenticating_idp',
+    'destination_attr_name' => 'authenticating_idp',
     'request_keys' => 'saml:AuthenticatingAuthority;0',
     'fail_on_nonexisting_keys' => 'true',
-    'default_value' => null,
+    'default_value' => [],
 ],
 ```
 

lib/Auth/Process/ExtractRequestAttribute.php

@@ -24,7 +24,7 @@ class ExtractRequestAttribute extends ProcessingFilter
     public const DEFAULT_VALUE = 'default_value';
 
     public const KEYS_SEPARATOR = ';';
-    public const FAILURE_VALUE = '%$FAILURE_VALUE$%';
+    public const FAILURE_VALUE = ['%$FAILURE_VALUE$%'];
 
     private $destinationAttrName;
     private $requestKeys;
@@ -38,7 +38,7 @@ public function __construct($config, $reserved)
         $this->filterConfig = Configuration::loadFromArray($config);
 
         $this->destinationAttrName = $this->filterConfig->getString(self::DESTINATION_ATTRIBUTE_NAME, null);
-        if (empty($this->requestKeys)) {
+        if (empty($this->destinationAttrName)) {
             throw new Exception(
                 self::DEBUG_PREFIX . 'missing mandatory configuration for option \'' . self::DESTINATION_ATTRIBUTE_NAME . '\''
             );
@@ -75,9 +75,7 @@ public function process(&$request)
             }
             if (!array_key_exists($key, $value)) {
                 Logger::warning(
-                    self::DEBUG_PREFIX . 'Cannot find key \'' .
-                    $key . '\' in the supposed path towards the value. Did you configure the right path of keys ' .
-                    'to extract it?'
+                    self::DEBUG_PREFIX . 'Cannot find key \'' . $key . '\' in the supposed path towards the value. Did you configure the right path of keys to extract it?'
                 );
                 if ($this->failOnNonExistingKey) {
                     throw new Exception(self::DEBUG_PREFIX . 'Specified chain of keys does not exist');
@@ -99,9 +97,9 @@ public function process(&$request)
             $value = [$value];
         }
         $request[PerunConstants::ATTRIBUTES][$this->destinationAttrName] = $value;
+        $logValue = implode(',', $value);
         Logger::debug(
-            self::DEBUG_PREFIX . 'Value \'' . implode(',', $value)
-            . '\' has been extracted and set to attribute ' . $this->destinationAttrName
+            self::DEBUG_PREFIX . 'Value \'' . $logValue . '\' has been extracted and set to attribute ' . $this->destinationAttrName
         );
     }
 }

lib/Auth/Process/PerunUser.php

@@ -60,21 +60,21 @@ public function __construct($config, $reserved)
         $this->userIdAttrs = $this->filterConfig->getArray(self::UID_ATTRS, []);
         if (empty($this->userIdAttrs)) {
             throw new Exception(
-                self::DEBUG_PREFIX . 'Invalid configuration: no attributes configured for ' . 'extracting UID. Use option \'' . self::UID_ATTRS . '\' to configure list of attributes, ' . 'that should be considered as IDs for a user'
+                self::DEBUG_PREFIX . 'Invalid configuration: no attributes configured for extracting UID. Use option \'' . self::UID_ATTRS . '\' to configure list of attributes, that should be considered as IDs for a user'
             );
         }
         $this->idpEntityIdAttr = $this->filterConfig->getString(self::IDP_ID_ATTR, null);
         if (empty($this->idpEntityIdAttr)) {
             throw new Exception(
-                self::DEBUG_PREFIX . 'Invalid configuration: no attribute containing IDP ' . 'ID has been configured. Use option \'' . self::IDP_ID_ATTR . '\' to configure the name of the ' . 'attribute, that has been previously used in the configuration of filter \'perun:ExtractIdpEntityId\''
+                self::DEBUG_PREFIX . 'Invalid configuration: no attribute containing IDP ID has been configured. Use option \'' . self::IDP_ID_ATTR . '\' to configure the name of the attribute, that has been previously used in the configuration of filter \'perun:ExtractIdpEntityId\''
             );
         }
         $this->registerUrl = $this->filterConfig->getString(self::REGISTER_URL, null);
         $this->callbackParameterName = $this->filterConfig->getString(self::CALLBACK_PARAMETER_NAME, null);
         $this->perunRegisterUrl = $this->filterConfig->getString(self::PERUN_REGISTER_URL, null);
         if (empty($this->registerUrl) && empty($this->callbackParameterName) && empty($this->perunRegisterUrl)) {
             throw new Exception(
-                self::DEBUG_PREFIX . 'Invalid configuration: no URL where user should register for the ' . 'account has been configured. Use option \'' . self::REGISTER_URL . '\' to configure the URL and ' . 'option \'' . self::CALLBACK_PARAMETER_NAME . '\' to configure name of the callback parameter. 
+                self::DEBUG_PREFIX . 'Invalid configuration: no URL where user should register for the account has been configured. Use option \'' . self::REGISTER_URL . '\' to configure the URL and option \'' . self::CALLBACK_PARAMETER_NAME . '\' to configure name of the callback parameter. 
                 . If you wish to use the Perun registrar, use the option \'' . self::PERUN_REGISTER_URL . '\'.'
             );
         }
@@ -92,10 +92,10 @@ public function process(&$request)
             }
         }
         if (empty($uids)) {
-            throw new Exception(self::DEBUG_PREFIX . 'missing at least one of mandatory attributes [' . implode(
-                ', ',
-                $this->userIdAttrs
-            ) . '] in request.');
+            $serializedUids = implode(', ', $this->userIdAttrs);
+            throw new Exception(
+                self::DEBUG_PREFIX . 'missing at least one of mandatory attributes [' . $serializedUids . '] in request.'
+            );
         }
 
         if (!empty($request[PerunConstants::ATTRIBUTES][$this->idpEntityIdAttr][0])) {
@@ -109,13 +109,10 @@ public function process(&$request)
         $user = $this->adapter->getPerunUser($idpEntityId, $uids);
 
         if (!empty($user)) {
-            Logger::debug(self::DEBUG_PREFIX . 'User identified, setting Perun user into request.');
             $this->processUser($request, $user, $uids);
-
-            return;
+        } else {
+            $this->register($request, $uids);
         }
-        Logger::debug(self::DEBUG_PREFIX . 'User not identified, redirecting to registration.');
-        $this->register($request, $uids);
     }
 
     private function processUser(array &$request, User $user, array $uids): void
@@ -126,9 +123,9 @@ private function processUser(array &$request, User $user, array $uids): void
 
         $request[PerunConstants::PERUN][PerunConstants::USER] = $user;
 
+        $logUids = implode(', ', $uids);
         Logger::info(
-            self::DEBUG_PREFIX . 'Perun user with identity/ies: ' . implode(',', $uids)
-            . ' has been found. Setting user ' . $user->getName() . ' with id: ' . $user->getId() . ' to the request.'
+            self::DEBUG_PREFIX . 'Perun user with identity/ies: \'' . $logUids . '\' has been found. Setting user ' . $user->getName() . ' with id: ' . $user->getId() . ' to the request.'
         );
     }
 
@@ -140,35 +137,34 @@ private function register(array &$request, array $uids): void
             self::PARAM_STATE_ID => $stateId,
         ]);
         Logger::debug(self::DEBUG_PREFIX . 'Produced callback URL \'' . $callback . '\'');
+        $url = '';
+        $params = [];
 
         if (!empty($this->registerUrl) && !empty($this->callbackParameterName)) {
+            $url = $this->registerUrl;
+            $params[$this->callbackParameterName] = $callback;
             Logger::debug(
-                self::DEBUG_PREFIX . 'Redirecting to \'' . $this->registerUrl . ', callback parameter \''
-                . $this->callbackParameterName . '\' with value \'' . $callback . '\''
+                self::DEBUG_PREFIX . 'Redirecting to \'' . $this->registerUrl . ', callback parameter \'' . $this->callbackParameterName . '\' set to value \'' . $callback . '\'.'
             );
-            HTTP::redirectTrustedURL($this->registerUrl, [
-                $this->callbackParameterName => $callback,
-            ]);
         } elseif (!empty($this->perunRegisterUrl)) {
-            $params[PerunConstants::TARGET_NEW] = $callback;
-            $params[PerunConstants::TARGET_EXISTING] = $callback;
-            $params[PerunConstants::TARGET_EXTENDED] = $callback;
+            $perunParams[PerunConstants::TARGET_NEW] = $callback;
+            $perunParams[PerunConstants::TARGET_EXISTING] = $callback;
+            $perunParams[PerunConstants::TARGET_EXTENDED] = $callback;
+            $registrationUrl = HTTP::addURLParameters($this->perunRegisterUrl, $perunParams);
 
             $url = Module::getModuleURL(self::REDIRECT);
-            $registrationUrl = HTTP::addURLParameters($this->perunRegisterUrl, $params);
+            $params[self::PARAM_REGISTRATION_URL] = $registrationUrl;
             Logger::debug(
-                self::DEBUG_PREFIX . 'Redirecting to \'' . self::REDIRECT . ', registration URL \''
-                . $registrationUrl . '\''
+                self::DEBUG_PREFIX . 'Redirecting to \'' . self::REDIRECT . ', param registration URL \'' . $registrationUrl . '\'.'
             );
-            HTTP::redirectTrustedURL($url, [
-                self::PARAM_REGISTRATION_URL => $registrationUrl,
-            ]);
         } else {
-            throw new Exception(self::DEBUG_PREFIX . 'No configuration for registration enabled. Cannot proceed');
+            throw new Exception(self::DEBUG_PREFIX . 'No configuration for registration set. Cannot proceed.');
         }
+
+        HTTP::redirectTrustedURL($url, $params);
+        $logUids = implode(', ', $uids);
         Logger::info(
-            self::DEBUG_PREFIX . 'Perun user with identity/ies: ' . implode(',', $uids) .
-            ' has not been found. Redirecting to registration.'
+            self::DEBUG_PREFIX . 'Perun user with identity/ies: \'' . $logUids . '\' has not been found. User has been redirected to registration.'
         );
     }
 }