Fix phpcs errors and warnings

Author: nathanjrobertson

src/Auth/Source/PasswordVerify.php

@@ -50,6 +50,7 @@ class PasswordVerify extends SQL
      */
     protected string $passwordhashcolumn = 'passwordhash';
 
+
     /**
      * Constructor for this authentication source.
      *
@@ -67,7 +68,6 @@ public function __construct(array $info, array $config)
     }
 
 
-
     /**
      * Attempt to log in using the given username and password.
      *

src/Auth/Source/PasswordVerify1Compat.php

@@ -18,7 +18,7 @@ class PasswordVerify1Compat extends SQL2
      */
     public function __construct(array $info, array $config)
     {
-        /* Transform PasswordVerify (version 1) config to SQL2 config 
+        /* Transform PasswordVerify (version 1) config to SQL2 config
          * Version 1 supported only one database, but multiple queries. The first query was defined
          * to be the "authentication query", all subsequent queries were "attribute queries".
          */
@@ -29,15 +29,15 @@ public function __construct(array $info, array $config)
                     'dsn' => $config['dsn'],
                     'username' => $config['username'],
                     'password' => $config['password'],
-                ]
+                ],
             ],
 
             'auth_queries' => [
                 'default' => [
                     'database' => 'default',
                     'query' => is_array($config['query']) ? $config['query'][0] : $config['query'],
                     'password_verify_hash_column' => 'passwordhash',
-                ]
+                ],
             ],
         ];
 

src/Auth/Source/SQL.php

@@ -70,6 +70,7 @@ class SQL extends UserPassBase
      */
     protected array $query;
 
+
     /**
      * Constructor for this authentication source.
      *
@@ -155,6 +156,7 @@ protected function connect(): PDO
         return $db;
     }
 
+
     /**
      * Extract SQL columns into SAML attribute array
      *
@@ -191,6 +193,7 @@ protected function extractAttributes(array &$attributes, array $data, array $for
         return $attributes;
     }
 
+
     /**
      * Execute the query with given parameters and return the tuples that result.
      *
@@ -223,6 +226,7 @@ protected function executeQuery(PDO $db, string $query, array $params): array
         }
     }
 
+
     /**
      * If there is a username_regex then verify the passed username against it and
      * throw an exception if it fails.
@@ -240,6 +244,7 @@ protected function verifyUserNameWithRegex(string $username): void
         }
     }
 
+
     /**
      * Attempt to log in using the given username and password.
      *

src/Auth/Source/SQL1Compat.php

@@ -18,7 +18,7 @@ class SQL1Compat extends SQL2
      */
     public function __construct(array $info, array $config)
     {
-        /* Transform SQL (version 1) config to SQL2 config 
+        /* Transform SQL (version 1) config to SQL2 config
          * Version 1 supported only one database, but multiple queries. The first query was defined
          * to be the "authentication query", all subsequent queries were "attribute queries".
          */
@@ -29,14 +29,14 @@ public function __construct(array $info, array $config)
                     'dsn' => $config['dsn'],
                     'username' => $config['username'],
                     'password' => $config['password'],
-                ]
+                ],
             ],
 
             'auth_queries' => [
                 'default' => [
                     'database' => 'default',
                     'query' => is_array($config['query']) ? $config['query'][0] : $config['query'],
-                ]
+                ],
             ],
         ];
 

src/Auth/Source/SQL2.php

@@ -26,15 +26,15 @@ class SQL2 extends UserPassBase
      * List of one or more databases that are used by auth and attribute queries.
      * Each database must have a unique name, and the name is used to refer to
      * the database in auth and attribute queries.
-     * 
+     *
      * @var array
      */
     private array $databases = [];
 
     /**
      * List of one or more authentication queries. The first query that returns a result
      * is considered to have authenticated the user (and termed "winning").
-     * 
+     *
      * @var array
      */
     private array $authQueries = [];
@@ -47,6 +47,7 @@ class SQL2 extends UserPassBase
      */
     private array $attributesQueries = [];
 
