Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/21/2015 10:37 AM, thierry bordaz wrote: On 08/20/2015 01:59 PM, Martin Babinsky wrote: On 08/20/2015 12:11 PM, Martin Babinsky wrote: On 08/20/2015 11:41 AM, thierry bordaz wrote: On 08/19/2015 06:28 PM, Martin Babinsky wrote: On 08/19/2015 02:54 PM, Martin Babinsky wrote: this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from happening. Actually, we (myself, mbasti, jcholast) found out that `user-del --preserve` could use some more usability improvements. This quick patch should fix both https://fedorahosted.org/freeipa/ticket/5234 and https://fedorahosted.org/freeipa/ticket/5236 and make user preservation operate on multiple arguments in a same way as plain deletion. Hi Martin, The tests are ok and the fix looks good to me. ACK thanks thierry That's nice, but I have found some small nitpicks and will send an updated version. So self-NACK. Attaching updated patch. This is working fine. ACK Pushed to: master: c6299a8cfde7d4e4bb9a50e3cf6406667cee0a6f ipa-4-2: 361a4fb4100824b27b777c27c329d50361ba69f4 -- 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
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/20/2015 01:59 PM, Martin Babinsky wrote: On 08/20/2015 12:11 PM, Martin Babinsky wrote: On 08/20/2015 11:41 AM, thierry bordaz wrote: On 08/19/2015 06:28 PM, Martin Babinsky wrote: On 08/19/2015 02:54 PM, Martin Babinsky wrote: this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from happening. Actually, we (myself, mbasti, jcholast) found out that `user-del --preserve` could use some more usability improvements. This quick patch should fix both https://fedorahosted.org/freeipa/ticket/5234 and https://fedorahosted.org/freeipa/ticket/5236 and make user preservation operate on multiple arguments in a same way as plain deletion. Hi Martin, The tests are ok and the fix looks good to me. ACK thanks thierry That's nice, but I have found some small nitpicks and will send an updated version. So self-NACK. Attaching updated patch. This is working fine. ACK -- 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
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/20/2015 12:11 PM, Martin Babinsky wrote:
On 08/20/2015 11:41 AM, thierry bordaz wrote:
On 08/19/2015 06:28 PM, Martin Babinsky wrote:
On 08/19/2015 02:54 PM, Martin Babinsky wrote:
this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from
happening.
Actually, we (myself, mbasti, jcholast) found out that `user-del
--preserve` could use some more usability improvements.
This quick patch should fix both
https://fedorahosted.org/freeipa/ticket/5234 and
https://fedorahosted.org/freeipa/ticket/5236 and make user
preservation operate on multiple arguments in a same way as plain
deletion.
Hi Martin,
The tests are ok and the fix looks good to me.
ACK
thanks
thierry
That's nice, but I have found some small nitpicks and will send an
updated version.
So self-NACK.
Attaching updated patch.
--
Martin^3 Babinsky
From 762f8b13748ad51213ed245bfe6915d302feaca8 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Wed, 19 Aug 2015 14:43:14 +0200
Subject: [PATCH 1/3] improve the usability of `ipa user-del --preserve`
command
`ipa user-del` with `--preserve` option will now process multiple entries and
handle `--continue` option in a manner analogous to `ipa user-del` in normal
mode.
In addition, it is now no longer possible to permanently delete a user by
accidentally running `ipa user-del --preserve` twice.
https://fedorahosted.org/freeipa/ticket/5234
https://fedorahosted.org/freeipa/ticket/5236
---
ipalib/plugins/user.py | 123 ++---
1 file changed, 66 insertions(+), 57 deletions(-)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 1d6073b4240d963e2b047c20fe5b8be702ef3184..9f8dc993bb6225668aeb1cabafd20c6b98cdfd8c 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -593,6 +593,60 @@ class user_del(baseuser_del):
),
)
+def _preserve_user(self, pkey, delete_container, **options):
+assert isinstance(delete_container, DN)
+
+dn = self.obj.get_either_dn(pkey, **options)
+delete_dn = DN(dn[0], delete_container)
+ldap = self.obj.backend
+self.log.debug("preserve move %s -> %s" % (dn, delete_dn))
+
+if dn.endswith(delete_container):
+raise errors.ExecutionError(
+_('%s: user is already preserved' % pkey)
+)
+# Check that this value is a Active user
+try:
+original_entry_attrs = self._exc_wrapper(
+pkey, options, ldap.get_entry)(dn, ['dn'])
+except errors.NotFound:
+self.obj.handle_not_found(pkey)
+
+# start to move the entry to Delete container
+self._exc_wrapper(pkey, options, ldap.move_entry)(dn, delete_dn,
+ del_old=True)
+
+# Then clear the credential attributes
+attrs_to_clear = ['krbPrincipalKey', 'krbLastPwdChange',
+ 'krbPasswordExpiration', 'userPassword']
+
+entry_attrs = self._exc_wrapper(pkey, options, ldap.get_entry)(
+delete_dn, attrs_to_clear)
+
+clearedCredential = False
+for attr in attrs_to_clear:
+if attr.lower() in entry_attrs:
+del entry_attrs[attr]
+clearedCredential = True
+if clearedCredential:
+self._exc_wrapper(pkey, options, ldap.update_entry)(entry_attrs)
+
+# Then restore some original entry attributes
+attrs_to_restore = ['secretary', 'managedby', 'manager', 'ipauniqueid',
+'uidnumber', 'gidnumber', 'passwordHistory']
+
+entry_attrs = self._exc_wrapper(
+pkey, options, ldap.get_entry)(delete_dn, attrs_to_restore)
+
+restoreAttr = False
+for attr in attrs_to_restore:
+if ((attr.lower() in original_entry_attrs) and
+not (attr.lower() in entry_attrs)):
+restoreAttr = True
+entry_attrs[attr.lower()] = original_entry_attrs[attr.lower()]
+if restoreAttr:
+self._exc_wrapper(pkey, options, ldap.update_entry)(entry_attrs)
+
def forward(self, *keys, **options):
if self.api.env.context == 'cli':
if options['no_preserve'] and options['preserve']:
@@ -633,68 +687,23 @@ class user_del(baseuser_del):
def execute(self, *keys, **options):
-dn = self.obj.get_either_dn(*keys, **options)
-
# We are going to permanent delete or the user is already in the delete container.
delete_container = DN(self.obj.delete_container_dn, self.api.env.basedn)
-user_from_delete_container = dn.endswith(delete_container)
-
-if not options.get('preserve', True) or user_from_delete_container:
-# Remove any ID overrides tied with this user
-remove_ipaobject_overrides(self.obj.backend, self.obj.api, dn)
-
-# Issue a true DEL on that entry
-return super(user_del
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/20/2015 11:41 AM, thierry bordaz wrote: On 08/19/2015 06:28 PM, Martin Babinsky wrote: On 08/19/2015 02:54 PM, Martin Babinsky wrote: this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from happening. Actually, we (myself, mbasti, jcholast) found out that `user-del --preserve` could use some more usability improvements. This quick patch should fix both https://fedorahosted.org/freeipa/ticket/5234 and https://fedorahosted.org/freeipa/ticket/5236 and make user preservation operate on multiple arguments in a same way as plain deletion. Hi Martin, The tests are ok and the fix looks good to me. ACK thanks thierry That's nice, but I have found some small nitpicks and will send an updated version. So self-NACK. -- Martin^3 Babinsky -- 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
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/19/2015 06:28 PM, Martin Babinsky wrote: On 08/19/2015 02:54 PM, Martin Babinsky wrote: this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from happening. Actually, we (myself, mbasti, jcholast) found out that `user-del --preserve` could use some more usability improvements. This quick patch should fix both https://fedorahosted.org/freeipa/ticket/5234 and https://fedorahosted.org/freeipa/ticket/5236 and make user preservation operate on multiple arguments in a same way as plain deletion. Hi Martin, The tests are ok and the fix looks good to me. ACK thanks thierry -- 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
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/20/2015 11:05 AM, thierry bordaz wrote: On 08/19/2015 06:28 PM, Martin Babinsky wrote: On 08/19/2015 02:54 PM, Martin Babinsky wrote: this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from happening. Actually, we (myself, mbasti, jcholast) found out that `user-del --preserve` could use some more usability improvements. This quick patch should fix both https://fedorahosted.org/freeipa/ticket/5234 and https://fedorahosted.org/freeipa/ticket/5236 and make user preservation operate on multiple arguments in a same way as plain deletion. Hi Martin, This is curious it is looking like in my test the fix does not prevent the deletion: [root@vm-141 freeipa]# ipa user-del ttest1 --preserve - Deleted user "ttest1" - [root@vm-141 freeipa]# ipa user-del ttest1 --preserve - Deleted user "ttest1" - [root@vm-141 freeipa]# ipa user-find ttest1 --preserve=true --- 0 users matched --- Number of entries returned 0 [20/Aug/2015:11:00:33 +0200] conn=124 op=9 MODRDN dn="uid=ttest1,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" newrdn="uid=ttest1" newsuperior="cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" [20/Aug/2015:11:00:33 +0200] conn=124 op=9 RESULT err=0 tag=109 nentries=0 etime=0 ... [20/Aug/2015:11:00:44 +0200] conn=125 op=14 SRCH base="uid=ttest1,cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" scope=0 filter="(objectClass=*)" attrs="distinguishedName" [20/Aug/2015:11:00:44 +0200] conn=125 op=14 RESULT err=0 tag=101 nentries=1 etime=0 [20/Aug/2015:11:00:44 +0200] conn=125 op=15 DEL dn="uid=ttest1,cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" [20/Aug/2015:11:00:44 +0200] conn=125 op=15 RESULT *err=0* tag=107 nentries=0 etime=0 ... [20/Aug/2015:11:00:57 +0200] conn=126 op=5 SRCH base="cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" scope=1 filter="(&(|(telephoneNumber=*ttest1*)(uid=*ttest1*)(title=*ttest1*)(sn=*ttest1*)(ou=*ttest1*)(givenName=*ttest1*))(objectClass=posixaccount))" attrs="telephoneNumber sshpubkeyfp uid title loginShell uidNumber gidNumber sn homeDirectory mail givenName nsAccountLock" [20/Aug/2015:11:00:57 +0200] conn=126 op=5 RESULT err=0 tag=101 *nentries=0* etime=0 Hi Martin, Sorry I did a mistake in my tests.. retesting it -- 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
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/19/2015 06:28 PM, Martin Babinsky wrote: On 08/19/2015 02:54 PM, Martin Babinsky wrote: this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from happening. Actually, we (myself, mbasti, jcholast) found out that `user-del --preserve` could use some more usability improvements. This quick patch should fix both https://fedorahosted.org/freeipa/ticket/5234 and https://fedorahosted.org/freeipa/ticket/5236 and make user preservation operate on multiple arguments in a same way as plain deletion. Hi Martin, This is curious it is looking like in my test the fix does not prevent the deletion: [root@vm-141 freeipa]# ipa user-del ttest1 --preserve - Deleted user "ttest1" - [root@vm-141 freeipa]# ipa user-del ttest1 --preserve - Deleted user "ttest1" - [root@vm-141 freeipa]# ipa user-find ttest1 --preserve=true --- 0 users matched --- Number of entries returned 0 [20/Aug/2015:11:00:33 +0200] conn=124 op=9 MODRDN dn="uid=ttest1,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" newrdn="uid=ttest1" newsuperior="cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" [20/Aug/2015:11:00:33 +0200] conn=124 op=9 RESULT err=0 tag=109 nentries=0 etime=0 ... [20/Aug/2015:11:00:44 +0200] conn=125 op=14 SRCH base="uid=ttest1,cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" scope=0 filter="(objectClass=*)" attrs="distinguishedName" [20/Aug/2015:11:00:44 +0200] conn=125 op=14 RESULT err=0 tag=101 nentries=1 etime=0 [20/Aug/2015:11:00:44 +0200] conn=125 op=15 DEL dn="uid=ttest1,cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" [20/Aug/2015:11:00:44 +0200] conn=125 op=15 RESULT *err=0* tag=107 nentries=0 etime=0 ... [20/Aug/2015:11:00:57 +0200] conn=126 op=5 SRCH base="cn=deleted users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" scope=1 filter="(&(|(telephoneNumber=*ttest1*)(uid=*ttest1*)(title=*ttest1*)(sn=*ttest1*)(ou=*ttest1*)(givenName=*ttest1*))(objectClass=posixaccount))" attrs="telephoneNumber sshpubkeyfp uid title loginShell uidNumber gidNumber sn homeDirectory mail givenName nsAccountLock" [20/Aug/2015:11:00:57 +0200] conn=126 op=5 RESULT err=0 tag=101 *nentries=0* etime=0 -- 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
Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
On 08/19/2015 02:54 PM, Martin Babinsky wrote:
this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from
happening.
Actually, we (myself, mbasti, jcholast) found out that `user-del
--preserve` could use some more usability improvements.
This quick patch should fix both
https://fedorahosted.org/freeipa/ticket/5234 and
https://fedorahosted.org/freeipa/ticket/5236 and make user preservation
operate on multiple arguments in a same way as plain deletion.
--
Martin^3 Babinsky
From e39668c86015c14ed13325db23470f0c66d13392 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Wed, 19 Aug 2015 14:43:14 +0200
Subject: [PATCH] improve the usability of `ipa user-del --preserve` command
`ipa user-del` with `--preserve` option will now process multiple entries and
handle `--continue` option in a manner analogous to `ipa user-del` in normal
mode.
In addition, it is now no longer possible to permanently delete a user by
accidentally running `ipa user-del --preserve` twice.
https://fedorahosted.org/freeipa/ticket/5234
https://fedorahosted.org/freeipa/ticket/5236
---
ipalib/plugins/user.py | 123 ++---
1 file changed, 66 insertions(+), 57 deletions(-)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 1d6073b4240d963e2b047c20fe5b8be702ef3184..6acea611127ee7b5af1f66c99c10dfe7b93ad47c 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -593,6 +593,60 @@ class user_del(baseuser_del):
),
)
+def preserve_user(self, pkey, delete_container, **options):
+assert isinstance(delete_container, DN)
+
+dn = self.obj.get_either_dn(pkey, **options)
+delete_dn = DN(dn[0], delete_container)
+ldap = self.obj.backend
+self.log.debug("preserve move %s -> %s" % (dn, delete_dn))
+
+if dn.endswith(delete_container):
+raise errors.ExecutionError(
+_('User "%s" is already preserved' % pkey)
+)
+# Check that this value is a Active user
+try:
+original_entry_attrs = self._exc_wrapper(
+pkey, options, ldap.get_entry)(dn, ['dn'])
+except errors.NotFound:
+self.obj.handle_not_found(pkey)
+
+# start to move the entry to Delete container
+self._exc_wrapper(pkey, options, ldap.move_entry)(dn, delete_dn,
+ del_old=True)
+
+# Then clear the credential attributes
+attrs_to_clear = ['krbPrincipalKey', 'krbLastPwdChange',
+ 'krbPasswordExpiration', 'userPassword']
+
+entry_attrs = self._exc_wrapper(pkey, options, ldap.get_entry)(
+delete_dn, attrs_to_clear)
+
+clearedCredential = False
+for attr in attrs_to_clear:
+if attr.lower() in entry_attrs:
+del entry_attrs[attr]
+clearedCredential = True
+if clearedCredential:
+self._exc_wrapper(pkey, options, ldap.update_entry)(entry_attrs)
+
+# Then restore some original entry attributes
+attrs_to_restore = ['secretary', 'managedby', 'manager', 'ipauniqueid',
+'uidnumber', 'gidnumber', 'passwordHistory']
+
+entry_attrs = self._exc_wrapper(
+pkey, options, ldap.get_entry)(delete_dn, attrs_to_restore)
+
+restoreAttr = False
+for attr in attrs_to_restore:
+if ((attr.lower() in original_entry_attrs) and
+not (attr.lower() in entry_attrs)):
+restoreAttr = True
+entry_attrs[attr.lower()] = original_entry_attrs[attr.lower()]
+if restoreAttr:
+self._exc_wrapper(pkey, options, ldap.update_entry)(entry_attrs)
+
def forward(self, *keys, **options):
if self.api.env.context == 'cli':
if options['no_preserve'] and options['preserve']:
@@ -633,68 +687,23 @@ class user_del(baseuser_del):
def execute(self, *keys, **options):
-dn = self.obj.get_either_dn(*keys, **options)
-
# We are going to permanent delete or the user is already in the delete container.
delete_container = DN(self.obj.delete_container_dn, self.api.env.basedn)
-user_from_delete_container = dn.endswith(delete_container)
-
-if not options.get('preserve', True) or user_from_delete_container:
-# Remove any ID overrides tied with this user
-remove_ipaobject_overrides(self.obj.backend, self.obj.api, dn)
-
-# Issue a true DEL on that entry
-return super(user_del, self).execute(*keys, **options)
# The user to delete is active and there is no 'no_preserve' option
if options.get('preserve', False):
-
-ldap = self.obj.backend
-
-# need to handle multiple keys (e.g. keys[-1]=(u'tb8', u'tb9')..
-active_dn = self.obj.get_either_dn(*keys, **options)
-s
[Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user
this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from
happening.
--
Martin^3 Babinsky
From a01ceb4906cb35141c664ba94c088f4e29209b68 Mon Sep 17 00:00:00 2001
From: Martin Babinsky
Date: Wed, 19 Aug 2015 14:43:14 +0200
Subject: [PATCH] raise an error when trying to preserve an already preserved
user
this also fixes a case when a user is permanently deleted when `ipa user-del
--preserve` is accidentally called multiple times on the same uid.
https://fedorahosted.org/freeipa/ticket/5234
---
ipalib/plugins/user.py | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 1d6073b4240d963e2b047c20fe5b8be702ef3184..1659830d77c822887ea7e6d0f293db02bffb3250 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -639,12 +639,16 @@ class user_del(baseuser_del):
delete_container = DN(self.obj.delete_container_dn, self.api.env.basedn)
user_from_delete_container = dn.endswith(delete_container)
-if not options.get('preserve', True) or user_from_delete_container:
+if not options.get('preserve', False):
# Remove any ID overrides tied with this user
remove_ipaobject_overrides(self.obj.backend, self.obj.api, dn)
# Issue a true DEL on that entry
return super(user_del, self).execute(*keys, **options)
+elif user_from_delete_container:
+raise errors.ExecutionError(
+_('One or more users are already preserved')
+)
# The user to delete is active and there is no 'no_preserve' option
if options.get('preserve', False):
--
2.4.3
--
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
