Moodle
  1. Moodle
  2. MDL-30787

review and fix use of capabilities for edit actions on main course page

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.2.1
    • Component/s: Groups
    • Labels:
    • Testing Instructions:
      Hide

      1/ bump up main version.php or execute "update_capabilities('moodle');"
      2/ create course with multiple activities
      3/ set up different overrides for 'moodle/course:manageactivities', 'moodle/course:activityvisibility', 'moodle/role:assign', 'moodle/course:update'
      4/ test if each capability is respected when course page in edit mode
      5/ repeat when courseajax disabled

      Show
      1/ bump up main version.php or execute "update_capabilities('moodle');" 2/ create course with multiple activities 3/ set up different overrides for 'moodle/course:manageactivities', 'moodle/course:activityvisibility', 'moodle/role:assign', 'moodle/course:update' 4/ test if each capability is respected when course page in edit mode 5/ repeat when courseajax disabled
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w51_MDL-30787_m23_coursemodedit
    • Rank:
      33724

      Description

      originally reported by Michael de Raadt in MDL30697

        Issue Links

          Activity

          Hide
          Petr Škoda added a comment - - edited

          This is much much bigger than I expected, I have found and fixed following:
          1/ permission overview for capability contexts now includes capabilities with component 'moodle' - that is those defined in main lib/db/access.php with context level CONTEXT_MODULE
          2/ 'moodle/course:activityvisibility' is now independent from other capabilities (used for module visibility) and is respected on the modedit form
          3/ the course/mod.php and course/rest.php finally use the same capability checks - this was very sloppy
          4/ courseajax now makes sure the buttons exists - this solves multiple issues with granular module capabilities
          5/ section hiding and moving is now using proper course update capability test
          6/ manageactivities is not required for module hiding, duplication and role link
          7/ make_editing_buttons is now always returning absolute links

          I would not consider this a security issue because all parts were protected by some teacher level capabilities, the problem was that overriding did not work at module level at all and sometimes inappropriate caps were used or there were some dead links - you needed to have course:update and manage:activities for pretty much anything.

          Simple cherry picking is not possible due to accesslib and renderer changes. I guess we can call this a "new feature" and backport to 2.2 only, right?

          Show
          Petr Škoda added a comment - - edited This is much much bigger than I expected, I have found and fixed following: 1/ permission overview for capability contexts now includes capabilities with component 'moodle' - that is those defined in main lib/db/access.php with context level CONTEXT_MODULE 2/ 'moodle/course:activityvisibility' is now independent from other capabilities (used for module visibility) and is respected on the modedit form 3/ the course/mod.php and course/rest.php finally use the same capability checks - this was very sloppy 4/ courseajax now makes sure the buttons exists - this solves multiple issues with granular module capabilities 5/ section hiding and moving is now using proper course update capability test 6/ manageactivities is not required for module hiding, duplication and role link 7/ make_editing_buttons is now always returning absolute links I would not consider this a security issue because all parts were protected by some teacher level capabilities, the problem was that overriding did not work at module level at all and sometimes inappropriate caps were used or there were some dead links - you needed to have course:update and manage:activities for pretty much anything. Simple cherry picking is not possible due to accesslib and renderer changes. I guess we can call this a "new feature" and backport to 2.2 only, right?
          Hide
          Petr Škoda added a comment - - edited

          There should not be any backwards compatibility issues - the only trouble could be course formats that are abusing the missing/incorrect access control, hopefully that should not matter much in 2.2.x because it is still very young...

          Show
          Petr Škoda added a comment - - edited There should not be any backwards compatibility issues - the only trouble could be course formats that are abusing the missing/incorrect access control, hopefully that should not matter much in 2.2.x because it is still very young...
          Hide
          Sam Hemelryk added a comment -

          Thanks Petr - this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Petr - this has been integrated now.
          Hide
          Sam Hemelryk added a comment -

          Tested during integration. Pass.

          Show
          Sam Hemelryk added a comment - Tested during integration. Pass.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

          Now... disconnect, relax and enjoy the next days, yay!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: