When investigating this issue I became very annoyed by the star import hiding where names come from, so I did some cleanup first.


In krbtpolicy, an ACIError is now raised if:
- the user doesn't have permission to read any one of the ticket policy
  attributes on the requested entry
  (checked using attribute-level rights)
- any ticket policy attribute from the default policy is not available
  (either not readable, or not there at all)
  (only checked if these are accessed, i.e. when the user entry doesn't
   override all of the defaults, or when requesting the global policy)

That means if the user is not available at all, you get a NotFound, but if global policy is not found it's assumed that it's just unreadable.

--
PetrĀ³
From 4760edee0db8dd7f1d24daeee0b2501c485dc828 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Thu, 29 May 2014 15:39:26 +0200
Subject: [PATCH] krbtpolicy plugin: Code cleanup

- Use the new plugin registration API
  See: http://www.freeipa.org/page/Coding_Best_Practices#Decorator-based_plugin_registration

- Remove the star import from baseldap
  Part of the work for: https://fedorahosted.org/freeipa/ticket/2653
---
 ipalib/plugins/krbtpolicy.py | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/ipalib/plugins/krbtpolicy.py b/ipalib/plugins/krbtpolicy.py
index 22a961a4ab6005b138d0fb261d2a90527b373c48..a3b971e14697470790f8cd01fb45b194223a8226 100644
--- a/ipalib/plugins/krbtpolicy.py
+++ b/ipalib/plugins/krbtpolicy.py
@@ -17,10 +17,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from ipalib import api
+from ipalib import api, errors, output, _
 from ipalib import Int, Str
-from ipalib.plugins.baseldap import *
-from ipalib import _
+from ipalib.plugins import baseldap
+from ipalib.plugins.baseldap import entry_to_dict, pkey_to_value
+from ipalib.plugable import Registry
+from ipapython.dn import DN
 
 __doc__ = _("""
 Kerberos ticket policy
@@ -60,6 +62,8 @@
   ipa krbtpolicy-mod admin --maxlife=3600
 """)
 
+register = Registry()
+
 # FIXME: load this from a config file?
 _default_values = {
     'krbmaxticketlife': 86400,
@@ -67,7 +71,8 @@
 }
 
 
-class krbtpolicy(LDAPObject):
+@register()
+class krbtpolicy(baseldap.LDAPObject):
     """
     Kerberos Ticket Policy object
     """
@@ -141,10 +146,9 @@ def get_dn(self, *keys, **kwargs):
             return self.api.Object.user.get_dn(*keys, **kwargs)
         return DN(self.container_dn, api.env.basedn)
 
-api.register(krbtpolicy)
 
-
-class krbtpolicy_mod(LDAPUpdate):
+@register()
+class krbtpolicy_mod(baseldap.LDAPUpdate):
     __doc__ = _('Modify Kerberos ticket policy.')
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
@@ -155,10 +159,9 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
             options['all'] = False
         return dn
 
-api.register(krbtpolicy_mod)
 
-
-class krbtpolicy_show(LDAPRetrieve):
+@register()
+class krbtpolicy_show(baseldap.LDAPRetrieve):
     __doc__ = _('Display the current Kerberos ticket policy.')
 
     def pre_callback(self, ldap, dn, attrs_list, *keys, **options):
@@ -180,10 +183,9 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
                     entry_attrs.setdefault(a, res['result'][a])
         return dn
 
-api.register(krbtpolicy_show)
 
-
-class krbtpolicy_reset(LDAPQuery):
+@register()
+class krbtpolicy_reset(baseldap.LDAPQuery):
     __doc__ = _('Reset Kerberos ticket policy to the default values.')
 
     has_output = output.standard_entry
@@ -217,5 +219,3 @@ def execute(self, *keys, **options):
         entry_attrs = entry_to_dict(entry_attrs, **options)
 
         return dict(result=entry_attrs, value=pkey_to_value(keys[-1], options))
-
-api.register(krbtpolicy_reset)
-- 
1.9.0

From cdaf5411c5f86eb18f63d54937be0d729bc441f5 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 27 May 2014 16:22:33 +0200
Subject: [PATCH] krbtpolicy plugin: Fix internal error when global policy is
 not readable

An ACIError is now raised if:
- the user doesn't have permission to read any one of the ticket policy
  attributes on the requested entry
  (checked using attribute-level rights)
- any ticket policy attribute from the default policy is not available
  (either not readable, or not there at all)
  (only checked if these are accessed, i.e. when the user entry doesn't
   override all of the defaults, or when requesting the global policy)

Part of the work for: https://fedorahosted.org/freeipa/ticket/4354
---
 ipalib/plugins/krbtpolicy.py | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/krbtpolicy.py b/ipalib/plugins/krbtpolicy.py
index a3b971e14697470790f8cd01fb45b194223a8226..2a2d323a2921e33bc78267fc240b9b3cc4f00fa2 100644
--- a/ipalib/plugins/krbtpolicy.py
+++ b/ipalib/plugins/krbtpolicy.py
@@ -172,15 +172,33 @@ def pre_callback(self, ldap, dn, attrs_list, *keys, **options):
             options['all'] = False
         return dn
 
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        assert isinstance(dn, DN)
-        if keys[-1] is not None:
-            # if policy for a specific user isn't set, display global values
-            if 'krbmaxticketlife' not in entry_attrs or \
-                'krbmaxrenewableage' not in entry_attrs:
-                res = self.api.Command.krbtpolicy_show()
-                for a in self.obj.default_attributes:
-                    entry_attrs.setdefault(a, res['result'][a])
+    def post_callback(self, ldap, dn, entry, *keys, **options):
+        default_entry = None
+        rights = None
+        for attrname in self.obj.default_attributes:
+            if attrname not in entry:
+                if keys[-1] is not None:
+                    # User entry ddoesn't override the attribute.
+                    # Check if this is caused by insufficient read rights
+                    if rights is None:
+                        rights = baseldap.get_effective_rights(
+                            ldap, dn, self.obj.default_attributes)
+                    if 'r' not in rights.get(attrname.lower(), ''):
+                        raise errors.ACIError(
+                            info=_('Ticket policy for %s could not be read') %
+                                keys[-1])
+                    # Fallback to the default
+                    if default_entry is None:
+                        try:
+                            default_dn = self.obj.get_dn(None)
+                            default_entry = ldap.get_entry(default_dn)
+                        except errors.NotFound:
+                            default_entry = {}
+                    if attrname in default_entry:
+                        entry[attrname] = default_entry[attrname]
+            if attrname not in entry:
+                raise errors.ACIError(
+                    info=_('Default ticket policy could not be read'))
         return dn
 
 
-- 
1.9.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to