[PATCH] Make sss_userdel check for logged in users

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

sss_userdel now refuses to delete users who are logged in unless --force
is used.

Fixes: #229
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkujfbIACgkQHsardTLnvCX4sACfZOYnwyRdRwP3vp6bSZXl0DbA
> 1oQAoK0FJuq6nnLl2pW4apXLhqK86PDS
> =OKSx
> -----END PGP SIGNATURE-----

[Sumit Bose]

On Fri, Mar 19, 2010 at 02:35:46PM +0100, Jakub Hrozek wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  sss_userdel now refuses to delete users who are logged in unless --force
>  is used.
>  
>  Fixes: #229
> >  -----BEGIN PGP SIGNATURE-----
> >  Version: GnuPG v1.4.10 (GNU/Linux)
> >  Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
> >  
> >  iEYEARECAAYFAkujfbIACgkQHsardTLnvCX4sACfZOYnwyRdRwP3vp6bSZXl0DbA
> >  1oQAoK0FJuq6nnLl2pW4apXLhqK86PDS
> >  =OKSx
> >  -----END PGP SIGNATURE----- 
>  
>   
>  +static bool is_logged_in(TALLOC_CTX *mem_ctx, uid_t uid)
>  +{
>  +    int ret;
>  +    hash_key_t key;
>  +    hash_value_t value;
>  +    hash_table_t *uid_table;
>  +
>  +    ret = get_uid_table(mem_ctx, &uid_table);
>  +    /* get_uid_table returns ENOSYS on non-Linux platforms. We proceed
>  +     * in that case
>  +     */
>  +    if (ret == ENOSYS) return false; 
I would return true here. Then the user can still be deleted with
--force, but the admin will know that he has to be careful on this
plattform.

bye,
Sumit


>  +    if (ret != EOK) {
>  +        DEBUG(1, ("Cannot initialize hash table.\n"));
>  +        return false;
>  +    }
>  +
>  +    key.type = HASH_KEY_ULONG;
>  +    key.ul   = (unsigned long) uid;
>  +
>  +    ret = hash_lookup(uid_table, &key, &value);
>  +    talloc_zfree(uid_table);
>  +    return ret == HASH_SUCCESS;
>  +}
>  +

[Simo Sorce]

On Fri, 19 Mar 2010 15:13:57 +0100
Sumit Bose <sbose(a)redhat.com&gt; wrote:

>  I would return true here. Then the user can still be deleted with
>  --force, but the admin will know that he has to be careful on this
>  plattform. 
Sumit,
I disagree, it would require --force for any delete, looks like
too much.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

[Sumit Bose]

On Fri, Mar 19, 2010 at 10:21:20AM -0400, Simo Sorce wrote:
>  On Fri, 19 Mar 2010 15:13:57 +0100
>  Sumit Bose <sbose(a)redhat.com&gt; wrote:
>  
>  > I would return true here. Then the user can still be deleted with
>  > --force, but the admin will know that he has to be careful on this
>  > plattform.
>  
>  Sumit,
>  I disagree, it would require --force for any delete, looks like
>  too much.
>   
Only on plattforms where the check is not implemented. I think people
will expect a warning if the user is logged in, so it is an unexpected
behaviour if I can delete a user who is logged in without --force.

bye,
Sumit

>  Simo.
>  
>  -- 
>  Simo Sorce * Red Hat, Inc * New York
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/sssd-devel

[Simo Sorce]

On Fri, 19 Mar 2010 15:30:17 +0100
Sumit Bose <sbose(a)redhat.com&gt; wrote:

>  On Fri, Mar 19, 2010 at 10:21:20AM -0400, Simo Sorce wrote:
>  > On Fri, 19 Mar 2010 15:13:57 +0100
>  > Sumit Bose <sbose(a)redhat.com&gt; wrote:
>  > 
>  > > I would return true here. Then the user can still be deleted with
>  > > --force, but the admin will know that he has to be careful on this
>  > > plattform.
>  > 
>  > Sumit,
>  > I disagree, it would require --force for any delete, looks like
>  > too much.
>  > 
>  
>  Only on plattforms where the check is not implemented. I think people
>  will expect a warning if the user is logged in, so it is an unexpected
>  behaviour if I can delete a user who is logged in without --force. 
If the check is not implemented you have no way to know, so you would
always return a warning even when there is no reason. This would make
an habbit to use --force without even thinking about it. So if later on
the platform gets a method it will never be used.

I agree with checks on platfroms where we can determine if a user is
logged or not, but always requiring --force on platforms that do not
have a way to check seem basically useless and badly educates the admin.

IMO.
Simo.


-- 
Simo Sorce * Red Hat, Inc * New York

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/19/2010 03:57 PM, Simo Sorce wrote:
>  On Fri, 19 Mar 2010 15:30:17 +0100
>  Sumit Bose <sbose(a)redhat.com&gt; wrote:
>  
> > On Fri, Mar 19, 2010 at 10:21:20AM -0400, Simo Sorce wrote:
> >> On Fri, 19 Mar 2010 15:13:57 +0100
> >> Sumit Bose <sbose(a)redhat.com&gt; wrote:
> >>
> >>> I would return true here. Then the user can still be deleted with
> >>> --force, but the admin will know that he has to be careful on this
> >>> plattform.
> >>
> >> Sumit,
> >> I disagree, it would require --force for any delete, looks like
> >> too much.
> >>
> >
> > Only on plattforms where the check is not implemented. I think people
> > will expect a warning if the user is logged in, so it is an unexpected
> > behaviour if I can delete a user who is logged in without --force.
>  
>  If the check is not implemented you have no way to know, so you would
>  always return a warning even when there is no reason. This would make
>  an habbit to use --force without even thinking about it. So if later on
>  the platform gets a method it will never be used.
>  
>  I agree with checks on platfroms where we can determine if a user is
>  logged or not, but always requiring --force on platforms that do not
>  have a way to check seem basically useless and badly educates the admin.
>  
>  IMO.
>  Simo.
>  
>   
What about issuing a warning and then proceeding with the delete?
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkujkb0ACgkQHsardTLnvCW24ACg2z/IMz+pLlHk1mTUxNNH4I0x
> VFoAoL0GmaeSDC9uibu9nI08TZ9EmjIE
> =Z/xu
> -----END PGP SIGNATURE-----

[Simo Sorce]

On Fri, 19 Mar 2010 16:01:17 +0100
Jakub Hrozek <jhrozek(a)redhat.com&gt; wrote:

>  What about issuing a warning and then proceeding with the delete?
IMO same educational issues, the admin will learn to ignore that
message. Also it may then spwan a request to add a --quiet switch.

But I am flexible on this.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

[Dmitri Pal]

Simo Sorce wrote:
>  On Fri, 19 Mar 2010 16:01:17 +0100
>  Jakub Hrozek <jhrozek(a)redhat.com&gt; wrote:
>
>    
> > What about issuing a warning and then proceeding with the delete?
> >     
>
>  IMO same educational issues, the admin will learn to ignore that
>  message. Also it may then spwan a request to add a --quiet switch.
>
>  But I am flexible on this.
>
>  Simo.
>
>     
How frequently you delete users? I suspect very rarely.
I do not think we should talk about a habit here.
If the platform does not implement the check we should just give warning
and proceed.
We can add quite switch later if that turns out to be annoying but I
doubt we should even open a ticket for this now.

-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/19/2010 04:09 PM, Dmitri Pal wrote:
>  How frequently you delete users? I suspect very rarely.
>  I do not think we should talk about a habit here.
>  If the platform does not implement the check we should just give warning
>  and proceed.
>  We can add quite switch later if that turns out to be annoying but I
>  doubt we should even open a ticket for this now. 
I agree with this. A patch that warns and proceeds is attached.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkujlmEACgkQHsardTLnvCWrKACgleiQxik8C8h2wZGhkJ4bUFCd
> EgAAoIS879spYriHYOY0z01NLmc7DzX1
> =P7ea
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/19/2010 11:21 AM, Jakub Hrozek wrote:
>  On 03/19/2010 04:09 PM, Dmitri Pal wrote:
> > How frequently you delete users? I suspect very rarely.
> > I do not think we should talk about a habit here.
> > If the platform does not implement the check we should just give warning
> > and proceed.
> > We can add quite switch later if that turns out to be annoying but I
> > doubt we should even open a ticket for this now.
>  
>  I agree with this. A patch that warns and proceeds is attached. 
Could someone explain to me why we need this switch at all? If you're
aiming to delete a user, why do you care if they're still logged in? If
you're deleting that user, you don't want them on your system; why would
it matter if their user suddenly disappeared out from under them?

