[#173,#496] Report better errors from LDAP user update DAO.

This operation did not do anything related with LDAP, so we simply run the corresponding SQL operation. We don't modify the behavior of the operation, so we add a comment about a detected problem. In the web service layer, we add try/catch blocks for the LDAPInvalidOperationException like the ones in the create and delete user services, to prevent crashing on operations that aren't defined in the LDAP backend (namely, assigning users to groups).
Igalia · Mar 20, 2023 · d9b6072 · d9b6072
1 parent 999ec3f
commit d9b6072
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 47 deletions.
diff --git a/model/dao/UserDAO/HybridUserDAO.php b/model/dao/UserDAO/HybridUserDAO.php
@@ -418,35 +418,14 @@ public function getJourneyHistory($userId) {
     /** User updater for LDAP/PostgreSQL Hybrid.
      *
      * This function updates the data of a User by its {@link UserVO}.
+     * WARNING: this function allows to update to a name that does not exist in LDAP.
+     * Do we really want this behavior?
      *
      * @param UserVO $userVO the {@link UserVO} with the data we want to update on database.
-     * @return int the number of rows that have been affected (it should be 1).
-     * @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException}
+     * @return OperationResult the result {@link OperationResult} with information about operation status
      */
     public function update(UserVO $userVO) {
-
-        $affectedRows = 0;
-
-        if($userVO->getId() >= 0) {
-            $currUserVO = $this->getById($userVO->getId());
-        }
-
-        // If the query returned a row then update
-        if(sizeof($currUserVO) > 0) {
-
-            $sql = "UPDATE usr SET login=" . DBPostgres::checkStringNull($userVO->getLogin()) . ", password=" . DBPostgres::checkStringNull($userVO->getPassword()) . " WHERE id=" .$userVO->getId();
-
-            $res = pg_query($this->connect, $sql);
-
-            if ($res == NULL)
-                if (strpos(pg_last_error(), "unique_usr_login"))
-                    throw new SQLUniqueViolationException(pg_last_error());
-                else throw new SQLQueryErrorException(pg_last_error());
-
-            $affectedRows = pg_affected_rows($res);
-        }
-
-        return $affectedRows;
+        return parent::update($userVO);
     }
 
     /** User creator for LDAP/PostgreSQL Hybrid.

diff --git a/web/services/updateUsersService.php b/web/services/updateUsersService.php
@@ -176,18 +176,18 @@
         } while ($parser->read());
 
 
-            foreach((array)$updateUsers as $updateUser)
-            {
-                $result = UsersFacade::UpdateUser($updateUser);
-                if (!$result->getIsSuccessful()) {
-                    http_response_code($result->getResponseCode());
-                    $string = "<return service='updateUsers'><error id='" . $result->getErrorNumber() . "'>" .
-                            $result->getMessage() . "</error></return>";
-                    break;
-                }
+        foreach((array)$updateUsers as $updateUser)
+        {
+            $result = UsersFacade::UpdateUser($updateUser);
+            if (!$result->getIsSuccessful()) {
+                http_response_code($result->getResponseCode());
+                $string = "<return service='updateUsers'><error id='" . $result->getErrorNumber() . "'>" .
+                        $result->getMessage() . "</error></return>";
+                break;
+            }
 
-                foreach((array) $updateUser->getGroups() as $group)
-                {
+            try {
+                foreach((array) $updateUser->getGroups() as $group) {
 
                     $group = UsersFacade::GetUserGroupByName($group->getName());
 
@@ -199,22 +199,33 @@
 
                 }
             }
+            catch (LDAPInvalidOperationException $e) {
+                // the LDAP backend is enabled so UserGroup operations are forbidden
+                // we can ignore this error
+            }
+        }
 
-            foreach((array) $deassignGroups as $userId=>$groups)
-                foreach((array) $groups as $group)
-                {
-
-                    $group = UsersFacade::GetUserGroupByName($group->getName());
-
-                    if (UsersFacade::DeassignUserFromUserGroup($userId, $group->getId()) == -1)
+        if (!$string) {
+            try {
+                foreach((array) $deassignGroups as $userId=>$groups)
+                    foreach((array) $groups as $group)
                     {
-                        $string = "<return service='updateUsers'><error id='1'>There was some error while updating the user groups new deassignements</error></return>";
-                        break;
-                    }
 
-                }
+                        $group = UsersFacade::GetUserGroupByName($group->getName());
 
+                        if (UsersFacade::DeassignUserFromUserGroup($userId, $group->getId()) == -1)
+                        {
+                            $string = "<return service='updateUsers'><error id='1'>There was some error while updating the user groups new deassignements</error></return>";
+                            break;
+                        }
 
+                    }
+            }
+            catch (LDAPInvalidOperationException $e) {
+                // the LDAP backend is enabled so UserGroup operations are forbidden
+                // we can ignore this error
+            }
+        }
 
         if (!$string)
         {