Moodle
  1. Moodle
  2. MDL-27653

Incorrect $user check in is_enrolled() function

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.4, 2.1.1
    • Component/s: Libraries
    • Labels:
    • Rank:
      17330

      Description

      This happened to me on a most recent Moodle 2.0.3+ site. I clicked the reply link in e-mail:
      http://lang.moodle.org/mod/forum/post.php?reply=722

      Instead of being offered the page "Sorry, guests are not allowed to post.", the following exception was thrown:

      PHP catchable fatal error
      Debug info: Object of class stdClass could not be converted to string
      Stack trace:
      
          * line 359 of /lib/setuplib.php: coding_exception thrown
          * line ? of unknownfile: call to default_error_handler()
          * line 636 of /lib/dml/pgsql_native_moodle_database.php: call to pg_query_params()
          * line 1562 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_recordset_sql()
          * line 2967 of /lib/accesslib.php: call to moodle_database->record_exists_sql()
          * line 7636 of /mod/forum/lib.php: call to is_enrolled()
          * line 3209 of /lib/navigationlib.php: call to forum_extend_settings_navigation()
          * line 2617 of /lib/navigationlib.php: call to settings_navigation->load_module_settings()
          * line 583 of /lib/pagelib.php: call to settings_navigation->initialise()
          * line 599 of /lib/pagelib.php: call to moodle_page->magic_get_settingsnav()
          * line 2430 of /lib/navigationlib.php: call to moodle_page->__get()
          * line 2395 of /lib/outputrenderers.php: call to navbar->get_items()
          * line 65 of /theme/moodleofficial/layout/general.php: call to core_renderer->navbar()
          * line 650 of /lib/outputrenderers.php: call to include()
          * line 608 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
          * line ? of unknownfile: call to core_renderer->header()
          * line 1245 of /lib/setuplib.php: call to call_user_func_array()
          * line ? of unknownfile: call to bootstrap_renderer->__call()
          * line 91 of /mod/forum/post.php: call to bootstrap_renderer->header()
      

      Unfortunately I am not able to reproduce it any more, getting correct redirect to the login offer page now. Anyway, I tracked down the issue and I believe there is a mistake in is_enrolled() function. At the top of it, the parameter $user is checked to realize whether it is integer or object. In my situation, a global $USER with $USER->id = 0 was somehow passed (anonymous user). In this case, the $userid is evaluated into the passed $USER object instead of its id. The following code demonstrates it:

      function test_is_enrolled($user = null) {
          global $USER;
      
          // make sure there is a real user specified
          if ($user === null) {
              $userid = !empty($USER->id) ? $USER->id : 0;
          } else {
              $userid = !empty($user->id) ? $user->id : $user;
          }
      
          var_dump($userid);
      }
      
      $USER = (object)array('id' => 0);
      $usr = (object)array('id' => 2);
      
      test_is_enrolled();
      test_is_enrolled(0);
      test_is_enrolled(1);
      test_is_enrolled($usr);
      test_is_enrolled($USER);
      

      This test code outputs:

      int(0)
      int(0)
      int(1)
      int(2)
      object(stdClass)#1 (1) {
        ["id"]=>
        int(0)
      }
      

      As you can see, the last case when $USER with $USER->id set to integer 0 is not evaluated correctly.

        Activity

        Hide
        Michael de Raadt added a comment -

        I'll leave that on your plate.

        Michael;

        Show
        Michael de Raadt added a comment - I'll leave that on your plate. Michael;
        Hide
        Petr Škoda added a comment -

        Thanks a lot for the report!!

        Show
        Petr Škoda added a comment - Thanks a lot for the report!!
        Hide
        Sam Hemelryk added a comment -

        Thanks guys, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks guys, this has been integrated now
        Hide
        Andrew Davis added a comment - - edited

        This appears testable contrary to the current testing instructions. Sample test instructions could be something like.

        Check that guest access is enabled.
        Log in and go to a forum. Copy and paste the reply link which looks like http://lang.moodle.org/mod/forum/post.php?reply=722
        Log out and visit that URL. You should be redirected to the login page.
        Log in as guest and vist the saved URL. Check that you receive a message like "Sorry, guests are not allowed to post. Would you like to log in now with a full user account?"

        I'm leaving this as "testing in progress" pending confirmation from Petr.

        Show
        Andrew Davis added a comment - - edited This appears testable contrary to the current testing instructions. Sample test instructions could be something like. Check that guest access is enabled. Log in and go to a forum. Copy and paste the reply link which looks like http://lang.moodle.org/mod/forum/post.php?reply=722 Log out and visit that URL. You should be redirected to the login page. Log in as guest and vist the saved URL. Check that you receive a message like "Sorry, guests are not allowed to post. Would you like to log in now with a full user account?" I'm leaving this as "testing in progress" pending confirmation from Petr.
        Hide
        Petr Škoda added a comment -

        I was not able to reproduce it, the stack trace highlighted obvious bug caused by incorrect use of empty().

        Clicking on few moodle pages when logged in as student and teacher should be enough.

        Show
        Petr Škoda added a comment - I was not able to reproduce it, the stack trace highlighted obvious bug caused by incorrect use of empty(). Clicking on few moodle pages when logged in as student and teacher should be enough.
        Hide
        Andrew Davis added a comment - - edited

        Ok, passing. As ever, please add more detailed testing instructions (even if they just say to view a few pages and check you dont get any errors)

        Show
        Andrew Davis added a comment - - edited Ok, passing. As ever, please add more detailed testing instructions (even if they just say to view a few pages and check you dont get any errors)
        Hide
        Petr Škoda added a comment -

        Thanks everybody, this is now part of the weekly build.

        Show
        Petr Škoda added a comment - Thanks everybody, this is now part of the weekly build.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: