[Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
thierry bordaz
tbordaz at redhat.com
Tue May 12 12:17:02 UTC 2015
On 05/05/2015 08:57 AM, Jan Cholasta wrote:
> Hi,
>
> Dne 28.4.2015 v 16:40 thierry bordaz napsal(a):
>> On 04/28/2015 10:40 AM, David Kupka wrote:
>>> On 04/28/2015 10:28 AM, thierry bordaz wrote:
>>>> On 04/28/2015 10:23 AM, David Kupka wrote:
>>>>> On 04/16/2015 01:00 PM, thierry bordaz wrote:
>>>>>> 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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> 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 "suser0 at ... not found in Kerberos database"
>>>>>
>>>> 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
>>>>
>>>
>>> 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.
>>>
>>
>>
>> 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
>>
>>
>
> 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
>
Hello,
This is the set of revisited patches for ULC. Here are the changes
done versus the previous patches
Patch 0005:
Refactoring stageuser+user => baseuser
Patch 0006:
Lock stage/delete users, add activated user into ipausers,
stageuser-activate process a single entry
Patch 0007
Refactoring stageuser+user => baseuser, GID number lost
during activation
Patch 0008
user take_options from baseuser_del rather than LDAPDelete
Patch 0009
Refactoring stageuser+user => baseuser, remove debug traces
Patch 0010
Refactoring stageuser+user => baseuser,, add undelete user
into ipausers
Patch 0011 (previous patch 0011 was merged in Patch 0006:
lockout stage/delete users)
Change due to CSV
Patch 0012
Change due to CSV, permission tests
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0013-User-life-cycle-Add-Stage-User-Provisioning-permissi.patch
Type: text/x-patch
Size: 5606 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0012-1-User-life-cycle-Stage-user-Administrators-permission.patch
Type: text/x-patch
Size: 30415 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0011-1-User-life-cycle-DNA-DS-plugin-should-exclude-provisi.patch
Type: text/x-patch
Size: 987 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0010-1-User-life-cycle-support-of-user-undel.patch
Type: text/x-patch
Size: 4502 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0009-1-User-life-cycle-user-find-support-finding-delete-use.patch
Type: text/x-patch
Size: 4608 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0008-1-User-life-cycle-user-del-supports-permanently-preser.patch
Type: text/x-patch
Size: 9203 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0007-1-User-life-cycle-new-stageuser-commands-activate-prov.patch
Type: text/x-patch
Size: 6710 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0006-1-User-life-cycle-new-stageuser-commands-activate.patch
Type: text/x-patch
Size: 17812 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbordaz-0005-1-User-life-cycle-new-stageuser-commands-del-mod-find-.patch
Type: text/x-patch
Size: 29803 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150512/34d57bb8/attachment-0008.bin>
More information about the Freeipa-devel
mailing list