Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-67181

LDAP user sync does not suspend users by suspended attribute

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Development in progress
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.5.8, 3.5.11, 3.6.9, 3.7.5, 3.8.2, 3.9
    • Fix Version/s: None
    • Component/s: Authentication
    • Labels:
    • Testing Instructions:
      Hide

      Preconditions:

      • We have a ldap MS ActiveDirectory with a X user.
      • The X user exist in moodle with ldap auth and it is active.
      • The ldap auth plugin is enabled.
      • Configure and test the ldap conection.
      • Set auth_ldap > suspended_attribute to useraccountcontrol
      • Set auth_ldap > removeuser to Suspend internal
      • In ActiveDirectory suspend the user X
      • In the console, go to admin/tool/task/cli/

      Testing:

      Before the patch:

      1. Set auth_ldap > sync_suspended to Not
      2. un: php schedule_task.php --execute="\auth_ldap\task\sync_task"
      3. Validate: The user X is active
      4. Change auth_ldap > sync_suspended to Yes
      5. Run: php schedule_task.php --execute="\auth_ldap\task\sync_task"
      6. Validate: The user X is active

      After the patch:

      1. Set auth_ldap > sync_suspended to Not
      2. Run: php schedule_task.php --execute="\auth_ldap\task\sync_task"
      3. Validate: The user X is active
      4. Change auth_ldap > sync_suspended to Yes
      5. Run: php schedule_task.php --execute="\auth_ldap\task\sync_task"
      6. Validate: The user X is suspended

       

      Show
      Preconditions: We have a ldap MS ActiveDirectory with a X user. The X user exist in moodle with ldap auth and it is active. The ldap auth plugin is enabled. Configure and test the ldap conection. Set auth_ldap > suspended_attribute to useraccountcontrol Set auth_ldap > removeuser to Suspend internal In ActiveDirectory suspend the user X In the console, go to admin/tool/task/cli/ Testing: Before the patch: Set auth_ldap > sync_suspended to Not un: php schedule_task.php --execute="\auth_ldap\task\sync_task" Validate: The user X is active Change auth_ldap > sync_suspended to Yes Run: php schedule_task.php --execute="\auth_ldap\task\sync_task" Validate: The user X is active After the patch: Set auth_ldap > sync_suspended to Not Run: php schedule_task.php --execute="\auth_ldap\task\sync_task" Validate: The user X is active Change auth_ldap > sync_suspended to Yes Run: php schedule_task.php --execute="\auth_ldap\task\sync_task" Validate: The user X is suspended  
    • Affected Branches:
      MOODLE_35_STABLE, MOODLE_36_STABLE, MOODLE_37_STABLE, MOODLE_38_STABLE, MOODLE_39_STABLE
    • Pull Master Branch:
      MDL-67181-master_ldap_sync_suspended_attribute

      Description

      Enable LDAP users sync job  to suspend user  with the following settings,  the LDAP users sync job

      dose not suspend the users whose AD acounts have been disabled. 

       

      The main setting on ldap settings:

      Suspended attriubte (auth_ldap | suspended_attribute): UserAccountControl

      Removed ext user (auth_ldap | removeuser ): Suspend internal

      Synchronise local user suspension status ( auth_ldap | sync_suspended ): Yes

       

      What I found

       after looking into the source code, I found a bug  in function update_user_records of  lib/authlib.php file.

      please see the patch belwo, the variable '$needsupdate' should be set to  'true' when  $user->suspned dose not equal $suspenduser.

      and then the user will be updated as well as his suspend status on Moodle. otherwise the user's suspend status will not be updated as the user will not be updated due to the $needsupdate is not set to 'true', which is not right. 

      // Some comments here
      Index: lib/authlib.php
      IDEA additional info:
      Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
      <+>UTF-8
      ===================================================================
      --- lib/authlib.php	(revision )
      +++ lib/authlib.php	(revision )
      @@ -669,7 +669,9 @@
                       $newuser = new stdClass();
                       $newuser->id = $userid;
                       // The cast to int is a workaround for MDL-53959.
      -                $newuser->suspended = (int) $suspenduser;
      +                if($user->suspended != (int)$suspenduser ) {
      +                    $newuser->suspended = (int) $suspenduser;+                    $needsupdate = true;
      +                }
                       // Load all custom fields.
                       $profilefields = (array) profile_user_record($user->id, false);
                       $newprofilefields = [];
      
      

      I am happy to amend any information please let me know.

        Attachments

          Activity

            People

            Assignee:
            cirano David Herney Bernal
            Reporter:
            duran.chen Duran Chen
            Peer reviewer:
            Mathew May
            Participants:
            Component watchers:
            Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan
            Votes:
            2 Vote for this issue
            Watchers:
            5 Start watching this issue

              Dates

              Created:
              Updated:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 30 minutes
                30m