[Freeipa-devel] [PATCH] 0005 User life cycle: del/mod/find/show stageuser commands
thierry bordaz
tbordaz at redhat.com
Fri Jun 19 13:10:06 UTC 2015
On 06/18/2015 03:42 PM, David Kupka wrote:
> Dne 18.6.2015 v 13:22 Petr Vobornik napsal(a):
>> On 06/18/2015 12:52 PM, Jan Cholasta wrote:
>>> Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a):
>>>> Dne 15.6.2015 v 17:29 thierry bordaz napsal(a):
>>>>> On 06/15/2015 05:00 PM, Simo Sorce wrote:
>>>>>> On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote:
>>>>>>> On 06/09/2015 02:02 PM, Jan Cholasta wrote:
>>>>>>>> Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a):
>>>>>>>>> Dne 18.5.2015 v 10:33 thierry bordaz napsal(a):
>>>>>>>>>> On 05/15/2015 04:44 PM, David Kupka wrote:
>>>>>>>>>>> Hello Thierry,
>>>>>>>>>>> thanks for the patch set. Overall functionality of ULC feature
>>>>>>>>>>> looks
>>>>>>>>>>> good to
>>>>>>>>>>> me and is definitely "alpha ready".
>>>>>>>>>>>
>>>>>>>>>>> I found following issues but don't insist on fixing it right
>>>>>>>>>>> now:
>>>>>>>>>>>
>>>>>>>>>>> 1) When stageuser-activate fails due to already existent
>>>>>>>>>>> active/deleted user.
>>>>>>>>>>> DN is show instead of user name that's used in other commands
>>>>>>>>>>> (user-add,
>>>>>>>>>>> stageuser-add).
>>>>>>>>>>> $ ipa user-add tuser --first Test --last User
>>>>>>>>>>> $ ipa stageuser-add tuser --first Test --last User
>>>>>>>>>>> $ ipa stageuser-activate tuser
>>>>>>>>>>> ipa: ERROR: Active user
>>>>>>>>>>> uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> already exists
>>>>>>>>>> Hi David, Jan,
>>>>>>>>>>
>>>>>>>>>> Thanks you so much for all those tests and feedback. I agree,
>>>>>>>>>> some
>>>>>>>>>> minor
>>>>>>>>>> bugs can be fixed separatly from this main patches.
>>>>>>>>>>
>>>>>>>>>> You are right, It should return the user ID not the DN.
>>>>>>>>>>
>>>>>>>>>>> 2) According to the design there should be '--only-delete' and
>>>>>>>>>>> '--also-delete'
>>>>>>>>>>> options for user-find command instead there is '--preserved'
>>>>>>>>>>> option.
>>>>>>>>>>> Honza proposed adding virtual boolean attribute 'deleted' to
>>>>>>>>>>> user
>>>>>>>>>>> entry and
>>>>>>>>>>> filter on it.
>>>>>>>>>>> The 'deleted' attribute would be useful also in user-show where
>>>>>>>>>>> is no
>>>>>>>>>>> way to
>>>>>>>>>>> tell if the displayed user is active or deleted. (Except
>>>>>>>>>>> running
>>>>>>>>>>> with
>>>>>>>>>>> --all
>>>>>>>>>>> and looking on the dn).
>>>>>>>>>> Yes a bit late to resynch the design.
>>>>>>>>>> The final option is 'preserved' for user-find and 'preserve' for
>>>>>>>>>> user-del. '--only-delete' or 'also-delete' are old name that I
>>>>>>>>>> need to
>>>>>>>>>> replace in the design.
>>>>>>>>>>
>>>>>>>>>> About the 'deleted' attribute, do you think adding a DS cos
>>>>>>>>>> virtual
>>>>>>>>>> attribute ?
>>>>>>>>> See the attached patch.
>>>>>>>> Can someone please review the patch?
>>>>>>>>
>>>>>>>>>>> 3) uidNumber and gidNumber can't be set back to '-1' once
>>>>>>>>>>> set to
>>>>>>>>>>> other
>>>>>>>>>>> value.
>>>>>>>>>>> This would be useful when admin changes its mind and want
>>>>>>>>>>> IPA to
>>>>>>>>>>> assign them.
>>>>>>>>>>> IIUC, there should be no validation in cn=staged user
>>>>>>>>>>> container.
>>>>>>>>>>> All
>>>>>>>>>>> validation should be done during stageuser-activate.
>>>>>>>>>> Yes that comes from user plugin that enforce the number to be
>>>>>>>>>> >0.
>>>>>>>>>> That is a good point giving the ability to reset
>>>>>>>>>> uidNumber/gidNumber.
>>>>>>>>>> I will check if it is possible, how (give a value or an
>>>>>>>>>> option to
>>>>>>>>>> reset), and also if it would not create other issue.
>>>>>>>>>>> 4) Support for deleted -> stage workflow is still missing. But
>>>>>>>>>>> I'm
>>>>>>>>>>> unsure if we
>>>>>>>>>>> agreed to finish it now or later.
>>>>>>>>>> Yes thanks
>>>>>>>>>>> 5) Twice deleting user with '--preserve' deletes him
>>>>>>>>>>> permanently.
>>>>>>>>>>> $ ipa user-add tuser --first Test --last User
>>>>>>>>>>> $ ipa user-del tuser --preserve
>>>>>>>>>>> $ ipa user-del tuser --preserve
>>>>>>>>>>> $ ipa user-find --preserved
>>>>>>>>>>> ------------------------
>>>>>>>>>>> 0 (delete) users matched
>>>>>>>>>>> ------------------------
>>>>>>>>>>> ----------------------------
>>>>>>>>>>> Number of entries returned 0
>>>>>>>>>>> ----------------------------
>>>>>>>>>> Deleting a deleted (preserved) entry, should permanently remove
>>>>>>>>>> the
>>>>>>>>>> entry.
>>>>>>> +1, but no-op if default behavior is "preserve"
>>>>>>>
>>>>>>>>>> Now if the second time the preserve option is present, it makes
>>>>>>>>>> sense to
>>>>>>>>>> not delete it.
>>>>>>> +1, should be no-op
>>>>>>>
>>>>>>>>> BTW: I might be stating the obvious here, but it would be
>>>>>>>>> better to
>>>>>>>>> use
>>>>>>>>> one boolean parameter rather than two mutually exclusive flags in
>>>>>>>>> user-del.
>>>>>>>> I would like an opinion on this as well.
>>>>>>>>
>>>>>>> So the proposal is, e.g.,:
>>>>>>>
>>>>>>> Replace:
>>>>>>> ipa user del fbar --preserve
>>>>>>> ipa user del fbar --permanently
>>>>>>> with:
>>>>>>> ipa user del fbar --permanently=False
>>>>>>> ipa user del fbar --permanently=True
>>>>>>> and
>>>>>>> ipa user del fbar
>>>>>>> uses the default behavior(permanently atm.)
>>>>>>>
>>>>>>> I don't think there is a big difference. A boolean is easier for
>>>>>>> scripting. 2 options are more descriptive for humans. With a single
>>>>>>> boolean, I would be afraid that omitting it would imply False to
>>>>>>> some
>>>>>>> users which is not always the same as "the default behavior" [1].
>>>>>>>
>>>>>>> With Web UI developer hat I would vote for single boolean but as a
>>>>>>> CLI
>>>>>>> user I would like the current options.
>>>>>>>
>>>>>>> Given that Web UI or any other API client should not define CLI, I
>>>>>>> would
>>>>>>> keep the current options.
>>>>>>>
>>>>>>> my 2c
>>>>>>>
>>>>>>> [1]
>>>>>>> http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User
>>>>>>>
>>>>>>> --
>>>>>>> Petr Vobornik
>>>>>>>
>>>>>> +1 --preserve is 100x better for a human than --permanently=False
>>>>>
>>>>> I also prefere --preserve for usability of 'user del'.
>>>>>
>>>>> In addition we have 'user find|show --preserved' to retrieve users
>>>>> that
>>>>> have been preserved. So it seems to me better that the action that
>>>>> preserved the user uses the option '--preserve' rather
>>>>> '--permanently=False'.
>>>>
>>>> It's ridiculous that the CLI taints the RPC API and it should be
>>>> fixed.
>>>>
>>>> Also on a more nitpicky side, I think the flag should be called
>>>> --no-preserve rather than --permanently. There is plenty of commands
>>>> (rm, cp, ...) which have --no-preserve as opposite of --preserve.
>>>>
>>>> The attached patch fixes both.
>>>
>>> ... and it also accidentaly changes the default behavior.
>>>
>>> Updated patch attached.
>>>
>>
>> ACK if others are ok with changing --permanently to --no-preserve.
>>
>> Patch 446 fixed also issue #5, patch 446.1 doesn't fix it. Could be
>> fixed separately.
>>
>> Attaching patch which addresses this API change in Web UI.
>>
>>
>
> pvoborni's patch 0880 works for me, ACK. I also applied jcholast's
> patch 446.1 and did not encounter any issue.
>
Hello,
A question about preserved users and the CLI user-del and user-find:
[root at vm-205 ~]# ipa user-del xy1 *--preserve*
------------------
Deleted user "xy1"
------------------
[root at vm-205 ~]# ipa user-find *--preserved=true*
---------------
3 users matched
---------------
...
User login: xy1
First name: x
Last name: y
Home directory: /home/xy1
Login shell: /bin/sh
Email address: xy1 at idm.lab.eng.brq.redhat.com
UID: 670400009
GID: 670400009
Account disabled: True
Preserved user: True
Password: False
Kerberos keys available: False
----------------------------
Number of entries returned 3
----------------------------
preserve is a flag for user-del and an option for user-find.
Would it be ok to switch the user-find option into a flag as well ?
Also doing further tests I think a permission is missing to delete a
preserved user.
[root at vm-205 ~]# ipa user-del tb10
ipa: ERROR: Insufficient access: Insufficient 'delete' privilege
to delete the entry 'uid=tb10,cn=deleted
users,cn=accounts,cn=provisioning,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com'.
[root at vm-205 ~]# klist
Ticket cache: KEYRING:persistent:0:krb_ccache_FflBNiM
Default principal: stageadm at IDM.LAB.ENG.BRQ.REDHAT.COM
Valid starting Expires Service principal
06/19/2015 14:16:47 06/20/2015 14:16:47
krbtgt/IDM.LAB.ENG.BRQ.REDHAT.COM at IDM.LAB.ENG.BRQ.REDHAT.COM
After the alpha release, do I need to open a ticket for any changes ?
thanks
thierry
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150619/82e4a1f2/attachment.htm>
More information about the Freeipa-devel
mailing list