Bug 30368 - univention.admin.objects.get opens any DN with any module
univention.admin.objects.get opens any DN with any module
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.2
Other Linux
: P3 minor (vote)
: UCS 4.2-1-errata
Assigned To: Florian Best
Johannes Keiser
: 10709 (view as bug list)
Depends on: 40839 44976 45011
Blocks: 45096
  Show dependency treegraph
Reported: 2013-02-10 13:36 CET by Jan Christoph Ebersbach
Modified: 2017-07-28 14:20 CEST (History)
6 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?: 2: Will only affect a 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.091
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Cleanup, Troubleshooting, Usability
Max CVSS v3 score:
best: Patch_Available+

patch (959 bytes, patch)
2016-10-20 19:28 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Christoph Ebersbach univentionstaff 2013-02-10 13:36:37 CET
I tried to use univention.admin.objects.get to retrieve UDM computer objects. The problem was that there might be computer objects of a different kind. Hoping that the get method would handle this by either raising an exception or returning nothing, I called it for a Windows computer object with the UCC computer module without any errors. I was also able to call the open method on this object.

I took it a bit further and discovered that it doesn't seem to matter what kind of DN is presented to method:

univention.admin.objects.get(computer_module, co, lo, position=position, dn='uid=Administrator,cn=users,' + baseDN)

This returned a valid UCC object although it's user's object.

