Merge pull request sonata-project#3186 from core23/patch5

Check routes before redirecting in CRUDController
mcwebb · Oct 6, 2015 · 1864af1 · 1864af1
2 parents 1feecf9 + 5d4b7c1
commit 1864af1
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 7 deletions.
diff --git a/Controller/CRUDController.php b/Controller/CRUDController.php
@@ -503,7 +503,16 @@ protected function redirectTo($object, Request $request = null)
         }
 
         if (!$url) {
-            $url = $this->admin->generateObjectUrl('edit', $object);
+            foreach (array('edit', 'show') as $route) {
+                if ($this->admin->hasRoute($route) && $this->admin->isGranted(strtoupper($route))) {
+                    $url = $this->admin->generateObjectUrl($route, $object);
+                    break;
+                }
+            }
+        }
+
+        if (!$url) {
+            $url = $this->admin->generateUrl('list');
         }
 
         return new RedirectResponse($url);

diff --git a/Tests/Controller/CRUDControllerTest.php b/Tests/Controller/CRUDControllerTest.php
@@ -619,6 +619,11 @@ public function testListActionAccessDenied()
 
     public function testPreList()
     {
+        $this->admin->expects($this->any())
+            ->method('hasRoute')
+            ->with($this->equalTo('list'))
+            ->will($this->returnValue(true));
+
         $this->admin->expects($this->once())
             ->method('checkAccess')
             ->with($this->equalTo('list'))
@@ -636,6 +641,11 @@ public function testListAction()
     {
         $datagrid = $this->getMock('Sonata\AdminBundle\Datagrid\DatagridInterface');
 
+        $this->admin->expects($this->any())
+            ->method('hasRoute')
+            ->with($this->equalTo('list'))
+            ->will($this->returnValue(true));
+
         $this->admin->expects($this->once())
             ->method('checkAccess')
             ->with($this->equalTo('list'))
@@ -857,7 +867,7 @@ public function testShowAction()
     /**
      * @dataProvider getRedirectToTests
      */
-    public function testRedirectTo($expected, $queryParams, $hasActiveSubclass)
+    public function testRedirectTo($expected, $route, $queryParams, $hasActiveSubclass)
     {
         $this->admin->expects($this->any())
             ->method('hasActiveSubclass')
@@ -869,6 +879,16 @@ public function testRedirectTo($expected, $queryParams, $hasActiveSubclass)
             $this->request->query->set($key, $value);
         }
 
+        $this->admin->expects($this->any())
+            ->method('hasRoute')
+            ->with($this->equalTo($route))
+            ->will($this->returnValue(true));
+
+        $this->admin->expects($this->any())
+            ->method('isGranted')
+            ->with($this->equalTo(strtoupper($route)))
+            ->will($this->returnValue(true));
+
         $response = $this->protectedTestedMethods['redirectTo']->invoke($this->controller, $object, $this->request);
         $this->assertInstanceOf('Symfony\Component\HttpFoundation\RedirectResponse', $response);
         $this->assertSame($expected, $response->getTargetUrl());
@@ -877,11 +897,11 @@ public function testRedirectTo($expected, $queryParams, $hasActiveSubclass)
     public function getRedirectToTests()
     {
         return array(
-            array('stdClass_edit', array(), false),
-            array('list', array('btn_update_and_list' => true), false),
-            array('list', array('btn_create_and_list' => true), false),
-            array('create', array('btn_create_and_create' => true), false),
-            array('create?subclass=foo', array('btn_create_and_create' => true, 'subclass' => 'foo'), true),
+            array('stdClass_edit', 'edit', array(), false),
+            array('list', 'list', array('btn_update_and_list' => true), false),
+            array('list', 'list', array('btn_create_and_list' => true), false),
+            array('create', 'create', array('btn_create_and_create' => true), false),
+            array('create?subclass=foo', 'create', array('btn_create_and_create' => true, 'subclass' => 'foo'), true),
         );
     }
 
@@ -1409,9 +1429,18 @@ public function testEditActionSuccess($expectedToStringValue, $toStringValue)
 
         $this->admin->expects($this->once())
             ->method('checkAccess')
+            ->with($this->equalTo('edit'));
+
+        $this->admin->expects($this->once())
+            ->method('hasRoute')
             ->with($this->equalTo('edit'))
             ->will($this->returnValue(true));
 
+        $this->admin->expects($this->once())
+            ->method('isGranted')
+            ->with($this->equalTo('EDIT'))
+            ->will($this->returnValue(true));
+
         $form = $this->getMockBuilder('Symfony\Component\Form\Form')
             ->disableOriginalConstructor()
             ->getMock();
@@ -1882,6 +1911,10 @@ public function testCreateActionSuccess($expectedToStringValue, $toStringValue)
         $this->admin->expects($this->exactly(2))
             ->method('checkAccess')
             ->will($this->returnCallback(function ($name, $objectIn = null) use ($object) {
+                if ($name == 'edit') {
+                    return true;
+                }
+
                 if ($name != 'create') {
                     return false;
                 }
@@ -1893,6 +1926,16 @@ public function testCreateActionSuccess($expectedToStringValue, $toStringValue)
                 return ($objectIn === $object);
             }));
 
+        $this->admin->expects($this->once())
+            ->method('hasRoute')
+            ->with($this->equalTo('edit'))
+            ->will($this->returnValue(true));
+
+        $this->admin->expects($this->once())
+            ->method('isGranted')
+            ->with($this->equalTo('EDIT'))
+            ->will($this->returnValue(true));
+
         $this->admin->expects($this->once())
             ->method('getNewInstance')
             ->will($this->returnValue($object));