Skip to content

Overhaul user management #1017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dlubawy
Copy link
Contributor

@dlubawy dlubawy commented Jul 25, 2024

This PR is related to issues #554 and #96. I've opened this more as discussion around possibilities rather than as a request to actually merge the code. There are a few problems I see with nix-darwin around user management. Using numbers for reference later, these problems are:

  1. No options to configure users as admin, system, or token users
    • New Macs and Nix installs use FileVault. dscl user management does not work for this as new users created will not be able to use FDE and so will be unable to login on reboot
    • dscl is an older (maybe arguably deprecated) tool for creating new users. New users created this way will lack specific metadata that prevents certain user management actions in System Settings such as profile picture selection and other odd behavior
    • dscl requires careful tracking of reserved UIDs/GIDs which can change between OS upgrades or other operations
  2. Missing options to set passwords
  3. Configured users/groups need an additional config option knownUsers/knownGroups in order to actually be managed by nix-darwin
    • This adds code duplication and a layer of confusing abstraction that diverges from NixOS; "If a user is configured isn't it already known?".
    • "Used to indicate what users are safe to create/delete based on the configuration. "; There is no mention if this option can do other things to those configured+unknown users/groups
    • Makes for a strange workflow that mixes mutable and immutable user management schemes (very different from NixOS)

The edits I made tackled these problems in the following manner (each numbered point corresponding to the problem with the same number):

  1. Add these as configuration options; move from dscl to sysadminctl commands
    • isNormalUser: normal user according to macOS (i.e. UID > 500)
    • isAdminUser: adds the user to the admin group
    • isSystemUser: configure system user according to macOS (i.e. 200 <= UID <= 400)
    • isTokenUser: configure the user for FDE by having an admin with a secure token add a token to this user
  2. Add these as configuration options; use sysadminctl to perform user changes
    • password: behaves like NixOS option; password is only changes when users are immutable
    • initialPassword: equivalent to password when users are immutable; otherwise sets the initial password when a user is first created
  3. Remove knownUsers/knownGroups in favor of NixOS users.mutableUsers flag
    • Adds a NixDeclarative attribute with dscl to track which users/groups are managed
    • If users are mutable then nothing should change, otherwise users with the NixDeclarative attribute are modified according to configs

This isn't a perfect approach to things as isTokenUser needs an admin user with a secure token to be able to enter their password as well as the user requiring the change to do the same, but in combination with the mutableUsers and password options this could be mitigated on new system builds by pulling the password from the config.

It's my intention with these changes to have a system whereby I can more easily manage an admin user (which runs nix-darwin and manages homebrew through it) and a normal user (for daily use) that can run sudo -Hu admin darwin-rebuild (because running as root is broken right now). This is typically how systems should be configured in macOS where running admin as a daily user goes against best practices according to Apple. I've been using this code in my personal system configuration and it has achieved that for me. Hopefully, this can be of use to others as well.

@emilazy
Copy link
Member

emilazy commented Jul 26, 2024

Thanks for this. Your changes absolutely seem directionally correct and I think we’re basically in complete agreement on what the ideal end‐state for nix-darwin looks like.

However, there are considerations to make in terms of the migration path for the inevitable breaking changes it would involve, and the best way to move things over smoothly without disrupting users’ setups multiple times along the way. I’ve been meaning to start tackling this for a long while now and it’s on me for not having written down my thoughts and plans in details, so I’m sorry about that! I really appreciate you taking the initiative on this work and it would be great to work with you on gradually moving towards a setup where nix-darwin exclusively manages the system, darwin-rebuild is always run as root, multiple‐user setups work in a reasonable way, we don’t duplicate work that better belongs in Home Manager, and so on.

I have a lot on my stack right now and my availability isn’t perfect, but if I don’t get around to replying again with my blueprint for what I think we should do here in a couple days please feel free to ping me.

@dlubawy
Copy link
Contributor Author

dlubawy commented Aug 5, 2024

@emilazy if you can manage it, I would love to hear the blueprint and maybe help out to achieve a better experience in this space. No worries if you are still too busy.

