diff --git a/lib/public/user.php b/lib/public/user.php index 23ff991642d..576a64d7048 100644 --- a/lib/public/user.php +++ b/lib/public/user.php @@ -102,7 +102,7 @@ class User { * @brief Check if the password is correct * @param $uid The username * @param $password The password - * @returns true/false + * @returns mixed username on success, false otherwise * * Check if the password is correct without logging in the user */ diff --git a/lib/user.php b/lib/user.php index 0f6f40aec9a..da774ff86f0 100644 --- a/lib/user.php +++ b/lib/user.php @@ -410,22 +410,18 @@ class OC_User { * @brief Check if the password is correct * @param string $uid The username * @param string $password The password - * @return bool + * @return mixed user id a string on success, false otherwise * * Check if the password is correct without logging in the user * returns the user id or false */ public static function checkPassword($uid, $password) { - $user = self::getManager()->get($uid); - if ($user) { - if ($user->checkPassword($password)) { - return $user->getUID(); - } else { - return false; - } - } else { - return false; + $manager = self::getManager(); + $username = $manager->checkPassword($uid, $password); + if ($username !== false) { + return $manager->get($username)->getUID(); } + return false; } /** diff --git a/lib/user/http.php b/lib/user/http.php index 1e044ed4188..e99afe59ba7 100644 --- a/lib/user/http.php +++ b/lib/user/http.php @@ -79,7 +79,11 @@ class OC_User_HTTP extends OC_User_Backend { curl_close($ch); - return $status==200; + if($status === 200) { + return $uid; + } + + return false; } /** diff --git a/lib/user/manager.php b/lib/user/manager.php index 8dc9bfe2729..13286bc28a4 100644 --- a/lib/user/manager.php +++ b/lib/user/manager.php @@ -118,6 +118,25 @@ class Manager extends PublicEmitter { return ($user !== null); } + /** + * Check if the password is valid for the user + * + * @param $loginname + * @param $password + * @return mixed the User object on success, false otherwise + */ + public function checkPassword($loginname, $password) { + foreach ($this->backends as $backend) { + if($backend->implementsActions(\OC_USER_BACKEND_CHECK_PASSWORD)) { + $uid = $backend->checkPassword($loginname, $password); + if ($uid !== false) { + return $this->getUserObject($uid, $backend); + } + } + } + return false; + } + /** * search by user id * diff --git a/lib/user/session.php b/lib/user/session.php index 9a6c669e935..b5e9385234d 100644 --- a/lib/user/session.php +++ b/lib/user/session.php @@ -121,15 +121,16 @@ class Session implements Emitter { */ public function login($uid, $password) { $this->manager->emit('\OC\User', 'preLogin', array($uid, $password)); - $user = $this->manager->get($uid); - if ($user) { - $result = $user->checkPassword($password); - if ($result and $user->isEnabled()) { - $this->setUser($user); - $this->manager->emit('\OC\User', 'postLogin', array($user, $password)); - return true; - } else { - return false; + $user = $this->manager->checkPassword($uid, $password); + if($user !== false) { + if (!is_null($user)) { + if ($user->isEnabled()) { + $this->setUser($user); + $this->manager->emit('\OC\User', 'postLogin', array($user, $password)); + return true; + } else { + return false; + } } } else { return false; diff --git a/lib/user/user.php b/lib/user/user.php index 8115c43198c..e5f842944f1 100644 --- a/lib/user/user.php +++ b/lib/user/user.php @@ -105,24 +105,6 @@ class User { return !($result === false); } - /** - * Check if the password is valid for the user - * - * @param $password - * @return bool - */ - public function checkPassword($password) { - if ($this->backend->implementsActions(\OC_USER_BACKEND_CHECK_PASSWORD)) { - $result = $this->backend->checkPassword($this->uid, $password); - if ($result !== false) { - $this->uid = $result; - } - return !($result === false); - } else { - return false; - } - } - /** * Set the password of the user * diff --git a/tests/lib/user/manager.php b/tests/lib/user/manager.php index bc49f6db4b2..00901dd4115 100644 --- a/tests/lib/user/manager.php +++ b/tests/lib/user/manager.php @@ -98,6 +98,51 @@ class Manager extends \PHPUnit_Framework_TestCase { $this->assertTrue($manager->userExists('foo')); } + public function testCheckPassword() { + /** + * @var \OC_User_Backend | \PHPUnit_Framework_MockObject_MockObject $backend + */ + $backend = $this->getMock('\OC_User_Dummy'); + $backend->expects($this->once()) + ->method('checkPassword') + ->with($this->equalTo('foo'), $this->equalTo('bar')) + ->will($this->returnValue(true)); + + $backend->expects($this->any()) + ->method('implementsActions') + ->will($this->returnCallback(function ($actions) { + if ($actions === \OC_USER_BACKEND_CHECK_PASSWORD) { + return true; + } else { + return false; + } + })); + + $manager = new \OC\User\Manager(); + $manager->registerBackend($backend); + + $user = $manager->checkPassword('foo', 'bar'); + $this->assertTrue($user instanceof \OC\User\User); + } + + public function testCheckPasswordNotSupported() { + /** + * @var \OC_User_Backend | \PHPUnit_Framework_MockObject_MockObject $backend + */ + $backend = $this->getMock('\OC_User_Dummy'); + $backend->expects($this->never()) + ->method('checkPassword'); + + $backend->expects($this->any()) + ->method('implementsActions') + ->will($this->returnValue(false)); + + $manager = new \OC\User\Manager(); + $manager->registerBackend($backend); + + $this->assertFalse($manager->checkPassword('foo', 'bar')); + } + public function testGetOneBackendExists() { /** * @var \OC_User_Dummy | \PHPUnit_Framework_MockObject_MockObject $backend diff --git a/tests/lib/user/session.php b/tests/lib/user/session.php index 274e9e2831e..e457a7bda30 100644 --- a/tests/lib/user/session.php +++ b/tests/lib/user/session.php @@ -61,10 +61,6 @@ class Session extends \PHPUnit_Framework_TestCase { $backend = $this->getMock('OC_User_Dummy'); $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); - $user->expects($this->once()) - ->method('checkPassword') - ->with('bar') - ->will($this->returnValue(true)); $user->expects($this->once()) ->method('isEnabled') ->will($this->returnValue(true)); @@ -73,8 +69,8 @@ class Session extends \PHPUnit_Framework_TestCase { ->will($this->returnValue('foo')); $manager->expects($this->once()) - ->method('get') - ->with('foo') + ->method('checkPassword') + ->with('foo', 'bar') ->will($this->returnValue($user)); $userSession = new \OC\User\Session($manager, $session); @@ -92,17 +88,13 @@ class Session extends \PHPUnit_Framework_TestCase { $backend = $this->getMock('OC_User_Dummy'); $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); - $user->expects($this->once()) - ->method('checkPassword') - ->with('bar') - ->will($this->returnValue(true)); $user->expects($this->once()) ->method('isEnabled') ->will($this->returnValue(false)); $manager->expects($this->once()) - ->method('get') - ->with('foo') + ->method('checkPassword') + ->with('foo', 'bar') ->will($this->returnValue($user)); $userSession = new \OC\User\Session($manager, $session); @@ -119,17 +111,13 @@ class Session extends \PHPUnit_Framework_TestCase { $backend = $this->getMock('OC_User_Dummy'); $user = $this->getMock('\OC\User\User', array(), array('foo', $backend)); - $user->expects($this->once()) - ->method('checkPassword') - ->with('bar') - ->will($this->returnValue(false)); $user->expects($this->never()) ->method('isEnabled'); $manager->expects($this->once()) - ->method('get') - ->with('foo') - ->will($this->returnValue($user)); + ->method('checkPassword') + ->with('foo', 'bar') + ->will($this->returnValue(false)); $userSession = new \OC\User\Session($manager, $session); $userSession->login('foo', 'bar'); @@ -145,9 +133,9 @@ class Session extends \PHPUnit_Framework_TestCase { $backend = $this->getMock('OC_User_Dummy'); $manager->expects($this->once()) - ->method('get') - ->with('foo') - ->will($this->returnValue(null)); + ->method('checkPassword') + ->with('foo', 'bar') + ->will($this->returnValue(false)); $userSession = new \OC\User\Session($manager, $session); $userSession->login('foo', 'bar'); diff --git a/tests/lib/user/user.php b/tests/lib/user/user.php index b0d170cbfc5..de5ccbf38c1 100644 --- a/tests/lib/user/user.php +++ b/tests/lib/user/user.php @@ -100,46 +100,6 @@ class User extends \PHPUnit_Framework_TestCase { $this->assertTrue($user->delete()); } - public function testCheckPassword() { - /** - * @var \OC_User_Backend | \PHPUnit_Framework_MockObject_MockObject $backend - */ - $backend = $this->getMock('\OC_User_Dummy'); - $backend->expects($this->once()) - ->method('checkPassword') - ->with($this->equalTo('foo'), $this->equalTo('bar')) - ->will($this->returnValue(true)); - - $backend->expects($this->any()) - ->method('implementsActions') - ->will($this->returnCallback(function ($actions) { - if ($actions === \OC_USER_BACKEND_CHECK_PASSWORD) { - return true; - } else { - return false; - } - })); - - $user = new \OC\User\User('foo', $backend); - $this->assertTrue($user->checkPassword('bar')); - } - - public function testCheckPasswordNotSupported() { - /** - * @var \OC_User_Backend | \PHPUnit_Framework_MockObject_MockObject $backend - */ - $backend = $this->getMock('\OC_User_Dummy'); - $backend->expects($this->never()) - ->method('checkPassword'); - - $backend->expects($this->any()) - ->method('implementsActions') - ->will($this->returnValue(false)); - - $user = new \OC\User\User('foo', $backend); - $this->assertFalse($user->checkPassword('bar')); - } - public function testGetHome() { /** * @var \OC_User_Backend | \PHPUnit_Framework_MockObject_MockObject $backend