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

Problem with language on login from Microsoft #221

Closed
dgmaxi1 opened this issue Jul 17, 2017 · 11 comments
Closed

Problem with language on login from Microsoft #221

dgmaxi1 opened this issue Jul 17, 2017 · 11 comments
Assignees
Labels
Issue type - bug Bugs in existing code that needs to be fixed.

Comments

@dgmaxi1
Copy link

dgmaxi1 commented Jul 17, 2017

Hi
Im having an issue with a Moodle with OAuth with Office365, the issue is that the language is always forced to english on the users that login from Microsoft (not with the ones created from Moodle). I tried desabling the Language field from the Office365 plugin, and nothing happened. I forced the language on the Moodle to spanish, I even changed the language from a user, and when he logs in it changes it to english again! And even the users in Office365 have spanish as their native language, so it makes no sense that it changes it to english when loggin to Moodle.

Moodle is the last version 3.3 and also the plugin, I installed it 2 weeks ago. Do you have a solution for this issue?

@GeraldST
Copy link

I can sadly confirm this issue - we have a German installation where we recently switched to OpenID and everytime a user is synced with Microsoft Azure (daily) the language settings in the personal profile are set back to English (EN). So basically all users are constantly reverted back to English - which is a big problem!

Pieterjan Heyse gave a hint to the problem here: https://moodle.org/plugins/local_o365

I guess that Commit MSFTMPP-627 created some kind of problem when hardcoding
$updateduser->lang = 'en';
on line 271 in file local/o365/classes/observers.php

@jamesmcq, in one of the former branches I saw something like

if (!empty($aaduserdata['preferredLanguage'])) {
$updateduser['lang'] = substr($aaduserdata['preferredLanguage'], 0, 2);
}

Is this the trick to respect an existing users preferred language?

@gunnar-restorff
Copy link

I can confirm this problem on Moodle 3.3. Every time a user logs in using O365, their default language gets changed to English and there is no way of preventing this. Our language (Faroese) is not available in O365, but it is in Moodle. I fixed this by commenting out

$updateduser->lang = 'en';

in the file local/o365/classes/observers.php (but then I have to remember to make this change every time I update Moodle - before any user logs in!). Another (somewhat) related issue is that we use the field "ID number". But every time a user logs in, it gets overwritten with blank - I fixed this by commenting out the line

$updateduser->idnumber = '';

in the same file.

@barry-matthias
Copy link

If you set the user mappings in the local o365 config and set language to on creation the users profile should remain at the language the user sets and ignore the hard coded english lang setting in the code?

@GeraldST
Copy link

I can see what you mean and I have to admit I haven't tested it yet (hard to do it on the live system). But I think it is definitely wrong to hard-code overwriting those to fields in the sync-routine (get_additional_user_info). I wouldn't want to sync the language at all.

@gunnar-restorff
Copy link

I haven't tried to make the mapping to see if it changes anything - this would anyway not solve my problem for two reasons: 1) The language of Office 365 is forced by the admin for all users, 2) our language (faroese) doesn't exist for Office 365.

I cannot see any good reason to hard-code overwriting these fields in the sync.

@jamesmcq
Copy link
Contributor

Hi All -

The language was originally included in these fields to ensure that users do have a language set in their profile, since language is a required Moodle field. As @barry-matthias mentioned, the user mappings should allow this value to be set from the Azure AD/Office 365 profile information.

Reading through the comments, it looks like there are a few cases where we don't have to explicitly set a language (updating a user, for example, probably doesn't need to ensure a language is included), so I'll look as some changes to improve this.

Thanks for the report!

@gunnar-restorff
Copy link

Hi jamesmcq

As mentioned, I just commented this out, and it works fine. The new users get our default language (Faroese) set upon creation, so I really cannot see any reason for having the line

$updateduser->lang = 'en';

in the file local/o365/classes/observers.php, since it works fine for us without this line.

@GeraldST
Copy link

GeraldST commented Jan 22, 2018

I have also commented out the "offending line" at the time of my first post and it works fine ever since.

I understand that for new users created by O365 a language needs to be set. But obviously it works without this line and I honestly find it very inappropriate to set it hard-coded to English. It should be set to the default language for the Moodle System anyway. Otherwise people like us that have languages like Faroese or German always run into problems.

In the case of updating a user no language needs to be set at all. The user already exists and therefore also a language setting exists. Overwriting this setting is not necessary, should be highly optional and could be accomplished by using the fields in the user mapping that you mentioned.

@jamesmcq
Copy link
Contributor

@gunnar-restorff @GeraldST Good to know you've each had success with this small change - I'll run it through tests over here, might be all that is necessary.

@jamesmcq
Copy link
Contributor

@gunnar-restorff @GeraldST Looks good over here, thanks all! I'll include this in our next release.

@jamesmcq jamesmcq self-assigned this Mar 30, 2018
@jamesmcq jamesmcq added Issue type - bug Bugs in existing code that needs to be fixed. resolved - to be released labels Mar 30, 2018
@jamesmcq jamesmcq added this to the Release 2018-3 milestone Apr 25, 2018
@jamesmcq
Copy link
Contributor

The latest release should resolve this. If you have any more issues on this, feel free to open a new issue. Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue type - bug Bugs in existing code that needs to be fixed.
Projects
None yet
Development

No branches or pull requests

5 participants