philbertphotos / osticket-multildap-auth-plugin

Plugin for OS Ticket that allows for authentication with multiple domains.
GNU General Public License v3.0
28 stars 17 forks source link

not all ldap users get synced #44

Closed lucize closed 3 years ago

lucize commented 3 years ago

Hi,

It seems that maximum users synced are 140 and the users needs to be first imported or created in the database

Sync executed on (February 14 2021 3:05 pm) next execution in (February 14 2021 3:05 pm)Total ldapusers: (4387) Total agents: () Total Updated Users: (140) Execute Time: (0:00:07.912)

Also LDAP_CONTROL_PAGEDRESULTS is not implemented and we must increase maxpagesize for ldap querry over the standard 1000 results

philbertphotos commented 3 years ago

This is correct it only syncs users that are in the database by design. On my system I have over 800 users and it syncs them all. I may add an import all users feature. The LDAP API can do over 1000 results last I checked.

lucize commented 3 years ago

I imported ~5000 user in database but only some of them are updated (~140 of them) not sure what makes it stop or what is the filter for updating them in the local db. as for ldap limit of 1000 this is a default value added in MS AD and you have to increase the maxpagesize setting form AD but not always we have the option to set this, as for ladp search we only need a generic ad user some hints https://stackoverflow.com/questions/24990243/ldap-search-not-returning-more-than-1000-user

Many thanks for this plugin

philbertphotos commented 3 years ago

The plug has a algorithm that syncs only users that have changed. Based on the ObjectID and the whenchanged attribute is saved in mysql.

philbertphotos commented 3 years ago

//Update user When Change time. if ($synckey['updated'] != $this->changetime($user->whenchanged) || !$this->contains($this->log_report['status'], 'Error')) $result = db_query("UPDATE " . TABLE_PREFIX . "ldap_sync SET updated = \"" . $this->changetime($user->whenchanged) . "\" WHERE id = " . $user->user_id);

Then I think it checks for what attributes need to updated if the values do not match it updates.

lucize commented 3 years ago

The use case in my part was that importing the csv users from ad didn't have all the usefull informations like department or title, and after assessing the needs we had to modify the php file for the missing options because the the sync didn't process all the attributes from all the existing imported csv users, even if they were defined in the plugin sync option

philbertphotos commented 3 years ago

Hmmm that is interesting.

philbertphotos commented 3 years ago

Can you show me the modifications you made so I can check why it did not work properly... I never actually imported uses from CSV since once a user logs in it auto creates the user.

lucize commented 3 years ago

I just changed the auth.php so it will populate more fields when the user is created by the login, we abandoned the import of all the users in database because of full sync issue nothing special, something like:

@@ -244,13 +244,13 @@
         * @return boolean
         */
        function firstRun() {
-               //$sql = 'SELECT version FROM ' . PLUGIN_TABLE . ' WHERE `name` LIKE \'%Multi LDAP%\'';
+               //$sql = 'SELECT version FROM ' . PLUGIN_TABLE . ' WHERE `name` LIKE \'%Multi-LDAP%\'';
                $sql = "SHOW TABLES LIKE '". TABLE_PREFIX ."ldap_sync'";
                $res = db_query($sql);
                $rows = db_num_rows($res);

                if ($rows <= 0) {
-                       $this->sync_copy();
+                       $this->sync_copy();
                        $this->createSyncTables();
                }
                return (db_num_rows($res) == 0);
@@ -274,7 +274,7 @@
        }

        function updateVersion() {
-               $sql = "UPDATE version ' . PLUGIN_TABLE . ' SET version='".MULTI_PLUGIN_VERSION."' WHERE `name` LIKE '%Multi LDAP%'";
+               $sql = "UPDATE version ' . PLUGIN_TABLE . ' SET version='".MULTI_PLUGIN_VERSION."' WHERE `name` LIKE '%Multi-LDAP%'";
                if (!($res = db_query($sql))) {
                        return true;
                }
@@ -283,7 +283,7 @@
        }

        function needUpgrade() {
-               $sql = 'SELECT version FROM ' . PLUGIN_TABLE . ' WHERE `name` LIKE \'%Multi LDAP%\'';
+               $sql = 'SELECT version FROM ' . PLUGIN_TABLE . ' WHERE `name` LIKE \'%Multi-LDAP%\'';
                if (!($res = db_query($sql))) {
                        return true;
                }
@@ -400,7 +400,11 @@
                        'displayName',
                        'mail',
                        'telephoneNumber',
+                       'mobile',
                        'distinguishedName',
+                       'title',
+                       'department',
+                       'company',
                ) , array(
                        'username',
                        'first',
@@ -408,7 +412,11 @@
                        'full',
                        'email',
                        'phone',
+                       'usermobile',
                        'dn',
+                       'usertitle',
+                       'userdepartment',
+                       'usercompany',
                )));
                return $keys;
        }
@@ -421,7 +429,11 @@
                        'displayName',
                        'mail',
                        'telephoneNumber',
-                       'distinguishedName'
+                       'mobile',
+                       'distinguishedName',
+                       'title',
+                       'department',
+                       'company'
                );
        }
philbertphotos commented 3 years ago

Will add this in the build