From 12256ed2023091ba35c73df5e7f2e79ba13275a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ou=C5=A1ek?=
<melanger@users.noreply.github.com>
Date: Fri, 25 Oct 2019 16:16:08 +0200
Subject: [PATCH] add store option to SQLPersistentNameID (#1219)
add store option to SQLPersistentNameID
---
modules/saml/docs/nameid.md | 11 +-
.../lib/Auth/Process/SQLPersistentNameID.php | 15 +-
modules/saml/lib/IdP/SQLNameID.php | 183 +++++++++++++-----
tests/modules/saml/lib/IdP/SQLNameIDTest.php | 110 +++++++++++
4 files changed, 268 insertions(+), 51 deletions(-)
create mode 100644 tests/modules/saml/lib/IdP/SQLNameIDTest.php
diff --git a/modules/saml/docs/nameid.md b/modules/saml/docs/nameid.md
index 2c719552c..bf3ae2b84 100644
--- a/modules/saml/docs/nameid.md
+++ b/modules/saml/docs/nameid.md
@@ -63,10 +63,11 @@ No extra options are available for this filter.
`saml:SQLPersistentNameID`
--------------------------
-Generates and stores persistent NameIDs in a SQL datastore.
+Generates and stores persistent NameIDs in a SQL database.
-This filter generates and stores a persistent NameID in a SQL datastore.
-To use this filter, SimpleSAMLphp must be configured to use a SQL datastore.
+This filter generates and stores a persistent NameID in a SQL database.
+To use this filter, either specify the `store` option and a database,
+or configure SimpleSAMLphp to use a SQL datastore.
See the `store.type` configuration option in `config.php`.
### Options
@@ -86,6 +87,10 @@ See the `store.type` configuration option in `config.php`.
: Whether to ignore an explicit `AllowCreate="false"` in the authentication request's NameIDPolicy.
The default is `FALSE`, which will only create new NameIDs when the SP specifies `AllowCreate="true"` in the authentication request.
+`store`
+: An array of database options passed to `\SimpleSAML\Database`, keys prefixed with `database.`.
+ The default is `[]`, which uses the global SQL datastore.
+
Setting both `allowUnspecified` and `alwaysCreate` to `TRUE` causes `saml:SQLPersistentNameID` to behave like `saml:PersistentNameID` (and other NameID generation filters), at the expense of creating unnecessary entries in the SQL datastore.
diff --git a/modules/saml/lib/Auth/Process/SQLPersistentNameID.php b/modules/saml/lib/Auth/Process/SQLPersistentNameID.php
index fc3decdbd..b6c0f48cd 100644
--- a/modules/saml/lib/Auth/Process/SQLPersistentNameID.php
+++ b/modules/saml/lib/Auth/Process/SQLPersistentNameID.php
@@ -42,6 +42,13 @@ class SQLPersistentNameID extends \SimpleSAML\Module\saml\BaseNameIDGenerator
*/
private $alwaysCreate = false;
+ /**
+ * Database store configuration.
+ *
+ * @var array
+ */
+ private $storeConfig = [];
+
/**
* Initialize this filter, parse configuration.
@@ -74,6 +81,10 @@ class SQLPersistentNameID extends \SimpleSAML\Module\saml\BaseNameIDGenerator
if (isset($config['alwaysCreate'])) {
$this->alwaysCreate = (bool) $config['alwaysCreate'];
}
+
+ if (isset($config['store'])) {
+ $this->storeConfig = (array) $config['store'];
+ }
}
@@ -148,7 +159,7 @@ class SQLPersistentNameID extends \SimpleSAML\Module\saml\BaseNameIDGenerator
return null;
}
- $value = \SimpleSAML\Module\saml\IdP\SQLNameID::get($idpEntityId, $spEntityId, $uid);
+ $value = \SimpleSAML\Module\saml\IdP\SQLNameID::get($idpEntityId, $spEntityId, $uid, $this->storeConfig);
if ($value !== null) {
Logger::debug(
'SQLPersistentNameID: Found persistent NameID ' . var_export($value, true) . ' for user ' .
@@ -172,7 +183,7 @@ class SQLPersistentNameID extends \SimpleSAML\Module\saml\BaseNameIDGenerator
'SQLPersistentNameID: Created persistent NameID ' . var_export($value, true) . ' for user ' .
var_export($uid, true) . '.'
);
- \SimpleSAML\Module\saml\IdP\SQLNameID::add($idpEntityId, $spEntityId, $uid, $value);
+ \SimpleSAML\Module\saml\IdP\SQLNameID::add($idpEntityId, $spEntityId, $uid, $value, $this->storeConfig);
return $value;
}
diff --git a/modules/saml/lib/IdP/SQLNameID.php b/modules/saml/lib/IdP/SQLNameID.php
index 69fb3911c..d4f91d9d3 100644
--- a/modules/saml/lib/IdP/SQLNameID.php
+++ b/modules/saml/lib/IdP/SQLNameID.php
@@ -5,6 +5,8 @@ namespace SimpleSAML\Module\saml\IdP;
use PDO;
use SimpleSAML\Error;
use SimpleSAML\Store;
+use SimpleSAML\Database;
+use SimpleSAML\Configuration;
/**
* Helper class for working with persistent NameIDs stored in SQL datastore.
@@ -13,40 +15,140 @@ use SimpleSAML\Store;
*/
class SQLNameID
{
+ const TABLE_VERSION = 1;
+ const DEFAULT_TABLE_PREFIX = '';
+ const TABLE_SUFFIX = '_saml_PersistentNameID';
+
+
/**
- * Create NameID table in SQL, if it is missing.
- *
- * @param \SimpleSAML\Store\SQL $store The datastore.
+ * @param string $query
+ * @param array $params Parameters
+ * @param array $config
+ * @return \PDOStatement object
+ */
+ private static function read($query, array $params = [], array $config = [])
+ {
+ if (!empty($config)) {
+ $database = Database::getInstance(Configuration::loadFromArray($config));
+ $stmt = $database->read($query, $params);
+ } else {
+ $store = self::getStore();
+ $stmt = $store->pdo->prepare($query);
+ $stmt->execute($params);
+ }
+ return $stmt;
+ }
+
+
+ /**
+ * @param string $query
+ * @param array $params Parameters
+ * @param array $config
+ * @return int|false The number of rows affected by the query or false on error.
+ */
+ private static function write($query, array $params = [], array $config = [])
+ {
+ if (!empty($config)) {
+ $database = Database::getInstance(Configuration::loadFromArray($config));
+ $res = $database->write($query, $params);
+ } else {
+ $store = self::getStore();
+ $query = $store->pdo->prepare($query);
+ $res = $query->execute($params);
+ if ($res) {
+ $res = $query->rowCount();
+ }
+ }
+ return $res;
+ }
+
+
+ /**
+ * @param array $config
+ * @return string
+ */
+ private static function tableName(array $config = [])
+ {
+ $store = empty($config) ? self::getStore() : null;
+ $prefix = $store === null ? self::DEFAULT_TABLE_PREFIX : $store->prefix;
+ $table = $prefix . self::TABLE_SUFFIX;
+ return $table;
+ }
+
+ /**
+ * @param array $config
* @return void
*/
- private static function createTable(Store\SQL $store)
+ private static function create(array $config = [])
{
- if ($store->getTableVersion('saml_PersistentNameID') === 1) {
- return;
+ $store = empty($config) ? self::getStore() : null;
+ $table = self::tableName($config);
+ if ($store === null) {
+ try {
+ self::createTable($table, $config);
+ } catch (\Exception $e) {
+ \SimpleSAML\Logger::debug('SQL persistent NameID table already exists.');
+ }
+ } elseif ($store->getTableVersion('saml_PersistentNameID') !== self::TABLE_VERSION) {
+ self::createTable($table);
+ $store->setTableVersion('saml_PersistentNameID', self::TABLE_VERSION);
}
+ }
+
- $query = 'CREATE TABLE ' . $store->prefix . '_saml_PersistentNameID (
+ /**
+ * @param string $query
+ * @param array $params
+ * @param array $config
+ * @return \PDOStatement
+ */
+ private static function createAndRead($query, array $params = [], array $config = [])
+ {
+ self::create($config);
+ return self::read($query, $params, $config);
+ }
+
+
+ /**
+ * @param string $query
+ * @param array $params
+ * @param array $config
+ * @return int|false The number of rows affected by the query or false on error.
+ */
+ private static function createAndWrite($query, array $params = [], array $config = [])
+ {
+ self::create($config);
+ return self::write($query, $params, $config);
+ }
+
+
+ /**
+ * Create NameID table in SQL.
+ *
+ * @param string $table The table name.
+ * @param array $config
+ * @return void
+ */
+ private static function createTable($table, array $config = [])
+ {
+ $query = 'CREATE TABLE ' . $table . ' (
_idp VARCHAR(256) NOT NULL,
_sp VARCHAR(256) NOT NULL,
_user VARCHAR(256) NOT NULL,
_value VARCHAR(40) NOT NULL,
UNIQUE (_idp, _sp, _user)
)';
- $store->pdo->exec($query);
-
- $query = 'CREATE INDEX ' . $store->prefix . '_saml_PersistentNameID_idp_sp ON ';
- $query .= $store->prefix . '_saml_PersistentNameID (_idp, _sp)';
- $store->pdo->exec($query);
+ self::write($query, [], $config);
- $store->setTableVersion('saml_PersistentNameID', 1);
+ $query = 'CREATE INDEX ' . $table . '_idp_sp ON ';
+ $query .= $table . ' (_idp, _sp)';
+ self::write($query, [], $config);
}
/**
* Retrieve the SQL datastore.
*
- * Will also ensure that the NameID table is present.
- *
* @return \SimpleSAML\Store\SQL SQL datastore.
*/
private static function getStore()
@@ -58,8 +160,6 @@ class SQLNameID
);
}
- self::createTable($store);
-
return $store;
}
@@ -67,22 +167,20 @@ class SQLNameID
/**
* Add a NameID into the database.
*
- * @param \SimpleSAML\Store\SQL $store The data store.
* @param string $idpEntityId The IdP entityID.
* @param string $spEntityId The SP entityID.
* @param string $user The user's unique identificator (e.g. username).
* @param string $value The NameID value.
+ * @param array $config
* @return void
*/
- public static function add($idpEntityId, $spEntityId, $user, $value)
+ public static function add($idpEntityId, $spEntityId, $user, $value, array $config = [])
{
assert(is_string($idpEntityId));
assert(is_string($spEntityId));
assert(is_string($user));
assert(is_string($value));
- $store = self::getStore();
-
$params = [
'_idp' => $idpEntityId,
'_sp' => $spEntityId,
@@ -90,10 +188,9 @@ class SQLNameID
'_value' => $value,
];
- $query = 'INSERT INTO ' . $store->prefix;
- $query .= '_saml_PersistentNameID (_idp, _sp, _user, _value) VALUES(:_idp, :_sp, :_user, :_value)';
- $query = $store->pdo->prepare($query);
- $query->execute($params);
+ $query = 'INSERT INTO ' . self::tableName($config);
+ $query .= ' (_idp, _sp, _user, _value) VALUES(:_idp, :_sp, :_user, :_value)';
+ self::createAndWrite($query, $params, $config);
}
@@ -103,26 +200,24 @@ class SQLNameID
* @param string $idpEntityId The IdP entityID.
* @param string $spEntityId The SP entityID.
* @param string $user The user's unique identificator (e.g. username).
+ * @param array $config
* @return string|null $value The NameID value, or NULL of no NameID value was found.
*/
- public static function get($idpEntityId, $spEntityId, $user)
+ public static function get($idpEntityId, $spEntityId, $user, array $config = [])
{
assert(is_string($idpEntityId));
assert(is_string($spEntityId));
assert(is_string($user));
- $store = self::getStore();
-
$params = [
'_idp' => $idpEntityId,
'_sp' => $spEntityId,
'_user' => $user,
];
- $query = 'SELECT _value FROM ' . $store->prefix;
- $query .= '_saml_PersistentNameID WHERE _idp = :_idp AND _sp = :_sp AND _user = :_user';
- $query = $store->pdo->prepare($query);
- $query->execute($params);
+ $query = 'SELECT _value FROM ' . self::tableName($config);
+ $query .= ' WHERE _idp = :_idp AND _sp = :_sp AND _user = :_user';
+ $query = self::createAndRead($query, $params, $config);
$row = $query->fetch(PDO::FETCH_ASSOC);
if ($row === false) {
@@ -140,26 +235,24 @@ class SQLNameID
* @param string $idpEntityId The IdP entityID.
* @param string $spEntityId The SP entityID.
* @param string $user The user's unique identificator (e.g. username).
+ * @param array $config
* @return void
*/
- public static function delete($idpEntityId, $spEntityId, $user)
+ public static function delete($idpEntityId, $spEntityId, $user, array $config = [])
{
assert(is_string($idpEntityId));
assert(is_string($spEntityId));
assert(is_string($user));
- $store = self::getStore();
-
$params = [
'_idp' => $idpEntityId,
'_sp' => $spEntityId,
'_user' => $user,
];
- $query = 'DELETE FROM ' . $store->prefix;
- $query .= '_saml_PersistentNameID WHERE _idp = :_idp AND _sp = :_sp AND _user = :_user';
- $query = $store->pdo->prepare($query);
- $query->execute($params);
+ $query = 'DELETE FROM ' . self::tableName($config);
+ $query .= ' WHERE _idp = :_idp AND _sp = :_sp AND _user = :_user';
+ self::createAndWrite($query, $params, $config);
}
@@ -168,24 +261,22 @@ class SQLNameID
*
* @param string $idpEntityId The IdP entityID.
* @param string $spEntityId The SP entityID.
+ * @param array $config
* @return array Array of userid => NameID.
*/
- public static function getIdentities($idpEntityId, $spEntityId)
+ public static function getIdentities($idpEntityId, $spEntityId, array $config = [])
{
assert(is_string($idpEntityId));
assert(is_string($spEntityId));
- $store = self::getStore();
-
$params = [
'_idp' => $idpEntityId,
'_sp' => $spEntityId,
];
- $query = 'SELECT _user, _value FROM ' . $store->prefix;
- $query .= '_saml_PersistentNameID WHERE _idp = :_idp AND _sp = :_sp';
- $query = $store->pdo->prepare($query);
- $query->execute($params);
+ $query = 'SELECT _user, _value FROM ' . self::tableName($config);
+ $query .= ' WHERE _idp = :_idp AND _sp = :_sp';
+ $query = self::createAndRead($query, $params, $config);
$res = [];
while (($row = $query->fetch(PDO::FETCH_ASSOC)) !== false) {
diff --git a/tests/modules/saml/lib/IdP/SQLNameIDTest.php b/tests/modules/saml/lib/IdP/SQLNameIDTest.php
new file mode 100644
index 000000000..5aacfcf59
--- /dev/null
+++ b/tests/modules/saml/lib/IdP/SQLNameIDTest.php
@@ -0,0 +1,110 @@
+<?php
+
+namespace SimpleSAML\Test\Module\saml\IdP;
+
+/**
+ * Test for the SQLNameID helper class.
+ *
+ * @author Pavel Brousek <brousek@ics.muni.cz>
+ * @package SimpleSAMLphp
+ */
+
+use PHPUnit\Framework\TestCase;
+use SimpleSAML\Configuration;
+use SimpleSAML\Error;
+use SimpleSAML\Module\saml\IdP\SQLNameID;
+use SimpleSAML\Store;
+
+class SQLNameIDTest extends TestCase
+{
+ /**
+ * @param array $config
+ * @return void
+ */
+ private function addGetDelete(array $config = [])
+ {
+ SQLNameID::add('idp', 'sp', 'user', 'value', $config);
+ $this->assertEquals('value', SQLNameID::get('idp', 'sp', 'user', $config));
+ SQLNameID::delete('idp', 'sp', 'user', $config);
+ $this->assertNull(SQLNameID::get('idp', 'sp', 'user', $config));
+ }
+
+ /**
+ * Test Store.
+ * @test
+ * @return void
+ */
+ public function testSQLStore()
+ {
+ Configuration::loadFromArray([
+ 'store.type' => 'sql',
+ 'store.sql.dsn' => 'sqlite::memory:',
+ 'store.sql.prefix' => 'phpunit_',
+ ], '[ARRAY]', 'simplesaml');
+ $this->addGetDelete();
+ $config = Configuration::getInstance();
+ /** @var \SimpleSAML\Store $store */
+ $store = Store::getInstance();
+ $this->clearInstance($config, Configuration::class);
+ $this->clearInstance($store, Store::class);
+ }
+
+ /**
+ * Test incompatible Store.
+ * @test
+ * @return void
+ */
+ public function testIncompatibleStore()
+ {
+ Configuration::loadFromArray([
+ 'store.type' => 'memcache',
+ ], '[ARRAY]', 'simplesaml');
+ $store = Store::getInstance();
+ $this->assertInstanceOf(Store\Memcache::class, $store);
+ $this->expectException(Error\Exception::class);
+ $this->addGetDelete();
+ $config = Configuration::getInstance();
+ /** @var \SimpleSAML\Store $store */
+ $store = Store::getInstance();
+ $this->clearInstance($config, Configuration::class);
+ $this->clearInstance($store, Store::class);
+ }
+
+ /**
+ * Test Database.
+ * @test
+ * @return void
+ */
+ public function testDatabase()
+ {
+ $config = [
+ 'database.dsn' => 'sqlite::memory:',
+ 'database.username' => null,
+ 'database.password' => null,
+ 'database.prefix' => 'phpunit_',
+ 'database.persistent' => true,
+ 'database.slaves' => [
+ [
+ 'dsn' => 'sqlite::memory:',
+ 'username' => null,
+ 'password' => null,
+ ],
+ ],
+ ];
+ $this->addGetDelete($config);
+ }
+
+ /**
+ * @param \SimpleSAML\Configuration|\SimpleSAML\Store $service
+ * @param string $className
+ * @return void
+ */
+ protected function clearInstance($service, $className)
+ {
+ $reflectedClass = new \ReflectionClass($className);
+ $reflectedInstance = $reflectedClass->getProperty('instance');
+ $reflectedInstance->setAccessible(true);
+ $reflectedInstance->setValue($service, null);
+ $reflectedInstance->setAccessible(false);
+ }
+}
--
GitLab