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
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
$ ipa user-add tuser --first Test --last User
$ ipa stageuser-add tuser --first Test --last User
$ ipa stageuser-activate tuser
ipa: ERROR: Active user

already exists
Hi David, Jan,

Thanks you so much for all those tests and feedback. I agree, some
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
options for user-find command instead there is '--preserved'
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
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
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
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.
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
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
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
+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
one boolean parameter rather than two mutually exclusive flags in
I would like an opinion on this as well.

So the proposal is, e.g.,:

    ipa user del fbar --preserve
    ipa user del fbar --permanently
    ipa user del fbar --permanently=False
    ipa user del fbar --permanently=True
    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
user I would like the current options.

Given that Web UI or any other API client should not define CLI, I
keep the current options.

my 2c

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

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.


   A question about preserved users and the CLI user-del and user-find:

       [root@vm-205 ~]# ipa user-del xy1 *--preserve*
       Deleted user "xy1"
       [root@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: x...@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@vm-205 ~]# ipa user-del tb10
       ipa: ERROR: Insufficient access: Insufficient 'delete' privilege
       to delete the entry 'uid=tb10,cn=deleted
       [root@vm-205 ~]# klist
       Ticket cache: KEYRING:persistent:0:krb_ccache_FflBNiM
       Default principal: stage...@idm.lab.eng.brq.redhat.com

       Valid starting       Expires              Service principal
       06/19/2015 14:16:47  06/20/2015 14:16:47

   After the alpha release, do I need to open a ticket for any changes ?


Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to