Custom fields data is stored on system context

[dmitriim]

Despite mod_cms custom fields are saved in activity/course context, it's always set as system see https://github.com/catalyst/moodle-mod_cms/blob/main/classes/customfield/cmsfield_handler.php#L60

This creates issues with file processing and permission checking.

E.g. After https://tracker.moodle.org/browse/MDL-79256 has been landed, even if a user has moodle/site:trustcontent capability in a course context, it's still set a field content to not trusted and some tags (e.g. iframes) are stripped.

The fix for this issue should also update context id for existing fields in mdl_customfield_data. Potentially figure out context from instanceid.

[marcusboon]

I'm not sure this is possible from what I understand since these fields are defined at the system level and basically reused in multiple (potentially all) courses so you won't really be able to set a specific contextid so permissions checking needs to be done another way

[dmitriim]

I may be missing something, but despite fields are in the system context, but data is saved on a course/activity context for a particular activity?

[marcusboon]

Ah ok so maybe this needs to change then https://github.com/catalyst/moodle-mod_cms/blob/main/classes/customfield/cmsfield_handler.php#L41 to pick up the specific context somehow

[dmitriim]

I think this one is ok. As it's where a field is actually created when we create a new content type. Which is on system context. But then when we actually add it to a course and create a new instance of mod_cms, this where the issue happens as there we should either check permissions in the course context when we add a new instance or in the activity context, when we editing data. Something like:

    /**
     * Get the context for this instance.
     *
     * @param int $instanceid the course module ID of module to get context for.
     *
     * @return \context
     * @throws \moodle_exception if no instance ID or course ID obtainable.
     */
    public function get_instance_context(int $instanceid = 0): \context {
        global $COURSE;

        if (!empty($instanceid)) {
            $context = \context_module::instance($instanceid, MUST_EXIST);
        } else if (empty($COURSE->id)) {
            throw new \moodle_exception('$COURSE->id was not defined, could not get context.');
        } else {
            $context = \context_course::instance($COURSE->id, MUST_EXIST);
        }

        return $context;
    }

[marcusboon]

Where's that change? Also is it the customfield_data that we should be setting the course context for? Or the customfield_field?

[jaypha]

It would be customfield_data.

[dmitriim]

@marcusboon @TomoTsuyuki @jaypha can you please confirm that this has been fixed as part of #119 ? Looks like it was