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



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


      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.

      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.



          Issue Links



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


                Fix Release Date: