Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
osutil: more create-user fixes #1651
Conversation
mwhudson
added some commits
Aug 9, 2016
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
whitelist this please |
mvo5
reviewed
Aug 9, 2016
| @@ -66,6 +70,11 @@ func AddExtraSudoUser(name string, sshKeys []string, gecos string) error { | ||
| return fmt.Errorf("cannot write %s: %s", authKeys, err) | ||
| } | ||
| + cmd = exec.Command("chown", "-R", u.Uid+":"+u.Gid, sshDir) |
mvo5
reviewed
Aug 9, 2016
| @@ -46,12 +46,16 @@ func AddExtraSudoUser(name string, sshKeys []string, gecos string) error { | ||
| "--gecos", gecos, | ||
| "--extrausers", | ||
| "--disabled-password", | ||
| - "--add_extra_groups", "sudo", |
|
|
|
add to whitelist |
|
The test is failing because adduser --extrausers $user $group doesn't actually take any notice of the --extrausers flag, and I don't see an obvious way of doing that command. The alternative is to drop a file in /etc/sudoers.d I guess... |
niemeyer
reviewed
Aug 9, 2016
| name) | ||
| if output, err := cmd.CombinedOutput(); err != nil { | ||
| return fmt.Errorf("adduser failed with %s: %s", err, output) | ||
| } | ||
| + cmd = exec.Command("adduser", "--extrausers", name, "sudo") |
|
I think it's possible that libnss-extrausers just doesn't support supplementary groups. But I'm hardly sure of that at this point. |
|
Tests are failing because the code has --extrausers and the mock doesn't. Can we have a spread tests for this as well? Given the amount of blind fiddling with options going on there it'd be nice to have an actual test that ensure this is working. |
|
There is an integration test for this, but I guess a spread test would be better. But I can't make this work by hand locally so I don't know what to put in the code at this point... |
|
What's the issue? |
|
Or rather, what specifically is not working? |
niemeyer
changed the title from
More create-user fixes
to
osutil: more create-user fixes
Aug 9, 2016
|
Ah (on stopping to think a bit about how the files work), the issue is that there is no way for an user only present in /var/lib/extrausers to be added to a group that's present in /etc/group without editing the /etc/group file, which we can't do. So I think the only thing we can do is dropping a file in /etc/sudoers.d. |
mwhudson
added some commits
Aug 9, 2016
niemeyer
reviewed
Aug 9, 2016
| name) | ||
| if output, err := cmd.CombinedOutput(); err != nil { | ||
| return fmt.Errorf("adduser failed with %s: %s", err, output) | ||
| } | ||
| + sudoersFile := filepath.Join(sudoersDotD, "create-user-"+name) |
niemeyer
Aug 9, 2016
Contributor
# This will cause sudo to read and parse any files in the /etc/sudoers.d
# directory that do not end in '~' or contain a '.' character.
mwhudson
Aug 10, 2016
Contributor
Ah good point. ~ is not permitted anyway but . is. I could replace . with some other character but it would have to not be one of the permitted characters or we could get collisions. : or , or something I guess...
niemeyer
reviewed
Aug 9, 2016
| name) | ||
| if output, err := cmd.CombinedOutput(); err != nil { | ||
| return fmt.Errorf("adduser failed with %s: %s", err, output) | ||
| } | ||
| + sudoersFile := filepath.Join(sudoersDotD, "create-user-"+name) | ||
| + if err := AtomicWriteFile(sudoersFile, []byte(fmt.Sprintf(sudoersTemplate, name)), 0400, 0); err != nil { | ||
| + return fmt.Errorf("creating sudoers fragment failed with %s", err) |
|
LGTM with these details fixed. |
niemeyer
added
the
Reviewed
label
Aug 9, 2016
mwhudson
added some commits
Aug 10, 2016
mwhudson
referenced this pull request
Aug 10, 2016
Merged
client, cmd, daemon, osutil: support --yaml and --sudoer flags for create-user #1662
niemeyer
reviewed
Aug 10, 2016
| @@ -60,30 +60,35 @@ func AddExtraSudoUser(name string, sshKeys []string, gecos string) error { | ||
| return fmt.Errorf("adduser failed with %s: %s", err, output) | ||
| } | ||
| - sudoersFile := filepath.Join(sudoersDotD, "create-user-"+name) | ||
| + sudoersFile := filepath.Join(sudoersDotD, "create-user-"+strings.Replace(name, ".", ",", -1)) |
niemeyer
Aug 10, 2016
•
Edited 1 time
-
niemeyer
Aug 10, 2016
Contributor
-
Aug 10, 2016
Comma reads incorrectly for me. It feels like two independent things, rather than an escaping sequence.
Let's please use %2E to escape the dot, which should work fine since % is disallowed too.
This also gives us a good pattern to fix further cases, if we find them.
niemeyer
Aug 10, 2016
Contributor
s/2e/2E/ too.. both should work, but the uppercasing creates a nice visual distinction since we're only allowing lowercase letters in the username.
mwhudson commentedAug 9, 2016
Plans meet reality etc.