Bug 36480

Summary: Handle additional (possibly duplicate) phone numbers
Product: UCS Reporter: Michael Grandjean <grandjean>
Component: AD ConnectorAssignee: Samba maintainers <samba-maintainers>
Status: REOPENED --- QA Contact: Felix Botner <botner>
Severity: normal    
Priority: P5 CC: best, gohmann, oyen, requate
Version: UCS 5.0Flags: oyen: Patch_Available+
Target Milestone: ---   
Hardware: Other   
OS: Linux   
See Also: https://forge.univention.org/bugzilla/show_bug.cgi?id=31172
https://forge.univention.org/bugzilla/show_bug.cgi?id=45252
https://forge.univention.org/bugzilla/show_bug.cgi?id=49092
https://forge.univention.org/bugzilla/show_bug.cgi?id=51592
https://forge.univention.org/bugzilla/show_bug.cgi?id=18501
What kind of report is it?: Bug Report What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 1: Will affect a very few installed domains How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.034 Enterprise Customer affected?: Yes
School Customer affected?: ISV affected?:
Waiting Support: Flags outvoted (downgraded) after PO Review:
Ticket number: 2014093021000791 Bug group (optional): Roadmap discussion (moved)
Max CVSS v3 score:
Bug Depends on: 18501, 45220, 45249    
Bug Blocks: 42524    
Attachments: Screenshot of how to set additional phone number in AD
Excerpt from connector.log with traceback
Patch: ad-connector deduplicate UCS LDAP attribute values

Description Michael Grandjean univentionstaff 2014-11-06 12:46:24 CET 
Created attachment 6314 [details]
Screenshot of how to set additional phone number in AD

In AD it possible to provide more than one phone number per 'attribute' (see attached screenshot). These are saved as follows:

> root@ucs:~# univention-adsearch CN="Ada Lovelace" | grep -i phone
> otherHomePhone: 0421 555666
> otherHomePhone: 0421 333444
> homePhone: 0421 333444

Please note that for the additional numbers, the additional attribute "otherHomePhone" is used. Because of this, it is possible to provide a 'duplicate' here: "0421 333444" is given two times.

Unfortunately, this brakes the AD-Connector sync (see attached logfile), since OpenLDAP uses only "homePhone":

> root@ucs:~# univention-ldapsearch uid=ada -LLL | grep -i phone
> homePhone: 0421 333444
> homePhone: 0421 555666

Reported via Ticket#2014093021000791, verified with UCS 3.2-3 errata 234 and Windows Server 2012 R2.
Comment 1 Michael Grandjean univentionstaff 2014-11-06 12:49:02 CET 
Created attachment 6315 [details]
Excerpt from connector.log with traceback
Comment 2 Lukas Oyen univentionstaff 2017-01-30 18:52:58 CET 
Created attachment 8385 [details]
Patch: ad-connector deduplicate UCS LDAP attribute values

This patch deduplicates the AD-attributes on sync to UCS, while preserving
ordering. This assumes, that the order of multi-valued attributes in OpenLDAP
is preserved.

The S4-Connector takes another approach. It deduplicates AD-attributes without
preserving ordering. On sync from UCS -> AD it corrects for the lost ordering
by a special case for `con_other` cases. This extra code is ommitted in this
patch.
Comment 3 Stefan Gohmann univentionstaff 2017-06-16 20:39:57 CEST 
This issue has been filed against UCS 3. UCS 3 is out of the normal maintenance and many UCS components have vastly changed in UCS 4.

If this issue is still valid, please change the version to a newer UCS version otherwise this issue will be automatically closed in the next weeks.
Comment 4 Lukas Oyen univentionstaff 2017-07-27 14:29:31 CEST 
Committed in r81433 (advisory r81449).
Comment 5 Lukas Oyen univentionstaff 2017-08-23 14:35:30 CEST 
Updated in r82432-r82435, tests r82436-r82439, YAML: r82445

Note, that this slightly changes the behaviour of the `con_other_attribute` sync. See the new test-case 55_adconnector/502_other_attribute_sync.py or the following comment from `univention-ad-connector/modules/univention/connector/ad/__init__.py` for details.

> # This is the case, where we map from a multi-valued UCS attribute to two AD attributes.
> # telephoneNumber/otherTelephone (AD) to telephoneNumber (UCS) would be an example.
> #
> # In Active Directory, for attributes that are split in two the administrator is
> # responsible for keeping a value in `telephoneNumber`. Imagine the following:
> # (a) telephoneNumber = '123', otherTelephone = ['123', '456']
> # In this case, if the administrator deletes the value of `telephoneNumber`,
> # Active Directory does NOT automatically pull a new value from `otherTelephone`.
> #
> # This is impossible to support with the connector. Imagine again case (a). If
> # we delete `123` from `phone` via UDM, AD would be synced into the following
> # state: (b) telephoneNumber = '', otherTelephone = ['456']
> # From now on, whenever we add a new value to `phone` via UDM, for example:
> # (c) phone = ['456', '789'] it MUST be synced as
> # (d) telephoneNumber = '', otherTelephone = ['456', '789'] as '456' came
> # before '789' and '456' is definitely in `otherTelephone`.
> #
> # We therefore implement, that `telephoneNumber` is never empty, as long as there
> # are values in `otherTelephone`. If a modification would delete the value of
> # `telephoneNumber` and at least one value exists in `otherTelephone`, the
> # connector duplicates the first entry of `otherTelephone` into
> # `telephoneNumber`.
Comment 6 Stefan Gohmann univentionstaff 2017-09-10 14:55:44 CEST 
I've disabled the test case 55_adconnector/502_other_attribute_sync.py in AD member mode. See for example:
http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/ADMemberMultiEnv/1/Mode=installation,Version=w2k8r2-english/testReport/55_adconnector/502_other_attribute_sync/test/
Comment 7 Arvid Requate univentionstaff 2017-10-24 18:02:33 CEST 
The patches for this bug depend on the patches for bug 18501. Since QA demands revert of the changes for Bug 18501 I have to revert these too.
Comment 8 Felix Botner univentionstaff 2017-10-25 14:45:48 CEST 
(In reply to Arvid Requate from comment #7)
> The patches for this bug depend on the patches for bug 18501. Since QA
> demands revert of the changes for Bug 18501 I have to revert these too.

OK f3015eda53cbedffb433ee7b428a5f49b5ac43b4
Comment 9 Stefan Gohmann univentionstaff 2017-12-19 09:28:00 CET 
It was not part of the voting process, so removing the target milestone.
Comment 10 Felix Botner univentionstaff 2018-01-19 16:37:12 CET 
disabled tests/55_adconnector/502_other_attribute_sync.py (4.3)
Comment 11 Arvid Requate univentionstaff 2018-10-06 18:38:18 CEST 
Strangely the skipped tests/55_adconnector/502_other_attribute_sync.py is now reported as "successful" in Jenkins.
Comment 12 Florian Best univentionstaff 2021-01-08 16:11:11 CET 
(In reply to Lukas Oyen from comment #2)
> Created attachment 8385 [details]
> Patch: ad-connector deduplicate UCS LDAP attribute values
> 
> This patch deduplicates the AD-attributes on sync to UCS, while preserving
> ordering. This assumes, that the order of multi-valued attributes in OpenLDAP
> is preserved.
> 
> The S4-Connector takes another approach. It deduplicates AD-attributes
> without
> preserving ordering. On sync from UCS -> AD it corrects for the lost ordering
> by a special case for `con_other` cases. This extra code is ommitted in this
> patch.

This patch is already part of the AD-Connector (git:3b31040eee514440f4358cf395c5615bea0ba586).
It references this bug but was commited in a series of 
https://git.knut.univention.de/univention/ucs/-/compare/d71e03ce01b1bf016b72333220bcb89ff2e6408b...638e895b3509b91945ef94b5d486b4da09269e5f.

Can this bug now be closed as duplicate of Bug #18501?