mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #858 from nextcloud/stable10-when-logged-in-then-just-redirect-to-redirected-page 

[stable10] when logged in then just redirect to redirected page
This commit is contained in:
Lukas Reschke 2016-08-16 18:13:24 +02:00 committed by GitHub
parent 9250ffdaa1 9ac6b83687
commit 737591f239
2 changed files with 133 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
core/Controller/LoginController.php
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 * @PublicPage
* @UseSession * @UseSession
* @NoCSRFRequired
* *
* @param string $user * @param string $user
* @param string $password * @param string $password
@ -184,6 +201,13 @@ class LoginController extends Controller {
$currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress()); $currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress());
$this->throttler->sleepDelay($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; $originalUser = $user;
// TODO: Add all the insane error handling // TODO: Add all the insane error handling
/* @var $loginResult IUser */ /* @var $loginResult IUser */
@ -223,15 +247,7 @@ class LoginController extends Controller {
return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge')); return new RedirectResponse($this->urlGenerator->linkToRoute('core.TwoFactorChallenge.selectChallenge'));
} }
if (!is_null($redirect_url) && $this->userSession->isLoggedIn()) { return $this->generateRedirect($redirect_url);
$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());
} }
} }

121
tests/Core/Controller/LoginControllerTest.php
View File

@ -57,14 +57,14 @@ class LoginControllerTest extends TestCase {
public function setUp() { public function setUp() {
parent::setUp(); parent::setUp();
$this->request = $this->getMock('\\OCP\\IRequest'); $this->request = $this->createMock('\\OCP\\IRequest');
$this->userManager = $this->getMock('\\OCP\\IUserManager'); $this->userManager = $this->createMock('\\OCP\\IUserManager');
$this->config = $this->getMock('\\OCP\\IConfig'); $this->config = $this->createMock('\\OCP\\IConfig');
$this->session = $this->getMock('\\OCP\\ISession'); $this->session = $this->createMock('\\OCP\\ISession');
$this->userSession = $this->getMockBuilder('\\OC\\User\\Session') $this->userSession = $this->getMockBuilder('\\OC\\User\\Session')
->disableOriginalConstructor() ->disableOriginalConstructor()
->getMock(); ->getMock();
$this->urlGenerator = $this->getMock('\\OCP\\IURLGenerator'); $this->urlGenerator = $this->createMock('\\OCP\\IURLGenerator');
$this->twoFactorManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager') $this->twoFactorManager = $this->getMockBuilder('\OC\Authentication\TwoFactorAuth\Manager')
->disableOriginalConstructor() ->disableOriginalConstructor()
->getMock(); ->getMock();
@ -110,7 +110,7 @@ class LoginControllerTest extends TestCase {
->method('getCookie') ->method('getCookie')
->with('oc_token') ->with('oc_token')
->willReturn('MyLoginToken'); ->willReturn('MyLoginToken');
$user = $this->getMock('\\OCP\\IUser'); $user = $this->createMock('\\OCP\\IUser');
$user $user
->expects($this->once()) ->expects($this->once())
->method('getUID') ->method('getUID')
@ -218,7 +218,7 @@ class LoginControllerTest extends TestCase {
->method('getSystemValue') ->method('getSystemValue')
->with('lost_password_link') ->with('lost_password_link')
->willReturn(false); ->willReturn(false);
$user = $this->getMock('\\OCP\\IUser'); $user = $this->createMock('\\OCP\\IUser');
$user $user
->expects($this->once()) ->expects($this->once())
->method('canChangePassword') ->method('canChangePassword')
@ -257,7 +257,7 @@ class LoginControllerTest extends TestCase {
->method('getSystemValue') ->method('getSystemValue')
->with('lost_password_link') ->with('lost_password_link')
->willReturn(false); ->willReturn(false);
$user = $this->getMock('\\OCP\\IUser'); $user = $this->createMock('\\OCP\\IUser');
$user $user
->expects($this->once()) ->expects($this->once())
->method('canChangePassword') ->method('canChangePassword')
@ -295,6 +295,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(4)) ->expects($this->exactly(4))
->method('getRemoteAddress') ->method('getRemoteAddress')
->willReturn('192.168.0.1'); ->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler $this->throttler
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('sleepDelay') ->method('sleepDelay')
@ -325,7 +329,7 @@ class LoginControllerTest extends TestCase {
public function testLoginWithValidCredentials() { public function testLoginWithValidCredentials() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */ /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMock('\OCP\IUser'); $user = $this->createMock('\OCP\IUser');
$password = 'secret'; $password = 'secret';
$indexPageUrl = \OC_Util::getDefaultPageUrl(); $indexPageUrl = \OC_Util::getDefaultPageUrl();
@ -333,6 +337,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('getRemoteAddress') ->method('getRemoteAddress')
->willReturn('192.168.0.1'); ->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler $this->throttler
->expects($this->once()) ->expects($this->once())
->method('sleepDelay') ->method('sleepDelay')
@ -360,9 +368,44 @@ class LoginControllerTest extends TestCase {
$this->assertEquals($expected, $this->loginController->tryLogin($user, $password, null)); $this->assertEquals($expected, $this->loginController->tryLogin($user, $password, null));
} }
public function testLoginWithValidCredentialsAndRedirectUrl() { public function testLoginWithoutPassedCsrfCheckAndNotLoggedIn() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */ /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMock('\OCP\IUser'); $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()) $user->expects($this->any())
->method('getUID') ->method('getUID')
->will($this->returnValue('jane')); ->will($this->returnValue('jane'));
@ -374,6 +417,50 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('getRemoteAddress') ->method('getRemoteAddress')
->willReturn('192.168.0.1'); ->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->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(true);
$this->throttler $this->throttler
->expects($this->once()) ->expects($this->once())
->method('sleepDelay') ->method('sleepDelay')
@ -405,7 +492,7 @@ class LoginControllerTest extends TestCase {
public function testLoginWithTwoFactorEnforced() { public function testLoginWithTwoFactorEnforced() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */ /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMock('\OCP\IUser'); $user = $this->createMock('\OCP\IUser');
$user->expects($this->any()) $user->expects($this->any())
->method('getUID') ->method('getUID')
->will($this->returnValue('john')); ->will($this->returnValue('john'));
@ -416,6 +503,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(2)) ->expects($this->exactly(2))
->method('getRemoteAddress') ->method('getRemoteAddress')
->willReturn('192.168.0.1'); ->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler $this->throttler
->expects($this->once()) ->expects($this->once())
->method('sleepDelay') ->method('sleepDelay')
@ -452,7 +543,7 @@ class LoginControllerTest extends TestCase {
public function testToNotLeakLoginName() { public function testToNotLeakLoginName() {
/** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */ /** @var IUser | \PHPUnit_Framework_MockObject_MockObject $user */
$user = $this->getMock('\OCP\IUser'); $user = $this->createMock('\OCP\IUser');
$user->expects($this->any()) $user->expects($this->any())
->method('getUID') ->method('getUID')
->will($this->returnValue('john')); ->will($this->returnValue('john'));
@ -478,6 +569,10 @@ class LoginControllerTest extends TestCase {
->expects($this->exactly(3)) ->expects($this->exactly(3))
->method('getRemoteAddress') ->method('getRemoteAddress')
->willReturn('192.168.0.1'); ->willReturn('192.168.0.1');
$this->request
->expects($this->once())
->method('passesCSRFCheck')
->willReturn(true);
$this->throttler $this->throttler
->expects($this->once()) ->expects($this->once())
->method('getDelay') ->method('getDelay')