@emilazy
Copy link
Member

emilazy commented Aug 10, 2024

Sorry for the delayed reply; I will try to get back to this ASAP.

@emilazy
Copy link
Member

emilazy commented Aug 13, 2024

I’ve replied in #1016 instead as it is more directly related to what I talk about there. This PR actually seems like it probably has no conflicts with any of that work and might just be totally fine as‐is; thank you for doing the work! I’ll have to give it a proper review some other time, though, since I just spent like five years writing up the big plan :)

@nix-darwin nix-darwin deleted a comment Aug 19, 2024
@dlubawy
Copy link
Contributor Author

dlubawy commented Aug 25, 2024

I responded in #1016 and closed it as a PR. That was a great comment, and I'm looking forward to those changes!

Back on this PR. This is definitely a separate topic, but it is in the same spirit of improving multi-user support on nix-darwin. I would greatly appreciate a good review on the changes and ideas I propose here. Ultimately my intention is to bring some parity to user management between nix-darwin and NixOS. I think I was pretty heavy handed in some aspects here so a review by someone more seasoned in nix-darwin is highly valuable, especially to determine better approaches for backwards compatibility. I would also greatly appreciate some feedback on the ideas I put out around the notion of admins, token users, and removal of knownUsers.

@emilazy
Copy link
Member

emilazy commented Sep 4, 2024

Sorry for the delay; I’ve been busy lately but I’ll try to get around to reviewing this soon because we are going to need to do stuff with user management within the next couple weeks to handle the _nixbld* UID clashes in the upcoming macOS Sequoia upgrade.

Do you know if this will handle, or could easily be made to handle, changing the UID and GID of existing users and groups? I am hoping we can effectively implement NixOS/nix#11075 just by changing the IDs we assign for those users.

@Enzime
Copy link
Member

Enzime commented Oct 17, 2024

I’ve taken a brief look at the changes in this PR, and I’m definitely interested in getting these changes merged.

I think that separating these changes into multiple PRs will make them easier to review and not block some changes on other parts.

Here’s how think we should split up these PRs:

  • Switching from dscl to sysadminctl
    • Making uid optional
    • Replace gid with group (defaulting to staff) to more closely match NixOS and not depend as much on the system state
    • Instead of having an isAdminUser, we could specify it using group = "admin"
  • Adding options for configuring password
    • Add passwordFile and hashedPassword{,File} options for allowing users to configure passwords without storing them in plaintext in their configuration
  • Adding options for configuring secure tokens
    • maybe we should rename isTokenUser to canUnlockFDE as currently isNormalUser and isSystemUser are mutually exclusive and it might be confusing to add another option which isn’t
  • Refactoring around users.knownUsers
    • This is the bulk of the code in the PR I didn’t review, due to the complexity and how little testing we have around this section

@emilazy what do you think?

@dlubawy are you still interested on working on this PR? I may start tackling the first split off PR listed above

@emilazy
Copy link
Member

emilazy commented Oct 17, 2024

Sounds like you reviewed this in more depth than I did 😅

I think the knownUsers stuff seemed like the right way forward to me, but if it’s possible to do the rest without significantly breaking compatibility then splitting it up makes perfect sense to me given the current state of things.

I’m currently focused on seeing the tail end of the Nixpkgs 24.11 release cycle through so we can get the huge Darwin SDK improvements shipped and there is some important follow‐up work due for immediately after but I’m hoping to be able to dedicate some time to starting to put The Plan™ into action before the end of the year. In the meantime I’m happy to review the split PRs as suggested.

@dlubawy
Copy link
Contributor Author

dlubawy commented Oct 28, 2024

@Enzime, sorry for the delay. Thank you for picking up #1108. It's great to see some progress on this. My free time has dropped significantly since I started this PR (went from unemployed to employed so lost my 8 hours of free project time during weekdays). I can still contribute though (just more slowly than before).

I think the options you laid out are a great idea, and I can pickup working on the additional password option for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants