Re: [Freeipa-devel] [PATCH 78] Ticket #2979 - prevent last admin from being disabled
On 09/03/2012 07:53 AM, Petr Viktorin wrote:
On 08/26/2012 07:19 PM, John Dennis wrote:
On 08/20/2012 01:37 PM, Petr Viktorin wrote:
(Sorry if you're getting this twice; I didn't send it to the list)
On 08/16/2012 08:38 PM, John Dennis wrote:
--
John Dennis jden...@redhat.com
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
freeipa-jdennis-0078-Ticket-2979-prevent-last-admin-from-being-disabled.patch
From c47109c63530e188db76986fdda48c76bf681d10 Mon Sep 17 00:00:00 2001
From: John Dennisjden...@redhat.com
Date: Thu, 16 Aug 2012 20:28:44 -0400
Subject: [PATCH 78] Ticket #2979 - prevent last admin from being
disabled
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
We prevent the last member of the admin group from being deleted. The
same check needs to be performed when disabling a user.
Moved the code in del_user to a common subroutine and call it from
both user_del and user_disable. Note, unlike user_del user_disable
does not have a 'pre' callback therefore the check function is called
in user_disable's execute routine.
This should also prevent disabling all admins if there's more than one:
# ipa user-add admin2 --first=a --last=b
---
Added user admin2
---
...
# ipa group-add-member admins --user=admin2
-
Number of members added 1
-
# ipa user-disable admin2
--
Disabled user account admin2
--
# ipa user-disable admin
--
Disabled user account admin
--
# ipa ping
ipa: ERROR: Server is unwilling to perform: Account inactivated. Contact
system administrator.
Also with one enabled and one disabled admin, it shouldn't be possible
to delete the enabled one.
Please add some tests; you can extend the ones added in commit f8e7b51.
Good catch with respect to disabled users, thank you.
Reworked patch attached, see patch comments.
Works well now, just the error message is incorrect: it mentions only
deleting, not disabling.
$ ipa user-disable admin
ipa: ERROR: admin cannot be deleted because it is the last member of
group admins
Updated the error message to say
... cannot be deleted or disabled because ...
--
John Dennis jden...@redhat.com
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From 798069ca293ce98ea62b20a27664ebd78d1e4a4d Mon Sep 17 00:00:00 2001
From: John Dennis jden...@redhat.com
Date: Thu, 16 Aug 2012 20:28:44 -0400
Subject: [PATCH 78-2] prevent last admin from being disabled
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
We prevent the last member of the admin group from being deleted. The
same check needs to be performed when disabling a user.
* Moved the code in del_user to the common subroutine
check_protected_member() and call it from both user_del and
user_disable. Note, unlike user_del user_disable does not have a
'pre' callback therefore the check function is called in
user_disable's execute routine.
* Make check_protected_member() aware of disabled members. It's not
sufficient to check which members of the protected group are
present, one must only consider those members which are enabled.
* Add tests to test_user_plugin.py.
- verify you cannot delete nor disable the last member of the admin
group
- verify when the admin group contains disabled users in addition to
enabled users only the enabled users are considered when
determining if the last admin is about to be disabled or deleted.
* Replace duplicated hardcoded values in the tests with variables or
subroutines, this makes the individual tests a bit more succinct and
easier to copy/modify.
* Update error msg to reflect either deleting or disabling is an error.
https://fedorahosted.org/freeipa/ticket/2979
---
ipalib/errors.py | 6 +-
ipalib/plugins/user.py| 27 ++-
tests/test_xmlrpc/test_user_plugin.py | 443 +-
3 files changed, 303 insertions(+), 173 deletions(-)
diff --git a/ipalib/errors.py b/ipalib/errors.py
index c25560b..1bff2ac 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1627,18 +1627,18 @@ class DependentEntry(ExecutionError):
class LastMemberError(ExecutionError):
-**4308** Raised when an entry being deleted is last member of a protected group
+**4308** Raised when an entry being deleted or disabled is last member of a protected group
For example:
raise LastMemberError(key=u'admin', label=u'group', container=u'admins')
Traceback (most recent call last):
...
-LastMemberError: admin cannot be deleted because it is the last member of group admins
+LastMemberError: admin cannot be deleted or disabled because it is the last member of group admins
errno = 4308
-format = _('%(key)s cannot be deleted because it is the last member of %(label)s
Re: [Freeipa-devel] [PATCH 78] Ticket #2979 - prevent last admin from being disabled
On 09/03/2012 04:41 PM, John Dennis wrote: On 09/03/2012 07:53 AM, Petr Viktorin wrote: On 08/26/2012 07:19 PM, John Dennis wrote: On 08/20/2012 01:37 PM, Petr Viktorin wrote: (Sorry if you're getting this twice; I didn't send it to the list) On 08/16/2012 08:38 PM, John Dennis wrote: -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ freeipa-jdennis-0078-Ticket-2979-prevent-last-admin-from-being-disabled.patch From c47109c63530e188db76986fdda48c76bf681d10 Mon Sep 17 00:00:00 2001 From: John Dennisjden...@redhat.com Date: Thu, 16 Aug 2012 20:28:44 -0400 Subject: [PATCH 78] Ticket #2979 - prevent last admin from being disabled Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit We prevent the last member of the admin group from being deleted. The same check needs to be performed when disabling a user. Moved the code in del_user to a common subroutine and call it from both user_del and user_disable. Note, unlike user_del user_disable does not have a 'pre' callback therefore the check function is called in user_disable's execute routine. This should also prevent disabling all admins if there's more than one: # ipa user-add admin2 --first=a --last=b --- Added user admin2 --- ... # ipa group-add-member admins --user=admin2 - Number of members added 1 - # ipa user-disable admin2 -- Disabled user account admin2 -- # ipa user-disable admin -- Disabled user account admin -- # ipa ping ipa: ERROR: Server is unwilling to perform: Account inactivated. Contact system administrator. Also with one enabled and one disabled admin, it shouldn't be possible to delete the enabled one. Please add some tests; you can extend the ones added in commit f8e7b51. Good catch with respect to disabled users, thank you. Reworked patch attached, see patch comments. Works well now, just the error message is incorrect: it mentions only deleting, not disabling. $ ipa user-disable admin ipa: ERROR: admin cannot be deleted because it is the last member of group admins Updated the error message to say ... cannot be deleted or disabled because ... ACK. Please push John's patch 81 before this one; that way it applies cleanly. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 78] Ticket #2979 - prevent last admin from being disabled
On 08/20/2012 01:37 PM, Petr Viktorin wrote:
(Sorry if you're getting this twice; I didn't send it to the list)
On 08/16/2012 08:38 PM, John Dennis wrote:
--
John Dennis jden...@redhat.com
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
freeipa-jdennis-0078-Ticket-2979-prevent-last-admin-from-being-disabled.patch
From c47109c63530e188db76986fdda48c76bf681d10 Mon Sep 17 00:00:00 2001
From: John Dennisjden...@redhat.com
Date: Thu, 16 Aug 2012 20:28:44 -0400
Subject: [PATCH 78] Ticket #2979 - prevent last admin from being disabled
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
We prevent the last member of the admin group from being deleted. The
same check needs to be performed when disabling a user.
Moved the code in del_user to a common subroutine and call it from
both user_del and user_disable. Note, unlike user_del user_disable
does not have a 'pre' callback therefore the check function is called
in user_disable's execute routine.
This should also prevent disabling all admins if there's more than one:
# ipa user-add admin2 --first=a --last=b
---
Added user admin2
---
...
# ipa group-add-member admins --user=admin2
-
Number of members added 1
-
# ipa user-disable admin2
--
Disabled user account admin2
--
# ipa user-disable admin
--
Disabled user account admin
--
# ipa ping
ipa: ERROR: Server is unwilling to perform: Account inactivated. Contact
system administrator.
Also with one enabled and one disabled admin, it shouldn't be possible
to delete the enabled one.
Please add some tests; you can extend the ones added in commit f8e7b51.
Good catch with respect to disabled users, thank you.
Reworked patch attached, see patch comments.
--
John Dennis jden...@redhat.com
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From c635ac107de3b0a3965c596224b0f8c73c9139ac Mon Sep 17 00:00:00 2001
From: John Dennis jden...@redhat.com
Date: Thu, 16 Aug 2012 20:28:44 -0400
Subject: [PATCH 78-1] Ticket #2979 - prevent last admin from being disabled
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Ticket #2979 - prevent last admin from being disabled
We prevent the last member of the admin group from being deleted. The
same check needs to be performed when disabling a user.
* Moved the code in del_user to the common subroutine
check_protected_member() and call it from both user_del and
user_disable. Note, unlike user_del user_disable does not have a
'pre' callback therefore the check function is called in
user_disable's execute routine.
* Make check_protected_member() aware of disabled members. It's not
sufficient to check which members of the protected group are
present, one must only consider those members which are enabled.
* Add tests to test_user_plugin.py.
- verify you cannot delete nor disable the last member of the admin
group
- verify when the admin group contains disabled users in addition to
enabled users only the enabled users are considered when
determining if the last admin is about to be disabled or deleted.
* Replace duplicated hardcoded values in the tests with variables or
subroutines, this makes the individual tests a bit more succinct and
easier to copy/modify.
---
ipalib/plugins/user.py| 27 ++-
tests/test_xmlrpc/test_user_plugin.py | 443 +-
2 files changed, 300 insertions(+), 170 deletions(-)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 529699f..d0f7da2 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -166,6 +166,24 @@ def normalize_principal(principal):
return unicode('%s@%s' % (user, realm))
+def check_protected_member(user, protected_group_name=u'admins'):
+'''
+Ensure the last enabled member of a protected group cannot be deleted or
+disabled by raising LastMemberError.
+'''
+
+# Get all users in the protected group
+result = api.Command.user_find(in_group=protected_group_name)
+
+# Build list of users in the protected group who are enabled
+result = result['result']
+enabled_users = [entry['uid'][0] for entry in result if not entry['nsaccountlock']]
+
+# If the user is the last enabled user raise LastMemberError exception
+if enabled_users == [user]:
+raise errors.LastMemberError(key=user, label=_(u'group'),
+container=protected_group_name)
+
class user(LDAPObject):
User object.
@@ -550,11 +568,7 @@ class user_del(LDAPDelete):
def pre_callback(self, ldap, dn, *keys, **options):
assert isinstance(dn, DN)
-protected_group_name = u'admins'
-result = api.Command.group_show(protected_group_name)
-if result['result'].get('member_user', []) == [keys[-1]]:
-
Re: [Freeipa-devel] [PATCH 78] Ticket #2979 - prevent last admin from being disabled
(Sorry if you're getting this twice; I didn't send it to the list) On 08/16/2012 08:38 PM, John Dennis wrote: -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ freeipa-jdennis-0078-Ticket-2979-prevent-last-admin-from-being-disabled.patch From c47109c63530e188db76986fdda48c76bf681d10 Mon Sep 17 00:00:00 2001 From: John Dennisjden...@redhat.com Date: Thu, 16 Aug 2012 20:28:44 -0400 Subject: [PATCH 78] Ticket #2979 - prevent last admin from being disabled Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit We prevent the last member of the admin group from being deleted. The same check needs to be performed when disabling a user. Moved the code in del_user to a common subroutine and call it from both user_del and user_disable. Note, unlike user_del user_disable does not have a 'pre' callback therefore the check function is called in user_disable's execute routine. This should also prevent disabling all admins if there's more than one: # ipa user-add admin2 --first=a --last=b --- Added user admin2 --- ... # ipa group-add-member admins --user=admin2 - Number of members added 1 - # ipa user-disable admin2 -- Disabled user account admin2 -- # ipa user-disable admin -- Disabled user account admin -- # ipa ping ipa: ERROR: Server is unwilling to perform: Account inactivated. Contact system administrator. Also with one enabled and one disabled admin, it shouldn't be possible to delete the enabled one. Please add some tests; you can extend the ones added in commit f8e7b51. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
