From 19e0d4af51883be17a70dcf08c81119db9b6ed80 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Fri, 1 Feb 2019 18:30:21 +0100
Subject: [PATCH] Fixes for lib/SimpleSAML/Auth

---
 lib/SimpleSAML/Auth/Default.php          | 23 +++++++++++
 lib/SimpleSAML/Auth/LDAP.php             | 52 ++++++++++++++++--------
 lib/SimpleSAML/Auth/ProcessingChain.php  | 11 ++++-
 lib/SimpleSAML/Auth/Simple.php           |  6 ++-
 lib/SimpleSAML/Auth/Source.php           |  3 +-
 lib/SimpleSAML/Auth/TimeLimitedToken.php |  6 ++-
 6 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/lib/SimpleSAML/Auth/Default.php b/lib/SimpleSAML/Auth/Default.php
index 86c7465cd..eb6ef0479 100644
--- a/lib/SimpleSAML/Auth/Default.php
+++ b/lib/SimpleSAML/Auth/Default.php
@@ -18,6 +18,11 @@ class DefaultAuth
 {
     /**
      * @deprecated This method will be removed in SSP 2.0. Use Source::initLogin() instead.
+     * @param string $authId
+     * @param string $return
+     * @param string|null $errorURL
+     * @param array $params
+     * @return void
      */
     public static function initLogin(
         $authId,
@@ -34,6 +39,8 @@ class DefaultAuth
     /**
      * @deprecated This method will be removed in SSP 2.0. Please use
      * State::getPersistentAuthData() instead.
+     * @param array &$state
+     * @return array
      */
     public static function extractPersistentAuthState(array &$state)
     {
@@ -43,6 +50,8 @@ class DefaultAuth
 
     /**
      * @deprecated This method will be removed in SSP 2.0. Please use Source::loginCompleted() instead.
+     * @param array $state
+     * @return void
      */
     public static function loginCompleted($state)
     {
@@ -52,6 +61,9 @@ class DefaultAuth
 
     /**
      * @deprecated This method will be removed in SSP 2.0.
+     * @param string $returnURL
+     * @param string $authority
+     * @return void
      */
     public static function initLogoutReturn($returnURL, $authority)
     {
@@ -78,6 +90,9 @@ class DefaultAuth
 
     /**
      * @deprecated This method will be removed in SSP 2.0.
+     * @param string $returnURL
+     * @param string $authority
+     * @return void
      */
     public static function initLogout($returnURL, $authority)
     {
@@ -92,6 +107,8 @@ class DefaultAuth
 
     /**
      * @deprecated This method will be removed in SSP 2.0.
+     * @param array $state
+     * @return void
      */
     public static function logoutCompleted($state)
     {
@@ -104,6 +121,8 @@ class DefaultAuth
 
     /**
      * @deprecated This method will be removed in SSP 2.0. Please use Source::logoutCallback() instead.
+     * @param array $state
+     * @return void
      */
     public static function logoutCallback($state)
     {
@@ -114,6 +133,10 @@ class DefaultAuth
     /**
      * @deprecated This method will be removed in SSP 2.0. Please use
      * \SimpleSAML\Module\saml\Auth\Source\SP::handleUnsolicitedAuth() instead.
+     * @param string $authId
+     * @param array $state
+     * @param string $redirectTo
+     * @return void
      */
     public static function handleUnsolicitedAuth($authId, array $state, $redirectTo)
     {
diff --git a/lib/SimpleSAML/Auth/LDAP.php b/lib/SimpleSAML/Auth/LDAP.php
index c83d221f9..47e90856a 100644
--- a/lib/SimpleSAML/Auth/LDAP.php
+++ b/lib/SimpleSAML/Auth/LDAP.php
@@ -34,12 +34,14 @@ class LDAP
     /**
      * LDAP link identifier.
      *
-     * @var resource
+     * @var resource|null
      */
     protected $ldap = null;
 
     /**
      * LDAP user: authz_id if SASL is in use, binding dn otherwise
+     *
+     * @var string|null
      */
     protected $authz_id = null;
 
@@ -59,6 +61,7 @@ class LDAP
      * @param int $timeout
      * @param int $port
      * @param bool $referrals
+     * @psalm-suppress NullArgument
      */
     public function __construct(
         $hostname,
@@ -145,8 +148,8 @@ class LDAP
      * Convenience method to create an LDAPException as well as log the
      * description.
      *
-     * @param string $description
-     * The exception's description
+     * @param string $description The exception's description
+     * @param int|null $type The exception's type
      * @return \Exception
      */
     private function makeException($description, $type = null)
@@ -235,6 +238,7 @@ class LDAP
      * - Failed to get DN for entry
      * @throws Error\UserNotFound if:
      * - Zero entries were found
+     * @psalm-suppress TypeDoesNotContainType
      */
     private function search($base, $attribute, $value, $searchFilter = null, $scope = "subtree")
     {
@@ -321,7 +325,7 @@ class LDAP
      * Additional searchFilter to be added to the (attribute=value) filter
      * @param string $scope
      * The scope of the search
-     * @return string
+     * @return string|null
      * The DN of the matching element, if found. If no element was found and
      * $allowZeroHits is set to FALSE, an exception will be thrown; otherwise
      * NULL will be returned.
@@ -420,6 +424,8 @@ class LDAP
             throw $this->makeException('ldap:LdapConnection->search_manual : No base DNs were passed', ERR_INTERNAL);
         }
 
+        $attributes = \SimpleSAML\Utils\Arrays::arrayize($attributes);
+
         // Search each base until result is found
         $result = false;
         foreach ($bases as $base) {
@@ -560,8 +566,8 @@ class LDAP
      * Applies an LDAP option to the current connection.
      *
      * @throws Exception
-     * @param $option
-     * @param $value
+     * @param mixed $option
+     * @param mixed $value
      * @return void
      */
     public function setOption($option, $value)
@@ -614,6 +620,7 @@ class LDAP
 
         // Attempt to get attributes
         // TODO: Should aliases be dereferenced?
+        /** @var array $attributes */
         $result = @ldap_read($this->ldap, $dn, 'objectClass=*', $attributes, 0, 0, $this->timeout);
         if ($result === false) {
             throw $this->makeException('Library - LDAP getAttributes(): Failed to get attributes from DN \''.$dn.'\'');
@@ -726,6 +733,7 @@ class LDAP
      *
      * @static
      * @param string|array $values Array of values to escape
+     * @param bool $singleValue
      * @return array Array $values, but escaped
      */
     public static function escape_filter_value($values = [], $singleValue = true)
@@ -734,20 +742,20 @@ class LDAP
         $values = \SimpleSAML\Utils\Arrays::arrayize($values);
 
         foreach ($values as $key => $val) {
-            // Escaping of filter meta characters
-            $val = str_replace('\\', '\5c', $val);
-            $val = str_replace('*', '\2a', $val);
-            $val = str_replace('(', '\28', $val);
-            $val = str_replace(')', '\29', $val);
-
-            // ASCII < 32 escaping
-            $val = self::asc2hex32($val);
-
-            if (null === $val) {
+            if ($val === null) {
                 $val = '\0'; // apply escaped "null" if string is empty
+            } else {
+                // Escaping of filter meta characters
+                $val = str_replace('\\', '\5c', $val);
+                $val = str_replace('*', '\2a', $val);
+                $val = str_replace('(', '\28', $val);
+                $val = str_replace(')', '\29', $val);
+
+                // ASCII < 32 escaping
+                $val = self::asc2hex32($val);
             }
 
-            $values[$key] = $val;
+            $values[$key] = $values;
         }
         if ($singleValue) {
             return $values[0];
@@ -783,6 +791,11 @@ class LDAP
 
     /**
      * Convert SASL authz_id into a DN
+     *
+     * @param string $searchBase
+     * @param array $searchAttributes
+     * @param string $authz_id
+     * @return string|null
      */
     private function authzidToDn($searchBase, $searchAttributes, $authz_id)
     {
@@ -811,6 +824,11 @@ class LDAP
      * When it was integrated into PHP repository, the function prototype
      * was changed, The new prototype was used in third party patch for
      * PHP 7.0 and 7.1, hence the version test below.
+     *
+     * @param string $searchBase
+     * @param array $searchAttributes
+     * @throws \Exception
+     * @return string|null
      */
     public function whoami($searchBase, $searchAttributes)
     {
diff --git a/lib/SimpleSAML/Auth/ProcessingChain.php b/lib/SimpleSAML/Auth/ProcessingChain.php
index a82d51334..e2ba6aa07 100644
--- a/lib/SimpleSAML/Auth/ProcessingChain.php
+++ b/lib/SimpleSAML/Auth/ProcessingChain.php
@@ -46,6 +46,7 @@ class ProcessingChain
      *
      * @param array $idpMetadata  The metadata for the IdP.
      * @param array $spMetadata  The metadata for the SP.
+     * @param string $mode
      */
     public function __construct($idpMetadata, $spMetadata, $mode = 'idp')
     {
@@ -84,6 +85,7 @@ class ProcessingChain
      *
      * @param array &$target  Target filter list. This list must be sorted.
      * @param array $src  Source filters. May be unsorted.
+     * @return void
      */
     private static function addFilters(&$target, $src)
     {
@@ -181,6 +183,9 @@ class ProcessingChain
      * @see State::EXCEPTION_HANDLER_FUNC
      *
      * @param array &$state  The state we are processing.
+     * @throws \SimpleSAML\Error\Exception
+     * @throws \SimpleSAML\Error\UnserializableException
+     * @return void
      */
     public function processState(&$state)
     {
@@ -226,6 +231,7 @@ class ProcessingChain
      * to whatever exception handler is defined in the state array.
      *
      * @param array $state  The state we are processing.
+     * @return void
      */
     public static function resumeProcessing($state)
     {
@@ -280,6 +286,7 @@ class ProcessingChain
      * This function will only return if processing completes.
      *
      * @param array &$state  The state we are processing.
+     * @return void
      */
     public function processStatePassive(&$state)
     {
@@ -316,7 +323,7 @@ class ProcessingChain
      *
      * @param string $id The state identifier.
      * @see State::parseStateID()
-     * @return array The state referenced by the $id parameter.
+     * @return array|null The state referenced by the $id parameter.
      */
     public static function fetchProcessedState($id)
     {
@@ -328,6 +335,8 @@ class ProcessingChain
 
     /**
      * @deprecated This method will be removed in SSP 2.0.
+     * @param array &$state
+     * @return void
      */
     private static function addUserID(&$state)
     {
diff --git a/lib/SimpleSAML/Auth/Simple.php b/lib/SimpleSAML/Auth/Simple.php
index ffaa0cc92..927418261 100644
--- a/lib/SimpleSAML/Auth/Simple.php
+++ b/lib/SimpleSAML/Auth/Simple.php
@@ -23,7 +23,7 @@ class Simple
      */
     protected $authSource;
 
-    /** @var \SimpleSAML\Configuration */
+    /** @var \SimpleSAML\Configuration|null */
     protected $app_config;
 
     /** @var \SimpleSAML\Session */
@@ -97,6 +97,7 @@ class Simple
      * method for a description.
      *
      * @param array $params Various options to the authentication request. See the documentation.
+     * @return void
      */
     public function requireAuth(array $params = [])
     {
@@ -122,6 +123,7 @@ class Simple
      * Please note: this function never returns.
      *
      * @param array $params Various options to the authentication request.
+     * @return void
      */
     public function login(array $params = [])
     {
@@ -181,6 +183,7 @@ class Simple
      *
      * @param string|array|null $params Either the URL the user should be redirected to after logging out, or an array
      * with parameters for the logout. If this parameter is null, we will return to the current page.
+     * @return void
      */
     public function logout($params = null)
     {
@@ -229,6 +232,7 @@ class Simple
      * This function never returns.
      *
      * @param array $state The state after the logout.
+     * @return void
      */
     public static function logoutCompleted($state)
     {
diff --git a/lib/SimpleSAML/Auth/Source.php b/lib/SimpleSAML/Auth/Source.php
index 6eaf69008..6bf02b352 100644
--- a/lib/SimpleSAML/Auth/Source.php
+++ b/lib/SimpleSAML/Auth/Source.php
@@ -293,7 +293,7 @@ abstract class Source
      * @param string $authId The authentication source identifier.
      * @param array  $config The configuration.
      *
-     * @return Source The parsed authentication source.
+     * @return \SimpleSAML\Auth\Source The parsed authentication source.
      * @throws \Exception If the authentication source is invalid.
      */
     private static function parseAuthSource($authId, $config)
@@ -514,6 +514,7 @@ abstract class Source
      * @param string $id The auth source identifier.
      *
      * @throws \Exception If the first element of $source is not an identifier for the auth source.
+     * @return void
      */
     protected static function validateSource($source, $id)
     {
diff --git a/lib/SimpleSAML/Auth/TimeLimitedToken.php b/lib/SimpleSAML/Auth/TimeLimitedToken.php
index eb6620d5e..5b4c3c2a2 100644
--- a/lib/SimpleSAML/Auth/TimeLimitedToken.php
+++ b/lib/SimpleSAML/Auth/TimeLimitedToken.php
@@ -66,6 +66,7 @@ class TimeLimitedToken
      * not only the same data must be added, but also in the same order.
      *
      * @param string $data The data to incorporate into the current token.
+     * @return void
      */
     public function addVerificationData($data)
     {
@@ -110,6 +111,7 @@ class TimeLimitedToken
     /**
      * @see generate
      * @deprecated This method will be removed in SSP 2.0. Use generate() instead.
+     * @return string
      */
     public function generate_token()
     {
@@ -122,7 +124,7 @@ class TimeLimitedToken
      *
      * @param string $token The token to validate.
      *
-     * @return boolean True if the given token is currently valid, false otherwise.
+     * @return bool True if the given token is currently valid, false otherwise.
      */
     public function validate($token)
     {
@@ -139,6 +141,8 @@ class TimeLimitedToken
     /**
      * @see validate
      * @deprecated This method will be removed in SSP 2.0. Use validate() instead.
+     * @param string $token
+     * @return bool
      */
     public function validate_token($token)
     {
-- 
GitLab