microsoft / o365-moodle

Office 365 and Azure Active Directory plugins for Moodle
GNU General Public License v3.0
180 stars 136 forks source link

Call Graph API function to update user details is skipped (caused by a bad get_config call?) #2624

Open jayuqar opened 1 month ago

jayuqar commented 1 month ago

Hi!

We updated our Moodle a couple of months ago and have just realized that certain fields in user profiles are no longer updating when they log in.

Here our setup before the update : Moodle 4.1.3 (Build: 20230424) Plugin local_o365 4.1.1 (2022112805) Plugin auth_oidc 4.1.1 (2022112805)

And after the update : Moodle 4.3.5+ (Build: 20240614) Plugin local_o365 4.3.3 (2023100915) Plugin auth_oidc 4.3.3 (2023100915)

We also have a test environment that is updated more regularly: Moodle 4.3.6+ (Build: 20240821) Plugin local_o365 4.3.5 (202310092) Plugin auth_oidc 4.3.4 (2023100920)

I’m fairly certain the synchronization was working correctly before the update, but now it’s not working in both our test and production environments.

After investigating, I think I may have found the root of the issue.

This line : https://github.com/microsoft/o365-moodle/blob/591e1debc7ce7f97129d06e3c3b28cbe369e47c2/auth/oidc/classes/loginflow/base.php#L127

was modified in December 2023: image

The commit in question : https://github.com/microsoft/o365-moodle/commit/9f84a45cdd5c14ed612d8f0cfa74650108b57b83

If I understand this code correctly, it compares the value of the configuration “microsofttenantid” with the tenant ID of the currently used token. If they are not the same, it assumes the user is from another tenant and synchronizes only the fields contained in the token (UPN, first name, last name, email). If the tenant IDs are the same, a call is made to the Graph API to retrieve more fields to sync in the Moodle user profile (e.g., city, department, etc.):

image

I realized that the call to the Graph API is never made because get_config('local_o365', 'microsofttenantid') always returns an empty value. I checked our mdl_config_plugin table, and this configuration ('local_o365', 'microsofttenantid') does not exist: image

Is it possible that this line (L127 of base.php) should be: $hostingtenantid = get_config('local_o365', 'entratenantid'); instead of $hostingtenantid = get_config('local_o365', 'microsofttenantid');

Thanks so much!

Syxton commented 3 weeks ago

This seems like a clear and obvious issue. +1