shadow-maint / shadow

Upstream shadow tree
Other
290 stars 228 forks source link

useradd --system returns error when user exists #982

Closed stsp closed 3 months ago

stsp commented 3 months ago

According to this: https://wiki.debian.org/AccountHandlingInMaintainerScripts

It should normally not be necessary to cross-check with getent whether an account already exists since adduser --system
generally does the right thing. If not, please report a bug against adduser to keep your maintainer scripts simple.

Which to me sounds like useradd should not return an error when the user already exist and --system is given. In practice it returns an error.

I think the best fix would be to do a few checks:

... and in that case do not return an error status. But if either of the above is false, then return error.

ikerexxe commented 3 months ago

To the best of my knowledge, if a user exists then useradd/adduser fail. It's up to the user to decide if that failure makes sense or not, and if they want to proceed in some other way.

In addition, what does the right thing mean in this case? It's kind of ambiguous...

stsp commented 3 months ago

"right thing" means that you put no extra code. They state this clearly: "If not, please report a bug against adduser to keep your maintainer scripts simple" which means no extra checks in the script.

There is another reason why I want this change: if you add a user inside a makefile (upon "sudo make install" stage), then the extra checks are problematic, too, as make processes the recipes invoking the shell per every line, and each failure is treated as a recipe failure (yes, its possible to override, but still).

On top of that, I find this logical: system user doesn't have any files (no home dir), and therefore 2 system users with the same name and uid are identical. Returning failure makes sense with regular users: if you failed to add a new one, you may want to delete the old one, delete (or leave) its home dir, and then add a new one. Or maybe just add a new one, but with an altered name. This is understandable. But for system users this all is simply irrelevant, so if the system user is already there, you don't need to care at all. You don't need to alter its name and add again, because the names of system users are fixed, and the clashes should never happen. This is why I find that feature logical for system users. Note that I do not propose to bluntly suppress an error code, no. The checks must be done to make sure the pre-existing user actually has a system-range uids and no home.

It's up to the user to decide if that failure makes sense or not

This is still possible by capturing the command's output. If the output is not empty, then the user existed.

And in case you firmly disagree, there is always an option to add a new option for this feature.

ikerexxe commented 3 months ago

If I'm being honest I don't know how to understand that "requirement" even after your clarification. In any case, that's an external project and the requirements should be written in this project, usually in the documentation (i.e. man pages), but I don't see anything like that in them.

That said, I do not have a strong opinion either way, so I will leave it to other maintainers to state their preferences.

hallyn commented 3 months ago

You are confusing adduser and useradd. adduser --system in debian does not exit error if the user exists:

root@debian12:~# adduser --system dummy2
Adding system user `dummy2' (UID 101) ...
Adding new user `dummy2' (UID 101) with group `nogroup' ...
Not creating `/nonexistent'.
root@debian12:~# adduser --system dummy2
The system user `dummy2' already exists. Exiting.
root@debian12:~# echo $?
0
stsp commented 3 months ago

But on fedora, adduser is a symlink to useradd. Why creating such a divergency? If debian's adduser does the right thing, why not to do the same in useradd?