Skip to content
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

nixos/users-groups: chown home on createHome #23623

Merged
merged 1 commit into from Mar 14, 2017

Conversation

fpletz
Copy link
Member

@fpletz fpletz commented Mar 7, 2017

Motivation for this change

Fixes #23619. This should work as documented and has valid use cases.

We should fix this in both 16.09 and 17.03.

Discussion

Should we also chmod(700) if the directory exists? In the docs it specifically says only the owner and group of the directory are modified but this could also be useful. Or should we expose an option to specify the mode?

It's a bit unfortunate that the semantics of createHome doesn't really match its name. I think manageHome would be more fitting but not sure if the breakage is worth it. This should obviously not make it into 17.03.

cc @arianvp @globin

@mention-bot
Copy link

@fpletz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @ruediger and @bjornfor to be potential reviewers.

@joachifm
Copy link
Contributor

joachifm commented Mar 8, 2017

Makes sense given the docs. Leaving the docs aside, I'd expect this beahvior if mutableUsers = false but perhaps not otherwise.

@arianvp
Copy link
Member

arianvp commented Mar 8, 2017

I'd say changing to 700 is useful . However the docs currently don't describe this so it wouldn't be neccesery to be docs complete. If we do add it, we should update the docs.

@arianvp
Copy link
Member

arianvp commented Mar 13, 2017

LGTM?

@fpletz fpletz merged commit 91744f3 into NixOS:master Mar 14, 2017
@fpletz fpletz deleted the fix/users-create-home branch March 14, 2017 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

createHome does not set permissions
5 participants