Catch SQL exceptions so that FALSE can be returned

Some methods must return FALSE on failure and if SQL exceptions are not caught,
they bubble up to Nexcloud classes and the user manager does not realize that
something went wrong, because the exceptions are only caught after that
operations result has been evaluated.

Author: alexeyabel

README.md

@@ -104,5 +104,9 @@ by default  */var/www/nextcloud/data/nextcloud.log* or */var/log/syslog*.
 - This app also logs non-SQL configuration errors, e.g. missing db name.
 
 # Release Notes
+## 1.0.1
+- Fixed a bug where for some (non security related) operations, SQL errors would prevent Nextcloud
+from realizing that that operation failed.
 ## 1.0.0
-- named parameter in query `get_users` was `:username`, is now `:search` because you search for usernames and display names.
+- Named parameter in query `get_users` was `:username`, is now `:search` because you search for
+user names and display names.

appinfo/info.xml

@@ -14,7 +14,7 @@ In contrast to the app *SQL user backend*, you write the SQL queries yourself. Y
 The app uses prepared statements and is written to be secure by default to prevent SQL injections. It understands the most popular standards for password hash formats: MD5-CRYPT, SHA256-CRYPT, SHA512-CRYPT, BCrypt and the state-of-the-art Argon2i. Because the various formats are recognized on-the-fly your db can can have differing hash string formats at the same time, which eases migration to newer formats.
 
 This app supports PostgreSQL and MariaDB/MySQL.]]></description>
-    <version>1.0.0</version>
+    <version>1.0.1</version>
     <licence>agpl</licence>
     <author mail="dev@abelonline.de" >Alexey Abel</author>
     <documentation>

lib/UserBackend.php

@@ -89,7 +89,7 @@ public function checkPassword($providedUsername, $providedPassword) {
 
 	public function deleteUser($providedUsername) {
 		$statement = $this->db->getDbHandle()->prepare($this->config->getQueryDeleteUser());
-		$wasUserDeleted = $statement->execute(['username' => $providedUsername]);
+		$wasUserDeleted = $this->executeOrCatchExceptionAndReturnFalse($statement, ['username' => $providedUsername]);
 		return $wasUserDeleted;
 	}
 
@@ -98,28 +98,28 @@ public function getUsers($searchString = '', $limit = null, $offset = null) {
 		// wildcards in the LIKE expression. Therefore they will be escaped.
 		$searchString = $this->escapePercentAndUnderscore($searchString);
 
-		$parameterSubstitution['search'] = '%' . $searchString . '%';
+		$parameterSubstitutions['search'] = '%' . $searchString . '%';
 
 		if (is_null($limit)) {
 			$limitSegment = '';
 		} else {
 			$limitSegment = ' LIMIT :limit';
-			$parameterSubstitution['limit'] = $limit;
+			$parameterSubstitutions['limit'] = $limit;
 		}
 
 		if (is_null($offset)) {
 			$offsetSegment = '';
 		} else {
 			$offsetSegment = ' OFFSET :offset';
-			$parameterSubstitution['offset'] = $offset;
+			$parameterSubstitutions['offset'] = $offset;
 		}
 
 		$queryFromConfig = $this->config->getQueryGetUsers();
 
 		$finalQuery = $queryFromConfig . $limitSegment . $offsetSegment;
 
 		$statement = $this->db->getDbHandle()->prepare($finalQuery);
-		$statement->execute($parameterSubstitution);
+		$statement->execute(parameterSubstitutions);
 		// Setting the second parameter to 0 will ensure, that only the first
 		// column is returned.
 		$matchedUsers = $statement->fetchAll(\PDO::FETCH_COLUMN, 0);
@@ -129,7 +129,7 @@ public function getUsers($searchString = '', $limit = null, $offset = null) {
 
 	public function userExists($providedUsername) {
 		$statement = $this->db->getDbHandle()->prepare($this->config->getQueryUserExists());
-		$statement->execute(['username' => $providedUsername]);
+		$this->executeOrCatchExceptionAndReturnFalse($statement, ['username' => $providedUsername]);
 		$doesUserExist = $statement->fetchColumn();
 		return $doesUserExist;
 	}
@@ -152,9 +152,13 @@ public function getDisplayNames($search = '', $limit = null, $offset = null) {
 
 	public function setDisplayName($username, $newDisplayName) {
 		$statement = $this->db->getDbHandle()->prepare($this->config->getQuerySetDisplayName());
-		$dbUpdateWasSuccessful = $statement->execute([
+		$parameterSubstitutions = [
 			':username' => $username,
-			':new_display_name' => $newDisplayName]);
+			':new_display_name' => $newDisplayName
+		];
+
+		$dbUpdateWasSuccessful =
+			$this->executeOrCatchExceptionAndReturnFalse($statement, $parameterSubstitutions);
 
 		if ($dbUpdateWasSuccessful) {
 			return TRUE;
@@ -203,9 +207,12 @@ public function setPassword($username, $newPassword) {
 		$dbHandle->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_SILENT);
 		$statement = $dbHandle->prepare($this->config->getQuerySetPasswordForUser());
 
-		$dbUpdateWasSuccessful = $statement->execute([
+		$parameterSubstitutions = [
 			':username' => $username,
-			':new_password_hash' => $this->hashPassword($newPassword)]);
+			':new_password_hash' => $this->hashPassword($newPassword)];
+
+		$dbUpdateWasSuccessful =
+			$this->executeOrCatchExceptionAndReturnFalse($statement, $parameterSubstitutions);
 
 		if ($dbUpdateWasSuccessful) {
 			return TRUE;
@@ -240,7 +247,7 @@ public function createUser($providedUsername, $providedPassword) {
 		$dbHandle = $this->db->getDbHandle();
 
 		$statement = $dbHandle->prepare($this->config->getQueryCreateUser());
-		$dbUpdateWasSuccessful = $statement->execute([
+		$dbUpdateWasSuccessful = $this->executeOrCatchExceptionAndReturnFalse($statement, [
 			':username' => $providedUsername,
 			':password_hash' => $this->hashPassword($providedPassword)]);
 
@@ -338,4 +345,25 @@ private function hashWithOldMethod($password, $algorithm) {
 		}
 		return $hashedPassword;
 	}
+
+	/**
+	 * Helper function that catches SQL exceptions for methods that should return FALSE on failure.
+	 * If exception is not caught here, it bubbles up to Nextcloud's dispatcher and the user manager
+	 * is not aware that a user backend method failed.
+	 *
+	 * @param \PDOStatement $pdoStatement the statement to execute
+	 * @param array $parameterSubstitutions the substitution parameters
+	 *
+	 * @return bool|\PDOStatement
+	 */
+	private function executeOrCatchExceptionAndReturnFalse(\PDOStatement $pdoStatement, array $parameterSubstitutions) {
+		try {
+			$pdoStatement->execute($parameterSubstitutions);
+		}
+		catch (\PDOException $e) {
+			$this->logger->logException($e, $this->logContext);
+			return FALSE;
+		}
+		return $pdoStatement;
+	}
 }