Bug 41368 - prevent wrong uldap class
prevent wrong uldap class
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.2-1-errata
Assigned To: Johannes Keiser
Florian Best
Depends on:
  Show dependency treegraph
Reported: 2016-05-27 11:13 CEST by Florian Best
Modified: 2017-07-26 14:39 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2017021521000568
Bug group (optional): Error handling, External feedback, Troubleshooting
Max CVSS v3 score:
best: Patch_Available+

patch (744 bytes, patch)
2016-05-27 11:13 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-05-27 11:13:57 CEST
Created attachment 7695 [details]

We should prevent to use univention.uldap.access in UDM because it has serious side effects. Patch attached.

+++ This bug was initially created as a clone of Bug #20381 +++

Es gibt zwei uldap-Klassen:
1. univention.uldap.search() liefert im Fehlerfall None
2. univention.admin.uldap.search() wirft eine Exception

In univention/admin/handlers/__init__.py:1169#__modify_dhcp_object() wird zwingend Variante 1 benötig, ansonsten passieren schlimme Dinge:

Traceback (most recent call last):
  File "/usr/sbin/univention-dvs-create-desktop", line 278, in ?
    dn = computer.create()
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 307, in create
    return self._create()
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 652, in _create
    raise e

Die eigentliche Ursache liegt aber tiefer, was dort erst ein zusätzliches "print traceback.format_exc()" zeigt:
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 645, in _create
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/computers/windows.py", line 438, in _ldap_post_create
    univention.admin.handlers.simpleComputer._ldap_post_create( self )
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 1880, in _ldap_post_create
    self.__modify_dhcp_object( dn, self[ 'name' ], ip, mac )
  File "/usr/lib/python2.4/site-packages/univention/admin/handlers/__init__.py", line 1169, in __modify_dhcp_object
    results = self.lo.search( base = position, scope = 'domain', attr = [ 'univentionDhcpFixedAddress' ], filter = 'dhcpHWAddress=%s' % ethernet, unique = 0 )
  File "/usr/lib/python2.4/site-packages/univention/admin/uldap.py", line 303, in search
    raise univention.admin.uexceptions.noObject, msg[0]['desc']
noObject: No such object

