From 750a392fe22aa8ddcb21077e8c24b96d36ecf20c Mon Sep 17 00:00:00 2001 From: Martin Babinsky Date: Thu, 23 Jun 2016 19:14:53 +0200 Subject: Allow for commands that use positional parameters to add/remove attributes Commands that modify a single multivalued attribute of an entry should use positional parameters to specify both the primary key and the values to add/remove. Named options are redundant in this case. The `--certificate option` of `*-add/remove-cert` commands was turned mandatory to avoid EmptyModlist when it is omitted. https://fedorahosted.org/freeipa/ticket/3961 https://fedorahosted.org/freeipa/ticket/5413 Reviewed-By: David Kupka Reviewed-By: Jan Cholasta --- API.txt | 16 +++++----- VERSION | 4 +-- ipaserver/plugins/baseldap.py | 74 +++++++++++++++++++++++++++++++++---------- ipaserver/plugins/host.py | 8 +++-- ipaserver/plugins/idviews.py | 13 +++++--- ipaserver/plugins/service.py | 8 +++-- ipaserver/plugins/user.py | 8 ++--- 7 files changed, 90 insertions(+), 41 deletions(-) diff --git a/API.txt b/API.txt index 704dcde3c..0c307fbbd 100644 --- a/API.txt +++ b/API.txt @@ -2294,7 +2294,7 @@ arg: Str('fqdn', cli_name='hostname') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('no_members', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -2461,7 +2461,7 @@ arg: Str('fqdn', cli_name='hostname') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('no_members', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -2698,7 +2698,7 @@ arg: Str('ipaanchoruuid', cli_name='anchor') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('fallback_to_ldap?', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -2770,7 +2770,7 @@ arg: Str('ipaanchoruuid', cli_name='anchor') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('fallback_to_ldap?', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -4293,7 +4293,7 @@ arg: Principal('krbprincipalname', cli_name='principal') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('no_members', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -4424,7 +4424,7 @@ arg: Principal('krbprincipalname', cli_name='principal') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('no_members', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -5668,7 +5668,7 @@ arg: Str('uid', cli_name='login') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('no_members', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) @@ -5828,7 +5828,7 @@ arg: Str('uid', cli_name='login') option: Flag('all', autofill=True, cli_name='all', default=False) option: Flag('no_members', autofill=True, default=False) option: Flag('raw', autofill=True, cli_name='raw', default=False) -option: Bytes('usercertificate*', alwaysask=True, cli_name='certificate') +option: Bytes('usercertificate+', alwaysask=True, cli_name='certificate') option: Str('version?') output: Entry('result') output: Output('summary', type=[, ]) diff --git a/VERSION b/VERSION index 23ceecc98..a35aec56c 100644 --- a/VERSION +++ b/VERSION @@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=205 -# Last change: Add --ca option to cert-revoke and cert-remove-hold +IPA_API_VERSION_MINOR=206 +# Last change: mbabinsk: commands that use positional parameters to manage attributes diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py index c35f660c7..6107e43a6 100644 --- a/ipaserver/plugins/baseldap.py +++ b/ipaserver/plugins/baseldap.py @@ -2288,28 +2288,34 @@ class LDAPRemoveReverseMember(LDAPModReverseMember): raise exc -class LDAPModAttribute(LDAPQuery): +class BaseLDAPModAttribute(LDAPQuery): attribute = None has_output = output.standard_entry - def get_options(self): - for option in super(LDAPModAttribute, self).get_options(): - yield option - - option = self.obj.params[self.attribute] - attribute = 'virtual_attribute' not in option.flags - yield option.clone(attribute=attribute, alwaysask=True) + def _get_attribute_param(self): + arg = self.obj.params[self.attribute] + attribute = 'virtual_attribute' not in arg.flags + return arg.clone(required=True, attribute=attribute, alwaysask=True) def _update_attrs(self, update, entry_attrs): raise NotImplementedError("%s.update_attrs()", self.__class__.__name__) def execute(self, *keys, **options): ldap = self.obj.backend + try: + index = tuple(self.args).index(self.attribute) + except ValueError: + obj_keys = keys + else: + obj_keys = keys[:index] - dn = self.obj.get_dn(*keys, **options) - entry_attrs = ldap.make_entry(dn, self.args_options_2_entry(**options)) + dn = self.obj.get_dn(*obj_keys, **options) + entry_attrs = ldap.make_entry(dn, self.args_options_2_entry( + *keys, **options)) + + entry_attrs.pop(self.obj.primary_key.name, None) if options.get('all', False): attrs_list = ['*', self.obj.primary_key.name] @@ -2326,6 +2332,7 @@ class LDAPModAttribute(LDAPQuery): try: update = self._exc_wrapper(keys, options, ldap.get_entry)( entry_attrs.dn, list(entry_attrs)) + self._update_attrs(update, entry_attrs) self._exc_wrapper(keys, options, ldap.update_entry)(update) @@ -2347,7 +2354,7 @@ class LDAPModAttribute(LDAPQuery): entry_attrs = entry_to_dict(entry_attrs, **options) if self.obj.primary_key: - pkey = keys[-1] + pkey = obj_keys[-1] else: pkey = None @@ -2367,7 +2374,7 @@ class LDAPModAttribute(LDAPQuery): raise exc -class LDAPAddAttribute(LDAPModAttribute): +class BaseLDAPAddAttribute(BaseLDAPModAttribute): msg_summary = _('added attribute value to entry %(value)') def _update_attrs(self, update, entry_attrs): @@ -2377,14 +2384,13 @@ class LDAPAddAttribute(LDAPModAttribute): if not old_value.isdisjoint(value_to_add): raise errors.ExecutionError( - message=_('\'%s\' already contains one or more values' - % name) - ) + message=_('\'%(attr)s\' already contains one or more ' + 'values') % dict(attr=name)) update[name] = list(old_value | value_to_add) -class LDAPRemoveAttribute(LDAPModAttribute): +class BaseLDAPRemoveAttribute(BaseLDAPModAttribute): msg_summary = _('removed attribute values from entry %(value)') def _update_attrs(self, update, entry_attrs): @@ -2397,3 +2403,39 @@ class LDAPRemoveAttribute(LDAPModAttribute): attr=name, value=_("one or more values to remove")) update[name] = list(old_value - value_to_remove) + + +class LDAPModAttribute(BaseLDAPModAttribute): + + def get_args(self): + for arg in super(LDAPModAttribute, self).get_args(): + yield arg + + yield self._get_attribute_param() + + +class LDAPAddAttribute(LDAPModAttribute, BaseLDAPAddAttribute): + pass + + +class LDAPRemoveAttribute(LDAPModAttribute, BaseLDAPRemoveAttribute): + pass + + +class LDAPModAttributeViaOption(BaseLDAPModAttribute): + + def get_options(self): + for option in super(LDAPModAttributeViaOption, self).get_options(): + yield option + + yield self._get_attribute_param() + + +class LDAPAddAttributeViaOption(LDAPModAttributeViaOption, + BaseLDAPAddAttribute): + pass + + +class LDAPRemoveAttributeViaOption(LDAPModAttributeViaOption, + BaseLDAPRemoveAttribute): + pass diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py index 6210e8c16..11bddb505 100644 --- a/ipaserver/plugins/host.py +++ b/ipaserver/plugins/host.py @@ -32,7 +32,9 @@ from .baseldap import (LDAPQuery, LDAPObject, LDAPCreate, LDAPRetrieve, LDAPAddMember, LDAPRemoveMember, host_is_master, pkey_to_value, add_missing_object_class, - LDAPAddAttribute, LDAPRemoveAttribute) + LDAPAddAttribute, LDAPRemoveAttribute, + LDAPAddAttributeViaOption, + LDAPRemoveAttributeViaOption) from ipaserver.plugins.service import ( validate_realm, normalize_principal, validate_certificate, set_certificate_attrs, ticket_flags_params, update_krbticketflags, @@ -1311,14 +1313,14 @@ class host_disallow_create_keytab(LDAPRemoveMember): @register() -class host_add_cert(LDAPAddAttribute): +class host_add_cert(LDAPAddAttributeViaOption): __doc__ = _('Add certificates to host entry') msg_summary = _('Added certificates to host "%(value)s"') attribute = 'usercertificate' @register() -class host_remove_cert(LDAPRemoveAttribute): +class host_remove_cert(LDAPRemoveAttributeViaOption): __doc__ = _('Remove certificates from host entry') msg_summary = _('Removed certificates from host "%(value)s"') attribute = 'usercertificate' diff --git a/ipaserver/plugins/idviews.py b/ipaserver/plugins/idviews.py index 755b07c78..92d47f553 100644 --- a/ipaserver/plugins/idviews.py +++ b/ipaserver/plugins/idviews.py @@ -23,7 +23,8 @@ import six from .baseldap import (LDAPQuery, LDAPObject, LDAPCreate, LDAPDelete, LDAPUpdate, LDAPSearch, - LDAPAddAttribute, LDAPRemoveAttribute, + LDAPAddAttributeViaOption, + LDAPRemoveAttributeViaOption, LDAPRetrieve, global_output_params) from .hostgroup import get_complete_hostgroup_member_list from .service import validate_certificate @@ -961,12 +962,13 @@ class idoverridegroup(baseidoverride): override_object = 'group' @register() -class idoverrideuser_add_cert(LDAPAddAttribute): +class idoverrideuser_add_cert(LDAPAddAttributeViaOption): __doc__ = _('Add one or more certificates to the idoverrideuser entry') msg_summary = _('Added certificates to idoverrideuser "%(value)s"') attribute = 'usercertificate' - takes_options = LDAPAddAttribute.takes_options + (fallback_to_ldap_option,) + takes_options = LDAPAddAttributeViaOption.takes_options + ( + fallback_to_ldap_option,) def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): @@ -983,12 +985,13 @@ class idoverrideuser_add_cert(LDAPAddAttribute): @register() -class idoverrideuser_remove_cert(LDAPRemoveAttribute): +class idoverrideuser_remove_cert(LDAPRemoveAttributeViaOption): __doc__ = _('Remove one or more certificates to the idoverrideuser entry') msg_summary = _('Removed certificates from idoverrideuser "%(value)s"') attribute = 'usercertificate' - takes_options = LDAPRemoveAttribute.takes_options + (fallback_to_ldap_option,) + takes_options = LDAPRemoveAttributeViaOption.takes_options + ( + fallback_to_ldap_option,) def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py index 70bf31fd4..ec0bc5fbe 100644 --- a/ipaserver/plugins/service.py +++ b/ipaserver/plugins/service.py @@ -39,7 +39,9 @@ from .baseldap import ( LDAPRemoveMember, LDAPQuery, LDAPAddAttribute, - LDAPRemoveAttribute) + LDAPRemoveAttribute, + LDAPAddAttributeViaOption, + LDAPRemoveAttributeViaOption) from ipalib import x509 from ipalib import _, ngettext from ipalib import util @@ -881,14 +883,14 @@ class service_disable(LDAPQuery): @register() -class service_add_cert(LDAPAddAttribute): +class service_add_cert(LDAPAddAttributeViaOption): __doc__ = _('Add new certificates to a service') msg_summary = _('Added certificates to service principal "%(value)s"') attribute = 'usercertificate' @register() -class service_remove_cert(LDAPRemoveAttribute): +class service_remove_cert(LDAPRemoveAttributeViaOption): __doc__ = _('Remove certificates from a service') msg_summary = _('Removed certificates from service principal "%(value)s"') attribute = 'usercertificate' diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py index 7c5221c85..c231847d5 100644 --- a/ipaserver/plugins/user.py +++ b/ipaserver/plugins/user.py @@ -53,8 +53,8 @@ from .baseldap import ( LDAPSearch, LDAPQuery, LDAPMultiQuery, - LDAPAddAttribute, - LDAPRemoveAttribute) + LDAPAddAttributeViaOption, + LDAPRemoveAttributeViaOption) from . import baseldap from ipalib.request import context from ipalib import _, ngettext @@ -1136,7 +1136,7 @@ class user_status(LDAPQuery): @register() -class user_add_cert(LDAPAddAttribute): +class user_add_cert(LDAPAddAttributeViaOption): __doc__ = _('Add one or more certificates to the user entry') msg_summary = _('Added certificates to user "%(value)s"') attribute = 'usercertificate' @@ -1158,7 +1158,7 @@ class user_add_cert(LDAPAddAttribute): @register() -class user_remove_cert(LDAPRemoveAttribute): +class user_remove_cert(LDAPRemoveAttributeViaOption): __doc__ = _('Remove one or more certificates to the user entry') msg_summary = _('Removed certificates from user "%(value)s"') attribute = 'usercertificate' -- cgit