Moodle
  1. Moodle
  2. MDL-39166

LDAP auth plugin doesn't call events_trigger() when user record updated

    Details

    • Testing Instructions:
      Hide

      Testing instructions

      All of the following tests require access to an LDAP server with the Moodle LDAP authentication plugin enabled and configured to connect to this external server. These tests also require administrative access to the LDAP server with the ability to add and remove accounts.

      NOTE: In all of the following queries, replace the mdl_ table name prefix with the value used in your local development environment if not using the default configuration.

      Run the following queries:

      INSERT INTO  mdl_events_handlers (id, eventname, component, handlerfile, handlerfunction, schedule, status, internal)
      VALUES (NULL, 'user_created', 'auth_ldap', '/auth/ldap/auth.php', 'a:2:{i:0;s:16:"auth_plugin_ldap";i:1;s:11:"init_plugin";}', 'cron', '0', '1');
      
      INSERT INTO  mdl_events_handlers (id, eventname, component, handlerfile, handlerfunction, schedule, status, internal)
      VALUES (NULL, 'user_updated', 'auth_ldap', '/auth/ldap/auth.php', 'a:2:{i:0;s:16:"auth_plugin_ldap";i:1;s:11:"init_plugin";}', 'cron', '0', '1');
      

      Queued event check query:

      •     SELECT h.eventname, h.component, h.handlerfile, q.*
              FROM mdl_events_queue q
        INNER JOIN mdl_events_queue_handlers qh ON qh.queuedeventid = q.id
        INNER JOIN mdl_events_handlers h ON h.id = qh.handlerid
             WHERE h.component = 'auth_ldap'
               AND h.handlerfile = '/auth/ldap/auth.php'
               AND h.schedule = 'cron'
               AND (h.eventname = 'user_updated' OR h.eventname = 'user_created');
        

      Queued event purge queries:

      • MySQL:
        •     DELETE q
                FROM mdl_events_queue q
          INNER JOIN mdl_events_queue_handlers qh ON qh.queuedeventid = q.id
          INNER JOIN mdl_events_handlers h ON h.id = qh.handlerid
               WHERE h.component = 'auth_ldap'
                 AND h.handlerfile = '/auth/ldap/auth.php'
                 AND h.schedule = 'cron'
                 AND (h.eventname = 'user_updated' OR h.eventname = 'user_created');
          
      • Other:
        •  DELETE FROM mdl_events_queue
          WHERE EXISTS (
              SELECT q.*
                FROM mdl_events_queue q
          INNER JOIN mdl_events_queue_handlers qh ON qh.queuedeventid = q.id
          INNER JOIN mdl_events_handlers h ON h.id = qh.handlerid
               WHERE h.component = 'auth_ldap'
                 AND h.handlerfile = '/auth/ldap/auth.php'
                 AND h.schedule = 'cron'
                 AND (h.eventname = 'user_updated' OR h.eventname = 'user_created')
          );
          

      user_updated event when creating a new external user

      NOTE: this test requires email sending to be active on your Moodle test site as a confirmation email is sent out during the test.

      1. Log into the site as a site administrator
      2. Navigate to Settings > Plugins > Authentication > LDAP server
      3. Set the Create users externally setting to Yes
      4. Set the Context for new users setting to a valid context on the LDAP server
      5. Click on Save changes
      6. Navigate to Settings > Plugins > Authentication > Manage authentication
      7. Set the Self registration setting to LDAP Server and click on Save changes
      8. Logout of the site
      9. Click on a *Login * link
      10. Click on the Create new account button
      11. Fill out the required fields (be sure to enter an email address that you can receive mail for) and click the Create my new account button
      12. Wait for the confirmation email to arrive and click on the confirmation link in the email
      13. Run the queued event check query listed above
      14. You should see a single user_updated record returned
      15. Run the appropriate database query listed above to purge queued events

      user_updated event when suspending an externally deleted user

      1. Run the CLI script /auth/ldap/cli/sync_users.php to ensure that all of the users from the LDAP server have Moodle accounts created
      2. Log into the site as a site administrator
      3. Navigate to Settings > Plugins > Authentication > LDAP server
      4. Set the Removed ext user setting to the value of Suspend internal and click on Save changes
      5. Remove a user account from the LDAP server that can be found by Moodle (based on the current search configuration within the LDAP auth plugin)
      6. Run the CLI script /auth/ldap/cli/sync_users.php
      7. Run the queued event check query listed above
      8. You should see a single user_updated record returned
      9. Run the appropriate database query listed above to purge queued events

      user_updated event when "reactivating" a previously externally deleted user account

      1. Complete the test above
      2. Recreate the LDAP user account that was removed in step 4 from the test above
      3. Run the CLI script /auth/ldap/cli/sync_users.php
      4. Run the queued event check query listed above
      5. You should see a single user_updated record returned
      6. Run the appropriate database query listed above to purge queued events

      user_created event when adding a new external user account

      1. Create a new user account in the LDAP server that can be found by Moodle (based on the current search configuration within the LDAP auth plugin)
      2. Run the CLI script /auth/ldap/cli/sync_users.php
      3. Run the queued event check query listed above
      4. You should see a single user_created record returned
      5. Run the appropriate database query listed above to purge queued events

      user_updated event when modifying an external user account

      1. Modify an existing user account in the LDAP server that can be found by Moodle (change the firstname and lastname values) based on the current search configuration within the LDAP auth plugin)
      2. Run the CLI script /auth/ldap/cli/sync_users.php
      3. Run the queued event check query listed above
      4. You should see a single user_updated record returned
      5. Run the appropriate database query listed above to purge queued events
      Show
      Testing instructions All of the following tests require access to an LDAP server with the Moodle LDAP authentication plugin enabled and configured to connect to this external server. These tests also require administrative access to the LDAP server with the ability to add and remove accounts. NOTE: In all of the following queries, replace the mdl_ table name prefix with the value used in your local development environment if not using the default configuration. Run the following queries: INSERT INTO mdl_events_handlers (id, eventname, component, handlerfile, handlerfunction, schedule, status, internal) VALUES (NULL, 'user_created', 'auth_ldap', '/auth/ldap/auth.php', 'a:2:{i:0;s:16:"auth_plugin_ldap";i:1;s:11:"init_plugin";}', 'cron', '0', '1'); INSERT INTO mdl_events_handlers (id, eventname, component, handlerfile, handlerfunction, schedule, status, internal) VALUES (NULL, 'user_updated', 'auth_ldap', '/auth/ldap/auth.php', 'a:2:{i:0;s:16:"auth_plugin_ldap";i:1;s:11:"init_plugin";}', 'cron', '0', '1'); Queued event check query: SELECT h.eventname, h.component, h.handlerfile, q.* FROM mdl_events_queue q INNER JOIN mdl_events_queue_handlers qh ON qh.queuedeventid = q.id INNER JOIN mdl_events_handlers h ON h.id = qh.handlerid WHERE h.component = 'auth_ldap' AND h.handlerfile = '/auth/ldap/auth.php' AND h.schedule = 'cron' AND (h.eventname = 'user_updated' OR h.eventname = 'user_created'); Queued event purge queries: MySQL: DELETE q FROM mdl_events_queue q INNER JOIN mdl_events_queue_handlers qh ON qh.queuedeventid = q.id INNER JOIN mdl_events_handlers h ON h.id = qh.handlerid WHERE h.component = 'auth_ldap' AND h.handlerfile = '/auth/ldap/auth.php' AND h.schedule = 'cron' AND (h.eventname = 'user_updated' OR h.eventname = 'user_created'); Other: DELETE FROM mdl_events_queue WHERE EXISTS ( SELECT q.* FROM mdl_events_queue q INNER JOIN mdl_events_queue_handlers qh ON qh.queuedeventid = q.id INNER JOIN mdl_events_handlers h ON h.id = qh.handlerid WHERE h.component = 'auth_ldap' AND h.handlerfile = '/auth/ldap/auth.php' AND h.schedule = 'cron' AND (h.eventname = 'user_updated' OR h.eventname = 'user_created') ); user_updated event when creating a new external user NOTE: this test requires email sending to be active on your Moodle test site as a confirmation email is sent out during the test. Log into the site as a site administrator Navigate to Settings > Plugins > Authentication > LDAP server Set the Create users externally setting to Yes Set the Context for new users setting to a valid context on the LDAP server Click on Save changes Navigate to Settings > Plugins > Authentication > Manage authentication Set the Self registration setting to LDAP Server and click on Save changes Logout of the site Click on a *Login * link Click on the Create new account button Fill out the required fields (be sure to enter an email address that you can receive mail for) and click the Create my new account button Wait for the confirmation email to arrive and click on the confirmation link in the email Run the queued event check query listed above You should see a single user_updated record returned Run the appropriate database query listed above to purge queued events user_updated event when suspending an externally deleted user Run the CLI script /auth/ldap/cli/sync_users.php to ensure that all of the users from the LDAP server have Moodle accounts created Log into the site as a site administrator Navigate to Settings > Plugins > Authentication > LDAP server Set the Removed ext user setting to the value of Suspend internal and click on Save changes Remove a user account from the LDAP server that can be found by Moodle (based on the current search configuration within the LDAP auth plugin) Run the CLI script /auth/ldap/cli/sync_users.php Run the queued event check query listed above You should see a single user_updated record returned Run the appropriate database query listed above to purge queued events user_updated event when "reactivating" a previously externally deleted user account Complete the test above Recreate the LDAP user account that was removed in step 4 from the test above Run the CLI script /auth/ldap/cli/sync_users.php Run the queued event check query listed above You should see a single user_updated record returned Run the appropriate database query listed above to purge queued events user_created event when adding a new external user account Create a new user account in the LDAP server that can be found by Moodle (based on the current search configuration within the LDAP auth plugin) Run the CLI script /auth/ldap/cli/sync_users.php Run the queued event check query listed above You should see a single user_created record returned Run the appropriate database query listed above to purge queued events user_updated event when modifying an external user account Modify an existing user account in the LDAP server that can be found by Moodle (change the firstname and lastname values) based on the current search configuration within the LDAP auth plugin) Run the CLI script /auth/ldap/cli/sync_users.php Run the queued event check query listed above You should see a single user_updated record returned Run the appropriate database query listed above to purge queued events
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-39166-m24
    • Pull Master Branch:
      MDL-39166-m25
    • Rank:
      49767

      Description

      LDAP auth plugin doesn't call events_trigger() when user record updated and, if some cases, created.

        Issue Links

          Activity

          Hide
          Brent Boghosian added a comment -

          Attached possible fix to LDAP auth plugin: mdl39166.patch

          Show
          Brent Boghosian added a comment - Attached possible fix to LDAP auth plugin: mdl39166.patch
          Hide
          Petr Škoda added a comment -

          Hello, it looks like each event is sending user object with different properties, I think it should be more consistent. Thanks anyway.

          Show
          Petr Škoda added a comment - Hello, it looks like each event is sending user object with different properties, I think it should be more consistent. Thanks anyway.
          Hide
          Michael de Raadt added a comment -

          Thanks for working on this guys. Please keep going.

          Show
          Michael de Raadt added a comment - Thanks for working on this guys. Please keep going.
          Hide
          Justin Filip added a comment -

          The branches were updated to always load the user record from the DB and send that through the event system.

          Show
          Justin Filip added a comment - The branches were updated to always load the user record from the DB and send that through the event system.
          Hide
          Petr Škoda added a comment -

          +1, sending to integration, please add some test instructions

          Show
          Petr Škoda added a comment - +1, sending to integration, please add some test instructions
          Hide
          Petr Škoda added a comment -

          Adding Iñaki Arenaza, any objections?

          Show
          Petr Škoda added a comment - Adding Iñaki Arenaza, any objections?
          Hide
          Iñaki Arenaza added a comment -

          No objections from me

          Show
          Iñaki Arenaza added a comment - No objections from me
          Hide
          Dan Poltawski added a comment -

          Just waiting for testing instructions. Thanks.

          Show
          Dan Poltawski added a comment - Just waiting for testing instructions. Thanks.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Damyon Wiese added a comment -

          This issue needs testing instructions before it will be integrated. Justin - can you add some please?

          Show
          Damyon Wiese added a comment - This issue needs testing instructions before it will be integrated. Justin - can you add some please?
          Hide
          Dan Poltawski added a comment -

          Reopening this for testing instructions (and get it off our radar whilst outstanding)

          Show
          Dan Poltawski added a comment - Reopening this for testing instructions (and get it off our radar whilst outstanding)
          Hide
          Justin Filip added a comment -

          Sorry guys, been busy with other things. I'll get some instructions up on this issue today.

          Show
          Justin Filip added a comment - Sorry guys, been busy with other things. I'll get some instructions up on this issue today.
          Hide
          Justin Filip added a comment - - edited

          Added testing instructions. This one is ready to go back into Integration testing now.

          Show
          Justin Filip added a comment - - edited Added testing instructions. This one is ready to go back into Integration testing now.
          Hide
          Petr Škoda added a comment -

          +1, submitting for integration

          Show
          Petr Škoda added a comment - +1, submitting for integration
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24, 25 & master, note 22 has been discarded, out of support). Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24, 25 & master, note 22 has been discarded, out of support). Thanks!
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Justin,

          Works as mentioned.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Justin, Works as mentioned.
          Hide
          Dan Poltawski added a comment -
          Feature: Thanks to our superb contributors
            In order to make Moodle better
            As an integrator
            I need to thank all our contributors
          
            Scenario: Dan thanks you all
              Given I log in as "dan"
              And I see "lots of fixed issues"
              When I follow "Close integrated issues"
              Then I should see "Lots of thanks to all our contributors"
          

          Your changes are upstream

          Show
          Dan Poltawski added a comment - Feature: Thanks to our superb contributors In order to make Moodle better As an integrator I need to thank all our contributors Scenario: Dan thanks you all Given I log in as "dan" And I see "lots of fixed issues" When I follow "Close integrated issues" Then I should see "Lots of thanks to all our contributors" Your changes are upstream
          Hide
          Petr Škoda added a comment -

          Hi, this caused a regression - MDL-40346. We already have ldap unit tests, next time please run them during testing (it requires extra config.php settings and openldap test server configuration).

          Show
          Petr Škoda added a comment - Hi, this caused a regression - MDL-40346 . We already have ldap unit tests, next time please run them during testing (it requires extra config.php settings and openldap test server configuration).

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: