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

User sync should allow new accounts to be created with samaccountname rather than UPN #2157

Open
aspark21 opened this issue Aug 30, 2022 · 21 comments
Assignees
Labels
Issue type - enhancement request New feature being requested outside of original scope. Plugin - auth_oidc Status - In Progress WIP

Comments

@aspark21
Copy link

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:

  1. other systems & data integration rely on the username being the samaccountname
  2. user's UPN will change over time - e.g. students who become alumni retain their account but move into a different domain (e.g. username@alumni.university.ac.uk) with different licensing applied to their accounts, later they could become staff and change again to a different domain (e.g. username@staff.university.ac.uk) which means new UPN and new Moodle account - we want them to retain their access as the same user account & the upn association will need to adjust rather than the pther way around.

We would be willing to support the implementation of this feature in whichever way (our devs, funding, etc).

@weilai-irl weilai-irl self-assigned this Sep 1, 2022
@weilai-irl
Copy link
Collaborator

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 samaccountname field is not a standard field/attribute/feature in the OpenID Connect protocol, and even in AzureAD, it only applies to tenants synced with on-premises AD. From the plugins point of view, this would make a relatively small portion of use - for the vast majority of use cases, UPN would still be the more reliable user identifier.

Please let me know what you think.

Regards,
Lai

@weilai-irl weilai-irl added Issue type - help wanted General questions on how to use the plugins, e.g. configurations etc. Status - need more info Further information requested to triage the issue. Plugin - auth_oidc labels Sep 1, 2022
@aboarken
Copy link

aboarken commented Sep 3, 2022

@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.

@aspark21
Copy link
Author

aspark21 commented Sep 6, 2022

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 samaccountname tied to a consistently defined field in AAD?

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. :)

@nbozhkov-ucl
Copy link

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.

@weilai-irl
Copy link
Collaborator

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:

  • The auth_oidc plugin works with or without the local_o365 plugin. Both use cases need to be supported.
  • The auth_oidc plugin can connect to AAD or any other IdP implementing OpenID Connect. The change needs to ensure it doesn't affect the ability to use other IdP, or AAD where the samaccountname claim is not included.
  • When processing field mapping, user profile details can be acquired from either user ID token or via Graph API call. This is determined by a few factors, e.g. whether local_o365 plugin is installed and configured, whether the field mapping configuration uses remote fields that are unavailable in ID tokens etc. The change will need to make sure that the field can be acquired from both sources. Effectively this means the field will need to be returned in Graph API calls too.

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,
Lai

@nbozhkov-ucl
Copy link

Thanks @weilai-irl

We will review internally and start exploring options. As @aspark21 mentioned we are more than happy to support this implementation.

Cheers.
Nikola

@sharpchi
Copy link

sharpchi commented Oct 3, 2022

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.

@thebenkahn
Copy link

Hi Lai
Just adding in another voice that I think this is worth looking into supporting. In my experience, almost every higher education institution in the U.S. that wants to use the Microsoft integration into Moodle uses Azure AD with a connector/sync back to local AD. In most cases these schools either are still using or used in the past the core Moodle LDAP plugin for authentication and user creation and have their usernames standardized in samaccountname format. If and when the migrate to use the Microsoft plugins and OIDC, they essentially have to have a split where their existing users are named like "lai.student" and new users are named like "lai.student@example.edu"

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.

@weilai-irl weilai-irl added Issue type - enhancement request New feature being requested outside of original scope. and removed Issue type - help wanted General questions on how to use the plugins, e.g. configurations etc. labels Oct 14, 2022
@tomklageshau
Copy link

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!
Tom

@aspark21
Copy link
Author

aspark21 commented Dec 8, 2022

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
https://learn.microsoft.com/en-us/graph/api/resources/user?view=graph-rest-1.0

Just needs to be retrieved through the graph api

@johnmichaelbradley
Copy link

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.

@amorrowbellarmine
Copy link
Contributor

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.

image

@mmulrthelp
Copy link

mmulrthelp commented Mar 20, 2023

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
image

We did this by mapping it in using 'Employee id' /admin/settings.php?section=authsettingoidc as ID number
image

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'.
image

Regards,
Ray.

@weilai-irl
Copy link
Collaborator

Hi all,

I have made some changes recently which will hopefully help resolving this issue. There are two changes that are specifically relevant:

  1. "onPremisesSamAccountName" is now one of the remote fields that can be mapped to a Moodle profile fields. This is in PR Support syncing "onPremisesSamAccountName" from Azure AD #2302, Support syncing "onPremisesSamAccountName" from Azure AD #2303 and Support syncing "onPremisesSamAccountName" from Azure AD #2304.
  2. A full support of UPN change is to be introduced. This is in PR Support UPN change in Azure AD #2291, Support UPN change in Azure AD #2292 and Support UPN change in Azure AD #2293.

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,
Lai

@amorrowbellarmine
Copy link
Contributor

amorrowbellarmine commented Jun 13, 2023

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:

  1. Add the option to Moodle account creation to use the "upn split" version of an AAD user's UPN as the username for the Moodle user. Remove @domain.com from username at account creation #2305
  2. Add the option to OIDC authentication to split the UPN, removing @domain.com. The "split upn" is used to find a corresponding Moodle user with the same username. Remove @domain.com from UPN during authentication #2306

@weilai-irl
Copy link
Collaborator

weilai-irl commented Jun 13, 2023

Hi @amorrowbellarmine

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:

  1. In Azure AD, it's possible to have users with UPN from different domains; and it's possible for accounts from different domains to have the same username part, e.g. user1@domain1.com and user1@domain2.com. See my screenshot attached for an example. Effectively the two users are different objects in Microsoft, and have different GUID. We need to consider what to do when this case.

  2. I think we need to consider how to handle existing users who already have their UPN as username, and keep login working for them.

  3. If it ain't broke, don't fix it :)

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,
Lai

users_with_same_username_parts

@tomklageshau
Copy link

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
Tom

@weilai-irl
Copy link
Collaborator

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,
Lai

@smangancap
Copy link

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.

@aspark21
Copy link
Author

aspark21 commented Jun 6, 2024

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

@smangancap
Copy link

That's fantastic - thanks for confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue type - enhancement request New feature being requested outside of original scope. Plugin - auth_oidc Status - In Progress WIP
Projects
None yet
Development

No branches or pull requests