Bug 36480 - Handle additional (possibly duplicate) phone numbers
Handle additional (possibly duplicate) phone numbers
Status: REOPENED
Product: UCS
Classification: Unclassified
Component: AD Connector
UCS 5.0
Other Linux
: P5 normal (vote)
: ---
Assigned To: Samba maintainers
Felix Botner
:
Depends on: 18501 45220 45249
Blocks: 42524
  Show dependency treegraph
 
Reported: 2014-11-06 12:46 CET by Michael Grandjean
Modified: 2021-01-08 16:11 CET (History)
4 users (show)

See Also:
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:
oyen: Patch_Available+


Attachments
Screenshot of how to set additional phone number in AD (23.59 KB, image/png)
2014-11-06 12:46 CET, Michael Grandjean
Details
Excerpt from connector.log with traceback (1.41 KB, text/x-log)
2014-11-06 12:49 CET, Michael Grandjean
Details
Patch: ad-connector deduplicate UCS LDAP attribute values (1.52 KB, patch)
2017-01-30 18:52 CET, Lukas Oyen
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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?