Moodle

User profile fields are overwritten by default value when user updates profile even if it's locked

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.8.8, 1.9.4
  • Fix Version/s: 1.8.9, 1.9.5
  • Component/s: Authentication
  • Labels:
    None
  • Difficulty:
    Easy
  • Affected Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_18_STABLE, MOODLE_19_STABLE

Description

Following scenario

  • user profile field created
  • defaullt value for user profile field specified
  • user profile field locked
  • user profile field visible to user

With these properties set, when a user edits the profile following happens

  • instead of displaying the actual value of the user profile field, the default value ist displayed in the form
  • when user saves the profile, the default value replaces the actual value of the user profile field

So these are actually two bugs, but the second bug could be a security issue, because the Capabilities of the user aren't respected (prevent from updating the user profile field, even if it's locked)

This bug is reproducable (tested on two different systems)

The issue could be there (/user/profile/field/lib.php)

function edit_field_set_locked(&$mform) {
if ($this->is_locked() and !has_capability('moodle/user:update', get_context_instance(CONTEXT_SYSTEM))) { $mform->hardFreeze($this->inputname); $mform->setConstant($this->inputname, $this->data); }
}

and there (/user/profile/field/lib.php)

function edit_field_set_default(&$mform) {
if (!empty($default)) { $mform->setDefault($this->inputname, $this->field->defaultdata); }
}

Attachments

  1. Text File
    field_menu_lock.patch
    10/Mar/09 7:24 PM
    1 kB
    Vlas Voloshin
  1. edit_profile.gif
    12 kB
    25/Feb/09 11:43 PM
  2. user_progile_editing.gif
    34 kB
    25/Feb/09 11:43 PM

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
David Bogner added a comment -

First picture are the settings of the specific user profile field
The second picture shows the default vaule displayed in user/edit.php. When the user profile is updated the actual value of user profile field is overwritten by default value

Show
David Bogner added a comment - First picture are the settings of the specific user profile field The second picture shows the default vaule displayed in user/edit.php. When the user profile is updated the actual value of user profile field is overwritten by default value
Hide
Permalink
David Bogner added a comment -

There is a discussion concerning the bug there: http://moodle.org/mod/forum/discuss.php?d=117357

Show
David Bogner added a comment - There is a discussion concerning the bug there: http://moodle.org/mod/forum/discuss.php?d=117357
Hide
Permalink
Anthony Borrow added a comment -

I've not confirmed this but I am bumping the priority because it results in a loss of data. Peace - Anthony

Show
Anthony Borrow added a comment - I've not confirmed this but I am bumping the priority because it results in a loss of data. Peace - Anthony
Hide
Permalink
Vlas Voloshin added a comment -

It seems I've solved the issue, see http://moodle.org/mod/forum/discuss.php?d=117357#p518127 for details. Shortly, setConstant sets element to invalid value, which is field's default. The reasonable solution is to comment out or delete that line in /user/profile/field/lib.php. I don't think that setConstant is needed after hardFreeze at all, but I'm not sure.

Show
Vlas Voloshin added a comment - It seems I've solved the issue, see http://moodle.org/mod/forum/discuss.php?d=117357#p518127 for details. Shortly, setConstant sets element to invalid value, which is field's default. The reasonable solution is to comment out or delete that line in /user/profile/field/lib.php. I don't think that setConstant is needed after hardFreeze at all, but I'm not sure.
Hide
Permalink
Petr Škoda (skodak) added a comment -

constants and freezing is kind of broken in current formslib
going to have a look at this next week

Show
Petr Škoda (skodak) added a comment - constants and freezing is kind of broken in current formslib going to have a look at this next week
Hide
Permalink
Vlas Voloshin added a comment -

Well, that's a pity Hope my solution will do the trick, at least temporary, for those who are interested.

Show
Vlas Voloshin added a comment - Well, that's a pity Hope my solution will do the trick, at least temporary, for those who are interested.
Hide
Permalink
Petr Škoda (skodak) added a comment -

should be fixed now, I have moved the locking to definition_after_data where userid is known, please test
thanks a lot for the report and all info!

Show
Petr Škoda (skodak) added a comment - should be fixed now, I have moved the locking to definition_after_data where userid is known, please test thanks a lot for the report and all info!
Hide
Permalink
Vlas Voloshin added a comment -

Petr, there's another problem here - as we still use setConstant in edit_field_set_locked, we must redefine this function in derived field classes! This is mostly true for menu, not sure if it's necessary for other field classes.
The problem is here:
$mform->setConstant($this->inputname, $this->data);
but $this->data holds actual profile data. In menu profile field type, we have to set data index, not the actual data:
$mform->setConstant($this->inputname, $this->datakey);
I'm attaching a patch to fix it, hope you'll see this.

Show
Vlas Voloshin added a comment - Petr, there's another problem here - as we still use setConstant in edit_field_set_locked, we must redefine this function in derived field classes! This is mostly true for menu, not sure if it's necessary for other field classes. The problem is here: $mform->setConstant($this->inputname, $this->data); but $this->data holds actual profile data. In menu profile field type, we have to set data index, not the actual data: $mform->setConstant($this->inputname, $this->datakey); I'm attaching a patch to fix it, hope you'll see this.
Hide
Permalink
Petr Škoda (skodak) added a comment -

reopening, yes - this may be a problem, going to have a look at that today - thanks a lot!

Show
Petr Škoda (skodak) added a comment - reopening, yes - this may be a problem, going to have a look at that today - thanks a lot!
Hide
Permalink
Petr Škoda (skodak) added a comment -

Thanks a lot, your patched worked fine - now committed into cvs.

Show
Petr Škoda (skodak) added a comment - Thanks a lot, your patched worked fine - now committed into cvs.
Hide
Permalink
Jerome Mouneyrac added a comment - - edited

Tested on 1.9/1.8 as following:
As admin, create a field with specified settings from the issue description
As admin, set a different field value for a student
As student edit profil, save without changing the field value
=> The default value was neither displayed, neither saved. It works fine.

Thanks everybody

Show
Jerome Mouneyrac added a comment - - edited Tested on 1.9/1.8 as following: As admin, create a field with specified settings from the issue description As admin, set a different field value for a student As student edit profil, save without changing the field value => The default value was neither displayed, neither saved. It works fine. Thanks everybody

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.