On 09/25/2012 02:31 PM, Martin Kosek wrote:
On 09/25/2012 02:22 PM, Tomas Babej wrote:
Hi,

Group-mod command no longer allows --rename and/or --external
changes made to the admins group. In such cases, ProtectedEntryError
is being raised.

https://fedorahosted.org/freeipa/ticket/3098

Tomas

Just based on a quick glance, I see few issues:

1) I would like to see unit tests for this scenario

2) Some overlooked debug output:

+        self.log.info(str(keys))

Martin
I removed the unfortunate debug output and added two unit tests to test the scenarios.

Tomas
>From ff16c3664168c71b536db612d1310892e84f2974 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 25 Sep 2012 08:14:57 -0400
Subject: [PATCH] Restrict admins group modifications

Group-mod command no longer allows --rename and/or --external
changes made to the admins group. In such cases, ProtectedEntryError
is being raised.

https://fedorahosted.org/freeipa/ticket/3098
---
 ipalib/errors.py                       |  6 +++---
 ipalib/plugins/group.py                | 11 +++++++++++
 tests/test_xmlrpc/test_group_plugin.py | 15 +++++++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/ipalib/errors.py b/ipalib/errors.py
index 6a4e2c5d68f6a6f9b94d8e6b3d7a81d5c1922093..3dc38a4fba1ce826dba03f75937e2609baf3b5bf 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1643,18 +1643,18 @@ class LastMemberError(ExecutionError):
 
 class ProtectedEntryError(ExecutionError):
     """
-    **4309** Raised when an entry being deleted is protected
+    **4309** Raised when an entry being deleted or modified in a forbidden way is protected
 
     For example:
     >>> raise ProtectedEntryError(label=u'group', key=u'admins', reason=_(u'privileged group'))
     Traceback (most recent call last):
       ...
-    ProtectedEntryError: group admins cannot be deleted: privileged group
+    ProtectedEntryError: group admins cannot be deleted/modified: privileged group
 
     """
 
     errno = 4309
-    format = _('%(label)s %(key)s cannot be deleted: %(reason)s')
+    format = _('%(label)s %(key)s cannot be deleted/modified: %(reason)s')
 
 
 ##############################################################################
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index f1e34bd56fc2427e2e9f60da89cab731021e1db0..c567990bd1105f214e9fb9fea6a1c4bba0fc976d 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -260,6 +260,13 @@ class group_mod(LDAPUpdate):
 
     def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
         assert isinstance(dn, DN)
+
+        is_admins_group = u'admins' in keys
+
+        if 'rename' in options:
+            if is_admins_group:
+                raise errors.ProtectedEntryError(label=u'group', key=u'admins', reason=u'Cannot be renamed.')
+
         if ('posix' in options and options['posix']) or 'gidnumber' in options:
             (dn, old_entry_attrs) = ldap.get_entry(dn, ['objectclass'])
             if 'ipaexternalgroup' in old_entry_attrs['objectclass']:
@@ -272,7 +279,10 @@ class group_mod(LDAPUpdate):
                 entry_attrs['objectclass'] = old_entry_attrs['objectclass']
                 if not 'gidnumber' in options:
                     entry_attrs['gidnumber'] = 999
+
         if options['external']:
+            if is_admins_group:
+                raise errors.ProtectedEntryError(label=u'group', key=u'admins', reason=u'Cannot support external non-IPA members.')
             (dn, old_entry_attrs) = ldap.get_entry(dn, ['objectclass'])
             if 'posixgroup' in old_entry_attrs['objectclass']:
                 raise errors.PosixGroupViolation()
@@ -281,6 +291,7 @@ class group_mod(LDAPUpdate):
             else:
                 old_entry_attrs['objectclass'].append('ipaexternalgroup')
                 entry_attrs['objectclass'] = old_entry_attrs['objectclass']
+
         # Can't check for this in a validator because we lack context
         if 'gidnumber' in options and options['gidnumber'] is None:
             raise errors.RequirementError(name='gid')
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 77a419b0c86c08cd8f16f452215706ae577a056a..007e9508388b38a900d8befa347075af6f623dde 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -870,6 +870,21 @@ class test_group(Declarative):
                 key='admins', reason='privileged group'),
         ),
 
+
+        dict(
+            desc='Try to rename the admins group',
+            command=('group_mod', [u'admins'], dict(rename=u'loosers')),
+            expected=errors.ProtectedEntryError(label=u'group',
+                key='admins', reason='Cannot be renamed.'),
+        ),
+
+        dict(
+            desc='Try to modify the admins group to support external membership',
+            command=('group_mod', [u'admins'], dict(external=True)),
+            expected=errors.ProtectedEntryError(label=u'group',
+                key='admins', reason='Cannot support external non-IPA members.'),
+        ),
+
         dict(
             desc='Delete %r' % user1,
             command=('user_del', [user1], {}),
-- 
1.7.11.4

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

Reply via email to