[Freeipa-devel] [PATCH] 0003 User life cycle: new stageuser plugin with add verb

[thierry bordaz]

On 09/01/2014 01:08 PM, Petr Viktorin wrote:
> On 08/08/2014 03:54 PM, thierry bordaz wrote:
>> Hi,
>>
>> The attached patch is related to 'User Life Cycle'
>> (https://fedorahosted.org/freeipa/ticket/3813)
>>
>> It creates a stageuser plugin with a first function stageuser-add. Stage
>> user entries are provisioned under 'cn=staged
>> users,cn=accounts,cn=provisioning,SUFFIX'.
>>
>> Thanks
>> thierry
>
> Avoid `from ipalib.plugins.baseldap import *` in new code; instead 
> import the module itself and use e.g. `baseldap.LDAPObject`.
>
> The stageuser help (docstring) is copied from the user plugin, and 
> discusses things like account lockout and disabling users. It should 
> rather explain what stageuser itself does. (And I don't very much like 
> the Note about the interface being badly designed...)
> Also decide if the docs should call it "staged user" or "stage user" 
> or "stageuser".
Hello Petr,

    Thanks for your review.

    I will rewrite the docstring to be only staged user related and I 
    will adopt 'stage user' and 'stageuser'.
    About the interface, that is correct that I was not able to find a
    good solution.
    I need to add a 'stageuser' I use 'stageuser-add' and  '--first' and
    '--last' are required.
    Now when a user got deleted ('user-del') and is later move to 'stage
    user' it also use the command line 'stageuser-add'.
    At this time the delete user is know/found by it 'user login'/uid.
    So '--first' and '--last' are not required to find it (can be used
    to check givenname/sn).
    Now I do not expect that 'stageuser-add' will be frequently used to
    move a Delete user to Stage user, so it is not a so painful constraint.

>
> A lot of the code is copied and pasted over from the users plugin. 
> Don't do that. Either import things (e.g. validate_nsaccountlock) from 
> the users plugin, or move the reused code into a shared module.
>
> For the `user` object, since so much is the same, it might be best to 
> create a common base class for user and stageuser; and similarly for 
> the Command plugins.

    I agree that user and stageuser have a lot of code in common. So it
    would be beneficial to have a common base class. Is it ok to put
    this in a file under freeipa/ipalib ? About the name of this class,
    what about 'accounts' or 'user_accounts' ?
    What do you mean by 'similarly for Command plugins' ?. If I create,
    for example, a freeipa/ipalib/accounts.py containing all the common
    code for 'user' and 'stageuser'. Then import it into
    freeipa/ipalib/plugins/user and freeipa/ipalib/plugins/stageuser I
    believe it will refactore the 'Command plugins'.

>
> The default permissions need different names, and you don't need 
> another copy of the 'non_object' ones. Also, run the makeaci script.

Ok. I will update the names. 'non_object' is not clear to me, what does 
this exactly mean when a managed_permission has 'non_object' True.
In update_managed_permissions.py it is said that if True, it target a 
defaults value. But on the other side, the managed permissions also 
define 'ipapermtarget' (like ipauser or UPG)

many thanks Petr
thierry





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140902/ae138241/attachment.htm>