[v15] Fix file uploads/downloads when user doesn't have a home directory

integration/hostuser_test.go

@@ -61,22 +61,26 @@ func TestRootHostUsersBackend(t *testing.T) {
 		SudoersPath: sudoersTestDir,
 		HostUUID:    "hostuuid",
 	}
-	t.Cleanup(func() {
-		// cleanup users if they got left behind due to a failing test
-		host.UserDel(testuser)
-		cmd := exec.Command("groupdel", testgroup)
-		cmd.Run()
-	})
 
 	t.Run("Test CreateGroup", func(t *testing.T) {
+		t.Cleanup(cleanupUsersAndGroups(nil, []string{testgroup}))
+
 		err := usersbk.CreateGroup(testgroup, "")
 		require.NoError(t, err)
 		err = usersbk.CreateGroup(testgroup, "")
 		require.True(t, trace.IsAlreadyExists(err))
+
+		_, err = usersbk.LookupGroup(testgroup)
+		require.NoError(t, err)
 	})
 
 	t.Run("Test CreateUser and group", func(t *testing.T) {
-		err := usersbk.CreateUser(testuser, []string{testgroup}, "", "")
+		t.Cleanup(cleanupUsersAndGroups([]string{testuser}, []string{testgroup}))
+		err := usersbk.CreateGroup(testgroup, "")
+		require.NoError(t, err)
+
+		testHome := filepath.Join("/home", testuser)
+		err = usersbk.CreateUser(testuser, []string{testgroup}, testHome, "", "")
 		require.NoError(t, err)
 
 		tuser, err := usersbk.Lookup(testuser)
@@ -89,17 +93,26 @@ func TestRootHostUsersBackend(t *testing.T) {
 		require.NoError(t, err)
 		require.Contains(t, tuserGids, group.Gid)
 
-		err = usersbk.CreateUser(testuser, []string{}, "", "")
+		err = usersbk.CreateUser(testuser, []string{}, testHome, "", "")
 		require.True(t, trace.IsAlreadyExists(err))
 
-		require.NoFileExists(t, filepath.Join("/home", testuser))
-		err = usersbk.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid)
+		require.NoFileExists(t, testHome)
+		err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid)
 		require.NoError(t, err)
-		require.FileExists(t, filepath.Join("/home", testuser, ".bashrc"))
+		t.Cleanup(func() {
+			os.RemoveAll(testHome)
+		})
+		require.FileExists(t, filepath.Join(testHome, ".bashrc"))
 	})
 
 	t.Run("Test DeleteUser", func(t *testing.T) {
-		err := usersbk.DeleteUser(testuser)
+		t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil))
+		err := usersbk.CreateUser(testuser, nil, "", "", "")
+		require.NoError(t, err)
+		_, err = usersbk.Lookup(testuser)
+		require.NoError(t, err)
+
+		err = usersbk.DeleteUser(testuser)
 		require.NoError(t, err)
 
 		_, err = user.Lookup(testuser)
