Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User import (ldap) 1102 (with 10.0.12/13/14) instead of 5394 (with 10.0.11) #16725

Closed
2 tasks done
TheRoby opened this issue Mar 7, 2024 · 4 comments
Closed
2 tasks done
Assignees
Labels
Milestone

Comments

@TheRoby
Copy link

TheRoby commented Mar 7, 2024

Code of Conduct

  • I agree to follow this project's Code of Conduct

Is there an existing issue for this?

  • I have searched the existing issues

Version

10.0.13-dev

Bug description

Hello,

I have a surprising functioning for LDAP import

In 10.0.11 5394 users imported

 5394/5394 [============================] 100%
+--------------+---------+-------------+--------------------------+----------------------------------+
| Serveur LDAP | Importé | Synchronisé | Supprimé du serveur LDAP | Restauré depuis un annuaire LDAP |
+--------------+---------+-------------+--------------------------+----------------------------------+
| 4            | 0       | 5394        | 0                        | 0                                |
+--------------+---------+-------------+--------------------------+----------------------------------+

In 10.0.12 (and .13-dev) 1102 users imported

 1102/1102 [============================] 100%
+--------------+---------+-------------+--------------------------+----------------------------------+
| Serveur LDAP | Importé | Synchronisé | Supprimé du serveur LDAP | Restauré depuis un annuaire LDAP |
+--------------+---------+-------------+--------------------------+----------------------------------+
| 4            | 0       | 1102        | 0                        | 0                                |
+--------------+---------+-------------+--------------------------+----------------------------------+

if I put the src/User.php file from version 10.0.11, then it works as before

As a result, all of my users are not updated. I haven't managed to find any common point that could explain why only some people go back.

The problem is that I use the Last Synchronization field in users...

Thank you :)

Relevant log output

No response

Page URL

No response

Steps To reproduce

No response

Your GLPI setup information

No response

Anything else?

No response

@TheRoby TheRoby changed the title User import (ldap) 1102 instead of User import (ldap) 1102 instead of 5394 (with 10.0.11) Mar 8, 2024
@TheRoby
Copy link
Author

TheRoby commented Mar 14, 2024

Hi,
Same problem with 10.0.13

@TheRoby TheRoby changed the title User import (ldap) 1102 instead of 5394 (with 10.0.11) User import (ldap) 1102 (with 10.0.12/13/14) instead of 5394 (with 10.0.11) Mar 14, 2024
@TheRoby
Copy link
Author

TheRoby commented Mar 14, 2024

2ee311a
if i change this :

public function getFromDBbyDn($user_dn)
    {
       [old] return $this->getFromDBByCrit(['user_dn' => $user_dn]);
        /**
         * We use the 'user_dn_hash' field instead of 'user_dn' for performance reasons.
         * The 'user_dn_hash' field is a hashed version of the 'user_dn' field
         * and is indexed in the database, making it faster to search.
         */
        return $this->getFromDBByCrit([
            'user_dn_hash' => md5($user_dn)
        ]);
    }

to the old version, I have 5394 records updated ...

Edit :
With the old getFromDBbyDN i have a lot of "getFromDBByCrit expects to get one result"
With the new, nothing ...

@TheRoby
Copy link
Author

TheRoby commented Mar 15, 2024

I think I found a clue :
2ee311a#commitcomment-139834247

@cedric-anne
Copy link
Member

Hi,

This issue will be fixed by #16806. As it requires a migration, the patch cannot be applied in a production environment.

You can apply the following patch instead, on GLPI 10.0.14, to fix the issue until GLPI 10.0.15 is released:

diff --git a/src/User.php b/src/User.php
index 695fe4df5f..5349a49322 100644
--- a/src/User.php
+++ b/src/User.php
@@ -567,7 +567,7 @@ class User extends CommonDBTM
          * and is indexed in the database, making it faster to search.
          */
         return $this->getFromDBByCrit([
-            'user_dn_hash' => md5($user_dn)
+            'user_dn_hash' => md5(Sanitizer::sanitize($user_dn))
         ]);
     }
 
@@ -847,7 +847,7 @@ class User extends CommonDBTM
     {
         // Hash user_dn if set
         if (isset($this->input['user_dn']) && is_string($this->input['user_dn']) && strlen($this->input['user_dn']) > 0) {
-            $this->input['user_dn_hash'] = md5($this->input['user_dn']);
+            $this->input['user_dn_hash'] = md5(Sanitizer::sanitize($this->input['user_dn']));
         }
     }
 
@@ -3503,7 +3503,7 @@ HTML;
         // Hash user_dn if is updated
         if (in_array('user_dn', $this->updates)) {
             $this->updates[] = 'user_dn_hash';
-            $this->fields['user_dn_hash'] = is_string($this->input['user_dn']) && strlen($this->input['user_dn']) > 0 ? md5($this->input['user_dn']) : null;
+            $this->fields['user_dn_hash'] = is_string($this->input['user_dn']) && strlen($this->input['user_dn']) > 0 ? md5(Sanitizer::sanitize($this->input['user_dn'])) : null;
         }
     }
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants