URL: https://github.com/freeipa/freeipa/pull/523
Author: frasertweedale
 Title: #523: cert-request: minor refactors
Action: opened

PR body:
"""
A couple of minor refactors done as part of GSS-API work
(https://pagure.io/freeipa/issue/5011).
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/523/head:pr523
git checkout pr523
From 2d85605be3cded5025426ed61e6833fcf9975012 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Wed, 25 Jan 2017 15:51:46 +1000
Subject: [PATCH 1/2] Remove redundant principal_type argument

Minor refactor to remove the redundant 'principal_type' argument
from 'caacl_check' and associated functions.

Part of: https://pagure.io/freeipa/issue/5011
---
 ipaserver/plugins/caacl.py |  8 +++++++-
 ipaserver/plugins/cert.py  | 13 +++++--------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/ipaserver/plugins/caacl.py b/ipaserver/plugins/caacl.py
index a7817c4..ff1178a 100644
--- a/ipaserver/plugins/caacl.py
+++ b/ipaserver/plugins/caacl.py
@@ -151,7 +151,13 @@ def _acl_make_rule(principal_type, obj):
     return rule
 
 
-def acl_evaluate(principal_type, principal, ca_id, profile_id):
+def acl_evaluate(principal, ca_id, profile_id):
+    if principal.is_user:
+        principal_type = 'user'
+    elif principal.is_host:
+        principal_type = 'host'
+    else:
+        principal_type = 'service'
     req = _acl_make_request(principal_type, principal, ca_id, profile_id)
     acls = api.Command.caacl_find(no_members=False)['result']
     rules = [_acl_make_rule(principal_type, obj) for obj in acls]
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 585a70e..46518d9 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -200,11 +200,9 @@ def ca_enabled_check(_api):
     if not _api.Command.ca_is_enabled()['result']:
         raise errors.NotFound(reason=_('CA is not configured'))
 
-def caacl_check(principal_type, principal, ca, profile_id):
-    principal_type_map = {USER: 'user', HOST: 'host', SERVICE: 'service'}
-    if not acl_evaluate(
-            principal_type_map[principal_type],
-            principal, ca, profile_id):
+
+def caacl_check(principal, ca, profile_id):
+    if not acl_evaluate(principal, ca, profile_id):
         raise errors.ACIError(info=_(
                 "Principal '%(principal)s' "
                 "is not permitted to use CA '%(ca)s' "
@@ -599,7 +597,7 @@ def execute(self, csr, all=False, raw=False, **kw):
             if principal_type == KRBTGT:
                 ca_kdc_check(ldap, bind_principal.hostname)
             else:
-                caacl_check(principal_type, principal, ca, profile_id)
+                caacl_check(principal, ca, profile_id)
 
         try:
             csr_obj = pkcs10.load_certificate_request(csr)
@@ -756,8 +754,7 @@ def execute(self, csr, all=False, raw=False, **kw):
                     if principal_type == KRBTGT:
                         ca_kdc_check(ldap, alt_principal.hostname)
                     else:
-                        caacl_check(principal_type, alt_principal, ca,
-                                    profile_id)
+                        caacl_check(alt_principal, ca, profile_id)
 
             elif isinstance(gn, (x509.KRB5PrincipalName, x509.UPN)):
                 if principal_type == KRBTGT:

From 4aa4ecea14827387d9e9430790d8a453a7fa9c96 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Wed, 25 Jan 2017 16:14:59 +1000
Subject: [PATCH 2/2] Extract method to map principal to princpal type

Part of: https://pagure.io/freeipa/issue/5011
---
 ipaserver/plugins/cert.py | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 46518d9..b53caf4 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -558,29 +558,17 @@ def execute(self, csr, all=False, raw=False, **kw):
 
         principal = kw.get('principal')
         principal_string = unicode(principal)
+        principal_type = principal_to_principal_type(principal)
 
-        if principal.is_user:
-            principal_type = USER
-        elif principal.is_host:
-            principal_type = HOST
-        elif principal.service_name == 'krbtgt':
-            principal_type = KRBTGT
+        if principal_type == KRBTGT:
             if profile_id != self.Backend.ra.KDC_PROFILE:
                 raise errors.ACIError(
                     info=_("krbtgt certs can use only the %s profile") % (
                            self.Backend.ra.KDC_PROFILE))
-        else:
-            principal_type = SERVICE
 
         bind_principal = kerberos.Principal(getattr(context, 'principal'))
         bind_principal_string = unicode(bind_principal)
-
-        if bind_principal.is_user:
-            bind_principal_type = USER
-        elif bind_principal.is_host:
-            bind_principal_type = HOST
-        else:
-            bind_principal_type = SERVICE
+        bind_principal_type = principal_to_principal_type(bind_principal)
 
         if (bind_principal_string != principal_string and
                 bind_principal_type != HOST):
@@ -834,6 +822,17 @@ def execute(self, csr, all=False, raw=False, **kw):
         )
 
 
+def principal_to_principal_type(principal):
+    if principal.is_user:
+        return USER
+    elif principal.is_host:
+        return HOST
+    elif principal.service_name == 'krbtgt':
+        return KRBTGT
+    else:
+        return SERVICE
+
+
 def _dns_name_matches_principal(name, principal, principal_obj):
     """
     Ensure that a DNS name matches the given principal.
-- 
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

Reply via email to