From 07e3539e206309c9a9c7f05b9da65815ae08323d Mon Sep 17 00:00:00 2001 From: Erik Tate Date: Tue, 22 Oct 2024 14:24:03 -0400 Subject: [PATCH] remove Expired field in favor of overriding HostUsers backend --- integration/hostuser_test.go | 57 +++++++++++++++++++++++++--------- lib/services/access_checker.go | 3 -- lib/srv/usermgmt.go | 17 +++++++--- lib/srv/usermgmt_test.go | 4 +++ lib/utils/host/hostusers.go | 7 ----- 5 files changed, 59 insertions(+), 29 deletions(-) diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 1c9753a227ef6..0b658e2f9ddcb 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -516,30 +516,41 @@ func TestRootHostUsers(t *testing.T) { t.Run("Test expiration removal", func(t *testing.T) { expiredUser := "expired-user" - t.Cleanup(func() { cleanupUsersAndGroups([]string{expiredUser}, []string{"test-group"}) }) + backendExpiredUser := "backend-expired-user" + t.Cleanup(func() { cleanupUsersAndGroups([]string{expiredUser, backendExpiredUser}, []string{"test-group"}) }) - users := srv.NewHostUsers(context.Background(), presence, "host_uuid") - _, err := users.UpsertUser(expiredUser, services.HostUsersInfo{ - Mode: services.HostUserModeKeep, - Expired: true, - }) + users := srv.NewHostUsers(context.Background(), presence, "host_uuid").(*srv.HostUserManagement) + backend := users.GetBackend() + users.OverrideBackend(&hostUsersBackendWithExp{HostUsersBackend: backend}) + backend = users.GetBackend() // GetBackend again so we get the overridden version + + // Make sure the backend actually creates expired users + err := backend.CreateUser("backend-expired-user", nil, host.UserOpts{}) require.NoError(t, err) - // Ensure new user expirations are removed - hasExpirations, _, err := host.UserHasExpirations(expiredUser) + hasExpirations, _, err := host.UserHasExpirations(backendExpiredUser) + require.NoError(t, err) + require.True(t, hasExpirations) + + // Upsert a new user which should have the expirations removed + _, err = users.UpsertUser(expiredUser, services.HostUsersInfo{ + Mode: services.HostUserModeKeep, + }) require.NoError(t, err) - require.False(t, hasExpirations) - chageBin, err := exec.LookPath("chage") + hasExpirations, _, err = host.UserHasExpirations(expiredUser) require.NoError(t, err) + require.False(t, hasExpirations) + // Expire existing user so we can test that updates also remove expirations expireUser := func(username string) error { + chageBin, err := exec.LookPath("chage") + require.NoError(t, err) + cmd := exec.Command(chageBin, "-E", "1", "-I", "1", "-M", "1", username) return cmd.Run() } require.NoError(t, expireUser(expiredUser)) - - // Assert expirations have been applied to user hasExpirations, _, err = host.UserHasExpirations(expiredUser) require.NoError(t, err) require.True(t, hasExpirations) @@ -550,12 +561,11 @@ func TestRootHostUsers(t *testing.T) { }) require.NoError(t, err) - // Ensure expirations are removed hasExpirations, _, err = host.UserHasExpirations(expiredUser) require.NoError(t, err) require.False(t, hasExpirations) - // Reinstate expirations + // Reinstate expirations again require.NoError(t, expireUser(expiredUser)) hasExpirations, _, err = host.UserHasExpirations(expiredUser) require.NoError(t, err) @@ -568,13 +578,30 @@ func TestRootHostUsers(t *testing.T) { }) require.NoError(t, err) - // Ensure expirations are removed again hasExpirations, _, err = host.UserHasExpirations(expiredUser) require.NoError(t, err) require.False(t, hasExpirations) }) } +type hostUsersBackendWithExp struct { + srv.HostUsersBackend +} + +func (u *hostUsersBackendWithExp) CreateUser(name string, groups []string, opts host.UserOpts) error { + if err := u.HostUsersBackend.CreateUser(name, groups, opts); err != nil { + return trace.Wrap(err) + } + + chageBin, err := exec.LookPath("chage") + if err != nil { + return trace.Wrap(err) + } + + cmd := exec.Command(chageBin, "-E", "1", "-I", "1", "-M", "1", name) + return cmd.Run() +} + func TestRootLoginAsHostUser(t *testing.T) { utils.RequireRoot(t) // Create test instance. diff --git a/lib/services/access_checker.go b/lib/services/access_checker.go index 4cb68e8848c7b..7a3a2b8469ce6 100644 --- a/lib/services/access_checker.go +++ b/lib/services/access_checker.go @@ -1014,9 +1014,6 @@ type HostUsersInfo struct { // users, 'keep' mode users still need to assign 'teleport-keep' in the // Groups slice in order to take ownership. TakeOwnership bool - // Expired determines whether or not the user should be created in an expired state - // (this should only be used for testing purposes) - Expired bool } // HostUsers returns host user information matching a server or nil if diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index d90fa493d1712..28f6262db3e2e 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -194,6 +194,16 @@ type HostUserManagement struct { userGrace time.Duration } +// GetBackend returns the unexported HostUsersBackend powering HostUsersManagement. +func (u *HostUserManagement) GetBackend() HostUsersBackend { + return u.backend +} + +// OverrideBackend overrides the HostUsersBackend implementation powering HostUsersManagement. +func (u *HostUserManagement) OverrideBackend(backend HostUsersBackend) { + u.backend = backend +} + type HostSudoersManagement struct { log *slog.Logger @@ -316,10 +326,9 @@ func (u *HostUserManagement) createUser(name string, ui services.HostUsersInfo) var err error userOpts := host.UserOpts{ - UID: ui.UID, - GID: ui.GID, - Shell: ui.Shell, - Expired: ui.Expired, + UID: ui.UID, + GID: ui.GID, + Shell: ui.Shell, } switch ui.Mode { diff --git a/lib/srv/usermgmt_test.go b/lib/srv/usermgmt_test.go index fb07adddf6a34..1a1908e0f431d 100644 --- a/lib/srv/usermgmt_test.go +++ b/lib/srv/usermgmt_test.go @@ -184,6 +184,10 @@ func (*testHostUserBackend) CheckSudoers(contents []byte) error { return errors.New("invalid") } +func (*testHostUserBackend) RemoveExpirations(user string) error { + return nil +} + // WriteSudoersFile implements HostUsersBackend func (tm *testHostUserBackend) WriteSudoersFile(user string, entries []byte) error { entry := strings.TrimSpace(string(entries)) diff --git a/lib/utils/host/hostusers.go b/lib/utils/host/hostusers.go index c800d318cef65..f856e53c04d12 100644 --- a/lib/utils/host/hostusers.go +++ b/lib/utils/host/hostusers.go @@ -85,9 +85,6 @@ type UserOpts struct { // Shell that the user should use when logging in. When empty, the default shell // for the host is used (typically /usr/bin/sh). Shell string - // Expired determines whether or not the user should be created in an expired state - // (this should only be used for testing purposes) - Expired bool } // UserAdd creates a user on a host using `useradd` @@ -123,10 +120,6 @@ func UserAdd(username string, groups []string, opts UserOpts) (exitCode int, err args = append(args, "--gid", opts.GID) } - if opts.Expired { - args = append(args, "-f", "0") - } - if opts.Shell != "" { if shell, err := exec.LookPath(opts.Shell); err != nil { log.Warnf("configured shell %q not found, falling back to host default", opts.Shell)