Bug 41829 - modifying boolean extended attribute writes '0' value to ldap
modifying boolean extended attribute writes '0' value to ldap
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.1-3-errata
Assigned To: Florian Best
Stefan Gohmann
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-07-21 11:48 CEST by Florian Best
Modified: 2019-08-29 12:24 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 4: Minor Usability: Impairs usability in secondary scenarios
Who will be affected by this bug?: 4: Will affect most installed domains
How will those affected feel about the bug?: 5: Blocking further progress on the daily work
User Pain: 0.457
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2016072521000461
Bug group (optional): External feedback, Regression
Max CVSS v3 score:
best: Patch_Available+


Attachments
patch (5.16 KB, patch)
2016-07-21 13:55 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2016-07-21 11:48:25 CEST
A regression since Bug #41207 http://errata.software-univention.de/ucs/4.1/199.html causes that when modifying a extended attribute property with a 'boolean' syntax the value '0' is written to LDAP. prior to this the attribute was removed from the entry.
Comment 1 Florian Best univentionstaff 2016-07-21 11:52:35 CEST
Added a test case which is currently skipped.

ucs-test (6.0.35-7):
r71127 | Bug #41829: add 68_udm-extendedattribute/38_extended_attribute_boolean_syntax
Comment 2 Florian Best univentionstaff 2016-07-21 11:56:50 CEST
Ticket#2016072021000505
Comment 3 Florian Best univentionstaff 2016-07-21 13:55:30 CEST
Created attachment 7820 [details]
patch

The patch moves the ungeneric handling of boolean extended attributes into the mapping functions.
Comment 4 Philipp Hahn univentionstaff 2016-07-28 16:18:38 CEST
This change in behavior broke usersert (Ticket #2016072521000461)
Comment 5 Florian Best univentionstaff 2016-08-15 17:35:24 CEST
univention-directory-manager-modules (11.0.3-29):
r71612 | Bug #41829: don't write 0 value to LDAP for boolean extended attributes

univention-directory-manager-modules.yaml:
r71615 | YAML Bug #41824 Bug #41829 Bug #41899

UCS 4.2:
univention-directory-manager-modules (12.0.1-1):
r71609 | Bug #41829: don't write 0 value to LDAP for boolean extended attributes
Comment 6 Stefan Gohmann univentionstaff 2016-08-17 07:51:54 CEST
Code review: OK

YAML: OK

ucs-test: OK

Tests: OK, I was able to reproduce it with the old version.

(In reply to Florian Best from comment #1)
> Added a test case which is currently skipped.
> 
> ucs-test (6.0.35-7):
> r71127 | Bug #41829: add
> 68_udm-extendedattribute/38_extended_attribute_boolean_syntax

I've re-enabled the test case: r71667 + r71668
Comment 7 Stefan Gohmann univentionstaff 2016-08-18 10:22:25 CEST
As discussed, the test case 62_udm-groups.22_group_posix_only.test fails now:

[2016-08-17 16:17:12.219469] Creating groups/group object with /usr/sbin/udm-test groups/group create --position cn=groups,dc=AutoTest221,dc=local --option posix --set name=qb36eqpivb
[2016-08-17 16:17:14.158189] . Cleanup after exception: <class 'univention.testing.utils.LDAPObjectUnexpectedValue'> DN: cn=qb36eqpivb,cn=groups,dc=AutoTest221,dc=local
[2016-08-17 16:17:14.158253] objectClass: ['univentionVirtualMachineGroupOC', 'top', 'univentionGroup', 'posixGroup', 'univentionObject'], unexpected: 'univentionVirtualMachineGroupOC'
[2016-08-17 16:17:14.158320] Performing UCSTestUDM cleanup...
[2016-08-17 16:17:14.363754] UCSTestUDM cleanup done
(2016-08-17 16:17:14.363791) Traceback (most recent call last):
(2016-08-17 16:17:14.363805)   File "22_group_posix_only", line 17, in <module>
(2016-08-17 16:17:14.363819)     utils.verify_ldap_object(group, {'objectClass': ['top', 'posixGroup', 'univentionGroup', 'univentionObject']})
(2016-08-17 16:17:14.363833)   File "/usr/lib/pymodules/python2.7/univention/testing/utils.py", line 160, in verify_ldap_object
(2016-08-17 16:17:14.363847)     raise LDAPObjectUnexpectedValue('DN: %s\n%s: %r, unexpected: \'%s\'' % (baseDn, attribute, list(found_values), '\', '.join(difference)))
(2016-08-17 16:17:14.363860) univention.testing.utils.LDAPObjectUnexpectedValue: DN: cn=qb36eqpivb,cn=groups,dc=AutoTest221,dc=local
(2016-08-17 16:17:14.363872) objectClass: ['univentionVirtualMachineGroupOC', 'top', 'univentionGroup', 'posixGroup', 'univentionObject'], unexpected: 'univentionVirtualMachineGroupOC'
Comment 8 Florian Best univentionstaff 2016-08-18 12:44:33 CEST
univention-directory-manager-modules (11.0.3-30):
r71706 | Bug #41829: don't add object class of '0' values for extended attributes with boolean syntax

univention-directory-manager-modules (12.0.1-2):
r71707 | Bug #41829: ignore '0' values for extended attributes with boolean syntax

In self.info there is still the value '0' for these properties. The check was done against the property values instead if the ldap attribute values. Therefore I reimplemented part of the old workaround. A better patch would be the following (but currently I don't dare this change because there might be side effects on unmapped attributes, I will have a look some other day):

diff --git a/management/univention-directory-manager-modules/modules/univention/admin/handlers/__init__.py b/management/univention-directory-manager-modules/modules/univention/admin/handlers/__init__.py
index 042ad0a..c9ca240 100644
--- a/management/univention-directory-manager-modules/modules/univention/admin/handlers/__init__.py
+++ b/management/univention-directory-manager-modules/modules/univention/admin/handlers/__init__.py
@@ -689 +689 @@ def _create(self):
-                       if self.has_key(prop.name) and self.info.get(prop.name):
+                       if self.has_key(prop.name) and _MergedAttributes(self, al).get_attribute(self.mapping.mapName(prop.name)):
@@ -800 +800 @@ def _ldap_object_classes(self, ml):
-                       if self.has_key(prop.name) and self.info.get(prop.name) and (True if prop.syntax != 'boolean' else self.info.get(prop.name) != '0'):
+                       if self.has_key(prop.name) and _MergedAttributes(self, ml).get_attribute(self.mapping.mapName(prop.name)):
Comment 9 Stefan Gohmann univentionstaff 2016-08-18 14:38:07 CEST
OK, it looks good. The test case works now.
Comment 10 Janek Walkenhorst univentionstaff 2016-08-18 15:15:36 CEST
<http://errata.software-univention.de/ucs/4.1/235.html>