From b8bb0e654b3b48ba075225b68391114f2634f2e9 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 13 Oct 2025 09:16:07 +1100 Subject: [PATCH 01/28] Add documentation for UNIX socket connections --- docs/sql.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/docs/sql.md b/docs/sql.md index 3fc54ee..bbad411 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -180,6 +180,37 @@ Example query - SHA512 of salt + password, stored as salt (32 bytes) + sha256(sa ) ``` +Connecting with UNIX Domain Sockets (Local Sockets) +--------------------------------------------------- + +When on a UNIX-like platform (Linux, *BSD, etc), and when your SQL database server is running on the same host as the web server +hosting SimpleSAMLphp, it is possible to use UNIX domain sockets instead of TCP sockets for the database connection. This +configuration should result in marginally better performance and security (when configured correctly). + +Here is an example using PostgreSQL: + +```php + 'example-unix-socket-sql' => [ + 'sqlauth:SQL', + 'dsn' => 'pgsql:host=/var/run/postgresql;dbname=simplesaml', + 'username' => 'www-data', + 'password' => 'this-is-ignored', + 'query' => 'SELECT uid, givenName, email, eduPersonPrincipalName FROM users WHERE uid = :username ' . + 'AND password = SHA2(CONCAT((SELECT salt FROM users WHERE uid = :username), :password), 256);', + ], +``` + +Configuration is largely the same as TCP sockets (documented above), with the differences being: + +`dsn` +: The key difference is that the `host` parameter. This needs to be the **directory** that contains the socket file used to connect to the PostgreSQL server. For example, actual socket file might be `/var/run/postgresql/.s.PGSQL.5432`, so `host=/var/run/postgresql` is the parameter that you need. If you're struggling to find where the socket is, the `unix_socket_directories` parameter in the server `postgresql.conf` is where that location is configured. + +`username` +: The UNIX username of the user running SimpleSAMLphp (ie. the web server user or the php-fpm user, depending on your setup). + +`password` +: Required, but the value you specify is ignored (so you can put any placeholder string value in there). All authentication for UNIX domain sockets are done by the operating system kernel. + Security considerations ----------------------- From d4efc2efe1b5667316bd18fc4aaba9681092aa68 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 13 Oct 2025 09:22:38 +1100 Subject: [PATCH 02/28] Spelling fix --- docs/sql.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sql.md b/docs/sql.md index bbad411..8c9328d 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -38,7 +38,7 @@ Options Writing a Query / Queries ------------------------- -A `query` can be either a single string with an SQL statement, or an array of queries, run in order. That single string (or the first query in the array) is the "authentication query" - the parameters `:username` and `:password` are available and should be evaluated by the query for authenticaion purposes. If the username/password is incorrect, the "authentication query" should return no rows. The rows returned represent attributes to be returned. +A `query` can be either a single string with an SQL statement, or an array of queries, run in order. That single string (or the first query in the array) is the "authentication query" - the parameters `:username` and `:password` are available and should be evaluated by the query for authentication purposes. If the username/password is incorrect, the "authentication query" should return no rows. The rows returned represent attributes to be returned. Taking this example schema: From 2286d34906b6084d90b44760007f363457a57016 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Fri, 17 Oct 2025 14:18:37 +1100 Subject: [PATCH 03/28] Implement sqlauth v2 - support multiple databases, multiple authentication queries, attribute queries selectable based on which authentication query was successful --- src/Auth/Source/SQL1Compat.php | 64 +++ src/Auth/Source/SQL2.php | 490 ++++++++++++++++++ tests/bootstrap.php | 5 + tests/src/Auth/Source/SQL1CompatTest.php | 15 + tests/src/Auth/Source/SQL1CompatWrapper.php | 35 ++ .../src/Auth/Source/SQL2MultipleAuthTest.php | 274 ++++++++++ tests/src/Auth/Source/SQL2SimpleTest.php | 267 ++++++++++ tests/src/Auth/Source/SQL2SingleAuthTest.php | 391 ++++++++++++++ tests/src/Auth/Source/SQL2Wrapper.php | 35 ++ tests/src/Auth/Source/SQLTest.php | 26 +- 10 files changed, 1590 insertions(+), 12 deletions(-) create mode 100644 src/Auth/Source/SQL1Compat.php create mode 100644 src/Auth/Source/SQL2.php create mode 100644 tests/src/Auth/Source/SQL1CompatTest.php create mode 100644 tests/src/Auth/Source/SQL1CompatWrapper.php create mode 100644 tests/src/Auth/Source/SQL2MultipleAuthTest.php create mode 100644 tests/src/Auth/Source/SQL2SimpleTest.php create mode 100644 tests/src/Auth/Source/SQL2SingleAuthTest.php create mode 100644 tests/src/Auth/Source/SQL2Wrapper.php diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php new file mode 100644 index 0000000..1c9de44 --- /dev/null +++ b/src/Auth/Source/SQL1Compat.php @@ -0,0 +1,64 @@ + [ + 'default' => [ + 'dsn' => $config['dsn'], + 'username' => $config['username'], + 'password' => $config['password'], + ] + ], + + 'auth_queries' => [ + 'default' => [ + 'database' => 'default', + 'query' => is_array($config['query']) ? $config['query'][0] : $config['query'], + ] + ], + ]; + + if (array_key_exists('username_regex', $config)) { + $v2config['auth_queries']['default']['username_regex'] = $config['username_regex']; + } + + if (is_array($config['query']) && count($config['query']) > 1) { + $v2config['attr_queries'] = []; + for ($i = 1; $i < count($config['query']); $i++) { + $v2config['attr_queries']['query' . $i] = [ + 'database' => 'default', + 'query' => $config['query'][$i], + ]; + } + } + + parent::__construct($info, $v2config); + } +} diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php new file mode 100644 index 0000000..f3f7452 --- /dev/null +++ b/src/Auth/Source/SQL2.php @@ -0,0 +1,490 @@ +authId . ' was provided and is expected to be an array. Instead it was: ' . + var_export($config['databases'], true)); + } + + if (empty($config['databases'])) { + throw new Exception('Required parameter \'databases\' for authentication source ' . + $this->authId . ' was provided but is an empty array.'); + } + + foreach ($config['databases'] as $dbname => $dbConfig) { + if (!is_array($dbConfig)) { + 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)); + } + foreach (['dsn', 'username', 'password'] as $param) { + if (!array_key_exists($param, $dbConfig)) { + throw new Exception('Database ' . + $dbname . ' is missing required attribute \'' . + $param . '\' for authentication source ' . + $this->authId); + } + if (!is_string($dbConfig[$param])) { + throw new Exception('Expected parameter \'' . $param . + '\' for authentication source ' . $this->authId . + ' to be a string. Instead it was: ' . + var_export($config[$param], true)); + } + } + + if (array_key_exists('options', $dbConfig) && !is_array($dbConfig['options'])) { + throw new Exception('Optional parameter \'options\' for authentication source ' . + $this->authId . ' was provided and is expected to be an array. Instead it was: ' . + var_export($dbConfig['options'], true)); + } + + $this->databases[$dbname] = [ + '_pdo' => null, // Will hold the PDO connection when connected + 'dsn' => $dbConfig['dsn'], + 'username' => $dbConfig['username'], + 'password' => $dbConfig['password'], + 'options' => $dbConfig['options'] ?? [], + ]; + } + } else { + throw new Exception('Missing required attribute \'databases\' for authentication source ' . $this->authId); + } + + // Check auth_queries configuration that all required parameters are present + if (array_key_exists('auth_queries', $config)) { + if (!is_array($config['auth_queries'])) { + throw new Exception('Required parameter \'auth_queries\' for authentication source ' . + $this->authId . ' was provided and is expected to be an array. Instead it was: ' . + var_export($config['auth_queries'], true)); + } + + if (empty($config['auth_queries'])) { + throw new Exception('Required parameter \'auth_queries\' for authentication source ' . + $this->authId . ' was provided but is an empty array.'); + } + + foreach ($config['auth_queries'] as $authQueryName => $authQueryConfig) { + if (!is_array($authQueryConfig)) { + 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)); + } + + foreach (['database', 'query'] as $param) { + if (!array_key_exists($param, $authQueryConfig)) { + throw new Exception('Auth query ' . + $authQueryName . ' is missing required attribute \'' . + $param . '\' for authentication source ' . + $this->authId); + } + if (!is_string($authQueryConfig[$param])) { + throw new Exception('Expected parameter \'' . $param . + '\' for authentication source \'' . $this->authId . '\'' . + ' to be a string. Instead it was: ' . + var_export($authQueryConfig[$param], true)); + } + } + + if (!array_key_exists($authQueryConfig['database'], $this->databases)) { + throw new Exception('Auth query ' . + $authQueryName . ' references unknown database \'' . + $authQueryConfig['database'] . '\' for authentication source ' . + $this->authId); + } + + $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 + 'database' => $authQueryConfig['database'], + 'query' => $authQueryConfig['query'], + ]; + + if (array_key_exists('username_regex', $authQueryConfig)) { + if (!is_string($authQueryConfig['username_regex'])) { + throw new Exception('Optional parameter \'username_regex\' for authentication source ' . + $this->authId . ' was provided and is expected to be a string. Instead it was: ' . + var_export($authQueryConfig['username_regex'], true)); + } + $this->authQueries[$authQueryName]['username_regex'] = $authQueryConfig['username_regex']; + } + + if (array_key_exists('extract_userid_from', $authQueryConfig)) { + if (!is_string($authQueryConfig['extract_userid_from'])) { + throw new Exception('Optional parameter \'extract_userid_from\' for authentication source ' . + $this->authId . ' was provided and is expected to be a string. Instead it was: ' . + var_export($authQueryConfig['extract_userid_from'], true)); + } + $this->authQueries[$authQueryName]['extract_userid_from'] = $authQueryConfig['extract_userid_from']; + } + + } + } else { + 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 + if (array_key_exists('attr_queries', $config)) { + if (!is_array($config['attr_queries'])) { + throw new Exception('Optional parameter \'attr_queries\' for authentication source ' . + $this->authId . ' was provided and is expected to be an array. Instead it was: ' . + var_export($config['attr_queries'], true)); + } + + foreach ($config['attr_queries'] as $attrQueryConfig) { + if (!is_array($attrQueryConfig)) { + throw new Exception('\'attr_queries\' parameter for authentication source ' . + $this->authId . ' is expected to be an array. Instead it was: ' . + var_export($attrQueryConfig, true)); + } + + foreach (['database', 'query'] as $param) { + if (!array_key_exists($param, $attrQueryConfig)) { + throw new Exception('Attribute query is missing required attribute \'' . + $param . '\' for authentication source ' . + $this->authId); + } + if (!is_string($attrQueryConfig[$param])) { + throw new Exception('Expected parameter \'' . $param . + '\' for authentication source \'' . $this->authId . '\'' . + ' to be a string. Instead it was: ' . + var_export($attrQueryConfig[$param], true)); + } + } + + $currentAttributeQuery = [ + 'database' => $attrQueryConfig['database'], + 'query' => $attrQueryConfig['query'], + ]; + + if (!array_key_exists($attrQueryConfig['database'], $this->databases)) { + throw new Exception('Attribute query references unknown database \'' . + $attrQueryConfig['database'] . '\' for authentication source ' . + $this->authId); + } + + if (array_key_exists('only_for_auth', $attrQueryConfig)) { + if (!is_array($attrQueryConfig['only_for_auth'])) { + throw new Exception('Optional parameter \'only_for_auth\' for authentication source ' . + $this->authId . ' was provided and is expected to be an array. Instead it was: ' . + var_export($attrQueryConfig['only_for_auth'], true)); + } + foreach ($attrQueryConfig['only_for_auth'] as $authQueryName) { + if (!is_string($authQueryName)) { + throw new Exception('Each entry in the \'only_for_auth\' array for authentication source ' . + $this->authId . ' is expected to be a string. Instead it was: ' . + var_export($authQueryName, true)); + } + if (!array_key_exists($authQueryName, $this->authQueries)) { + throw new Exception('Attribute query references unknown auth query \'' . + $authQueryName . '\' for authentication source ' . + $this->authId); + } + } + $currentAttributeQuery['only_for_auth'] = $attrQueryConfig['only_for_auth']; + } + + $this->attributesQueries[] = $currentAttributeQuery; + } + } + } + + /** + * Create a database connection. + * + * @return \PDO The database connection. + */ + protected function connect(string $dbname): PDO + { + if (!array_key_exists($dbname, $this->databases)) { + throw new Exception('sqlauth:' . $this->authId . ': Attempt to connect to unknown database \'' . + $dbname . '\''); + } + if ($this->databases[$dbname]['_pdo'] !== null) { + // 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'] + ); + } catch (PDOException $e) { + // Obfuscate the password if it's part of the 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()); + } + + $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $driver = explode(':', $this->databases[$dbname]['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; + } + + Logger::debug('sqlauth:' . $this->authId . ': Connected to database ' . $dbname); + $this->databases[$dbname]['_pdo'] = $db; + 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()); + } + } + + /** + * 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 { + + $attributes = []; + $winning_auth_query = null; + + // 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)) { + Logger::debug('sqlauth:' . $this->authId . ': Skipping auth query ' . $queryname . + ' because username ' . $username . ' does not match username_regex ' . + $queryConfig['username_regex']); + continue; + } + + Logger::debug('sqlauth:' . $this->authId . ': Trying auth query ' . $queryname); + + $db = $this->connect($queryConfig['database']); + + try { + $data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username, 'password' => $password]); + } catch (PDOException $e) { + Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' failed with error: ' . $e->getMessage()); + continue; + } + + // If we got any rows, the authentication succeeded. If not, try the next query. + if (count($data) > 0) { + Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' succeeded with ' . count($data) . ' rows'); + $queryConfig['_winning_auth_query'] = true; + if (array_key_exists('extract_userid_from', $queryConfig)) { + $queryConfig['_extracted_userid'] = $data[0][$queryConfig['extract_userid_from']]; + } + $winning_auth_query = $queryname; + $this->extractAttributes($attributes, $data, []); + + // 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'); + } + } + + if (empty($attributes)) { + // No auth query succeeded + Logger::error('sqlauth:' . $this->authId . ': No auth query succeeded. Probably wrong username/password.'); + throw new Error\Error('WRONGUSERPASS'); + } + + // Run attribute queries. Each attribute query can specify which auth queries it applies to. + 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 . ': 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']] : + ['username' => $username]; + $data = $this->executeQuery($db, $attrQueryConfig['query'], $params); + } catch (PDOException $e) { + Logger::error('sqlauth:' . $this->authId . ': Attribute query ' . $attrQueryConfig['query'] . + ' failed with error: ' . $e->getMessage()); + continue; + } + + Logger::debug('sqlauth:' . $this->authId . ': Attribute query ' . $attrQueryConfig['query'] . + ' returned ' . count($data) . ' rows'); + + $this->extractAttributes($attributes, $data, []); + } else { + Logger::debug('sqlauth:' . $this->authId . ': Skipping attribute query ' . $attrQueryConfig['query'] . + ' because it does not apply to winning auth query ' . $winning_auth_query); + } + } + + // At the end, disconnect from all databases + $db = null; + foreach ($this->databases as $dbname => $dbConfig) { + if ($dbConfig['_pdo'] !== null) { + $this->databases[$dbname]['_pdo'] = null; + Logger::debug('sqlauth:' . $this->authId . ': Disconnected from database ' . $dbname); + } + } + + Logger::info('sqlauth:' . $this->authId . ': Attributes: ' . implode(',', array_keys($attributes))); + + return $attributes; + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index b21ae98..77a2702 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -7,8 +7,13 @@ // Load our wrapper class to get around login() being declared protected in SQL.php require_once($projectRoot . '/tests/src/Auth/Source/SQLWrapper.php'); +require_once($projectRoot . '/tests/src/Auth/Source/SQL2Wrapper.php'); +require_once($projectRoot . '/tests/src/Auth/Source/SQL1CompatWrapper.php'); require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyWrapper.php'); +// Get around SQL1CompatTest extending SQLTest and not being able to find SQLTest +require_once($projectRoot . '/tests/src/Auth/Source/SQLTest.php'); + // Symlink module into ssp vendor lib so that templates and urls can resolve correctly $linkPath = $projectRoot . '/vendor/simplesamlphp/simplesamlphp/modules/sqlauth'; if (file_exists($linkPath) === false) { diff --git a/tests/src/Auth/Source/SQL1CompatTest.php b/tests/src/Auth/Source/SQL1CompatTest.php new file mode 100644 index 0000000..ff8d03f --- /dev/null +++ b/tests/src/Auth/Source/SQL1CompatTest.php @@ -0,0 +1,15 @@ + $info + * @param array $config + */ + public function __construct(array $info, array $config) + { + parent::__construct($info, $config); + } + + + /** + * @return array + */ + public function callLogin(string $username, string $password): array + { + return $this->login($username, $password); + } +} diff --git a/tests/src/Auth/Source/SQL2MultipleAuthTest.php b/tests/src/Auth/Source/SQL2MultipleAuthTest.php new file mode 100644 index 0000000..de24e9c --- /dev/null +++ b/tests/src/Auth/Source/SQL2MultipleAuthTest.php @@ -0,0 +1,274 @@ + 'testAuthId']; + private array $config = [ + "databases" => [ + "studentsdb" => [ + "dsn" => 'sqlite:file:studentsdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + "physics_staffdb" => [ + "dsn" => 'sqlite:file:physics_staffdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + "staffdb" => [ + "dsn" => 'sqlite:file:staffdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + ], + "auth_queries" => [ + "auth_query_students" => [ + "database" => "studentsdb", + "query" => "select studentid, givenName, lastName, email, course, year from students where email=:username and password=:password", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@student\.example\.edu$/', + "extract_userid_from" => 'studentid', + ], + + // We specify the physics_staffdb auth query before the staffdb one, so that if a user exists in both + // staff databases, they will be authenticated against the physics_staffdb one. + "auth_query_physics_staff" => [ + "database" => "physics_staffdb", + "query" => "select psid as uid, CASE WHEN typically_wears_matching_socks=true THEN 'true' ELSE 'false' END as \"typically_wears_matching_socks\" from staff where email=:username and password=:password", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', + "extract_userid_from" => 'uid', + ], + + "auth_query_staff" => [ + "database" => "staffdb", + "query" => "select uid, givenName, lastName, email, department from staff where email=:username and password=:password", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', + "extract_userid_from" => 'uid', + ], + ], + "attr_queries" => [ + [ + 'database' => 'staffdb', + 'query' => "select givenName, lastName, email, department from staff where uid=:userid", + 'only_for_auth' => ['auth_query_staff', 'auth_query_physics_staff'], + ], + [ + 'database' => 'staffdb', + 'query' => "select role from staff_roles where uid=:userid", + 'only_for_auth' => ['auth_query_staff', 'auth_query_physics_staff'], + ], + [ + 'database' => 'physics_staffdb', + 'query' => "select qualification from staff_qualifications where psid=:userid order by qualification desc", + 'only_for_auth' => ['auth_query_physics_staff'], + ], + [ + 'database' => 'studentsdb', + 'query' => "select unit_code from units_enrolled where studentid=:userid", + 'only_for_auth' => ['auth_query_students'], + ], + ], + ]; + + public static function setUpBeforeClass(): void + { + // Students database + $studentsPdo = new PDO('sqlite:file:studentsdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $studentsPdo->exec("DROP TABLE IF EXISTS students"); + // Create tables + $studentsPdo->exec(" + CREATE TABLE students ( + studentid int NOT NULL PRIMARY KEY, + givenName TEXT NOT NULL, + lastName TEXT NOT NULL, + email TEXT NOT NULL, + password TEXT NOT NULL, + course TEXT NOT NULL, + year int NOT NULL + ) + "); + + // Create test data for students table + $students = [ + [1, 'Alice', 'Gibson', 'alice.gibson@student.example.edu', 'password', 'Engineering', 1], + [2, 'Bob', 'Builder', 'bob.builder@student.example.edu', 'password', 'Architecture', 2], + [3, 'Trudy', 'Tester', 'trudy.tester@student.example.edu', 'password', 'Computer Science', 3], + ]; + foreach ($students as $student) { + $studentsPdo->prepare("INSERT INTO students VALUES (?,?,?,?,?,?,?)") + ->execute($student); + } + + $studentsPdo->exec("DROP TABLE IF EXISTS units_enrolled"); + $studentsPdo->exec(" + CREATE TABLE units_enrolled ( + studentid int NOT NULL, + unit_code TEXT NOT NULL, + PRIMARY KEY (studentid, unit_code) + ) + "); + $enrollments = [ + [1, 'ENG101'], + [1, 'ENG102'], + [2, 'ARCH201'], + [3, 'CS101'], + [3, 'CS102'], + ]; + foreach ($enrollments as $e) { + $studentsPdo->prepare("INSERT INTO units_enrolled VALUES (?,?)") + ->execute($e); + } + + + // Staff database + $staffPdo = new PDO('sqlite:file:staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $staffPdo->exec("DROP TABLE IF EXISTS staff"); + $staffPdo->exec(" + CREATE TABLE staff ( + uid int NOT NULL PRIMARY KEY, + givenName TEXT NOT NULL, + lastName TEXT NOT NULL, + email TEXT NOT NULL, + password TEXT DEFAULT NULL, + department TEXT NOT NULL + ) + "); + $staff = [ + [1, 'Eve', 'Evans', 'eve.evans@example.edu', 'password', 'Mathematics'], + [2, 'Mallory', 'Mallory', 'mallory.mallory@example.edu', 'password', 'Physics'], + ]; + foreach ($staff as $s) { + $staffPdo->prepare("INSERT INTO staff VALUES (?,?,?,?,?,?)") + ->execute($s); + } + + $staffPdo->exec("DROP TABLE IF EXISTS staff_roles"); + $staffPdo->exec(" + CREATE TABLE staff_roles ( + uid int NOT NULL, + role TEXT NOT NULL, + PRIMARY KEY (uid, role) + ) + "); + $roles = [ + [1, 'lecturer'], + [2, 'professor'], + ]; + foreach ($roles as $r) { + $staffPdo->prepare("INSERT INTO staff_roles VALUES (?,?)") + ->execute($r); + } + + // Physics staff database + $physicsStaffPdo = new PDO('sqlite:file:physics_staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $physicsStaffPdo->exec("DROP TABLE IF EXISTS staff"); + $physicsStaffPdo->exec(" + CREATE TABLE staff ( + psid int NOT NULL PRIMARY KEY, + email TEXT NOT NULL, + password TEXT DEFAULT NULL, + typically_wears_matching_socks BOOLEAN NOT NULL + ) + "); + $physicsStaff = [ + [2, 'mallory.mallory@example.edu', 'password', false], + ]; + foreach ($physicsStaff as $ps) { + $physicsStaffPdo->prepare("INSERT INTO staff VALUES (?,?,?,?)") + ->execute($ps); + } + + $physicsStaffPdo->exec("DROP TABLE IF EXISTS staff_qualifications"); + $physicsStaffPdo->exec(" + CREATE TABLE staff_qualifications ( + psid int NOT NULL, + qualification TEXT NOT NULL, + PRIMARY KEY (psid, qualification) + ) + "); + $physicsStaff = [ + [2, 'PhD in Physics'], + [2, 'MSc in Astrophysics'], + ]; + foreach ($physicsStaff as $ps) { + $physicsStaffPdo->prepare("INSERT INTO staff_qualifications VALUES (?,?)") + ->execute($ps); + } + } + + public function testStudentLoginSuccess(): void + { + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice.gibson@student.example.edu', 'password'); + asort($ret); + $this->assertCount(7, $ret); + $this->assertCount(2, $ret['unit_code']); + $this->assertEquals($ret, [ + 'studentid' => ['1'], + 'givenName' => ["Alice"], + 'lastName' => ["Gibson"], + 'email' => ['alice.gibson@student.example.edu'], + 'course' => ["Engineering"], + 'year' => ["1"], + 'unit_code' => ["ENG101", "ENG102"], + ]); + } + + public function testNonPhysicsStaffLoginSuccess(): void + { + // Correct username/password for non-physics staff + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('eve.evans@example.edu', 'password'); + asort($ret); + $this->assertCount(6, $ret); + $this->assertCount(1, $ret['role']); + $this->assertEquals($ret, [ + 'uid' => ['1'], + 'givenName' => ["Eve"], + 'lastName' => ["Evans"], + 'email' => ['eve.evans@example.edu'], + 'role' => ['lecturer'], + 'department' => ['Mathematics'], + ]); + } + + public function testPhysicsStaffLoginSuccess(): void + { + // Correct username/password for physics staff + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('mallory.mallory@example.edu', 'password'); + asort($ret); + var_dump($ret); + $this->assertCount(8, $ret); + $this->assertCount(1, $ret['role']); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'givenName' => ["Mallory"], + 'lastName' => ["Mallory"], + 'email' => ['mallory.mallory@example.edu'], + 'role' => ['professor'], + 'department' => ['Physics'], + 'qualification' => ['PhD in Physics', 'MSc in Astrophysics'], + 'typically_wears_matching_socks' => ['false'], + ]); + } +} diff --git a/tests/src/Auth/Source/SQL2SimpleTest.php b/tests/src/Auth/Source/SQL2SimpleTest.php new file mode 100644 index 0000000..596e6e7 --- /dev/null +++ b/tests/src/Auth/Source/SQL2SimpleTest.php @@ -0,0 +1,267 @@ + 'testAuthId']; + private array $config = [ + "databases" => [ + "defaultdb" => [ + "dsn" => 'sqlite:file:defaultdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + ], + "auth_queries" => [ + "auth_query" => [ + "database" => "defaultdb", + "query" => null, // Filled out by each test case + ], + ], + ]; + + public static function setUpBeforeClass(): void + { + $pdo = new PDO('sqlite:file:defaultdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $pdo->exec("DROP TABLE IF EXISTS users"); + $pdo->exec("DROP TABLE IF EXISTS usergroups"); + + // Create tables + $pdo->exec(" + CREATE TABLE users ( + uid VARCHAR(30) NOT NULL PRIMARY KEY, + password TEXT NOT NULL, + givenName TEXT NOT NULL, + email TEXT NOT NULL + ) + "); + $pdo->exec(" + CREATE TABLE usergroups ( + uid VARCHAR(30) NOT NULL REFERENCES users (uid) ON DELETE CASCADE ON UPDATE CASCADE, + groupname VARCHAR(30) NOT NULL, + UNIQUE(uid, groupname) + ) + "); + + // Create test data for users table + $users = [ + ['alice', 'password', 'Alice', 'alice@example.com'], + ['bob', 'password', 'Bob', 'bob@example.com'], + ['trudy', 'password', 'Trudy', 'trudy@example.com'], + ]; + foreach ($users as $user) { + $pdo->prepare("INSERT INTO users VALUES (?,?,?,?)") + ->execute($user); + } + + // Create test data for usergroups table + $groups = [ + ['alice', 'users'], + ['alice', 'staff'], + ['bob', 'users'], + ['bob', 'students'], + ['trudy', 'users'], + ['trudy', 'students'], + ['trudy', 'tutors'], + ]; + foreach ($groups as $group) { + $pdo->prepare("INSERT INTO usergroups VALUES (?,?)") + ->execute($group); + } + } + + public function testBasicSingleSuccess(): void + { + // Correct username/password + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + $this->assertCount(2, $ret); + $this->assertEquals($ret, [ + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + ]); + } + + public function testBasicSingleUsernameRegexSuccess(): void + { + // Correct username/password + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['username_regex'] = '/^[a-z]+$/'; // Username must be a single lower case word + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + $this->assertCount(2, $ret); + $this->assertEquals($ret, [ + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + ]); + } + + public function testBasicSingleUsernameRegexFailedLogin(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + // Correct username/password, but doesn't match the username regex + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + $this->assertCount(0, $ret); + } + + public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + // Correct username/password, but doesn't match the username regex + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('henry', 'password'); + asort($ret); + $this->assertCount(0, $ret); + } + + public function testBasicSingleFailedLogin(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + // Wrong username/password + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $this->assertCount(0, $ret); + } + + public function testJoinSingleSuccess(): void + { + // Correct username/password + $this->config['auth_queries']['auth_query']['query'] = " + select u.givenName, u.email, ug.groupname + from users u left join usergroups ug on (u.uid=ug.uid) + where u.uid=:username and u.password=:password"; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + asort($ret['groupname']); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + 'groupname' => ['students', 'users'], + ]); + } + + public function testJoinSingleFailedLogin(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + // Wrong username/password + $this->config['auth_queries']['auth_query']['query'] = " + select u.givenName, u.email, ug.groupname + from users u left join usergroups ug on (u.uid=ug.uid) + where u.uid=:username and u.password=:password"; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $this->assertCount(0, $ret); + } + + public function testMultiQuerySuccess(): void + { + // Correct username/password + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email from users where uid=:username and password=:password"; + $this->config['attr_queries'] = [ + [ + 'database' => 'defaultdb', + 'query' => "select groupname from usergroups where uid=:username", + ] + ]; + + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + asort($ret['groupname']); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + 'groupname' => ['students', 'users'], + ]); + } + + public function testMultiQueryFailedLogin(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + // Wrong username/password + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email from users where uid=:username and password=:password"; + $this->config['attr_queries'] = [ + [ + 'database' => 'defaultdb', + 'query' => "select groupname from usergroups where uid=:username", + ] + ]; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $this->assertCount(0, $ret); + } + + public function testMultiQuerySubsequentNoRowsSuccess(): void + { + // Correct username/password. Second query returns no rows, third query returns just one row + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email from users where uid=:username and password=:password"; + $this->config['attr_queries'] = [ + [ + 'database' => 'defaultdb', + 'query' => "select groupname from usergroups where uid=:username and groupname like '%nomatch%'", + ], + [ + 'database' => 'defaultdb', + 'query' => "select groupname from usergroups where uid=:username and groupname like 'stud%'", + ], + ]; + + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + asort($ret['groupname']); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + 'groupname' => ['students'], + ]); + } + + public function testMultiQuerySubsequentAppendSuccess(): void + { + // Correct username/password. Second query returns a row, third query appends one row + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email from users where uid=:username and password=:password"; + $this->config['attr_queries'] = [ + [ + 'database' => 'defaultdb', + 'query' => "select groupname from usergroups where uid=:username and groupname like 'stud%'", + ], + [ + 'database' => 'defaultdb', + 'query' => "select groupname from usergroups where uid=:username and groupname like '%sers'", + ], + ]; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + asort($ret); + asort($ret['groupname']); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + 'groupname' => ['students', 'users'], + ]); + } +} diff --git a/tests/src/Auth/Source/SQL2SingleAuthTest.php b/tests/src/Auth/Source/SQL2SingleAuthTest.php new file mode 100644 index 0000000..ae93822 --- /dev/null +++ b/tests/src/Auth/Source/SQL2SingleAuthTest.php @@ -0,0 +1,391 @@ + 'testAuthId']; + + /* Different tests require different combinations of databases, auth queries and attr queries. + * This function returns a config with the requested number of each. + */ + private function getConfig(int $numDatabases, int $numAuthQueries, array $authQueryAttributes, int $numAttrQueries): array + { + $config = [ + "databases" => [ + "authdb" => [ + "dsn" => 'sqlite:file:authdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + "staffdb" => [ + "dsn" => 'sqlite:file:staffdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + "studentsdb" => [ + "dsn" => 'sqlite:file:studentsdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + ], + "auth_queries" => [ + "auth_query_id" => [ + "database" => "authdb", + "query" => "select uid, givenName, email from users where uid=:username and password=:password", + "username_regex" => '/^\\d+$/', + "extract_userid_from" => 'uid', + ], + "auth_query_email" => [ + "database" => "authdb", + "query" => "select uid, givenName, email from users where email=:username and password=:password", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/', + "extract_userid_from" => 'uid', + ], + ], + "attr_queries" => [ + [ + 'database' => 'staffdb', + 'query' => "select department, role from staff where uid=:userid", + ], + [ + 'database' => 'studentsdb', + 'query' => "select course, year from students where uid=:userid", + ], + [ + 'database' => 'studentsdb', + 'query' => "select unit_code from units_enrolled where uid=:userid", + ], + ], + ]; + + $ret = []; + $ret['databases'] = array_slice($config['databases'], 0, $numDatabases); + $ret['auth_queries'] = array_slice($config['auth_queries'], 0, $numAuthQueries); + $ret['attr_queries'] = array_slice($config['attr_queries'], 0, $numAttrQueries); + + // Only return the auth query attributes that were requested + foreach ($ret['auth_queries'] as $authQueryName => $authQuery) { + // Firstly, go through each auth query, removing any that weren't requested. + foreach (array_keys($authQuery) as $authQueryKey) { + if (!in_array($authQueryKey, $authQueryAttributes)) { + unset($ret['auth_queries'][$authQueryName][$authQueryKey]); + } + } + + // Then check all of the requested attributes are in each auth query. + foreach ($authQueryAttributes as $attribute) { + if (!array_key_exists($attribute, $authQuery)) { + throw new \InvalidArgumentException("Auth query attribute \"$attribute\" not found in auth query \"$authQueryName\""); + } + } + } + + return $ret; + } + + public static function setUpBeforeClass(): void + { + // Auth database + $authPdo = new PDO('sqlite:file:authdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $authPdo->exec("DROP TABLE IF EXISTS users"); + + // Create tables + $authPdo->exec(" + CREATE TABLE users ( + uid int NOT NULL PRIMARY KEY, + password TEXT NOT NULL, + givenName TEXT NOT NULL, + email TEXT NOT NULL + ) + "); + + // Create test data for users table + $users = [ + [1, 'password', 'Alice', 'alice@example.com'], + [2, 'password', 'Bob', 'bob@example.com'], + [3, 'password', 'Trudy', 'trudy@example.com'], + [4, 'password', 'Eve', 'eve@example.com'], + [5, 'password', 'Mallory', 'mallory@example.com'], + ]; + foreach ($users as $user) { + $authPdo->prepare("INSERT INTO users VALUES (?,?,?,?)") + ->execute($user); + } + + // Staff database + $staffPdo = new PDO('sqlite:file:staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $staffPdo->exec("DROP TABLE IF EXISTS staff"); + $staffPdo->exec(" + CREATE TABLE staff ( + uid int NOT NULL PRIMARY KEY, + department TEXT NOT NULL, + role TEXT NOT NULL + ) + "); + $staff = [ + [1, 'HR', 'Manager'], + [2, 'IT', 'Developer'], + ]; + foreach ($staff as $s) { + $staffPdo->prepare("INSERT INTO staff VALUES (?,?,?)") + ->execute($s); + } + + // Students database + $studentsPdo = new PDO('sqlite:file:studentsdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $studentsPdo->exec("DROP TABLE IF EXISTS students"); + $studentsPdo->exec(" + CREATE TABLE students ( + uid int NOT NULL PRIMARY KEY, + course TEXT NOT NULL, + year int NOT NULL + ) + "); + $students = [ + [3, 'Computer Science', 2], + [4, 'Mathematics', 1], + [5, 'Physics', 3], + ]; + foreach ($students as $s) { + $studentsPdo->prepare("INSERT INTO students VALUES (?,?,?)") + ->execute($s); + } + + $studentsPdo->exec("DROP TABLE IF EXISTS units_enrolled"); + + $studentsPdo->exec(" + CREATE TABLE units_enrolled ( + uid int NOT NULL, + unit_code TEXT NOT NULL, + PRIMARY KEY (uid, unit_code) + ) + "); + $enrollments = [ + [3, 'CS101'], + [3, 'CS102'], + [5, 'PHYS101'], + ]; + foreach ($enrollments as $e) { + $studentsPdo->prepare("INSERT INTO units_enrolled VALUES (?,?)") + ->execute($e); + } + } + + public function testSingleAuthQueryOnlySuccess(): void + { + $config = $this->getConfig(1, 1, ['database', 'query'], 0); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('2', 'password'); + asort($ret); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + ]); + } + + public function testSingleAuthQueryOnlyPasswordFailure(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + + $config = $this->getConfig(1, 1, ['database', 'query'], 0); + + // Wrong password + (new SQL2Wrapper($this->info, $config))->callLogin('2', 'wrongpassword'); + } + + public function testSingleAuthQueryOnlyUsernameFailure(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + + $config = $this->getConfig(1, 1, ['database', 'query'], 0); + + // Nonexistent username + (new SQL2Wrapper($this->info, $config))->callLogin('201', 'password'); + } + + public function testSingleAuthQueryOnlySuccessWithRegex(): void + { + $config = $this->getConfig(1, 1, ['database', 'query', 'username_regex'], 0); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('2', 'password'); + asort($ret); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + ]); + } + + public function testSingleAuthQueryOnlyFailureDueToRegex(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + $config = $this->getConfig(1, 1, ['database', 'query', 'username_regex'], 0); + + // Correct username/password + (new SQL2Wrapper($this->info, $config))->callLogin('bad-username', 'password'); + } + + public function testSingleAuthQuerySingleAttrQuerySuccess(): void + { + $config = $this->getConfig(2, 1, ['database', 'query', 'extract_userid_from'], 1); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('2', 'password'); + asort($ret); + + $this->assertCount(5, $ret); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + 'department' => ['IT'], + 'role' => ['Developer'], + ]); + } + + public function testSingleAuthQuerySingleAttrQueryPasswordFailure(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + + $config = $this->getConfig(2, 1, ['database', 'query'], 1); + + // Wrong password + (new SQL2Wrapper($this->info, $config))->callLogin('2', 'wrongpassword'); + } + + + public function testMultipleAuthQueryNoAttrQueryUsernameIsIdSuccess(): void + { + $config = $this->getConfig(2, 2, ['database', 'query', 'username_regex'], 0); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('2', 'password'); + asort($ret); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + ]); + } + + public function testMultipleAuthQueryNoAttrQueryUsernameIsEmailSuccess(): void + { + $config = $this->getConfig(2, 2, ['database', 'query', 'username_regex'], 0); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('bob@example.com', 'password'); + asort($ret); + $this->assertCount(3, $ret); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + ]); + } + + public function testMultipleAuthQueryNoAttrQueryUsernameIsEmailFailure(): void + { + $this->expectException(\SimpleSAML\Error\Error::class); + + $config = $this->getConfig(2, 2, ['database', 'query', 'username_regex'], 0); + + // Correct username/password + (new SQL2Wrapper($this->info, $config))->callLogin('nonexistent@example.com', 'password'); + } + + public function testMultipleAuthQuerySingleAttrQueryUsernameIsEmailSuccess(): void + { + $config = $this->getConfig(2, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 1); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('bob@example.com', 'password'); + asort($ret); + + $this->assertCount(5, $ret); + $this->assertEquals($ret, [ + 'uid' => ['2'], + 'email' => ['bob@example.com'], + 'givenName' => ["Bob"], + 'department' => ['IT'], + 'role' => ['Developer'], + ]); + } + + public function testMultipleAuthQueryStudentWithMultipleEnrolmentsSuccess(): void + { + $config = $this->getConfig(3, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 3); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('3', 'password'); + asort($ret); + $this->assertCount(6, $ret); + $this->assertCount(2, $ret['unit_code']); + $this->assertEquals($ret, [ + 'uid' => ['3'], + 'email' => ['trudy@example.com'], + 'givenName' => ["Trudy"], + 'course' => ['Computer Science'], + 'year' => ['2'], + 'unit_code' => ['CS101', 'CS102'], + ]); + } + + public function testMultipleAuthQueryStudentWithNoEnrolmentsSuccess(): void + { + $config = $this->getConfig(3, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 3); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('4', 'password'); + asort($ret); + $this->assertCount(5, $ret); + $this->assertArrayNotHasKey('unit_code', $ret); + $this->assertEquals($ret, [ + 'uid' => ['4'], + 'email' => ['eve@example.com'], + 'givenName' => ["Eve"], + 'course' => ['Mathematics'], + 'year' => ['1'], + // No units_enrolled, 'unit_code' is not set + ]); + } + + public function testMultipleAuthQueryStudentWithSingleEnrolmentSuccess(): void + { + $config = $this->getConfig(3, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 3); + + // Correct username/password + $ret = (new SQL2Wrapper($this->info, $config))->callLogin('5', 'password'); + asort($ret); + $this->assertCount(6, $ret); + $this->assertCount(1, $ret['unit_code']); + $this->assertEquals($ret, [ + 'uid' => ['5'], + 'email' => ['mallory@example.com'], + 'givenName' => ["Mallory"], + 'course' => ['Physics'], + 'year' => ['3'], + 'unit_code' => ['PHYS101'], + ]); + } +} diff --git a/tests/src/Auth/Source/SQL2Wrapper.php b/tests/src/Auth/Source/SQL2Wrapper.php new file mode 100644 index 0000000..0badfe9 --- /dev/null +++ b/tests/src/Auth/Source/SQL2Wrapper.php @@ -0,0 +1,35 @@ + $info + * @param array $config + */ + public function __construct(array $info, array $config) + { + parent::__construct($info, $config); + } + + + /** + * @return array + */ + public function callLogin(string $username, string $password): array + { + return $this->login($username, $password); + } +} diff --git a/tests/src/Auth/Source/SQLTest.php b/tests/src/Auth/Source/SQLTest.php index cb773f3..655b3dc 100644 --- a/tests/src/Auth/Source/SQLTest.php +++ b/tests/src/Auth/Source/SQLTest.php @@ -6,7 +6,6 @@ use PDO; use PHPUnit\Framework\TestCase; -use SimpleSAML\Test\Module\sqlauth\Auth\Source\SQLWrapper; /** * Test for the core:AttributeLimit filter. @@ -15,6 +14,9 @@ */ class SQLTest extends TestCase { + // Subclasses can override this to test other wrapper classes + protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\SQLWrapper'; + /** @var array */ private array $info = ['AuthId' => 'testAuthId']; @@ -81,7 +83,7 @@ public function testBasicSingleSuccess(): void { // Correct username/password $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); $this->assertEquals($ret, [ @@ -95,7 +97,7 @@ public function testBasicSingleUsernameRegexSuccess(): void // Correct username/password $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; $this->config['username_regex'] = '/^[a-z]+$/'; // Username must be a single lower case word - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); $this->assertEquals($ret, [ @@ -110,7 +112,7 @@ public function testBasicSingleUsernameRegexFailedLogin(): void // Correct username/password, but doesn't match the username regex $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; $this->config['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(0, $ret); } @@ -121,7 +123,7 @@ public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void // Correct username/password, but doesn't match the username regex $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; $this->config['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('henry', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('henry', 'password'); asort($ret); $this->assertCount(0, $ret); } @@ -131,7 +133,7 @@ public function testBasicSingleFailedLogin(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -142,7 +144,7 @@ public function testJoinSingleSuccess(): void select u.givenName, u.email, ug.groupname from users u left join usergroups ug on (u.uid=ug.uid) where u.uid=:username and u.password=:password"; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); @@ -161,7 +163,7 @@ public function testJoinSingleFailedLogin(): void select u.givenName, u.email, ug.groupname from users u left join usergroups ug on (u.uid=ug.uid) where u.uid=:username and u.password=:password"; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -172,7 +174,7 @@ public function testMultiQuerySuccess(): void "select givenName, email from users where uid=:username and password=:password", "select groupname from usergroups where uid=:username", ]; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); @@ -191,7 +193,7 @@ public function testMultiQueryFailedLogin(): void "select givenName, email from users where uid=:username and password=:password", "select groupname from usergroups where uid=:username", ]; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -203,7 +205,7 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void "select groupname from usergroups where uid=:username and groupname like '%nomatch%'", "select groupname from usergroups where uid=:username and groupname like 'stud%'", ]; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); @@ -222,7 +224,7 @@ public function testMultiQuerySubsequentAppendSuccess(): void "select groupname from usergroups where uid=:username and groupname like 'stud%'", "select groupname from usergroups where uid=:username and groupname like '%sers'", ]; - $ret = (new SQLWrapper($this->info, $this->config))->callLogin('bob', 'password'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); From facb3288aee1438d4bc0cd8f4d0a67ecfdf4e420 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 20 Oct 2025 13:43:41 +1100 Subject: [PATCH 04/28] Support password_verify() in version 2. Provide v1 compat interfaces. --- src/Auth/Source/PasswordVerify1Compat.php | 65 +++++++++ src/Auth/Source/SQL1Compat.php | 5 - src/Auth/Source/SQL2.php | 69 +++++++++- tests/bootstrap.php | 8 +- .../Auth/Source/PasswordVerify1CompatTest.php | 15 ++ .../Source/PasswordVerify1CompatWrapper.php | 35 +++++ tests/src/Auth/Source/PasswordVerifyTest.php | 23 ++-- .../src/Auth/Source/SQL2MultipleAuthTest.php | 129 +++++++++--------- .../SQL2PasswordVerifyMultipleAuthTest.php | 52 +++++++ .../Source/SQL2PasswordVerifySimpleTest.php | 47 +++++++ .../SQL2PasswordVerifySingleAuthTest.php | 49 +++++++ tests/src/Auth/Source/SQL2SimpleTest.php | 67 +++++---- tests/src/Auth/Source/SQL2SingleAuthTest.php | 9 +- 13 files changed, 465 insertions(+), 108 deletions(-) create mode 100644 src/Auth/Source/PasswordVerify1Compat.php create mode 100644 tests/src/Auth/Source/PasswordVerify1CompatTest.php create mode 100644 tests/src/Auth/Source/PasswordVerify1CompatWrapper.php create mode 100644 tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php create mode 100644 tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php create mode 100644 tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php diff --git a/src/Auth/Source/PasswordVerify1Compat.php b/src/Auth/Source/PasswordVerify1Compat.php new file mode 100644 index 0000000..d017c21 --- /dev/null +++ b/src/Auth/Source/PasswordVerify1Compat.php @@ -0,0 +1,65 @@ + [ + 'default' => [ + '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', + ] + ], + ]; + + if (array_key_exists('username_regex', $config)) { + $v2config['auth_queries']['default']['username_regex'] = $config['username_regex']; + } + + // Override the default passwordhash column if configured + if (array_key_exists('passwordhash_column', $config)) { + $v2config['auth_queries']['default']['password_verify_hash_column'] = $config['passwordhash_column']; + } + + if (is_array($config['query']) && count($config['query']) > 1) { + $v2config['attr_queries'] = []; + for ($i = 1; $i < count($config['query']); $i++) { + $v2config['attr_queries']['query' . $i] = [ + 'database' => 'default', + 'query' => $config['query'][$i], + ]; + } + } + + parent::__construct($info, $v2config); + } +} diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php index 1c9de44..959f7d3 100644 --- a/src/Auth/Source/SQL1Compat.php +++ b/src/Auth/Source/SQL1Compat.php @@ -5,11 +5,6 @@ namespace SimpleSAML\Module\sqlauth\Auth\Source; /** - * Simple SQL authentication source - * - * This class is an example authentication source which authenticates an user - * against a SQL database. - * * @package SimpleSAMLphp */ diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index f3f7452..58f7c1b 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -179,6 +179,14 @@ public function __construct(array $info, array $config) $this->authQueries[$authQueryName]['extract_userid_from'] = $authQueryConfig['extract_userid_from']; } + 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 ' . + $this->authId . ' was provided and is expected to be a string. Instead it was: ' . + var_export($authQueryConfig['password_verify_hash_column'], true)); + } + $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); @@ -408,7 +416,14 @@ protected function login( $db = $this->connect($queryConfig['database']); try { - $data = $this->executeQuery($db, $queryConfig['query'], ['username' => $username, 'password' => $password]); + 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]); + } } catch (PDOException $e) { Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . ' failed with error: ' . $e->getMessage()); @@ -417,14 +432,64 @@ protected function login( // If we got any rows, the authentication succeeded. If not, try the next query. if (count($data) > 0) { + /* This is where we need to run password_verify() if we are using password_verify() to + * authenticate hashed passwords that are only stored in the database. */ + if (array_key_exists('password_verify_hash_column', $queryConfig)) { + $hashColumn = $queryConfig['password_verify_hash_column']; + if (!array_key_exists($hashColumn, $data[0])) { + Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' did not return expected hash column \'' . $hashColumn . '\''); + throw new Error\Error('WRONGUSERPASS'); + } + + $passwordHash = null; + foreach ($data as $row) { + if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) { + Logger::error(sprintf( + 'sqlauth:%s: column `%s` must be in every result tuple.', + $this->authId, + $hashColumn, + )); + throw new Error\Error('WRONGUSERPASS'); + } + if ($passwordHash === null) { + $passwordHash = $row[$hashColumn]; + } elseif ($passwordHash != $row[$hashColumn]) { + Logger::error(sprintf( + 'sqlauth:%s: column %s must be THE SAME in every result tuple.', + $this->authId, + $hashColumn, + )); + throw new Error\Error('WRONGUSERPASS'); + } + } + + if (($passwordHash === null) || (!password_verify($password, $passwordHash))) { + Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' password verification failed'); + /* Authentication with verify_password() failed, however that only means that + * this auth query did not succeed. We should try the next auth query if any. */ + continue; + } + + Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' password verification using password_verify() succeeded'); + } + Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname . ' succeeded with ' . count($data) . ' rows'); $queryConfig['_winning_auth_query'] = true; + if (array_key_exists('extract_userid_from', $queryConfig)) { $queryConfig['_extracted_userid'] = $data[0][$queryConfig['extract_userid_from']]; } $winning_auth_query = $queryname; - $this->extractAttributes($attributes, $data, []); + + $forbiddenAttributes = []; + if (array_key_exists('password_verify_hash_column', $queryConfig)) { + $forbiddenAttributes[] = $queryConfig['password_verify_hash_column']; + } + $this->extractAttributes($attributes, $data, $forbiddenAttributes); // The first auth query that succeeds is the winning one, so we can stop here. break; diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 77a2702..0bf89df 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -5,14 +5,20 @@ $projectRoot = dirname(__DIR__); require_once($projectRoot . '/vendor/autoload.php'); + // Load our wrapper class to get around login() being declared protected in SQL.php require_once($projectRoot . '/tests/src/Auth/Source/SQLWrapper.php'); require_once($projectRoot . '/tests/src/Auth/Source/SQL2Wrapper.php'); require_once($projectRoot . '/tests/src/Auth/Source/SQL1CompatWrapper.php'); require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyWrapper.php'); +require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php'); -// Get around SQL1CompatTest extending SQLTest and not being able to find SQLTest +// We use inheritance quite extensively in our test cases, so we need to +// make sure all the classes that are subclassed are loaded before we run any tests. +require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyTest.php'); require_once($projectRoot . '/tests/src/Auth/Source/SQLTest.php'); +require_once($projectRoot . '/tests/src/Auth/Source/SQL2SimpleTest.php'); +require_once($projectRoot . '/tests/src/Auth/Source/SQL2SingleAuthTest.php'); // Symlink module into ssp vendor lib so that templates and urls can resolve correctly $linkPath = $projectRoot . '/vendor/simplesamlphp/simplesamlphp/modules/sqlauth'; diff --git a/tests/src/Auth/Source/PasswordVerify1CompatTest.php b/tests/src/Auth/Source/PasswordVerify1CompatTest.php new file mode 100644 index 0000000..9aad2c3 --- /dev/null +++ b/tests/src/Auth/Source/PasswordVerify1CompatTest.php @@ -0,0 +1,15 @@ + $info + * @param array $config + */ + public function __construct(array $info, array $config) + { + parent::__construct($info, $config); + } + + + /** + * @return array + */ + public function callLogin(string $username, string $password): array + { + return $this->login($username, $password); + } +} diff --git a/tests/src/Auth/Source/PasswordVerifyTest.php b/tests/src/Auth/Source/PasswordVerifyTest.php index 25efd2f..4cb2d8e 100644 --- a/tests/src/Auth/Source/PasswordVerifyTest.php +++ b/tests/src/Auth/Source/PasswordVerifyTest.php @@ -14,6 +14,9 @@ */ class PasswordVerifyTest extends TestCase { + // Subclasses can override this to test other wrapper classes + protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\PasswordVerifyWrapper'; + /** @var array */ private array $info = ['AuthId' => 'testAuthId']; @@ -83,7 +86,7 @@ public function testBasicSingleSuccess(): void { // Correct username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); asort($ret); $this->assertCount(2, $ret); $this->assertEquals($ret, [ @@ -98,7 +101,7 @@ public function testBasicSingleFailedLogin(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -107,7 +110,7 @@ public function testBasicSingleFailedLoginNonExisting(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('henry', 'boo'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('henry', 'boo'); $this->assertCount(0, $ret); } @@ -117,7 +120,7 @@ public function testBasicSingleFailedLoginNonExistingNoPassword(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice2', ''); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice2', ''); $this->assertCount(0, $ret); } @@ -129,7 +132,7 @@ public function testJoinSingleSuccess(): void select u.givenName, u.email, ug.groupname, passwordhash from users u left join usergroups ug on (u.uid=ug.uid) where u.uid=:username "; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); @@ -148,7 +151,7 @@ public function testJoinSingleFailedLogin(): void select u.givenName, u.email, ug.groupname, passwordhash from users u left join usergroups ug on (u.uid=ug.uid) where u.uid=:username"; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -159,7 +162,7 @@ public function testMultiQuerySuccess(): void "select givenName, email, passwordhash from users where uid=:username", "select groupname from usergroups where uid=:username", ]; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); @@ -178,7 +181,7 @@ public function testMultiQueryFailedLogin(): void "select givenName, email, passwordhash from users where uid=:username", "select groupname from usergroups where uid=:username", ]; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -191,7 +194,7 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void "select groupname from usergroups where uid=:username and groupname like '%nomatch%'", "select groupname from usergroups where uid=:username and groupname like 'stud%'", ]; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); @@ -210,7 +213,7 @@ public function testMultiQuerySubsequentAppendSuccess(): void "select groupname from usergroups where uid=:username and groupname like 'stud%'", "select groupname from usergroups where uid=:username and groupname like '%sers'", ]; - $ret = (new PasswordVerifyWrapper($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); asort($ret); asort($ret['groupname']); $this->assertCount(3, $ret); diff --git a/tests/src/Auth/Source/SQL2MultipleAuthTest.php b/tests/src/Auth/Source/SQL2MultipleAuthTest.php index de24e9c..3306bcf 100644 --- a/tests/src/Auth/Source/SQL2MultipleAuthTest.php +++ b/tests/src/Auth/Source/SQL2MultipleAuthTest.php @@ -25,71 +25,79 @@ class SQL2MultipleAuthTest extends TestCase { private array $info = ['AuthId' => 'testAuthId']; - private array $config = [ - "databases" => [ - "studentsdb" => [ - "dsn" => 'sqlite:file:studentsdb?mode=memory&cache=shared', - "username" => "notused", - "password" => "notused", - ], - "physics_staffdb" => [ - "dsn" => 'sqlite:file:physics_staffdb?mode=memory&cache=shared', - "username" => "notused", - "password" => "notused", - ], - "staffdb" => [ - "dsn" => 'sqlite:file:staffdb?mode=memory&cache=shared', - "username" => "notused", - "password" => "notused", - ], - ], - "auth_queries" => [ - "auth_query_students" => [ - "database" => "studentsdb", - "query" => "select studentid, givenName, lastName, email, course, year from students where email=:username and password=:password", - "username_regex" => '/^[a-zA-Z0-9._%+-]+@student\.example\.edu$/', - "extract_userid_from" => 'studentid', - ], + protected array $config = []; // Filled out in setUp() - // We specify the physics_staffdb auth query before the staffdb one, so that if a user exists in both - // staff databases, they will be authenticated against the physics_staffdb one. - "auth_query_physics_staff" => [ - "database" => "physics_staffdb", - "query" => "select psid as uid, CASE WHEN typically_wears_matching_socks=true THEN 'true' ELSE 'false' END as \"typically_wears_matching_socks\" from staff where email=:username and password=:password", - "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', - "extract_userid_from" => 'uid', - ], + protected string $extraSqlSelectColumns = ''; + protected string $extraSqlAndClauses = ' and password=:password'; - "auth_query_staff" => [ - "database" => "staffdb", - "query" => "select uid, givenName, lastName, email, department from staff where email=:username and password=:password", - "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', - "extract_userid_from" => 'uid', - ], - ], - "attr_queries" => [ - [ - 'database' => 'staffdb', - 'query' => "select givenName, lastName, email, department from staff where uid=:userid", - 'only_for_auth' => ['auth_query_staff', 'auth_query_physics_staff'], - ], - [ - 'database' => 'staffdb', - 'query' => "select role from staff_roles where uid=:userid", - 'only_for_auth' => ['auth_query_staff', 'auth_query_physics_staff'], + public function setUp(): void + { + $this->config = [ + "databases" => [ + "studentsdb" => [ + "dsn" => 'sqlite:file:studentsdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + "physics_staffdb" => [ + "dsn" => 'sqlite:file:physics_staffdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + "staffdb" => [ + "dsn" => 'sqlite:file:staffdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], ], - [ - 'database' => 'physics_staffdb', - 'query' => "select qualification from staff_qualifications where psid=:userid order by qualification desc", - 'only_for_auth' => ['auth_query_physics_staff'], + "auth_queries" => [ + "auth_query_students" => [ + "database" => "studentsdb", + "query" => "select studentid, givenName, lastName, email, course, year " . $this->extraSqlSelectColumns . " from students where email=:username" . $this->extraSqlAndClauses, + "username_regex" => '/^[a-zA-Z0-9._%+-]+@student\.example\.edu$/', + "extract_userid_from" => 'studentid', + ], + + // We specify the physics_staffdb auth query before the staffdb one, so that if a user exists in both + // staff databases, they will be authenticated against the physics_staffdb one. + "auth_query_physics_staff" => [ + "database" => "physics_staffdb", + "query" => "select psid as uid, CASE WHEN typically_wears_matching_socks=true THEN 'true' ELSE 'false' END as \"typically_wears_matching_socks\" " . $this->extraSqlSelectColumns . " from staff where email=:username" . $this->extraSqlAndClauses, + "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', + "extract_userid_from" => 'uid', + ], + + "auth_query_staff" => [ + "database" => "staffdb", + "query" => "select uid, givenName, lastName, email, department " . $this->extraSqlSelectColumns . " from staff where email=:username" . $this->extraSqlAndClauses, + "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', + "extract_userid_from" => 'uid', + ], ], - [ - 'database' => 'studentsdb', - 'query' => "select unit_code from units_enrolled where studentid=:userid", - 'only_for_auth' => ['auth_query_students'], + "attr_queries" => [ + [ + 'database' => 'staffdb', + 'query' => "select givenName, lastName, email, department from staff where uid=:userid", + 'only_for_auth' => ['auth_query_staff', 'auth_query_physics_staff'], + ], + [ + 'database' => 'staffdb', + 'query' => "select role from staff_roles where uid=:userid", + 'only_for_auth' => ['auth_query_staff', 'auth_query_physics_staff'], + ], + [ + 'database' => 'physics_staffdb', + 'query' => "select qualification from staff_qualifications where psid=:userid order by qualification desc", + 'only_for_auth' => ['auth_query_physics_staff'], + ], + [ + 'database' => 'studentsdb', + 'query' => "select unit_code from units_enrolled where studentid=:userid", + 'only_for_auth' => ['auth_query_students'], + ], ], - ], - ]; + ]; + } public static function setUpBeforeClass(): void { @@ -257,7 +265,6 @@ public function testPhysicsStaffLoginSuccess(): void // Correct username/password for physics staff $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('mallory.mallory@example.edu', 'password'); asort($ret); - var_dump($ret); $this->assertCount(8, $ret); $this->assertCount(1, $ret['role']); $this->assertEquals($ret, [ diff --git a/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php new file mode 100644 index 0000000..f9eaebc --- /dev/null +++ b/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php @@ -0,0 +1,52 @@ +config['auth_queries'] as &$query) { + $query['password_verify_hash_column'] = 'password'; + } + } + + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + $studentsPdo = new PDO('sqlite:file:studentsdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $studentsPdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + $studentsPdo->prepare("UPDATE students SET password=?") + ->execute([password_hash('password', PASSWORD_ARGON2ID)]); + + $staffPdo = new PDO('sqlite:file:staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $staffPdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + $staffPdo->prepare("UPDATE staff SET password=?") + ->execute([password_hash('password', PASSWORD_ARGON2ID)]); + + $physicsStaffPdo = new PDO('sqlite:file:physics_staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $physicsStaffPdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + $physicsStaffPdo->prepare("UPDATE staff SET password=?") + ->execute([password_hash('password', PASSWORD_ARGON2ID)]); + } +} diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php new file mode 100644 index 0000000..7a94d28 --- /dev/null +++ b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php @@ -0,0 +1,47 @@ +config['auth_queries'] as &$query) { + $query['password_verify_hash_column'] = 'password'; + } + } + + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + $pdo = new PDO('sqlite:file:defaultdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $pdo->prepare("UPDATE users SET password=?") + ->execute([password_hash('password', PASSWORD_ARGON2ID)]); + } +} diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php new file mode 100644 index 0000000..631c152 --- /dev/null +++ b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php @@ -0,0 +1,49 @@ + true]); + $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + + $pdo->prepare("UPDATE users SET password=?") + ->execute([password_hash('password', PASSWORD_ARGON2ID)]); + } +} diff --git a/tests/src/Auth/Source/SQL2SimpleTest.php b/tests/src/Auth/Source/SQL2SimpleTest.php index 596e6e7..1aacc61 100644 --- a/tests/src/Auth/Source/SQL2SimpleTest.php +++ b/tests/src/Auth/Source/SQL2SimpleTest.php @@ -17,21 +17,36 @@ class SQL2SimpleTest extends TestCase { private array $info = ['AuthId' => 'testAuthId']; - private array $config = [ - "databases" => [ - "defaultdb" => [ - "dsn" => 'sqlite:file:defaultdb?mode=memory&cache=shared', - "username" => "notused", - "password" => "notused", + protected array $config = []; // Filled out in setUp() + + protected string $extraSqlSelectColumns = ''; + protected string $extraSqlAndClauses = ' and password=:password'; + + public function setUp(): void + { + $this->config = [ + "databases" => [ + "defaultdb" => [ + "dsn" => 'sqlite:file:defaultdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], ], - ], - "auth_queries" => [ - "auth_query" => [ - "database" => "defaultdb", - "query" => null, // Filled out by each test case + "auth_queries" => [ + "auth_query" => [ + "database" => "defaultdb", + "query" => null, // Filled out by each test case + ], ], - ], - ]; + ]; + } + + protected static function transformPassword(string $password): string + { + // In this simple test, passwords are stored in plaintext, so no transformation is needed. + // The SQL2PasswordVerifySimpleTest subclass override this to hash the password appropriately. + return $password; + } public static function setUpBeforeClass(): void { @@ -88,7 +103,7 @@ public static function setUpBeforeClass(): void public function testBasicSingleSuccess(): void { // Correct username/password - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); @@ -101,7 +116,7 @@ public function testBasicSingleSuccess(): void public function testBasicSingleUsernameRegexSuccess(): void { // Correct username/password - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['auth_queries']['auth_query']['username_regex'] = '/^[a-z]+$/'; // Username must be a single lower case word $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); @@ -116,7 +131,7 @@ public function testBasicSingleUsernameRegexFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Correct username/password, but doesn't match the username regex - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); @@ -127,7 +142,7 @@ public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Correct username/password, but doesn't match the username regex - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('henry', 'password'); asort($ret); @@ -138,7 +153,7 @@ public function testBasicSingleFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email from users where uid=:username and password=:password"; + $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -147,9 +162,9 @@ public function testJoinSingleSuccess(): void { // Correct username/password $this->config['auth_queries']['auth_query']['query'] = " - select u.givenName, u.email, ug.groupname + select u.givenName, u.email, ug.groupname" . $this->extraSqlSelectColumns . " from users u left join usergroups ug on (u.uid=ug.uid) - where u.uid=:username and u.password=:password"; + where u.uid=:username" . $this->extraSqlAndClauses; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); asort($ret['groupname']); @@ -166,9 +181,9 @@ public function testJoinSingleFailedLogin(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['auth_queries']['auth_query']['query'] = " - select u.givenName, u.email, ug.groupname + select u.givenName, u.email, ug.groupname" . $this->extraSqlSelectColumns . " from users u left join usergroups ug on (u.uid=ug.uid) - where u.uid=:username and u.password=:password"; + where u.uid=:username" . $this->extraSqlAndClauses; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -177,7 +192,7 @@ public function testMultiQuerySuccess(): void { // Correct username/password $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email from users where uid=:username and password=:password"; + "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', @@ -201,7 +216,7 @@ public function testMultiQueryFailedLogin(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email from users where uid=:username and password=:password"; + "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', @@ -216,7 +231,7 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void { // Correct username/password. Second query returns no rows, third query returns just one row $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email from users where uid=:username and password=:password"; + "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', @@ -243,7 +258,7 @@ public function testMultiQuerySubsequentAppendSuccess(): void { // Correct username/password. Second query returns a row, third query appends one row $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email from users where uid=:username and password=:password"; + "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', diff --git a/tests/src/Auth/Source/SQL2SingleAuthTest.php b/tests/src/Auth/Source/SQL2SingleAuthTest.php index ae93822..263d3ff 100644 --- a/tests/src/Auth/Source/SQL2SingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2SingleAuthTest.php @@ -20,10 +20,13 @@ class SQL2SingleAuthTest extends TestCase { private array $info = ['AuthId' => 'testAuthId']; + protected string $extraSqlSelectColumns = ''; + protected string $extraSqlAndClauses = ' and password=:password'; + /* Different tests require different combinations of databases, auth queries and attr queries. * This function returns a config with the requested number of each. */ - private function getConfig(int $numDatabases, int $numAuthQueries, array $authQueryAttributes, int $numAttrQueries): array + protected function getConfig(int $numDatabases, int $numAuthQueries, array $authQueryAttributes, int $numAttrQueries): array { $config = [ "databases" => [ @@ -46,13 +49,13 @@ private function getConfig(int $numDatabases, int $numAuthQueries, array $authQu "auth_queries" => [ "auth_query_id" => [ "database" => "authdb", - "query" => "select uid, givenName, email from users where uid=:username and password=:password", + "query" => "select uid, givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses, "username_regex" => '/^\\d+$/', "extract_userid_from" => 'uid', ], "auth_query_email" => [ "database" => "authdb", - "query" => "select uid, givenName, email from users where email=:username and password=:password", + "query" => "select uid, givenName, email " . $this->extraSqlSelectColumns . " from users where email=:username" . $this->extraSqlAndClauses, "username_regex" => '/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/', "extract_userid_from" => 'uid', ], From 9e62356062a8f150c4c549ff6dcd57ec7fa5889f Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 20 Oct 2025 14:21:20 +1100 Subject: [PATCH 05/28] Fix phpcs errors and warnings --- src/Auth/Source/PasswordVerify.php | 2 +- src/Auth/Source/PasswordVerify1Compat.php | 6 +- src/Auth/Source/SQL.php | 5 ++ src/Auth/Source/SQL1Compat.php | 6 +- src/Auth/Source/SQL2.php | 83 ++++++++++++------- tests/src/Auth/Source/PasswordVerifyTest.php | 7 ++ .../src/Auth/Source/SQL2MultipleAuthTest.php | 48 +++++++++-- .../SQL2PasswordVerifyMultipleAuthTest.php | 26 ++++-- .../Source/SQL2PasswordVerifySimpleTest.php | 8 +- .../SQL2PasswordVerifySingleAuthTest.php | 14 +++- tests/src/Auth/Source/SQL2SimpleTest.php | 76 ++++++++++++----- tests/src/Auth/Source/SQL2SingleAuthTest.php | 47 +++++++++-- tests/src/Auth/Source/SQLTest.php | 12 +++ 13 files changed, 256 insertions(+), 84 deletions(-) diff --git a/src/Auth/Source/PasswordVerify.php b/src/Auth/Source/PasswordVerify.php index 1989612..f463637 100644 --- a/src/Auth/Source/PasswordVerify.php +++ b/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. * diff --git a/src/Auth/Source/PasswordVerify1Compat.php b/src/Auth/Source/PasswordVerify1Compat.php index d017c21..50c3c2b 100644 --- a/src/Auth/Source/PasswordVerify1Compat.php +++ b/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,7 +29,7 @@ public function __construct(array $info, array $config) 'dsn' => $config['dsn'], 'username' => $config['username'], 'password' => $config['password'], - ] + ], ], 'auth_queries' => [ @@ -37,7 +37,7 @@ public function __construct(array $info, array $config) 'database' => 'default', 'query' => is_array($config['query']) ? $config['query'][0] : $config['query'], 'password_verify_hash_column' => 'passwordhash', - ] + ], ], ]; diff --git a/src/Auth/Source/SQL.php b/src/Auth/Source/SQL.php index 09eb093..fd56bb2 100644 --- a/src/Auth/Source/SQL.php +++ b/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. * diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php index 959f7d3..36825f3 100644 --- a/src/Auth/Source/SQL1Compat.php +++ b/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'], - ] + ], ], ]; diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 58f7c1b..cc47f5c 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -26,7 +26,7 @@ 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 = []; @@ -34,7 +34,7 @@ class SQL2 extends UserPassBase /** * 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) { diff --git a/tests/src/Auth/Source/PasswordVerifyTest.php b/tests/src/Auth/Source/PasswordVerifyTest.php index 4cb2d8e..98feac0 100644 --- a/tests/src/Auth/Source/PasswordVerifyTest.php +++ b/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 diff --git a/tests/src/Auth/Source/SQL2MultipleAuthTest.php b/tests/src/Auth/Source/SQL2MultipleAuthTest.php index 3306bcf..9978838 100644 --- a/tests/src/Auth/Source/SQL2MultipleAuthTest.php +++ b/tests/src/Auth/Source/SQL2MultipleAuthTest.php @@ -8,12 +8,12 @@ use PHPUnit\Framework\TestCase; /** - * The scenario for this test case is two separate types of users (staff and students), + * The scenario for this test case is two separate types of users (staff and students), * each of which has its own set of attributes in separate databases. In addition, the - * physics department have their own legacy database of staff, which has some extra + * physics department have their own legacy database of staff, which has some extra * attributes which are not in the main staff database, and their passwords are still * in that legacy database. But physics staff records exist in the new staff database too. - * + * * The technical scenario is that there is one student database, two staff databases, * which a subset of the staff are in only one database, whereas some staff are in both. * Attributes for students come from just the student database, whereas the staff attributes @@ -25,11 +25,14 @@ class SQL2MultipleAuthTest extends TestCase { private array $info = ['AuthId' => 'testAuthId']; + protected array $config = []; // Filled out in setUp() protected string $extraSqlSelectColumns = ''; + protected string $extraSqlAndClauses = ' and password=:password'; + public function setUp(): void { $this->config = [ @@ -53,7 +56,9 @@ public function setUp(): void "auth_queries" => [ "auth_query_students" => [ "database" => "studentsdb", - "query" => "select studentid, givenName, lastName, email, course, year " . $this->extraSqlSelectColumns . " from students where email=:username" . $this->extraSqlAndClauses, + "query" => + "select studentid, givenName, lastName, email, course, year " . $this->extraSqlSelectColumns . + " from students where email=:username" . $this->extraSqlAndClauses, "username_regex" => '/^[a-zA-Z0-9._%+-]+@student\.example\.edu$/', "extract_userid_from" => 'studentid', ], @@ -62,14 +67,23 @@ public function setUp(): void // staff databases, they will be authenticated against the physics_staffdb one. "auth_query_physics_staff" => [ "database" => "physics_staffdb", - "query" => "select psid as uid, CASE WHEN typically_wears_matching_socks=true THEN 'true' ELSE 'false' END as \"typically_wears_matching_socks\" " . $this->extraSqlSelectColumns . " from staff where email=:username" . $this->extraSqlAndClauses, + "query" => + "select psid as uid, " . + "CASE WHEN typically_wears_matching_socks=true " . + "THEN 'true' " . + "ELSE 'false' " . + "END as \"typically_wears_matching_socks\" " . + $this->extraSqlSelectColumns . + " from staff where email=:username" . $this->extraSqlAndClauses, "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', "extract_userid_from" => 'uid', ], "auth_query_staff" => [ "database" => "staffdb", - "query" => "select uid, givenName, lastName, email, department " . $this->extraSqlSelectColumns . " from staff where email=:username" . $this->extraSqlAndClauses, + "query" => + "select uid, givenName, lastName, email, department " . $this->extraSqlSelectColumns . + " from staff where email=:username" . $this->extraSqlAndClauses, "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', "extract_userid_from" => 'uid', ], @@ -87,7 +101,9 @@ public function setUp(): void ], [ 'database' => 'physics_staffdb', - 'query' => "select qualification from staff_qualifications where psid=:userid order by qualification desc", + 'query' => + "select qualification from staff_qualifications " . + "where psid=:userid order by qualification desc", 'only_for_auth' => ['auth_query_physics_staff'], ], [ @@ -99,10 +115,16 @@ public function setUp(): void ]; } + public static function setUpBeforeClass(): void { // Students database - $studentsPdo = new PDO('sqlite:file:studentsdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $studentsPdo = new PDO( + 'sqlite:file:studentsdb?mode=memory&cache=shared', + null, + null, + [PDO::ATTR_PERSISTENT => true], + ); $studentsPdo->exec("DROP TABLE IF EXISTS students"); // Create tables $studentsPdo->exec(" @@ -189,7 +211,12 @@ public static function setUpBeforeClass(): void } // Physics staff database - $physicsStaffPdo = new PDO('sqlite:file:physics_staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $physicsStaffPdo = new PDO( + 'sqlite:file:physics_staffdb?mode=memory&cache=shared', + null, + null, + [PDO::ATTR_PERSISTENT => true], + ); $physicsStaffPdo->exec("DROP TABLE IF EXISTS staff"); $physicsStaffPdo->exec(" CREATE TABLE staff ( @@ -225,6 +252,7 @@ public static function setUpBeforeClass(): void } } + public function testStudentLoginSuccess(): void { // Correct username/password @@ -243,6 +271,7 @@ public function testStudentLoginSuccess(): void ]); } + public function testNonPhysicsStaffLoginSuccess(): void { // Correct username/password for non-physics staff @@ -260,6 +289,7 @@ public function testNonPhysicsStaffLoginSuccess(): void ]); } + public function testPhysicsStaffLoginSuccess(): void { // Correct username/password for physics staff diff --git a/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php index f9eaebc..8e2fd52 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php @@ -5,7 +5,6 @@ namespace SimpleSAML\Test\Module\sqlauth\Auth\Source; use PDO; -use PHPUnit\Framework\TestCase; /** * The scenario is SQL2MultipleAuthTest but with passwords hashed using password_hash() @@ -21,6 +20,7 @@ class SQL2PasswordVerifyMultipleAuthTest extends SQL2MultipleAuthTest // as password_verify() does not work that way. protected string $extraSqlAndClauses = ''; + public function setUp(): void { parent::setUp(); @@ -30,21 +30,37 @@ public function setUp(): void } } + public static function setUpBeforeClass(): void { parent::setUpBeforeClass(); - $studentsPdo = new PDO('sqlite:file:studentsdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $studentsPdo = new PDO( + 'sqlite:file:studentsdb?mode=memory&cache=shared', + null, + null, + [PDO::ATTR_PERSISTENT => true], + ); $studentsPdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $studentsPdo->prepare("UPDATE students SET password=?") ->execute([password_hash('password', PASSWORD_ARGON2ID)]); - $staffPdo = new PDO('sqlite:file:staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $staffPdo = new PDO( + 'sqlite:file:staffdb?mode=memory&cache=shared', + null, + null, + [PDO::ATTR_PERSISTENT => true], + ); $staffPdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $staffPdo->prepare("UPDATE staff SET password=?") ->execute([password_hash('password', PASSWORD_ARGON2ID)]); - - $physicsStaffPdo = new PDO('sqlite:file:physics_staffdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + + $physicsStaffPdo = new PDO( + 'sqlite:file:physics_staffdb?mode=memory&cache=shared', + null, + null, + [PDO::ATTR_PERSISTENT => true], + ); $physicsStaffPdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $physicsStaffPdo->prepare("UPDATE staff SET password=?") ->execute([password_hash('password', PASSWORD_ARGON2ID)]); diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php index 7a94d28..6517056 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php @@ -10,10 +10,10 @@ * The scenario for this test case is a single database of customers who have their * metadata in a single database, and passwords are hashed using password_hash(), * and verification done using password_verify(). - * - * The tests are essentially the same tests as SQLTest, but using the SQLv2 + * + * The tests are essentially the same tests as SQLTest, but using the SQLv2 * configuration and code, not the SQL1 code or SQL1Compat interface. - * + * * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ class SQL2PasswordVerifySimpleTest extends SQL2SimpleTest @@ -25,6 +25,7 @@ class SQL2PasswordVerifySimpleTest extends SQL2SimpleTest // as password_verify() does not work that way. protected string $extraSqlAndClauses = ''; + public function setUp(): void { parent::setUp(); @@ -34,6 +35,7 @@ public function setUp(): void } } + public static function setUpBeforeClass(): void { parent::setUpBeforeClass(); diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php index 631c152..36a799e 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php @@ -11,9 +11,9 @@ * metadata spread across multiple databases, and passwords are hashed using password_hash(), * and verification done using password_verify(). Customers login with their email address, * but the common identifier across all databases is the userid (uid). - * + * * The attributes then come from multiple databases. - * + * * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ class SQL2PasswordVerifySingleAuthTest extends SQL2SingleAuthTest @@ -25,8 +25,13 @@ class SQL2PasswordVerifySingleAuthTest extends SQL2SingleAuthTest // as password_verify() does not work that way. protected string $extraSqlAndClauses = ''; - protected function getConfig(int $numDatabases, int $numAuthQueries, array $authQueryAttributes, int $numAttrQueries): array - { + + protected function getConfig( + int $numDatabases, + int $numAuthQueries, + array $authQueryAttributes, + int $numAttrQueries, + ): array { $config = parent::getConfig($numDatabases, $numAuthQueries, $authQueryAttributes, $numAttrQueries); foreach ($config['auth_queries'] as &$query) { @@ -36,6 +41,7 @@ protected function getConfig(int $numDatabases, int $numAuthQueries, array $auth return $config; } + public static function setUpBeforeClass(): void { parent::setUpBeforeClass(); diff --git a/tests/src/Auth/Source/SQL2SimpleTest.php b/tests/src/Auth/Source/SQL2SimpleTest.php index 1aacc61..a000cbd 100644 --- a/tests/src/Auth/Source/SQL2SimpleTest.php +++ b/tests/src/Auth/Source/SQL2SimpleTest.php @@ -11,17 +11,20 @@ * The scenario for this test case is a single database of customers who have their * metadata in a single database. This is essentially the same tests as SQLTest, but * using the SQLv2 configuration and code, not the SQL1 code or SQL1Compat interface. - * + * * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ class SQL2SimpleTest extends TestCase { private array $info = ['AuthId' => 'testAuthId']; + protected array $config = []; // Filled out in setUp() protected string $extraSqlSelectColumns = ''; + protected string $extraSqlAndClauses = ' and password=:password'; + public function setUp(): void { $this->config = [ @@ -41,6 +44,7 @@ public function setUp(): void ]; } + protected static function transformPassword(string $password): string { // In this simple test, passwords are stored in plaintext, so no transformation is needed. @@ -48,6 +52,7 @@ protected static function transformPassword(string $password): string return $password; } + public static function setUpBeforeClass(): void { $pdo = new PDO('sqlite:file:defaultdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); @@ -100,10 +105,13 @@ public static function setUpBeforeClass(): void } } + public function testBasicSingleSuccess(): void { // Correct username/password - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); @@ -113,11 +121,17 @@ public function testBasicSingleSuccess(): void ]); } + public function testBasicSingleUsernameRegexSuccess(): void { // Correct username/password - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; - $this->config['auth_queries']['auth_query']['username_regex'] = '/^[a-z]+$/'; // Username must be a single lower case word + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . + $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + + // Username must be a single lower case word + $this->config['auth_queries']['auth_query']['username_regex'] = '/^[a-z]+$/'; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); @@ -127,37 +141,52 @@ public function testBasicSingleUsernameRegexSuccess(): void ]); } + public function testBasicSingleUsernameRegexFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Correct username/password, but doesn't match the username regex - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; - $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; + + // Username must be a non-negative integer + $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); $this->assertCount(0, $ret); } + public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Correct username/password, but doesn't match the username regex - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; - $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; + + // Username must be a non-negative integer + $this->config['auth_queries']['auth_query']['username_regex'] = '/^\d+$/'; + $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('henry', 'password'); asort($ret); $this->assertCount(0, $ret); } + public function testBasicSingleFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password - $this->config['auth_queries']['auth_query']['query'] = "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } + public function testJoinSingleSuccess(): void { // Correct username/password @@ -176,6 +205,7 @@ public function testJoinSingleSuccess(): void ]); } + public function testJoinSingleFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -188,16 +218,18 @@ public function testJoinSingleFailedLogin(): void $this->assertCount(0, $ret); } + public function testMultiQuerySuccess(): void { // Correct username/password - $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', 'query' => "select groupname from usergroups where uid=:username", - ] + ], ]; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); @@ -211,27 +243,31 @@ public function testMultiQuerySuccess(): void ]); } + public function testMultiQueryFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password - $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', 'query' => "select groupname from usergroups where uid=:username", - ] + ], ]; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } + public function testMultiQuerySubsequentNoRowsSuccess(): void { // Correct username/password. Second query returns no rows, third query returns just one row - $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', @@ -254,11 +290,13 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void ]); } + public function testMultiQuerySubsequentAppendSuccess(): void { // Correct username/password. Second query returns a row, third query appends one row - $this->config['auth_queries']['auth_query']['query'] = - "select givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses; + $this->config['auth_queries']['auth_query']['query'] = + "select givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses; $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', diff --git a/tests/src/Auth/Source/SQL2SingleAuthTest.php b/tests/src/Auth/Source/SQL2SingleAuthTest.php index 263d3ff..d92253a 100644 --- a/tests/src/Auth/Source/SQL2SingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2SingleAuthTest.php @@ -11,9 +11,9 @@ * The scenario for this test case is a single database of customers who have their * metadata spread across multiple databases. Customers login with their email address, * but the common identifier across all databases is the userid (uid). - * + * * The attributes then come from multiple databases. - * + * * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ class SQL2SingleAuthTest extends TestCase @@ -21,13 +21,19 @@ class SQL2SingleAuthTest extends TestCase private array $info = ['AuthId' => 'testAuthId']; protected string $extraSqlSelectColumns = ''; + protected string $extraSqlAndClauses = ' and password=:password'; + /* Different tests require different combinations of databases, auth queries and attr queries. * This function returns a config with the requested number of each. */ - protected function getConfig(int $numDatabases, int $numAuthQueries, array $authQueryAttributes, int $numAttrQueries): array - { + protected function getConfig( + int $numDatabases, + int $numAuthQueries, + array $authQueryAttributes, + int $numAttrQueries, + ): array { $config = [ "databases" => [ "authdb" => [ @@ -49,13 +55,17 @@ protected function getConfig(int $numDatabases, int $numAuthQueries, array $auth "auth_queries" => [ "auth_query_id" => [ "database" => "authdb", - "query" => "select uid, givenName, email " . $this->extraSqlSelectColumns . " from users where uid=:username" . $this->extraSqlAndClauses, + "query" => + "select uid, givenName, email " . $this->extraSqlSelectColumns . + " from users where uid=:username" . $this->extraSqlAndClauses, "username_regex" => '/^\\d+$/', "extract_userid_from" => 'uid', ], "auth_query_email" => [ "database" => "authdb", - "query" => "select uid, givenName, email " . $this->extraSqlSelectColumns . " from users where email=:username" . $this->extraSqlAndClauses, + "query" => + "select uid, givenName, email " . $this->extraSqlSelectColumns . + " from users where email=:username" . $this->extraSqlAndClauses, "username_regex" => '/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/', "extract_userid_from" => 'uid', ], @@ -93,7 +103,9 @@ protected function getConfig(int $numDatabases, int $numAuthQueries, array $auth // Then check all of the requested attributes are in each auth query. foreach ($authQueryAttributes as $attribute) { if (!array_key_exists($attribute, $authQuery)) { - throw new \InvalidArgumentException("Auth query attribute \"$attribute\" not found in auth query \"$authQueryName\""); + throw new \InvalidArgumentException( + "Auth query attribute \"$attribute\" not found in auth query \"$authQueryName\"", + ); } } } @@ -101,6 +113,7 @@ protected function getConfig(int $numDatabases, int $numAuthQueries, array $auth return $ret; } + public static function setUpBeforeClass(): void { // Auth database @@ -150,7 +163,12 @@ public static function setUpBeforeClass(): void } // Students database - $studentsPdo = new PDO('sqlite:file:studentsdb?mode=memory&cache=shared', null, null, [PDO::ATTR_PERSISTENT => true]); + $studentsPdo = new PDO( + 'sqlite:file:studentsdb?mode=memory&cache=shared', + null, + null, + [PDO::ATTR_PERSISTENT => true], + ); $studentsPdo->exec("DROP TABLE IF EXISTS students"); $studentsPdo->exec(" CREATE TABLE students ( @@ -189,6 +207,7 @@ public static function setUpBeforeClass(): void } } + public function testSingleAuthQueryOnlySuccess(): void { $config = $this->getConfig(1, 1, ['database', 'query'], 0); @@ -204,6 +223,7 @@ public function testSingleAuthQueryOnlySuccess(): void ]); } + public function testSingleAuthQueryOnlyPasswordFailure(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -214,6 +234,7 @@ public function testSingleAuthQueryOnlyPasswordFailure(): void (new SQL2Wrapper($this->info, $config))->callLogin('2', 'wrongpassword'); } + public function testSingleAuthQueryOnlyUsernameFailure(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -224,6 +245,7 @@ public function testSingleAuthQueryOnlyUsernameFailure(): void (new SQL2Wrapper($this->info, $config))->callLogin('201', 'password'); } + public function testSingleAuthQueryOnlySuccessWithRegex(): void { $config = $this->getConfig(1, 1, ['database', 'query', 'username_regex'], 0); @@ -239,6 +261,7 @@ public function testSingleAuthQueryOnlySuccessWithRegex(): void ]); } + public function testSingleAuthQueryOnlyFailureDueToRegex(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -248,6 +271,7 @@ public function testSingleAuthQueryOnlyFailureDueToRegex(): void (new SQL2Wrapper($this->info, $config))->callLogin('bad-username', 'password'); } + public function testSingleAuthQuerySingleAttrQuerySuccess(): void { $config = $this->getConfig(2, 1, ['database', 'query', 'extract_userid_from'], 1); @@ -266,6 +290,7 @@ public function testSingleAuthQuerySingleAttrQuerySuccess(): void ]); } + public function testSingleAuthQuerySingleAttrQueryPasswordFailure(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -292,6 +317,7 @@ public function testMultipleAuthQueryNoAttrQueryUsernameIsIdSuccess(): void ]); } + public function testMultipleAuthQueryNoAttrQueryUsernameIsEmailSuccess(): void { $config = $this->getConfig(2, 2, ['database', 'query', 'username_regex'], 0); @@ -307,6 +333,7 @@ public function testMultipleAuthQueryNoAttrQueryUsernameIsEmailSuccess(): void ]); } + public function testMultipleAuthQueryNoAttrQueryUsernameIsEmailFailure(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -317,6 +344,7 @@ public function testMultipleAuthQueryNoAttrQueryUsernameIsEmailFailure(): void (new SQL2Wrapper($this->info, $config))->callLogin('nonexistent@example.com', 'password'); } + public function testMultipleAuthQuerySingleAttrQueryUsernameIsEmailSuccess(): void { $config = $this->getConfig(2, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 1); @@ -335,6 +363,7 @@ public function testMultipleAuthQuerySingleAttrQueryUsernameIsEmailSuccess(): vo ]); } + public function testMultipleAuthQueryStudentWithMultipleEnrolmentsSuccess(): void { $config = $this->getConfig(3, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 3); @@ -354,6 +383,7 @@ public function testMultipleAuthQueryStudentWithMultipleEnrolmentsSuccess(): voi ]); } + public function testMultipleAuthQueryStudentWithNoEnrolmentsSuccess(): void { $config = $this->getConfig(3, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 3); @@ -373,6 +403,7 @@ public function testMultipleAuthQueryStudentWithNoEnrolmentsSuccess(): void ]); } + public function testMultipleAuthQueryStudentWithSingleEnrolmentSuccess(): void { $config = $this->getConfig(3, 2, ['database', 'query', 'username_regex', 'extract_userid_from'], 3); diff --git a/tests/src/Auth/Source/SQLTest.php b/tests/src/Auth/Source/SQLTest.php index 655b3dc..b6b87f6 100644 --- a/tests/src/Auth/Source/SQLTest.php +++ b/tests/src/Auth/Source/SQLTest.php @@ -28,6 +28,7 @@ class SQLTest 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]); @@ -79,6 +80,7 @@ public static function setUpBeforeClass(): void } } + public function testBasicSingleSuccess(): void { // Correct username/password @@ -92,6 +94,7 @@ public function testBasicSingleSuccess(): void ]); } + public function testBasicSingleUsernameRegexSuccess(): void { // Correct username/password @@ -106,6 +109,7 @@ public function testBasicSingleUsernameRegexSuccess(): void ]); } + public function testBasicSingleUsernameRegexFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -117,6 +121,7 @@ public function testBasicSingleUsernameRegexFailedLogin(): void $this->assertCount(0, $ret); } + public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -128,6 +133,7 @@ public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void $this->assertCount(0, $ret); } + public function testBasicSingleFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -137,6 +143,7 @@ public function testBasicSingleFailedLogin(): void $this->assertCount(0, $ret); } + public function testJoinSingleSuccess(): void { // Correct username/password @@ -155,6 +162,7 @@ public function testJoinSingleSuccess(): void ]); } + public function testJoinSingleFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -167,6 +175,7 @@ public function testJoinSingleFailedLogin(): void $this->assertCount(0, $ret); } + public function testMultiQuerySuccess(): void { // Correct username/password @@ -185,6 +194,7 @@ public function testMultiQuerySuccess(): void ]); } + public function testMultiQueryFailedLogin(): void { $this->expectException(\SimpleSAML\Error\Error::class); @@ -197,6 +207,7 @@ public function testMultiQueryFailedLogin(): void $this->assertCount(0, $ret); } + public function testMultiQuerySubsequentNoRowsSuccess(): void { // Correct username/password. Second query returns no rows, third query returns just one row @@ -216,6 +227,7 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void ]); } + public function testMultiQuerySubsequentAppendSuccess(): void { // Correct username/password. Second query returns a row, third query appends one row From de4021bec167f86395639400ac935366309dac2d Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 20 Oct 2025 14:31:06 +1100 Subject: [PATCH 06/28] phpstan fixes --- src/Auth/Source/SQL2.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index cc47f5c..a8a2d5c 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -482,7 +482,7 @@ protected function login( } } - if (($passwordHash === null) || (!password_verify($password, $passwordHash))) { + if ($passwordHash == null || (!password_verify($password, $passwordHash))) { Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . ' password verification failed'); /* Authentication with verify_password() failed, however that only means that From c0a0e771a8f9926ed82c4575f547466692f5ce5f Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 20 Oct 2025 16:27:33 +1100 Subject: [PATCH 07/28] More phpstan fixes --- phpstan-baseline-dev.neon | 22 ++++++------ src/Auth/Source/SQL2.php | 13 +++++-- tests/src/Auth/Source/PasswordVerifyTest.php | 2 +- .../src/Auth/Source/SQL2MultipleAuthTest.php | 6 ++-- .../Source/SQL2PasswordVerifySimpleTest.php | 2 +- .../SQL2PasswordVerifySingleAuthTest.php | 16 +++++++-- tests/src/Auth/Source/SQL2SimpleTest.php | 34 +++++++++++++------ tests/src/Auth/Source/SQL2SingleAuthTest.php | 12 +++++-- tests/src/Auth/Source/SQLTest.php | 2 +- 9 files changed, 73 insertions(+), 36 deletions(-) diff --git a/phpstan-baseline-dev.neon b/phpstan-baseline-dev.neon index e1bab15..1b53fd6 100644 --- a/phpstan-baseline-dev.neon +++ b/phpstan-baseline-dev.neon @@ -1,21 +1,21 @@ parameters: ignoreErrors: - - message: "#^Parameter \\#1 \\$array of function asort expects array, string given\\.$#" - count: 4 + message: "#Call to an undefined method object::callLogin().#" + count: 10 path: tests/src/Auth/Source/PasswordVerifyTest.php - - message: "#^Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\PasswordVerifyTest\\:\\:\\$config \\(array\\\\) does not accept array\\\\|string\\|null\\>\\.$#" - count: 4 - path: tests/src/Auth/Source/PasswordVerifyTest.php + message: "#Call to an undefined method object::callLogin().#" + count: 11 + path: tests/src/Auth/Source/SQLTest.php - - message: "#^Parameter \\#1 \\$array of function asort expects array, mixed given\\.$#" - count: 4 - path: tests/src/Auth/Source/SQLTest.php + message: "#Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQL2SimpleTest\\:\\:\\$config type has no value type specified in iterable type array\\.#" + count: 1 + path: tests/src/Auth/Source/SQL2SimpleTest.php - - message: "#^Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQLTest\\:\\:\\$config \\(array\\\\) does not accept array\\\\|string\\|null\\>\\.$#" - count: 4 - path: tests/src/Auth/Source/SQLTest.php + message: "#Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQL2MultipleAuthTest\\:\\:\\$config type has no value type specified in iterable type array\\.#" + count: 1 + path: tests/src/Auth/Source/SQL2MultipleAuthTest.php diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index a8a2d5c..34498d0 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -460,6 +460,7 @@ protected function login( throw new Error\Error('WRONGUSERPASS'); } + $validPasswordHashFound = false; $passwordHash = null; foreach ($data as $row) { if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) { @@ -470,8 +471,9 @@ protected function login( )); throw new Error\Error('WRONGUSERPASS'); } - if ($passwordHash === null) { + if (($passwordHash === null) && (strlen($row[$hashColumn]) > 0)) { $passwordHash = $row[$hashColumn]; + $validPasswordHashFound = true; } elseif ($passwordHash != $row[$hashColumn]) { Logger::error(sprintf( 'sqlauth:%s: column %s must be THE SAME in every result tuple.', @@ -479,10 +481,17 @@ protected function login( $hashColumn, )); throw new Error\Error('WRONGUSERPASS'); + } elseif (strlen($row[$hashColumn]) === 0) { + Logger::error(sprintf( + 'sqlauth:%s: column `%s` must contain a valid password hash.', + $this->authId, + $hashColumn, + )); + throw new Error\Error('WRONGUSERPASS'); } } - if ($passwordHash == null || (!password_verify($password, $passwordHash))) { + if ((!$validPasswordHashFound) || (!password_verify($password, $passwordHash))) { Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . ' password verification failed'); /* Authentication with verify_password() failed, however that only means that diff --git a/tests/src/Auth/Source/PasswordVerifyTest.php b/tests/src/Auth/Source/PasswordVerifyTest.php index 98feac0..4d3cb31 100644 --- a/tests/src/Auth/Source/PasswordVerifyTest.php +++ b/tests/src/Auth/Source/PasswordVerifyTest.php @@ -20,7 +20,7 @@ class PasswordVerifyTest extends TestCase /** @var array */ private array $info = ['AuthId' => 'testAuthId']; - /** @var array */ + /** @var array|string|null> */ private array $config = [ "dsn" => 'sqlite:file::memory:?cache=shared', "username" => "notused", diff --git a/tests/src/Auth/Source/SQL2MultipleAuthTest.php b/tests/src/Auth/Source/SQL2MultipleAuthTest.php index 9978838..391412e 100644 --- a/tests/src/Auth/Source/SQL2MultipleAuthTest.php +++ b/tests/src/Auth/Source/SQL2MultipleAuthTest.php @@ -24,6 +24,7 @@ */ class SQL2MultipleAuthTest extends TestCase { + /** @var array */ private array $info = ['AuthId' => 'testAuthId']; protected array $config = []; // Filled out in setUp() @@ -108,7 +109,7 @@ public function setUp(): void ], [ 'database' => 'studentsdb', - 'query' => "select unit_code from units_enrolled where studentid=:userid", + 'query' => "select unit_code from units_enrolled where studentid=:userid order by unit_code", 'only_for_auth' => ['auth_query_students'], ], ], @@ -259,7 +260,6 @@ public function testStudentLoginSuccess(): void $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('alice.gibson@student.example.edu', 'password'); asort($ret); $this->assertCount(7, $ret); - $this->assertCount(2, $ret['unit_code']); $this->assertEquals($ret, [ 'studentid' => ['1'], 'givenName' => ["Alice"], @@ -278,7 +278,6 @@ public function testNonPhysicsStaffLoginSuccess(): void $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('eve.evans@example.edu', 'password'); asort($ret); $this->assertCount(6, $ret); - $this->assertCount(1, $ret['role']); $this->assertEquals($ret, [ 'uid' => ['1'], 'givenName' => ["Eve"], @@ -296,7 +295,6 @@ public function testPhysicsStaffLoginSuccess(): void $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('mallory.mallory@example.edu', 'password'); asort($ret); $this->assertCount(8, $ret); - $this->assertCount(1, $ret['role']); $this->assertEquals($ret, [ 'uid' => ['2'], 'givenName' => ["Mallory"], diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php index 6517056..d198b8a 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php @@ -23,7 +23,7 @@ class SQL2PasswordVerifySimpleTest extends SQL2SimpleTest // We need to not specify the 'password=:password' clause in the WHERE clause, // as password_verify() does not work that way. - protected string $extraSqlAndClauses = ''; + protected string $extraSqlAndClauses = ' '; public function setUp(): void diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php index 36a799e..26f7378 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php @@ -26,6 +26,16 @@ class SQL2PasswordVerifySingleAuthTest extends SQL2SingleAuthTest protected string $extraSqlAndClauses = ''; + /** + * Different tests require different combinations of databases, auth queries and attr queries. + * This function returns a config with the requested number of each. + * + * @param int $numDatabases + * @param int $numAuthQueries + * @param array $authQueryAttributes + * @param int $numAttrQueries + * @return array + */ protected function getConfig( int $numDatabases, int $numAuthQueries, @@ -34,8 +44,10 @@ protected function getConfig( ): array { $config = parent::getConfig($numDatabases, $numAuthQueries, $authQueryAttributes, $numAttrQueries); - foreach ($config['auth_queries'] as &$query) { - $query['password_verify_hash_column'] = 'password'; + // @phpstan-ignore argument.type + foreach (array_keys($config['auth_queries']) as $authQueryName) { + // @phpstan-ignore offsetAccess.nonOffsetAccessible + $config['auth_queries'][$authQueryName]['password_verify_hash_column'] = 'password'; } return $config; diff --git a/tests/src/Auth/Source/SQL2SimpleTest.php b/tests/src/Auth/Source/SQL2SimpleTest.php index a000cbd..acac6d0 100644 --- a/tests/src/Auth/Source/SQL2SimpleTest.php +++ b/tests/src/Auth/Source/SQL2SimpleTest.php @@ -16,13 +16,14 @@ */ class SQL2SimpleTest extends TestCase { + /** @var array */ private array $info = ['AuthId' => 'testAuthId']; protected array $config = []; // Filled out in setUp() protected string $extraSqlSelectColumns = ''; - protected string $extraSqlAndClauses = ' and password=:password'; + protected string $extraSqlAndClauses = ' and password=:password '; public function setUp(): void @@ -193,10 +194,10 @@ public function testJoinSingleSuccess(): void $this->config['auth_queries']['auth_query']['query'] = " select u.givenName, u.email, ug.groupname" . $this->extraSqlSelectColumns . " from users u left join usergroups ug on (u.uid=ug.uid) - where u.uid=:username" . $this->extraSqlAndClauses; + where u.uid=:username" . $this->extraSqlAndClauses . + "order by ug.groupname"; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -228,13 +229,14 @@ public function testMultiQuerySuccess(): void $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', - 'query' => "select groupname from usergroups where uid=:username", + 'query' => + "select groupname from usergroups where uid=:username " . + "order by groupname", ], ]; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -271,17 +273,22 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', - 'query' => "select groupname from usergroups where uid=:username and groupname like '%nomatch%'", + 'query' => + "select groupname from usergroups " . + "where uid=:username and groupname like '%nomatch%' " . + "order by groupname", ], [ 'database' => 'defaultdb', - 'query' => "select groupname from usergroups where uid=:username and groupname like 'stud%'", + 'query' => + "select groupname from usergroups " . + "where uid=:username and groupname like 'stud%' " . + "order by groupname", ], ]; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -300,16 +307,21 @@ public function testMultiQuerySubsequentAppendSuccess(): void $this->config['attr_queries'] = [ [ 'database' => 'defaultdb', - 'query' => "select groupname from usergroups where uid=:username and groupname like 'stud%'", + 'query' => + "select groupname from usergroups " . + "where uid=:username and groupname like 'stud%'" . + " order by groupname", ], [ 'database' => 'defaultdb', - 'query' => "select groupname from usergroups where uid=:username and groupname like '%sers'", + 'query' => + "select groupname from usergroups " . + "where uid=:username and groupname like '%sers' " . + "order by groupname", ], ]; $ret = (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], diff --git a/tests/src/Auth/Source/SQL2SingleAuthTest.php b/tests/src/Auth/Source/SQL2SingleAuthTest.php index d92253a..54b4924 100644 --- a/tests/src/Auth/Source/SQL2SingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2SingleAuthTest.php @@ -18,6 +18,7 @@ */ class SQL2SingleAuthTest extends TestCase { + /** @var array */ private array $info = ['AuthId' => 'testAuthId']; protected string $extraSqlSelectColumns = ''; @@ -25,8 +26,15 @@ class SQL2SingleAuthTest extends TestCase protected string $extraSqlAndClauses = ' and password=:password'; - /* Different tests require different combinations of databases, auth queries and attr queries. + /** + * Different tests require different combinations of databases, auth queries and attr queries. * This function returns a config with the requested number of each. + * + * @param int $numDatabases + * @param int $numAuthQueries + * @param array $authQueryAttributes + * @param int $numAttrQueries + * @return array */ protected function getConfig( int $numDatabases, @@ -372,7 +380,6 @@ public function testMultipleAuthQueryStudentWithMultipleEnrolmentsSuccess(): voi $ret = (new SQL2Wrapper($this->info, $config))->callLogin('3', 'password'); asort($ret); $this->assertCount(6, $ret); - $this->assertCount(2, $ret['unit_code']); $this->assertEquals($ret, [ 'uid' => ['3'], 'email' => ['trudy@example.com'], @@ -412,7 +419,6 @@ public function testMultipleAuthQueryStudentWithSingleEnrolmentSuccess(): void $ret = (new SQL2Wrapper($this->info, $config))->callLogin('5', 'password'); asort($ret); $this->assertCount(6, $ret); - $this->assertCount(1, $ret['unit_code']); $this->assertEquals($ret, [ 'uid' => ['5'], 'email' => ['mallory@example.com'], diff --git a/tests/src/Auth/Source/SQLTest.php b/tests/src/Auth/Source/SQLTest.php index b6b87f6..799b614 100644 --- a/tests/src/Auth/Source/SQLTest.php +++ b/tests/src/Auth/Source/SQLTest.php @@ -20,7 +20,7 @@ class SQLTest extends TestCase /** @var array */ private array $info = ['AuthId' => 'testAuthId']; - /** @var array */ + /** @var array|string|null> */ private array $config = [ "dsn" => 'sqlite:file::memory:?cache=shared', "username" => "notused", From 51c60e7f284520425f357a0fe791f919288a519b Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 21 Oct 2025 14:41:27 +1100 Subject: [PATCH 08/28] Provide documentation for the Version 2 configuration format --- docs/sql.md | 552 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 497 insertions(+), 55 deletions(-) diff --git a/docs/sql.md b/docs/sql.md index 8c9328d..4521be3 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -1,21 +1,390 @@ -`sqlauth:SQL` -============= +# `sqlauth:SQL` -These are authentication modules for authenticating a user against a -SQL database. -The SQL module performs password verification in the database itself -using database functions such as sha512 and storing a salt in the -database. The PasswordVerify module verifies passwords in php using -the password_verify() function. The PasswordVerify module was created -to ask the least of the database either because there is minimal -support in the database or to allow the same code to work against many -databases without modification. More information on PasswordVerify is -provided at the end of this document. +These are authentication modules for authenticating a user against and retrieving attributes from an SQL database. -Options -------- +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. + +The Version 1 configuration support comes in two flavours (but identical configurations): +- `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. + +You enable the module in `config/config.php`. +```php + 'module.enable' => [ + [...] + 'sqlauth' => true, + [...] + ], +``` + +## Basic Concepts + +The basic concepts of how `sqlauth` works is common between versions 1 and 2. + +`Authentication Query` +: An SQL query which takes the parameters `:username` and `:password`, which are evaluated by the query for authentication purposes. If the username/password is incorrect, the "authentication query" **must** return no rows. If the "authentication query" returns one or more rows, authentication is deemed to succeed (ie. the username/password were correct). The resulting rows returned represent SAML attributes to be returned. Version 1 supports only one authentication query, whereas Version 2 supports one or more. + +`Attribute Query` +: Optional SQL queries executed after the authentication queries are executed. The resulting rows returned represent SAML attributes to be returned. If no rows are returned, this is not an error condition - it just doesn't add any extra SAML attributes to be returned. + +- Authentication queries. If this returns zero rows, authentication fails. If it returns more than one row, authentication is deemed to succeed. the parameters `:username` and `:password` are available and should be evaluated by the query for authentication purposes. Each column returned becomes an attribute.The rows returned represent attributes to be returned. +- Zero or more Attribute queries. All columns returned become attributes. Duplicates are supressed. Arrays with multiple values come from multiple rows being returned.The rows returned represent attributes to be returned. + +As a worked example, consider the following example table useful for authentication: + +| uid | password | salt | givenName | email | +|-----|----------|------|-----------|-----------------| +| bob | ******** | **** | Bob | bob@example.com | + +and another table (potentially in a completely separate database) which has attributes we want to return: + +| uid | groupName | +|-----|-----------| +| bob | users | +| bob | staff | + +An example authentication query might be: + +```sql +select uid, givenName as \"givenName\", email from users where uid=:username and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64') +``` + +And we could use an attribute query like: + +```sql +select groupName from usergroups where uid=:username +``` + +Note: Depending upon configuration, attibute queries using the Version 2 configuration format may use `:username` or `:userid`. See the Version 2 section below for more details. + +In summary: + +- If the authentication query returns no rows, that indicates authentication failed. +- The authentication query is passed `:username` and `:password` query parameters to do authentication. +- If more than one query is desirable or required to get all of the attributes, you can use attribute queries to capture those. In this case, the result set of the attribute queries in that array provide attributes only - only the authentication query is used to determine if the username/password is correct or not, and as such `:password` is not passed to attribute queries. +- Because attribute queries have no role in authentication, these queries are allowed to return no rows, simply indicating that query should have no effect on the final returned attribute set. +- If any query returns multiple rows, they will be merged into the attributes. +- The column names are used for the attribute names. Some databases will lowercase all column names unless you specify a seemingly unneeded "as" clause (eg. `select givenName as \"givenName\"`). SAML is case sensitive in attribute names, so this matters. +- If multiple queries return the same column names, they will also be merged into the same attributes. +- Duplicate values and NULL values will be removed. + + +## Version 2 Configuration Format + +The Version 2 configuration format supports: +- One or more database connections. +- One or more authentication queries using any database defined in the `databases` section. +- Zero or more attribute queries. Each query can use any database defined in the database section, and can be restricted to apply only to one or more authentication queries. + +All configuration for this module is done in `authsources.php`. A trivial example with a single database, only a single authentication query and no extra attribute queries: + +```php +$config = [ + [...] + 'example-sql' => [ + 'sqlauth:SQL2', + + 'databases' => [ + 'idp' => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=simplesaml', + 'username' => 'simplesaml', + 'password' => 'secretpassword', + ], + ] + + 'auth_queries' => [ + 'auth_username' => [ + 'database' => 'idp', + 'query' => "select uid, givenName as \"givenName\", email from users where uid=:username and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')", + ], + ], + ], + [...] +]; +``` + +Assuming the correct `:username` and `:password` are passed, the resulting SAML attributes returned by this configuration would be: + +| Attribute Name | Attribute Value | +|----------------|-----------------| +| uid | [ bob ] | +| givenName | [ Bob ] | +| email | [ bob@example.com ] | + + +It's really easy to add extra attributes by adding one or more attribute queries: + +```php +$config = [ + [...] + 'example-sql' => [ + 'sqlauth:SQL2', + + 'databases' => [ + 'idp' => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=simplesaml', + 'username' => 'simplesaml', + 'password' => 'secretpassword', + ], + ] + + 'auth_queries' => [ + 'auth_username' => [ + 'database' => 'idp', + 'query' => "select uid, givenName as \"givenName\", email from users where uid=:username and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')" + ], + ], + + 'attr_queries' => [ + [ + 'database' => 'idp', + 'query' => "select groupName from usergroups where uid=:username", + ], + ], + ], + [...] +]; +``` + +Assuming the correct `:username` and `:password` are passed, the resulting SAML attributes returned by this configuration would be: + +| Attribute Name | Attribute Value | +|----------------|-----------------| +| uid | [ bob ] | +| givenName | [ Bob ] | +| email | [ bob@example.com ] | +| groupName | [ users, staff ] | + + +In the below example, we have users in two separate databases and two authentication queries. Authentication queries are run in the order they are configured, and once an authentication query successfully authenticates a user it is deemed to be authenticated using that query, and no further authentication queries are run. In the below case, the username formats are defined (single lower case word for staff, suppliers have a "supp_" prefix), and as a result we can use the optional `username_regex` parameter to get a slight performance boost out of not running unneccessary queries. + +```php +$config = [ + [...] + 'example-sql' => [ + 'sqlauth:SQL2', + + 'databases' => [ + 'staff' => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=staff', + 'username' => 'simplesaml', + 'password' => 'secretpassword', + ], + + 'suppliers' => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=suppliers', + 'username' => 'anotheruser', + 'password' => 'somepassword', + ], + ] + + 'auth_queries' => [ + 'auth_username' => [ + 'database' => 'staff', + 'query' => "select uid, givenName as \"givenName\", email from users where uid=:username and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')" + 'username_regex' => '/^[a-z]+$/', // Username will only be acceptable if it is a single lower case word + ], + + 'auth_supplier' => [ + 'database' => 'suppliers', + 'query' => "select supplierId as \"uid\", supplierName as \"givenName\", email from suppliers where supplierId=:username and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')" + 'username_regex' => '/^supp_[a-z]+$/', // Suppliers have a "supp_" prefix + ] + ], + ], + [...] +]; +``` + +An example staff login with the above configuration might result in SAML attribues like: + +| Attribute Name | Attribute Value | +|----------------|-----------------------| +| uid | [ brian ] | +| givenName | [ Brian ] | +| email | [ brian@example.com ] | + + +The next example shows a case where we have a single database we are authenticating against, but are aggregating attributes from a number of different databases. In such cases it is common that users might login with an email address, however the shared User ID between databases is some other ID. To support this, the `extract_userid` takes the value from this other ID field in the authentication query and makes it available as `:userid` in the attribute queries instead of `:username`. + +```php +$config = [ + [...] + 'example-sql' => [ + "databases" => [ + "authdb" => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=authdb', + 'username' => 'someuser', + 'password' => 'somepassword', + ], + "staffdb" => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=staffdb', + 'username' => 'anotheruser', + 'password' => 'anotherpassword', + ], + "studentsdb" => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=studentsdb', + 'username' => 'differentuser', + 'password' => 'differentpassword', + ], + ], + "auth_queries" => [ + "auth_query_email" => [ + "database" => "authdb", + "query" => + "select uid, givenName, email " + "from users where email=:username " + "and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/', + "extract_userid_from" => 'uid', + ], + ], + "attr_queries" => [ + [ + 'database' => 'staffdb', + 'query' => "select department, role from staff where uid=:userid", + ], + [ + 'database' => 'studentsdb', + 'query' => "select course, year from students where uid=:userid", + ], + ], + ], + [...] +]; +``` + +A staff member authenticating might return SAML attributes like: + +| Attribute Name | Attribute Value | +|----------------|-----------------------| +| uid | [ 10543 ] | +| givenName | [ Brian ] | +| email | [ brian@example.edu ] | +| department | [ Physics ] | +| role | [ Lecturer ] | + +and a student might look like: + +| Attribute Name | Attribute Value | +|----------------|------------------------------| +| uid | [ 20625 ] | +| givenName | [ Jane ] | +| email | [ jane@student.example.edu ] | +| course | [ Mathematics ] | +| year | [ 2 ] | + + +When you've got more than one authentication query, it is possible to restrict attribute queries to only run for certain authentication queries using the `only_for_auth` attribute query configuration parameter: + +```php +$this->config = [ + 'example-sql' => [ + [...] + "databases" => [ + "staffdb" => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=staffdb', + 'username' => 'anotheruser', + 'password' => 'anotherpassword', + ], + "studentsdb" => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=studentsdb', + 'username' => 'differentuser', + 'password' => 'differentpassword', + ], + "auth_queries" => [ + "auth_query_students" => [ + "database" => "studentsdb", + "query" => + "select studentid, givenName, lastName, email, course, year " . + "from students where email=:username " + "and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@student\.example\.edu$/', + "extract_userid_from" => 'studentid', + ], + "auth_query_staff" => [ + "database" => "staffdb", + "query" => + "select uid, givenName, lastName, email, department " . + "from staff where email=:username " + "and password=encode(sha512(concat((select salt from users where uid=1), :password)::bytea), 'base64')", + "username_regex" => '/^[a-zA-Z0-9._%+-]+@example\.edu$/', + "extract_userid_from" => 'uid', + ], + ], + "attr_queries" => [ + [ + 'database' => 'staffdb', + 'query' => "select role from staff_roles where uid=:userid", + 'only_for_auth' => ['auth_query_staff' ], + ], + [ + 'database' => 'studentsdb', + 'query' => "select unit_code from units_enrolled where studentid=:userid order by unit_code", + 'only_for_auth' => ['auth_query_students'], + ], + ], + ], + ], + [...] +]; +``` + +A staff member authenticating might return SAML attributes like: + +| Attribute Name | Attribute Value | +|----------------|-----------------------| +| uid | [ 10543 ] | +| givenName | [ Brian ] | +| lastName | [ Perkins ] | +| email | [ brian@example.edu ] | +| department | [ Physics ] | +| role | [ Lecturer, Tutor] | + +and a student might look like: + +| Attribute Name | Attribute Value | +|----------------|------------------------------| +| studentid | [ 20625 ] | +| givenName | [ Jane ] | +| lastName | [ Smith ] | +| email | [ jane@student.example.edu ] | +| course | [ Mathematics ] | +| year | [ 2 ] | +| unit_code | [ MATH201, MATH202, MATH203] | + + + +### Configuration Parameter Dictionary + +There are three sections in the configuration, as follows: + +```php +$this->config = [ + [...] + 'example-sql' => [ + "databases" => [ + // One or more databases + ], + "auth_queries" => [ + // One or more Authentication Queries + ], + "attr_queries" => [ + // Zero or more Attribute Queries + ], + ], + [...] +]; +``` + +#### databases `dsn` : The DSN which should be used to connect to the database server. Check the various database drivers in the [PHP documentation](http://php.net/manual/en/pdo.drivers.php) for a description of the various DSN formats. @@ -26,19 +395,42 @@ Options `password` : The password which should be used when connecting to the database server. + +#### auth_queries + +`database` +: ID of the database in the `databases` configuration (previous section) that this authentication query should run on. + `query` -: The SQL query or queries which should be used to authenticate the user and retrieve their attributes. +: The SQL query which should be used to authenticate the user and retrieve attributes. This query is passed the `:username` and `:password` SQL parameters. `username_regex` : (Optional) A regular expression that the username must match. Useful if the type of the username column in the database isn't a string (eg. an integer), or if the format is well known (eg. email address, single word with no spaces, etc) to avoid going to the database for a query that will never result in successful authentication. -`passwordhashcolumn` -: (Optional) Only When using the sqlauth:PasswordVerify module. This is the name of the column that contains the hashed password. The default is to look for a column 'passwordhash' in the database. +`extract_userid_from` +: (Optional) If the username from the authentication is not the ID used in the attribute queries. A common example is where they login with their email address (ie. `:username` is an email address), but their real user ID is in a different column. In that case, specify the column their real user ID is in. + +`password_verify_hash_column` +: (Optional) See the section at the bottom of this page covering Password Verify support. + +#### attr_queries + +`database` +: ID of the database in the `databases` configuration (previous section) that this authentication query should run on. + +`query` +: The SQL query which should be used to gather attributes with. This query is passed either the `:username` or `:userid` parameters - if the `extract_userid_from` parameter was specified in the authentication query, the `:userid` SQL parameter will be passed to the query. Otherwise, `:username` is passed as an SQL parameter. + +`only_for_auth` +: (Optional) Only run the attribute query if the user authenticated using one of the authentication queries referenced in this list. + -Writing a Query / Queries -------------------------- +## Version 1 Configuration Format -A `query` can be either a single string with an SQL statement, or an array of queries, run in order. That single string (or the first query in the array) is the "authentication query" - the parameters `:username` and `:password` are available and should be evaluated by the query for authentication purposes. If the username/password is incorrect, the "authentication query" should return no rows. The rows returned represent attributes to be returned. +The Version 1 format is more basic, both in terms of configuration and different use cases it supports. Specifically, it supports: +- One database only +- One authentication query +- Zero or more attribute queries Taking this example schema: @@ -130,8 +522,29 @@ In summary: - If multiple queries return the same column names, they will also be merged into the same attributes. - Duplicate values and NULL values will be removed. -Further Examples ----------------- + +### Configuration Parameter Dictionary + +`dsn` +: The DSN which should be used to connect to the database server. + Check the various database drivers in the [PHP documentation](http://php.net/manual/en/pdo.drivers.php) for a description of the various DSN formats. + +`username` +: The username which should be used when connecting to the database server. + +`password` +: The password which should be used when connecting to the database server. + +`query` +: Either a single string with an SQL statement, or an array of queries, run in order. That single string (or the first query in the array) is the "authentication query" - the parameters `:username` and `:password` are available and should be evaluated by the query for authentication purposes. If the username/password is incorrect, the "authentication query" should return no rows. The rows returned represent attributes to be returned. + +`username_regex` +: (Optional) A regular expression that the username must match. Useful if the type of the username column in the database isn't a string (eg. an integer), or if the format is well known (eg. email address, single word with no spaces, etc) to avoid going to the database for a query that will never result in successful authentication. + +`passwordhashcolumn` +: (Optional) Only When using the sqlauth:PasswordVerify module. This is the name of the column that contains the hashed password. The default is to look for a column 'passwordhash' in the database. See the section at the bottom of this page covering Password Verify support. + +## Further Authentication Query Examples ```sql Example query - SHA256 of salt + password, with the salt stored in an independent column, MySQL server: @@ -180,24 +593,38 @@ Example query - SHA512 of salt + password, stored as salt (32 bytes) + sha256(sa ) ``` -Connecting with UNIX Domain Sockets (Local Sockets) ---------------------------------------------------- +## Connecting with UNIX Domain Sockets (Local Sockets) When on a UNIX-like platform (Linux, *BSD, etc), and when your SQL database server is running on the same host as the web server hosting SimpleSAMLphp, it is possible to use UNIX domain sockets instead of TCP sockets for the database connection. This configuration should result in marginally better performance and security (when configured correctly). -Here is an example using PostgreSQL: +Here is an example Version 2 configuration using PostgreSQL: ```php - 'example-unix-socket-sql' => [ - 'sqlauth:SQL', - 'dsn' => 'pgsql:host=/var/run/postgresql;dbname=simplesaml', - 'username' => 'www-data', - 'password' => 'this-is-ignored', - 'query' => 'SELECT uid, givenName, email, eduPersonPrincipalName FROM users WHERE uid = :username ' . - 'AND password = SHA2(CONCAT((SELECT salt FROM users WHERE uid = :username), :password), 256);', +$config = [ + [...] + 'example-sql' => [ + 'sqlauth:SQL2', + + 'databases' => [ + 'idp' => [ + 'dsn' => 'pgsql:host=/var/run/postgresql;dbname=simplesaml', + 'username' => 'www-data', + 'password' => 'this-is-ignored', + ], + ] + + 'auth_queries' => [ + 'auth_username' => [ + 'database' => 'idp', + 'query' => 'SELECT uid, givenName, email, eduPersonPrincipalName FROM users WHERE uid = :username ' . + 'AND password = SHA2(CONCAT((SELECT salt FROM users WHERE uid = :username), :password), 256);', + ], + ], ], + [...] +]; ``` Configuration is largely the same as TCP sockets (documented above), with the differences being: @@ -211,8 +638,7 @@ Configuration is largely the same as TCP sockets (documented above), with the di `password` : Required, but the value you specify is ignored (so you can put any placeholder string value in there). All authentication for UNIX domain sockets are done by the operating system kernel. -Security considerations ------------------------ +## Security considerations Please never store passwords in plaintext in a database. You should always hash your passwords with a secure one-way function like the ones in the SHA2 family. Use randomly generated salts with a length at least equal to the hash of the @@ -223,12 +649,13 @@ used instead as an additional security measure. One way hashing algorithms like MD5 or SHA1 are considered insecure and should therefore be avoided. -The PasswordVerify module -------------------------- +## `password_verify()` support + +A common one-way password hashing function is the [crypt](https://www.php.net/manual/en/function.crypt.php) function that `libc` on UNIX has provided natively for decades. PHP provides a useful [password_verify()](https://www.php.net/password_verify) function to authenticate a password against a previously stored `crypt` hash. Hashes can be created in PHP using the [password_hash()](https://www.php.net/password_hash) function. -Users and passwords have to be set in the database by other means than the PasswordVerify module. +In doing this, the authentication query no longer actually does the authentication - it returns the password hash. As a result, the authentication query is no longer passed the `:password` parameter. -For example: +Given the SQL schema: ```sql CREATE TABLE users ( @@ -240,26 +667,35 @@ For example: ); ``` -A user can be added with a known password "FIXMEPASSWORD" as shown below. +the Version 2 configuration parameter `password_verify_hash_column` specifies which column has the `crypt` hash: ```php -$dsn = "pgsql:host=..."; -$username = "fixme"; -$password = ""; -$options = array(); - -$query = "insert into users values ('test@example.com',:passwordhash, 'test', 'test@example.com', 'test@example.com' )"; - -$db = new PDO($dsn, $username, $password, $options); -$db->exec("SET NAMES 'UTF8'"); - -$params = ["passwordhash" => password_hash("FIXMEPASSWORD", PASSWORD_ARGON2ID ) ]; -$sth = $db->prepare($query); -$sth->execute($params); +$config = [ + [...] + 'example-sql' => [ + 'sqlauth:SQL2', + + 'databases' => [ + 'idp' => [ + 'dsn' => 'pgsql:host=postgresql;port=5432;dbname=simplesaml', + 'username' => 'simplesaml', + 'password' => 'secretpassword', + ], + ] + + 'auth_queries' => [ + 'auth_username' => [ + 'database' => 'idp', + 'query' => "select uid, email, passwordhash, eduPersonPrincipalName from users where uid = :username", + 'password_verify_hash_column' => 'passwordhash', + ], + ], + ], + [...] +]; ``` -Since the above is using the default passwordhash column name this can -then be used with the following addition to authsources.php. +The equivalent Version 1 configuration has slight differences, in that `PasswordVerify` was a separate module in Version 1 (whereas it is supported in them main `SQL2` Version 2 module), and the `passwordhashcolumn` parameter specifies the column that has the `crypt` hash: ```php 'smalldb-dbauth' => [ @@ -267,8 +703,14 @@ then be used with the following addition to authsources.php. 'dsn' => 'pgsql:host=...', 'username' => 'dbuser', 'password' => 'dbpassword', - 'passwodhashcolumn' => 'passwordhash', + 'passwordhashcolumn' => 'passwordhash', 'query' => 'select uid, email, passwordhash, eduPersonPrincipalName from users where uid = :username ', ], ``` + +In both cases, the authentication query must return the column referenced by the `password_verify_hash_column` (Version 2) or `passwordhashcolumn` (Version 1). `sqlauth` will then call [password_verify()](https://www.php.net/password_verify) with that hash and the user provided password to determine whether authentication is successful. + +If the authentication is successful, all attributes returned by the authentication query are returned as SAML attributes (as per any other authentication query) **except the password hash column**. This is dropped and not exposed as a SAML attribute for security reasons. + +Note: An inconsistency between Version 1 and Version 2 configurations is that the Version 1 had `passwordhashcolumn` being an optional element with a default value of `passwordhash`. With Version 2, [password_verify()](https://www.php.net/password_verify) support is enabled by specifying the optional `password_verify_hash_column` configuration parameter, hence it does not have a default value. From 92221a03e19a601520cd72a34b5a2c44e6f7c153 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 22 Oct 2025 12:03:13 +1100 Subject: [PATCH 09/28] Fix linting for README --- docs/sql.md | 45 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/docs/sql.md b/docs/sql.md index 4521be3..9ae4468 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -1,21 +1,23 @@ # `sqlauth:SQL` - These are authentication modules for authenticating a user against and retrieving attributes from an SQL database. 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. The Version 1 configuration support comes in two flavours (but identical configurations): + - `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. You enable the module in `config/config.php`. + ```php 'module.enable' => [ [...] @@ -39,9 +41,9 @@ The basic concepts of how `sqlauth` works is common between versions 1 and 2. As a worked example, consider the following example table useful for authentication: -| uid | password | salt | givenName | email | -|-----|----------|------|-----------|-----------------| -| bob | ******** | **** | Bob | bob@example.com | +| uid | password | salt | givenName | email | +|-----|----------|------|-----------|-------------------| +| bob | ******** | **** | Bob | "bob@example.com" | and another table (potentially in a completely separate database) which has attributes we want to return: @@ -75,10 +77,10 @@ In summary: - If multiple queries return the same column names, they will also be merged into the same attributes. - Duplicate values and NULL values will be removed. - ## Version 2 Configuration Format The Version 2 configuration format supports: + - One or more database connections. - One or more authentication queries using any database defined in the `databases` section. - Zero or more attribute queries. Each query can use any database defined in the database section, and can be restricted to apply only to one or more authentication queries. @@ -112,13 +114,12 @@ $config = [ Assuming the correct `:username` and `:password` are passed, the resulting SAML attributes returned by this configuration would be: -| Attribute Name | Attribute Value | -|----------------|-----------------| -| uid | [ bob ] | -| givenName | [ Bob ] | +| Attribute Name | Attribute Value | +|----------------|---------------------| +| uid | [ bob ] | +| givenName | [ Bob ] | | email | [ bob@example.com ] | - It's really easy to add extra attributes by adding one or more attribute queries: ```php @@ -155,13 +156,12 @@ $config = [ Assuming the correct `:username` and `:password` are passed, the resulting SAML attributes returned by this configuration would be: -| Attribute Name | Attribute Value | -|----------------|-----------------| -| uid | [ bob ] | -| givenName | [ Bob ] | +| Attribute Name | Attribute Value | +|----------------|---------------------| +| uid | [ bob ] | +| givenName | [ Bob ] | | email | [ bob@example.com ] | -| groupName | [ users, staff ] | - +| groupName | [ users, staff ] | In the below example, we have users in two separate databases and two authentication queries. Authentication queries are run in the order they are configured, and once an authentication query successfully authenticates a user it is deemed to be authenticated using that query, and no further authentication queries are run. In the below case, the username formats are defined (single lower case word for staff, suppliers have a "supp_" prefix), and as a result we can use the optional `username_regex` parameter to get a slight performance boost out of not running unneccessary queries. @@ -211,7 +211,6 @@ An example staff login with the above configuration might result in SAML attribu | givenName | [ Brian ] | | email | [ brian@example.com ] | - The next example shows a case where we have a single database we are authenticating against, but are aggregating attributes from a number of different databases. In such cases it is common that users might login with an email address, however the shared User ID between databases is some other ID. To support this, the `extract_userid` takes the value from this other ID field in the authentication query and makes it available as `:userid` in the attribute queries instead of `:username`. ```php @@ -281,7 +280,6 @@ and a student might look like: | course | [ Mathematics ] | | year | [ 2 ] | - When you've got more than one authentication query, it is possible to restrict attribute queries to only run for certain authentication queries using the `only_for_auth` attribute query configuration parameter: ```php @@ -360,9 +358,7 @@ and a student might look like: | year | [ 2 ] | | unit_code | [ MATH201, MATH202, MATH203] | - - -### Configuration Parameter Dictionary +### Configuration Parameter Dictionary (Version 2) There are three sections in the configuration, as follows: @@ -385,6 +381,7 @@ $this->config = [ ``` #### databases + `dsn` : The DSN which should be used to connect to the database server. Check the various database drivers in the [PHP documentation](http://php.net/manual/en/pdo.drivers.php) for a description of the various DSN formats. @@ -395,7 +392,6 @@ $this->config = [ `password` : The password which should be used when connecting to the database server. - #### auth_queries `database` @@ -424,10 +420,10 @@ $this->config = [ `only_for_auth` : (Optional) Only run the attribute query if the user authenticated using one of the authentication queries referenced in this list. - ## Version 1 Configuration Format The Version 1 format is more basic, both in terms of configuration and different use cases it supports. Specifically, it supports: + - One database only - One authentication query - Zero or more attribute queries @@ -522,8 +518,7 @@ In summary: - If multiple queries return the same column names, they will also be merged into the same attributes. - Duplicate values and NULL values will be removed. - -### Configuration Parameter Dictionary +### Configuration Parameter Dictionary (Version 1) `dsn` : The DSN which should be used to connect to the database server. From bac816c70a09fccb3de8bf9c7b8e3424a0d2ba0f Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 22 Oct 2025 12:10:10 +1100 Subject: [PATCH 10/28] Fix linting --- docs/sql.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sql.md b/docs/sql.md index 9ae4468..d70a1c3 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -43,7 +43,7 @@ As a worked example, consider the following example table useful for authenticat | uid | password | salt | givenName | email | |-----|----------|------|-----------|-------------------| -| bob | ******** | **** | Bob | "bob@example.com" | +| bob | ******** | **** | Bob | | and another table (potentially in a completely separate database) which has attributes we want to return: From 80574d5f7a590ad66763b13bb0d9bfcda388a023 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 22 Oct 2025 12:12:24 +1100 Subject: [PATCH 11/28] Spelling fixes --- docs/sql.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sql.md b/docs/sql.md index d70a1c3..456a207 100644 --- a/docs/sql.md +++ b/docs/sql.md @@ -37,7 +37,7 @@ The basic concepts of how `sqlauth` works is common between versions 1 and 2. : Optional SQL queries executed after the authentication queries are executed. The resulting rows returned represent SAML attributes to be returned. If no rows are returned, this is not an error condition - it just doesn't add any extra SAML attributes to be returned. - Authentication queries. If this returns zero rows, authentication fails. If it returns more than one row, authentication is deemed to succeed. the parameters `:username` and `:password` are available and should be evaluated by the query for authentication purposes. Each column returned becomes an attribute.The rows returned represent attributes to be returned. -- Zero or more Attribute queries. All columns returned become attributes. Duplicates are supressed. Arrays with multiple values come from multiple rows being returned.The rows returned represent attributes to be returned. +- Zero or more Attribute queries. All columns returned become attributes. Duplicates are suppressed. Arrays with multiple values come from multiple rows being returned.The rows returned represent attributes to be returned. As a worked example, consider the following example table useful for authentication: @@ -64,7 +64,7 @@ And we could use an attribute query like: select groupName from usergroups where uid=:username ``` -Note: Depending upon configuration, attibute queries using the Version 2 configuration format may use `:username` or `:userid`. See the Version 2 section below for more details. +Note: Depending upon configuration, attribute queries using the Version 2 configuration format may use `:username` or `:userid`. See the Version 2 section below for more details. In summary: @@ -163,7 +163,7 @@ Assuming the correct `:username` and `:password` are passed, the resulting SAML | email | [ bob@example.com ] | | groupName | [ users, staff ] | -In the below example, we have users in two separate databases and two authentication queries. Authentication queries are run in the order they are configured, and once an authentication query successfully authenticates a user it is deemed to be authenticated using that query, and no further authentication queries are run. In the below case, the username formats are defined (single lower case word for staff, suppliers have a "supp_" prefix), and as a result we can use the optional `username_regex` parameter to get a slight performance boost out of not running unneccessary queries. +In the below example, we have users in two separate databases and two authentication queries. Authentication queries are run in the order they are configured, and once an authentication query successfully authenticates a user it is deemed to be authenticated using that query, and no further authentication queries are run. In the below case, the username formats are defined (single lower case word for staff, suppliers have a "supp_" prefix), and as a result we can use the optional `username_regex` parameter to get a slight performance boost out of not running unnecessary queries. ```php $config = [ @@ -203,7 +203,7 @@ $config = [ ]; ``` -An example staff login with the above configuration might result in SAML attribues like: +An example staff login with the above configuration might result in SAML attributes like: | Attribute Name | Attribute Value | |----------------|-----------------------| From 693a25547189508c4119bb1ad51238607d339a0f Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 22 Oct 2025 13:12:36 +1100 Subject: [PATCH 12/28] Run phpcs from vendor, not GitHub installed version --- .github/workflows/php.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 7c98970..0d7c5eb 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -50,7 +50,7 @@ jobs: with: # Should be the higest supported version, so we can use the newest tools php-version: '8.4' - tools: composer, composer-require-checker, composer-unused, phpcs, phpstan + tools: composer, composer-require-checker, composer-unused extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, pdo, posix, spl, xml - name: Setup problem matchers for PHP @@ -81,7 +81,8 @@ jobs: run: composer-unused - name: PHP Code Sniffer - run: phpcs + run: | + vendor/bin/phpcs - name: PHPStan run: | From 833cd52f8c74c48e8c80b4ff897841624f114936 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 22 Oct 2025 14:09:04 +1100 Subject: [PATCH 13/28] Fix deprecation warnings in phpunit, strangely causing phpunit failures on PHP 8.1 and 8.2 --- tests/src/Auth/Source/PasswordVerify1CompatTest.php | 6 +----- tests/src/Auth/Source/PasswordVerifyTest.php | 6 +----- tests/src/Auth/Source/SQL1CompatTest.php | 6 +----- tests/src/Auth/Source/SQL2MultipleAuthTest.php | 3 +-- .../src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php | 3 +-- tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php | 3 +-- tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php | 3 +-- tests/src/Auth/Source/SQL2SimpleTest.php | 3 +-- tests/src/Auth/Source/SQL2SingleAuthTest.php | 3 +-- tests/src/Auth/Source/SQLTest.php | 3 +-- 10 files changed, 10 insertions(+), 29 deletions(-) diff --git a/tests/src/Auth/Source/PasswordVerify1CompatTest.php b/tests/src/Auth/Source/PasswordVerify1CompatTest.php index 9aad2c3..b444aa7 100644 --- a/tests/src/Auth/Source/PasswordVerify1CompatTest.php +++ b/tests/src/Auth/Source/PasswordVerify1CompatTest.php @@ -4,11 +4,7 @@ namespace SimpleSAML\Test\Module\sqlauth\Auth\Source; -/** - * Test for the core:AttributeLimit filter. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit - */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\PasswordVerify1Compat::class) class PasswordVerify1CompatTest extends PasswordVerifyTest { protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\PasswordVerify1CompatWrapper'; diff --git a/tests/src/Auth/Source/PasswordVerifyTest.php b/tests/src/Auth/Source/PasswordVerifyTest.php index 4d3cb31..b680236 100644 --- a/tests/src/Auth/Source/PasswordVerifyTest.php +++ b/tests/src/Auth/Source/PasswordVerifyTest.php @@ -7,11 +7,7 @@ use PDO; use PHPUnit\Framework\TestCase; -/** - * Test for the core:AttributeLimit filter. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit - */ +#CoversClass(\SimpleSAML\Module\sqlauth\Auth\Source\PasswordVerify::class) class PasswordVerifyTest extends TestCase { // Subclasses can override this to test other wrapper classes diff --git a/tests/src/Auth/Source/SQL1CompatTest.php b/tests/src/Auth/Source/SQL1CompatTest.php index ff8d03f..e9570b7 100644 --- a/tests/src/Auth/Source/SQL1CompatTest.php +++ b/tests/src/Auth/Source/SQL1CompatTest.php @@ -4,11 +4,7 @@ namespace SimpleSAML\Test\Module\sqlauth\Auth\Source; -/** - * Test for the core:AttributeLimit filter. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit - */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL1Compat::class) class SQL1CompatTest extends SQLTest { protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\SQL1CompatWrapper'; diff --git a/tests/src/Auth/Source/SQL2MultipleAuthTest.php b/tests/src/Auth/Source/SQL2MultipleAuthTest.php index 391412e..c964975 100644 --- a/tests/src/Auth/Source/SQL2MultipleAuthTest.php +++ b/tests/src/Auth/Source/SQL2MultipleAuthTest.php @@ -19,9 +19,8 @@ * Attributes for students come from just the student database, whereas the staff attributes * come from both staff databases if the user is in both, and only the main staff database * if they are not in the physics department. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL2::class) class SQL2MultipleAuthTest extends TestCase { /** @var array */ diff --git a/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php index 8e2fd52..5bfd843 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifyMultipleAuthTest.php @@ -8,9 +8,8 @@ /** * The scenario is SQL2MultipleAuthTest but with passwords hashed using password_hash() - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL2::class) class SQL2PasswordVerifyMultipleAuthTest extends SQL2MultipleAuthTest { // We need to return password column for password_verify() to use. diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php index d198b8a..2d686e3 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifySimpleTest.php @@ -13,9 +13,8 @@ * * The tests are essentially the same tests as SQLTest, but using the SQLv2 * configuration and code, not the SQL1 code or SQL1Compat interface. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL2::class) class SQL2PasswordVerifySimpleTest extends SQL2SimpleTest { // We need to return password column for password_verify() to use. diff --git a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php index 26f7378..22cd1ac 100644 --- a/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2PasswordVerifySingleAuthTest.php @@ -13,9 +13,8 @@ * but the common identifier across all databases is the userid (uid). * * The attributes then come from multiple databases. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL2::class) class SQL2PasswordVerifySingleAuthTest extends SQL2SingleAuthTest { // We need to return password column for password_verify() to use. diff --git a/tests/src/Auth/Source/SQL2SimpleTest.php b/tests/src/Auth/Source/SQL2SimpleTest.php index acac6d0..9abdb26 100644 --- a/tests/src/Auth/Source/SQL2SimpleTest.php +++ b/tests/src/Auth/Source/SQL2SimpleTest.php @@ -11,9 +11,8 @@ * The scenario for this test case is a single database of customers who have their * metadata in a single database. This is essentially the same tests as SQLTest, but * using the SQLv2 configuration and code, not the SQL1 code or SQL1Compat interface. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL2::class) class SQL2SimpleTest extends TestCase { /** @var array */ diff --git a/tests/src/Auth/Source/SQL2SingleAuthTest.php b/tests/src/Auth/Source/SQL2SingleAuthTest.php index 54b4924..c3470ca 100644 --- a/tests/src/Auth/Source/SQL2SingleAuthTest.php +++ b/tests/src/Auth/Source/SQL2SingleAuthTest.php @@ -13,9 +13,8 @@ * but the common identifier across all databases is the userid (uid). * * The attributes then come from multiple databases. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL2::class) class SQL2SingleAuthTest extends TestCase { /** @var array */ diff --git a/tests/src/Auth/Source/SQLTest.php b/tests/src/Auth/Source/SQLTest.php index 799b614..eb8d8df 100644 --- a/tests/src/Auth/Source/SQLTest.php +++ b/tests/src/Auth/Source/SQLTest.php @@ -9,9 +9,8 @@ /** * Test for the core:AttributeLimit filter. - * - * @covers \SimpleSAML\Module\core\Auth\Process\AttributeLimit */ +#CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL::class) class SQLTest extends TestCase { // Subclasses can override this to test other wrapper classes From 86da39e3ced8848b848260b7671fa4fc22176ca6 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 22 Oct 2025 14:41:12 +1100 Subject: [PATCH 14/28] composer install with "--prefer-dist" fails. Remove that flag. --- .github/workflows/php.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 0d7c5eb..c0671f3 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -240,7 +240,7 @@ jobs: restore-keys: ${{ runner.os }}-composer- - name: Install Composer dependencies - run: composer install --no-progress --prefer-dist --optimize-autoloader --ignore-platform-req=ext-posix + run: composer install --no-progress --optimize-autoloader --ignore-platform-req=ext-posix - name: Run unit tests run: vendor/bin/phpunit --no-coverage From 64142b92f8628a233b41a1921b2a8295522b68c6 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Thu, 23 Oct 2025 11:26:11 +1100 Subject: [PATCH 15/28] Have test classes use concrete interface to make it clearer to phpstan what we're doing --- phpstan-baseline-dev.neon | 34 +++--------- .../Auth/Source/PasswordVerify1CompatTest.php | 9 +++- .../Source/PasswordVerify1CompatWrapper.php | 2 +- tests/src/Auth/Source/PasswordVerifyTest.php | 48 +++++++++-------- .../src/Auth/Source/PasswordVerifyWrapper.php | 2 +- tests/src/Auth/Source/SQL1CompatTest.php | 9 +++- tests/src/Auth/Source/SQL1CompatWrapper.php | 2 +- tests/src/Auth/Source/SQL2Wrapper.php | 2 +- tests/src/Auth/Source/SQLTest.php | 52 ++++++++++--------- tests/src/Auth/Source/SQLWrapper.php | 2 +- tests/src/Auth/Source/WrapperInterface.php | 20 +++++++ 11 files changed, 101 insertions(+), 81 deletions(-) create mode 100644 tests/src/Auth/Source/WrapperInterface.php diff --git a/phpstan-baseline-dev.neon b/phpstan-baseline-dev.neon index f8d976b..b24421b 100644 --- a/phpstan-baseline-dev.neon +++ b/phpstan-baseline-dev.neon @@ -1,35 +1,13 @@ parameters: ignoreErrors: - - message: '#^Parameter \#1 \$array of function asort expects array, string given\.$#' - identifier: argument.type - count: 4 - path: tests/src/Auth/Source/PasswordVerifyTest.php - - - - message: '#^Property SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\PasswordVerifyTest\:\:\$config \(array\\) does not accept array\\|string\|null\>\.$#' - identifier: assign.propertyType - count: 4 - path: tests/src/Auth/Source/PasswordVerifyTest.php - - - - message: '#^Parameter \#1 \$array of function asort expects array, mixed given\.$#' - identifier: argument.type - count: 4 - path: tests/src/Auth/Source/SQLTest.php - - - - message: '#^Property SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\SQLTest\:\:\$config \(array\\) does not accept array\\|string\|null\>\.$#' - identifier: assign.propertyType - count: 4 - path: tests/src/Auth/Source/SQLTest.php - - - - message: "#Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQL2SimpleTest\\:\\:\\$config type has no value type specified in iterable type array\\.#" + message: '#^Property SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\SQL2MultipleAuthTest\:\:\$config type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue count: 1 - path: tests/src/Auth/Source/SQL2SimpleTest.php + path: tests/src/Auth/Source/SQL2MultipleAuthTest.php - - message: "#Property SimpleSAML\\\\Test\\\\Module\\\\sqlauth\\\\Auth\\\\Source\\\\SQL2MultipleAuthTest\\:\\:\\$config type has no value type specified in iterable type array\\.#" + message: '#^Property SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\SQL2SimpleTest\:\:\$config type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue count: 1 - path: tests/src/Auth/Source/SQL2MultipleAuthTest.php \ No newline at end of file + path: tests/src/Auth/Source/SQL2SimpleTest.php diff --git a/tests/src/Auth/Source/PasswordVerify1CompatTest.php b/tests/src/Auth/Source/PasswordVerify1CompatTest.php index b444aa7..5228766 100644 --- a/tests/src/Auth/Source/PasswordVerify1CompatTest.php +++ b/tests/src/Auth/Source/PasswordVerify1CompatTest.php @@ -7,5 +7,12 @@ #CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\PasswordVerify1Compat::class) class PasswordVerify1CompatTest extends PasswordVerifyTest { - protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\PasswordVerify1CompatWrapper'; + /** + * @param array $info + * @param array $config + */ + protected function createWrapper(array $info, array $config): WrapperInterface + { + return new PasswordVerify1CompatWrapper($info, $config); + } } diff --git a/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php b/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php index bb600e7..225eeea 100644 --- a/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php +++ b/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php @@ -13,7 +13,7 @@ * method in SQL.php */ -class PasswordVerify1CompatWrapper extends PasswordVerify1Compat +class PasswordVerify1CompatWrapper extends PasswordVerify1Compat implements WrapperInterface { /** * @param array $info diff --git a/tests/src/Auth/Source/PasswordVerifyTest.php b/tests/src/Auth/Source/PasswordVerifyTest.php index b680236..e4b0447 100644 --- a/tests/src/Auth/Source/PasswordVerifyTest.php +++ b/tests/src/Auth/Source/PasswordVerifyTest.php @@ -10,9 +10,6 @@ #CoversClass(\SimpleSAML\Module\sqlauth\Auth\Source\PasswordVerify::class) class PasswordVerifyTest extends TestCase { - // Subclasses can override this to test other wrapper classes - protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\PasswordVerifyWrapper'; - /** @var array */ private array $info = ['AuthId' => 'testAuthId']; @@ -80,11 +77,21 @@ public static function setUpBeforeClass(): void } + /** + * @param array $info + * @param array $config + */ + protected function createWrapper(array $info, array $config): WrapperInterface + { + return new PasswordVerifyWrapper($info, $config); + } + + public function testBasicSingleSuccess(): void { // Correct username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password1'); asort($ret); $this->assertCount(2, $ret); $this->assertEquals($ret, [ @@ -99,7 +106,7 @@ public function testBasicSingleFailedLogin(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -109,7 +116,7 @@ public function testBasicSingleFailedLoginNonExisting(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('henry', 'boo'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('henry', 'boo'); $this->assertCount(0, $ret); } @@ -119,7 +126,7 @@ public function testBasicSingleFailedLoginNonExistingNoPassword(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email, passwordhash from users where uid=:username"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice2', ''); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice2', ''); $this->assertCount(0, $ret); } @@ -130,10 +137,10 @@ public function testJoinSingleSuccess(): void $this->config['query'] = " select u.givenName, u.email, ug.groupname, passwordhash from users u left join usergroups ug on (u.uid=ug.uid) - where u.uid=:username "; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); + where u.uid=:username + order by ug.groupname"; + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password1'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -151,7 +158,7 @@ public function testJoinSingleFailedLogin(): void select u.givenName, u.email, ug.groupname, passwordhash from users u left join usergroups ug on (u.uid=ug.uid) where u.uid=:username"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -161,11 +168,10 @@ public function testMultiQuerySuccess(): void // Correct username/password $this->config['query'] = [ "select givenName, email, passwordhash from users where uid=:username", - "select groupname from usergroups where uid=:username", + "select groupname from usergroups where uid=:username order by groupname", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password1'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -183,7 +189,7 @@ public function testMultiQueryFailedLogin(): void "select givenName, email, passwordhash from users where uid=:username", "select groupname from usergroups where uid=:username", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -194,11 +200,10 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void $this->config['query'] = [ "select givenName, email, passwordhash from users where uid=:username", "select groupname from usergroups where uid=:username and groupname like '%nomatch%'", - "select groupname from usergroups where uid=:username and groupname like 'stud%'", + "select groupname from usergroups where uid=:username and groupname like 'stud%' order by groupname", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password1'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -213,12 +218,11 @@ public function testMultiQuerySubsequentAppendSuccess(): void // Correct username/password. Second query returns a row, third query appends one row $this->config['query'] = [ "select givenName, email, passwordhash from users where uid=:username", - "select groupname from usergroups where uid=:username and groupname like 'stud%'", - "select groupname from usergroups where uid=:username and groupname like '%sers'", + "select groupname from usergroups where uid=:username and groupname like 'stud%' order by groupname", + "select groupname from usergroups where uid=:username and groupname like '%sers' order by groupname", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password1'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password1'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], diff --git a/tests/src/Auth/Source/PasswordVerifyWrapper.php b/tests/src/Auth/Source/PasswordVerifyWrapper.php index a4e457f..839539b 100644 --- a/tests/src/Auth/Source/PasswordVerifyWrapper.php +++ b/tests/src/Auth/Source/PasswordVerifyWrapper.php @@ -13,7 +13,7 @@ * method in PasswordVerify.php */ -class PasswordVerifyWrapper extends PasswordVerify +class PasswordVerifyWrapper extends PasswordVerify implements WrapperInterface { /** * @param array $info diff --git a/tests/src/Auth/Source/SQL1CompatTest.php b/tests/src/Auth/Source/SQL1CompatTest.php index e9570b7..903a5c9 100644 --- a/tests/src/Auth/Source/SQL1CompatTest.php +++ b/tests/src/Auth/Source/SQL1CompatTest.php @@ -7,5 +7,12 @@ #CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL1Compat::class) class SQL1CompatTest extends SQLTest { - protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\SQL1CompatWrapper'; + /** + * @param array $info + * @param array $config + */ + protected function createWrapper(array $info, array $config): WrapperInterface + { + return new SQL1CompatWrapper($info, $config); + } } diff --git a/tests/src/Auth/Source/SQL1CompatWrapper.php b/tests/src/Auth/Source/SQL1CompatWrapper.php index 79b2a30..7b2e4a3 100644 --- a/tests/src/Auth/Source/SQL1CompatWrapper.php +++ b/tests/src/Auth/Source/SQL1CompatWrapper.php @@ -13,7 +13,7 @@ * method in SQL.php */ -class SQL1CompatWrapper extends SQL1Compat +class SQL1CompatWrapper extends SQL1Compat implements WrapperInterface { /** * @param array $info diff --git a/tests/src/Auth/Source/SQL2Wrapper.php b/tests/src/Auth/Source/SQL2Wrapper.php index 0badfe9..4fbbffb 100644 --- a/tests/src/Auth/Source/SQL2Wrapper.php +++ b/tests/src/Auth/Source/SQL2Wrapper.php @@ -13,7 +13,7 @@ * method in SQL.php */ -class SQL2Wrapper extends SQL2 +class SQL2Wrapper extends SQL2 implements WrapperInterface { /** * @param array $info diff --git a/tests/src/Auth/Source/SQLTest.php b/tests/src/Auth/Source/SQLTest.php index eb8d8df..d947f64 100644 --- a/tests/src/Auth/Source/SQLTest.php +++ b/tests/src/Auth/Source/SQLTest.php @@ -13,9 +13,6 @@ #CoversClass(SimpleSAML\Module\sqlauth\Auth\Source\SQL::class) class SQLTest extends TestCase { - // Subclasses can override this to test other wrapper classes - protected string $wrapperClassName = '\SimpleSAML\Test\Module\sqlauth\Auth\Source\SQLWrapper'; - /** @var array */ private array $info = ['AuthId' => 'testAuthId']; @@ -80,11 +77,21 @@ public static function setUpBeforeClass(): void } + /** + * @param array $info + * @param array $config + */ + protected function createWrapper(array $info, array $config): WrapperInterface + { + return new SQLWrapper($info, $config); + } + + public function testBasicSingleSuccess(): void { // Correct username/password $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); $this->assertEquals($ret, [ @@ -99,7 +106,7 @@ public function testBasicSingleUsernameRegexSuccess(): void // Correct username/password $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; $this->config['username_regex'] = '/^[a-z]+$/'; // Username must be a single lower case word - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); $this->assertCount(2, $ret); $this->assertEquals($ret, [ @@ -115,7 +122,7 @@ public function testBasicSingleUsernameRegexFailedLogin(): void // Correct username/password, but doesn't match the username regex $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; $this->config['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); $this->assertCount(0, $ret); } @@ -127,7 +134,7 @@ public function testBasicSingleUsernameRegexFailedLoginNonExistingUser(): void // Correct username/password, but doesn't match the username regex $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; $this->config['username_regex'] = '/^\d+$/'; // Username must be a non-negative integer - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('henry', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('henry', 'password'); asort($ret); $this->assertCount(0, $ret); } @@ -138,7 +145,7 @@ public function testBasicSingleFailedLogin(): void $this->expectException(\SimpleSAML\Error\Error::class); // Wrong username/password $this->config['query'] = "select givenName, email from users where uid=:username and password=:password"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -149,10 +156,10 @@ public function testJoinSingleSuccess(): void $this->config['query'] = " select u.givenName, u.email, ug.groupname from users u left join usergroups ug on (u.uid=ug.uid) - where u.uid=:username and u.password=:password"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + where u.uid=:username and u.password=:password + order by ug.groupname"; + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -170,7 +177,7 @@ public function testJoinSingleFailedLogin(): void select u.givenName, u.email, ug.groupname from users u left join usergroups ug on (u.uid=ug.uid) where u.uid=:username and u.password=:password"; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -180,11 +187,10 @@ public function testMultiQuerySuccess(): void // Correct username/password $this->config['query'] = [ "select givenName, email from users where uid=:username and password=:password", - "select groupname from usergroups where uid=:username", + "select groupname from usergroups where uid=:username order by groupname", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -202,7 +208,7 @@ public function testMultiQueryFailedLogin(): void "select givenName, email from users where uid=:username and password=:password", "select groupname from usergroups where uid=:username", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('alice', 'wrong'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('alice', 'wrong'); $this->assertCount(0, $ret); } @@ -212,12 +218,11 @@ public function testMultiQuerySubsequentNoRowsSuccess(): void // Correct username/password. Second query returns no rows, third query returns just one row $this->config['query'] = [ "select givenName, email from users where uid=:username and password=:password", - "select groupname from usergroups where uid=:username and groupname like '%nomatch%'", - "select groupname from usergroups where uid=:username and groupname like 'stud%'", + "select groupname from usergroups where uid=:username and groupname like '%nomatch%' order by groupname", + "select groupname from usergroups where uid=:username and groupname like 'stud%' order by groupname", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], @@ -232,12 +237,11 @@ public function testMultiQuerySubsequentAppendSuccess(): void // Correct username/password. Second query returns a row, third query appends one row $this->config['query'] = [ "select givenName, email from users where uid=:username and password=:password", - "select groupname from usergroups where uid=:username and groupname like 'stud%'", - "select groupname from usergroups where uid=:username and groupname like '%sers'", + "select groupname from usergroups where uid=:username and groupname like 'stud%' order by groupname", + "select groupname from usergroups where uid=:username and groupname like '%sers' order by groupname", ]; - $ret = (new $this->wrapperClassName($this->info, $this->config))->callLogin('bob', 'password'); + $ret = $this->createWrapper($this->info, $this->config)->callLogin('bob', 'password'); asort($ret); - asort($ret['groupname']); $this->assertCount(3, $ret); $this->assertEquals($ret, [ 'email' => ['bob@example.com'], diff --git a/tests/src/Auth/Source/SQLWrapper.php b/tests/src/Auth/Source/SQLWrapper.php index 1dd9cd5..5b49e6e 100644 --- a/tests/src/Auth/Source/SQLWrapper.php +++ b/tests/src/Auth/Source/SQLWrapper.php @@ -13,7 +13,7 @@ * method in SQL.php */ -class SQLWrapper extends SQL +class SQLWrapper extends SQL implements WrapperInterface { /** * @param array $info diff --git a/tests/src/Auth/Source/WrapperInterface.php b/tests/src/Auth/Source/WrapperInterface.php new file mode 100644 index 0000000..bb22052 --- /dev/null +++ b/tests/src/Auth/Source/WrapperInterface.php @@ -0,0 +1,20 @@ + $info + * @param array $config + */ + public function __construct(array $info, array $config); + + + /** + * @return array + */ + public function callLogin(string $username, string $password): array; +} From 8761cb75934c64423c8431c32a8a5865037b8e97 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Thu, 23 Oct 2025 11:29:42 +1100 Subject: [PATCH 16/28] Add wrapper interface to bootstrap.php --- tests/bootstrap.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 0bf89df..6d32139 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -7,6 +7,7 @@ // Load our wrapper class to get around login() being declared protected in SQL.php +require_once($projectRoot . '/tests/src/Auth/Source/WrapperInterface.php'); require_once($projectRoot . '/tests/src/Auth/Source/SQLWrapper.php'); require_once($projectRoot . '/tests/src/Auth/Source/SQL2Wrapper.php'); require_once($projectRoot . '/tests/src/Auth/Source/SQL1CompatWrapper.php'); From 6a2919e896064a21918724dbc34f949bf726edc2 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Fri, 24 Oct 2025 09:20:53 +1100 Subject: [PATCH 17/28] Remove require_all() lines in bootstrap.php. Move to using autoload-dev in composer.json --- composer.json | 3 ++- tests/bootstrap.php | 16 ---------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/composer.json b/composer.json index a165227..a49fb9b 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,8 @@ }, "autoload-dev": { "psr-4": { - "SimpleSAML\\Test\\Utils\\": "vendor/simplesamlphp/simplesamlphp/tests/Utils" + "SimpleSAML\\Test\\Utils\\": "vendor/simplesamlphp/simplesamlphp/tests/Utils", + "SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\": "tests/src/Auth/Source/" } }, "require": { diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 6d32139..d3b131a 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -5,22 +5,6 @@ $projectRoot = dirname(__DIR__); require_once($projectRoot . '/vendor/autoload.php'); - -// Load our wrapper class to get around login() being declared protected in SQL.php -require_once($projectRoot . '/tests/src/Auth/Source/WrapperInterface.php'); -require_once($projectRoot . '/tests/src/Auth/Source/SQLWrapper.php'); -require_once($projectRoot . '/tests/src/Auth/Source/SQL2Wrapper.php'); -require_once($projectRoot . '/tests/src/Auth/Source/SQL1CompatWrapper.php'); -require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyWrapper.php'); -require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerify1CompatWrapper.php'); - -// We use inheritance quite extensively in our test cases, so we need to -// make sure all the classes that are subclassed are loaded before we run any tests. -require_once($projectRoot . '/tests/src/Auth/Source/PasswordVerifyTest.php'); -require_once($projectRoot . '/tests/src/Auth/Source/SQLTest.php'); -require_once($projectRoot . '/tests/src/Auth/Source/SQL2SimpleTest.php'); -require_once($projectRoot . '/tests/src/Auth/Source/SQL2SingleAuthTest.php'); - // Symlink module into ssp vendor lib so that templates and urls can resolve correctly $linkPath = $projectRoot . '/vendor/simplesamlphp/simplesamlphp/modules/sqlauth'; if (file_exists($linkPath) === false) { From 1230b28702ef9b6ae23c3913de4ff61d9062471b Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Fri, 24 Oct 2025 09:31:05 +1100 Subject: [PATCH 18/28] Fix scrutinizer issues --- src/Auth/Source/PasswordVerify1Compat.php | 5 +++-- src/Auth/Source/SQL1Compat.php | 5 +++-- src/Auth/Source/SQL2.php | 1 - 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Auth/Source/PasswordVerify1Compat.php b/src/Auth/Source/PasswordVerify1Compat.php index 50c3c2b..d373f43 100644 --- a/src/Auth/Source/PasswordVerify1Compat.php +++ b/src/Auth/Source/PasswordVerify1Compat.php @@ -50,9 +50,10 @@ public function __construct(array $info, array $config) $v2config['auth_queries']['default']['password_verify_hash_column'] = $config['passwordhash_column']; } - if (is_array($config['query']) && count($config['query']) > 1) { + $numQueries = is_array($config['query']) ? count($config['query']) : 0; + if ($numQueries > 1) { $v2config['attr_queries'] = []; - for ($i = 1; $i < count($config['query']); $i++) { + for ($i = 1; $i < $numQueries; $i++) { $v2config['attr_queries']['query' . $i] = [ 'database' => 'default', 'query' => $config['query'][$i], diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php index 36825f3..8c2e185 100644 --- a/src/Auth/Source/SQL1Compat.php +++ b/src/Auth/Source/SQL1Compat.php @@ -44,9 +44,10 @@ public function __construct(array $info, array $config) $v2config['auth_queries']['default']['username_regex'] = $config['username_regex']; } - if (is_array($config['query']) && count($config['query']) > 1) { + $numQueries = is_array($config['query']) ? count($config['query']) : 0; + if ($numQueries > 1) { $v2config['attr_queries'] = []; - for ($i = 1; $i < count($config['query']); $i++) { + for ($i = 1; $i < $numQueries; $i++) { $v2config['attr_queries']['query' . $i] = [ 'database' => 'default', 'query' => $config['query'][$i], diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 34498d0..57e7423 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -574,7 +574,6 @@ protected function login( } // At the end, disconnect from all databases - $db = null; foreach ($this->databases as $dbname => $dbConfig) { if ($dbConfig['_pdo'] !== null) { $this->databases[$dbname]['_pdo'] = null; From 4b38aa5a1215a8692cc48396e6785b4f603e5767 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 27 Oct 2025 12:02:02 +1100 Subject: [PATCH 19/28] Move password_verify_hash_column handling from login into a member function --- src/Auth/Source/SQL2.php | 142 ++++++++++++++++++++++++--------------- 1 file changed, 88 insertions(+), 54 deletions(-) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 57e7423..ce2c190 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -396,6 +396,89 @@ protected function executeQuery(PDO $db, string $query, array $params): array } + /** + * Authenticate using the optional password_verify() support against a hash retrieved from the database. + * + * @param string $queryname Name of the auth query being processed + * @param array $queryConfig Configuration from authsources.php for this auth query + * @param array $data Result data from the database query + * @param string $password Password to verify with password_verify() + * @return bool True if password_verify() password verification succeeded, false otherwise + */ + protected function authenticatePasswordVerifyHash( + string $queryname, + array $queryConfig, + array $data, + string $password, + ): bool { + // If password_verify_hash_column is not set, we are not using password_verify() + if (!array_key_exists('password_verify_hash_column', $queryConfig)) { + Logger::error(sprintf( + 'sqlauth:%s: authenticatePasswordVerifyHash() called but configuration for ' . + '"password_verify_hash_column" not found in query config for query %s.', + $this->authId, + $queryname, + )); + throw new Error\Error('WRONGUSERPASS'); + } elseif (count($data) < 1) { + // No rows returned, password_verify() cannot succeed + return false; + } + + /* This is where we need to run password_verify() if we are using password_verify() to + * authenticate hashed passwords that are only stored in the database. */ + $hashColumn = $queryConfig['password_verify_hash_column']; + if (!array_key_exists($hashColumn, $data[0])) { + Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' did not return expected hash column \'' . $hashColumn . '\''); + throw new Error\Error('WRONGUSERPASS'); + } + + $validPasswordHashFound = false; + $passwordHash = null; + foreach ($data as $row) { + if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) { + Logger::error(sprintf( + 'sqlauth:%s: column `%s` must be in every result tuple.', + $this->authId, + $hashColumn, + )); + throw new Error\Error('WRONGUSERPASS'); + } + if (($passwordHash === null) && (strlen($row[$hashColumn]) > 0)) { + $passwordHash = $row[$hashColumn]; + $validPasswordHashFound = true; + } elseif ($passwordHash != $row[$hashColumn]) { + Logger::error(sprintf( + 'sqlauth:%s: column %s must be THE SAME in every result tuple.', + $this->authId, + $hashColumn, + )); + throw new Error\Error('WRONGUSERPASS'); + } elseif (strlen($row[$hashColumn]) === 0) { + Logger::error(sprintf( + 'sqlauth:%s: column `%s` must contain a valid password hash.', + $this->authId, + $hashColumn, + )); + throw new Error\Error('WRONGUSERPASS'); + } + } + + if ((!$validPasswordHashFound) || (!password_verify($password, $passwordHash))) { + Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' password verification failed'); + /* Authentication with verify_password() failed, however that only means that + * this auth query did not succeed. We should try the next auth query if any. */ + return false; + } + + Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname . + ' password verification using password_verify() succeeded'); + return true; + } + + /** * Attempt to log in using the given username and password. * @@ -449,60 +532,11 @@ protected function login( } // If we got any rows, the authentication succeeded. If not, try the next query. - if (count($data) > 0) { - /* This is where we need to run password_verify() if we are using password_verify() to - * authenticate hashed passwords that are only stored in the database. */ - if (array_key_exists('password_verify_hash_column', $queryConfig)) { - $hashColumn = $queryConfig['password_verify_hash_column']; - if (!array_key_exists($hashColumn, $data[0])) { - Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . - ' did not return expected hash column \'' . $hashColumn . '\''); - throw new Error\Error('WRONGUSERPASS'); - } - - $validPasswordHashFound = false; - $passwordHash = null; - foreach ($data as $row) { - if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) { - Logger::error(sprintf( - 'sqlauth:%s: column `%s` must be in every result tuple.', - $this->authId, - $hashColumn, - )); - throw new Error\Error('WRONGUSERPASS'); - } - if (($passwordHash === null) && (strlen($row[$hashColumn]) > 0)) { - $passwordHash = $row[$hashColumn]; - $validPasswordHashFound = true; - } elseif ($passwordHash != $row[$hashColumn]) { - Logger::error(sprintf( - 'sqlauth:%s: column %s must be THE SAME in every result tuple.', - $this->authId, - $hashColumn, - )); - throw new Error\Error('WRONGUSERPASS'); - } elseif (strlen($row[$hashColumn]) === 0) { - Logger::error(sprintf( - 'sqlauth:%s: column `%s` must contain a valid password hash.', - $this->authId, - $hashColumn, - )); - throw new Error\Error('WRONGUSERPASS'); - } - } - - if ((!$validPasswordHashFound) || (!password_verify($password, $passwordHash))) { - Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . - ' password verification failed'); - /* Authentication with verify_password() failed, however that only means that - * this auth query did not succeed. We should try the next auth query if any. */ - continue; - } - - Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname . - ' password verification using password_verify() succeeded'); - } - + if ( + (count($data) > 0) && + ((array_key_exists('password_verify_hash_column', $queryConfig) === false) || + $this->authenticatePasswordVerifyHash($queryname, $queryConfig, $data, $password)) + ) { Logger::debug('sqlauth:' . $this->authId . ': Auth query ' . $queryname . ' succeeded with ' . count($data) . ' rows'); $queryConfig['_winning_auth_query'] = true; From 0ffef411ad7da11e53b84b736d03b223fa7059fa Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Mon, 3 Nov 2025 16:52:43 +1100 Subject: [PATCH 20/28] Fix core:loginpage_links (and other extra parameters) in SQL1 compat layer --- src/Auth/Source/PasswordVerify1Compat.php | 9 +++++++++ src/Auth/Source/SQL1Compat.php | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/src/Auth/Source/PasswordVerify1Compat.php b/src/Auth/Source/PasswordVerify1Compat.php index d373f43..3df9ead 100644 --- a/src/Auth/Source/PasswordVerify1Compat.php +++ b/src/Auth/Source/PasswordVerify1Compat.php @@ -61,6 +61,15 @@ public function __construct(array $info, array $config) } } + // Copy other config keys that are not specific to SQL1 (eg. core:login_links) + foreach (array_keys($config) as $key) { + if (in_array($key, ['dsn', 'username', 'password', 'query', 'username_regex', 'passwordhashcolumn'])) { + continue; + } + + $v2config[$key] = $config[$key]; + } + parent::__construct($info, $v2config); } } diff --git a/src/Auth/Source/SQL1Compat.php b/src/Auth/Source/SQL1Compat.php index 8c2e185..434eac2 100644 --- a/src/Auth/Source/SQL1Compat.php +++ b/src/Auth/Source/SQL1Compat.php @@ -55,6 +55,15 @@ public function __construct(array $info, array $config) } } + // Copy other config keys that are not specific to SQL1 (eg. core:login_links) + foreach (array_keys($config) as $key) { + if (in_array($key, ['dsn', 'username', 'password', 'query', 'username_regex'])) { + continue; + } + + $v2config[$key] = $config[$key]; + } + parent::__construct($info, $v2config); } } From 3f511dd9052e9fb541e90265b212bfdc873391b2 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 09:11:47 +1100 Subject: [PATCH 21/28] Add test case for database ID being wrong in configuration --- phpstan-baseline-dev.neon | 6 +++ .../src/Auth/Source/SQL2NonExistentDbTest.php | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tests/src/Auth/Source/SQL2NonExistentDbTest.php diff --git a/phpstan-baseline-dev.neon b/phpstan-baseline-dev.neon index b24421b..4375313 100644 --- a/phpstan-baseline-dev.neon +++ b/phpstan-baseline-dev.neon @@ -6,6 +6,12 @@ parameters: count: 1 path: tests/src/Auth/Source/SQL2MultipleAuthTest.php + - + message: '#^Property SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\SQL2NonExistentDbTest\:\:\$config type has no value type specified in iterable type array\.$#' + identifier: missingType.iterableValue + count: 1 + path: tests/src/Auth/Source/SQL2NonExistentDbTest.php + - message: '#^Property SimpleSAML\\Test\\Module\\sqlauth\\Auth\\Source\\SQL2SimpleTest\:\:\$config type has no value type specified in iterable type array\.$#' identifier: missingType.iterableValue diff --git a/tests/src/Auth/Source/SQL2NonExistentDbTest.php b/tests/src/Auth/Source/SQL2NonExistentDbTest.php new file mode 100644 index 0000000..279bce3 --- /dev/null +++ b/tests/src/Auth/Source/SQL2NonExistentDbTest.php @@ -0,0 +1,43 @@ + */ + private array $info = ['AuthId' => 'testAuthId']; + + protected array $config = [ + "databases" => [ + "defaultdb" => [ + "dsn" => 'sqlite:file:defaultdb?mode=memory&cache=shared', + "username" => "notused", + "password" => "notused", + ], + ], + "auth_queries" => [ + "auth_query" => [ + "database" => "wrong-name", // Non-existent database + "query" => "select 1;", + ], + ], + ]; + + public function testNonExistentDatabaseFailure(): void + { + $this->expectException(Exception::class); + (new SQL2Wrapper($this->info, $this->config))->callLogin('bob', 'password'); + $this->fail('Expected exception was not thrown.'); + } +} From 233342253b33b172ce0e6e04ce0c524ab134e057 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 09:16:28 +1100 Subject: [PATCH 22/28] Fix phpcs error --- tests/src/Auth/Source/SQL2NonExistentDbTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/Auth/Source/SQL2NonExistentDbTest.php b/tests/src/Auth/Source/SQL2NonExistentDbTest.php index 279bce3..decd662 100644 --- a/tests/src/Auth/Source/SQL2NonExistentDbTest.php +++ b/tests/src/Auth/Source/SQL2NonExistentDbTest.php @@ -34,6 +34,7 @@ class SQL2NonExistentDbTest extends TestCase ], ]; + public function testNonExistentDatabaseFailure(): void { $this->expectException(Exception::class); From a495d6b1e5bd0d40cc6425df8e24d6562ef8acee Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 17:02:52 +1100 Subject: [PATCH 23/28] Improve readability of authenticatePasswordVerifyHash() --- src/Auth/Source/SQL2.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index ce2c190..9ca0a8c 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -434,7 +434,6 @@ protected function authenticatePasswordVerifyHash( throw new Error\Error('WRONGUSERPASS'); } - $validPasswordHashFound = false; $passwordHash = null; foreach ($data as $row) { if ((!array_key_exists($hashColumn, $row)) || is_null($row[$hashColumn])) { @@ -445,19 +444,19 @@ protected function authenticatePasswordVerifyHash( )); throw new Error\Error('WRONGUSERPASS'); } - if (($passwordHash === null) && (strlen($row[$hashColumn]) > 0)) { - $passwordHash = $row[$hashColumn]; - $validPasswordHashFound = true; - } elseif ($passwordHash != $row[$hashColumn]) { + + if (strlen($row[$hashColumn]) === 0) { Logger::error(sprintf( - 'sqlauth:%s: column %s must be THE SAME in every result tuple.', + 'sqlauth:%s: column `%s` must contain a valid password hash.', $this->authId, $hashColumn, )); throw new Error\Error('WRONGUSERPASS'); - } elseif (strlen($row[$hashColumn]) === 0) { + } elseif ($passwordHash === null) { + $passwordHash = $row[$hashColumn]; + } elseif ($passwordHash != $row[$hashColumn]) { Logger::error(sprintf( - 'sqlauth:%s: column `%s` must contain a valid password hash.', + 'sqlauth:%s: column %s must be THE SAME in every result tuple.', $this->authId, $hashColumn, )); @@ -465,7 +464,7 @@ protected function authenticatePasswordVerifyHash( } } - if ((!$validPasswordHashFound) || (!password_verify($password, $passwordHash))) { + if (($passwordHash == null) || (!password_verify($password, $passwordHash))) { Logger::error('sqlauth:' . $this->authId . ': Auth query ' . $queryname . ' password verification failed'); /* Authentication with verify_password() failed, however that only means that From 6b4e981233d7abb5b14f4cba75fd2192f8acc3b1 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 17:09:07 +1100 Subject: [PATCH 24/28] Drop reference to most recently used database in the cleanup phase --- src/Auth/Source/SQL2.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 9ca0a8c..5b14464 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -607,6 +607,7 @@ protected function login( } // At the end, disconnect from all databases + $db = null; foreach ($this->databases as $dbname => $dbConfig) { if ($dbConfig['_pdo'] !== null) { $this->databases[$dbname]['_pdo'] = null; From 630316dcc1198b20461a2dd46f0e3365d59fe5a4 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 17:41:13 +1100 Subject: [PATCH 25/28] Make error handling for missing databases key more obvious --- src/Auth/Source/SQL2.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 5b14464..9e5e9fc 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -60,7 +60,9 @@ public function __construct(array $info, array $config) parent::__construct($info, $config); // Check databases configuration that all required parameters are present - if (array_key_exists('databases', $config)) { + if (!array_key_exists('databases', $config)) { + throw new Exception('Missing required attribute \'databases\' for authentication source ' . $this->authId); + } else { if (!is_array($config['databases'])) { throw new Exception('Required parameter \'databases\' for authentication source ' . $this->authId . ' was provided and is expected to be an array. Instead it was: ' . @@ -108,8 +110,6 @@ public function __construct(array $info, array $config) 'options' => $dbConfig['options'] ?? [], ]; } - } else { - throw new Exception('Missing required attribute \'databases\' for authentication source ' . $this->authId); } // Check auth_queries configuration that all required parameters are present From 618e0d37e8b69499b25203c72c774133750b43ff Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 17:45:06 +1100 Subject: [PATCH 26/28] Make error handling for missing auth_queries key more obvious --- src/Auth/Source/SQL2.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 9e5e9fc..52cf8b1 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -113,7 +113,12 @@ public function __construct(array $info, array $config) } // Check auth_queries configuration that all required parameters are present - if (array_key_exists('auth_queries', $config)) { + if (!array_key_exists('auth_queries', $config)) { + throw new Exception( + 'Missing required attribute \'auth_queries\' for authentication source ' . + $this->authId, + ); + } else { if (!is_array($config['auth_queries'])) { throw new Exception('Required parameter \'auth_queries\' for authentication source ' . $this->authId . ' was provided and is expected to be an array. Instead it was: ' . @@ -197,11 +202,6 @@ public function __construct(array $info, array $config) $authQueryConfig['password_verify_hash_column']; } } - } else { - 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 From 9b83f78093826192ea0feaa8d8c9d7ea05289ced Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Tue, 11 Nov 2025 18:00:03 +1100 Subject: [PATCH 27/28] Fix $winning_auth_query not being camelCase --- src/Auth/Source/SQL2.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Auth/Source/SQL2.php b/src/Auth/Source/SQL2.php index 52cf8b1..2ab0c32 100644 --- a/src/Auth/Source/SQL2.php +++ b/src/Auth/Source/SQL2.php @@ -498,7 +498,7 @@ protected function login( ): array { $attributes = []; - $winning_auth_query = null; + $winningAuthQuery = null; // Run authentication queries in order until one succeeds. foreach ($this->authQueries as $queryname => &$queryConfig) { @@ -543,7 +543,7 @@ protected function login( if (array_key_exists('extract_userid_from', $queryConfig)) { $queryConfig['_extracted_userid'] = $data[0][$queryConfig['extract_userid_from']]; } - $winning_auth_query = $queryname; + $winningAuthQuery = $queryname; $forbiddenAttributes = []; if (array_key_exists('password_verify_hash_column', $queryConfig)) { @@ -572,22 +572,22 @@ protected function login( Logger::debug( 'sqlauth:' . $this->authId . ': ' . 'Considering attribute query ' . $attrQueryConfig['query'] . - ' for winning auth query ' . $winning_auth_query . + ' for winning auth query ' . $winningAuthQuery . ' 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) + in_array($winningAuthQuery, $attrQueryConfig['only_for_auth'], true) ) { Logger::debug('sqlauth:' . $this->authId . ': Running attribute query ' . $attrQueryConfig['query'] . - ' for winning auth query ' . $winning_auth_query); + ' for winning auth query ' . $winningAuthQuery); $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[$winningAuthQuery]['_extracted_userid'] !== null) ? + ['userid' => $this->authQueries[$winningAuthQuery]['_extracted_userid']] : ['username' => $username]; $data = $this->executeQuery($db, $attrQueryConfig['query'], $params); } catch (PDOException $e) { @@ -602,7 +602,7 @@ protected function login( $this->extractAttributes($attributes, $data, []); } else { Logger::debug('sqlauth:' . $this->authId . ': Skipping attribute query ' . $attrQueryConfig['query'] . - ' because it does not apply to winning auth query ' . $winning_auth_query); + ' because it does not apply to winning auth query ' . $winningAuthQuery); } } From 2d7157b801b65b23182bfd1cf7826752f679d7c8 Mon Sep 17 00:00:00 2001 From: Nathan Robertson Date: Wed, 19 Nov 2025 14:24:29 +1100 Subject: [PATCH 28/28] Upgrade simplesamlphp/composer-module-installer dependency from ~1.5.0 to ~1.6.0 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index a49fb9b..1f54cdb 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,7 @@ "ext-pdo": "*", "simplesamlphp/assert": "~1.9.1", - "simplesamlphp/composer-module-installer": "~1.5.0", + "simplesamlphp/composer-module-installer": "~1.6.0", "simplesamlphp/simplesamlphp": "^2.2" }, "require-dev": {