The --*attr handling code makes assumptions about attribute values that are not always true:
--addattr assumes that the attribute is multi-valued:
$ ipa user-add test --first Test --last Test --addattr sn=Test ipa: ERROR: an internal error has occurred
--delattr assumes that the attribute is unicode:
$ ipa user-mod test --delattr ipasshpubkey=AAAA<snip> ipa: ERROR: ipasshpubkey does not contain 'AAAA<snip>'
Fix --*attr handling so all of the above work correctly.
master: 72cc54b
When trying to rebase the DN work I ran into significant issues with this patch, I don't think it should have been accepted. I spent almost a full day trying to make the patch work but gave up. In the dn branch I've reverted the patch so we can move forward, below are my commit comments from the commit which reverts it. We need to take a deeper look at how setattr, addattr and delattr are handled, they are a very peculiar exception to our public API whereby we expose internal LDAP that we otherwise have tried very hard to hide. Because these are LDAP and NOT command parameters we should be using code that knows how to read and convert LDAP attribute/values (we have code to do this but it probably needs refactoring). At a minimum the code that reads the setattr, addattr, delattr MUST know the syntax of the attribute so it can perform proper type conversion and we have to finally decide how binary data is passed in commands (I think we're still a bit inconsistent). For example do we have a rule which says when binary data is passed to a command parameters it must be base64 encoded? If so then at the command parameter level for setattr, addattr, delattr we need ask the question "is the attribute binary?", if so then base64 decode it. The ldap2 code knows which attributes are binary and has table driven logic to perform type conversion based on the syntax, but currently it never expects the input will be base64,. We need an internal API that knows how to perform attribute/value conversion not in the context of LDAP results, but in the context of command parameters.
There were multiple problems with this patch: * test_attr.py cannot assume the tests/test_xmlrpc/service.crt file exists it needs to test for its existence and skip if it's not there. * test_attr.py is using the wrong type of data for certs - cert LDAP attribute values are DER encoded binary data - the cert data should have been loaded like this: servercert_der = x509.load_certificate_from_file('tests/test_xmlrpc/service.crt').der_data - Commands which accept cert parameters expect base64 encoded data, thus usercertificate=base64.b64encode(servercert_der) - But results are binary, thus the expected value needs to be: usercertificate=servercert_der - setattr, addattr, delattr operate on LDAP values, *NOT* command parameters In the case of certs the expected type is binary because that's the LDAP type for that attribute. It's impossible to pass binary data to setattr, addattr, delattr because we require those parameters to be unicode strings but you can't have a unicode string that looks like: delattr=u'usercertificate=%s' % servercert_der, The %s will attempt encoding, using %r produces a string representaton which isn't correct either. If you try to concatenate like this: u'usercertificate=' + servercert_der you'll get decode errrors because it attempt to decode the binary data to produce unicode, and if you try to use str like this: 'usercertificate=' + servercert_der The param type will reject it as non-Unicode. Hence with our current mechanisms you can't pass binary data to setattr, addattr, delattr. We need a much more comprehensive mechanism for type conversion. In the case of setattr, addattr, delattr since those are LDAP attributes with known syntax we need to interrogate the schema. The ldap code already has mechaism to convert values based on attribute syntax, we should be using those functions. * The _convert_entry() function was introducing base64 encoded values where they didn't belong playing havoc with our data model. * The _convert_entry() function changed scalar values into lists. Thus command results prior to the patch which were a scalar now became a list with one scalar value, once again changing the data model.
Changing 3.2 priority
Note: interactive_prompt_callback (for example in dns-zone-add) should work sanely with --*attr, too.
3.4 development was shifted by one month, moving tickets to reflect reality better.
Adjusting time plan - 3.4 development was postponed as we focused on 3.3.x testing and stabilization.
Moving unfinished November tickets to January.
This refactoring was not finished in 4.0 time frame, moving to later release.
The FreeIPA 4.2 was already shaped (see [[milestone:FreeIPA 4.2]] milestone), this does not fit. Pushing out.
If anyone is willing to help and contribute to this one, please let us know!
Metadata Update from @jcholast: - Issue assigned to jcholast - Issue set to the milestone: FreeIPA 4.5 backlog
Thank you taking time to submit this request for FreeIPA. Unfortunately this bug was not given priority and the team lacks the capacity to work on it at this time.
Given that we are unable to fulfil this request I am closing the issue as wontfix. To request re-consideration of this decision please reopen this issue and provide additional technical details about its importance to you.
Metadata Update from @rcritten: - Issue close_status updated to: wontfix - Issue status updated to: Closed (was: Open)
Login to comment on this ticket.