Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After Update (Moodle 4.1) warning message upon login: "Your idnumber wasn't updated" #753

Open
peta3000 opened this issue Mar 6, 2023 · 7 comments

Comments

@peta3000
Copy link

peta3000 commented Mar 6, 2023

What happened?

Actual Behavior:
After upgrading a Moodle instance from 3.9 to 4.1.1 (Build: 20230116), upon login via sso (saml2) users are greeted with the warning message:
"Your idnumber wasn't updated"

--> besides this, the SSO login via saml2 works fine, though

Expected behavior:
Message is not shown.

Version used:
2022111700
--> updated to commit 1992e3d on branch MOODLE_39_STABLE:
https://github.com/catalyst/moodle-auth_saml2/blob/1992e3d1c8452fb5657921a63dd36e3c2e6d8e27/version.php

Plugin-Settings:
(selection of some relevant mappings)
auth_saml2 | idpattr = idnumber
auth_saml2 | mdlattr = ID number
auth_saml2 | field_updatelocal_idnumber = On every login

Related issue:
A related issue (solved) with similar warning message but usage of user email-address for mapping (idpattr, mdlattr):
#728
Although this behavior was fixed for usage of email as mapping between idp and moodle, I was wandering whether this fix hasn't included other mapping-variants and whether this warning message could be disabled for all mapping-fields-variants?

Thanks a lot and best regards

peta

@peta3000
Copy link
Author

peta3000 commented Apr 6, 2023

Dear @danmarsden
I was wondering if there is any chance that a patch to resolve the documented issue might be included in one of the next releases of the plugin?
Since it seems to be closely related to the already resolved issue #728
fixed by #730
which in turn was caused by: #715

Many thanks and have a lovely day

peta

@peta3000
Copy link
Author

Dear @danmarsden
I totally understand that there is a huge pile of open issues demanding the dev's attention.
I would merely like to inquire, whether there are any chances that a patch for the reported issue wold be considered to be included in the future releases?

Many thanks and best regards

peta

@danmarsden
Copy link
Member

Hi @peta3000 - our ability to provide development support "for free" is pretty limited - we tend to fix bugs when we run into them ourselves, or when our clients hit the issues and we need to fix them.

If you know someone with development capabilities, feel free to submit a pull request and we'll take a look, otherwise feel free to reach out privately if you would like commercial level support and would like to fund improvements or bug fixes.

thanks!

@peta3000
Copy link
Author

Hi @danmarsden
Thanks a lot for taking the time to reply and to explain your reasons!

Cheers

peta

@sarahjcotton
Copy link
Contributor

Hi @peta3000 @danmarsden - We have also stumbled across this issue, and having looked at the various related issues, it feels to me as though the plugin is doing the right thing by throwing an error, however it might be better to stop that configuration happening in the first place.

I would like to propose updating the settings so that whatever is selected in mdlattr, the equivalent mapping is set to 'Update on creation' and that form element become locked/disabled. That way we shouldn't run into the scenario where the error is thrown, regardless of which value is selected for mdlattr (I noticed a similar case where the same thing happened when email was selected).

Although the email issue was fixed in PR 730, the fact that the 'Update on login' can still be set feels a bit misleading, hence the suggestion of enhancing the settings.

I hope that makes sense; thoughts welcome.
Sarah

@danmarsden
Copy link
Member

Thanks Sarah - I'd add a +1 - makes sense, but I have a vague recollection that this uses the settings api's - not mforms, and I don't think the settings api has a disabledif style handler so it might need a bit of work to implement. (didn't pull up the code though to verify)

@sarahjcotton
Copy link
Contributor

sarahjcotton commented Oct 5, 2023

Yes you're right... I spent a bit of time yesterday trying to force the current settings page to do what I wanted but kept hitting dead ends after getting part way there.

Edit: I've managed to get some JS to load on the settings page, so could manipulate the form fields that way if that's an acceptable approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants