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

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

Closed
jayuqar opened this issue Aug 27, 2024 · 3 comments · Fixed by #2654, #2655, #2656 or #2657
Assignees
Labels
Feature - user sync Issue type - bug Bugs in existing code that needs to be fixed. Plugin - auth_oidc Plugin - local_o365 Status - PR ready / pending release Dev is done and PR ready. Will be included in the next release.
Milestone

Comments

@jayuqar
Copy link

jayuqar commented Aug 27, 2024

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 :

$hostingtenantid = get_config('local_o365', 'microsofttenantid');

was modified in December 2023:
image

The commit in question :
9f84a45

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
Copy link

Syxton commented Sep 12, 2024

This seems like a clear and obvious issue. +1

@weilai-irl
Copy link
Collaborator

Hi @jayuqar

Thank you for reporting the issue. The fix to the issue has been included in the release from yesterday. Please check it out.

Regards,
Lai

@jayuqar
Copy link
Author

jayuqar commented Oct 18, 2024

Thanks so much, @weilai-irl !
I'll be back at work in a couple of weeks, I'll test that and keep you posted once it's done ! :)

Sincerely,
Jay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment