Redirect users when already logged-in on login form

This commit is contained in:
Lukas Reschke 2016-08-09 19:01:50 +02:00 committed by Jan-Christoph Borchardt
parent 3b27a2a412
commit be1e64486f
2 changed files with 120 additions and 9 deletions

View File

@ -171,9 +171,26 @@ class LoginController extends Controller {
);
}
/**
* @param string $redirectUrl
* @return RedirectResponse
*/
private function generateRedirect($redirectUrl) {
if (!is_null($redirectUrl) && $this->userSession->isLoggedIn()) {
$location = $this->urlGenerator->getAbsoluteURL(urldecode($redirectUrl));
// Deny the redirect if the URL contains a @
// This prevents unvalidated redirects like ?redirect_url=:user@domain.com
if (strpos($location, '@') === false) {
return new RedirectResponse($location);
}
}
return new RedirectResponse(OC_Util::getDefaultPageUrl());
}
/**
* @PublicPage
* @UseSession
* @NoCSRFRequired
*
* @param string $user
* @param string $password
@ -184,6 +201,13 @@ class LoginController extends Controller {
$currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress());
$this->throttler->sleepDelay($this->request->getRemoteAddress());
// If the user is already logged in and the CSRF check does not pass then
// simply redirect the user to the correct page as required. This is the
// case when an user has already logged-in, in another tab.
if(!$this->request->passesCSRFCheck()) {
return $this->generateRedirect($redirect_url);
}
$originalUser = $user;
// TODO: Add all the insane error handling
/* @var $loginResult IUser */
@ -223,15 +247,7 @@ class LoginController extends Controller {
return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge'));
}
if (!is_null($redirect_url) && $this->userSession->isLoggedIn()) {
$location = $this->urlGenerator->getAbsoluteURL(urldecode($redirect_url));
// Deny the redirect if the URL contains a @
// This prevents unvalidated redirects like ?redirect_url=:user@domain.com
if (strpos($location, '@') === false) {
return new RedirectResponse($location);
}
}
return new RedirectResponse(OC_Util::getDefaultPageUrl());
return $this->generateRedirect($redirect_url);
}
}

View File

@ -292,6 +292,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(4))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->exactly(2))
->method('sleepDelay')
@ -330,6 +334,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
@ -361,6 +369,81 @@ class LoginControllerTest extends TestCase {
$this->assertEquals($expected, $this->loginController->tryLogin($user, $password, null));
}
public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock('\OCP\IUser');
$user->expects($this->any())
->method('getUID')
->will($this->returnValue('jane'));
$password = 'secret';
$originalUrl = 'another%20url';
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(false);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userSession->expects($this->once())
->method('isLoggedIn')
->with()
->will($this->returnValue(false));
$expected = new \OCP\AppFramework\Http\RedirectResponse(\OC_Util::getDefaultPageUrl());
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
}
public function testLoginWithoutPassedCsrfCheckAndLoggedIn() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->createMock('\OCP\IUser');
$user->expects($this->any())
->method('getUID')
->will($this->returnValue('jane'));
$password = 'secret';
$originalUrl = 'another%20url';
$redirectUrl = 'http://localhost/another url';
$this->request
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(false);
$this->throttler
->expects($this->once())
->method('sleepDelay')
->with('192.168.0.1');
$this->throttler
->expects($this->once())
->method('getDelay')
->with('192.168.0.1')
->willReturn(200);
$this->userSession->expects($this->once())
->method('isLoggedIn')
->with()
->will($this->returnValue(true));
$this->urlGenerator->expects($this->once())
->method('getAbsoluteURL')
->with(urldecode($originalUrl))
->will($this->returnValue($redirectUrl));
$expected = new \OCP\AppFramework\Http\RedirectResponse($redirectUrl);
$this->assertEquals($expected, $this->loginController->tryLogin('Jane', $password, $originalUrl));
}
public function testLoginWithValidCredentialsAndRedirectUrl() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMock('\OCP\IUser');
@ -375,6 +458,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
@ -417,6 +504,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(2))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('sleepDelay')
@ -479,6 +570,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(3))
->method('getRemoteAddress')
->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler
->expects($this->once())
->method('getDelay')