I can't really think of a reason why this would be useful, except for
notifying the admin to do a 'killall -u <user>' first to boot the user
off the system, but that would be better served to be done after they
were deleted anyway.

I think this isn't really a bug. I think the need for --force is
redundant. We should always proceed with the user deletion and always
warn that the user is still logged in if we are capable of detecting that.

In other words:

On platforms that support get_uid_table():
If the user is not logged in:
  1) Delete the user.
If the user is logged in:
  1) Delete the user.
  2) Print a message stating that the user (with uid N) was logged in
     when you deleted it.
  3) (Optional) Add a --kick flag to sss_userdel that disables the
     user's password, then invokes 'killall -u <username>' and then
     delete the user (to avoid race conditions where a user might
     log back in immediately after being kicked)


On platforms that do not support get_uid_table():
 1) Delete the user.
 2) (Optional) Add a --kick flag to sss_userdel that disables the
     user's password, then invokes 'killall -u <username>' and then
     delete the user (to avoid race conditions where a user might
     log back in immediately after being kicked)

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkuqf5AACgkQeiVVYja6o6M6JwCfboOXJIu/CSFEZrO4ssU21v2C
> AooAn1mdAx5c75xbWjEpDDZTG7aeNJBu
> =Xq/M
> -----END PGP SIGNATURE-----

[Dmitri Pal]

Stephen Gallagher wrote:
>  On 03/19/2010 11:21 AM, Jakub Hrozek wrote:
>  > On 03/19/2010 04:09 PM, Dmitri Pal wrote:
>  >> How frequently you delete users? I suspect very rarely.
>  >> I do not think we should talk about a habit here.
>  >> If the platform does not implement the check we should just give
>  warning
>  >> and proceed.
>  >> We can add quite switch later if that turns out to be annoying but I
>  >> doubt we should even open a ticket for this now.
>  > I agree with this. A patch that warns and proceeds is attached.
>
>  Could someone explain to me why we need this switch at all? If you're
>  aiming to delete a user, why do you care if they're still logged in? If
>  you're deleting that user, you don't want them on your system; why would
>  it matter if their user suddenly disappeared out from under them?
>
>  I can't really think of a reason why this would be useful, except for
>  notifying the admin to do a 'killall -u <user>' first to boot the user
>  off the system, but that would be better served to be done after they
>  were deleted anyway.
>
>  I think this isn't really a bug. I think the need for --force is
>  redundant. We should always proceed with the user deletion and always
>  warn that the user is still logged in if we are capable of detecting that.
>
>  In other words:
>
>  On platforms that support get_uid_table():
>  If the user is not logged in:
>    1) Delete the user.
>  If the user is logged in:
>    1) Delete the user.
>    2) Print a message stating that the user (with uid N) was logged in
>       when you deleted it.
>    3) (Optional) Add a --kick flag to sss_userdel that disables the
>       user's password, then invokes 'killall -u <username>' and then
>       delete the user (to avoid race conditions where a user might
>       log back in immediately after being kicked)
>
>
>  On platforms that do not support get_uid_table():
>   1) Delete the user.
>   2) (Optional) Add a --kick flag to sss_userdel that disables the
>       user's password, then invokes 'killall -u <username>' and then
>       delete the user (to avoid race conditions where a user might
>       log back in immediately after being kicked)
>  
This is a different approach. Here you try to automate the operation
yourself by providing the --kick flag.
With --force flag you leave the responsibility to kick user first to the
caller.

What you propose is better but more work.
I am not sure it is justified.
Opinions?
 


_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/24/2010 10:24 PM, Dmitri Pal wrote:
>  Stephen Gallagher wrote:
> > On 03/19/2010 11:21 AM, Jakub Hrozek wrote:
> >> On 03/19/2010 04:09 PM, Dmitri Pal wrote:
> >>> How frequently you delete users? I suspect very rarely.
> >>> I do not think we should talk about a habit here.
> >>> If the platform does not implement the check we should just give
> > warning
> >>> and proceed.
> >>> We can add quite switch later if that turns out to be annoying but I
> >>> doubt we should even open a ticket for this now.
> >> I agree with this. A patch that warns and proceeds is attached.
> >
> > Could someone explain to me why we need this switch at all? If you're
> > aiming to delete a user, why do you care if they're still logged in? If
> > you're deleting that user, you don't want them on your system; why would
> > it matter if their user suddenly disappeared out from under them?
> >
> > I can't really think of a reason why this would be useful, except for
> > notifying the admin to do a 'killall -u <user>' first to boot the user
> > off the system, but that would be better served to be done after they
> > were deleted anyway.
> >
> > I think this isn't really a bug. I think the need for --force is
> > redundant. We should always proceed with the user deletion and always
> > warn that the user is still logged in if we are capable of detecting that.
> >
> > In other words:
> >
> > On platforms that support get_uid_table():
> > If the user is not logged in:
> >   1) Delete the user.
> > If the user is logged in:
> >   1) Delete the user.
> >   2) Print a message stating that the user (with uid N) was logged in
> >      when you deleted it.
> >   3) (Optional) Add a --kick flag to sss_userdel that disables the
> >      user's password, then invokes 'killall -u <username>' and
> then
> >      delete the user (to avoid race conditions where a user might
> >      log back in immediately after being kicked)
> >
> >
> > On platforms that do not support get_uid_table():
> >  1) Delete the user.
> >  2) (Optional) Add a --kick flag to sss_userdel that disables the
> >      user's password, then invokes 'killall -u <username>' and
> then
> >      delete the user (to avoid race conditions where a user might
> >      log back in immediately after being kicked)
> >
>  This is a different approach. Here you try to automate the operation
>  yourself by providing the --kick flag.
>  With --force flag you leave the responsibility to kick user first to the
>  caller.
>   
I was thinking that shutting down users' processes before actually
deleting him might allow for a cleaner shutdown -- and perhaps remind
the admin that he should migrate or backup data or files of these
processes etc..

But you are probably right, this was perhaps trying to think too much
for the admin. I'll implement your proposal.

>  What you propose is better but more work.
>  I am not sure it is justified.
>  Opinions?
>    
It is really not that much work. We already have code in place to
disable the account (locking the user in sss_usermod).
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkurPKUACgkQHsardTLnvCWB0gCcDTgZWsUTUfTCpy093Zw0zBsl
> CLcAn0FF67Lxf7TvqjZd4W9jaC+bD73O
> =bn+i
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/24/2010 10:09 PM, Stephen Gallagher wrote:
>  In other words:
>  
>  On platforms that support get_uid_table():
>  If the user is not logged in:
>    1) Delete the user.
>  If the user is logged in:
>    1) Delete the user.
>    2) Print a message stating that the user (with uid N) was logged in
>       when you deleted it.
>    3) (Optional) Add a --kick flag to sss_userdel that disables the
>       user's password, then invokes 'killall -u <username>' and then
>       delete the user (to avoid race conditions where a user might
>       log back in immediately after being kicked)
>  
>  
>  On platforms that do not support get_uid_table():
>   1) Delete the user.
>   2) (Optional) Add a --kick flag to sss_userdel that disables the
>       user's password, then invokes 'killall -u <username>' and then
>       delete the user (to avoid race conditions where a user might
>       log back in immediately after being kicked) 
A patch that implements the above is attached.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkuzLh8ACgkQHsardTLnvCU8+ACdE8UXFF+DdLj8Qq4m5JrIGfoP
> oI0An3r7DB9502DFhyBMH9W8eOiNJfJC
> =UqAG
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/31/2010 07:12 AM, Jakub Hrozek wrote:
>  On 03/24/2010 10:09 PM, Stephen Gallagher wrote:
> > In other words:
>  
> > On platforms that support get_uid_table():
> > If the user is not logged in:
> >   1) Delete the user.
> > If the user is logged in:
> >   1) Delete the user.
> >   2) Print a message stating that the user (with uid N) was logged in
> >      when you deleted it.
> >   3) (Optional) Add a --kick flag to sss_userdel that disables the
> >      user's password, then invokes 'killall -u <username>' and
> then
> >      delete the user (to avoid race conditions where a user might
> >      log back in immediately after being kicked)
>  
>  
> > On platforms that do not support get_uid_table():
> >  1) Delete the user.
> >  2) (Optional) Add a --kick flag to sss_userdel that disables the
> >      user's password, then invokes 'killall -u <username>' and
> then
> >      delete the user (to avoid race conditions where a user might
> >      log back in immediately after being kicked)
>  
>  A patch that implements the above is attached. 
Nack.

