Regression caused by #715 leading to "Your email wasn't updated" in some configurations

[simoncoggins]

What happened?

I configured the saml2 plugin using the email address as the uid and also set "auth_saml2 | field_updatelocal_email" to "Every login".

On login as a user who did not yet exist locally, a warning was logged to the error logs and shown on screen to the user.

On screen: "Your email wasn't updated"

In logs: auth_saml2: update_user_record_from_attribute_map user 'testuser1@example.com' email can't be updated once set

The issue appears to be due to this commit:

https://github.com/catalyst/moodle-auth_saml2/commit/2952f4345b87194a3c150c38bd93dc29d3c647e4

From what I can tell this change:

-   $this->update_user_profile_fields($user, $attributes, $newuser);
+   $this->update_user_profile_fields($user, $attributes, false);

impacts the logic in update_user_record_from_attribute_map() because there is a check:

if ($newuser || $updateonlogin) {

which is still true when $updateonlogin is set, which has a further check for !$newuser that is now incorrectly triggered because $newuser is false even when it is a new user:

                                // We don't want Mapping Moodle field or username to be updated once they are set on user creation.
                                if (!$newuser) {
                                    if ($field == $this->config->mdlattr || $field == 'username') {
                                        $this->log(__FUNCTION__ .
                                            " user '$user->username' $field can't be updated once set");
                                        \core\notification::warning("Your $field wasn't updated");
                                        continue;
                                    }
                                }

What you expected:

No warning about not updating the field when the user is first created.

[simoncoggins]

Attaching screenshot:

Screenshot of issue. Note this was for prerelease testing in Totara 17 with PHP8.1/MariaDB10.8 but I believe this issue is due to the change mentioned above and will affect Moodle and other environments.

[danmarsden]

Nice catch thanks Simon! - I'm off on leave, but will see if someone can fix.

[peta3000]

Dear @danmarsden, dear @sumaiyamannan Thanks for documenting this issue. Although, this ticket was closed, I encounter the same behavior but instead of using email-address for mapping, we use "id-number": auth_saml2 | idpattr = idnumber auth_saml2 | mdlattr = ID number

Context:

• the issue came up after upgrading the Moodle instance from 3.9 to 4.1
• the saml2 plugin is updated to last commit of the default branch "MOODLE_39_STABLE", which already includes the fix of #730 https://github.com/catalyst/moodle-auth_saml2/tree/1992e3d1c8452fb5657921a63dd36e3c2e6d8e27

--> the warning-message resembles the one reported above: "your idnumber wasn't updated"

The SSO login works fine. Expected behavior (as above): Message is not shown.

Any help/clue is highly appreciated :)

Cheers

peta

[danmarsden]

@peta3000 please make sure you are using the latest version of the saml2 plugin from github here (version 2022111700) - then if you are still having issues, please create a new issue in the tracker here covering the details (feel free to link to this closed issue in your new issue)

thanks!