Merge pull request #4968 from owncloud/user_checkpwd

User: move checkPassword from User to Manager to not break API

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
 	 */

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;
 	}
 
 	/**

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;
 	}
 
 	/**

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
 	 *

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;

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
 	 *

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

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');

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