Description Florian Best 2023-05-03 17:56:37 CEST

univention.admin.hook.AttributeHook is not idempotent and breaks for extended attributes if the object was open()ed twice.

This is for example the case in the UCS@school importer.
Fetchmail uses the AttributeHook with a extended attribute which is multivalue and has a complex syntax.

Reproducer:
/usr/share/ucs-school-import/scripts/ucs-school-user-import --conffile /var/lib/ucs-school-import/configs/user_import.json --user_role teacher_and_staff --infile foo.csv --dry-run

Code reproducer:
import univention.admin.uldap
lo,po=univention.admin.uldap.getAdminConnection()
univention.admin.modules.update()
U = univention.admin.modules.get('users/user')
univention.admin.modules.init(lo,po,U)
dn = 'uid=…'  # insert me here
u = U.object(None, lo, po, dn)
print(u.oldattr['univentionFetchmailSingle'])
print(u.info['FetchMailSingle'])
u.open()
print(u.oldattr['univentionFetchmailSingle'])
print(u.info['FetchMailSingle'])
u.open()  # will break
print(u.oldattr['univentionFetchmailSingle'])
print(u.info['FetchMailSingle'])
print('------------')

h = univention.admin.hook.FetchMailSingleHook()
print(h.map_attribute_value_to_ldap(u.info['FetchMailSingle']))
print(h.map_attribute_value_to_udm(u.oldattr['univentionFetchmailSingle']))

Causes:
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/ucsschool/importer/frontend/cmdline.py", line 241, in main
    self.do_import()
  File "/usr/lib/python3/dist-packages/ucsschool/importer/frontend/cmdline.py", line 164, in do_import
    six.reraise(etype, exc, etraceback)
  File "/usr/lib/python3/dist-packages/six.py", line 693, in reraise
    raise value
  File "/usr/lib/python3/dist-packages/ucsschool/importer/frontend/cmdline.py", line 150, in do_import
    importer.mass_import()
  File "/usr/lib/python3/dist-packages/ucsschool/importer/mass_import/mass_import.py", line 84, in mass_import
    self.import_users()
  File "/usr/lib/python3/dist-packages/ucsschool/importer/mass_import/mass_import.py", line 143, in import_users
    raise exception
  File "/usr/lib/python3/dist-packages/ucsschool/importer/mass_import/mass_import.py", line 117, in import_users
    user_import.delete_users(users_to_delete)  # 0% - 10%
  File "/usr/lib/python3/dist-packages/ucsschool/importer/mass_import/user_import.py", line 467, in delete_users
    self.connection, source_uid, record_uid, udm_properties=additional_udm_properties
  File "/usr/lib/python3/dist-packages/ucsschool/importer/models/import_user.py", line 440, in get_by_import_id
    import_obj = cls.from_udm_obj(obj, None, connection)
  File "/usr/lib/python3/dist-packages/ucsschool/lib/models/user.py", line 267, in from_udm_obj
    obj = super(User, cls).from_udm_obj(udm_obj, school, lo)
  File "/usr/lib/python3/dist-packages/ucsschool/lib/models/base.py", line 1017, in from_udm_obj
    return klass.from_udm_obj(udm_obj, school, lo)
  File "/usr/lib/python3/dist-packages/ucsschool/lib/models/user.py", line 267, in from_udm_obj
    obj = super(User, cls).from_udm_obj(udm_obj, school, lo)
  File "/usr/lib/python3/dist-packages/ucsschool/lib/models/base.py", line 1018, in from_udm_obj
    udm_obj.open()
  File "/usr/lib/python3/dist-packages/univention/admin/handlers/users/user.py", line 1169, in open
    univention.admin.handlers.simpleLdap.open(self)
  File "/usr/lib/python3/dist-packages/univention/admin/handlers/__init__.py", line 1101, in open
    self.call_udm_property_hook('hook_open', self)
  File "/usr/lib/python3/dist-packages/univention/admin/handlers/__init__.py", line 1077, in call_udm_property_hook
    func(module)
  File "/usr/lib/python3/dist-packages/univention/admin/hook.py", line 207, in hook_open
    new_value = self.map_attribute_value_to_udm(old_value)
  File "<string>", line 52, in map_attribute_value_to_udm
  File "<string>", line 41, in unmap_value
  File "<string>", line 41, in <listcomp>
