Permalink
Browse files

Allow AuthComponent::$unauthorizedRedirect to be an url.

Closes #3494
  • Loading branch information...
1 parent e7330fa commit 676872d623753db5f748a696582d7298a9a28914 @ADmad ADmad committed Dec 30, 2012
@@ -215,11 +215,13 @@ class AuthComponent extends Component {
public $authError = null;
/**
- * Controls handling of unauthorized access. By default unauthorized user is
- * redirected to the referrer url or AuthComponent::$loginRedirect or '/'.
- * If set to false a ForbiddenException exception is thrown instead of redirecting.
+ * Controls handling of unauthorized access.
+ * - For default value `true` unauthorized user is redirected to the referrer url
+ * or AuthComponent::$loginRedirect or '/'.
+ * - If set to a string or array the value is used as an url to redirect to.
+ * - If set to false a ForbiddenException exception is thrown instead of redirecting.
*
- * @var boolean
+ * @var mixed
*/
public $unauthorizedRedirect = true;
@@ -345,16 +347,21 @@ public function startup(Controller $controller) {
* @throws ForbiddenException
*/
protected function _unauthorized(Controller $controller) {
- if (!$this->unauthorizedRedirect) {
+ if ($this->unauthorizedRedirect === false) {
throw new ForbiddenException($this->authError);
}
$this->flash($this->authError);
- $default = '/';
- if (!empty($this->loginRedirect)) {
- $default = $this->loginRedirect;
+ if ($this->unauthorizedRedirect === true) {
+ $default = '/';
+ if (!empty($this->loginRedirect)) {
+ $default = $this->loginRedirect;
@SimonEast

SimonEast May 24, 2013

Shouldn't the 2 lines above refer to ->loginAction, not ->loginRedirect...?

@ADmad

ADmad May 24, 2013

Member

No. It's been like this historically.

@SimonEast

SimonEast May 26, 2013

OK, fair enough, it was like that before your edit.

I guess my question is: is it logical to send an not-yet-authorised user to loginRedirect? From the class comments it appears that loginAction is used to typically display a login form, while loginRedirect is where to send a user post-login, such as a member area, dashboard, admin area or whatsoever. Does it make sense to send unauthorised users there?

@markstory

markstory May 26, 2013

Owner

Changing things around with where redirects go often results in more issues being opened, as some new edge case is created. Being sent to loginRedirect will most likely result in another redirect, but the proper session state.

@ADmad

ADmad May 26, 2013

Member

Given the fact that redirection url is customizable using unauthorizedRedirect I don't see any need to change the default redirection and potentially cause more issues.

+ }
+ $url = $controller->referer($default, true);
+ } else {
+ $url = $this->unauthorizedRedirect;
}
- $controller->redirect($controller->referer($default, true), null, true);
+ $controller->redirect($url, null, true);
return false;
}
@@ -908,6 +908,37 @@ public function testDefaultToLoginRedirect() {
}
/**
+ * testRedirectToUnauthorizedRedirect
+ *
+ * @return void
+ */
+ public function testRedirectToUnauthorizedRedirect() {
+ $url = '/party/on';
+ $this->Auth->request = $CakeRequest = new CakeRequest($url);
+ $this->Auth->request->addParams(Router::parse($url));
+ $this->Auth->authorize = array('Controller');
+ $this->Auth->login(array('username' => 'admad', 'password' => 'cake'));
+ $this->Auth->unauthorizedRedirect = array(
+ 'controller' => 'no_can_do', 'action' => 'jack'
+ );
+
+ $CakeResponse = new CakeResponse();
+ $Controller = $this->getMock(
+ 'Controller',
+ array('on', 'redirect'),
+ array($CakeRequest, $CakeResponse)
+ );
+
+ $expected = array(
+ 'controller' => 'no_can_do', 'action' => 'jack'
+ );
+ $Controller->expects($this->once())
+ ->method('redirect')
+ ->with($this->equalTo($expected));
+ $this->Auth->startup($Controller);
+ }
+
+/**
* Throw ForbiddenException if AuthComponent::$unauthorizedRedirect set to false
* @expectedException ForbiddenException
* @return void

0 comments on commit 676872d

Please sign in to comment.