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

calendar_event::update() should not do capability checks

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Won't Fix
    • Affects Version/s: 2.2.2
    • Fix Version/s: None
    • Component/s: Calendar
    • Labels:
    • Affected Branches:
      MOODLE_22_STABLE

      Description

      This is a tricky issue that was spotted while working on MDL-32630. The calendar_event::update() by default calls calendar_add_event_allowed(). While this is appropriate when dealing with manually created events, doing such check is wrong for events created/updated by module themselves. Imagine the following scenario:

      • the user has capability to edit the assignment settings
      • but the same user does not have the permissions to modify the course calendar (moodle/calendar:manageentries)

      What should happen now when the user modifies the assignment due date in the settings form? The common sense says that the event should be updated in the calendar as it is actually the assignment updating the event, not the user. And having different date in the assignment and in the calendar would definitely be wrong for students' experience.

      Sooo... This is a proposal to move the permission checks completely from the calendar_event API to the caller's scope where appropriate (that is call calendar_add_event_allowed() bascially only when dealing with manually created events.

      Not convienced yet? Here are some arguments:

      • As a general rule, libraries should not check for permissions, it is the caller's duty
      • While calendar_event::update() checks for the capabilities, calendar_event::delete() ignores them completely anyway so even the current behaviour is not ideal
      • The checks in calendar_add_event_allowed() are based on $event->eventtype but that field can have literally any value for events created by modules themselves. So calling that functions from {module}

        _add_instance() is potentially buggy anyway.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                • Votes:
                  2 Vote for this issue
                  Watchers:
                  4 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: