Fix autoPostRedirect()

Author: Mark Scherer

src/Controller/Component/CommonComponent.php

@@ -155,10 +155,10 @@ public function currentUrl($asString = false) {
 	 *
 	 * @param mixed $whereTo URL
 	 * @param bool $allowSelf if redirect to the same controller/action (url) is allowed
-	 * @param int|null $status
+	 * @param int $status
 	 * @return \Cake\Network\Response
 	 */
-	public function autoRedirect($whereTo, $allowSelf = false, $status = null) {
+	public function autoRedirect($whereTo, $allowSelf = false, $status = 302) {
 		if ($allowSelf || $this->Controller->referer(null, true) !== '/' . $this->Controller->request->url) {
 			return $this->Controller->redirect($this->Controller->referer($whereTo, true), $status);
 		}
@@ -200,7 +200,7 @@ public function autoPostRedirect($whereTo, $conditionalAutoRedirect = true, $sta
 			$referer = Router::parse($referer);
 		}
 
-		if (!$conditionalAutoRedirect || empty($this->Controller->autoRedirectActions) || is_array($referer) && !empty($referer['action'])) {
+		if ($conditionalAutoRedirect && !empty($this->Controller->autoRedirectActions) && is_array($referer) && !empty($referer['action'])) {
 			// Be sure that controller offset exists, otherwise you
 			// will run into problems, if you use url rewriting.
 			$refererController = null;
@@ -211,12 +211,13 @@ public function autoPostRedirect($whereTo, $conditionalAutoRedirect = true, $sta
 			if (!isset($this->Controller->autoRedirectActions)) {
 				$this->Controller->autoRedirectActions = [];
 			}
+
 			foreach ($this->Controller->autoRedirectActions as $action) {
 				list($controller, $action) = pluginSplit($action);
-				if (!empty($controller) && $refererController !== '*' && $refererController != $controller) {
+				if (!empty($controller) && $refererController !== '*' && $refererController !== $controller) {
 					continue;
 				}
-				if (empty($controller) && $refererController != $this->Controller->request->params['controller']) {
+				if (empty($controller) && $refererController !== $this->Controller->request->params['controller']) {
 					continue;
 				}
 				if (!in_array($referer['action'], $this->Controller->autoRedirectActions, true)) {
@@ -229,15 +230,15 @@ public function autoPostRedirect($whereTo, $conditionalAutoRedirect = true, $sta
 	}
 
 	/**
-	 * Automatically add missing url parts of the current url including
+	 * Automatically add missing URL parts of the current URL including
 	 * - querystring (especially for 3.x then)
 	 * - passed params
 	 *
 	 * @param mixed|null $url
 	 * @param int|null $status
 	 * @return \Cake\Network\Response
 	 */
-	public function completeRedirect($url = null, $status = null) {
+	public function completeRedirect($url = null, $status = 302) {
 		if ($url === null) {
 			$url = $this->Controller->request->params;
 			unset($url['pass']);

tests/TestCase/Controller/Component/CommonComponentTest.php

@@ -12,17 +12,25 @@
  */
 class CommonComponentTest extends TestCase {
 
-	//public $fixtures = array('core.sessions', 'plugin.tools.tools_users', 'plugin.tools.roles');
-
+	/**
+	 * @return void
+	 */
 	public function setUp() {
 		parent::setUp();
 
 		Configure::write('App.namespace', 'TestApp');
+		Configure::write('App.fullBaseUrl', 'http://localhost');
 
-		$this->Controller = new CommonComponentTestController(new Request('/test'));
+		$this->request = new Request('/my_controller/foo');
+		$this->request->params['controller'] = 'MyController';
+		$this->request->params['action'] = 'foo';
+		$this->Controller = new CommonComponentTestController($this->request);
 		$this->Controller->startupProcess();
 	}
 
+	/**
+	 * @return void
+	 */
 	public function tearDown() {
 		parent::tearDown();
 
@@ -31,8 +39,6 @@ public function tearDown() {
 	}
 
 	/**
-	 * CommonComponentTest::testLoadComponent()
-	 *
 	 * @return void
 	 */
 	public function testLoadComponent() {
@@ -70,8 +76,6 @@ public function testLoadComponent() {
 	}
 
 	/**
-	 * CommonComponentTest::testGetParams()
-	 *
 	 * @return void
 	 */
 	public function testGetParams() {
@@ -83,8 +87,6 @@ public function testGetParams() {
 	}
 
 	/**
-	 * CommonComponentTest::testGetDefaultUrlParams()
-	 *
 	 * @return void
 	 */
 	public function testGetDefaultUrlParams() {
@@ -106,8 +108,6 @@ public function testCurrentUrl() {
 	}
 
 	/**
-	 * CommonComponentTest::testIsForeignReferer()
-	 *
 	 * @return void
 	 */
 	public function testIsForeignReferer() {
@@ -125,39 +125,69 @@ public function testIsForeignReferer() {
 	}
 
 	/**
-	 * CommonComponentTest::testAutoRedirect()
-	 *
 	 * @return void
 	 */
 	public function testPostRedirect() {
 		$is = $this->Controller->Common->postRedirect(['action' => 'foo']);
 		$is = $this->Controller->response->header();
-		$this->assertSame('/foo', $is['Location']);
+		$this->assertSame('http://localhost/foo', $is['Location']);
 		$this->assertSame(302, $this->Controller->response->statusCode());
 	}
 
 	/**
-	 * CommonComponentTest::testAutoRedirect()
-	 *
 	 * @return void
 	 */
 	public function testAutoRedirect() {
 		$is = $this->Controller->Common->autoRedirect(['action' => 'foo']);
 		$is = $this->Controller->response->header();
-		$this->assertSame('/foo', $is['Location']);
-		$this->assertSame(200, $this->Controller->response->statusCode());
+		$this->assertSame('http://localhost/foo', $is['Location']);
+		$this->assertSame(302, $this->Controller->response->statusCode());
 	}
 
 	/**
-	 * CommonComponentTest::testAutoRedirect()
-	 *
 	 * @return void
 	 */
 	public function testAutoRedirectReferer() {
+		$this->request->env('HTTP_REFERER', 'http://localhost/my_controller/some-referer-action');
+
 		$is = $this->Controller->Common->autoRedirect(['action' => 'foo'], true);
 		$is = $this->Controller->response->header();
-		$this->assertSame('/foo', $is['Location']);
-		$this->assertSame(200, $this->Controller->response->statusCode());
+		$this->assertSame('http://localhost/my_controller/some-referer-action', $is['Location']);
+		$this->assertSame(302, $this->Controller->response->statusCode());
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testAutoPostRedirect() {
+		$is = $this->Controller->Common->autoPostRedirect(['action' => 'foo'], true);
+		$is = $this->Controller->response->header();
+		$this->assertSame('http://localhost/foo', $is['Location']);
+		$this->assertSame(302, $this->Controller->response->statusCode());
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testAutoPostRedirectReferer() {
+		$this->request->env('HTTP_REFERER', 'http://localhost/my_controller/allowed');
+
+		$is = $this->Controller->Common->autoPostRedirect(['controller' => 'MyController', 'action' => 'foo'], true);
+		$is = $this->Controller->response->header();
+		$this->assertSame('http://localhost/my_controller/allowed', $is['Location']);
+		$this->assertSame(302, $this->Controller->response->statusCode());
+	}
+
+	/**
+	 * @return void
+	 */
+	public function testAutoPostRedirectRefererNotWhitelisted() {
+		$this->request->env('HTTP_REFERER', 'http://localhost/my_controller/wrong');
+
+		$is = $this->Controller->Common->autoPostRedirect(['controller' => 'MyController', 'action' => 'foo'], true);
+		$is = $this->Controller->response->header();
+		$this->assertSame('http://localhost/my_controller/foo', $is['Location']);
+		$this->assertSame(302, $this->Controller->response->statusCode());
 	}
 
 }
@@ -167,9 +197,16 @@ public function testAutoRedirectReferer() {
  */
 class CommonComponentTestController extends Controller {
 
+	public $name = 'MyController';
+
 	/**
 	 * @var array
 	 */
 	public $components = ['Tools.Common'];
 
+	/**
+	 * @var array
+	 */
+	public $autoRedirectActions = ['allowed'];
+
 }