The switch statement after is_logged_in() should check for ENOSYS and
print a warning that the status of the user cannot be determined on this
platform.

Why are you passing KILL_CMD as an argument to KILL_CMD? I think this is
probably a typo.

Failing to kick the user should be a survivable error. You should
probably warn loudly and proceed with the user deletion anyway.
Otherwise, you may end up in an inconsistent state (the account lock may
have worked, but the killall command failed, for example).



- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkuzpvMACgkQeiVVYja6o6OXlACfVrn6zFESKO7tD6RB+yhPGAeN
> ki0AnjdTsrHRmBkuUG8NWPFbbs70BS1A
> =V4+T
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/31/2010 09:48 PM, Stephen Gallagher wrote:
>  Nack.
>  
>  The switch statement after is_logged_in() should check for ENOSYS and
>  print a warning that the status of the user cannot be determined on this
>  platform.
>   
OK

>  Why are you passing KILL_CMD as an argument to KILL_CMD? I think this
> is
>  probably a typo.
>   
That's argv[0]

>  Failing to kick the user should be a survivable error. You should
>  probably warn loudly and proceed with the user deletion anyway.
>  Otherwise, you may end up in an inconsistent state (the account lock may
>  have worked, but the killall command failed, for example).
>   
Currently we fail and return != EOK only if the usermod transaction
fails (in which case userdel would fail anyway) or the fork() call fails
(I wasn't sure if we should proceed in this case but I think we are in
enough trouble if we cannot fork). The return code of exec or killall is
not taken into account.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku0TNoACgkQHsardTLnvCVkWwCgx2qLAjcnHS636LB2E7M5bFcQ
> elgAoOQMKgftaHmTpaJc2cQOYh8kjpFj
> =xy4Y
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/01/2010 03:35 AM, Jakub Hrozek wrote:
>  On 03/31/2010 09:48 PM, Stephen Gallagher wrote:
> > Nack.
>  
> > The switch statement after is_logged_in() should check for ENOSYS and
> > print a warning that the status of the user cannot be determined on this
> > platform.
>  
>  
>  OK 
Thanks

>  
> > Why are you passing KILL_CMD as an argument to KILL_CMD? I think this is
> > probably a typo.
>  
>  
>  That's argv[0] 
Right, I forgot exec() requires that.

>  
> > Failing to kick the user should be a survivable error. You should
> > probably warn loudly and proceed with the user deletion anyway.
> > Otherwise, you may end up in an inconsistent state (the account lock may
> > have worked, but the killall command failed, for example). 

>  
>  Currently we fail and return != EOK only if the usermod transaction
>  fails (in which case userdel would fail anyway) or the fork() call fails
>  (I wasn't sure if we should proceed in this case but I think we are in
>  enough trouble if we cannot fork). The return code of exec or killall is
>  not taken into account. 
Ok, but I still think that if the killall returns a nonzero error code,
we should still emit a warning that there may still be live processes
owned by this user.


- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku0hn4ACgkQeiVVYja6o6N7HQCgqL37+hkgyqVGxhqoTZWRyRUi
> fpkAnR27oMDjs2sKVvh7r5JDvNjdxZwn
> =ZR/c
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/01/2010 01:41 PM, Stephen Gallagher wrote:
>  Ok, but I still think that if the killall returns a nonzero error
> code,
>  we should still emit a warning that there may still be live processes
>  owned by this user.
>   
Sorry I just realized I sent the reply to Stephen only before -
resending for clarity.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku7KmsACgkQHsardTLnvCXXAQCgmQKugLssY0Zr23ZZd38ry5ni
> 28sAoIXFS3ocKcICKVwUtcdIQkb5qzIf
> =D19S
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/06/2010 08:34 AM, Jakub Hrozek wrote:
>  On 04/01/2010 01:41 PM, Stephen Gallagher wrote:
> > Ok, but I still think that if the killall returns a nonzero error code,
> > we should still emit a warning that there may still be live processes
> > owned by this user.
>  
>  
>  Sorry I just realized I sent the reply to Stephen only before -
>  resending for clarity. 

Sorry, I hate to say this, but I realized that killall returns nonzero
if there are no processes running by that user name. So this warning is
going to look wrong if we run the userdel command on a system where the
user isn't logged in. I withdraw my request for this warning.

