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

Function get_users function at datalib.php does not remove exceptions

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide

      The $exceptions parameter to get_users isn't actually used anywhere in core that I can see.

      To test you'll need to edit an existing page or create a test page to demonstrate the issue. I've attached a page which demonstrates the fix.

      Show
      The $exceptions parameter to get_users isn't actually used anywhere in core that I can see. To test you'll need to edit an existing page or create a test page to demonstrate the issue. I've attached a page which demonstrates the fix.
    • Workaround:
      Hide

      It can be solved changing on datalib.php in the function get_users:

              //$except = " AND id $exceptions";

      for:

              $select .= " AND id $exceptions";

      The $except variable is not used in the function

      Show
      It can be solved changing on datalib.php in the function get_users: //$except = " AND id $exceptions"; for: $select .= " AND id $exceptions"; The $except variable is not used in the function
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31877-master-1

      Description

      Exceptions does not modifies the $select variable to remove the users selected from the query.

      It could be a security issue if it ignores some users to be assigned as admins for example.

        Gliffy Diagrams

        1. 31877.php
          0.9 kB
          Andrew Nicols

          Activity

          Hide
          dobedobedoh Andrew Nicols added a comment -

          This should cherry-pick cleanly to:

          • master
          • MOODLE_22_STABLE
          • MOODLE_21_STABLE
          Show
          dobedobedoh Andrew Nicols added a comment - This should cherry-pick cleanly to: master MOODLE_22_STABLE MOODLE_21_STABLE
          Hide
          phalacee Jason Fowler added a comment -

          Code looks good, and logical

          Show
          phalacee Jason Fowler added a comment - Code looks good, and logical
          Hide
          dobedobedoh Andrew Nicols added a comment -

          Integrators: This should cherry-pick cleanly to all stable branches and master

          Show
          dobedobedoh Andrew Nicols added a comment - Integrators : This should cherry-pick cleanly to all stable branches and master
          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for spotting that and sharing a solution. It should be integrated in the next couple of weeks.

          Show
          salvetore Michael de Raadt added a comment - Thanks for spotting that and sharing a solution. It should be integrated in the next couple of weeks.
          Hide
          pferre22 Pau Ferrer Ocaña (crazyserver) added a comment -

          Happy to read that! Thanks you all!

          Show
          pferre22 Pau Ferrer Ocaña (crazyserver) added a comment - Happy to read that! Thanks you all!
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Some hours ago...

          the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Some hours ago... the main moodle.git repository has been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks everyone this has been integrated now and will be available as part of the next releases on all 2.x branches.

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks everyone this has been integrated now and will be available as part of the next releases on all 2.x branches.
          Hide
          skodak Petr Skoda added a comment -

          tested with: $users = get_users(true, '', true, array(1, 3, 2));

          thanks

          Show
          skodak Petr Skoda added a comment - tested with: $users = get_users(true, '', true, array(1, 3, 2)); thanks
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          FCT (fixed, closing, thanks). Ciao

          "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
          ~ Benjamin Disraeli

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/12