Moodle
  1. Moodle
  2. MDL-7968

check_access not defined in class part_of_admin_tree

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7
    • Fix Version/s: 1.9.2
    • Component/s: Roles / Access
    • Labels:
      None
    • Affected Branches:
      MOODLE_17_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      27621

      Description

      I wanted to set up a "researcher" role, who would have access to view information about our users and run reports, but not have any site configuration permissions. So I copied the admin role and switched off a few bits and pieces.

      When I tested it, I expected to see a cut down tree in the site admin block and be able to directly access report urls. However, I see no block, and get an access denied error if I call the reports directly.

      I've tracked it through the code, and it appears to be because there's a call to check_access in the part_of_admin_tree class in adminlib.php. This function simply throws a warning because it hasn't been implemented yet. The display code interprets this as return false and so doesn't think the user has the appropriate capabilities.

      There's a complicated comment for the function which appears to explain what should be done, but I'm not sure I understand it, so I didn't want to try to offer an implementation based on it.

        Activity

        Hide
        Jenny Gray added a comment -

        I've just returned to looking at this again, and realised that I've got completely the wrong end of the stick. What the problem really is, is that in admin/settings/top.php the setup for the reports part of the tree doesn't declare the capability to be tested, so it defaults to moodle/site:config when the admin_externalpage object is created.

        Given that there is a specific capability for reports, my feeling is that the following line should be changed to add a test for that capability:

        $ADMIN->add('reports', new admin_externalpage('report'.$plugin, get_string($plugin, 'admin'), "$CFG->wwwroot/$CFG->admin/report/$plugin/index.php",'moodle/site:viewreports'));

        This then acheives my desired goal of allowing the "researcher" role to only see the reports part of the admin tree.

        If some-one could check me, I'm more than happy to check this in, but didn't want to do so without confirmation in case I've missed the point again!

        Show
        Jenny Gray added a comment - I've just returned to looking at this again, and realised that I've got completely the wrong end of the stick. What the problem really is, is that in admin/settings/top.php the setup for the reports part of the tree doesn't declare the capability to be tested, so it defaults to moodle/site:config when the admin_externalpage object is created. Given that there is a specific capability for reports, my feeling is that the following line should be changed to add a test for that capability: $ADMIN->add('reports', new admin_externalpage('report'.$plugin, get_string($plugin, 'admin'), "$CFG->wwwroot/$CFG->admin/report/$plugin/index.php",'moodle/site:viewreports')); This then acheives my desired goal of allowing the "researcher" role to only see the reports part of the admin tree. If some-one could check me, I'm more than happy to check this in, but didn't want to do so without confirmation in case I've missed the point again!
        Hide
        Jenny Gray added a comment -

        Revisited and checked, and I still think this is the right way to go, so I'm committing it anyway. Hope that's OK!!

        Show
        Jenny Gray added a comment - Revisited and checked, and I still think this is the right way to go, so I'm committing it anyway. Hope that's OK!!
        Hide
        Petr Škoda added a comment -

        looks ok, +1

        Show
        Petr Škoda added a comment - looks ok, +1

          People

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

            Dates

            • Created:
              Updated:
              Resolved: