merged branch dpb587/patch-sectok (PR #2414)

Commits
-------

ab9caa0 [Security] Check for request's session before attempting writes.
dabff0e [Security] Support removing tokens from a session.

Discussion
----------

[Security] Support removing tokens from a session.

Currently there is no way to remove a session's security token without invalidating the entire session and all its data (the ContextListener will only update the session if a token is non-null and non-anonymous). This patch fixes that.

I consider this a bug and I found no tests to prove otherwise. Let me know if I'm mistaken. Originally mentioned at https://groups.google.com/d/topic/symfony-devs/ojLvh0WUbfo/discussion

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -

---------------------------------------------------------------------------

by ms937 at 2011/10/24 05:19:21 -0700

This change looks good to me. In fact I'm using similar patch in my app and it works as intended. Also, several other people requested this on the mailing list. Could someone from Symfony team merge this? Thanks.

src/Symfony/Component/Security/Http/Firewall/ContextListener.php

@@ -93,19 +93,19 @@ class ContextListener implements ListenerInterface
             return;
         }
 
-        if (null === $token = $this->context->getToken()) {
-            return;
-        }
-
-        if (null === $token || $token instanceof AnonymousToken) {
-            return;
-        }
-
         if (null !== $this->logger) {
             $this->logger->debug('Write SecurityContext in the session');
         }
 
-        $event->getRequest()->getSession()->set('_security_'.$this->contextKey, serialize($token));
+        if (null === $session = $event->getRequest()->getSession()) {
+            return;
+        }
+
+        if ((null === $token = $this->context->getToken()) || ($token instanceof AnonymousToken)) {
+            $session->remove('_security_'.$this->contextKey);
+        } else {
+            $session->set('_security_'.$this->contextKey, serialize($token));
+        }
     }
 
     /**

tests/Symfony/Tests/Component/Security/Http/Firewall/ContextListenerTest.php

@@ -0,0 +1,107 @@
+<?php
+
+namespace Symfony\Test\Component\Security\Http\Firewall;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\Session;
+use Symfony\Component\HttpFoundation\SessionStorage\ArraySessionStorage;
+use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
+use Symfony\Component\Security\Core\SecurityContext;
+use Symfony\Component\Security\Http\Firewall\ContextListener;
+
+class ContextListenerTest extends \PHPUnit_Framework_TestCase
+{
+    protected function setUp()
+    {
+        $this->securityContext = new SecurityContext(
+            $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface'),
+            $this->getMock('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')
+        );
+    }
+
+    protected function tearDown()
+    {
+        unset($this->securityContext);
+    }
+
+    public function testOnKernelResponseWillAddSession()
+    {
+        $session = $this->runSessionOnKernelResponse(
+            new UsernamePasswordToken('test1', 'pass1', 'phpunit'),
+            null
+        );
+
+        $token = unserialize($session->get('_security_session'));
+        $this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $token);
+        $this->assertEquals('test1', $token->getUsername());
+    }
+
+    public function testOnKernelResponseWillReplaceSession()
+    {
+        $session = $this->runSessionOnKernelResponse(
+            new UsernamePasswordToken('test1', 'pass1', 'phpunit'),
+            'C:10:"serialized"'
+        );
+
+        $token = unserialize($session->get('_security_session'));
+        $this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $token);
+        $this->assertEquals('test1', $token->getUsername());
+    }
+
+    public function testOnKernelResponseWillRemoveSession()
+    {
+        $session = $this->runSessionOnKernelResponse(
+            null,
+            'C:10:"serialized"'
+        );
+
+        $this->assertFalse($session->has('_security_session'));
+    }
+
+    protected function runSessionOnKernelResponse($newToken, $original = null)
+    {
+        $session = new Session(new ArraySessionStorage());
+
+        if ($original !== null) {
+            $session->set('_security_session', $original);
+        }
+
+        $this->securityContext->setToken($newToken);
+
+        $request = new Request();
+        $request->setSession($session);
+
+        $event = new FilterResponseEvent(
+            $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface'),
+            $request,
+            HttpKernelInterface::MASTER_REQUEST,
+            new Response()
+        );
+
+        $listener = new ContextListener($this->securityContext, array(), 'session');
+        $listener->onKernelResponse($event);
+
+        return $session;
+    }
+
+    public function testOnKernelResponseWithoutSession()
+    {
+        $this->securityContext->setToken(new UsernamePasswordToken('test1', 'pass1', 'phpunit'));
+        $request = new Request();
+
+        $event = new FilterResponseEvent(
+            $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface'),
+            $request,
+            HttpKernelInterface::MASTER_REQUEST,
+            new Response()
+        );
+
+        $listener = new ContextListener($this->securityContext, array(), 'session');
+        $listener->onKernelResponse($event);
+
+        $this->assertFalse($request->hasSession());
+    }
+}