Also, we probably need to do 'killall -9' rather than simply killall, as
SIGTERM doesn't terminate shell processes (which are the most important
ones to kill if we need to kick a user)

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku7RnIACgkQeiVVYja6o6OXYgCfRI/j/RM++xb2Y34oOIQHzNu5
> z6AAn39IzsOQAQFKoU34qoMlF6OZfxhH
> =u4kU
> -----END PGP SIGNATURE-----

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/06/2010 04:34 PM, Stephen Gallagher wrote:
>  Sorry, I hate to say this, but I realized that killall returns
> nonzero
>  if there are no processes running by that user name. So this warning is
>  going to look wrong if we run the userdel command on a system where the
>  user isn't logged in. I withdraw my request for this warning.
>  
>  Also, we probably need to do 'killall -9' rather than simply killall, as
>  SIGTERM doesn't terminate shell processes (which are the most important
>  ones to kill if we need to kick a user) 
Ah, correct. I only tested with ssh session where the kick worked fine.

Thank you for the review, a new patch is attached.

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku7W2YACgkQHsardTLnvCUBSACZATLBa5vYZqlX68uj49KouLcw
> uOgAoI7d/7VoXxtoI9Cb9hGc4/cwUCub
> =2f+g
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/06/2010 12:03 PM, Jakub Hrozek wrote:
>  On 04/06/2010 04:34 PM, Stephen Gallagher wrote:
> > Sorry, I hate to say this, but I realized that killall returns nonzero
> > if there are no processes running by that user name. So this warning is
> > going to look wrong if we run the userdel command on a system where the
> > user isn't logged in. I withdraw my request for this warning.
>  
> > Also, we probably need to do 'killall -9' rather than simply killall, as
> > SIGTERM doesn't terminate shell processes (which are the most important
> > ones to kill if we need to kick a user)
>  
>  Ah, correct. I only tested with ssh session where the kick worked fine.
>  
>  Thank you for the review, a new patch is attached.
>   

Ack

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku7fmIACgkQeiVVYja6o6MaSwCdFPnV0z0kG1A3DJbGD412jRh/
> a0wAnAs5VRwjNW6nbSl9XCNy5KYGd4NF
> =pg3r
> -----END PGP SIGNATURE-----

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/06/2010 02:33 PM, Stephen Gallagher wrote:
>  On 04/06/2010 12:03 PM, Jakub Hrozek wrote:
> > On 04/06/2010 04:34 PM, Stephen Gallagher wrote:
> >> Sorry, I hate to say this, but I realized that killall returns nonzero
> >> if there are no processes running by that user name. So this warning is
> >> going to look wrong if we run the userdel command on a system where the
> >> user isn't logged in. I withdraw my request for this warning.
>  
> >> Also, we probably need to do 'killall -9' rather than simply killall, as
> >> SIGTERM doesn't terminate shell processes (which are the most important
> >> ones to kill if we need to kick a user)
>  
> > Ah, correct. I only tested with ssh session where the kick worked fine.
>  
> > Thank you for the review, a new patch is attached.
>  
>  
>  
>  Ack
>   
Pushed to master.

- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAku7ftMACgkQeiVVYja6o6O/ZgCdGAYl17VK4d0bxtjc2vxsJ7HF
> 7LkAn1CZq2jWe3cyKmIzkL4hI8QosI6F
> =7/Dg
> -----END PGP SIGNATURE-----

[David O'Brien]

Jakub Hrozek wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 04/06/2010 04:34 PM, Stephen Gallagher wrote:
> > Sorry, I hate to say this, but I realized that killall returns nonzero
> > if there are no processes running by that user name. So this warning is
> > going to look wrong if we run the userdel command on a system where the
> > user isn't logged in. I withdraw my request for this warning.
> >
> > Also, we probably need to do 'killall -9' rather than simply killall, as
> > SIGTERM doesn't terminate shell processes (which are the most important
> > ones to kill if we need to kick a user)
>  
>  Ah, correct. I only tested with ssh session where the kick worked fine.
>  
>  Thank you for the review, a new patch is attached.
>   
I'm not much for reading patches, and while I've got the gist of this, I 
have a couple of questions:
1. What's ENOSYS?
2. From the man page:
"-f,--force
This option forces sss_userdel to remove the user´s home directory and 
mail spool, even if they are not owned by the specified user."
  Who else would they be owned by, and how would that come about, and why?
3. "Kill users' processes before removing him"  s/users'/user's/ 
:-)
4. So far I've been able to use sss_userdel -k on a logged in user and 
not see the expected warning message about the user being logged in. 
This is with multiple processes running or just logging in in a shell.

Any further updates or examples the doc needs to be aware of for other 
clients, e.g., Solaris, Windows, Mac?

thanks

-- 

David O'Brien
Red Hat Asia Pacific Pty Ltd

He who asks is a fool for five minutes, but he who does not ask remains 
a fool forever."
  ~ Chinese proverb

[Jakub Hrozek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/28/2010 05:14 AM, David O'Brien wrote:
>  I'm not much for reading patches, and while I've got the gist
> of this, I 
>  have a couple of questions:
>  1. What's ENOSYS? 
In this context, it means not implemented.

>  2. From the man page:
>  "-f,--force
>  This option forces sss_userdel to remove the user´s home directory and 
>  mail spool, even if they are not owned by the specified user."
>    Who else would they be owned by, and how would that come about, and why? 
A different UID. For example, consider this sequence:

1) sss_useradd foo (homedir and spool are created)
2) sss_userdel --no-remove foo (--no-remove can also be a system-wide
default)
3) sss_useradd foo (homedir and spool are not created, just a warning is
printed that they already exist)
4) sss_userdel --force foo (homedir and spool are removed even though
they belong to a different user)

>  3. "Kill users' processes before removing him" 
> s/users'/user's/  :-) 
Yup, manpage bug. Thanks for catching it.

>  4. So far I've been able to use sss_userdel -k on a logged in
> user and 
>  not see the expected warning message about the user being logged in. 
>  This is with multiple processes running or just logging in in a shell.
>   
The warning message is printed when deleting a user who is logged in
/without/ the -k flag. The -k flag just kicks the user out of the
system, by terminating all processes run by him - including his login shell.

>  Any further updates or examples the doc needs to be aware of for
> other 
>  clients, e.g., Solaris, Windows, Mac?
>   
So far we don't port SSSD to these platforms, but when we do, just a
note that this flag is not supported outside Linux yet.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkvX7NgACgkQHsardTLnvCXIpgCeJ2rFs2kcazt7cjD2kapM3C9F
> MlMAoOa8I4fgrhqE+tiE+TEZZf+eBoGD
> =IqW3
> -----END PGP SIGNATURE-----

[David O'Brien]

Jakub Hrozek wrote:
>  -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>  
>  On 04/28/2010 05:14 AM, David O'Brien wrote:
> > I'm not much for reading patches, and while I've got the gist of this, I 
> > have a couple of questions:
> > 1. What's ENOSYS?
>  
>  In this context, it means not implemented.
>  
> > 2. From the man page:
> > "-f,--force
> > This option forces sss_userdel to remove the user´s home directory and 
> > mail spool, even if they are not owned by the specified user."
> >   Who else would they be owned by, and how would that come about, and why?
>  
>  A different UID. For example, consider this sequence:
>  
>  1) sss_useradd foo (homedir and spool are created)
>  2) sss_userdel --no-remove foo (--no-remove can also be a system-wide
>  default)
>  3) sss_useradd foo (homedir and spool are not created, just a warning is
>  printed that they already exist)
>  4) sss_userdel --force foo (homedir and spool are removed even though
>  they belong to a different user)
>  
> > 3. "Kill users' processes before removing him" 
> s/users'/user's/  :-)
>  
>  Yup, manpage bug. Thanks for catching it.
>  
> > 4. So far I've been able to use sss_userdel -k on a logged in user and 
> > not see the expected warning message about the user being logged in. 
> > This is with multiple processes running or just logging in in a shell.
> >
>  
>  The warning message is printed when deleting a user who is logged in
>  /without/ the -k flag. The -k flag just kicks the user out of the
>  system, by terminating all processes run by him - including his login shell.
>  
> > Any further updates or examples the doc needs to be aware of for other 
> > clients, e.g., Solaris, Windows, Mac?
> >
>  
>  So far we don't port SSSD to these platforms, but when we do, just a
>  note that this flag is not supported outside Linux yet. 
ok, all that makes sense. I misread how the -k flag works.

thanks!

-- 

David O'Brien
Red Hat Asia Pacific Pty Ltd

He who asks is a fool for five minutes, but he who does not ask remains 
a fool forever."
  ~ Chinese proverb