Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-28351 add more support fro user->suspended META
  3. MDL-34505

Database authentication plugin fails to revive previously suspended user records

    XMLWordPrintable

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Authentication
    • Labels:

      Description

      Summary:
      When an 'external' user is suspended due to no longer being present in the external user table, that user can never be revived - even when present again on the external user table. Additionally, it causes the sync script to crash due to an attempt to insert a duplicate record into the user table.

      Setup:
      1. Turn on 'External Database' authentication and configure the external tables as required. Ensure 'Removed ext user' = 'Suspend Interal'
      2. Add a user to the external user table and run the sync script (/user/bin/php moodle/auth/db/cli/sync_users.php)
      3. Verify that the user has been created in Moodle
      4. Remove the user from the external user table and run the same sync_users.php script
      5. Verify that the user has been suspended in Moodle (authentication set to 'nologin')
      6. Add the same user back into the external user table (ALL fields should be the same as before) and run sync_users.php

      Expected result:
      The user should be revived in Moodle (auth changed from 'NoLogin' to 'db')

      Actual result:
      The script crashes with an error and the user is not revived.

      Some notes:
      The code in moodle/auth/db/auth.php does some checking of the config->removeuser flag when removing users. That is, should the user be fully deleted (AUTH_REMOVEUSER_FULLDELETE) or just suspended (AUTH_REMOVEUSER_SUSPEND). The suspend option changes the auth from 'db' to 'nologin' and DOESN'T set the deleted field to 1.

      However, when deciding if the user has been previously deleted (line ~364), it assumes the deleted field will be 1 (which it's not) and assumes the auth will still be 'db' (which it's not). When the existing user record fails this test, it then attempts to insert a brand new user record, and fails due to a key violation (I assume on username). It probably should do some checking of the config->removeuser flag when attempting to revive or insert new users, however I'm not sure how it will know that the 'nologin' users used to be 'db' users, especially in conjunction with other authentication types.

      For now, I've had to modify line 364 to remove the deleted=>1 clause and change the auth clause to check only for 'nologin' (rather than $user->auth). This now revives previously suspended users, but it's not ideal and serves only our specific use case.

      Regards,
      Michael

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              skodak Petr Skoda
              Reporter:
              mcwoods Michael Woods
              Integrator:
              Aparup Banerjee
              Tester:
              Adrian Greeve
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias
              Votes:
              2 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                3/Dec/12