<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 05/05/2015 08:57 AM, Jan Cholasta wrote: </div> <blockquote cite="mid:554869C3.2040509@redhat.com" type="cite">Hi, Dne 28.4.2015 v 16:40 thierry bordaz napsal(a): <blockquote type="cite">On 04/28/2015 10:40 AM, David Kupka wrote: <blockquote type="cite">On 04/28/2015 10:28 AM, thierry bordaz wrote: <blockquote type="cite">On 04/28/2015 10:23 AM, David Kupka wrote: <blockquote type="cite">On 04/16/2015 01:00 PM, thierry bordaz wrote: <blockquote type="cite">Hello, Here is the next patch for User life cycle that introduces del/mod/find and show stageuser plugin commands. * 0000-User Life Cycle (create containers and scoping DS plugins): *pushed* * 0001-User-Life-Cycle-Exclude-subtree-for-ipaUniqueID-gene.patch: *pushed* * 0002-User-life-cycle-stageuser-add-verb.patch: *pushed* * 0007-User-life-cycle-allows-MODRDN-from-ldap2.patch: *pushed* * 0003-User-life-cycle-new-stageuser-commands-del-mod-find-*under review *(this one)** * 0004-User-life-cycle-new-stageuser-commands-activate.patch * 0005-User-life-cycle-new-stageuser-commands-activate-prov.patch * 0006-User-life-cycle-user-del-supports-permanently-preser.patch * 0008-User-life-cycle-user-find-support-finding-delete-use.patch * 0009-User-life-cycle-support-of-user-undel.patch * 0010-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch * 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch * 0012-User-life-cycle-Create-stage-Admin-provisioning-acco.patch * 0013-User-life-cycle-Stage-Admin-permission-priviledge.patch Thanks thierry </blockquote> Hi Thierry, thanks for the patch, the code looks good to me but there is probably a bug in ACIs. After creating a stage user and setting password for him I can kinit as the stage user. I'm unable to login to the IPA client and id command for this stage user responds "no such user" but I can kinit and invoke ipa commands. Steps: 0. build freeipa with your patch 1. # ipa-server-install 2. $ kinit admin 3. $ ipa stageuser-add suser0 --first Stage --last User --password 4. $ kdestroy 5. $ kinit suser0 6. $ ipa user-find Actual: Prints out list of ipa users. Expected: kinit fails with <a class="moz-txt-link-rfc2396E" href="mailto:suser0@...notfoundinKerberosdatabase">"suser0@... not found in Kerberos database"</a> </blockquote> Hi David, Thank you so much for having looked at this patch :-) You are right. The Staging users (as well as the Delete users) are not lockout in that patch. The patch 0011-User-life-cycle-lockout-provisioning-stage-and-delet.patch will take care of this. Do you prefer that I merged the two patches right now ? thanks thierry </blockquote> Hi Thierry, no, it is not necessary to merge the patches it's ok to have it separated. I'm not sure if the patch should be pushed now or rather wait and push it together with the others. I'm looking forward to next ULC patches from you. </blockquote> Hi David, Here are all the available patches. I also attach a test script that is a kind of regression tests that I am using. Thanks again thierry </blockquote> Some issues I have found during a quick read of the patches: Patch 0005: 1) This does nothing and can be safely removed: + def pre_callback(self, ldap, dn, *keys, **options): + assert isinstance(dn, DN) + return dn 2) stageuser_{mod,find,show} have a lot of code that seems to be copy-pasted from user_{mod,find,show}. I would prefer if the code was shared instead of copy-pasted, preferably in baseuser_{mod,find,show}. Patch 0006: 1) These do nothing and can be safely removed: + def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): + assert isinstance(dn, DN) + return dn + + def post_callback(self, ldap, dn, entry_attrs, *keys, **options): + assert isinstance(dn, DN) + return dn 2) You should use output.standard_entry for has_output, as you are only returning one entry. Or you could add support for activating multiple users at once. 3) IMO the "time to build the new entry" and "add the active entry" parts should be done by calling user-add, so that the (active) user creation routine is the same. 4) Please use a single line to separate method definitions in classes. Patch 008: 1) I guess you forgot to remove these: + self.log.error("====> user-add pre_callback 1 %s " % dn) + self.log.error("====> user-add pre_callback %s " % dn) 2) + takes_options = LDAPDelete.takes_options + ( This should be "baseuser_del.takes_options + ...". Honza </blockquote> Hello, <blockquote>This is the set of revisited patches for ULC. Here are the changes done versus the previous patches </blockquote> <blockquote> <blockquote><tt>Patch 0005:</tt><tt> </tt> <blockquote><tt>Refactoring stageuser+user => baseuser </tt></blockquote> <tt>Patch 0006:</tt><tt> </tt> <blockquote><tt>Lock stage/delete users, add activated user into ipausers, stageuser-activate process a single entry</tt><tt> </tt><tt> </tt></blockquote> <tt> </tt><tt>Patch 0007</tt><tt> </tt><tt> </tt> <blockquote><tt>Refactoring stageuser+user => baseuser, GID number lost during activation</tt><tt> </tt></blockquote> <tt> Patch 0008 </tt><tt> </tt> <blockquote><tt>user take_options from baseuser_del rather than LDAPDelete</tt><tt> </tt></blockquote> <tt>Patch 0009</tt><tt> </tt><tt> </tt> <blockquote><tt>Refactoring stageuser+user => baseuser, remove debug traces </tt></blockquote> <tt>Patch 0010</tt><tt> </tt> <blockquote><tt>Refactoring stageuser+user => baseuser,, add undelete user into ipausers </tt></blockquote> <tt>Patch 0011 (previous patch 0011 was merged in Patch 0006: lockout stage/delete users)</tt><tt> </tt> <blockquote><tt>Change due to CSV </tt></blockquote> <tt>Patch 0012 </tt><tt> </tt> <blockquote><tt>Change due to CSV, permission tests</tt> </blockquote> </blockquote> </blockquote> <blockquote>It does not take into account your request Honza to use 'user-add' command when activating a user. I will work on that now and submit an other patch later for that. Thanks thierry </blockquote> </body> </html>