+
     /**
      * Constructor for this authentication source.
      *
@@ -73,7 +74,7 @@ public function __construct(array $info, array $config)
 
             foreach ($config['databases'] as $dbname => $dbConfig) {
                 if (!is_array($dbConfig)) {
-                    throw new Exception('Each entry in the ' . 
+                    throw new Exception('Each entry in the ' .
                         $dbname . ' \'databases\' parameter for authentication source ' .
                         $this->authId . ' is expected to be an array. Instead it was: ' .
                         var_export($dbConfig, true));
@@ -126,7 +127,7 @@ public function __construct(array $info, array $config)
 
             foreach ($config['auth_queries'] as $authQueryName => $authQueryConfig) {
                 if (!is_array($authQueryConfig)) {
-                    throw new Exception('Each entry in the ' . 
+                    throw new Exception('Each entry in the ' .
                         $authQueryName . ' \'auth_queries\' parameter for authentication source ' .
                         $this->authId . ' is expected to be an array. Instead it was: ' .
                         var_export($authQueryConfig, true));
@@ -146,7 +147,7 @@ public function __construct(array $info, array $config)
                             var_export($authQueryConfig[$param], true));
                     }
                 }
-                
+
                 if (!array_key_exists($authQueryConfig['database'], $this->databases)) {
                     throw new Exception('Auth query ' .
                         $authQueryName . ' references unknown database \'' .
@@ -155,8 +156,13 @@ public function __construct(array $info, array $config)
                 }
 
                 $this->authQueries[$authQueryName] = [
-                    '_winning_auth_query' => false, // Will be set to true for the query that successfully authenticated the user
-                    '_extracted_userid' => null, // Will hold the value of the attribute named by 'extract_userid_from' if specified and authentication succeeds
+                    // Will be set to true for the query that successfully authenticated the user
+                    '_winning_auth_query' => false,
+
+                    // Will hold the value of the attribute named by 'extract_userid_from'
+                    // if specified and authentication succeeds
+                    '_extracted_userid' => null,
+
                     'database' => $authQueryConfig['database'],
                     'query' => $authQueryConfig['query'],
                 ];
@@ -181,15 +187,21 @@ public function __construct(array $info, array $config)
 
                 if (array_key_exists('password_verify_hash_column', $authQueryConfig)) {
                     if (!is_string($authQueryConfig['password_verify_hash_column'])) {
-                        throw new Exception('Optional parameter \'password_verify_hash_column\' for authentication source ' .
+                        throw new Exception(
+                            'Optional parameter \'password_verify_hash_column\' for authentication source ' .
                             $this->authId . ' was provided and is expected to be a string. Instead it was: ' .
-                            var_export($authQueryConfig['password_verify_hash_column'], true));
+                            var_export($authQueryConfig['password_verify_hash_column'], true),
+                        );
                     }
-                    $this->authQueries[$authQueryName]['password_verify_hash_column'] = $authQueryConfig['password_verify_hash_column'];
+                    $this->authQueries[$authQueryName]['password_verify_hash_column'] =
+                        $authQueryConfig['password_verify_hash_column'];
                 }
             }
         } else {
-            throw new Exception('Missing required attribute \'auth_queries\' for authentication source ' . $this->authId);
+            throw new Exception(
+                'Missing required attribute \'auth_queries\' for authentication source ' .
+                $this->authId,
+            );
         }
 
         // attr_queries is optional, but if specified, we need to check the parameters
@@ -220,7 +232,7 @@ public function __construct(array $info, array $config)
                             var_export($attrQueryConfig[$param], true));
                     }
                 }
-                
+
                 $currentAttributeQuery = [
                     'database' => $attrQueryConfig['database'],
                     'query' => $attrQueryConfig['query'],
@@ -258,6 +270,7 @@ public function __construct(array $info, array $config)
         }
     }
 
+
     /**
      * Create a database connection.
      *
@@ -273,17 +286,18 @@ protected function connect(string $dbname): PDO
             // Already connected
             return $this->databases[$dbname]['_pdo'];
         }
-    
+
         try {
             $db = new PDO(
                 $this->databases[$dbname]['dsn'],
                 $this->databases[$dbname]['username'],
                 $this->databases[$dbname]['password'],
-                $this->databases[$dbname]['options']
+                $this->databases[$dbname]['options'],
             );
         } catch (PDOException $e) {
             // Obfuscate the password if it's part of the dsn
-            $obfuscated_dsn =  preg_replace('/(user|password)=(.*?([;]|$))/', '${1}=***', $this->databases[$dbname]['dsn']);
+            $obfuscated_dsn =
+                preg_replace('/(user|password)=(.*?([;]|$))/', '${1}=***', $this->databases[$dbname]['dsn']);
 
             throw new Exception('sqlauth:' . $this->authId . ': - Failed to connect to \'' .
                 $obfuscated_dsn . '\': ' . $e->getMessage());
@@ -311,6 +325,7 @@ protected function connect(string $dbname): PDO
         return $db;
     }
 
+
     /**
      * Extract SQL columns into SAML attribute array
      *
@@ -347,6 +362,7 @@ protected function extractAttributes(array &$attributes, array $data, array $for
         return $attributes;
     }
 
+
     /**
      * Execute the query with given parameters and return the tuples that result.
      *
@@ -379,6 +395,7 @@ protected function executeQuery(PDO $db, string $query, array $params): array
         }
     }
 
+
     /**
      * Attempt to log in using the given username and password.
      *
@@ -404,7 +421,10 @@ protected function login(
         // Run authentication queries in order until one succeeds.
         foreach ($this->authQueries as $queryname => &$queryConfig) {
             // Check if the username matches the username_regex for this query
-            if (array_key_exists('username_regex', $queryConfig) && !preg_match($queryConfig['username_regex'], $username)) {
+            if (
+                array_key_exists('username_regex', $queryConfig) &&
+                !preg_match($queryConfig['username_regex'], $username)
+            ) {
                 Logger::debug('sqlauth:' . $this->authId . ': Skipping auth query ' . $queryname .
                              ' because username ' . $username . ' does not match username_regex ' .
                              $queryConfig['username_regex']);
@@ -416,14 +436,12 @@ protected function login(
             $db = $this->connect($queryConfig['database']);
 
             try {
-                if (array_key_exists('password_verify_hash_column', $queryConfig)) {
-                    // We will verify the password using password_verify() later, so we do not
-                    // pass the password to the query.
-                    $data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username]);
-                } else {
-                    // Pass both username and password to the query
-                    $data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username, 'password' => $password]);
+                $sqlParams = ['username' => $username];
+                if (!array_key_exists('password_verify_hash_column', $queryConfig)) {
+                    // If we are not using password_verify(), pass the password to the query
+                    $sqlParams['password'] = $password;
                 }
+                $data = $this->executeQuery($db, $queryConfig['query'], $sqlParams);
             } catch (PDOException $e) {
                 Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
                               ' failed with error: ' . $e->getMessage());
@@ -493,7 +511,6 @@ protected function login(
 
                 // The first auth query that succeeds is the winning one, so we can stop here.
                 break;
-
             } else {
                 Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname .
                              ' returned no rows, trying next auth query if any');
@@ -510,17 +527,25 @@ protected function login(
         foreach ($this->attributesQueries as $attrQueryConfig) {
             // If the attribute query is limited to certain auth queries, check if the winning auth query
             // is one of those.
-            Logger::debug('sqlauth:' . $this->authId . ': Considering attribute query ' . $attrQueryConfig['query'] .
-                         ' for winning auth query ' . $winning_auth_query . ' with only_for_auth ' . implode(',', $attrQueryConfig['only_for_auth'] ?? []));
-            if ((!array_key_exists('only_for_auth', $attrQueryConfig)) || in_array($winning_auth_query, $attrQueryConfig['only_for_auth'], true)) {
+            Logger::debug(
+                'sqlauth:' . $this->authId . ': ' .
+                'Considering attribute query ' . $attrQueryConfig['query'] .
+                ' for winning auth query ' . $winning_auth_query .
+                ' with only_for_auth ' . implode(',', $attrQueryConfig['only_for_auth'] ?? []),
+            );
+
+            if (
+                (!array_key_exists('only_for_auth', $attrQueryConfig)) ||
+                in_array($winning_auth_query, $attrQueryConfig['only_for_auth'], true)
+            ) {
                 Logger::debug('sqlauth:' . $this->authId . ': Running attribute query ' . $attrQueryConfig['query'] .
                              ' for winning auth query ' . $winning_auth_query);
 
                 $db = $this->connect($attrQueryConfig['database']);
 
                 try {
-                    $params = ($this->authQueries[$winning_auth_query]['_extracted_userid'] !== null) ? 
-                        ['userid' => $this->authQueries[$winning_auth_query]['_extracted_userid']] : 
+                    $params = ($this->authQueries[$winning_auth_query]['_extracted_userid'] !== null) ?
+                        ['userid' => $this->authQueries[$winning_auth_query]['_extracted_userid']] :
                         ['username' => $username];
                     $data = $this->executeQuery($db, $attrQueryConfig['query'], $params);
                 } catch (PDOException $e) {

tests/src/Auth/Source/PasswordVerifyTest.php

@@ -28,6 +28,7 @@ class PasswordVerifyTest extends TestCase
         "query" => null, // Filled out by each test case
     ];
 
+
     public static function setUpBeforeClass(): void
     {
         $pdo = new PDO('sqlite:file::memory:?cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]);
@@ -82,6 +83,7 @@ public static function setUpBeforeClass(): void
         }
     }
 
+
     public function testBasicSingleSuccess(): void
     {
         // Correct username/password
@@ -105,6 +107,7 @@ public function testBasicSingleFailedLogin(): void
         $this->assertCount(0, $ret);
     }
 
+
     public function testBasicSingleFailedLoginNonExisting(): void
     {
         $this->expectException(\SimpleSAML\Error\Error::class);
@@ -143,6 +146,7 @@ public function testJoinSingleSuccess(): void
         ]);
     }
 
+
     public function testJoinSingleFailedLogin(): void
     {
         $this->expectException(\SimpleSAML\Error\Error::class);
@@ -155,6 +159,7 @@ public function testJoinSingleFailedLogin(): void
         $this->assertCount(0, $ret);
     }
 
+
     public function testMultiQuerySuccess(): void
     {
         // Correct username/password
@@ -173,6 +178,7 @@ public function testMultiQuerySuccess(): void
         ]);
     }
 
+
     public function testMultiQueryFailedLogin(): void
     {
         $this->expectException(\SimpleSAML\Error\Error::class);
@@ -205,6 +211,7 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void
         ]);
     }
 
+
     public function testMultiQuerySubsequentAppendSuccess(): void
     {
         // Correct username/password. Second query returns a row, third query appends one row