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

Undeletable block should not be deletable!

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.5, 2.3.2
    • Fix Version/s: 2.4
    • Component/s: Blocks
    • Labels:
    • Testing Instructions:
      Hide

      1. Add a HTML block to the site font page and set it to be visible on all pages throughout the site.

      2. Add a HTML block to a course page.

      3. Check that both blocks currently have a delete icon. (Ideally, prove to yourself that you can delete and re-create them.)

      4. Go to Admin -> Plugins -> Blocks -> Manage blocks and click the lock icon for the HTML block.

      5. Check that both blocks you added above do not have a delete icon.

      6. Try to hack the system by taking the URL for the delete icon of another block, and editing it to delete one of the HTML blocks. (Change the bui_deleteid to the block_instances.id of the HTML block.) Make sure you get a permission denied error.

      7. Turn editing on in a coures. Make sure you are not offered the opportunity to create a HTML block.

      (8. You probably want to unlock the HTML block type now!)

      Show
      1. Add a HTML block to the site font page and set it to be visible on all pages throughout the site. 2. Add a HTML block to a course page. 3. Check that both blocks currently have a delete icon. (Ideally, prove to yourself that you can delete and re-create them.) 4. Go to Admin -> Plugins -> Blocks -> Manage blocks and click the lock icon for the HTML block. 5. Check that both blocks you added above do not have a delete icon. 6. Try to hack the system by taking the URL for the delete icon of another block, and editing it to delete one of the HTML blocks. (Change the bui_deleteid to the block_instances.id of the HTML block.) Make sure you get a permission denied error. 7. Turn editing on in a coures. Make sure you are not offered the opportunity to create a HTML block. (8. You probably want to unlock the HTML block type now!)
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This may be contentious, we (the OU) think the logic that was implemented in MDL-27122 is wrong.

      The logic currently is:

      if (!in_array($block->instance->blockname, $undeletableblocktypes)
              || !in_array($block->instance->pagetypepattern, array('*', 'site-index'))
              || $block->instance->parentcontextid != SITEID) {
          // Delete icon.
          $controls[] = array('url' => $actionurl . '&bui_deleteid=' . $block->instance->id,
                  'icon' => 't/delete', 'caption' => get_string('delete'), 'class' => 'editing_delete');
      }

      what that means is that a block is only protected from deletion if all of these are true:

      1. The type of block is protected
      2. The block's parent context is the system context
      3. The block is set to show on all pages, or just on the site index page.

      This causes problems. Steps to reproduce:

      1. Create a new type of block that implements some special feature.
      2. As admin, carefully set up one instance of this block as follows:
        • parent context: system context
        • show in that context and subcontexts
        • page type pattern course-view-*
      3. As a stupid person, who has rashly been trusted with Manager role, go to any course page. Decide you don't like that block, and click its delete icon.

      Expected result: actually, that block should not have had a delete icon at all.

      Actual result: the global instance of the block is deleted (again), admin cries.

      I cannot see any problem if we change the logic to just

      if (!in_array($block->instance->blockname, $undeletableblocktypes)) {
          // Delete icon.
          $controls[] = array('url' => $actionurl . '&bui_deleteid=' . $block->instance->id,
                  'icon' => 't/delete', 'caption' => get_string('delete'), 'class' => 'editing_delete');
      }

      If we do this, we should also change get_addable_blocks so you cannot add blocks that you are then unable to delete.

      If you ever do need to change the protected block instances, the admin can temporarily un-protect that type of block.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              timhunt Tim Hunt
              Reporter:
              timhunt Tim Hunt
              Peer reviewer:
              Martin Dougiamas Martin Dougiamas
              Integrator:
              Dan Poltawski Dan Poltawski
              Tester:
              Rossiani Wijaya Rossiani Wijaya
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias, Sujith Haridasan
              Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                3/Dec/12