Explorar el Código

fixes several bugs

Johannes Schmitt hace 14 años
padre
commit
f300edebe4

+ 3 - 0
UPDATE.md

@@ -77,6 +77,9 @@ RC4 to RC5
         Session::getAttributes() -> Session::all()
         Session::setAttributes() -> Session::replace()
 
+* {_locale} is not supported in paths in the access_control section anymore. You can
+  rewrite the paths using a regular expression such as "(?:[a-z]{2})".
+
 RC3 to RC4
 ----------
 

+ 5 - 0
src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Controller/LocalizedController.php

@@ -34,6 +34,11 @@ class LocalizedController extends ContainerAware
         throw new \RuntimeException('logoutAction() should never be called.');
     }
 
+    public function secureAction()
+    {
+        throw new \RuntimeException('secureAction() should never be called.');
+    }
+
     public function profileAction()
     {
         return new Response('Profile');

+ 12 - 1
src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Resources/config/localized_routing.yml

@@ -1,19 +1,30 @@
 localized_login_path:
     pattern: /{_locale}/login
     defaults: { _controller: FormLoginBundle:Localized:login }
+    requirements: { _locale: "^[a-z]{2}$" }
 
 localized_check_path:
     pattern: /{_locale}/login_check
     defaults: { _controller: FormLoginBundle:Localized:loginCheck }
+    requirements: { _locale: "^[a-z]{2}$" }
     
 localized_default_target_path:
     pattern: /{_locale}/profile
     defaults: { _controller: FormLoginBundle:Localized:profile }
+    requirements: { _locale: "^[a-z]{2}$" }
 
 localized_logout_path:
     pattern: /{_locale}/logout
     defaults: { _controller: FormLoginBundle:Localized:logout }
+    requirements: { _locale: "^[a-z]{2}$" }
     
 localized_logout_target_path:
     pattern: /{_locale}/
-    defaults: { _controller: FormLoginBundle:Localized:homepage }
+    defaults: { _controller: FormLoginBundle:Localized:homepage }
+    requirements: { _locale: "^[a-z]{2}$" }
+    
+localized_secure_path:
+    pattern: /{_locale}/secure/
+    defaults: { _controller: FormLoginBundle:Localized:secure }
+    requirements: { _locale: "^[a-z]{2}$" }
+    

+ 24 - 0
src/Symfony/Bundle/SecurityBundle/Tests/Functional/LocalizedRoutesAsPathTest.php

@@ -26,6 +26,30 @@ class LocalizedRoutesAsPathTest extends WebTestCase
         $this->assertEquals('Homepage', $client->followRedirect()->text());
     }
 
+    /**
+     * @dataProvider getLocales
+     */
+    public function testAccessRestrictedResource($locale)
+    {
+        $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => 'localized_routes.yml'));
+        $client->insulate();
+
+        $client->request('GET', '/'.$locale.'/secure/');
+        $this->assertRedirect($client->getResponse(), '/'.$locale.'/login');
+    }
+
+    /**
+     * @dataProvider getLocales
+     */
+    public function testAccessRestrictedResourceWithForward($locale)
+    {
+        $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => 'localized_routes_with_forward.yml'));
+        $client->insulate();
+
+        $crawler = $client->request('GET', '/'.$locale.'/secure/');
+        $this->assertEquals(1, count($crawler->selectButton('login')), (string) $client->getResponse());
+    }
+
     public function getLocales()
     {
         return array(array('en'), array('de'));

+ 24 - 7
src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php

@@ -13,25 +13,37 @@ namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
 
 class SecurityRoutingIntegrationTest extends WebTestCase
 {
-    public function testRoutingErrorIsNotExposedForProtectedResourceWhenAnonymous()
+    /**
+     * @dataProvider getConfigs
+     */
+    public function testRoutingErrorIsNotExposedForProtectedResourceWhenAnonymous($config)
     {
-        $client = $this->createClient(array('test_case' => 'StandardFormLogin'));
+        $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => $config));
+        $client->insulate();
         $client->request('GET', '/protected_resource');
 
         $this->assertRedirect($client->getResponse(), '/login');
     }
 
-    public function testRoutingErrorIsExposedWhenNotProtected()
+    /**
+     * @dataProvider getConfigs
+     */
+    public function testRoutingErrorIsExposedWhenNotProtected($config)
     {
-        $client = $this->createClient(array('test_case' => 'StandardFormLogin'));
+        $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => $config));
+        $client->insulate();
         $client->request('GET', '/unprotected_resource');
 
-        $this->assertEquals(404, $client->getResponse()->getStatusCode());
+        $this->assertEquals(404, $client->getResponse()->getStatusCode(), (string) $client->getResponse());
     }
 
-    public function testRoutingErrorIsNotExposedForProtectedResourceWhenLoggedInWithInsufficientRights()
+    /**
+     * @dataProvider getConfigs
+     */
+    public function testRoutingErrorIsNotExposedForProtectedResourceWhenLoggedInWithInsufficientRights($config)
     {
-        $client = $this->createClient(array('test_case' => 'StandardFormLogin'));
+        $client = $this->createClient(array('test_case' => 'StandardFormLogin', 'root_config' => $config));
+        $client->insulate();
 
         $form = $client->request('GET', '/login')->selectButton('login')->form();
         $form['_username'] = 'johannes';
@@ -43,6 +55,11 @@ class SecurityRoutingIntegrationTest extends WebTestCase
         $this->assertNotEquals(404, $client->getResponse()->getStatusCode());
     }
 
