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

          Attachments

            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 added a comment -

            Happy to read that! Thanks you all!

            Show
            pferre22 Pau Ferrer 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