On 02/28/2012 09:57 PM, Rob Crittenden wrote:
Ondrej Hamada wrote:
On 02/27/2012 03:22 PM, Rob Crittenden wrote:
Ondrej Hamada wrote:
When adding or modifying permission with both type and attributes
specified, check whether the attributes are allowed for specified
type.
In case of disallowed attributes the InvalidSyntax error is raised.
New tests were also added to the unit-tests.
https://fedorahosted.org/freeipa/ticket/2293
https://www.redhat.com/mailman/listinfo/freeipa-devel
NACK. You should use obj.object_class_config to determine if the
default list of objectclasses comes from LDAP.
I think that may be it, otherwise the patch reads ok.
I'm very glad to see unit tests!
rob
Corrected
Sorry, found a couple of more things I should have found the first
review.
Please use the dn module to construct dn_ipaconfig. Or you can also
get the DN on-the-fly since the config object using get_dn().
Probably just as safe to call: if obj.object_class_config: ... rather
than hasattr. I suppose its just a style thing.
Done.
I wonder if ObjectclassViolation is a better exception. SyntaxError
means the data type is wrong, not that it isn't allowed.
I agree that it makes more sense and I've updated the patch that way,
but the documentation says: "permission operation fails with schema
syntax errors" - maybe we should also update the documentation.
rob
--
Regards,
Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada
From 1f98c50a64cfa5f564ac77f60796d952f2d44edf Mon Sep 17 00:00:00 2001
From: Ondrej Hamada <oham...@redhat.com>
Date: Wed, 29 Feb 2012 11:40:31 +0100
Subject: [PATCH] Validate attributes in permission-add
When adding or modifying permission with both type and attributes
specified, check whether the attributes are allowed for specified type.
In case of disallowed attributes raises the ObjectclassViolation
exception.
New tests were also added to the unit-tests.
https://fedorahosted.org/freeipa/ticket/2293
---
ipalib/plugins/permission.py | 55 ++++++++++++++++++++++
tests/test_xmlrpc/test_permission_plugin.py | 65 +++++++++++++++++++++++++++
2 files changed, 120 insertions(+), 0 deletions(-)
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 08781ce2ef3df30d10565a071a338edf77c52d23..c9fd5649f338b5c92b86e471fb817b9d964084d3 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -23,6 +23,7 @@ from ipalib import api, _, ngettext
from ipalib import Flag, Str, StrEnum
from ipalib.request import context
from ipalib import errors
+from ipalib.dn import DN
__doc__ = _("""
Permissions
@@ -89,6 +90,43 @@ output_params = (
),
)
+dn_ipaconfig = str(DN('cn=ipaconfig,cn=etc,%s' % api.env.basedn))
+
+def check_attrs(attrs, type):
+ # Trying to delete attributes - no need for validation
+ if attrs is None:
+ return True
+ allowed_objcls=[]
+ disallowed_objcls=[]
+ obj=api.Object[type]
+
+ if obj.object_class_config:
+ (dn,objcls)=api.Backend.ldap2.get_entry(
+ dn_ipaconfig,[obj.object_class_config]
+ )
+ allowed_objcls=objcls[obj.object_class_config]
+ else:
+ allowed_objcls=obj.object_class
+ if obj.possible_objectclasses:
+ allowed_objcls+=obj.possible_objectclasses
+ if obj.disallow_object_classes:
+ disallowed_objcls=obj.disallow_object_classes
+
+ allowed_attrs=[]
+ disallowed_attrs=[]
+ if allowed_objcls:
+ allowed_attrs=api.Backend.ldap2.get_allowed_attributes(allowed_objcls)
+ if disallowed_objcls:
+ disallowed_attrs=api.Backend.ldap2.get_allowed_attributes(disallowed_objcls)
+ failed_attrs=[]
+ for attr in attrs:
+ if (attr not in allowed_attrs) or (attr in disallowed_attrs):
+ failed_attrs.append(attr)
+ if failed_attrs:
+ raise errors.ObjectclassViolation(info='attribute(s) \"%s\" not allowed' % ','.join(failed_attrs))
+ return True
+
+
class permission(LDAPObject):
"""
Permission object.
@@ -192,6 +230,8 @@ class permission_add(LDAPCreate):
opts['permission'] = keys[-1]
opts['aciprefix'] = ACI_PREFIX
try:
+ if 'type' in entry_attrs and 'attrs' in entry_attrs:
+ check_attrs(entry_attrs['attrs'],entry_attrs['type'])
self.api.Command.aci_add(keys[-1], **opts)
except Exception, e:
raise e
@@ -273,6 +313,21 @@ class permission_mod(LDAPUpdate):
except errors.NotFound:
self.obj.handle_not_found(*keys)
+ # check the correctness of attributes only when the type is specified
+ type=None
+ attrs_to_check=[]
+ current_values=self.api.Command.permission_show(attrs['cn'][0])['result']
+ if 'type' in entry_attrs:
+ type = entry_attrs['type']
+ elif 'type' in current_values:
+ type = current_values['type']
+ if 'attrs' in entry_attrs:
+ attrs_to_check = entry_attrs['attrs']
+ elif 'attrs' in current_values:
+ attrs_to_check = current_values['attrs']
+ if attrs_to_check and type is not None:
+ check_attrs(attrs_to_check,type)
+
# when renaming permission, check if the target permission does not
# exists already. Then, make changes to underlying ACI
if 'rename' in options:
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index e8e6bebcd387307f30e4a7bc4d266092b7e41424..68a3cebf97943b330926fcf9837e9815e142d086 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -124,6 +124,71 @@ class test_permission(Declarative):
dict(
+ desc='Try to create %r with invalid attribute \'ipaclientversion\'' % permission2,
+ command=(
+ 'permission_add', [permission2], dict(
+ type=u'user',
+ permissions=u'write',
+ attrs=u'ipaclientversion',
+ ),
+ ),
+ expected=errors.ObjectclassViolation(info=u'attribute(s) \"ipaclientversion\" not allowed'),
+ ),
+
+
+ dict(
+ desc='Add allowed attribute \'cn\' to %r' % permission1,
+ command=(
+ 'permission_mod', [permission1], dict(
+ attrs=u'cn',
+ )
+ ),
+ expected=dict(
+ value=permission1,
+ summary=u'Modified permission "%s"' % permission1,
+ result=dict(
+ dn=lambda x: DN(x) == permission1_dn,
+ cn=[permission1],
+ type=u'user',
+ permissions=[u'write'],
+ attrs=[u'cn'],
+ ),
+ ),
+ ),
+
+
+ dict(
+ desc='Try to modify %r with invalid attribute \'ipaclientversion\'' % permission1,
+ command=(
+ 'permission_mod', [permission1], dict(
+ attrs=u'ipaclientversion',
+ ),
+ ),
+ expected=errors.ObjectclassViolation(info=u'attribute(s) \"ipaclientversion\" not allowed'),
+ ),
+
+
+ dict(
+ desc='Unset attribute \'cn\' of %r' % permission1,
+ command=(
+ 'permission_mod', [permission1], dict(
+ attrs=None,
+ )
+ ),
+ expected=dict(
+ value=permission1,
+ summary=u'Modified permission "%s"' % permission1,
+ result=dict(
+ dn=lambda x: DN(x) == permission1_dn,
+ cn=[permission1],
+ type=u'user',
+ permissions=[u'write'],
+ ),
+ ),
+ ),
+
+
+ dict(
desc='Create %r' % privilege1,
command=('privilege_add', [privilege1],
dict(description=u'privilege desc. 1')
--
1.7.6.5
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