It would be very helpful to have some error handling for this method. At least the lookup method for finding objects makes sure I only get objects of the kind I was asking for. As far as I know a DN can't be fed to lookup but maybe there is a better way of how to open an object for a known DN that I'm not aware of.
Comment 1 Florian Best univentionstaff 2016-06-28 10:40:57 CEST
(In reply to Jan Christoph Ebersbach from comment #0)
> As far as I know a DN can't be fed to lookup but
> maybe there is a better way of how to open an object for a known DN that I'm
> not aware of.

You can use lookup() with a single DN!

>>> import univention.admin.uldap
>>> lo,po=univention.admin.uldap.getMachineConnection()
>>> import univention.admin.modules
>>> univention.admin.modules.update()
>>> users = univention.admin.modules.get('users/user')
>>> windows = univention.admin.modules.get('computers/windows')
>>> admin = users.lookup(None, lo, '', base='uid=Administrator,cn=users,dc=school,dc=local', scope='base', unique=True)
>>> admin
[<univention.admin.handlers.users.user.object object at 0x3346c50>]
>>> windows.lookup(None, lo, '', base='uid=Administrator,cn=users,dc=school,dc=local', scope='base', unique=True)
Comment 2 Florian Best univentionstaff 2016-06-28 10:50:17 CEST
If you want to find out which object type it actually is use the following:
>>> univention.admin.modules.identify('uid=Administrator,cn=users,dc=school,dc=local', lo.get('uid=Administrator,cn=users,dc=school,dc=local'))
[<module 'univention.admin.handlers.users.user' from '/usr/lib/pymodules/python2.7/univention/admin/handlers/users/user.pyc'>]
Comment 3 Florian Best univentionstaff 2016-10-20 19:28:02 CEST
Created attachment 8143 [details]

patch: return None if object type is wrong.
Comment 4 Florian Best univentionstaff 2016-10-21 15:22:47 CEST
UDM-CLI uses the function and doesn't handle None. We should simply introduce a new exception "wrongObjectType". This causes Bug #10709.
Comment 5 Florian Best univentionstaff 2016-10-21 15:23:12 CEST
*** Bug 10709 has been marked as a duplicate of this bug. ***
Comment 6 Florian Best univentionstaff 2017-07-06 14:18:18 CEST
This also affects the CLI:

For example when a group is treated as share:

udm shares/share modify --dn "cn=Schule1-testklasse,cn=klassen,cn=schueler,cn=groups,ou=Schule1,dc=base" --set path="/home/groups/klassen/Schule1-1A" --set host="foo.bar.example.com"
Invalid combination of options: Need Export for Samba clients or Export for NFS clients (NFSv3 and NFSv4) in options to create a share.
Comment 7 Florian Best univentionstaff 2017-07-06 15:07:57 CEST
The problem has been fixed directly in simpleLDAP instead which is the root of the problem.

univention-directory-manager-modules (12.0.17-66):
r80919 | Bug #30368: check object type before opening object

r80920 | YAML Bug #30368
Comment 8 Florian Best univentionstaff 2017-07-07 14:25:52 CEST
Bug #40839 blocked this because the detection was wrong there. I fixed this.
Nevertheless out in the wild might be further broken identify() functions so that I changed the detection from simpleLDAP.__init__ back to univention.admin.objects.get() and do a lookup() instead of a identify(). The identify() in simpleLDAP.__init__ keeps being there and logs a error message in case the detection went wrong, to be able to find further broken objects.

univention-directory-manager-modules (12.0.17-70):
r80967 | Bug #30368: don't force identify() function to match results of the lookup, yet

ucs-test (7.0.22-22):
r80956 | Bug #30368: fix wrong object type given in test scripts
Comment 9 Florian Best univentionstaff 2017-07-11 22:50:26 CEST
Some more commits were necessary:
The subtree move failed for objects containing special chars such as (!"§$%&/()=?+#-}{[]\_-*) with the changes as the DN escaping there wasn't fully correct. This has been fixed.

Also the S4-Connector has problems with the change currently: Bug #44976 / Bug #35559.
Therefor the old behavior has been restored! Only the necessary fixes are kept. The work on this issue will be continued in Bug #44976.

r81066 | YAML Bug #30368

ucs-test (7.0.22-25):
r81063 | Bug #30368: fix detection of modified DN (modrdn detected)

univention-directory-manager-modules (12.0.17-80):
r81065 | Bug #30368: temporarily restore previous behavior
r81064 | Bug #30368: fix DN comparision
r81019 | Bug #30368: fix typo
r81014 | Bug #30368: fix typo, improve error message
r81012 | Bug #30368: fix subtree move for DNs containing special characters
Comment 10 Johannes Keiser univentionstaff 2017-07-12 14:58:43 CEST
OK Old behavior restored
OK Subtree move working

-> verified
Comment 11 Florian Best univentionstaff 2017-07-13 12:01:06 CEST
As we didn't release the errata yesterday we can continue in this bug with the UDM changes. Bug #44976 is for the S4-Connector changes.
Comment 12 Florian Best univentionstaff 2017-07-18 18:41:45 CEST
The S4-Connector changes look good now. The revert has been reverted.
There was still an error in the subtree move for containers containing \ characters, which messed up the inernal DN comparisions. This has been fixed as well.

univention-directory-manager-modules (12.0.17-93):
r81229 | Bug #30368: fix DN handling for subtree move. Prevents DECODING_ERROR if the new DN contains "\".
r81141 | Revert "Bug #30368: temporarily restore previous behavior"

r81230 | Revert "YAML Bug #30368"
Comment 13 Johannes Keiser univentionstaff 2017-07-19 11:08:36 CEST
OK Subtree move working (!"§$%&/()=?'`¸{[]}\ß^*+~#'-_.:,;<>|äüö@)
OK univention.admin.objects.get raises error if the dn is not of the specified
OK udm modify does not work with a wrong object type anmore

-> verified
Comment 14 Florian Best univentionstaff 2017-07-25 15:47:01 CEST
I reverted the changes again so that the previous behavior is restored and some error messages are logged. Stefan wants that the change will be done in a minor/patchlevel release instead of an errata update so that the product tests are performed before this API change.

univention-directory-manager-modules (12.0.18-5):
r81363 | Bug #30368: revert svn r81141
r81364 | YAML Bug #30368
Comment 15 Johannes Keiser univentionstaff 2017-07-25 17:31:01 CEST
OK behavior from Comment #0 is restored

-> verified
Comment 16 Erik Damrose univentionstaff 2017-07-26 14:39:08 CEST