Feature: Make AD connector synchronization of userPrincipalName configurable

[matsimon]

Thank you for providing a pull request!

Please make sure you considered the following things

• I read the contribution guidelines.
• I read the code of conduct.
• I created a bug report in the Univention Bugzilla.
• I will add a bugzilla comment about this pull request.

Link to the issue in Bugzilla

https://forge.univention.org/bugzilla/show_bug.cgi?id=49954 (updated, initially pasted the URL for another issue)

Description of the changes

This is a proposal for making the mapping userPrincipalName configurable details are

• What kind of change does this PR introduce? Feature

• How can we reproduce the issue?

  • Have a UCS domain controller with AD connector configure for synchronization with a Windows Server AD (in my case Server 2012 R2, english, patched)
  • Put AD connector in write mode: ucr set connector/ad/mapping/syncmode='write' (For the sake of having comparable environments)
  • Apply the patch to init.py
  • Define the mapping attribute, i.e. mailPrimaryAddress: ucr set ad/mapping/user/userPrincipalNameAttribute='mailPrimaryAddress'
  • Enable highest debug level of the connector to see changes in the log: ucr set connector/debug/level='4'
  • Restart the univention AD connector: systemctl restart univention-ad-connector
  • Modify the mailprimaryAddress of a user via UDM Web or CLI
  • UPN should be updated in AD and the log output of the AD connector should contain mailPrimaryAddress instead of uid@kerberosrealm

• To which UCS version does the issue apply? 4.4-1 and before.

• Please list relevant screenshots, error messages, logs or tracebacks n/a yet

• Are there any breaking or API changes? The code was modified so that it shouldn't change default behaviour. However if a person purposely maps an attribute that is not single-valued or RFC 822 compliant, there are errors to be expected as that are AD's requirement for userPrincipalName.

• Are there changes in the documentation necessary? The variable should be mentioned in a changelog if the MR is accepted.

• Are there descriptions for newly introduced UCR variables? Yes, however the definitive variable name can be discussed :-)

Looking forward to hearing from you

[CLAassistant]

All committers have signed the CLA.

[matsimon]

Q: Is there another way of sending in the signed CLA other than by fax? I don't have that sort of device readily available to me anymore. That is the only indicated way of sending the signed CLA back.

[spaceone]

@matsimon You already signed it digitially at https://cla-assistant.io/univention/univention-corporate-server?pullRequest=13. That should be enough :-)

[matsimon]

Thanks @spaceone that was back with another employer so I wasn't certain I had to re-sign the CLA as individual person.

I'd be interested if you are even open to having the option mappable, based on a user-defined user Attribute, so please consider this merge request WiP.

Going through all of the additional checks that I outline below - maybe you come up with other things - it isn't going to be worth the time (i.e. my time) if you don't think that it is a of interest for an integration. However I'm thankful for pointers and hints, maybe I'm actually proposing something completely dangerous even though I do see an actual use case of mine.

There are some things that would need to be discussed, cleaned up or checked at more closely before integration anyway. For some I can provide test results, but maybe not for all. Possibly some additional tests would need to be written (if so, where?):

• Definitive name of the new UCR variable and likely fixing the description
• Does the attribute need to be checked for RFC 822 compliance and "single-valued-ness", or should we consider that if someone needs this option that they ought to know what they are doing?
• Behaviour / Regression checks that I can think of:
  • Need to check what happens when an no RFC 822 compliant attribute value is sent to AD
    • Using AD connector: (open)
    • Using samba-tool: Creation of a user is possible, native GUI tool ADUC don't crash or break their use. User-side breakage is not yet known (result based on testing done by @reqa, see [his comment]( * Possible negative impact: After some research)
    • Using native GUI tool ADUC: Creation (and modification) of users with non-conpliant UPNs is possible (you cannot save the user) (i.e. guybrush@threepwood@monkeyisland.example.org)
    • Using native PowerShell New-ADUser/Set-ADUser: (open)
  • Need to check what happens when the attribute is not single-valued
  • Need to check what happens (breaks for the user?) when a UPN suffix does not exist in the AD forest. In a vanilla AD you first add a UPN suffix using Active Directory Domains and Trusts or using PowerShell with Set-ADForest
    • Using AD connector: The userPrincipalName gets updated with whatever UPN Suffix is part of the AD UPN whether or not the UPN suffix was defined in the AD forest or not.
    • Using native GUI-based management tool AD Users and Computers: Doesn't permit creating users with UPN suffixes not registered in the forest
    • Using native PowerShell cmdlet New-ADUser: Allows creating users with UPN suffixes not registered in the forest. (confirmed on Server 2012 R2 and 2019, so MS doesn't consider it a bug) - See example snippet in my comment
    • Possible negative impact of using unregistered UPN suffixes: If the AD domain doesn't know about a UPN suffix routing authentication requests between trusted domains for users might not work for such users as the trusted domains don't know where to route that authentication request to. (did dome reading, but I don't have a trusted domain setup at hand to validate)

Edit 2019-08-04: Included some updated information about unregistered UPN suffixes and non-compliant UPNs where I've been able to do some testing.

[reqa]

Thanks for taking the time and effort to supply this input! While reading your list I quickly checked the AD schema for userPrincipalName and it says "Is-Single-Valued | True". Same for krb5PrincipalName in the OpenLDAP Schema. At least that point is easy :-)

Also I quickly started an Microsoft Windows 2008 R2 DC and created a user with a non-RFC822 value as UPN, and the AD accepted this:

root@mytestucs:~# samba-tool user create foo2 Univention.1 --upn=invalid \
                                            -H ldap://<IP of AD DC> -U Administrator%Some.Complex.Password
User 'foo2' created successfully

And the MS ADUC gui tool doesn't crash when opening this user :-) So I'd consider that a non-issue until proven wrong. Probably all things Kerberos things won't work well with such an account. Maybe AD uses a default REALM in that case (though it doesn't do it in the GUI). For example if you compare this to the servicePrincipalName attribute, that one is without the @realm part. I agree, somebody should test that at some point.

Anyway, thanks again! Maybe you can briefly explain the use case for this (e.g. as a bug comment), like, in which scenario this would help. E.g. synchronization with a AD domain that is part of a AD forest/tree. A "user story" like that helps the UCS product owner to estimate the priority of the bug reports.

[matsimon]

Hi Arvid

Thank you for your time on the feedback. I'll provide a "user story" in the bugzilla item since this is more likely to be given a look at by product owners?

Interestingly the PowerShell cmdlet New-ADUser doesn't prevent you from creating users with a UPN suffix not defined in the AD Forest, however I have no idea if that causes issues elsewhere in AD. Here is an example where the AD Forest has no UPN suffix configured like used here:

New-ADUser -Name "Threepwood, Guybrush" `
 -SamAccountName guybrush.threepwood `
 -UserPrincipalName guybrush.threepwood@monkeyisland.example.org

Edit: See expanded & updated test results in this comment.