Extracted code from sync_users() into separate functions
in order to enable ad-hoc sheduling, and chunking.
Co-authored-by: David Woloszyn <david.woloszyn@moodle.com>
In PHP 8.2 and later, setting a value to an undeclared class property is
deprecated and emits a deprecation notice.
So we need to add missing class properties that still need to be declared.
This commit removes code that only was being executed by php < 73
and it's 100% safe to do so because Moodle 3.11 and up require
php 73, hence it was not executed ever.
Removed code includes:
- ldap_control_paged_result and ldap_control_paged_result_response
(that were deprecated in php 73 and have been removed in php 80).
- conditional code in the session manager, where some hacks were
needed for php < 73. Note that this removes the private function
append_samesite_cookie_attribute() completely because it was
doinf nothing (first line was returning for php < 73).
- Also removed the old session.hash_function ini setting because
it was removed in php 71.
Kept code includes:
- The environmental check_igbinary322_version test has not been
removed because it doesn't hurt (always returns "ok" for php 73
sites) and doing it would involve to backport the environment.xml
file to 39 and 310. Instead, a note has been added to MDL-71747
in order to get rid of that check for 4.1 and up.
Starting with php74 the following functions are deprecated:
- ldap_control_paged_result()
- ldap_control_paged_result_response()
Starting with php73, ldap servercontrols were included. One of those
servercontrols, LDAP_CONTROL_PAGEDRESULTS, is the one in charge of
controlling paged results.
So, we are going to add some conditional code here:
1) if php < 7.3, use old paged result functions.
2) if php >= 7.3, switch to LDAP_CONTROL_PAGEDRESULTS servercontrol.
With a TODO about removing 1) in Moodle 4.1, once php73 becomes required.
CSRF protection for the login form. The authenticate_user_login function was
extended to validate the token (in \core\session\manager) but by default it
does not perform the extra validation. Existing uses of this function from
auth plugins and features like "change password" will continue to work without
changes. New config value $CFG->disablelogintoken can bypass this check.
get_cache_flags() is only useful when you want multiple responses or
you don't know the name/key for the value pair. We know the name/key
here is the session ID, so use get_cache_flag() instead and gain the
performance benefit.
The unit tests were failing because the upgrade step was missed.
In addition, there was a typo in a variable name, which meant that the
role was not actually synchronised.
While ldap_get_entries_moodle() PHPdocs state that it returns "array
ldap-entries with lower-cased attributes as indexes.", this is not true. It
uses ldap_get_attributes() internally, which returns both numerically indexed
attribute names, and dictionary-like entries indexed by attribute names.
Current code lowercases the dictionary-like entries, but then uses the
numerically indexed entries for the attribute names used as keys in the
returned array. The numerically indexed names might or might not be lowercased,
depending on the LDAP server and PHP version) version. E.g., OpenLDAP 2.x,
Novell eDirectory 8.x and MS Active Directory return mixed-cased attribute
names, and PHP 5.x and PHP 7.x don't lowercase them inside ldap_get_entries().
This is probably why all calls to ldap_get_entries_moodle() are followed by
calls to array_change_key_case(), even if that shouldn't be necessary.
So make sure we always return lower-cased attributs as indexes and add some
unit tests to avoid regressions in the future.
There is at least one LDAP server (Sun Directory Server) that doesn't
support Paged Results extension, even if it supports LDAP version 3. So
checking just for LDAP version is not enough.
If possible, we check the supportedControl attribute of the LDAP rootDSE
and see if the paged results control is available. This needs an LDAP
connection, which might not be possible to establish before we configure
some essential LDAP settings (server, bind user, password, etc.). Thus
we try to establish the connection and check the supportedControl
attribute. But if we fail, we perform only basic checks that are less
accurate and err on the side of cautiousness.
If there is a required custom field that the user can fill by editing
their profile, and that field is missing, the user should be considered
as not fully set up. Instead, we want to redirect them to edit their
profile first.
There are some exceptions when we want to fall back to the previous
behaviour and check just the name and email fields. These exceptional
cases include checking remote user data in incoming MNet request (no
user id, no custom fields supported) and calls to require_login() with
redirecting disabled (typically ajax filepicker requests on profile
editing page itself).
Additional plugins that call the function user_not_fully_set_up()
themselves, should perform the strict check in most/typical cases. So
the strict mode is enabled by default even if it changes the behaviour
slightly. In improbable case of additional plugins relying on the
previous behaviour of the function, they can use the $strict parameter
and keep performing the lax check. However, I am sure the correct fix in
that case will likely be to stop abusing this function.
Note that custom fields are not currently transferred during the MNet
roaming. So having custom fields configured as required on MNet service
provider site (where users can't edit their profiles) is expected to
display an error (as the site is considered as misconfigured).
dn with fine grained password should be used
to get msds-maximumpasswordage attribute.
No checks should be used to see if the array
key is set as entry should always return them.
When calling update_user_record() for auth_ldap the method
now has option to trigger event core\event\user_updated when
syncing with domain controller.
This means that the event will be triggered by sync_users()
but not by user_signup().
Srdjan