+    public function getConfigs()
+    {
+        return array(array('config.yml'), array('routes_as_path.yml'));
+    }
+
     protected function setUp()
     {
         parent::setUp();

+ 1 - 1
src/Symfony/Bundle/SecurityBundle/Tests/Functional/WebTestCase.php

@@ -18,7 +18,7 @@ class WebTestCase extends BaseWebTestCase
 {
     static public function assertRedirect($response, $location)
     {
-        self::assertTrue($response->isRedirect());
+        self::assertTrue($response->isRedirect(), 'Response is not a redirect, got status code: '.$response->getStatusCode());
         self::assertEquals('http://localhost'.$location, $response->headers->get('Location'));
     }
 

+ 5 - 1
src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/localized_routes.yml

@@ -19,4 +19,8 @@ security:
             logout:
                 path: localized_logout_path
                 target: localized_logout_target_path
-            anonymous: ~
+            anonymous: ~
+            
+    access_control:
+        - { path: '^/(?:[a-z]{2})/secure/.*', roles: ROLE_USER }       
+    

+ 9 - 0
src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/localized_routes_with_forward.yml

@@ -0,0 +1,9 @@
+imports:
+    - { resource: ./localized_routes.yml }
+    
+security:
+    firewalls: 
+        default:
+            form_login:
+                use_forward: true
+                failure_forward: true

+ 1 - 5
src/Symfony/Component/HttpFoundation/RequestMatcher.php

@@ -100,11 +100,7 @@ class RequestMatcher implements RequestMatcherInterface
         }
 
         if (null !== $this->path) {
-            if (null !== $session = $request->getSession()) {
-                $path = strtr($this->path, array('{_locale}' => $session->getLocale(), '#' => '\\#'));
-            } else {
-                $path = str_replace('#', '\\#', $this->path);
-            }
+            $path = str_replace('#', '\\#', $this->path);
 
             if (!preg_match('#'.$path.'#', $request->getPathInfo())) {
                 return false;

+ 42 - 15
src/Symfony/Component/Security/Http/HttpUtils.php

@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Security\Http;
 
+use Symfony\Component\Security\Core\SecurityContextInterface;
+
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\Routing\RouterInterface;
@@ -45,22 +47,10 @@ class HttpUtils
      */
     public function createRedirectResponse(Request $request, $path, $status = 302)
     {
-        if (0 === strpos($path, '/')) {
+        if ('/' === $path[0]) {
             $path = $request->getUriForPath($path);
         } elseif (0 !== strpos($path, 'http')) {
-            // hack (don't have a better solution for now)
-            $context = $this->router->getContext();
-            try {
-                $parameters = $this->router->match($request->getPathInfo());
-            } catch (\Exception $e) {
-            }
-
-            if (isset($parameters['_locale'])) {
-                $context->setParameter('_locale', $parameters['_locale']);
-            } elseif ($session = $request->getSession()) {
-                $context->setParameter('_locale', $session->getLocale());
-            }
-
+            $this->resetLocale($request);
             $path = $this->generateUrl($path, true);
         }
 
@@ -78,10 +68,26 @@ class HttpUtils
     public function createRequest(Request $request, $path)
     {
         if ($path && '/' !== $path[0] && 0 !== strpos($path, 'http')) {
+            $this->resetLocale($request);
             $path = $this->generateUrl($path, true);
         }
 
-        return Request::create($path, 'get', array(), $request->cookies->all(), array(), $request->server->all());
+        $newRequest = Request::create($path, 'get', array(), $request->cookies->all(), array(), $request->server->all());
+        if ($session = $request->getSession()) {
+            $newRequest->setSession($session);
+        }
+
+        if ($request->attributes->has(SecurityContextInterface::AUTHENTICATION_ERROR)) {
+            $newRequest->attributes->set(SecurityContextInterface::AUTHENTICATION_ERROR, $request->attributes->get(SecurityContextInterface::AUTHENTICATION_ERROR));
+        }
+        if ($request->attributes->has(SecurityContextInterface::ACCESS_DENIED_ERROR)) {
+            $newRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $request->attributes->get(SecurityContextInterface::ACCESS_DENIED_ERROR));
+        }
+        if ($request->attributes->has(SecurityContextInterface::LAST_USERNAME)) {
+            $newRequest->attributes->set(SecurityContextInterface::LAST_USERNAME, $request->attributes->get(SecurityContextInterface::LAST_USERNAME));
+        }
+
+        return $newRequest;
     }
 
     /**
@@ -107,6 +113,27 @@ class HttpUtils
         return $path === $request->getPathInfo();
     }
 
+    // hack (don't have a better solution for now)
+    private function resetLocale(Request $request)
+    {
+        $context = $this->router->getContext();
+        if ($context->getParameter('_locale')) {
+            return;
+        }
+
+        try {
+            $parameters = $this->router->match($request->getPathInfo());
+
+            if (isset($parameters['_locale'])) {
+                $context->setParameter('_locale', $parameters['_locale']);
+            } elseif ($session = $request->getSession()) {
+                $context->setParameter('_locale', $session->getLocale());
+            }
+        } catch (\Exception $e) {
+            // let's hope user doesn't use the locale in the path
+        }
+    }
+
     private function generateUrl($route, $absolute = false)
     {
         if (null === $this->router) {

+ 1 - 4
tests/Symfony/Tests/Component/HttpFoundation/RequestMatcherTest.php

@@ -142,7 +142,7 @@ class RequestMatcherTest extends \PHPUnit_Framework_TestCase
         $this->assertFalse($matcher->matches($request));
     }
 
-    public function testPathWithLocale()
+    public function testPathWithLocaleIsNotSupported()
     {
         $matcher = new RequestMatcher();
         $request = Request::create('/en/login');
@@ -152,9 +152,6 @@ class RequestMatcherTest extends \PHPUnit_Framework_TestCase
         $request->setSession($session);
 
         $matcher->matchPath('^/{_locale}/login$');
-        $this->assertTrue($matcher->matches($request));
-
-        $session->setLocale('de');
         $this->assertFalse($matcher->matches($request));
     }
 

+ 11 - 0
tests/Symfony/Tests/Component/Security/Http/HttpUtilsTest.php

@@ -61,6 +61,17 @@ class HttpUtilsTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('bar', $subRequest->server->get('Foo'));
 
         // route name
+        $utils = new HttpUtils($router = $this->getMockBuilder('Symfony\Component\Routing\Router')->disableOriginalConstructor()->getMock());
+        $router
+            ->expects($this->once())
+            ->method('generate')
+            ->will($this->returnValue('/foo/bar'))
+        ;
+        $router
+            ->expects($this->any())
+            ->method('getContext')
+            ->will($this->returnValue($this->getMock('Symfony\Component\Routing\RequestContext')))
+        ;
         $subRequest = $utils->createRequest($this->getRequest(), 'foobar');
         $this->assertEquals('/foo/bar', $subRequest->getPathInfo());