[Freeipa-devel] [PATCH] 0082 cert-request: better error msg when 'add' not supported

[Fraser Tweedale]

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
-------------- next part --------------
From f5e6d032d437904a7fcadecf90ec9b74a0f4348e Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal at 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          |  9 +++++++++
 ipaserver/plugins/cert.py | 20 +++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/ipalib/errors.py b/ipalib/errors.py
index 10491a94211648df8bda60f3dbc9e52d19e83d10..70d17d64f53c75aabf7ae99c56bebd136230c7a3 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1396,6 +1396,15 @@ class ServerRemovalError(ExecutionError):
     errno = 4033
     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):
     """
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 888621fc5af634b95addc9c0ade58c76ce42edfe..9c3b3d2cd5630ce9f05efd2c8c0020ab8af284bd 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,14 @@ 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'", principal_type=princtype_str)
             else:
                 raise errors.NotFound(
                     reason=_("The principal for this request doesn't exist."))
-- 
2.5.5