From fab2c2d06e150b4f9fa983abd69f888576b56a41 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 22 Dec 2025 13:43:12 +1100 Subject: [PATCH 1/2] Deprecate Version 1 confguration, remove old implementation, redirect V1 configuration through Compat interfaces --- docs/sql.md | 9 +- src/Auth/Source/PasswordVerify.php | 127 +--------- src/Auth/Source/PasswordVerify1Compat.php | 11 +- src/Auth/Source/SQL.php | 294 +--------------------- src/Auth/Source/SQL1Compat.php | 9 + 5 files changed, 25 insertions(+), 425 deletions(-) diff --git a/docs/sql.md b/docs/sql.md index 456a207..da4bb8f 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -7,14 +7,11 @@ The authentication can be done in one of two ways: - Most commonly, as a part of the SQL query itself (ie. using SQL functions to hash a parameterized password and compare that to a value stored in the database). - Less commonly, just store the hash in the database, retrieve that then compare that hash using PHP's `password_verify()` function to authenticate. This is useful in cases where there is minimal support in the database or to allow the same code to work against many databases without modification. The differences in how this is configured are in a section towards the bottom of this file. -There are two different configuration formats supported ("version 1" and "version 2"). Version 1 is simpler, but is more limited in functionality. Version 2 is more powerful and configurable, but a little more verbose. If you wish to authenticate or gather attributes from more than one SQL database, or need more than one SQL query for authentication then you definitely need Version 2. +There are two different configuration formats ("Version 1" and "Version 2"). We highly recommend using the more powerful and configurable Version 2 configuration. Version 1 is now considered deprecated and support for this legacy configuration format will be removed in a future release. -The Version 1 configuration support comes in two flavours (but identical configurations): +If you are starting out you should use the Version 2 (`sqlauth:SQL2`) configuration format. -- `sqlauth:SQL` uses the legacy Version 1 configuration format and code. Eventually the old code will be phased out, and `sqlauth:SQL` will become a synonym for `sqlauth:SQL1Compat`. -- `sqlauth:SQL1Compat` uses the legacy Version 1 configuration, but applies it to the Version 2 code. - -If you are starting out we recommend the Version 2 (`sqlauth:SQL2`) configuration format. +If you have existing Version 1 (`sqlauth:SQL` or `sqlauth:SQL1Compat`) configuration, you should migrate to the new Version 2 (`sqlauth:SQL2`) configuration format. You enable the module in `config/config.php`. diff --git a/src/Auth/Source/PasswordVerify.php b/src/Auth/Source/PasswordVerify.php index 2e46cfe..c17961e 100644 --- a/src/Auth/Source/PasswordVerify.php +++ b/src/Auth/Source/PasswordVerify.php @@ -44,129 +44,4 @@ * @package SimpleSAMLphp */ -class PasswordVerify extends SQL -{ - /** - * The column in the result set containing the passwordhash. - */ - protected string $passwordhashcolumn = 'passwordhash'; - - - /** - * Constructor for this authentication source. - * - * @param array $info Information about this authentication source. - * @param array $config Configuration. - */ - public function __construct(array $info, array $config) - { - // Call the parent constructor first, as required by the interface - parent::__construct($info, $config); - - if (array_key_exists('passwordhashcolumn', $config)) { - $this->passwordhashcolumn = $config['passwordhashcolumn']; - } - } - - - /** - * Attempt to log in using the given username and password. - * - * On a successful login, this function should return the users attributes. On failure, - * it should throw an exception. If the error was caused by the user entering the wrong - * username or password, a \SimpleSAML\Error\Error('WRONGUSERPASS') should be thrown. - * - * Note that both the username and the password are UTF-8 encoded. - * - * @param string $username The username the user wrote. - * @param string $password The password the user wrote. - * @return array Associative array with the users attributes. - */ - protected function login(string $username, string $password): array - { - $this->verifyUserNameWithRegex($username); - - $db = $this->connect(); - $params = ['username' => $username]; - $attributes = []; - - $numQueries = count($this->query); - for ($x = 0; $x < $numQueries; $x++) { - $data = $this->executeQuery($db, $this->query[$x], $params); - - Logger::info('sqlauth:' . $this->authId . ': Got ' . count($data) . - ' rows from database'); - - /** - * Sanity check, passwordhash must be in each resulting tuple and must have - * the same value in every tuple. - * - * Note that $pwhash will contain the passwordhash value after this loop. - */ - $pwhash = null; - if ($x === 0) { - if (count($data) === 0) { - // No rows returned - invalid username/password - Logger::error(sprintf( - 'sqlauth:%s: No rows in result set. Probably wrong username/password.', - $this->authId, - )); - throw new Error\Error('WRONGUSERPASS'); - } - - foreach ($data as $row) { - if ( - !array_key_exists($this->passwordhashcolumn, $row) - || is_null($row[$this->passwordhashcolumn]) - ) { - Logger::error(sprintf( - 'sqlauth:%s: column `%s` must be in every result tuple.', - $this->authId, - $this->passwordhashcolumn, - )); - throw new Error\Error('WRONGUSERPASS'); - } - if ($pwhash) { - if ($pwhash != $row[$this->passwordhashcolumn]) { - Logger::error(sprintf( - 'sqlauth:%s: column %s must be THE SAME in every result tuple.', - $this->authId, - $this->passwordhashcolumn, - )); - throw new Error\Error('WRONGUSERPASS'); - } - } - $pwhash = $row[$this->passwordhashcolumn]; - } - - /** - * This should never happen as the count(data) test above would have already thrown. - * But checking twice doesn't hurt. - */ - Assert::notNull($pwhash); - - /** - * VERIFICATION! - * Now to check if the password the user supplied is actually valid - */ - if (!password_verify($password, $pwhash)) { - Logger::error(sprintf( - 'sqlauth:%s: password is incorrect.', - $this->authId, - )); - throw new Error\Error('WRONGUSERPASS'); - } - } - - $this->extractAttributes($attributes, $data, [$this->passwordhashcolumn]); - } - - Logger::info(sprintf( - 'sqlauth:%s: Attributes: %s', - $this->authId, - implode(',', array_keys($attributes)), - )); - - return $attributes; - } -} +class PasswordVerify extends PasswordVerify1Compat {} diff --git a/src/Auth/Source/PasswordVerify1Compat.php b/src/Auth/Source/PasswordVerify1Compat.php index 3df9ead..6958f8d 100644 --- a/src/Auth/Source/PasswordVerify1Compat.php +++ b/src/Auth/Source/PasswordVerify1Compat.php @@ -4,7 +4,11 @@ namespace SimpleSAML\Module\sqlauth\Auth\Source; +use SimpleSAML\Logger; + /** + * @deprecated Use the SQL2 class and the new SQL2 configuration format instead. + * * @package SimpleSAMLphp */ @@ -18,7 +22,12 @@ class PasswordVerify1Compat extends SQL2 */ public function __construct(array $info, array $config) { - /* Transform PasswordVerify (version 1) config to SQL2 config + Logger::warning( + 'The sqlauth:PasswordVerify and sqlauth:PasswordVerify1Compat authentication sources are deprecated. '. + 'Please migrate to sqlauth:SQL2 with the new configuration format.' + ); + + /* 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". */ diff --git a/src/Auth/Source/SQL.php b/src/Auth/Source/SQL.php index fd56bb2..f7cc0cf 100644 --- a/src/Auth/Source/SQL.php +++ b/src/Auth/Source/SQL.php @@ -4,300 +4,10 @@ namespace SimpleSAML\Module\sqlauth\Auth\Source; -use Exception; -use PDO; -use PDOException; -use SimpleSAML\Error; -use SimpleSAML\Logger; -use SimpleSAML\Module\core\Auth\UserPassBase; - -use function array_key_exists; -use function array_keys; -use function explode; -use function implode; -use function in_array; -use function is_string; -use function preg_replace; -use function strtolower; -use function var_export; - /** - * Simple SQL authentication source - * - * This class is an example authentication source which authenticates an user - * against a SQL database. + * @deprecated Use the SQL2 class and the new SQL2 configuration format instead. * * @package SimpleSAMLphp */ -class SQL extends UserPassBase -{ - /** - * The DSN we should connect to. - * @var string - */ - private string $dsn; - - /** - * The username we should connect to the database with. - * @var string - */ - private string $username; - - /** - * The password we should connect to the database with. - * @var string - */ - private string $password; - - /** - * An optional regex that the username should match. - * @var string - */ - protected ?string $username_regex; - - /** - * The options that we should connect to the database with. - * @var array - */ - private array $options = []; - - /** - * The query or queries we should use to retrieve the attributes for the user. - * - * The username and password will be available as :username and :password. - * @var array - */ - protected array $query; - - - /** - * Constructor for this authentication source. - * - * @param array $info Information about this authentication source. - * @param array $config Configuration. - */ - public function __construct(array $info, array $config) - { - // Call the parent constructor first, as required by the interface - parent::__construct($info, $config); - - // Make sure that all required parameters are present. - foreach (['dsn', 'username', 'password'] as $param) { - if (!array_key_exists($param, $config)) { - throw new Exception('Missing required attribute \'' . $param . - '\' for authentication source ' . $this->authId); - } - - if (!is_string($config[$param])) { - throw new Exception('Expected parameter \'' . $param . - '\' for authentication source ' . $this->authId . - ' to be a string. Instead it was: ' . - var_export($config[$param], true)); - } - } - - // Query can be a single query or an array of queries. - if (!array_key_exists('query', $config)) { - throw new Exception('Missing required attribute \'query\' ' . - 'for authentication source ' . $this->authId); - } elseif (is_array($config['query']) && (count($config['query']) < 1)) { - throw new Exception('Required attribute \'query\' is an empty ' . - 'list of queries for authentication source ' . $this->authId); - } - - $this->dsn = $config['dsn']; - $this->username = $config['username']; - $this->password = $config['password']; - $this->query = is_string($config['query']) ? [$config['query']] : $config['query']; - if (isset($config['options'])) { - $this->options = $config['options']; - } - - // Optional "username_regex" parameter - $this->username_regex = array_key_exists('username_regex', $config) ? $config['username_regex'] : null; - } - - - /** - * Create a database connection. - * - * @return \PDO The database connection. - */ - protected function connect(): PDO - { - try { - $db = new PDO($this->dsn, $this->username, $this->password, $this->options); - } catch (PDOException $e) { - // Obfuscate the password if it's part of the dsn - $obfuscated_dsn = preg_replace('/(user|password)=(.*?([;]|$))/', '${1}=***', $this->dsn); - - throw new Exception('sqlauth:' . $this->authId . ': - Failed to connect to \'' . - $obfuscated_dsn . '\': ' . $e->getMessage()); - } - - $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - - $driver = explode(':', $this->dsn, 2); - $driver = strtolower($driver[0]); - - // Driver specific initialization - switch ($driver) { - case 'mysql': - // Use UTF-8 - $db->exec("SET NAMES 'utf8mb4'"); - break; - case 'pgsql': - // Use UTF-8 - $db->exec("SET NAMES 'UTF8'"); - break; - } - - return $db; - } - - - /** - * Extract SQL columns into SAML attribute array - * - * @param array $attributes output place to store extracted attributes - * @param array $data Associative array from database in the format of PDO fetchAll - * @param array $forbiddenAttributes An array of attributes to never return - * @return array &$attributes - */ - protected function extractAttributes(array &$attributes, array $data, array $forbiddenAttributes = []): array - { - foreach ($data as $row) { - foreach ($row as $name => $value) { - if ($value === null) { - continue; - } - if (in_array($name, $forbiddenAttributes)) { - continue; - } - - $value = (string) $value; - - if (!array_key_exists($name, $attributes)) { - $attributes[$name] = []; - } - - if (in_array($value, $attributes[$name], true)) { - // Value already exists in attribute - continue; - } - - $attributes[$name][] = $value; - } - } - return $attributes; - } - - - /** - * Execute the query with given parameters and return the tuples that result. - * - * @param string $query SQL to execute - * @param array $params parameters to the SQL query - * @return array tuples that result - */ - protected function executeQuery(PDO $db, string $query, array $params): array - { - try { - $sth = $db->prepare($query); - } catch (PDOException $e) { - throw new Exception('sqlauth:' . $this->authId . - ': - Failed to prepare query: ' . $e->getMessage()); - } - - try { - $sth->execute($params); - } catch (PDOException $e) { - throw new Exception('sqlauth:' . $this->authId . - ': - Failed to execute query: ' . $e->getMessage()); - } - - try { - $data = $sth->fetchAll(PDO::FETCH_ASSOC); - return $data; - } catch (PDOException $e) { - throw new Exception('sqlauth:' . $this->authId . - ': - Failed to fetch result set: ' . $e->getMessage()); - } - } - - - /** - * If there is a username_regex then verify the passed username against it and - * throw an exception if it fails. - * - * @param string $username The username the user wrote. - */ - protected function verifyUserNameWithRegex(string $username): void - { - if ($this->username_regex !== null) { - if (!preg_match($this->username_regex, $username)) { - Logger::error('sqlauth:' . $this->authId . - ": Username doesn't match username_regex."); - throw new Error\Error('WRONGUSERPASS'); - } - } - } - - - /** - * Attempt to log in using the given username and password. - * - * On a successful login, this function should return the users attributes. On failure, - * it should throw an exception. If the error was caused by the user entering the wrong - * username or password, a \SimpleSAML\Error\Error('WRONGUSERPASS') should be thrown. - * - * Note that both the username and the password are UTF-8 encoded. - * - * @param string $username The username the user wrote. - * @param string $password The password the user wrote. - * @return array Associative array with the users attributes. - */ - protected function login( - string $username, - #[\SensitiveParameter] - string $password, - ): array { - $this->verifyUserNameWithRegex($username); - - $db = $this->connect(); - $params = ['username' => $username, 'password' => $password]; - $attributes = []; - - $numQueries = count($this->query); - for ($x = 0; $x < $numQueries; $x++) { - $data = $this->executeQuery($db, $this->query[$x], $params); - - Logger::info('sqlauth:' . $this->authId . ': Got ' . count($data) . - ' rows from database'); - - if ($x === 0) { - if (count($data) === 0) { - // No rows returned from first query - invalid username/password - Logger::error('sqlauth:' . $this->authId . - ': No rows in result set. Probably wrong username/password.'); - throw new Error\Error('WRONGUSERPASS'); - } - /* Only the first query should be passed the password, as that is the only - * one used for authentication. Subsequent queries are only used for - * getting attribute lists, so only need the username. */ - unset($params['password']); - } - - /* Extract attributes. We allow the resultset to consist of multiple rows. Attributes - * which are present in more than one row will become multivalued. null values and - * duplicate values will be skipped. All values will be converted to strings. - */ - $this->extractAttributes($attributes, $data, []); - } - - Logger::info('sqlauth:' . $this->authId . ': Attributes: ' . implode(',', array_keys($attributes))); - - return $attributes; - } -} +class SQL extends SQL1Compat {} diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php index 434eac2..065887a 100644 --- a/src/Auth/Source/SQL1Compat.php +++ b/src/Auth/Source/SQL1Compat.php @@ -4,7 +4,11 @@ namespace SimpleSAML\Module\sqlauth\Auth\Source; +use SimpleSAML\Logger; + /** + * @deprecated Use the SQL2 class and the new SQL2 configuration format instead. + * * @package SimpleSAMLphp */ @@ -18,6 +22,11 @@ class SQL1Compat extends SQL2 */ public function __construct(array $info, array $config) { + Logger::warning( + 'The sqlauth:SQL and sqlauth:SQL1Compat authentication sources are deprecated. '. + 'Please migrate to sqlauth:SQL2 with the new configuration format.' + ); + /* 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". From 5061ea4204dde55ba9bd6c5aba968ea6b5c2bc30 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 22 Dec 2025 13:56:15 +1100 Subject: [PATCH 2/2] Fix phpcs nitpicks --- src/Auth/Source/PasswordVerify.php | 17 +++-------------- src/Auth/Source/PasswordVerify1Compat.php | 6 +++--- src/Auth/Source/SQL.php | 4 +++- src/Auth/Source/SQL1Compat.php | 4 ++-- 4 files changed, 11 insertions(+), 20 deletions(-) diff --git a/src/Auth/Source/PasswordVerify.php b/src/Auth/Source/PasswordVerify.php index c17961e..98e366c 100644 --- a/src/Auth/Source/PasswordVerify.php +++ b/src/Auth/Source/PasswordVerify.php @@ -4,19 +4,6 @@ namespace SimpleSAML\Module\sqlauth\Auth\Source; -use SimpleSAML\Assert\Assert; -use SimpleSAML\Error; -use SimpleSAML\Logger; -use SimpleSAML\Module\sqlauth\Auth\Source\SQL; - -use function array_key_exists; -use function array_keys; -use function count; -use function implode; -use function is_null; -use function password_verify; -use function sprintf; - /** * Simple SQL authentication source * @@ -44,4 +31,6 @@ * @package SimpleSAMLphp */ -class PasswordVerify extends PasswordVerify1Compat {} +class PasswordVerify extends PasswordVerify1Compat +{ +} diff --git a/src/Auth/Source/PasswordVerify1Compat.php b/src/Auth/Source/PasswordVerify1Compat.php index 6958f8d..6f7b235 100644 --- a/src/Auth/Source/PasswordVerify1Compat.php +++ b/src/Auth/Source/PasswordVerify1Compat.php @@ -23,9 +23,9 @@ class PasswordVerify1Compat extends SQL2 public function __construct(array $info, array $config) { Logger::warning( - 'The sqlauth:PasswordVerify and sqlauth:PasswordVerify1Compat authentication sources are deprecated. '. - 'Please migrate to sqlauth:SQL2 with the new configuration format.' - ); + 'The sqlauth:PasswordVerify and sqlauth:PasswordVerify1Compat authentication sources are deprecated. ' . + 'Please migrate to sqlauth:SQL2 with the new configuration format.', + ); /* Transform PasswordVerify (version 1) config to SQL2 config * Version 1 supported only one database, but multiple queries. The first query was defined diff --git a/src/Auth/Source/SQL.php b/src/Auth/Source/SQL.php index f7cc0cf..64d73e5 100644 --- a/src/Auth/Source/SQL.php +++ b/src/Auth/Source/SQL.php @@ -10,4 +10,6 @@ * @package SimpleSAMLphp */ -class SQL extends SQL1Compat {} +class SQL extends SQL1Compat +{ +} diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php index 065887a..83a7091 100644 --- a/src/Auth/Source/SQL1Compat.php +++ b/src/Auth/Source/SQL1Compat.php @@ -23,8 +23,8 @@ class SQL1Compat extends SQL2 public function __construct(array $info, array $config) { Logger::warning( - 'The sqlauth:SQL and sqlauth:SQL1Compat authentication sources are deprecated. '. - 'Please migrate to sqlauth:SQL2 with the new configuration format.' + 'The sqlauth:SQL and sqlauth:SQL1Compat authentication sources are deprecated. ' . + 'Please migrate to sqlauth:SQL2 with the new configuration format.', ); /* Transform SQL (version 1) config to SQL2 config