Skip to content

Commit

Permalink
resolves the correct primary gid for logins that share a name with a (#…
Browse files Browse the repository at this point in the history
…46639)

group in host_groups
  • Loading branch information
eriktate authored Sep 16, 2024
1 parent fc2e357 commit d32531d
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 8 deletions.
2 changes: 1 addition & 1 deletion integration/hostuser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func TestRootHostUsers(t *testing.T) {

t.Cleanup(func() {
os.Remove(sudoersPath(testuser, uuid))
host.UserDel(testuser)
cleanupUsersAndGroups([]string{testuser}, nil)
})
closer, err := users.UpsertUser(testuser,
services.HostUsersInfo{
Expand Down
37 changes: 30 additions & 7 deletions lib/srv/usermgmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,32 @@ func (u *HostUserManagement) updateUser(name string, ui services.HostUsersInfo)
return nil
}

func (u *HostUserManagement) resolveGID(username string, groups []string, gid string) (string, error) {
if gid != "" {
// ensure user's primary group exists if a gid is explicitly provided
err := u.backend.CreateGroup(username, gid)
if err != nil && !trace.IsAlreadyExists(err) {
return "", trace.Wrap(err)
}

return gid, nil
}

// user's without an explicit gid should use the group that shares their login
// name if defined, otherwise user creation will fail due to their primary group
// already existing
if slices.Contains(groups, username) {
return username, nil
}

// avoid automatic assignment of groups not defined in the role
if _, err := u.backend.LookupGroup(username); err == nil {
return "", trace.AlreadyExists("host login %q conflicts with an existing group that is not defined in user's role, either add %q to host_groups or explicitly assign a GID", username, username)
}

return "", nil
}

func (u *HostUserManagement) createUser(name string, ui services.HostUsersInfo) error {
var home string
var err error
Expand All @@ -323,15 +349,12 @@ func (u *HostUserManagement) createUser(name string, ui services.HostUsersInfo)
}
}

if ui.GID != "" {
// if gid is specified a group must already exist
err := u.backend.CreateGroup(name, ui.GID)
if err != nil && !trace.IsAlreadyExists(err) {
return trace.Wrap(err)
}
gid, err := u.resolveGID(name, ui.Groups, ui.GID)
if err != nil {
return trace.Wrap(err)
}

err = u.backend.CreateUser(name, ui.Groups, home, ui.UID, ui.GID)
err = u.backend.CreateUser(name, ui.Groups, home, ui.UID, gid)
if err != nil && !trace.IsAlreadyExists(err) {
return trace.WrapWithMessage(err, "error while creating user")
}
Expand Down
50 changes: 50 additions & 0 deletions lib/srv/usermgmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,3 +653,53 @@ func Test_AllowExplicitlyManageExistingUsers(t *testing.T) {
assert.NotContains(t, backend.users["bob"], types.TeleportKeepGroup)

}

func TestCreateUserWithExistingPrimaryGroup(t *testing.T) {
t.Parallel()
backend := newTestUserMgmt()
bk, err := memory.New(memory.Config{})
require.NoError(t, err)
pres := local.NewPresenceService(bk)
users := HostUserManagement{
backend: backend,
storage: pres,
}

existingGroups := []string{"alice", "simon"}
for _, group := range existingGroups {
require.NoError(t, backend.CreateGroup(group, ""))
}

userinfo := services.HostUsersInfo{
Groups: []string{},
Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP,
}

// create a user without an existing primary group
closer, err := users.UpsertUser("bob", userinfo)
assert.NoError(t, err)
assert.NotEqual(t, nil, closer)
assert.Zero(t, backend.setUserGroupsCalls)

// create a user with primary group defined in userinfo.Groups, but not yet on the host
userinfo.Groups = []string{"fred"}
closer, err = users.UpsertUser("fred", userinfo)
assert.NoError(t, err)
assert.NotEqual(t, nil, closer)
assert.Zero(t, backend.setUserGroupsCalls)

// create a user with primary group defined in userinfo.Groups that already exists on the host
userinfo.Groups = []string{"alice"}
closer, err = users.UpsertUser("alice", userinfo)
assert.NoError(t, err)
assert.NotEqual(t, nil, closer)
assert.Zero(t, backend.setUserGroupsCalls)

// create a user with primary group that already exists on the host but is not defined in userinfo.Groups
userinfo.Groups = []string{""}
closer, err = users.UpsertUser("simon", userinfo)
assert.True(t, trace.IsAlreadyExists(err))
assert.Contains(t, err.Error(), "conflicts with an existing group")
assert.Equal(t, nil, closer)
assert.Zero(t, backend.setUserGroupsCalls)
}
8 changes: 8 additions & 0 deletions lib/utils/host/hostusers.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"errors"
"os/exec"
"slices"
"strings"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -76,6 +77,13 @@ func UserAdd(username string, groups []string, home, uid, gid string) (exitCode
return -1, trace.BadParameter("home is a required parameter")
}

// user's without an explicit gid should be added to the group that shares their
// login name if it's defined, otherwise user creation will fail because their primary
// group already exists
if slices.Contains(groups, username) && gid == "" {
gid = username
}

// useradd ---no-create-home (username) (groups)...
args := []string{"--no-create-home", "--home-dir", home, username}
if len(groups) != 0 {
Expand Down

0 comments on commit d32531d

Please sign in to comment.