AttributeError: 'list' object has no attribute 'split'

A correct fix which introduces a API change is:

diff --git mail/univention-fetchmail/share/hooks.d/fetchmail.py mail/univention-fetchmail/share/hooks.d/fetchmail.py
index 3a109d5b1e..5fd1efe152 100644
--- mail/univention-fetchmail/share/hooks.d/fetchmail.py
+++ mail/univention-fetchmail/share/hooks.d/fetchmail.py
@@ -34,11 +34,11 @@ from univention.admin.hook import AttributeHook
 
 
 def map_value(value):
-    return [';'.join('"{}"'.format(w.decode('UTF-8')) for w in v).encode('UTF-8') for v in value]
+    return [';'.join('"{}"'.format(w.replace('"', '\\"')) for w in v).encode('UTF-8') for v in value]
 
 
 def unmap_value(value):
-    return [[w.strip('\"') for w in v.split(';')] for v in value]
+    return [[w.strip('"') for w in v.decode('UTF-8').split(';')] for v in value]
 
 
 class FetchMailSingleHook(AttributeHook):
diff --git management/univention-directory-manager-modules/modules/univention/admin/hook.py management/univention-directory-manager-modules/modules/univention/admin/hook.py
index b844b33c5f..6539711b11 100644
--- management/univention-directory-manager-modules/modules/univention/admin/hook.py
+++ management/univention-directory-manager-modules/modules/univention/admin/hook.py
@@ -202,7 +202,7 @@ class AttributeHook(simpleHook):
         """
         assert isinstance(self.udm_attribute_name, six.string_types), "udm_attribute_name has to be a str"  # noqa: F821
         ud.debug(ud.ADMIN, ud.INFO, 'admin.syntax.hook.AttributeHook: Mapping %s (LDAP) -> %s (UDM)' % (self.ldap_attribute_name, self.udm_attribute_name))
-        old_value = obj[self.udm_attribute_name]
+        old_value = obj.oldattr.get(self.ldap_attribute_name, [])
         new_value = self.map_attribute_value_to_udm(old_value)
         ud.debug(ud.ADMIN, ud.INFO, 'admin.syntax.hook.AttributeHook: Setting UDM value from %r to %r' % (old_value, new_value))
         obj[self.udm_attribute_name] = new_value
@@ -235,16 +235,17 @@ class AttributeHook(simpleHook):
             else:
                 key, old_value, new_value = ml_value
             if key == self.ldap_attribute_name:
-                ud.debug(ud.ADMIN, ud.INFO, 'admin.syntax.hook.AttributeHook: Mapping %s (UDM) -> %s (LDAP)' % (self.udm_attribute_name, self.ldap_attribute_name))
-                old_value = self.map_attribute_value_to_ldap(old_value)
-                new_new_value = self.map_attribute_value_to_ldap(new_value)
-                ud.debug(ud.ADMIN, ud.INFO, 'admin.syntax.hook.AttributeHook: Setting LDAP value from %r to %r' % (new_value, new_new_value))
-                new_value = new_new_value
+                continue
+        if obj.hasChanged(self.udm_attribute_name):
+            old_value = obj.oldattr.get(self.ldap_attribute_name, [])
+            new_value = self.info.get(self.udm_attribute_name)
+            if new_value is not None:
+                new_value = self.map_attribute_value_to_udm(new_value)
             new_ml.append((key, old_value, new_value))
         return new_ml
 
     def map_attribute_value_to_ldap(self, value):
-        # type: (bytes) -> bytes
+        # type: (Any) -> List[bytes]
         """
         Return value as it shall be saved in |LDAP|.
 
@@ -254,7 +255,7 @@ class AttributeHook(simpleHook):
         return value
 
     def map_attribute_value_to_udm(self, value):
-        # type: (str) -> str
+        # type: (List[bytes]) -> Any
         """
         Return value as it shall be used in |UDM| objects.