@@ -108,13 +121,10 @@ func TestRootHostUsersBackend(t *testing.T) {
 
 	t.Run("Test GetAllUsers", func(t *testing.T) {
 		checkUsers := []string{"teleport-usera", "teleport-userb", "teleport-userc"}
-		t.Cleanup(func() {
-			for _, u := range checkUsers {
-				usersbk.DeleteUser(u)
-			}
-		})
+		t.Cleanup(cleanupUsersAndGroups(checkUsers, nil))
+
 		for _, u := range checkUsers {
-			err := usersbk.CreateUser(u, []string{}, "", "")
+			err := usersbk.CreateUser(u, []string{}, "", "", "")
 			require.NoError(t, err)
 		}
 
@@ -143,7 +153,8 @@ func TestRootHostUsersBackend(t *testing.T) {
 	})
 
 	t.Run("Test CreateHomeDirectory does not follow symlinks", func(t *testing.T) {
-		err := usersbk.CreateUser(testuser, []string{testgroup}, "", "")
+		t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil))
+		err := usersbk.CreateUser(testuser, nil, "", "", "")
 		require.NoError(t, err)
 
 		tuser, err := usersbk.Lookup(testuser)
@@ -154,14 +165,15 @@ func TestRootHostUsersBackend(t *testing.T) {
 		bashrcPath := filepath.Join("/home", testuser, ".bashrc")
 		require.NoFileExists(t, bashrcPath)
 
-		require.NoError(t, os.MkdirAll(filepath.Join("/home", testuser), 0o700))
+		testHome := filepath.Join("/home", testuser)
+		require.NoError(t, os.MkdirAll(testHome, 0o700))
 
 		require.NoError(t, os.Symlink("/tmp/ignoreme", bashrcPath))
 		require.NoFileExists(t, "/tmp/ignoreme")
 
-		err = usersbk.CreateHomeDirectory(testuser, tuser.Uid, tuser.Gid)
+		err = usersbk.CreateHomeDirectory(testHome, tuser.Uid, tuser.Gid)
 		t.Cleanup(func() {
-			os.RemoveAll(filepath.Join("/home", testuser))
+			os.RemoveAll(testHome)
 		})
 		require.NoError(t, err)
 		require.NoFileExists(t, "/tmp/ignoreme")
@@ -216,7 +228,7 @@ func TestRootHostUsers(t *testing.T) {
 		u, err := user.Lookup(testuser)
 		require.NoError(t, err)
 		requireUserInGroups(t, u, testGroups)
-		require.NoDirExists(t, u.HomeDir)
+		require.Equal(t, string(os.PathSeparator), u.HomeDir)
 
 		require.NoError(t, closer.Close())
 		_, err = user.Lookup(testuser)
@@ -256,6 +268,25 @@ func TestRootHostUsers(t *testing.T) {
 		require.Equal(t, err, user.UnknownUserError(testuser))
 	})
 
+	t.Run("test create permanent user", func(t *testing.T) {
+		users := srv.NewHostUsers(context.Background(), presence, "host_uuid")
+		expectedHome := filepath.Join("/home", testuser)
+		require.NoDirExists(t, expectedHome)
+
+		closer, err := users.CreateUser(testuser, &services.HostUsersInfo{Mode: types.CreateHostUserMode_HOST_USER_MODE_KEEP})
+		require.NoError(t, err)
+		require.Nil(t, closer)
+		t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil))
+
+		u, err := user.Lookup(testuser)
+		require.NoError(t, err)
+		require.Equal(t, expectedHome, u.HomeDir)
+		require.DirExists(t, expectedHome)
+		t.Cleanup(func() {
+			os.RemoveAll(expectedHome)
+		})
+	})
+
 	t.Run("test create sudoers enabled users", func(t *testing.T) {
 		if _, err := exec.LookPath("visudo"); err != nil {
 			t.Skip("Visudo not found on path")

lib/srv/usermgmt.go

@@ -99,11 +99,11 @@ type HostUsersBackend interface {
 	// CreateGroup creates a group on a host.
 	CreateGroup(group string, gid string) error
 	// CreateUser creates a user on a host.
-	CreateUser(name string, groups []string, uid, gid string) error
+	CreateUser(name string, groups []string, home, uid, gid string) error
 	// DeleteUser deletes a user from a host.
 	DeleteUser(name string) error
 	// CreateHomeDirectory creates the users home directory and copies in /etc/skel
-	CreateHomeDirectory(user string, uid, gid string) error
+	CreateHomeDirectory(userHome string, uid, gid string) error
 }
 
 type userCloser struct {
@@ -298,6 +298,14 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo)
 		return nil, trace.WrapWithMessage(err, "error while creating groups")
 	}
 
+	var home string
+	if ui.Mode != types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP {
+		home, err = readDefaultHome(name)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+	}
+
 	err = u.doWithUserLock(func(_ types.SemaphoreLease) error {
 		if err := u.storage.UpsertHostUserInteractionTime(u.ctx, name, time.Now()); err != nil {
 			return trace.Wrap(err)
@@ -310,7 +318,7 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo)
 			}
 		}
 
-		err = u.backend.CreateUser(name, groups, ui.UID, ui.GID)
+		err = u.backend.CreateUser(name, groups, home, ui.UID, ui.GID)
 		if err != nil && !trace.IsAlreadyExists(err) {
 			return trace.WrapWithMessage(err, "error while creating user")
 		}
@@ -320,8 +328,8 @@ func (u *HostUserManagement) CreateUser(name string, ui *services.HostUsersInfo)
 			return trace.Wrap(err)
 		}
 
-		if ui.Mode != types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP {
-			if err := u.backend.CreateHomeDirectory(name, user.Uid, user.Gid); err != nil {
+		if home != "" {
+			if err := u.backend.CreateHomeDirectory(home, user.Uid, user.Gid); err != nil {
 				return trace.Wrap(err)
 			}
 		}

lib/srv/usermgmt_linux.go

@@ -92,12 +92,8 @@ func (*HostUsersProvisioningBackend) CreateGroup(name string, gid string) error
 }
 
 // CreateUser creates a user on a host
-func (*HostUsersProvisioningBackend) CreateUser(name string, groups []string, uid, gid string) error {
-	home, err := readDefaultHome(name)
-	if err != nil {
-		return trace.Wrap(err)
-	}
-	_, err = host.UserAdd(name, groups, home, uid, gid)
+func (*HostUsersProvisioningBackend) CreateUser(name string, groups []string, home, uid, gid string) error {
+	_, err := host.UserAdd(name, groups, home, uid, gid)
 	return trace.Wrap(err)
 }
 
@@ -214,7 +210,7 @@ func readDefaultSkel() (string, error) {
 	return skel, trace.Wrap(err)
 }
 
-func (u *HostUsersProvisioningBackend) CreateHomeDirectory(user string, uidS, gidS string) error {
+func (u *HostUsersProvisioningBackend) CreateHomeDirectory(userHome, uidS, gidS string) error {
 	uid, err := strconv.Atoi(uidS)
 	if err != nil {
 		return trace.Wrap(err)
@@ -224,11 +220,6 @@ func (u *HostUsersProvisioningBackend) CreateHomeDirectory(user string, uidS, gi
 		return trace.Wrap(err)
 	}
 
-	userHome, err := readDefaultHome(user)
-	if err != nil {
-		return trace.Wrap(err)
-	}
-
 	err = os.Mkdir(userHome, 0o700)
 	if err != nil {
 		if os.IsExist(err) {

lib/srv/usermgmt_other.go

@@ -34,3 +34,7 @@ func newHostUsersBackend() (HostUsersBackend, error) {
 func newHostSudoersBackend(_ string) (HostSudoersBackend, error) {
 	return nil, trace.NotImplemented("Host user creation management is only supported on linux")
 }
+
+func readDefaultHome(user string) (string, error) {
+	return "", trace.NotImplemented("readDefaultHome is only supported on linux")
+}

lib/srv/usermgmt_test.go

@@ -106,7 +106,7 @@ func (tm *testHostUserBackend) CreateGroup(group, gid string) error {
 	return nil
 }
 
-func (tm *testHostUserBackend) CreateUser(user string, groups []string, uid, gid string) error {
+func (tm *testHostUserBackend) CreateUser(user string, groups []string, home, uid, gid string) error {
 	_, ok := tm.users[user]
 	if ok {
 		return trace.AlreadyExists("Group %q, already exists", user)
@@ -194,7 +194,7 @@ func TestUserMgmt_CreateTemporaryUser(t *testing.T) {
 	require.NotContains(t, backend.users, "bob")
 
 	backend.CreateGroup("testgroup", "")
-	backend.CreateUser("simon", []string{}, "", "")
+	backend.CreateUser("simon", []string{}, "", "", "")
 
 	// try to create a temporary user for simon
 	closer, err = users.CreateUser("simon", userinfo)
@@ -240,7 +240,7 @@ func TestUserMgmtSudoers_CreateTemporaryUser(t *testing.T) {
 		}
 		// test user already exists but teleport-service group has not yet
 		// been created
-		backend.CreateUser("testuser", nil, "", "")
+		backend.CreateUser("testuser", nil, "", "", "")
 		_, err := users.CreateUser("testuser", &services.HostUsersInfo{
 			Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP,
 		})
@@ -288,7 +288,7 @@ func TestUserMgmt_DeleteAllTeleportSystemUsers(t *testing.T) {
 		if slices.Contains(user.groups, types.TeleportServiceGroup) {
 			users.CreateUser(user.user, &services.HostUsersInfo{Groups: user.groups})
 		} else {
-			mgmt.CreateUser(user.user, user.groups, "", "")
+			mgmt.CreateUser(user.user, user.groups, "", "", "")
 		}
 	}
 	require.NoError(t, users.DeleteAllUsers())

lib/utils/host/hostusers.go

@@ -21,7 +21,9 @@ package host
 import (
 	"bytes"
 	"errors"
+	"os"
 	"os/exec"
+	"os/user"
 	"strings"
 
 	"github.com/gravitational/trace"
@@ -65,7 +67,8 @@ func UserAdd(username string, groups []string, home, uid, gid string) (exitCode
 	}
 
 	if home == "" {
-		return -1, trace.BadParameter("home is a required parameter")
+		// Users without a home directory should land at the root, to match OpenSSH behavior.
+		home = string(os.PathSeparator)
 	}
 
 	// useradd ---no-create-home (username) (groups)...
@@ -111,8 +114,18 @@ func UserDel(username string) (exitCode int, err error) {
 	if err != nil {
 		return -1, trace.Wrap(err, "cant find userdel binary")
 	}
+	u, err := user.Lookup(username)
+	if err != nil {
+		return -1, trace.Wrap(err)
+	}
+	args := make([]string, 0, 2)
+	// Only remove the home dir if it exists and isn't the root.
+	if u.HomeDir != "" && u.HomeDir != string(os.PathSeparator) {
+		args = append(args, "--remove")
+	}
+	args = append(args, username)
 	// userdel --remove (remove home) username
-	cmd := exec.Command(userdelBin, "--remove", username)
+	cmd := exec.Command(userdelBin, args...)
 	output, err := cmd.CombinedOutput()
 	log.Debugf("%s output: %s", cmd.Path, string(output))
 	return cmd.ProcessState.ExitCode(), trace.Wrap(err)