Moodle
  1. Moodle
  2. MDL-30697

Wrong capability check for role assign button in course?

    Details

    • Rank:
      33522

      Description

      In /course/lib.php, line 3177, there is the output for the "assign role" button which will be shown in a course in editing mode near every activity:

          if (has_capability('moodle/course:managegroups', $modcontext)){
              $context = get_context_instance(CONTEXT_MODULE, $mod->id);
              $assign = '<a class="editing_assign" title="'.$str->assign.'" href="'.$CFG->wwwroot.'/'.$CFG->admin.'/roles/assign.php?contextid='.
                  $context->id.'"><img src="'.$OUTPUT->pix_url('i/roles') . '" alt="'.$str->assign.'" class="iconsmall"/></a>';
          } else {
              $assign = '';
          }
      

      I think the capability check is wrong as this button has nothing to do with groups. Shouldn't it be:

          if (has_capability('moodle/role:assign', $modcontext)){
              $context = get_context_instance(CONTEXT_MODULE, $mod->id);
              $assign = '<a class="editing_assign" title="'.$str->assign.'" href="'.$CFG->wwwroot.'/'.$CFG->admin.'/roles/assign.php?contextid='.
                  $context->id.'"><img src="'.$OUTPUT->pix_url('i/roles') . '" alt="'.$str->assign.'" class="iconsmall"/></a>';
          } else {
              $assign = '';
          }
      

      ?

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that. I think you are correct.

          Also, the managegroups capability check should be being applied to the group mode toggle icon immediately before the roles icon in that file.

          Show
          Michael de Raadt added a comment - Thanks for reporting that. I think you are correct. Also, the managegroups capability check should be being applied to the group mode toggle icon immediately before the roles icon in that file.
          Hide
          Petr Škoda added a comment -

          Hello,

          I am lowering the severity and removing security level because the button itself does not give users rights to modify the roles - worst case scenario is that they get button pointing to page they can not access.

          The second issue with groups is a bit more tricky because the standard groupmode switch and ajax script use different access control, I am going to create a new issue for that and link it from here.

          Show
          Petr Škoda added a comment - Hello, I am lowering the severity and removing security level because the button itself does not give users rights to modify the roles - worst case scenario is that they get button pointing to page they can not access. The second issue with groups is a bit more tricky because the standard groupmode switch and ajax script use different access control, I am going to create a new issue for that and link it from here.
          Hide
          Petr Škoda added a comment -

          Thanks a lot for the report and proposed solution.

          Petr

          Show
          Petr Škoda added a comment - Thanks a lot for the report and proposed solution. Petr
          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 review. Passed.

          Show
          Sam Hemelryk added a comment - Tested during integration review. Passed.
          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: