Moodle
  1. Moodle
  2. MDL-7250

Roles: load_defaultuser_role can set 'inherit' into $USER->capabilities, should leave unset

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7, 1.8
    • Fix Version/s: 1.7, 1.8
    • Component/s: Roles / Access
    • Labels:
      None
    • Affected Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE
    • Rank:
      27866

      Description

      The defaultuser role can load 0 (inherit) into $USER->capabilities, which shouldn't happen. (Basically if you have something set for a context then it stops looking at parents.) At least on our ystem this caused problems.

      I pasted the patch below, but will check it in too.

      Index: lib/accesslib.php
      ===================================================================
      RCS file: /cvsroot/moodle/moodle/lib/accesslib.php,v
      retrieving revision 1.166.2.14
      diff -u -r1.166.2.14 accesslib.php
      — lib/accesslib.php 26 Oct 2006 09:42:58 -0000 1.166.2.14
      +++ lib/accesslib.php 27 Oct 2006 16:07:48 -0000
      @@ -130,7 +130,8 @@
      if ($capabilities = get_records_select('role_capabilities',
      "roleid = $CFG->defaultuserroleid AND contextid = $sitecontext->id")) {
      foreach ($capabilities as $capability) {

      • if (!isset($USER->capabilities[$sitecontext->id][$capability->capability]))
        Unknown macro: { // Don't overwrite+ if (!isset($USER->capabilities[$sitecontext->id][$capability->capability]) + && $capability->permission) { // Don't overwrite if it's already set, nor store 'inherit' $USER->capabilities[$sitecontext->id][$capability->capability] = $capability->permission; } }

        Activity

        Hide
        Sam Marshall added a comment -

        ...Although actually, since this is for the site context, there shouldn't be a parent, so it shouldn't matter. Hrm. Still, in our system having zeros there screwed things up.

        Show
        Sam Marshall added a comment - ...Although actually, since this is for the site context, there shouldn't be a parent, so it shouldn't matter. Hrm. Still, in our system having zeros there screwed things up.
        Hide
        Sam Marshall added a comment -

        OK, following Martin's suggestion I actually changed it to put the extra condition in the SQL. (I also managed to make an error along the way... sorry for all the commits.)

        I'm still not clear why this caused problems, but the change should do no harm also, so...

        Show
        Sam Marshall added a comment - OK, following Martin's suggestion I actually changed it to put the extra condition in the SQL. (I also managed to make an error along the way... sorry for all the commits.) I'm still not clear why this caused problems, but the change should do no harm also, so...

          People

          • Assignee:
            Sam Marshall
            Reporter:
            Sam Marshall
            Tester:
            Nobody
            Participants:
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: