-
Notifications
You must be signed in to change notification settings - Fork 138
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
User sync should allow new accounts to be created with samaccountname rather than UPN #2157
Comments
Hi @aspark21, I'm afraid this may need to be done as a customisation to the release version of the plugins. If I understand it correctly, the Please let me know what you think. Regards, |
@weilai-irl You are totally right if we are assuming the use of Azure Active Directory but with using the Local Active Directory it is always required to use the "samaccountname" field when integrating with any 3rd-Party local applications. Can we make it optional to chose between UPN or samaccountname when doing an integration "If the attribute (samaccountname) is available on Azure"? Please correct me if I am wrong. Thank you. |
I'm not the Windows/O365/Identity person so apologies for my ignorance about AAD/OIDC implementation details on this. In the case where the AzureAD tenant is synced with on-prem AD (i.e. the vast majority of cases in the education space I would venture to guess) is the If it isn't, I guess some kind of site setting to select it would do the trick. This may not be the end goal but it will take most institutions a very long time to migrate to being AAD first where UPN rules, there's going to be a very long transition period where apps start using AAD/OIDC auth and then one day, on-prem AD will stop being the primary source of truth and AAD will take over but that could be a good decade off depending on the institution. So I really don't think this is an edge case. If anything it's a major factor preventing institutions from switching to OIDC and adopting the O365 plugin set. From all the universities I know in London we are the first to truly adopt the O365 set, one adopted AAD auth but via the auth_saml2 auth plugin. Which as a side note is the core blocker to anyone adopting the Teams integration, so when institutions have been asking for an LTI integration into Teams it's precisely to avoid adopting OIDC & this kind of problems with UPN vs samaccountname. Though yeah that's a requirement so lol, but we've put the effort in to get there. :) |
From my discussion with our AAD admins samaccountname is available in AAD and can be added to the claim. It should be relatively straightforward to add a setting in o365 plugin to make use of it. |
Hi @nbozhkov-ucl, Please be aware that the fact that the value can be made available in the token claim doesn't necessarily mean the change request will be processed. It simply means it may be technically feasible to achieve. As you can imagine, we receive a large number of change requests and we need to ensure that the effort required to make the change worths the value added by the change. So far UCL seems to be the only community user requesting this feature. We can put this feature request in the queue and process it accordingly, but it may need to wait in the queue for sometime unless more community users express interest in this one. Of course you are more than welcome to make the changes yourselves and submit pull request too, which will be merged into the release upon review. If you want to go down this route, please consider the following in your implementation:
From my experience, this change may not be as straightforward as you would have thought. As I said, I'll keep this feature request in the queue for now. Regards, |
Thanks @weilai-irl We will review internally and start exploring options. As @aspark21 mentioned we are more than happy to support this implementation. Cheers. |
I've looked at this on and off over the years tempted by the promise of integration between Moodle and o365. I think Alistair's explanation is accurate of the problem. When I realised that oicd was going to create new accounts with a different username and/or we'd have to let the users do their own account linking, it was just too much of a burden on us and our staff and students to bother with. Put that together with the complexity of setting this up (Moodle developers are not always embedded in IT and have the access rights to set things up in in Azure) to find the time to explore, let alone implement, it just gets pushed down the priority list. |
Hi Lai In some cases these intuitions need to feed data into other systems the LMS is integrated with and the split in username style presents challenges. Additionally, it makes intuitive sense to me that since the integration makes accommodations to match samaccountname style names and to Azure accounts during the user sync, it should allow admins the choice the create new accounts that use the same convention for username. |
If I could also add a voice to this in support of adding a mapping to the "On-premise samaccountname", I too face this problem, and had raised my own issue regarding this problem without realising this one existed. We too have a difference between usernames from Active Directory and UPN in AAD, but the username from AD is stored against the user in the "On-premise samaccountname" field in AAD. If we were able to map to this rather than UPN then the effect on other tool providers and the way in which they identify our users will be greatly reduced. I had considered whether to map this field to one of the extension attribute fields and pull it from there instead, but a direct mapping would be ideal. Here's a link to my logged issue, which can be removed in favour of supporting this one. If this gets enough support to be added, thanks in advance! |
Just a side note while exploring an unrelated issue, that there is a standard field in AAD - onPremisesSamAccountName - which has the data we're after Just needs to be retrieved through the graph api |
Please implement this request. The inability to set username to onPremisesSamAccountName on account creation means we simply cannot use these plugins. We have other systems and products integrated into our Moodle installation that require the username to be as it has been for several years: a username. |
The University I work for is in a similar situation. Due to integrations with LTI and other systems, we need to keep users' username in a sAMAccountName formation instead of UPN. Up until now we have relied on SAML authentication and mapped sAMAccountName to the eduPersonPrincipalName claim, which was used for a user's username. With AAD's SAML configuration it is possible to take a user's UPN and use the "ExtractMailPrefix()" transform to remove the @domain.com part. And it looks like Microsoft has added (as a preview) the option to more easily configure claims for OIDC. I'm not as well versed with working with OpenID as I am with AAD+SAML. Is there any way we can leverage these new features in AAD to modify the claim used for Moodle? With AAD+SAML it was possible to transform the nameID claim and remove the domain part of the UPN. |
I too work for a large UK university, we did this back in 2019 in order to use 'single sign on' on Moodle which these plugins offer, the way we did it was to change the existing accounts in the Moodle database to the Azure AD format of the User Principle Name (UPN) & any further created accounts from the Azure AD sync would then continue to be in this new format. Not sure if it will help anyone above, but here is the link explaining how we did it Our LTIs use the email address to be passed to the 3rd parties so we did not have the same issues as you do. But we do have the username available to us (we use this in our Coursework plugin), as in the start of the UPN before the @ symbol, shown here in the Moodle user profile under Optional We did this by mapping it in using 'Employee id' /admin/settings.php?section=authsettingoidc as ID number This is coming from the following field in our Azure AD, 'Employee ID' shown under 'Properties' which in our case is the same 'On-premise SAM account name'. Regards, |
Hi all, I have made some changes recently which will hopefully help resolving this issue. There are two changes that are specifically relevant:
Note all these PRs are work in process so far, which haven't been fully tested. We will perform full system tests before release, and there might be changes to the current implementation. While working on the changes, I explored the possibility of creating Moodle users using onPremisesSamAccountName too, but this doesn't seem to fit. The main reason is user creation would happen first based on information available in user ID token; only after the user creation is completed, we can check to see if calling Graph API to get additional user profile data is available, and only at this stage, the "onPremisesSamAccountName" may be fetched. It would be too late to set it to be the username of the user. In theory it's possible to update the username of the user at this stage, but there are a few other steps involved in this (updating token/connection records) etc, which seems that not too many clients would benefit. My suggestion is check if the two changes to be introduced in this release will help in any way. If this isn't enough, we can convert this to a formal feature request, and we can seek go-ahead from Microsoft to proceed it, or arrange alternative approval. I hope this makes sense to you. Regards, |
It's probably too late, but I was also working on a solution to this issue for our Moodle environment. Rather than mapping the sAMAccountName field through the OIDC claim, I added the option to remove the @domain.com portion of the username that is received from the claim. I think mapping the sAMAccountName attribute is a valid option to resolving this issue. When looking at the project's code, I discovered usersync already does a "upn split" to help with matching Moodle users to AAD users. I also discovered that if you use usersync to match existing Moodle users to AAD users, the OIDC authentication plugin will use that match (stored in the o365_objects table) to map the user to the correct Moodle user. So, a Moodle username not match an authenticating user's UPN as long as the usersync process is able to match them. I created two pull requests:
|
Thank you for making the pull request. To be honest, I have considered this feature a few times in the last years, the reasons that I didn't go ahead and implement it were:
Needless to say, both items 1 and 2 require consideration and testing. Could you confirm if the PRs handle both please. Once you are happy with them, we can conduct system testing on them, with the aim to integrate it back to release. Unfortunately I don't think we have enough time to finish this in time for this release due end of June/early July, but if everything goes smoothly, the next one planned for September is quite doable. Regards, |
Hi @weilai-irl, Thanks for your work on this. It's a shame however if we're not able to set the username to be the samaccountname from AAD for the reasons you've described, as it probably won't help us unfortunately. My main concern was how existing infrastructure (third party tools/services) will identify users through Moodle, as users already have accounts which exist in those systems using the samaccountname equivalent from Active Directory. Without a process to set or update the usernames, we'll probably start to see duplicated user accounts in third party apps if we end up implementing OIDC auth using this plugin. Thanks |
Hi all, The latest release from today includes the feature to map the "samaccountname" with Moodle profile fields. It is not exactly what this request is about, so I won't mark this as closed. The requested feature still needs to be investigated. Regards, |
Just wanted to add my comments to indicate this is a problem for our institution as well. We broke one of our integrations after we moved to Open_ID not realizing the implications of the change based on UPN vs samaccountname. Converting the other app to UPN isn't a viable option right now so we are looking to revert username creation via the automatic script back to samaccountname from UPN but alas like everyone else on this thread, cannot quite figure out how to do it. I see that onprem_sam_account_name does exist in the OpenID claims and we can return the value to a Moodle field by mapping to it, but I'm not sure how this helps us when it comes to the Azure sync. I'm assuming the change will have to happen on the IdP tenant side? Right now, our Azure Administrator cannot see a way to map onprem_sam_account_name to be the unique user identifier rather than the UPN. |
This feature is coming. We did fund it (took us a while to get that sorted) and it’s being tested at the moment. Watch this space |
That's fantastic - thanks for confirming! |
@weilai-irl sorry to be a bother.. you mentioned you were in testing so I'm just wondering if this feature will be implemented for September? This is when our next semester starts. If not, we may have to have a plan B. Thank you so much! |
@smangancap testing completed successfully a few days ago Lai is going to deal with the code merge and it should be in the next release where we’ll do a final re-test before implementing ourselves Probably plan your testing to start in early August @smangancap |
Hi all, This feature has been included in the release from yesterday. The new versions have been pushed to the related plugins in the Moodle plugins directory too. Please check them out. Regards, |
User sync should allow new accounts to be created with samaccountname rather than UPN.
Quite simply our users have been created from AD via ldap plugin and all of the usernames are based on samaccountname, they have been matched & migrated to auth_oidc but new accounts would not be following the same pattern, so after go live we will have to continue syncing/creating accounts via auth_ldap to then be matched & switched to auth_oidc - this is not particularly future proof, we should get rid of auth_ldap from the picture.
However, we do not want UPN to be the users' Moodle username - this is problematic on multiple levels:
We would be willing to support the implementation of this feature in whichever way (our devs, funding, etc).
The text was updated successfully, but these errors were encountered: