nixos-install: don't prompt for passwords when mutableUsers = false #104343
Conversation
| @@ -200,7 +200,7 @@ fi | |||
| # Ask the user to set a root password, but only if the passwd command | |||
| # exists (i.e. when mutable user accounts are enabled). | |||
| if [[ -z $noRootPasswd ]] && [ -t 0 ]; then | |||
| if nixos-enter --root "$mountPoint" -c 'test -e /nix/var/nix/profiles/system/sw/bin/passwd'; then | |||
| if jq -e '.mutableUsers' "$system/users-groups.json" > /dev/null; then | |||
There was a problem hiding this comment.
this should also have a fallback to cover when the json file doesnt exist, for users installing an old nixpkgs with a new nixos-install
There was a problem hiding this comment.
Great point, thanks Clever. I made the logic here a bit more robust against that.
|
87,640 PRs / issues later :) I just tried this patch on a fresh install on my new laptop, and it worked a treat. |
| @@ -200,7 +200,7 @@ fi | |||
| # Ask the user to set a root password, but only if the passwd command | |||
| # exists (i.e. when mutable user accounts are enabled). | |||
| if [[ -z $noRootPasswd ]] && [ -t 0 ]; then | |||
| if nixos-enter --root "$mountPoint" -c 'test -e /nix/var/nix/profiles/system/sw/bin/passwd'; then | |||
| if jq -e '.mutableUsers' "$system/users-groups.json" > /dev/null; then | |||
| set +e | |||
| nixos-enter --root "$mountPoint" -c 'echo "setting root password..." && /nix/var/nix/profiles/system/sw/bin/passwd' | |||
There was a problem hiding this comment.
We might as well ask to set passwords for all isNormalUser users in users-groups.json without initialHashedPassword, hashedPassword, initialPassword, password or passwordFile set.
There was a problem hiding this comment.
I don't think that is in scope for this PR. Didn't the installer used to do that, and are we sure it doesn't still?
There was a problem hiding this comment.
It only asks password for root. You have to log in as root on tty to set user password after installation.
There was a problem hiding this comment.
That should probably be a feature ticket and second PR, since it is adding new behavior -- and not necessarily trivial behavior, either, since:
- a reasonable implementation might should also work with subsequently added users, after installation
- it should work in a headless environment without a tty
This fixes a regression reported in NixOS#95778, caused by NixOS#16703.
grahamc
force-pushed
the
nixos-install-mutable-users
branch
from
b26835e
to
0b36e65
Compare
There was a problem hiding this comment.
This looks fine to me. I'd love if the commit message actually had a sentence about the motivation. The references are nice but having another piece of information there could be helpful while "digging through the archives".
I am uneasy approving it as I haven't tested it and apparently we have zero tests that verify that the password prompt during installation actually works. If it isn't too much to ask could you add that bit to one (some?) of the tests?
|
@andir I'd be happy to! I'm not super sure how to go about writing that test, but I'll take a look. Thanks. |
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
This fixes a regression reported in #95778, caused by #16703.
Motivation for this change
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)