-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WIP] [Security] Session concurrency control #12009
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
66ed331
90eba34
82c5f71
5772a75
c6b1488
9c3aebb
2b5e2ed
5e22c79
5664571
f458bd4
f92286a
52b7ba4
4ca3a47
6224ba0
81c01de
c0edefd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bridge\Doctrine\Security\SessionRegistry; | ||
|
||
use Doctrine\DBAL\Schema\Schema as BaseSchema; | ||
|
||
/** | ||
* The schema used for the ACL system. | ||
* | ||
* @author Stefan Paschke <stefan.paschke@gmail.com> | ||
* @author Antonio J. García Lagar <aj@garcialagar.es> | ||
*/ | ||
final class Schema extends BaseSchema | ||
{ | ||
/** | ||
* @param string $table The name of the table to create. | ||
*/ | ||
public function __construct($table) | ||
{ | ||
parent::__construct(); | ||
|
||
$this->addSessionInformationTable($table); | ||
} | ||
|
||
/** | ||
* Adds the session_information table to the schema | ||
* | ||
* @param string $table The name of the table to create. | ||
*/ | ||
private function addSessionInformationTable($table) | ||
{ | ||
$table = $this->createTable($table); | ||
$table->addColumn('session_id', 'string'); | ||
$table->addColumn('username', 'string'); | ||
$table->addColumn('expired', 'datetime', array('unsigned' => true, 'notnull' => false)); | ||
$table->addColumn('last_request', 'datetime', array('unsigned' => true, 'notnull' => false)); | ||
$table->setPrimaryKey(array('session_id')); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,190 @@ | ||
<?php | ||
|
||
namespace Symfony\Bridge\Doctrine\Security\SessionRegistry; | ||
|
||
use Doctrine\DBAL\Connection; | ||
use Doctrine\DBAL\DBALException; | ||
use Symfony\Component\Security\Http\Session\SessionInformation; | ||
use Symfony\Component\Security\Http\Session\SessionRegistryStorageInterface; | ||
|
||
/** | ||
* @author Stefan Paschke <stefan.paschke@gmail.com> | ||
* @author Antonio J. García Lagar <aj@garcialagar.es> | ||
*/ | ||
class SessionRegistryStorage implements SessionRegistryStorageInterface | ||
{ | ||
private $connection; | ||
private $table; | ||
|
||
/** | ||
* @param Connection $connection The DB connection | ||
* @param string $table The table name to store session information | ||
*/ | ||
public function __construct(Connection $connection, $table) | ||
{ | ||
$this->connection = $connection; | ||
$this->table = $table; | ||
} | ||
|
||
/** | ||
* Gets the stored information for the given session. | ||
* | ||
* @param string $sessionId the session identifier key. | ||
* @return SessionInformation a SessionInformation object. | ||
*/ | ||
public function getSessionInformation($sessionId) | ||
{ | ||
$statement = $this->connection->executeQuery( | ||
'SELECT * FROM '.$this->table.' WHERE session_id = :session_id', | ||
array('session_id' => $sessionId) | ||
); | ||
|
||
$data = $statement->fetch(\PDO::FETCH_ASSOC); | ||
|
||
return $data ? $this->instantiateSessionInformationFromResultSet($data) : null; | ||
} | ||
|
||
/** | ||
* Gets the stored sessions information for the given username. | ||
* | ||
* @param string $username The user identifier. | ||
* @param bool $includeExpiredSessions If true, expired sessions information is included. | ||
* @return SessionInformations[] An array of SessionInformation objects. | ||
*/ | ||
public function getSessionInformations($username, $includeExpiredSessions = false) | ||
{ | ||
$sessionInformations = array(); | ||
|
||
$statement = $this->connection->executeQuery( | ||
'SELECT * | ||
FROM '.$this->table.' | ||
WHERE username = :username'.($includeExpiredSessions ? '' : ' AND expired IS NULL ').' | ||
ORDER BY last_request DESC', | ||
array('username' => $username) | ||
); | ||
|
||
while ($data = $statement->fetch(\PDO::FETCH_ASSOC)) { | ||
$sessionInformations[] = $this->instantiateSessionInformationFromResultSet($data); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should call |
||
|
||
$statement->closeCursor(); | ||
|
||
return $sessionInformations; | ||
} | ||
|
||
/** | ||
* Adds information for one session. | ||
* | ||
* @param SessionInformation a SessionInformation object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrong phpdoc |
||
*/ | ||
public function setSessionInformation(SessionInformation $sessionInformation) | ||
{ | ||
$mergeSql = $this->getMergeSql(); | ||
|
||
if (null !== $mergeSql) { | ||
$this->connection->executeQuery( | ||
$mergeSql, | ||
array( | ||
'session_id' => $sessionInformation->getSessionId(), | ||
'username' => $sessionInformation->getUsername(), | ||
'last_request' => $sessionInformation->getLastRequest(), | ||
'expired' => $sessionInformation->getExpired(), | ||
), | ||
array( | ||
'last_request' => 'datetime', | ||
'expired' => 'datetime', | ||
) | ||
); | ||
|
||
return true; | ||
} | ||
|
||
$updateStmt = $this->connection->prepare( | ||
"UPDATE $this->table SET username=:username, last_request=:last_request, expired=:expired WHERE session_id = :session_id" | ||
); | ||
$updateStmt->bindValue('session_id', $sessionInformation->getSessionId()); | ||
$updateStmt->bindValue('username', $sessionInformation->getUsername()); | ||
$updateStmt->bindValue('last_request', $sessionInformation->getLastRequest(), 'datetime'); | ||
$updateStmt->bindValue('expired', $sessionInformation->getExpired(), 'datetime'); | ||
$updateStmt->execute(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could use the DBAL API for this: $this->connection->update($this->table, array(
'username' => $sessionInformation->getUsername(),
'expired' => $sessionInformation->getExpired(),
'last_request' => $sessionInformation->getLastRequest(),
), array('session_id' => $sessionInformation->getSessionId()), array(
'last_request' => 'datetime',
'expired' => 'datetime',
)); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, actually no, given that the update statement is reused a second time |
||
|
||
// When MERGE is not supported, like in Postgres, we have to use this approach that can result in | ||
// duplicate key errors when the same sessioninfo is written simultaneously. We can just catch such an | ||
// error and re-execute the update. This is similar to a serializable transaction with retry logic | ||
// on serialization failures but without the overhead and without possible false positives due to | ||
// longer gap locking. | ||
if (!$updateStmt->rowCount()) { | ||
try { | ||
$this->connection->insert( | ||
$this->table, | ||
array( | ||
'session_id' => $sessionInformation->getSessionId(), | ||
'username' => $sessionInformation->getUsername(), | ||
'last_request' => $sessionInformation->getLastRequest(), | ||
'expired' => $sessionInformation->getExpired(), | ||
), | ||
array( | ||
'last_request' => 'datetime', | ||
'expired' => 'datetime', | ||
) | ||
); | ||
} catch (DBALException $e) { | ||
// Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys | ||
if ($e->getPrevious() instanceof \PDOException && 0 === strpos($e->getPrevious()->getCode(), '23')) { | ||
$updateStmt->execute(); | ||
} else { | ||
throw $e; | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Deletes stored information of one session. | ||
* | ||
* @param string $sessionId the session identifier key. | ||
*/ | ||
public function removeSessionInformation($sessionId) | ||
{ | ||
$this->connection->delete($this->table, array('session_id' => $sessionId)); | ||
} | ||
|
||
private function instantiateSessionInformationFromResultSet($data) | ||
{ | ||
return new SessionInformation( | ||
$data['session_id'], | ||
$data['username'], | ||
null === $data['last_request'] ? null : new \DateTime($data['last_request']), | ||
null === $data['expired'] ? null : new \DateTime($data['expired']) | ||
); | ||
} | ||
|
||
/** | ||
* Returns a merge/upsert (i.e. insert or update) SQL query when supported by the database. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't fully understand this phrase. Could it be simplified as follows?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The returned SQL is not an |
||
* | ||
* @return string|null The SQL string or null when not supported | ||
*/ | ||
private function getMergeSql() | ||
{ | ||
switch ($this->connection->getDatabasePlatform()->getName()) { | ||
case 'mysql': | ||
return "INSERT INTO $this->table (session_id, username, last_request, expired) VALUES (:session_id, :username, :last_request, :expired) ". | ||
"ON DUPLICATE KEY UPDATE username = VALUES(username), last_request = VALUES(last_request), expired = VALUES(expired)"; | ||
case 'oracle': | ||
// DUAL is Oracle specific dummy table | ||
return "MERGE INTO $this->table USING DUAL ON (session_id= :session_id) ". | ||
"WHEN NOT MATCHED THEN INSERT (session_id, username, last_request, expired) VALUES (:session_id, :username, :last_request, :expired) ". | ||
"WHEN MATCHED THEN UPDATE SET username = :username, last_request = :last_request, expired = :expired"; | ||
case 'mssql': | ||
if ($this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\SQLServer2008Platform || $this->connection->getDatabasePlatform() instanceof \Doctrine\DBAL\Platforms\SQLServer2012Platform) { | ||
// MERGE is only available since SQL Server 2008 and must be terminated by semicolon | ||
// It also requires HOLDLOCK according to http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx | ||
return "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 AS dummy) AS src ON (session_id = :session_id) ". | ||
"WHEN NOT MATCHED THEN INSERT (session_id, username, last_request, expired) VALUES (:session_id, :username, :last_request, :expired) ". | ||
"WHEN MATCHED THEN UPDATE SET username = :username, last_request = :last_request, expired = :expired;"; | ||
} | ||
case 'sqlite': | ||
return "INSERT OR REPLACE INTO $this->table (session_id, username, last_request, expired) VALUES (:session_id, :username, :last_request, :expired)"; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Bundle\SecurityBundle\Command; | ||
|
||
use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand; | ||
use Symfony\Bridge\Doctrine\Security\SessionRegistry\Schema; | ||
use Symfony\Component\Console\Input\InputInterface; | ||
use Symfony\Component\Console\Output\OutputInterface; | ||
|
||
/** | ||
* Installs the database schema required by the concurrent session Doctrine implementation | ||
* | ||
* @author Stefan Paschke <stefan.paschke@gmail.com> | ||
* @author Antonio J. García Lagar <aj@garcialagar.es> | ||
*/ | ||
class InitConcurrentSessionsCommand extends ContainerAwareCommand | ||
{ | ||
/** | ||
* @see Command | ||
*/ | ||
protected function configure() | ||
{ | ||
$this | ||
->setName('init:concurrent-session') | ||
->setDescription('Executes the SQL needed to generate the database schema required by the concurrent sessions feature.') | ||
->setHelp(<<<EOT | ||
The <info>init:concurrent-session</info> command executes the SQL needed to | ||
generate the database schema required by the concurrent session Doctrine implementation: | ||
|
||
<info>./app/console init:concurrent-session</info> | ||
EOT | ||
); | ||
} | ||
|
||
/** | ||
* @see Command | ||
*/ | ||
protected function execute(InputInterface $input, OutputInterface $output) | ||
{ | ||
$connection = $this->getContainer()->get('security.session_registry.dbal.connection'); | ||
$sm = $connection->getSchemaManager(); | ||
$tableNames = $sm->listTableNames(); | ||
$table = $this->getContainer()->getParameter('security.session_registry.dbal.session_information_table_name'); | ||
|
||
if (in_array($table, $tableNames, true)) { | ||
$output->writeln(sprintf('The table "%s" already exists. Aborting.', $table)); | ||
|
||
return; | ||
} | ||
|
||
$schema = new Schema($table); | ||
foreach ($schema->toSql($connection->getDatabasePlatform()) as $sql) { | ||
$connection->exec($sql); | ||
} | ||
|
||
$output->writeln('concurrent session table have been initialized successfully.'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing PHPdoc in this constructor