cannatag / ldap3

a strictly RFC 4510 conforming LDAP V3 pure Python client. The same codebase works with Python 2. Python 3, PyPy and PyPy3
Other
873 stars 267 forks source link

Add support for extensibleObject #455

Open daenney opened 6 years ago

daenney commented 6 years ago

So... this involves extensibleObject which I'm aware of is an awful idea. But I have to deal with some legacy so here we are :|.

I've defined a user as such:

user = ObjectDef(['inetOrgPerson', 'posixAccount', 'shadowAcount', etc etc])
user += 'sshPublicKey'

sshPublicKey normally comes from ldapPublicKey from the openssh-lpk_openldap.schema. However, since this thing had extensibleObject on all user attributes, people helpfully forgot to actually add ldapPublicKey to the objectClasses. However, b/c of extensibleObject any attr defined by any of the schema's the server has loaded is fair game.

I then issued a search and got results back but needed to modify the results for some of the entries. So I loop over .entries, and call entry_writable(). At that point I'm greeted with:

Traceback (most recent call last):
[...]
  File "/home/daenney/Development/mirage/mirage/cli.py", line 101, in sync
    writable_entry = user_entry.entry_writable()
  File "/home/daenney/Applications/virtualenvs/mirage/lib/python3.6/site-packages/ldap3/abstract/entry.py", line 416, in entry_writable
    writable_entry = writable_cursor._create_entry(self._state.response)
  File "/home/daenney/Applications/virtualenvs/mirage/lib/python3.6/site-packages/ldap3/abstract/cursor.py", line 301, in _create_entry
    entry._state.attributes = self._get_attributes(response, self.definition._attributes, entry)
  File "/home/daenney/Applications/virtualenvs/mirage/lib/python3.6/site-packages/ldap3/abstract/cursor.py", line 244, in _get_attributes
    raise LDAPCursorError(error_message)
ldap3.core.exceptions.LDAPCursorError: attribute 'sshPublicKey' not in object class 'apple-user, extensibleObject, inetOrgPerson, ldapProfile, posixAccount, shadowAccount, sambaSamAccount' for entry <redacted>

Now, I thought I could solve this by changing the ObjectDef to include ldapPublicKey too but then my search() fails to return any results b/c the filter is constructed based on all of the classes in the ObjectDef.

Since this thing includes extensibleObject I would expect to be able to call .entry_writable() and not hit this snag because any attribute not defined in any of the other object classes is implicitly a member of extensibleObject. Since the connection is opened with get_info=ALL it should be able to convert and write back this separate attribute properly, just like it managed to read it.

To be honest I'm not sure one would want to fix this, b/c extensibleObject but it took me by surprise. The easiest way I can see to fix this is to allow to override the filter that is created when passing the object definition to Reader, so that I could there exclude ldapPublicKey. Since it'll then still be in the ObjectDef the conversion to a writable entry should then succeed since that one will include the object class that has the rogue attribute defined. In the meam time I'll run a modify by hand to add that object class to everyone first and then carry on.

cannatag commented 6 years ago

Hi, I've not implemented the extensibleObject behaviour in ldap3. I don't see a valid use for it. Anyway if you think it is helpful in some way I will implement it.

daenney commented 6 years ago

So, in general I loathe extensibleObject and I don't think support for it is really worth it. However, I do think my initial bug report still stands. If you can create a query with an additional AttrDef and then call .entry_writable on it you'll be greeted by this error. You can see that behaviour too in my comment on #437 when trying to get operational attributes through the ORM layer. I don't think that should happen. In such cases .entry_writable should probably log a different, or at least more descriptive, error that informs you you cannot transform a response into a writable object unless it's only constructed out of ObjDefs.