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 --- 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 ++--- 5 files changed, 80 insertions(+), 31 deletions(-) (limited to 'ipaserver/plugins') 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