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

Guest user account gets deleted

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Authentication
    • Labels:

      Description

      Recently a couple of admins at moodle.cz reported that their guest user got deleted after upgrade. Here are links to the relevant discussions in Czech:

      I have a suspicion there might be a bug in the recent 2.2 and 2.3 code causing this. Surely it is difficult to reproduce but we might prevent guest and admin users being deleted via delete_user() and the thrown exception might help to diagnose the problem (credits to Skodak for this idea).

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              skodak Petr Skoda added a comment -

              Thanks for the report, I have added delete_user() parameter validation which detects different types of problems including deleting of guest account. In the end I have decided to throw exception only for non-user record parameter and return false + print debugging message if user does not exist, is guest account or is one of local admins.

              Show
              skodak Petr Skoda added a comment - Thanks for the report, I have added delete_user() parameter validation which detects different types of problems including deleting of guest account. In the end I have decided to throw exception only for non-user record parameter and return false + print debugging message if user does not exist, is guest account or is one of local admins.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated (22, 23 & master), thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              phpunit ran ok

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - phpunit ran ok
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Many thanks for the hard work.

              These changes have been spread upstream and are already available in the git and cvs repositories.

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao
              Hide
              upo Roberto Pinna added a comment -

              I think I understand why guest user was deleted.

              In /lib/cronlib.php (line 92-107) there is a check for incomplete users that try to delete users without firstname, lastname or email field valued.
              If admin enabled this check, guest user was deleted, before Petr patch. Now an exception is throw.

              The real problem is how guest user record is set. Firstname is "Guest user" but no lastname is set. We can do as for admin user with "Guest" as firstname and "user" as lastname.

              Ciao

              Show
              upo Roberto Pinna added a comment - I think I understand why guest user was deleted. In /lib/cronlib.php (line 92-107) there is a check for incomplete users that try to delete users without firstname, lastname or email field valued. If admin enabled this check, guest user was deleted, before Petr patch. Now an exception is throw. The real problem is how guest user record is set. Firstname is "Guest user" but no lastname is set. We can do as for admin user with "Guest" as firstname and "user" as lastname. Ciao
              Hide
              skodak Petr Skoda added a comment -

              Thanks Roberto! I have created a new issue MDL-40558 and submitted a patch for integration.

              Show
              skodak Petr Skoda added a comment - Thanks Roberto! I have created a new issue MDL-40558 and submitted a patch for integration.

                People

                • Votes:
                  1 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    10/Sep/12