userdel: --force: Allow the flag to be specified twice, for more granularity #1071
Conversation
alejandro-colomar
force-pushed
the
ff
branch
3 times, most recently
from
11c0e8b
to
cb58058
Compare
| If specified once, | ||
| a user is removed | ||
| even if it's still logged in, | ||
| and its primary group is removed | ||
| even if it's the primary group of another user. | ||
| If specified twice, | ||
| it skips all safety checks. |
There was a problem hiding this comment.
Does this distinction make sense? Should the group thing go with -ff?
There was a problem hiding this comment.
I think it's ok, but I ask just in case.
|
@wolfsage, can you please check if this looks good to you? Also, would you please test it? |
|
I don't like this change and I think it'll be problematic to some users. |
Yep, we have problems either way. This is half broken, so users will complain about it being quite unsafe. Yet, scripts may rely on that brokenness, so fixing it may break scripts. I'm in favour of fixing it and potentially breaking some random scripts. Any other opinions? |
|
From my perspective, the |
There's prior art in other software for this schema of asking for double This is for example from git-clean(1): The only thing that might be a problem is existing scripts that use this (or interactive users that might not read the new docs if they remember well the old docs; but if I were them, I'd probably re-read the docs just in case). But for interactive use, or for new scripts, I don't think requiring the option twice would be problematic. |
|
Alex, thank you for continuing to try to make this better. Please don't make any changes yet based on what I say here, I'm just thinking aloud :) First, there's a difference between "home is not owned by the user" and "home is / and we're all gonna die!". Shadow is not that "generic" of a package. I think it would be ok to add a list of "do not touch" paths, and always refuse to delete those no matter what: /, /usr, /etc, and /var at least. I suppose we could even make the list configurable through /etc/login.defs, so distros can add their own entries. Then, for the "home not owned by user" case, we just need to draw line between not breaking any current users and complicating code. The main options are: 1. change nothing (which if we implement the never-delete list above becomes far less scary), 2. add a default-off DELETE_UNOWNED_HOME option to login.defs, 3. add -ff like you have, and 4. both 2 and 3. |
How about refusing to add users with such homedirs in the first place? (Just thinking aloud too.) |
I tend to dislike configurability of programs. It adds complexity to both sysadmins, and the program itself. I think I would prefer a hardcoded list of directories that should not be removed. |
The problem with that is that a mistaken or malicious /etc/passwd edit could easily change it afterwards. |
I think What's your opinion about breaking existing scripts (or hardcoded brains)? I don't know how much people would rely on |
Well actually, I would imagine there are a lot of places that have end-of-semester or whatever scripts that nuke the out-going students' accounts. For those, having -f return error and -ff succeed would be a problem - if they ever have a situation like this. It might be better if -f just skipped the deletion and returned success. But that has its own problems. |
But I expect such user accounts to either have an owned dir, or have something like /nonexistent that shouldn't be removed. In what case would one have a directory that isn't owned but they want to remove it anyway? |
alejandro-colomar
force-pushed
the
ff
branch
2 times, most recently
from
72af7d3
to
372fdbe
Compare
That makes sense, yeah. So system accounts pointing at some /var/lib/something owned by another system user would be more likely. But, I was thinking based on the discussion that this patch would make it so that whereas @ikerexxe do you still feel uncomfortable with this patch? |
…ularity Previously, with a single --force, one could only chose between not forcing, and forcing (which could destroy one's system entirely). Now one can pass --force, with only a small danger, to skip some checks, and a double --force --force with the meaning of the previous --force, which will possibly destroy the entire system. Closes: <shadow-maint#1050> Reported-by: Matthew Horsfall <[email protected]> Cc: Lennart Poettering <[email protected]> Cc: Serge Hallyn <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>
|
It seems counter-intuitive to me, but I won't object if you think it's the best solution |
Previously, with a single --force, one could only chose between not forcing, and forcing (which could destroy one's system entirely).
Now one can pass --force, with only a small danger, to skip some checks, and a double --force --force with the meaning of the previous --force, which will possibly destroy the entire system.
Closes: #1050
Reported-by: @wolfsage
Cc: @poettering
Cc: @hallyn
I have NOT tested this. Please test.
Revisions:
v2
v2b
v2c
v2d
v2e