Skip to content

[2.2][Security] concurrent sessions #786

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

Closed
wants to merge 38 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c0b33f8
added listener, strategy, registry, configuration for concurrent sess…
May 4, 2011
0f3d7c4
added SessionLogoutHandler to setup of ConcurrentSessionListener
May 5, 2011
e9bcd84
removed uneeded interface
May 5, 2011
9f4a52d
Merge branch 'master' of https://github.com/symfony/symfony
May 5, 2011
507befa
removed security.authentication.session_registry_storage service from…
May 5, 2011
5559ef8
removed unneeded methods
May 5, 2011
238606b
Merge branch 'master' of https://github.com/symfony/symfony
May 6, 2011
455197f
prepare Doctrine implementation of SessionRegistryStorage
May 17, 2011
a4eeae5
Merge branch 'master' of github.com:paschke/symfony
May 28, 2011
aadb2a7
default Doctrine implementation of SessionInformation
Jun 2, 2011
5b5edd8
Merge branch 'master' of https://github.com/symfony/symfony
Jun 2, 2011
33d83b4
removed SessionInformationIterator class
Jun 2, 2011
624b01f
forgot query condition
Jun 2, 2011
7032534
Merge branch 'master' of github.com:symfony/symfony
Dec 15, 2011
bc182f9
added DBAL default implementation of Symfony\Component\Security\Http\…
Dec 22, 2011
d0c826a
recreate SessionInformation if they were lost from the registry
Dec 22, 2011
c9f08a4
minor fixes
Dec 22, 2011
13813da
added listener, strategy, registry, configuration for concurrent sess…
May 4, 2011
8ef6923
added SessionLogoutHandler to setup of ConcurrentSessionListener
May 5, 2011
f4f0a8c
removed uneeded interface
May 5, 2011
e11a540
removed security.authentication.session_registry_storage service from…
May 5, 2011
a3d0a59
removed unneeded methods
May 5, 2011
c800408
prepare Doctrine implementation of SessionRegistryStorage
May 17, 2011
9c8bd21
default Doctrine implementation of SessionInformation
Jun 2, 2011
2efbf11
removed SessionInformationIterator class
Jun 2, 2011
3cb882c
forgot query condition
Jun 2, 2011
0aa9353
added DBAL default implementation of Symfony\Component\Security\Http\…
Dec 22, 2011
ff2122b
recreate SessionInformation if they were lost from the registry
Dec 22, 2011
efc783b
minor fixes
Dec 22, 2011
d302e37
Merge branch 'master' of http://github.com/paschke/symfony
Dec 23, 2011
bed28d2
moved DBAL implemention of SessionInformation to /Bridge
Dec 23, 2011
29e2f27
Merge branch 'master' of git://github.com/symfony/symfony
Jan 3, 2012
7a0f46a
use HttpUtils in ConcurrentSessionListener
Jan 4, 2012
ac7a1d6
removed @return void comments
Apr 8, 2012
b24a364
Merge branch 'master' of git://github.com/symfony/symfony
Apr 8, 2012
f7dfb3f
reacting to comments on PR 786
Apr 9, 2012
3cef9bb
reuse the instance level loader n src/Symfony/Bundle/SecurityBundle/D…
Apr 9, 2012
d567980
passing sessionStrategy into authenticationListenerFactories individu…
Apr 9, 2012
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/Symfony/Bridge/Doctrine/Security/SessionRegistry/Schema.php
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>
*/
final class Schema extends BaseSchema
{
/**
* Constructor
*
* @param array $options the names for tables
*/
public function __construct(array $options)
{
parent::__construct();

$this->addSessionInformationTable($options);
}

/**
* Adds the session_information table to the schema
*/
protected function addSessionInformationTable(array $options)
{
$table = $this->createTable($options['session_information_table_name']);
$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'));
$table->addUniqueIndex(array('session_id'));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien.potencier@symfony-project.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 Symfony\Component\Security\Http\Session\SessionInformation as BaseSessionInformation;

/**
* SessionInformation.
*
* Allows to persist SessionInformation using Doctrine DBAL.
*
* @author Stefan Paschke <stefan.paschke@gmail.com>
*/
class SessionInformation extends BaseSessionInformation
{
public function __construct($sessionId, $username, \DateTime $lastRequest = null, \DateTime $expired = null)
{
parent::__construct($sessionId, $username);

if (null !== $lastRequest) {
$this->setLastRequest($lastRequest);
}

if (null !== $expired) {
$this->setExpired($expired);
}
}

public function getExpired()
{
return parent::getExpired();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?php

namespace Symfony\Bridge\Doctrine\Security\SessionRegistry;

use Doctrine\DBAL\Driver\Connection;
use Symfony\Component\Security\Http\Session\SessionInformation;
use Symfony\Component\Security\Http\Session\SessionRegistryStorageInterface;

class SessionRegistryStorage implements SessionRegistryStorageInterface
{
protected $connection;
protected $options;

public function __construct(Connection $connection, array $options)
{
$this->connection = $connection;
$this->options = $options;
}

/**
* not implemented
*/
public function getUsers()
{
throw new \BadMethodCallException("Not implemented.");
}

/**
* Obtains the maintained information for one session.
*
* @param string $sessionId the session identifier key.
* @return SessionInformation a SessionInformation object.
*/
public function getSessionInformation($sessionId)
{
$statement = $this->connection->executeQuery(
'SELECT * FROM '.$this->options['session_information_table_name'].' WHERE session_id = :session_id',
array('session_id' => $sessionId)
);

$data = $statement->fetch(\PDO::FETCH_ASSOC);

return $data ? $this->instantiateSessionInformationFromResultSet($data) : null;
}

/**
* Obtains the maintained information for one user.
*
* @param string $username The user identifier.
* @param boolean $includeExpiredSessions.
* @return array An array of SessionInformation objects.
*/
public function getSessionInformations($username, $includeExpiredSessions = false)
{
$sessionInformations = array();

$statement = $this->connection->executeQuery(
'SELECT *
FROM '.$this->options['session_information_table_name'].'
WHERE username = :username'.($includeExpiredSessions ? '' : ' AND expired IS NULL ').'
ORDER BY last_request DESC',
array('username' => $username)
);

while ($data = $statement->fetch(\PDO::FETCH_ASSOC))
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brace need to be in same line as while.

$sessionInformations[] = $this->instantiateSessionInformationFromResultSet($data);
}

return $sessionInformations;
}

/**
* Adds information for one session.
*
* @param string $sessionId the session identifier key.
* @param SessionInformation a SessionInformation object.
*/
public function setSessionInformation(SessionInformation $sessionInformation)
{
$statement = $this->connection->prepare(
'INSERT INTO '.$this->options['session_information_table_name'].'
(session_id, username, last_request, expired) VALUES(:session_id, :username, :last_request, :expired)
ON DUPLICATE KEY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see your not using DQL or some sort of query abstraction.
ON DUPLICATE KEY does not work in PostgreSQL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed you are right .. i hear @kimhemsoe is working on UPSERT abstraction for the Doctrine DBAL, so it might be in the summer release.
until then i guess all we can sensibly do is add a switch statement and hardcode handling for different RDBMS.

for sqlite we can use REPLACE INTO, for postgresql (and maybe also DB2) MERGE etc.
but i think its ok to add these later on a need basis rather than burden this PR with supporting all RDBMS

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best solution is to either.

  1. UPDATE check if any rows are affected and if not perform an INSERT, but when using triggers that always fire this may lead to bugs,
  2. INSERT and when hitting an error check the error-code and perform INSERT instead.

As far as I can find PostgreSQL does not support MERGE/UPSERT just yet.

https://wiki.postgresql.org/wiki/Add_MERGE_command_GSoC_2010
https://wiki.postgresql.org/wiki/SQL_MERGE

REPLACE INTO (In MySQL) is not completely safe, first it performs an DELETE (also firing any FK's or triggers) and then inserting it! ⚠️
http://bugs.mysql.com/bug.php?id=2418

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REPLACE INTO should be avoided, but on SQLite its still better than anything else for UPSERT
for MySQL INSERT .. ON DUPLICATE KEY UPDATE is the way to go.

option 1) and 2) are problematic because it can trigger all sorts of ugly side effects if one causes such error conditions. i guess if users use a separate connection to the database for their session handler and the rest of the code, those side effects should not be a big issue, but then it could also lead to other issue with the RDBMS f.e. running out of connections.

UPDATE username=:username, last_request=:last_request, expired=:expired');

$statement->bindValue('session_id', $sessionInformation->getSessionId());
$statement->bindValue('username', $sessionInformation->getUsername());
$statement->bindValue('last_request', $sessionInformation->getLastRequest(), 'datetime');
$statement->bindValue('expired', $sessionInformation->getExpired(), 'datetime');
$statement->execute();
}

/**
* Deletes the maintained information of one session.
*
* @param string $sessionId the session identifier key.
*/
public function removeSessionInformation($sessionId)
{
$this->connection->delete($this->options['session_information_table_name'], array('session_id' => $sessionId));
}

private function instantiateSessionInformationFromResultSet($data)
{
return new $this->options['session_information_class_name'](
$data['session_id'],
$data['username'],
(null == $data['last_request']) ? null : new \DateTime($data['last_request']),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( and ) can be removed, but you need to change check into null ===.

Same line below.

(null == $data['expired']) ? null : new \DateTime($data['expired'])
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?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>
*/
class InitConcurrentSessionsCommand extends ContainerAwareCommand
{
/**
* @see Command
*/
protected function configure()
{
parent::configure();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed as the parent method (in the base Command class) is empty


$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>

You can also output the SQL instead of executing it:

<info>./app/console init:concurrent-session --dump-sql</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();
$tables = array(
'session_information_table_name' => $this->getContainer()->getParameter('security.session_registry.dbal.session_information_table_name'),
);

foreach ($tables as $table) {
if (in_array($table, $tableNames, true)) {
$output->writeln(sprintf('The table "%s" already exists. Aborting.', $table));

return;
}
}

$schema = new Schema($tables);
foreach ($schema->toSql($connection->getDatabasePlatform()) as $sql) {
$connection->exec($sql);
}

$output->writeln('concurrent session tables have been initialized successfully.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,31 @@ public function getConfigTreeBuilder()
$this->addFirewallsSection($rootNode, $this->factories);
$this->addAccessControlSection($rootNode);
$this->addRoleHierarchySection($rootNode);
$this->addSessionRegistrySection($rootNode);

return $tb;
}

private function addSessionRegistrySection(ArrayNodeDefinition $rootNode)
{
$rootNode
->children()
->arrayNode('session_registry')
->children()
->scalarNode('connection')->end()
->arrayNode('tables')
->addDefaultsIfNotSet()
->children()
->scalarNode('session_information')->defaultValue('cs_session_information')->end()
->end()
->end()
->scalarNode('session_registry_storage')->end()
->end()
->end()
->end()
;
}

private function addAclSection(ArrayNodeDefinition $rootNode)
{
$rootNode
Expand Down Expand Up @@ -248,6 +269,13 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
->scalarNode('role')->defaultValue('ROLE_ALLOWED_TO_SWITCH')->end()
->end()
->end()
->arrayNode('session_concurrency')
->canBeUnset()
->children()
->scalarNode('max_sessions')->defaultNull()->end()
->scalarNode('expiration_url')->defaultValue('/')->end()
->end()
->end()
;

$abstractFactoryKeys = array();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ abstract class AbstractFactory implements SecurityFactoryInterface
'failure_forward' => false,
);

public function create(ContainerBuilder $container, $id, $config, $userProviderId, $defaultEntryPointId)
public function create(ContainerBuilder $container, $id, $config, $userProviderId, $defaultEntryPointId, $sessionStrategy)
{
// authentication provider
$authProviderId = $this->createAuthProvider($container, $id, $config, $userProviderId);

// authentication listener
$listenerId = $this->createListener($container, $id, $config, $userProviderId);
$listenerId = $this->createListener($container, $id, $config, $userProviderId, $sessionStrategy);

// add remember-me aware tag if requested
if ($this->isRememberMeAware($config)) {
Expand Down Expand Up @@ -144,10 +144,11 @@ protected function isRememberMeAware($config)
return $config['remember_me'];
}

protected function createListener($container, $id, $config, $userProvider)
protected function createListener($container, $id, $config, $userProvider, $sessionStrategy)
{
$listenerId = $this->getListenerId();
$listener = new DefinitionDecorator($listenerId);
$listener->replaceArgument(2, $sessionStrategy);
$listener->replaceArgument(4, $id);
$listener->replaceArgument(5, array_intersect_key($config, $this->options));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $config,
return $provider;
}

protected function createListener($container, $id, $config, $userProvider)
protected function createListener($container, $id, $config, $userProvider, $sessionStrategy)
{
$listenerId = parent::createListener($container, $id, $config, $userProvider);
$listenerId = parent::createListener($container, $id, $config, $userProvider, $sessionStrategy);

if (isset($config['csrf_provider'])) {
$container
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/
class HttpBasicFactory implements SecurityFactoryInterface
{
public function create(ContainerBuilder $container, $id, $config, $userProvider, $defaultEntryPoint)
public function create(ContainerBuilder $container, $id, $config, $userProvider, $defaultEntryPoint, $sessionStrategy)
{
$provider = 'security.authentication.provider.dao.'.$id;
$container
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/
class HttpDigestFactory implements SecurityFactoryInterface
{
public function create(ContainerBuilder $container, $id, $config, $userProvider, $defaultEntryPoint)
public function create(ContainerBuilder $container, $id, $config, $userProvider, $defaultEntryPoint, $sessionStrategy)
{
$provider = 'security.authentication.provider.dao.'.$id;
$container
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class RememberMeFactory implements SecurityFactoryInterface
'remember_me_parameter' => '_remember_me',
);

public function create(ContainerBuilder $container, $id, $config, $userProvider, $defaultEntryPoint)
public function create(ContainerBuilder $container, $id, $config, $userProvider, $defaultEntryPoint, $sessionStrategy)
{
// authentication provider
$authProviderId = 'security.authentication.provider.rememberme.'.$id;
Expand Down
Loading