Merge pull request #9604 from cakephp/bugfix/auth-remember

Fix redirect to login with non GET requests and remembering location.

src/Controller/Component/AuthComponent.php

@@ -205,7 +205,7 @@ class AuthComponent extends Component
     /**
      * Request object
      *
-     * @var \Cake\Network\Request
+     * @var \Cake\Http\ServerRequest
      */
     public $request;
 
@@ -402,14 +402,18 @@ class AuthComponent extends Component
      */
     protected function _loginActionRedirectUrl()
     {
-        $currentUrl = $this->request->here(false);
+        $urlToRedirectBackTo = $this->_getUrlToRedirectBackTo();
 
         $loginAction = $this->_config['loginAction'];
+        if ($urlToRedirectBackTo === '/') {
+            return $loginAction;
+        }
+
         if (is_array($loginAction)) {
-            $loginAction['?'][static::QUERY_STRING_REDIRECT] = $currentUrl;
+            $loginAction['?'][static::QUERY_STRING_REDIRECT] = $urlToRedirectBackTo;
         } else {
             $char = strpos($loginAction, '?') === false ? '?' : '&';
-            $loginAction .= $char . static::QUERY_STRING_REDIRECT . '=' . urlencode($currentUrl);
+            $loginAction .= $char . static::QUERY_STRING_REDIRECT . '=' . urlencode($urlToRedirectBackTo);
         }
 
         return $loginAction;
@@ -504,7 +508,7 @@ class AuthComponent extends Component
      *
      * @param array|\ArrayAccess|null $user The user to check the authorization of.
      *   If empty the user fetched from storage will be used.
-     * @param \Cake\Network\Request|null $request The request to authenticate for.
+     * @param \Cake\Http\ServerRequest|null $request The request to authenticate for.
      *   If empty, the current request will be used.
      * @return bool True if $user is authorized, otherwise false
      */
@@ -992,4 +996,22 @@ class AuthComponent extends Component
     {
         return $this->_authorizationProvider;
     }
+
+    /**
+     * Returns the URL to redirect back to or / if not possible.
+     *
+     * This method takes the referrer into account if the
+     * request is not of type GET.
+     *
+     * @return string
+     */
+    protected function _getUrlToRedirectBackTo()
+    {
+        $urlToRedirectBackTo = $this->request->here(false);
+        if (!$this->request->is('get')) {
+            $urlToRedirectBackTo = $this->request->referer(true);
+        }
+
+        return $urlToRedirectBackTo;
+    }
 }

tests/TestCase/Controller/Component/AuthComponentTest.php

@@ -67,6 +67,8 @@ class AuthComponentTest extends TestCase
         });
 
         $request = new Request();
+        $request->env('REQUEST_METHOD', 'GET');
+
         $response = $this->getMockBuilder('Cake\Network\Response')
             ->setMethods(['stop'])
             ->getMock();
@@ -638,6 +640,53 @@ class AuthComponentTest extends TestCase
     }
 
     /**
+     * testLoginRedirect method with non GET
+     *
+     * @return void
+     */
+    public function testLoginRedirectPost()
+    {
+        $this->Auth->session->delete('Auth');
+
+        $url = '/posts/view/1';
+        $this->Auth->request->addParams(Router::parse($url));
+        $this->Auth->request->env('HTTP_REFERER', Router::url('/foo/bar', true));
+        $this->Auth->request->env('REQUEST_METHOD', 'POST');
+        $this->Auth->request->url = $this->Auth->request->here = Router::normalize($url);
+        $this->Auth->config('loginAction', ['controller' => 'AuthTest', 'action' => 'login']);
+        $event = new Event('Controller.startup', $this->Controller);
+        $response = $this->Auth->startup($event);
+
+        $this->assertInstanceOf('Cake\Network\Response', $response);
+        $expected = Router::url(['controller' => 'AuthTest', 'action' => 'login', '?' => ['redirect' => '/foo/bar']], true);
+        $redirectHeader = $response->header()['Location'];
+        $this->assertEquals($expected, $redirectHeader);
+    }
+
+    /**
+     * testLoginRedirect method with non GET and no referrer
+     *
+     * @return void
+     */
+    public function testLoginRedirectPostNoReferer()
+    {
+        $this->Auth->session->delete('Auth');
+
+        $url = '/posts/view/1';
+        $this->Auth->request->addParams(Router::parse($url));
+        $this->Auth->request->env('REQUEST_METHOD', 'POST');
+        $this->Auth->request->url = $this->Auth->request->here = Router::normalize($url);
+        $this->Auth->config('loginAction', ['controller' => 'AuthTest', 'action' => 'login']);
+        $event = new Event('Controller.startup', $this->Controller);
+        $response = $this->Auth->startup($event);
+
+        $this->assertInstanceOf('Cake\Network\Response', $response);
+        $expected = Router::url(['controller' => 'AuthTest', 'action' => 'login'], true);
+        $redirectHeader = $response->header()['Location'];
+        $this->assertEquals($expected, $redirectHeader);
+    }
+
+    /**
      * @return void
      */
     public function testLoginRedirectQueryString()
@@ -703,6 +752,7 @@ class AuthComponentTest extends TestCase
 
         $url = '/posts/add';
         $this->Auth->request = $this->Controller->request = new Request($url);
+        $this->Auth->request->env('REQUEST_METHOD', 'GET');
         $this->Auth->request->addParams(Router::parse($url));
         $this->Auth->request->url = Router::normalize($url);