Solche fiesen Fallen sollten deutlich dokumentiert sein und noch besser rechtzeitig abgefangen werden!
Comment 1 Alexander Kläser univentionstaff 2016-09-05 11:47:18 CEST
What about logging an error/warning instead? This would not change the behavior of existing code, however, it would make it possible to identify code that uses univention.uldap.access directly.
Comment 2 Florian Best univentionstaff 2016-09-05 11:51:05 CEST
(In reply to Alexander Kläser from comment #1)
> What about logging an error/warning instead? This would not change the
> behavior of existing code, however, it would make it possible to identify
> code that uses univention.uldap.access directly.
There is no existing and must not be code which uses the wrong class.
Comment 3 Daniel Tröder univentionstaff 2017-01-12 15:21:47 CET
Stumbled upon this - again! Please at least make univention.uldap.get*Connection() log *deprecated* to some log, so developers don't loose time debugging strange behavior.
Comment 4 Florian Best univentionstaff 2017-02-15 16:35:46 CET
Version: 4.1-4 errata396 (Vahr)

Remark: install kopano webapp

Execution of command 'appcenter/invoke_dry_run' has failed:

Traceback (most recent call last):
  File "%PY2.7%/univention/management/console/base.py", line 281, in execute
    function(self, request)
  File "%PY2.7%/univention/management/console/modules/appcenter/__init__.py", line 341, in invoke_dry_run
  File "%PY2.7%/univention/management/console/modules/appcenter/__init__.py", line 129, in _deferred
    return func(self, *args, **kwargs)
  File "%PY2.7%/univention/management/console/modules/decorators.py", line 656, in _decorated
    return function(self, request, *args, **kwargs)
  File "%PY2.7%/univention/management/console/modules/decorators.py", line 190, in _response
    return function(self, request)
  File "%PY2.7%/univention/management/console/modules/appcenter/__init__.py", line 515, in invoke
    forbidden, warnings = application.check_invokation(function, self.package_manager)
  File "%PY2.7%/univention/management/console/modules/appcenter/app_center.py", line 1244, in check_invokation
    return _check(True), _check(False)
  File "%PY2.7%/univention/management/console/modules/appcenter/app_center.py", line 1240, in _check
    reason = method(**kwargs)
  File "%PY2.7%/univention/management/console/modules/appcenter/app_center.py", line 1169, in must_have_no_unmet_dependencies
    app = app.to_dict(package_manager, domainwide_managed=True)
  File "%PY2.7%/univention/management/console/modules/decorators.py", line 648, in wrapper
    return func(*args, **kwargs)
  File "%PY2.7%/univention/management/console/modules/appcenter/app_center.py", line 1009, in to_dict
    res['installations'] = self.get_installations(hosts)
  File "%PY2.7%/univention/management/console/ldap.py", line 141, in _decorated
    result = func(*args, **kwargs)
  File "%PY2.7%/univention/management/console/modules/appcenter/app_center.py", line 961, in get_installations
    app_objs = appcenter_udm_module.lookup(None, lo, None, base=self.ldap_container)
  File "%PY2.7%/univention/admin/handlers/appcenter/app.py", line 391, in lookup
    res.append(object(co, lo, None, dn, attributes=attrs))
  File "%PY2.7%/univention/admin/handlers/appcenter/app.py", line 362, in __init__
    univention.admin.handlers.simpleLdap.__init__(self, co, lo, position, dn, superordinate, attributes=attributes)
  File "%PY2.7%/univention/admin/handlers/__init__.py", line 592, in __init__
  File "%PY2.7%/univention/admin/handlers/__init__.py", line 622, in _validate_superordinate
    if self.dn and not self._ensure_dn_in_subtree(self.superordinate.dn, self.lo.parentDn(self.dn)):
  File "%PY2.7%/univention/admin/handlers/__init__.py", line 627, in _ensure_dn_in_subtree
    if self.lo.lo.compare_dn(dn, parent):
  File "/usr/lib/python2.7/dist-packages/ldap/ldapobject.py", line 136, in __getattr__
AttributeError: ReconnectLDAPObject has no attribute 'compare_dn'
Comment 5 Johannes Keiser univentionstaff 2017-07-06 19:30:10 CEST
(In reply to Florian Best from comment #0)
> Created attachment 7695 [details]
> patch

Applied patch:

r 80941
univention-directory-manager-modules (12.0.17-68) 
* Bug #41368: Applied patch from Florian Best - Prevent the direct use
of univention.uldap.access in UDM

YAML: r 80943
Comment 6 Florian Best univentionstaff 2017-07-07 10:49:21 CEST
It seems mail/univention-mail-dovecot/modules/univention/mail/dovecot_shared_folder.py and in some ucs-test scripts this is used wrong.
I will fix the test cases.

As an alternative I would suggest to log an ERROR and execute lo=univention.admin.uldap.access(lo=lo) if isinstance(lo, univention.uldap.access).
Comment 7 Johannes Keiser univentionstaff 2017-07-07 11:03:42 CEST
Applied patch:

r 80952
univention-mail-dovecot (3.0.1-2) 
* Bug #41368: Applied patch from Florian Best - Use univention.admin.uldap
instead of univention.uldap

YAML: r 80953
Comment 8 Johannes Keiser univentionstaff 2017-07-07 11:56:52 CEST
r 80957
univention-directory-manager-modules (12.0.17-69) 
* Bug #41368: Use univention.admin.uldap.access if univention.uldap.access
is used and log deprecated warning
Comment 9 Florian Best univentionstaff 2017-07-10 19:31:18 CEST
OK: backwards compatibility
OK: wrong use is now gracefully treated
OK: mail listener
OK: YAML (adjusted in r81017)