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

Potential problem with get_users_by_capability

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Roles / Access
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      I fixed the unit tests for accesslib (MDL-24842). However in the process I believe one of the failures to possibly indicate a genuine error. I was unable to reproduce this by setting up supposedly-equivalent values in the real system UI so it may or may not be that real problem.

      The test that fails is this one:

      // Prohibit on logged-in user should trump student/teacher allow.
      $this->assert(new ArraysHaveSameValuesExpectation(
      array()),
      array_map(create_function('$o', 'return $o->id;'),
      get_users_by_capability($contexts[3], 'mod/forum:viewrating')));

      It returns a number of users when it should return none. The situation is:

      • There is a Prohibit override at category level on the given capability
      • The users it returns have roles at course level
      • The test is at module level

      I went through most of the process but was unable to determine anything wrong with the setup code, however there is very likely something I missed.

      Finally I made it output the last SQL statement in get_users_by_capability. This is the thing that really concerns me:

      SELECT u.*
      FROM

      {user}

      u
      JOIN (SELECT DISTINCT userid FROM ( SELECT userid
      FROM

      {role_assignments}

      WHERE contextid IN (1,2,3,4)
      AND roleid IN (2,3,4)
      AND roleid NOT IN (6) ) us) ra ON ra.userid = u.id
      WHERE u.deleted = 0 AND u.id <> :guestid
      ORDER BY u.lastaccess

      The 'NOT IN (6)' is the prohibit check, but as you can see, there is no way this will work because each individual role, like say a role with 3 for roleid, does not have role 6.

      Role 6 in this case is logged-in user and i think it needs a special case so that it knows everyone has it, however my concern is, this query looks like being a problem even if it is not the logged-in user specialcase.

      Looks like a possible major problem with 'prohibit' feature (which is really important for us at least, we actually just had a problem with prisoners accessing things they shouldn't last week).

        Attachments

          Activity

            People

            Assignee:
            skodak Petr Skoda
            Reporter:
            quen Sam Marshall
            Tester:
            Nobody
            Participants:
            Component watchers:
            Amaia Anabitarte, Carlos Escobedo, Ferran Recio, Sara Arjona (@sarjona), Víctor Déniz Falcón
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Fix Release Date:
              24/Nov/10