Merge pull request #15489 from owncloud/dont_hide_exceptions_master

Dont hide exceptions master

4 files changed, 218 insertions, 68 deletions

diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php
index 1577d9facb8..b9d83aad684 100644
--- a/apps/user_ldap/lib/connection.php
+++ b/apps/user_ldap/lib/connection.php
@@ -30,8 +30,10 @@
 
 namespace OCA\user_ldap\lib;
 
-//magic properties (incomplete)
+use OC\ServerNotAvailableException;
+
 /**
+ * magic properties (incomplete)
  * responsible for LDAP connections in context with the provided configuration
  *
  * @property string ldapUserFilter
@@ -54,7 +56,7 @@ class Connection extends LDAPUtility {
 	//cache handler
 	protected $cache;
 
-	//settings handler
+	/** @var Configuration settings handler **/
 	protected $configuration;
 
 	protected $doNotValidate = false;
@@ -167,7 +169,8 @@ class Connection extends LDAPUtility {
 			$this->establishConnection();
 		}
 		if(is_null($this->ldapConnectionRes)) {
-			\OCP\Util::writeLog('user_ldap', 'Connection could not be established', \OCP\Util::ERROR);
+			\OCP\Util::writeLog('user_ldap', 'No LDAP Connection to server ' . $this->configuration->ldapHost, \OCP\Util::ERROR);
+			throw new ServerNotAvailableException('Connection to LDAP server could not be established');
 		}
 		return $this->ldapConnectionRes;
 	}
diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php
index c0bc8ea6230..c8c89374e98 100644
--- a/apps/user_ldap/lib/user/manager.php
+++ b/apps/user_ldap/lib/user/manager.php
@@ -149,6 +149,11 @@ class Manager {
 			$this->access->getUserMapper());
 	}
 
+	/**
+	 * @brief returns a User object by it's ownCloud username
+	 * @param string the DN or username of the user
+	 * @return \OCA\user_ldap\lib\user\User|\OCA\user_ldap\lib\user\OfflineUser|null
+	 */
 	protected function createInstancyByUserName($id) {
 		//most likely a uid. Check whether it is a deleted user
 		if($this->isDeletedUser($id)) {
@@ -158,13 +163,14 @@ class Manager {
 		if($dn !== false) {
 			return $this->createAndCache($dn, $id);
 		}
-		throw new \Exception('Could not create User instance');
+		return null;
 	}
 
 	/**
 	 * @brief returns a User object by it's DN or ownCloud username
 	 * @param string the DN or username of the user
 	 * @return \OCA\user_ldap\lib\user\User|\OCA\user_ldap\lib\user\OfflineUser|null
+	 * @throws \Exception when connection could not be established
 	 */
 	public function get($id) {
 		$this->checkAccess();
@@ -181,12 +187,7 @@ class Manager {
 			}
 		}
 
-		try {
-			$user = $this->createInstancyByUserName($id);
-			return $user;
-		} catch (\Exception $e) {
-			return null;
-		}
+		return $this->createInstancyByUserName($id);
 	}
 
 }
diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php
index b9beed1d35a..53229e2d64a 100644
--- a/apps/user_ldap/tests/user_ldap.php
+++ b/apps/user_ldap/tests/user_ldap.php
@@ -417,21 +417,53 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 		$this->prepareMockForUserExists($access);
 
 		$access->expects($this->any())
-			   ->method('readAttribute')
-			   ->will($this->returnCallback(function($dn) {
-					if($dn === 'dnOfRoland,dc=test') {
-						return array();
-					}
-					return false;
-			   }));
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn) {
+				if($dn === 'dnOfRoland,dc=test') {
+					return array();
+				}
+				return false;
+			}));
 
 		//test for existing user
 		$result = $backend->userExists('gunslinger');
 		$this->assertTrue($result);
+	}
+
+	/**
+	 * @expectedException \Exception
+	 */
+	public function testUserExistsForDeleted() {
+		$access = $this->getAccessMock();
+		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
+		$this->prepareMockForUserExists($access);
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn) {
+				if($dn === 'dnOfRoland,dc=test') {
+					return array();
+				}
+				return false;
+			}));
 
 		//test for deleted user
 		$result = $backend->userExists('formerUser');
-		$this->assertFalse($result);
+	}
+
+	public function testUserExistsForNeverExisting() {
+		$access = $this->getAccessMock();
+		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
+		$this->prepareMockForUserExists($access);
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn) {
+				if($dn === 'dnOfRoland,dc=test') {
+					return array();
+				}
+				return false;
+			}));
 
 		//test for never-existing user
 		$result = $backend->userExists('mallory');
@@ -445,21 +477,55 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 		\OC_User::useBackend($backend);
 
 		$access->expects($this->any())
-			   ->method('readAttribute')
-			   ->will($this->returnCallback(function($dn) {
-					if($dn === 'dnOfRoland,dc=test') {
-						return array();
-					}
-					return false;
-			   }));
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn) {
+				if($dn === 'dnOfRoland,dc=test') {
+					return array();
+				}
+				return false;
+			}));
 
 		//test for existing user
 		$result = \OCP\User::userExists('gunslinger');
 		$this->assertTrue($result);
+	}
+
+	/**
+	 * @expectedException \Exception
+	 */
+	public function testUserExistsPublicAPIForDeleted() {
+		$access = $this->getAccessMock();
+		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
+		$this->prepareMockForUserExists($access);
+		\OC_User::useBackend($backend);
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn) {
+				if($dn === 'dnOfRoland,dc=test') {
+					return array();
+				}
+				return false;
+			}));
 
 		//test for deleted user
 		$result = \OCP\User::userExists('formerUser');
-		$this->assertFalse($result);
+	}
+
+	public function testUserExistsPublicAPIForNeverExisting() {
+		$access = $this->getAccessMock();
+		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
+		$this->prepareMockForUserExists($access);
+		\OC_User::useBackend($backend);
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn) {
+				if($dn === 'dnOfRoland,dc=test') {
+					return array();
+				}
+				return false;
+			}));
 
 		//test for never-existing user
 		$result = \OCP\User::userExists('mallory');
@@ -475,54 +541,105 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 		$this->assertFalse($result);
 	}
 
-	public function testGetHome() {
+	public function testGetHomeAbsolutePath() {
 		$access = $this->getAccessMock();
 		$config = $this->getMock('\OCP\IConfig');
 		$backend = new UserLDAP($access, $config);
 		$this->prepareMockForUserExists($access);
 
 		$access->connection->expects($this->any())
-			   ->method('__get')
-			   ->will($this->returnCallback(function($name) {
-					if($name === 'homeFolderNamingRule') {
-						return 'attr:testAttribute';
-					}
-					return null;
-			   }));
+			->method('__get')
+			->will($this->returnCallback(function($name) {
+				if($name === 'homeFolderNamingRule') {
+					return 'attr:testAttribute';
+				}
+				return null;
+			}));
 
 		$access->expects($this->any())
-			   ->method('readAttribute')
-			   ->will($this->returnCallback(function($dn, $attr) {
-					switch ($dn) {
-						case 'dnOfRoland,dc=test':
-							if($attr === 'testAttribute') {
-								return array('/tmp/rolandshome/');
-							}
-							return array();
-							break;
-						case 'dnOfLadyOfShadows,dc=test':
-							if($attr === 'testAttribute') {
-								return array('susannah/');
-							}
-							return array();
-							break;
-						default:
-							return false;
-				   }
-			   }));
-
-		$datadir = '/my/data/dir';
-		$config->expects($this->once())
-			->method('getSystemValue')
-			->will($this->returnValue($datadir));
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn, $attr) {
+				switch ($dn) {
+					case 'dnOfRoland,dc=test':
+						if($attr === 'testAttribute') {
+							return array('/tmp/rolandshome/');
+						}
+						return array();
+						break;
+					default:
+						return false;
+				}
+			}));
 
 		//absolut path
 		$result = $backend->getHome('gunslinger');
 		$this->assertEquals('/tmp/rolandshome/', $result);
+	}
 
+	public function testGetHomeRelative() {
+		$access = $this->getAccessMock();
+		$config = $this->getMock('\OCP\IConfig');
+		$backend = new UserLDAP($access, $config);
+		$this->prepareMockForUserExists($access);
+
+		$access->connection->expects($this->any())
+			->method('__get')
+			->will($this->returnCallback(function($name) {
+				if($name === 'homeFolderNamingRule') {
+					return 'attr:testAttribute';
+				}
+				return null;
+			}));
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn, $attr) {
+				switch ($dn) {
+					case 'dnOfLadyOfShadows,dc=test':
+						if($attr === 'testAttribute') {
+							return array('susannah/');
+						}
+						return array();
+						break;
+					default:
+						return false;
+				}
+			}));
 		//datadir-relativ path
+		$datadir = '/my/data/dir';
+		$config->expects($this->once())
+			->method('getSystemValue')
+			->will($this->returnValue($datadir));
+
 		$result = $backend->getHome('ladyofshadows');
 		$this->assertEquals($datadir.'/susannah/', $result);
+	}
+
+	/**
+	 * @expectedException \Exception
+	 */
+	public function testGetHomeNoPath() {
+		$access = $this->getAccessMock();
+		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
+		$this->prepareMockForUserExists($access);
+
+		$access->connection->expects($this->any())
+			->method('__get')
+			->will($this->returnCallback(function($name) {
+				if($name === 'homeFolderNamingRule') {
+					return 'attr:testAttribute';
+				}
+				return null;
+			}));
+
+		$access->expects($this->any())
+			->method('readAttribute')
+			->will($this->returnCallback(function($dn, $attr) {
+				switch ($dn) {
+					default:
+						return false;
+				}
+			}));
 
 		//no path at all – triggers OC default behaviour
 		$result = $backend->getHome('newyorker');
@@ -562,6 +679,12 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
 		$this->prepareMockForUserExists($access);
 
+		$access->connection->expects($this->any())
+			->method('getConnectionResource')
+			->will($this->returnCallback(function() {
+				return true;
+			}));
+
 		//with displayName
 		$result = $backend->getDisplayName('gunslinger');
 		$this->assertEquals('Roland Deschain', $result);
@@ -573,9 +696,36 @@ class Test_User_Ldap_Direct extends \Test\TestCase {
 
 	public function testGetDisplayNamePublicAPI() {
 		$access = $this->getAccessMock();
+		$access->expects($this->any())
+			->method('username2dn')
+			->will($this->returnCallback(function($uid) {
+				switch ($uid) {
+					case 'gunslinger':
+						return 'dnOfRoland,dc=test';
+						break;
+					case 'formerUser':
+						return 'dnOfFormerUser,dc=test';
+						break;
+					case 'newyorker':
+						return 'dnOfNewYorker,dc=test';
+						break;
+					case 'ladyofshadows':
+						return 'dnOfLadyOfShadows,dc=test';
+						break;
+					default:
+						return false;
+				}
+			}));
 		$this->prepareAccessForGetDisplayName($access);
 		$backend = new UserLDAP($access, $this->getMock('\OCP\IConfig'));
 		$this->prepareMockForUserExists($access);
+
+		$access->connection->expects($this->any())
+			->method('getConnectionResource')
+			->will($this->returnCallback(function() {
+				return true;
+			}));
+
 		\OC_User::useBackend($backend);
 
 		//with displayName
diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php
index 54e14c093f3..cd8a2dd251c 100644
--- a/apps/user_ldap/user_ldap.php
+++ b/apps/user_ldap/user_ldap.php
@@ -190,6 +190,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 	 * check if a user exists
 	 * @param string $uid the username
 	 * @return boolean
+	 * @throws \Exception when connection could not be established
 	 */
 	public function userExists($uid) {
 		if($this->access->connection->isCached('userExists'.$uid)) {
@@ -208,17 +209,12 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn
 			return true;
 		}
 
-		try {
-			$result = $this->userExistsOnLDAP($user);
-			$this->access->connection->writeToCache('userExists'.$uid, $result);
-			if($result === true) {
-				$user->update();
-			}
-			return $result;
-		} catch (\Exception $e) {
-			\OCP\Util::writeLog('user_ldap', $e->getMessage(), \OCP\Util::WARN);
-			return false;
+		$result = $this->userExistsOnLDAP($user);
+		$this->access->connection->writeToCache('userExists'.$uid, $result);
+		if($result === true) {
+			$user->update();
 		}
+		return $result;
 	}
 
 	/**