From 0486d750aade407739977911cc1aab40e65dc460 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Mon, 9 May 2016 15:33:56 +0200 Subject: [PATCH] use the UID for creating the session token, not the login name --- apps/dav/lib/connector/sabre/auth.php | 8 ++++---- apps/dav/tests/unit/connector/sabre/auth.php | 13 +++++++++--- core/Controller/LoginController.php | 8 ++++---- lib/private/User/Session.php | 1 + tests/core/controller/LoginControllerTest.php | 20 +++++++++++-------- 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/apps/dav/lib/connector/sabre/auth.php b/apps/dav/lib/connector/sabre/auth.php index fdc074f5cf7..b8047e779f5 100644 --- a/apps/dav/lib/connector/sabre/auth.php +++ b/apps/dav/lib/connector/sabre/auth.php @@ -33,7 +33,7 @@ use Exception; use OC\AppFramework\Http\Request; use OCP\IRequest; use OCP\ISession; -use OCP\IUserSession; +use OC\User\Session; use Sabre\DAV\Auth\Backend\AbstractBasic; use Sabre\DAV\Exception\NotAuthenticated; use Sabre\DAV\Exception\ServiceUnavailable; @@ -45,7 +45,7 @@ class Auth extends AbstractBasic { /** @var ISession */ private $session; - /** @var IUserSession */ + /** @var Session */ private $userSession; /** @var IRequest */ private $request; @@ -54,12 +54,12 @@ class Auth extends AbstractBasic { /** * @param ISession $session - * @param IUserSession $userSession + * @param Session $userSession * @param IRequest $request * @param string $principalPrefix */ public function __construct(ISession $session, - IUserSession $userSession, + Session $userSession, IRequest $request, $principalPrefix = 'principals/users/') { $this->session = $session; diff --git a/apps/dav/tests/unit/connector/sabre/auth.php b/apps/dav/tests/unit/connector/sabre/auth.php index b81a5e003b5..a0168e435e2 100644 --- a/apps/dav/tests/unit/connector/sabre/auth.php +++ b/apps/dav/tests/unit/connector/sabre/auth.php @@ -28,7 +28,7 @@ use OCP\IRequest; use OCP\IUser; use Test\TestCase; use OCP\ISession; -use OCP\IUserSession; +use OC\User\Session; /** * Class Auth @@ -41,7 +41,7 @@ class Auth extends TestCase { private $session; /** @var \OCA\DAV\Connector\Sabre\Auth */ private $auth; - /** @var IUserSession */ + /** @var Session */ private $userSession; /** @var IRequest */ private $request; @@ -50,7 +50,7 @@ class Auth extends TestCase { parent::setUp(); $this->session = $this->getMockBuilder('\OCP\ISession') ->disableOriginalConstructor()->getMock(); - $this->userSession = $this->getMockBuilder('\OCP\IUserSession') + $this->userSession = $this->getMockBuilder('\OC\User\Session') ->disableOriginalConstructor()->getMock(); $this->request = $this->getMockBuilder('\OCP\IRequest') ->disableOriginalConstructor()->getMock(); @@ -170,6 +170,10 @@ class Auth extends TestCase { ->method('login') ->with('MyTestUser', 'MyTestPassword') ->will($this->returnValue(true)); + $this->userSession + ->expects($this->once()) + ->method('createSessionToken') + ->with($this->request, 'MyTestUser', 'MyTestPassword'); $this->session ->expects($this->once()) ->method('set') @@ -559,6 +563,9 @@ class Auth extends TestCase { ->method('login') ->with('username', 'password') ->will($this->returnValue(true)); + $this->userSession + ->expects($this->once()) + ->method('createSessionToken'); $user = $this->getMockBuilder('\OCP\IUser') ->disableOriginalConstructor() ->getMock(); diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index d30ec88bd45..6985e2be87e 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -167,8 +167,8 @@ class LoginController extends Controller { */ public function tryLogin($user, $password, $redirect_url) { // TODO: Add all the insane error handling - $loginResult = $this->userManager->checkPassword($user, $password) !== false; - if (!$loginResult) { + $loginResult = $this->userManager->checkPassword($user, $password); + if ($loginResult === false) { $users = $this->userManager->getByEmail($user); // we only allow login by email if unique if (count($users) === 1) { @@ -176,7 +176,7 @@ class LoginController extends Controller { $loginResult = $this->userManager->checkPassword($user, $password); } } - if (!$loginResult) { + if ($loginResult === false) { $this->session->set('loginMessages', [ [], ['invalidpassword'] @@ -185,7 +185,7 @@ class LoginController extends Controller { $args = !is_null($user) ? ['user' => $user] : []; return new RedirectResponse($this->urlGenerator->linkToRoute('core.login.showLoginForm', $args)); } - $this->userSession->createSessionToken($this->request, $user, $password); + $this->userSession->createSessionToken($this->request, $loginResult->getUID(), $password); if (!is_null($redirect_url) && $this->userSession->isLoggedIn()) { $location = $this->urlGenerator->getAbsoluteURL(urldecode($redirect_url)); // Deny the redirect if the URL contains a @ diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 0b0d298db24..c9f42d7e414 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -454,6 +454,7 @@ class Session implements IUserSession, Emitter { * Tries to login the user with auth token header * * @todo check remember me cookie + * @return boolean */ public function tryTokenLogin(IRequest $request) { $authHeader = $request->getHeader('Authorization'); diff --git a/tests/core/controller/LoginControllerTest.php b/tests/core/controller/LoginControllerTest.php index 3c94fd61c6b..0b5a143f4e3 100644 --- a/tests/core/controller/LoginControllerTest.php +++ b/tests/core/controller/LoginControllerTest.php @@ -268,7 +268,7 @@ class LoginControllerTest extends TestCase { } public function testLoginWithInvalidCredentials() { - $user = 'jane'; + $user = $this->getMock('\OCP\IUser'); $password = 'secret'; $loginPageUrl = 'some url'; @@ -288,16 +288,16 @@ class LoginControllerTest extends TestCase { } public function testLoginWithValidCredentials() { - $user = 'jane'; + $user = $this->getMock('\OCP\IUser'); $password = 'secret'; $indexPageUrl = 'some url'; $this->userManager->expects($this->once()) ->method('checkPassword') - ->will($this->returnValue(true)); + ->will($this->returnValue($user)); $this->userSession->expects($this->once()) ->method('createSessionToken') - ->with($this->request, $user, $password); + ->with($this->request, $user->getUID(), $password); $this->urlGenerator->expects($this->once()) ->method('linkTo') ->with('files', 'index') @@ -308,17 +308,21 @@ class LoginControllerTest extends TestCase { } public function testLoginWithValidCredentialsAndRedirectUrl() { - $user = 'jane'; + $user = $this->getMock('\OCP\IUser'); + $user->expects($this->any()) + ->method('getUID') + ->will($this->returnValue('jane')); $password = 'secret'; $originalUrl = 'another%20url'; $redirectUrl = 'http://localhost/another url'; $this->userManager->expects($this->once()) ->method('checkPassword') - ->will($this->returnValue(true)); + ->with('jane', $password) + ->will($this->returnValue($user)); $this->userSession->expects($this->once()) ->method('createSessionToken') - ->with($this->request, $user, $password); + ->with($this->request, $user->getUID(), $password); $this->userSession->expects($this->once()) ->method('isLoggedIn') ->with() @@ -329,7 +333,7 @@ class LoginControllerTest extends TestCase { ->will($this->returnValue($redirectUrl)); $expected = new \OCP\AppFramework\Http\RedirectResponse(urldecode($redirectUrl)); - $this->assertEquals($expected, $this->loginController->tryLogin($user, $password, $originalUrl)); + $this->assertEquals($expected, $this->loginController->tryLogin($user->getUID(), $password, $originalUrl)); } }