Browse files

Fixed infinite redirects for authenticated users accessing login page.

  • Loading branch information...
1 parent af36813 commit 4dbf9107a89ce56180eb1226d2696e1aaa23ac30 @ADmad ADmad committed Sep 24, 2013
View
24 lib/Cake/Controller/Component/AuthComponent.php
@@ -304,7 +304,10 @@ public function startup(Controller $controller) {
return $this->_unauthenticated($controller);
}
- if (empty($this->authorize) || $this->isAuthorized($this->user())) {
+ if ($this->_isLoginAction($controller) ||
+ empty($this->authorize) ||
+ $this->isAuthorized($this->user())
+ ) {
return true;
}
@@ -347,6 +350,11 @@ protected function _unauthenticated(Controller $controller) {
}
if ($this->_isLoginAction($controller)) {
+ if (empty($controller->request->data)) {
+ if (!$this->Session->check('Auth.redirect') && env('HTTP_REFERER')) {
+ $this->Session->write('Auth.redirect', $controller->referer(null, true));

What is the purpose of this code? Looks like it renders the loginRedirect setting useless because it always sets Auth.redirect with the referer URL.
This makes it impossible to have a link to the login form that then automatically redirects to the default page set via loginRedirect.

@dereuromark
CakePHP member

As from what I understand or think it should do in such a case, the loginRedirect is always only a fallback.
So if there is no referrer to jump to (user previously tried to access that page) it then uses loginRedirect. Not right away - that would be very annoying to always end up there when you actually wanted to go somewhere else.

@ADmad
I do, however, see a slight problem with && env('HTTP_REFERER').
This could return true, but $controller->referer(null, true) could then decide this is not a valid internal URL and returns the / - so maybe the first env() check should be removed and the result of the referer() call should be checked upon at that point to avoid running into this issue?

I see I haven't dug deep enough and this code has been present earlier (also, I'm not 100% sure this is causing the problem) so I have created the issue #3748. It should be a better place to discuss it than here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ }
return true;
}
@@ -367,9 +375,7 @@ protected function _unauthenticated(Controller $controller) {
}
/**
- * Normalizes $loginAction and checks if current request url is same as login
- * action. If current url is same as login action, referrer url is saved in session
- * which is later accessible using redirectUrl().
+ * Normalizes $loginAction and checks if current request url is same as login action.
*
* @param Controller $controller A reference to the controller object.
* @return boolean True if current action is login action else false.
@@ -382,15 +388,7 @@ protected function _isLoginAction(Controller $controller) {
$url = Router::normalize($url);
$loginAction = Router::normalize($this->loginAction);
- if ($loginAction == $url) {
- if (empty($controller->request->data)) {
- if (!$this->Session->check('Auth.redirect') && env('HTTP_REFERER')) {
- $this->Session->write('Auth.redirect', $controller->referer(null, true));
- }
- }
- return true;
- }
- return false;
+ return $loginAction === $url;
}
/**
View
22 lib/Cake/Test/Case/Controller/Component/AuthComponentTest.php
@@ -877,6 +877,28 @@ public function testLoginRedirect() {
}
/**
+ * testNoLoginRedirectForAuthenticatedUser method
+ *
+ * @return void
+ */
+ public function testNoLoginRedirectForAuthenticatedUser() {
+ $this->Controller->request['controller'] = 'auth_test';
+ $this->Controller->request['action'] = 'login';
+ $this->Controller->here = '/auth_test/login';
+ $this->Auth->request->url = 'auth_test/login';
+
+ $this->Auth->Session->write('Auth.User.id', '1');
+ $this->Auth->authenticate = array('Form');
+ $this->getMock('BaseAuthorize', array('authorize'), array(), 'NoLoginRedirectMockAuthorize', false);
+ $this->Auth->authorize = array('NoLoginRedirectMockAuthorize');
+ $this->Auth->loginAction = array('controller' => 'auth_test', 'action' => 'login');
+
+ $return = $this->Auth->startup($this->Controller);
+ $this->assertTrue($return);
+ $this->assertNull($this->Controller->testUrl);
+ }
+
+/**
* Default to loginRedirect, if set, on authError.
*
* @return void

0 comments on commit 4dbf910

Please sign in to comment.