diff --git a/docs/simplesamlphp-upgrade-notes-2.0.md b/docs/simplesamlphp-upgrade-notes-2.0.md index 979d6d96ede499023d1a2d96bf95ec589ce672c2..b2684c375b473dece98af96f35a36e4f7e0dbd92 100644 --- a/docs/simplesamlphp-upgrade-notes-2.0.md +++ b/docs/simplesamlphp-upgrade-notes-2.0.md @@ -15,18 +15,18 @@ Upgrade notes for SimpleSAMLphp 2.0 to manually switch back the `sharedkey_algorithm`. Note that CBC is vulnerable to the Padding oracle attack. - In compliancy with SAML2INT, AuthnRequests that are signed will have their signature validated unless specifically disabled by setting `validate.authnrequest` to `false`. If unset, or set to true, signatures will be validated and requests not passing validation will be refused. - The following classes have been migrated to non-static: - + lib/SimpleSAMLphp\Utils\Arrays - + lib/SimpleSAMLphp\Utils\Attributes - + lib/SimpleSAMLphp\Utils\Auth - + lib/SimpleSAMLphp\Utils\Config - + lib/SimpleSAMLphp\Utils\Crypto - + lib/SimpleSAMLphp\Utils\EMail - + lib/SimpleSAMLphp\Utils\HTTP - + lib/SimpleSAMLphp\Utils\Net - + lib/SimpleSAMLphp\Utils\Random - + lib/SimpleSAMLphp\Utils\System - + lib/SimpleSAMLphp\Utils\Time - + lib/SimpleSAMLphp\Utils\XML + + \SimpleSAML\Utils\Arrays + + \SimpleSAML\Utils\Attributes + + \SimpleSAML\Utils\Auth + + \SimpleSAML\Utils\Config + + \SimpleSAML\Utils\Crypto + + \SimpleSAML\Utils\EMail + + \SimpleSAML\Utils\HTTP + + \SimpleSAML\Utils\Net + + \SimpleSAML\Utils\Random + + \SimpleSAML\Utils\System + + \SimpleSAML\Utils\Time + + \SimpleSAML\Utils\XML If you use any of these classes in your modules or themes, you will now have to instantiate them so that: @@ -38,3 +38,14 @@ Upgrade notes for SimpleSAMLphp 2.0 // New style $arrayUtils = new \SimpleSAML\Utils\Arrays(); $x = $arrayUtils->arrayize($someVar); + +- Database table schemes have been flattened. Upgrade paths are: + - Generic KVStore: 1.16+ > 2.0 + - Logout store: 1.18+ > 2.0 + +- Data stores have been refactored: + - lib/SimpleSAML/Store.php has been renamed to lib/SimpleSAML/Store/StoreFactory.php and is now solely a Factory-class + - All store implementations now implement \SimpleSAML\Store\StoreInterface: + - lib/SimpleSAML/Store/SQL.php has been renamed to lib/SimpleSAML/Store/SQLStore.php + - lib/SimpleSAML/Store/Memcache.php has been renamed to lib/SimpleSAML/Store/MemcacheStore.php + - lib/SimpleSAML/Store/Redis.php has been renamed to lib/SimpleSAML/Store/RedisStore.php diff --git a/lib/SimpleSAML/SessionHandler.php b/lib/SimpleSAML/SessionHandler.php index 1a234ed4ed4eab0b2473a6deacfddf3c8be0bc6a..c69e8f233a9061a166101f56f3c5b01940c875f2 100644 --- a/lib/SimpleSAML/SessionHandler.php +++ b/lib/SimpleSAML/SessionHandler.php @@ -15,6 +15,8 @@ declare(strict_types=1); namespace SimpleSAML; +use SimpleSAML\Store\StoreFactory; + abstract class SessionHandler { /** @@ -133,7 +135,10 @@ abstract class SessionHandler */ private static function createSessionHandler(): void { - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + $store = StoreFactory::getInstance($storeType); if ($store === false) { self::$sessionHandler = new SessionHandlerPHP(); } else { diff --git a/lib/SimpleSAML/SessionHandlerStore.php b/lib/SimpleSAML/SessionHandlerStore.php index ec7cbfe1a20b6491bc85043709a5f980abcd16f0..f4f89e71068def0cc4206318dfd850ce233394d5 100644 --- a/lib/SimpleSAML/SessionHandlerStore.php +++ b/lib/SimpleSAML/SessionHandlerStore.php @@ -11,23 +11,24 @@ declare(strict_types=1); namespace SimpleSAML; use SimpleSAML\Assert\Assert; +use SimpleSAML\Store\StoreInterface; class SessionHandlerStore extends SessionHandlerCookie { /** * The data store we save the session to. * - * @var \SimpleSAML\Store + * @var \SimpleSAML\Store\StoreInterface */ - private Store $store; + private StoreInterface $store; /** * Initialize the session. * - * @param \SimpleSAML\Store $store The store to use. + * @param \SimpleSAML\Store\StoreInterface $store The store to use. */ - protected function __construct(Store $store) + protected function __construct(StoreInterface $store) { parent::__construct(); diff --git a/lib/SimpleSAML/Store/Memcache.php b/lib/SimpleSAML/Store/MemcacheStore.php similarity index 80% rename from lib/SimpleSAML/Store/Memcache.php rename to lib/SimpleSAML/Store/MemcacheStore.php index 230a3ca752cf7d66e35ccb0ec53f203505638fe9..b67a1f8acd774b21e1c53082d3057a013101aee7 100644 --- a/lib/SimpleSAML/Store/Memcache.php +++ b/lib/SimpleSAML/Store/MemcacheStore.php @@ -6,14 +6,14 @@ namespace SimpleSAML\Store; use SimpleSAML\Assert\Assert; use SimpleSAML\Configuration; -use SimpleSAML\Store; +use SimpleSAML\Memcache; /** * A memcache based data store. * - * @package SimpleSAMLphp + * @package simplesamlphp/simplesamlphp */ -class Memcache extends Store +class MemcacheStore implements StoreInterface { /** * This variable contains the session name prefix. @@ -26,7 +26,7 @@ class Memcache extends Store /** * This function implements the constructor for this class. It loads the Memcache configuration. */ - protected function __construct() + public function __construct() { $config = Configuration::getInstance(); $this->prefix = $config->getString('memcache_store.prefix', 'simpleSAMLphp'); @@ -42,7 +42,7 @@ class Memcache extends Store */ public function get(string $type, string $key) { - return \SimpleSAML\Memcache::get($this->prefix . '.' . $type . '.' . $key); + return Memcache::get($this->prefix . '.' . $type . '.' . $key); } @@ -62,7 +62,7 @@ class Memcache extends Store $expire = 0; } - \SimpleSAML\Memcache::set($this->prefix . '.' . $type . '.' . $key, $value, $expire); + Memcache::set($this->prefix . '.' . $type . '.' . $key, $value, $expire); } @@ -74,6 +74,6 @@ class Memcache extends Store */ public function delete(string $type, string $key): void { - \SimpleSAML\Memcache::delete($this->prefix . '.' . $type . '.' . $key); + Memcache::delete($this->prefix . '.' . $type . '.' . $key); } } diff --git a/lib/SimpleSAML/Store/Redis.php b/lib/SimpleSAML/Store/RedisStore.php similarity index 94% rename from lib/SimpleSAML/Store/Redis.php rename to lib/SimpleSAML/Store/RedisStore.php index 6a6aec8b404640066972717b7b02009b01c5ffdb..20dfd28bda72ec60e9ef692472ddf4a4b557c113 100644 --- a/lib/SimpleSAML/Store/Redis.php +++ b/lib/SimpleSAML/Store/RedisStore.php @@ -8,14 +8,13 @@ use Predis\Client; use SimpleSAML\Assert\Assert; use SimpleSAML\Configuration; use SimpleSAML\Error; -use SimpleSAML\Store; /** * A data store using Redis to keep the data. * - * @package SimpleSAMLphp + * @package simplesamlphp/simplesamlphp */ -class Redis extends Store +class RedisStore implements StoreInterface { /** @var \Predis\Client */ public Client $redis; @@ -64,9 +63,7 @@ class Redis extends Store */ public function __destruct() { - if (method_exists($this->redis, 'disconnect')) { - $this->redis->disconnect(); - } + $this->redis->disconnect(); } diff --git a/lib/SimpleSAML/Store/SQL.php b/lib/SimpleSAML/Store/SQLStore.php similarity index 74% rename from lib/SimpleSAML/Store/SQL.php rename to lib/SimpleSAML/Store/SQLStore.php index e5bcd6fb51a4c9096374043dc34bff24044486b2..640bbb5dafcb56c2041cb7b27cc696b76bd32e72 100644 --- a/lib/SimpleSAML/Store/SQL.php +++ b/lib/SimpleSAML/Store/SQLStore.php @@ -4,19 +4,19 @@ declare(strict_types=1); namespace SimpleSAML\Store; +use Exception; use PDO; use PDOException; use SimpleSAML\Assert\Assert; use SimpleSAML\Configuration; use SimpleSAML\Logger; -use SimpleSAML\Store; /** * A data store using a RDBMS to keep the data. * - * @package SimpleSAMLphp + * @package simplesamlphp/simplesamlphp */ -class SQL extends Store +class SQLStore implements StoreInterface { /** * The PDO object for our database. @@ -62,7 +62,7 @@ class SQL extends Store try { $this->pdo = new PDO($dsn, $username, $password, $options); } catch (PDOException $e) { - throw new \Exception("Database error: " . $e->getMessage()); + throw new Exception("Database error: " . $e->getMessage()); } $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); @@ -95,7 +95,7 @@ class SQL extends Store } while (($row = $fetchTableVersion->fetch(PDO::FETCH_ASSOC)) !== false) { - $this->tableVersions[$row['_name']] = (int) $row['_version']; + $this->tableVersions[$row['_name']] = intval($row['_version']); } } @@ -105,74 +105,37 @@ class SQL extends Store */ private function initKVTable(): void { - $current_version = $this->getTableVersion('kvstore'); + $tableVer = $this->getTableVersion('kvstore'); + if ($tableVer === 2) { + return; + } elseif ($tableVer < 2 && $tableVer > 0) { + throw new Exception('No upgrade path available. Please migrate to the latest 1.16+ version of SimpleSAMLphp first before upgrading to 2.x.'); + } $text_t = 'TEXT'; - $time_field = 'TIMESTAMP'; if ($this->driver === 'mysql') { // TEXT data type has size constraints that can be hit at some point, so we use LONGTEXT instead $text_t = 'LONGTEXT'; } + + $time_field = 'TIMESTAMP'; if ($this->driver === 'sqlsrv') { // TIMESTAMP will not work for MSSQL. TIMESTAMP is automatically generated and cannot be inserted // so we use DATETIME instead $time_field = 'DATETIME'; } - /** - * Queries for updates, grouped by version. - * New updates can be added as a new array in this array - */ - $table_updates = [ - [ - 'CREATE TABLE ' . $this->prefix . - '_kvstore (_type VARCHAR(30) NOT NULL, _key VARCHAR(50) NOT NULL, _value ' . $text_t . - ' NOT NULL, _expire ' . $time_field . ', PRIMARY KEY (_key, _type))', - $this->driver === 'sqlite' || $this->driver === 'sqlsrv' || $this->driver === 'pgsql' ? - 'CREATE INDEX ' . $this->prefix . '_kvstore_expire ON ' . $this->prefix . '_kvstore (_expire)' : - 'ALTER TABLE ' . $this->prefix . '_kvstore ADD INDEX ' . $this->prefix . '_kvstore_expire (_expire)' - ], - /** - * This upgrade removes the default NOT NULL constraint on the _expire field in MySQL. - * Because SQLite does not support field alterations, the approach is to: - * Create a new table without the NOT NULL constraint - * Copy the current data to the new table - * Drop the old table - * Rename the new table correctly - * Read the index - */ - [ - 'CREATE TABLE ' . $this->prefix . - '_kvstore_new (_type VARCHAR(30) NOT NULL, _key VARCHAR(50) NOT NULL, _value ' . $text_t . - ' NOT NULL, _expire ' . $time_field . ' NULL, PRIMARY KEY (_key, _type))', - 'INSERT INTO ' . $this->prefix . '_kvstore_new SELECT * FROM ' . $this->prefix . '_kvstore', - 'DROP TABLE ' . $this->prefix . '_kvstore', - // FOR MSSQL use EXEC sp_rename to rename a table (RENAME won't work) - $this->driver === 'sqlsrv' ? - 'EXEC sp_rename ' . $this->prefix . '_kvstore_new, ' . $this->prefix . '_kvstore' : - 'ALTER TABLE ' . $this->prefix . '_kvstore_new RENAME TO ' . $this->prefix . '_kvstore', - $this->driver === 'sqlite' || $this->driver === 'sqlsrv' || $this->driver === 'pgsql' ? - 'CREATE INDEX ' . $this->prefix . '_kvstore_expire ON ' . $this->prefix . '_kvstore (_expire)' : - 'ALTER TABLE ' . $this->prefix . '_kvstore ADD INDEX ' . $this->prefix . '_kvstore_expire (_expire)' - ] - ]; - - $latest_version = count($table_updates); - - if ($current_version == $latest_version) { - return; - } - - // Only run queries for after the current version - $updates_to_run = array_slice($table_updates, $current_version); + $query = 'CREATE TABLE ' . $this->prefix . + '_kvstore (_type VARCHAR(30) NOT NULL, _key VARCHAR(50) NOT NULL, _value ' . $text_t . + ' NOT NULL, _expire ' . $time_field . ' NULL, PRIMARY KEY (_key, _type))'; + $this->pdo->exec($query); - foreach ($updates_to_run as $version_updates) { - foreach ($version_updates as $query) { - $this->pdo->exec($query); - } - } + $query = $this->driver === 'sqlite' || $this->driver === 'sqlsrv' || $this->driver === 'pgsql' ? + 'CREATE INDEX ' . $this->prefix . '_kvstore_expire ON ' . $this->prefix . '_kvstore (_expire)' : + 'ALTER TABLE ' . $this->prefix . '_kvstore ADD INDEX ' . $this->prefix . '_kvstore_expire (_expire)'; + $this->pdo->exec($query); - $this->setTableVersion('kvstore', $latest_version); + $this->setTableVersion('kvstore', 2); } diff --git a/lib/SimpleSAML/Store.php b/lib/SimpleSAML/Store/StoreFactory.php similarity index 55% rename from lib/SimpleSAML/Store.php rename to lib/SimpleSAML/Store/StoreFactory.php index 12e856f03dfd70f0781686fbc57d35177fef5b26..9f3c93f49b3cb722037deb9dbe639928c52571ef 100644 --- a/lib/SimpleSAML/Store.php +++ b/lib/SimpleSAML/Store/StoreFactory.php @@ -2,24 +2,27 @@ declare(strict_types=1); -namespace SimpleSAML; +namespace SimpleSAML\Store; use Exception; +use SimpleSAML\Configuration; use SimpleSAML\Error; +use SimpleSAML\Module; +use SimpleSAML\Utils; /** * Base class for data stores. * - * @package SimpleSAMLphp + * @package simplesamlphp/simplesamlphp */ -abstract class Store implements Utils\ClearableState +abstract class StoreFactory implements Utils\ClearableState { /** * Our singleton instance. * * This is false if the data store isn't enabled, and null if we haven't attempted to initialize it. * - * @var \SimpleSAML\Store|false|null + * @var \SimpleSAML\Store\StoreInterface|false|null */ private static $instance = null; @@ -27,38 +30,37 @@ abstract class Store implements Utils\ClearableState /** * Retrieve our singleton instance. * - * @return \SimpleSAML\Store|false The data store, or false if it isn't enabled. + * @param string $storeType The type of store we need to instantiate + * @return \SimpleSAML\Store\StoreInterface|false The data store, or false if it isn't enabled. * * @throws \SimpleSAML\Error\CriticalConfigurationError */ - public static function getInstance() + public static function getInstance(string $storeType) { if (self::$instance !== null) { return self::$instance; } - $config = Configuration::getInstance(); - $storeType = $config->getString('store.type', 'phpsession'); - switch ($storeType) { case 'phpsession': // we cannot support advanced features with the PHP session store self::$instance = false; break; case 'memcache': - self::$instance = new Store\Memcache(); + self::$instance = new MemcacheStore(); break; case 'sql': - self::$instance = new Store\SQL(); + self::$instance = new SQLStore(); break; case 'redis': - self::$instance = new Store\Redis(); + self::$instance = new RedisStore(); break; default: // datastore from module try { - $className = Module::resolveClass($storeType, 'Store', '\SimpleSAML\Store'); + $className = Module::resolveClass($storeType, 'StoreInterface'); } catch (Exception $e) { + $config = Configuration::getInstance(); $c = $config->toArray(); $c['store.type'] = 'phpsession'; throw new Error\CriticalConfigurationError( @@ -67,7 +69,7 @@ abstract class Store implements Utils\ClearableState $c ); } - /** @var \SimpleSAML\Store|false */ + /** @var \SimpleSAML\Store\StoreInterface|false */ self::$instance = new $className(); } @@ -75,37 +77,6 @@ abstract class Store implements Utils\ClearableState } - /** - * Retrieve a value from the data store. - * - * @param string $type The data type. - * @param string $key The key. - * - * @return mixed|null The value. - */ - abstract public function get(string $type, string $key); - - - /** - * Save a value to the data store. - * - * @param string $type The data type. - * @param string $key The key. - * @param mixed $value The value. - * @param int|null $expire The expiration time (unix timestamp), or null if it never expires. - */ - abstract public function set(string $type, string $key, $value, ?int $expire = null): void; - - - /** - * Delete a value from the data store. - * - * @param string $type The data type. - * @param string $key The key. - */ - abstract public function delete(string $type, string $key): void; - - /** * Clear any SSP specific state, such as SSP environmental variables or cached internals. */ diff --git a/lib/SimpleSAML/Store/StoreInterface.php b/lib/SimpleSAML/Store/StoreInterface.php new file mode 100644 index 0000000000000000000000000000000000000000..7111eb341cbda79e42874fe08370064c2dffb792 --- /dev/null +++ b/lib/SimpleSAML/Store/StoreInterface.php @@ -0,0 +1,43 @@ +<?php + +declare(strict_types=1); + +namespace SimpleSAML\Store; + +/** + * An interface describing data stores. + * + * @package simplesamlphp/simplesamlphp + */ +interface StoreInterface +{ + /** + * Retrieve a value from the data store. + * + * @param string $type The data type. + * @param string $key The key. + * + * @return mixed|null The value. + */ + public function get(string $type, string $key); + + + /** + * Save a value to the data store. + * + * @param string $type The data type. + * @param string $key The key. + * @param mixed $value The value. + * @param int|null $expire The expiration time (unix timestamp), or null if it never expires. + */ + public function set(string $type, string $key, $value, ?int $expire = null): void; + + + /** + * Delete a value from the data store. + * + * @param string $type The data type. + * @param string $key The key. + */ + public function delete(string $type, string $key): void; +} diff --git a/modules/saml/lib/Auth/Source/SP.php b/modules/saml/lib/Auth/Source/SP.php index 1a6f8db57369e11c51f3f82b5b31fb8136471f99..93e40e92298dac4b7cf61961321c58bcd75e803f 100644 --- a/modules/saml/lib/Auth/Source/SP.php +++ b/modules/saml/lib/Auth/Source/SP.php @@ -18,6 +18,7 @@ use SimpleSAML\Metadata\MetaDataStorageHandler; use SimpleSAML\Module; use SimpleSAML\Session; use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; use SimpleSAML\Utils; class SP extends \SimpleSAML\Auth\Source @@ -385,7 +386,10 @@ class SP extends \SimpleSAML\Auth\Source */ private function getSLOEndpoints(): array { - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + $store = StoreFactory::getInstance($storeType); $bindings = $this->metadata->getArray( 'SingleLogoutServiceBinding', [ @@ -398,7 +402,7 @@ class SP extends \SimpleSAML\Auth\Source $endpoints = []; foreach ($bindings as $binding) { - if ($binding == Constants::BINDING_SOAP && !($store instanceof Store\SQL)) { + if ($binding == Constants::BINDING_SOAP && !($store instanceof Store\SQLStore)) { // we cannot properly support SOAP logout continue; } diff --git a/modules/saml/lib/IdP/SQLNameID.php b/modules/saml/lib/IdP/SQLNameID.php index 45623212a50115cd13011b476c71bdc2aefd775f..7a43e101698a44428b4c999eaaca2b87841eca71 100644 --- a/modules/saml/lib/IdP/SQLNameID.php +++ b/modules/saml/lib/IdP/SQLNameID.php @@ -4,13 +4,16 @@ declare(strict_types=1); namespace SimpleSAML\Module\saml\IdP; +use Exception; use PDO; use PDOStatement; use SimpleSAML\Assert\Assert; use SimpleSAML\Error; use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; use SimpleSAML\Database; use SimpleSAML\Configuration; +use SimpleSAML\Logger; /** * Helper class for working with persistent NameIDs stored in SQL datastore. @@ -89,8 +92,8 @@ class SQLNameID if ($store === null) { try { self::createTable($table, $config); - } catch (\Exception $e) { - \SimpleSAML\Logger::debug('SQL persistent NameID table already exists.'); + } catch (Exception $e) { + Logger::debug('SQL persistent NameID table already exists.'); } } elseif ($store->getTableVersion('saml_PersistentNameID') !== self::TABLE_VERSION) { self::createTable($table); @@ -151,16 +154,20 @@ class SQLNameID /** * Retrieve the SQL datastore. * - * @return \SimpleSAML\Store\SQL SQL datastore. + * @return \SimpleSAML\Store\SQLStore SQL datastore. */ - private static function getStore(): Store\SQL + private static function getStore(): Store\SQLStore { - $store = Store::getInstance(); - if (!($store instanceof Store\SQL)) { - throw new Error\Exception( - 'SQL NameID store requires SimpleSAMLphp to be configured with a SQL datastore.' - ); - } + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + $store = StoreFactory::getInstance($storeType); + Assert::isInstanceOf( + $store, + Store\SQLStore::class, + 'SQL NameID store requires SimpleSAMLphp to be configured with a SQL datastore.', + Error\Exception::class + ); return $store; } diff --git a/modules/saml/lib/SP/LogoutStore.php b/modules/saml/lib/SP/LogoutStore.php index 13922d1309358b1c9cf76ee000d43cb078b8cf40..e6205ce6e7c35c788fc76c8ad3ab0be3059f882b 100644 --- a/modules/saml/lib/SP/LogoutStore.php +++ b/modules/saml/lib/SP/LogoutStore.php @@ -4,12 +4,16 @@ declare(strict_types=1); namespace SimpleSAML\Module\saml\SP; +use Exception; use PDO; use SAML2\XML\saml\NameID; use SimpleSAML\Assert\Assert; +use SimpleSAML\Configuration; use SimpleSAML\Logger; use SimpleSAML\Session; use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; +use SimpleSAML\Store\StoreInterface; use SimpleSAML\Utils; /** @@ -23,163 +27,15 @@ class LogoutStore /** * Create logout table in SQL, if it is missing. * - * @param \SimpleSAML\Store\SQL $store The datastore. + * @param \SimpleSAML\Store\SQLStore $store The datastore. */ - private static function createLogoutTable(Store\SQL $store): void + private static function createLogoutTable(Store\SQLStore $store): void { $tableVer = $store->getTableVersion('saml_LogoutStore'); if ($tableVer === 4) { return; - } elseif ($tableVer === 3) { - /** - * Table version 4 fixes the column type for the _expire column. - * We now use DATETIME instead of TIMESTAMP to support MSSQL. - */ - switch ($store->driver) { - case 'pgsql': - // This does not affect the NOT NULL constraint - $update = [ - 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore ALTER COLUMN _expire TYPE TIMESTAMP' - ]; - break; - case 'sqlsrv': - $update = [ - 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore ALTER COLUMN _expire DATETIME NOT NULL' - ]; - break; - case 'sqlite': - /** - * Because SQLite does not support field alterations, the approach is to: - * Create a new table without the proper column size - * Copy the current data to the new table - * Drop the old table - * Rename the new table correctly - * Read the index - */ - $update = [ - 'CREATE TABLE ' . $store->prefix . '_saml_LogoutStore_new (' . - '_authSource VARCHAR(255) NOT NULL, _nameId VARCHAR(40) NOT NULL' . - ', _sessionIndex VARCHAR(50) NOT NULL, _expire DATETIME NOT NULL,' . - '_sessionId VARCHAR(50) NOT NULL, UNIQUE (_authSource, _nameID, _sessionIndex))', - 'INSERT INTO ' . $store->prefix . '_saml_LogoutStore_new SELECT * FROM ' . - $store->prefix . '_saml_LogoutStore', - 'DROP TABLE ' . $store->prefix . '_saml_LogoutStore', - 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore_new RENAME TO ' . - $store->prefix . '_saml_LogoutStore', - 'CREATE INDEX ' . $store->prefix . '_saml_LogoutStore_expire ON ' . - $store->prefix . '_saml_LogoutStore (_expire)', - 'CREATE INDEX ' . $store->prefix . '_saml_LogoutStore_nameId ON ' . - $store->prefix . '_saml_LogoutStore (_authSource, _nameId)' - ]; - break; - default: - $update = [ - 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore MODIFY _expire DATETIME NOT NULL' - ]; - break; - } - - try { - foreach ($update as $query) { - $store->pdo->exec($query); - } - } catch (\Exception $e) { - Logger::warning('Database error: ' . var_export($store->pdo->errorInfo(), true)); - return; - } - $store->setTableVersion('saml_LogoutStore', 4); - return; - } elseif ($tableVer === 2) { - /** - * TableVersion 3 fixes the indexes that were set to 255 in version 2; - * they cannot be larger than 191 on MySQL - */ - - if ($store->driver === 'mysql') { - // Drop old indexes - $query = 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore DROP INDEX ' . - $store->prefix . '_saml_LogoutStore_nameId'; - $store->pdo->exec($query); - $query = 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore DROP INDEX _authSource'; - $store->pdo->exec($query); - - // Create new indexes - $query = 'CREATE INDEX ' . $store->prefix . '_saml_LogoutStore_nameId ON '; - $query .= $store->prefix . '_saml_LogoutStore (_authSource(191), _nameId)'; - $store->pdo->exec($query); - - $query = 'ALTER TABLE ' . $store->prefix . - '_saml_LogoutStore ADD UNIQUE KEY (_authSource(191), _nameID, _sessionIndex)'; - $store->pdo->exec($query); - } - - $store->setTableVersion('saml_LogoutStore', 3); - return; - } elseif ($tableVer === 1) { - // TableVersion 2 increased the column size to 255 (191 for mysql) which is the maximum length of a FQDN - switch ($store->driver) { - case 'pgsql': - // This does not affect the NOT NULL constraint - $update = [ - 'ALTER TABLE ' . $store->prefix . - '_saml_LogoutStore ALTER COLUMN _authSource TYPE VARCHAR(255)']; - break; - case 'sqlsrv': - $update = [ - 'ALTER TABLE ' . $store->prefix . - '_saml_LogoutStore ALTER COLUMN _authSource VARCHAR(255) NOT NULL' - ]; - break; - case 'sqlite': - /** - * Because SQLite does not support field alterations, the approach is to: - * Create a new table without the proper column size - * Copy the current data to the new table - * Drop the old table - * Rename the new table correctly - * Read the index - */ - $update = [ - 'CREATE TABLE ' . $store->prefix . - '_saml_LogoutStore_new (_authSource VARCHAR(255) NOT NULL,' . - '_nameId VARCHAR(40) NOT NULL, _sessionIndex VARCHAR(50) NOT NULL, ' . - '_expire TIMESTAMP NOT NULL, _sessionId VARCHAR(50) NOT NULL, UNIQUE ' . - '(_authSource, _nameID, _sessionIndex))', - 'INSERT INTO ' . $store->prefix . '_saml_LogoutStore_new SELECT * FROM ' . - $store->prefix . '_saml_LogoutStore', - 'DROP TABLE ' . $store->prefix . '_saml_LogoutStore', - 'ALTER TABLE ' . $store->prefix . '_saml_LogoutStore_new RENAME TO ' . - $store->prefix . '_saml_LogoutStore', - 'CREATE INDEX ' . $store->prefix . '_saml_LogoutStore_expire ON ' . - $store->prefix . '_saml_LogoutStore (_expire)', - 'CREATE INDEX ' . $store->prefix . '_saml_LogoutStore_nameId ON ' . - $store->prefix . '_saml_LogoutStore (_authSource, _nameId)' - ]; - break; - case 'mysql': - $update = [ - 'ALTER TABLE ' . $store->prefix . - '_saml_LogoutStore MODIFY _authSource VARCHAR(191) NOT NULL' - ]; - break; - default: - $update = [ - 'ALTER TABLE ' . $store->prefix . - '_saml_LogoutStore MODIFY _authSource VARCHAR(255) NOT NULL' - ]; - break; - } - - try { - foreach ($update as $query) { - $store->pdo->exec($query); - } - } catch (\Exception $e) { - Logger::warning('Database error: ' . var_export($store->pdo->errorInfo(), true)); - return; - } - $store->setTableVersion('saml_LogoutStore', 2); - return; + } elseif ($tableVer < 4 && $tableVer > 0) { + throw new Exception('No upgrade path available. Please migrate to the latest 1.18+ version of SimpleSAMLphp first before upgrading to 2.x.'); } $query = 'CREATE TABLE ' . $store->prefix . '_saml_LogoutStore ( @@ -208,9 +64,9 @@ class LogoutStore /** * Clean the logout table of expired entries. * - * @param \SimpleSAML\Store\SQL $store The datastore. + * @param \SimpleSAML\Store\SQLStore $store The datastore. */ - private static function cleanLogoutStore(Store\SQL $store): void + private static function cleanLogoutStore(Store\SQLStore $store): void { Logger::debug('saml.LogoutStore: Cleaning logout store.'); @@ -225,7 +81,7 @@ class LogoutStore /** * Register a session in the SQL datastore. * - * @param \SimpleSAML\Store\SQL $store The datastore. + * @param \SimpleSAML\Store\SQLStore $store The datastore. * @param string $authId The authsource ID. * @param string $nameId The hash of the users NameID. * @param string $sessionIndex The SessionIndex of the user. @@ -233,7 +89,7 @@ class LogoutStore * @param string $sessionId */ private static function addSessionSQL( - Store\SQL $store, + Store\SQLStore $store, string $authId, string $nameId, string $sessionIndex, @@ -264,12 +120,12 @@ class LogoutStore /** * Retrieve sessions from the SQL datastore. * - * @param \SimpleSAML\Store\SQL $store The datastore. + * @param \SimpleSAML\Store\SQLStore $store The datastore. * @param string $authId The authsource ID. * @param string $nameId The hash of the users NameID. * @return array Associative array of SessionIndex => SessionId. */ - private static function getSessionsSQL(Store\SQL $store, string $authId, string $nameId): array + private static function getSessionsSQL(Store\SQLStore $store, string $authId, string $nameId): array { self::createLogoutTable($store); @@ -298,13 +154,13 @@ class LogoutStore /** * Retrieve all session IDs from a key-value store. * - * @param \SimpleSAML\Store $store The datastore. + * @param \SimpleSAML\Store\StoreInterface $store The datastore. * @param string $nameId The hash of the users NameID. * @param array $sessionIndexes The session indexes. * @return array Associative array of SessionIndex => SessionId. */ private static function getSessionsStore( - Store $store, + StoreInterface $store, string $nameId, array $sessionIndexes ): array { @@ -354,7 +210,10 @@ class LogoutStore $sessionIndex = $randomUtils->generateID(); } - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + $store = StoreFactory::getInstance($storeType); if ($store === false) { // We don't have a datastore. return; @@ -372,7 +231,7 @@ class LogoutStore /** @var string $sessionId */ $sessionId = $session->getSessionId(); - if ($store instanceof Store\SQL) { + if ($store instanceof Store\SQLStore) { self::addSessionSQL($store, $authId, $strNameId, $sessionIndex, $expire, $sessionId); } else { $store->set('saml.LogoutStore', $strNameId . ':' . $sessionIndex, $sessionId, $expire); @@ -390,7 +249,10 @@ class LogoutStore */ public static function logoutSessions(string $authId, NameID $nameId, array $sessionIndexes) { - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + $store = StoreFactory::getInstance($storeType); if ($store === false) { // We don't have a datastore return false; @@ -411,7 +273,7 @@ class LogoutStore // Remove reference unset($sessionIndex); - if ($store instanceof Store\SQL) { + if ($store instanceof Store\SQLStore) { $sessions = self::getSessionsSQL($store, $authId, $strNameId); } else { if (empty($sessionIndexes)) { diff --git a/modules/saml/www/sp/metadata.php b/modules/saml/www/sp/metadata.php index 603209c53301c7b4ed03a3a40c3e211ffcee43fc..4a9fabcb55afb8b5ee72b0d11ec8b0816456818c 100644 --- a/modules/saml/www/sp/metadata.php +++ b/modules/saml/www/sp/metadata.php @@ -7,7 +7,7 @@ use SimpleSAML\Error; use SimpleSAML\Locale\Translate; use SimpleSAML\Metadata; use SimpleSAML\Module; -use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; use SimpleSAML\Utils; use SimpleSAML\XHTML\Template; use Symfony\Component\VarExporter\VarExporter; @@ -37,7 +37,9 @@ if (!($source instanceof Module\saml\Auth\Source\SP)) { $entityId = $source->getEntityId(); $spconfig = $source->getMetadata(); $metaArray20 = $source->getHostedMetadata(); -$store = Store::getInstance(); + +$storeType = $config->getString('store.type', 'phpsession'); +$store = StoreFactory::getInstance($storeType); $metaBuilder = new Metadata\SAMLBuilder($entityId); $metaBuilder->addMetadataSP20($metaArray20, $source->getSupportedProtocols()); diff --git a/modules/saml/www/sp/saml2-acs.php b/modules/saml/www/sp/saml2-acs.php index 585705c6ef64d9acf93e37518a52060347ef747f..23aad339125a78136261b8a1ea446cdfa3fe9fa4 100644 --- a/modules/saml/www/sp/saml2-acs.php +++ b/modules/saml/www/sp/saml2-acs.php @@ -10,11 +10,12 @@ use SAML2\HTTPArtifact; use SAML2\Response; use SimpleSAML\Assert\Assert; use SimpleSAML\Auth; +use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\Module; use SimpleSAML\Logger; use SimpleSAML\Session; -use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; use SimpleSAML\Utils; if (!array_key_exists('PATH_INFO', $_SERVER)) { @@ -159,7 +160,10 @@ $foundAuthnStatement = false; foreach ($assertions as $assertion) { // check for duplicate assertion (replay attack) - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + $store = StoreFactory::getInstance($storeType); if ($store !== false) { $aID = $assertion->getId(); if ($store->get('saml.AssertionReceived', $aID) !== null) { diff --git a/tests/Utils/StateClearer.php b/tests/Utils/StateClearer.php index 0a9147168c1fd88453fc4b58c930c2c8bb207c0f..569c8f90eea3b514c3b8194276d73b7e8c471e0f 100644 --- a/tests/Utils/StateClearer.php +++ b/tests/Utils/StateClearer.php @@ -22,7 +22,7 @@ class StateClearer private array $clearableState = [ 'SimpleSAML\Configuration', 'SimpleSAML\Metadata\MetaDataStorageHandler', - 'SimpleSAML\Store', + 'SimpleSAML\Store\StoreFactory', 'SimpleSAML\Session' ]; diff --git a/tests/lib/SimpleSAML/Store/RedisTest.php b/tests/lib/SimpleSAML/Store/RedisStoreTest.php similarity index 62% rename from tests/lib/SimpleSAML/Store/RedisTest.php rename to tests/lib/SimpleSAML/Store/RedisStoreTest.php index 9625027c6a11ed8e6bcdf9e37265768a2b38a25f..7bc70076f2afd7c4a50b66018ad5185ffe465600 100644 --- a/tests/lib/SimpleSAML/Store/RedisTest.php +++ b/tests/lib/SimpleSAML/Store/RedisStoreTest.php @@ -7,9 +7,9 @@ namespace SimpleSAML\Test\Store; use PHPUnit\Framework\TestCase; use PHPUnit\Framework\MockObject\MockObject; use Predis\Client; -use ReflectionClass; use SimpleSAML\Configuration; use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; /** * Tests for the Redis store. @@ -17,16 +17,16 @@ use SimpleSAML\Store; * For the full copyright and license information, please view the LICENSE file that was distributed with this source * code. * - * @covers \SimpleSAML\Store\Redis + * @covers \SimpleSAML\Store\RedisStore * @package simplesamlphp/simplesamlphp */ -class RedisTest extends TestCase +class RedisStoreTest extends TestCase { /** @var \PHPUnit\Framework\MockObject\MockObject */ protected MockObject $mocked_redis; - /** @var \SimpleSAML\Store\Redis */ - protected Store\Redis $redis; + /** @var \SimpleSAML\Store\RedisStore */ + protected Store\RedisStore $store; /** @var array */ protected array $config; @@ -39,7 +39,7 @@ class RedisTest extends TestCase $this->config = []; $this->mocked_redis = $this->getMockBuilder(Client::class) - ->setMethods(['get', 'set', 'setex', 'del', 'disconnect']) + ->setMethods(['get', 'set', 'setex', 'del', 'disconnect', '__destruct']) ->disableOriginalConstructor() ->getMock(); @@ -55,15 +55,7 @@ class RedisTest extends TestCase $this->mocked_redis->method('del') ->will($this->returnCallback([$this, 'delMocked'])); - $nop = /** @return void */ function () { - return; - }; - - $this->mocked_redis->method('disconnect') - ->will($this->returnCallback($nop)); - - /** @var \Predis\Client $this->mocked_redis */ - $this->redis = new Store\Redis($this->mocked_redis); + $this->store = new Store\RedisStore($this->mocked_redis); } @@ -118,13 +110,7 @@ class RedisTest extends TestCase 'store.redis.prefix' => 'phpunit_', ], '[ARRAY]', 'simplesaml'); - /** @var \SimpleSAML\Store\Redis $store */ - $store = Store::getInstance(); - - $this->assertInstanceOf(Store\Redis::class, $store); - - $this->clearInstance($config, Configuration::class); - $this->clearInstance($store, Store::class); + $this->assertInstanceOf(Store\RedisStore::class, $this->store); } @@ -139,13 +125,7 @@ class RedisTest extends TestCase 'store.redis.password' => 'password', ], '[ARRAY]', 'simplesaml'); - /** @var \SimpleSAML\Store\Redis $store */ - $store = Store::getInstance(); - - $this->assertInstanceOf(Store\Redis::class, $store); - - $this->clearInstance($config, Configuration::class); - $this->clearInstance($store, Store::class); + $this->assertInstanceOf(Store\RedisStore::class, $this->store); } @@ -156,8 +136,8 @@ class RedisTest extends TestCase { $value = 'TEST'; - $this->redis->set('test', 'key', $value); - $res = $this->redis->get('test', 'key'); + $this->store->set('test', 'key', $value); + $res = $this->store->get('test', 'key'); $expected = $value; $this->assertEquals($expected, $res); @@ -171,8 +151,8 @@ class RedisTest extends TestCase { $value = 'TEST'; - $this->redis->set('test', 'key', $value, $expire = 80808080); - $res = $this->redis->get('test', 'key'); + $this->store->set('test', 'key', $value, $expire = 80808080); + $res = $this->store->get('test', 'key'); $expected = $value; $this->assertEquals($expected, $res); @@ -184,7 +164,7 @@ class RedisTest extends TestCase */ public function testGetEmptyData(): void { - $res = $this->redis->get('test', 'key'); + $res = $this->store->get('test', 'key'); $this->assertNull($res); } @@ -198,9 +178,9 @@ class RedisTest extends TestCase $value1 = 'TEST1'; $value2 = 'TEST2'; - $this->redis->set('test', 'key', $value1); - $this->redis->set('test', 'key', $value2); - $res = $this->redis->get('test', 'key'); + $this->store->set('test', 'key', $value1); + $this->store->set('test', 'key', $value2); + $res = $this->store->get('test', 'key'); $expected = $value2; $this->assertEquals($expected, $res); @@ -212,28 +192,10 @@ class RedisTest extends TestCase */ public function testDeleteData(): void { - $this->redis->set('test', 'key', 'TEST'); - $this->redis->delete('test', 'key'); - $res = $this->redis->get('test', 'key'); + $this->store->set('test', 'key', 'TEST'); + $this->store->delete('test', 'key'); + $res = $this->store->get('test', 'key'); $this->assertNull($res); } - - - /** - * @param \SimpleSAML\Configuration|\SimpleSAML\Store $service - * @param class-string $className - */ - protected function clearInstance($service, string $className): void - { - $reflectedClass = new ReflectionClass($className); - $reflectedInstance = $reflectedClass->getProperty('instance'); - $reflectedInstance->setAccessible(true); - if ($service instanceof Configuration) { - $reflectedInstance->setValue($service, []); - } else { - $reflectedInstance->setValue($service, null); - } - $reflectedInstance->setAccessible(false); - } } diff --git a/tests/lib/SimpleSAML/Store/SQLStoreTest.php b/tests/lib/SimpleSAML/Store/SQLStoreTest.php new file mode 100644 index 0000000000000000000000000000000000000000..84c91c185bbbe66fcfeacd4d023cbc999e1762df --- /dev/null +++ b/tests/lib/SimpleSAML/Store/SQLStoreTest.php @@ -0,0 +1,145 @@ +<?php + +declare(strict_types=1); + +namespace SimpleSAML\Test\Store; + +use PHPUnit\Framework\TestCase; +use SimpleSAML\Configuration; +use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; + +/** + * Tests for the SQL store. + * + * For the full copyright and license information, please view the LICENSE file that was distributed with this source + * code. + * + * @covers \SimpleSAML\Store\SQLStore + * @package simplesamlphp/simplesamlphp + */ +class SQLStoreTest extends TestCase +{ + /** @var \SimpleSAML\Store\SQLStore $store */ + private Store\SQLStore $store; + + + /** + */ + protected function setUp(): void + { + Configuration::loadFromArray([ + 'store.type' => 'sql', + 'store.sql.dsn' => 'sqlite::memory:', + 'store.sql.prefix' => 'phpunit_', + ], '[ARRAY]', 'simplesaml'); + + $this->store = new Store\SQLStore(); + } + + + /** + * @test + */ + public function SQLInstance(): void + { + $this->assertInstanceOf(Store\SQLStore::class, $this->store); + } + + + /** + * @test + */ + public function kvstoreTableVersion(): void + { + $version = $this->store->getTableVersion('kvstore'); + + $this->assertEquals(2, $version); + } + + + /** + * @test + */ + public function newTableVersion(): void + { + $version = $this->store->getTableVersion('test'); + + $this->assertEquals(0, $version); + } + + + /** + * @test + */ + public function testSetTableVersion(): void + { + $this->store->setTableVersion('kvstore', 2); + $version = $this->store->getTableVersion('kvstore'); + + $this->assertEquals(2, $version); + } + + + /** + * @test + */ + public function testGetEmptyData(): void + { + $value = $this->store->get('test', 'foo'); + + $this->assertNull($value); + } + + + /** + * @test + */ + public function testInsertData(): void + { + $this->store->set('test', 'foo', 'bar'); + $value = $this->store->get('test', 'foo'); + + $this->assertEquals('bar', $value); + } + + + /** + * @test + */ + public function testOverwriteData(): void + { + $this->store->set('test', 'foo', 'bar'); + $this->store->set('test', 'foo', 'baz'); + $value = $this->store->get('test', 'foo'); + + $this->assertEquals('baz', $value); + } + + + /** + * @test + */ + public function testDeleteData(): void + { + $this->store->set('test', 'foo', 'bar'); + $this->store->delete('test', 'foo'); + $value = $this->store->get('test', 'foo'); + + $this->assertNull($value); + } + + + /** + * @test + */ + public function testVeryLongKey(): void + { + $key = str_repeat('x', 100); + $this->store->set('test', $key, 'bar'); + $this->store->delete('test', $key); + $value = $this->store->get('test', $key); + + $this->assertNull($value); + } +} diff --git a/tests/lib/SimpleSAML/Store/SQLTest.php b/tests/lib/SimpleSAML/Store/SQLTest.php deleted file mode 100644 index 14358f87f29af74bba2c91e6ff884fd90955c03c..0000000000000000000000000000000000000000 --- a/tests/lib/SimpleSAML/Store/SQLTest.php +++ /dev/null @@ -1,198 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace SimpleSAML\Test\Store; - -use PHPUnit\Framework\TestCase; -use ReflectionClass; -use SimpleSAML\Configuration; -use SimpleSAML\Store; - -/** - * Tests for the SQL store. - * - * For the full copyright and license information, please view the LICENSE file that was distributed with this source - * code. - * - * @covers \SimpleSAML\Store\SQL - * - * @package simplesamlphp/simplesamlphp - */ -class SQLTest extends TestCase -{ - /** - */ - protected function setUp(): void - { - Configuration::loadFromArray([ - 'store.type' => 'sql', - 'store.sql.dsn' => 'sqlite::memory:', - 'store.sql.prefix' => 'phpunit_', - ], '[ARRAY]', 'simplesaml'); - } - - - /** - * @test - */ - public function SQLInstance(): void - { - $store = Store::getInstance(); - - $this->assertInstanceOf(Store\SQL::class, $store); - } - - - /** - * @test - */ - public function kvstoreTableVersion(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $version = $store->getTableVersion('kvstore'); - - $this->assertEquals(2, $version); - } - - - /** - * @test - */ - public function newTableVersion(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $version = $store->getTableVersion('test'); - - $this->assertEquals(0, $version); - } - - - /** - * @test - */ - public function testSetTableVersion(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $store->setTableVersion('kvstore', 2); - $version = $store->getTableVersion('kvstore'); - - $this->assertEquals(2, $version); - } - - - /** - * @test - */ - public function testGetEmptyData(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $value = $store->get('test', 'foo'); - - $this->assertNull($value); - } - - - /** - * @test - */ - public function testInsertData(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $store->set('test', 'foo', 'bar'); - $value = $store->get('test', 'foo'); - - $this->assertEquals('bar', $value); - } - - - /** - * @test - */ - public function testOverwriteData(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $store->set('test', 'foo', 'bar'); - $store->set('test', 'foo', 'baz'); - $value = $store->get('test', 'foo'); - - $this->assertEquals('baz', $value); - } - - - /** - * @test - */ - public function testDeleteData(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $store->set('test', 'foo', 'bar'); - $store->delete('test', 'foo'); - $value = $store->get('test', 'foo'); - - $this->assertNull($value); - } - - - /** - * @test - */ - public function testVeryLongKey(): void - { - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $key = str_repeat('x', 100); - $store->set('test', $key, 'bar'); - $store->delete('test', $key); - $value = $store->get('test', $key); - - $this->assertNull($value); - } - - - /** - */ - protected function tearDown(): void - { - $config = Configuration::getInstance(); - - /** @var \SimpleSAML\Store\SQL $store */ - $store = Store::getInstance(); - - $this->clearInstance($config, Configuration::class); - $this->clearInstance($store, Store::class); - } - - - /** - * @param \SimpleSAML\Configuration|\SimpleSAML\Store $service - * @param class-string $className - */ - protected function clearInstance($service, string $className): void - { - $reflectedClass = new ReflectionClass($className); - $reflectedInstance = $reflectedClass->getProperty('instance'); - $reflectedInstance->setAccessible(true); - if ($service instanceof Configuration) { - $reflectedInstance->setValue($service, []); - } else { - $reflectedInstance->setValue($service, null); - } - $reflectedInstance->setAccessible(false); - } -} diff --git a/tests/lib/SimpleSAML/StoreTest.php b/tests/lib/SimpleSAML/Store/StoreFactoryTest.php similarity index 51% rename from tests/lib/SimpleSAML/StoreTest.php rename to tests/lib/SimpleSAML/Store/StoreFactoryTest.php index 8f278eb6e008c519ea35ff33d5270410316c208d..3164d5f1c67dfe7cebaa00870f39fd53b16cd538 100644 --- a/tests/lib/SimpleSAML/StoreTest.php +++ b/tests/lib/SimpleSAML/Store/StoreFactoryTest.php @@ -4,23 +4,25 @@ declare(strict_types=1); namespace SimpleSAML\Test; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Predis\Client; use ReflectionClass; use SimpleSAML\Configuration; use SimpleSAML\Error\CriticalConfigurationError; use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; /** - * Tests for the Store abstract class. + * Tests for the StoreFactory class. * * For the full copyright and license information, please view the LICENSE file that was * distributed with this source code. * - * @covers \SimpleSAML\Store - * + * @covers \SimpleSAML\Store\StoreFactory * @package simplesamlphp/simplesamlphp */ -class StoreTest extends TestCase +class StoreFactoryTest extends TestCase { /** * @test @@ -29,8 +31,11 @@ class StoreTest extends TestCase { Configuration::loadFromArray([], '[ARRAY]', 'simplesaml'); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + /** @var false $store */ - $store = Store::getInstance(); + $store = StoreFactory::getInstance($storeType); $this->assertFalse($store); } @@ -41,10 +46,15 @@ class StoreTest extends TestCase */ public function phpSessionStore(): void { - Configuration::loadFromArray([], '[ARRAY]', 'simplesaml'); + Configuration::loadFromArray([ + 'store.type' => 'phpsession', + ], '[ARRAY]', 'simplesaml'); + + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); /** @var false $store */ - $store = Store::getInstance(); + $store = StoreFactory::getInstance($storeType); $this->assertFalse($store); } @@ -59,9 +69,36 @@ class StoreTest extends TestCase 'store.type' => 'memcache', ], '[ARRAY]', 'simplesaml'); - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); + + $store = StoreFactory::getInstance($storeType); + + $this->assertInstanceOf(Store\MemcacheStore::class, $store); + } + + + /** + * @test + */ + public function redisStore(): void + { + Configuration::loadFromArray([ + 'store.type' => 'redis', + 'store.redis.prefix' => 'phpunit_', + ], '[ARRAY]', 'simplesaml'); + + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); + + /** @psalm-var \SimpleSAML\Store\RedisStore $store */ + $store = StoreFactory::getInstance($storeType); + $store->redis = $this->getMockBuilder(Client::class) + ->setMethods(['get', 'set', 'setex', 'del', 'disconnect', '__destruct']) + ->disableOriginalConstructor() + ->getMock(); - $this->assertInstanceOf(Store\Memcache::class, $store); + $this->assertInstanceOf(Store\RedisStore::class, $store); } @@ -76,9 +113,12 @@ class StoreTest extends TestCase 'store.sql.prefix' => 'phpunit_', ], '[ARRAY]', 'simplesaml'); - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); + + $store = StoreFactory::getInstance($storeType); - $this->assertInstanceOf(Store\SQL::class, $store); + $this->assertInstanceOf(Store\SQLStore::class, $store); } @@ -88,14 +128,17 @@ class StoreTest extends TestCase public function pathStore(): void { Configuration::loadFromArray([ - 'store.type' => '\SimpleSAML\Store\SQL', + 'store.type' => '\SimpleSAML\Store\SQLStore', 'store.sql.dsn' => 'sqlite::memory:', 'store.sql.prefix' => 'phpunit_', ], '[ARRAY]', 'simplesaml'); - $store = Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); + + $store = StoreFactory::getInstance($storeType); - $this->assertInstanceOf(Store\SQL::class, $store); + $this->assertInstanceOf(Store\SQLStore::class, $store); } @@ -111,7 +154,10 @@ class StoreTest extends TestCase 'store.sql.prefix' => 'phpunit_', ], '[ARRAY]', 'simplesaml'); - Store::getInstance(); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); + + StoreFactory::getInstance($storeType); } @@ -120,16 +166,18 @@ class StoreTest extends TestCase protected function tearDown(): void { $config = Configuration::getInstance(); - /** @var \SimpleSAML\Store $store */ - $store = Store::getInstance(); + $storeType = $config->getString('store.type', 'phpsession'); + + /** @var \SimpleSAML\Store\StoreInterface $store */ + $store = StoreFactory::getInstance($storeType); $this->clearInstance($config, Configuration::class); - $this->clearInstance($store, Store::class); + $this->clearInstance($store, StoreFactory::class); } /** - * @param \SimpleSAML\Configuration|\SimpleSAML\Store $service + * @param \SimpleSAML\Configuration|\SimpleSAML\Store\StoreInterface $service * @param class-string $className */ protected function clearInstance($service, string $className): void diff --git a/tests/modules/saml/lib/IdP/SQLNameIDTest.php b/tests/modules/saml/lib/IdP/SQLNameIDTest.php index e33bd604e99d487a7202c8728a0aa8aa76b24ce7..e12629ae73eb48ea695a8242d5a5c1962139ee1d 100644 --- a/tests/modules/saml/lib/IdP/SQLNameIDTest.php +++ b/tests/modules/saml/lib/IdP/SQLNameIDTest.php @@ -10,6 +10,7 @@ use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\Module\saml\IdP\SQLNameID; use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; /** * Test for the SQLNameID helper class. @@ -45,10 +46,11 @@ class SQLNameIDTest extends TestCase ], '[ARRAY]', 'simplesaml'); $this->addGetDelete(); $config = Configuration::getInstance(); - /** @var \SimpleSAML\Store $store */ - $store = Store::getInstance(); + $storeType = $config->getString('store.type'); + /** @var \SimpleSAML\Store\StoreInterface $store */ + $store = StoreFactory::getInstance($storeType); $this->clearInstance($config, Configuration::class); - $this->clearInstance($store, Store::class); + $this->clearInstance($store, StoreFactory::class); } @@ -61,15 +63,18 @@ class SQLNameIDTest extends TestCase Configuration::loadFromArray([ 'store.type' => 'memcache', ], '[ARRAY]', 'simplesaml'); - $store = Store::getInstance(); - $this->assertInstanceOf(Store\Memcache::class, $store); + $config = Configuration::getInstance(); + $storeType = $config->getString('store.type'); + $store = StoreFactory::getInstance($storeType); + $this->assertInstanceOf(Store\MemcacheStore::class, $store); $this->expectException(Error\Exception::class); $this->addGetDelete(); $config = Configuration::getInstance(); - /** @var \SimpleSAML\Store $store */ - $store = Store::getInstance(); + $storeType = $config->getString('store.type'); + /** @var \SimpleSAML\Store\StoreInterface $store */ + $store = StoreFactory::getInstance($storeType); $this->clearInstance($config, Configuration::class); - $this->clearInstance($store, Store::class); + $this->clearInstance($store, StoreFactory::class); } @@ -98,7 +103,7 @@ class SQLNameIDTest extends TestCase /** - * @param \SimpleSAML\Configuration|\SimpleSAML\Store $service + * @param \SimpleSAML\Configuration|\SimpleSAML\Store\StoreInterface $service * @param class-string $className */ protected function clearInstance($service, string $className): void diff --git a/www/saml2/idp/ArtifactResolutionService.php b/www/saml2/idp/ArtifactResolutionService.php index 76cc4a18504ea194b628a0a1dbcc14a4a8870acc..5e91f3fc3d715dc592c41cc930f6db96c7bd2372 100644 --- a/www/saml2/idp/ArtifactResolutionService.php +++ b/www/saml2/idp/ArtifactResolutionService.php @@ -20,7 +20,7 @@ use SimpleSAML\Configuration; use SimpleSAML\Error; use SimpleSAML\Module; use SimpleSAML\Metadata; -use SimpleSAML\Store; +use SimpleSAML\Store\StoreFactory; $config = Configuration::getInstance(); if (!$config->getBoolean('enable.saml20-idp', false) || !Module::isModuleEnabled('saml')) { @@ -35,7 +35,8 @@ if (!$idpMetadata->getBoolean('saml20.sendartifact', false)) { throw new Error\Error('NOACCESS'); } -$store = Store::getInstance(); +$storeType = $config->getString('store.type', 'phpsession'); +$store = StoreFactory::getInstance($storeType); if ($store === false) { throw new Exception('Unable to send artifact without a datastore configured.'); }