On Thu, Jun 30, 2016 at 07:49:04PM +1000, Fraser Tweedale wrote: > On Thu, Jun 30, 2016 at 11:38:35AM +0200, Florence Blanc-Renaud wrote: > > On 06/30/2016 06:29 AM, Fraser Tweedale wrote: > > > On Wed, Jun 29, 2016 at 11:30:14AM +0200, Florence Blanc-Renaud wrote: > > > > On 06/29/2016 07:25 AM, Fraser Tweedale wrote: > > > > > The attached patch fixes > > > > > https://fedorahosted.org/freeipa/ticket/5991. > > > > > > > > > > Thanks, > > > > > Fraser > > > > > > > > > > > > > > > > > > > Hi Fraser, > > > > > > > > A few cosmetic comments: > > > > > > > > PEP8 issues: > > > > ./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1 > > > > ./ipaserver/plugins/cert.py:394:80: E501 line too long (98 > 79 > > > > characters) > > > > ./ipaserver/plugins/cert.py:496:80: E501 line too long (81 > 79 > > > > characters) > > > > > > > > and there is a typo in ipaserver/plugins/cert.py > > > > + doc=_("automatically add the principal if it doesn't exist > > > > (service princpals only)"), > > > > > > > > should be "princ*i*pals only" > > > > > > > > Otherwise LGTM, > > > > Flo > > > > > > > Thanks for review, Flo. Updated patch attached. > > > > > > Cheers, > > > Fraser > > > > > Hi Fraser, > > > > thanks for updated patch. There is still a pep8 error: > > ./ipalib/errors.py:1399:1: E302 expected 2 blank lines, found 1 > > > Whups! > > > I am also wondering if the message is clear enough. When running the CLI > > it's ok because the user typed --add, but the GUI displays "'add' is not > > supported for user principals" > > and I feel > > "'add' option is not supported for user principals" > > would be more user-friendly. What do you think? > > > Yes, I concur that mentioning "option" is an improvement. Will cut > a new patch shortly > > Thanks! > Fraser > Updated patch attached. Third time's the charm? ;)
Cheers, Fraser
From 99c1d5c32b19a070f5e995fce4de32dee39433c1 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale <ftwee...@redhat.com> Date: Wed, 29 Jun 2016 15:02:51 +1000 Subject: [PATCH] cert-request: better error msg when 'add' not supported cert-request supports adding service principals that don't exist. If add is requested for other principal types, the error message just says "the principal doesn't exist". Add a new error type with better error message to explain that 'add' is not supported for host or user principals. Fixes: https://fedorahosted.org/freeipa/ticket/5991 --- ipalib/errors.py | 10 ++++++++++ ipaserver/plugins/cert.py | 21 ++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ipalib/errors.py b/ipalib/errors.py index 10491a94211648df8bda60f3dbc9e52d19e83d10..7b4f15dd60ee80719195ba1b9b85d075b10bdf4f 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1397,6 +1397,16 @@ class ServerRemovalError(ExecutionError): format = _('Server removal aborted: %(reason)s.') +class OperationNotSupportedForPrincipalType(ExecutionError): + """ + **4034** Raised when an operation is not supported for a principal type + """ + + errno = 4034 + format = _( + '%(operation)s is not supported for %(principal_type)s principals') + + class BuiltinError(ExecutionError): """ **4100** Base class for builtin execution errors (*4100 - 4199*). diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py index 63351c54c134f2bd58e8eb18bb0dc3bc6a5734b3..526360bb6b777abd398ad6d5e63f040fb52ac529 100644 --- a/ipaserver/plugins/cert.py +++ b/ipaserver/plugins/cert.py @@ -145,6 +145,12 @@ http://www.ietf.org/rfc/rfc5280.txt USER, HOST, SERVICE = range(3) +PRINCIPAL_TYPE_STRING_MAP = { + USER: _('user'), + HOST: _('host'), + SERVICE: _('service'), +} + register = Registry() PKIDATE_FORMAT = '%Y-%m-%d' @@ -385,7 +391,9 @@ class cert_request(Create, BaseCertMethod, VirtualCommand): ), Flag( 'add', - doc=_("automatically add the principal if it doesn't exist"), + doc=_( + "automatically add the principal if it doesn't exist " + "(service principals only)"), ), ) @@ -480,8 +488,15 @@ class cert_request(Create, BaseCertMethod, VirtualCommand): elif principal_type == USER: principal_obj = api.Command['user_show'](principal_name, all=True) except errors.NotFound as e: - if principal_type == SERVICE and add: - principal_obj = api.Command['service_add'](principal_string, force=True) + if add: + if principal_type == SERVICE: + principal_obj = api.Command['service_add']( + principal_string, force=True) + else: + princtype_str = PRINCIPAL_TYPE_STRING_MAP[principal_type] + raise errors.OperationNotSupportedForPrincipalType( + operation=_("'add' option"), + principal_type=princtype_str) else: raise errors.NotFound( reason=_("The principal for this request doesn't exist.")) -- 2.5.5
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
