diff --git a/api/types/constants.go b/api/types/constants.go index 0c1980e75c965..593184682ef3e 100644 --- a/api/types/constants.go +++ b/api/types/constants.go @@ -1233,10 +1233,14 @@ var KubernetesClusterWideResourceKinds = []string{ } const ( - // TeleportServiceGroup is a default group that users of the - // teleport automated user provisioning system get added to so - // already existing users are not deleted - TeleportServiceGroup = "teleport-system" + // TeleportDropGroup is a default group that users of the teleport automated user + // provisioning system get added to when provisioned in INSECURE_DROP mode. This + // prevents already existing users from being tampered with or deleted. + TeleportDropGroup = "teleport-system" + // TeleportKeepGroup is a default group that users of the teleport automated user + // provisioning system get added to when provisioned in KEEP mode. This prevents + // already existing users from being tampered with or deleted. + TeleportKeepGroup = "teleport-keep" ) const ( diff --git a/integration/hostuser_test.go b/integration/hostuser_test.go index 97263d5b8b0b8..25ea1c0c161e7 100644 --- a/integration/hostuser_test.go +++ b/integration/hostuser_test.go @@ -226,7 +226,7 @@ func TestRootHostUsers(t *testing.T) { closer, err := users.UpsertUser(testuser, services.HostUsersInfo{Groups: testGroups, Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP}) require.NoError(t, err) - testGroups = append(testGroups, types.TeleportServiceGroup) + testGroups = append(testGroups, types.TeleportDropGroup) t.Cleanup(cleanupUsersAndGroups([]string{testuser}, testGroups)) u, err := user.Lookup(testuser) @@ -255,7 +255,7 @@ func TestRootHostUsers(t *testing.T) { }) require.NoError(t, err) - t.Cleanup(cleanupUsersAndGroups([]string{testuser}, []string{types.TeleportServiceGroup})) + t.Cleanup(cleanupUsersAndGroups([]string{testuser}, []string{types.TeleportDropGroup})) group, err := user.LookupGroupId(testGID) require.NoError(t, err) @@ -280,7 +280,7 @@ func TestRootHostUsers(t *testing.T) { closer, err := users.UpsertUser(testuser, services.HostUsersInfo{Mode: types.CreateHostUserMode_HOST_USER_MODE_KEEP}) require.NoError(t, err) require.Nil(t, closer) - t.Cleanup(cleanupUsersAndGroups([]string{testuser}, nil)) + t.Cleanup(cleanupUsersAndGroups([]string{testuser}, []string{types.TeleportKeepGroup})) u, err := user.Lookup(testuser) require.NoError(t, err) @@ -351,7 +351,7 @@ func TestRootHostUsers(t *testing.T) { t.Cleanup(cleanupUsersAndGroups( []string{"teleport-user1", "teleport-user2", "teleport-user3", "teleport-user4"}, - []string{types.TeleportServiceGroup})) + []string{types.TeleportDropGroup, types.TeleportKeepGroup})) err = users.DeleteAllUsers() require.NoError(t, err) diff --git a/lib/srv/reexec.go b/lib/srv/reexec.go index c123ca739a568..ceb25caa0b1a2 100644 --- a/lib/srv/reexec.go +++ b/lib/srv/reexec.go @@ -570,9 +570,9 @@ func (o *osWrapper) startNewParker(ctx context.Context, credential *syscall.Cred return nil } - group, err := o.LookupGroup(types.TeleportServiceGroup) + group, err := o.LookupGroup(types.TeleportDropGroup) if err != nil { - if isUnknownGroupError(err, types.TeleportServiceGroup) { + if isUnknownGroupError(err, types.TeleportDropGroup) { // The service group doesn't exist. Auto-provision is disabled, do nothing. return nil } @@ -593,7 +593,7 @@ func (o *osWrapper) startNewParker(ctx context.Context, credential *syscall.Cred } if !found { - // Check if the new user guid matches the TeleportServiceGroup. If not + // Check if the new user guid matches the TeleportDropGroup. If not // this user hasn't been created by Teleport, and we don't need the parker. return nil } diff --git a/lib/srv/reexec_test.go b/lib/srv/reexec_test.go index 071f7efe7156e..4847198c52b12 100644 --- a/lib/srv/reexec_test.go +++ b/lib/srv/reexec_test.go @@ -94,8 +94,8 @@ func TestStartNewParker(t *testing.T) { newOsPack: func(t *testing.T) (*osWrapper, func()) { return &osWrapper{ LookupGroup: func(name string) (*user.Group, error) { - require.Equal(t, types.TeleportServiceGroup, name) - return nil, user.UnknownGroupError(types.TeleportServiceGroup) + require.Equal(t, types.TeleportDropGroup, name) + return nil, user.UnknownGroupError(types.TeleportDropGroup) }, }, func() {} }, @@ -106,7 +106,7 @@ func TestStartNewParker(t *testing.T) { newOsPack: func(t *testing.T) (*osWrapper, func()) { return &osWrapper{ LookupGroup: func(name string) (*user.Group, error) { - require.Equal(t, types.TeleportServiceGroup, name) + require.Equal(t, types.TeleportDropGroup, name) return &user.Group{Gid: "1234"}, nil }, CommandContext: func(ctx context.Context, name string, arg ...string) *exec.Cmd { @@ -132,7 +132,7 @@ func TestStartNewParker(t *testing.T) { return &osWrapper{ LookupGroup: func(name string) (*user.Group, error) { - require.Equal(t, types.TeleportServiceGroup, name) + require.Equal(t, types.TeleportDropGroup, name) return &user.Group{Gid: currentUser.Gid}, nil }, CommandContext: func(ctx context.Context, name string, arg ...string) *exec.Cmd { diff --git a/lib/srv/sess.go b/lib/srv/sess.go index c7e2754c1e747..0857c78d54e26 100644 --- a/lib/srv/sess.go +++ b/lib/srv/sess.go @@ -288,12 +288,17 @@ func (s *SessionRegistry) TryCreateHostUser(identityContext IdentityContext) (cr if trace.IsAccessDenied(err) && existsErr != nil { return false, nil, trace.WrapWithMessage(err, "Insufficient permission for host user creation") } + userCloser, err := s.users.UpsertUser(identityContext.Login, *ui) - if err != nil && !trace.IsAlreadyExists(err) { + if err != nil && !trace.IsAlreadyExists(err) && !errors.Is(err, unmanagedUserErr) { log.Debugf("Error creating user %s: %s", identityContext.Login, err) return false, nil, trace.Wrap(err) } + if errors.Is(err, unmanagedUserErr) { + log.Warnf("User %q is not managed by teleport. Either manually delete the user from this machine or update the host_groups defined in their role to include %q. https://goteleport.com/docs/enroll-resources/server-access/guides/host-user-creation/#migrating-unmanaged-users", identityContext.Login, types.TeleportKeepGroup) + } + return true, userCloser, nil } diff --git a/lib/srv/usermgmt.go b/lib/srv/usermgmt.go index dddbd3763b725..813a45ecdedb5 100644 --- a/lib/srv/usermgmt.go +++ b/lib/srv/usermgmt.go @@ -118,7 +118,7 @@ type userCloser struct { } func (u *userCloser) Close() error { - teleportGroup, err := u.backend.LookupGroup(types.TeleportServiceGroup) + teleportGroup, err := u.backend.LookupGroup(types.TeleportDropGroup) if err != nil { return trace.Wrap(err) } @@ -131,7 +131,7 @@ func (u *userCloser) Close() error { var ErrUserLoggedIn = errors.New("User logged in error") type HostSudoers interface { - // WriteSudoers creates a temporary Teleport user in the TeleportServiceGroup + // WriteSudoers creates a temporary Teleport user in the TeleportDropGroup WriteSudoers(name string, sudoers []string) error // RemoveSudoers removes the users sudoer file RemoveSudoers(name string) error @@ -139,7 +139,7 @@ type HostSudoers interface { type HostSudoersNotImplemented struct{} -// WriteSudoers creates a temporary Teleport user in the TeleportServiceGroup +// WriteSudoers creates a temporary Teleport user in the TeleportDropGroup func (*HostSudoersNotImplemented) WriteSudoers(string, []string) error { return trace.NotImplemented("host sudoers functionality not implemented on this platform") } @@ -150,12 +150,12 @@ func (*HostSudoersNotImplemented) RemoveSudoers(name string) error { } type HostUsers interface { - // UpsertUser creates a temporary Teleport user in the TeleportServiceGroup + // UpsertUser creates a temporary Teleport user in the TeleportDropGroup UpsertUser(name string, hostRoleInfo services.HostUsersInfo) (io.Closer, error) // DeleteUser deletes a temporary Teleport user only if they are // in a specified group DeleteUser(name string, gid string) error - // DeleteAllUsers deletes all suer in the TeleportServiceGroup + // DeleteAllUsers deletes all suer in the TeleportDropGroup DeleteAllUsers() error // UserCleanup starts a periodic user deletion cleanup loop for // users that failed to delete @@ -226,7 +226,11 @@ func (u *HostSudoersManagement) RemoveSudoers(name string) error { return nil } +// unmanagedUserErr is returned when attempting to modify or interact with a user that is not managed by Teleport. +var unmanagedUserErr = errors.New("user not managed by teleport") + func (u *HostUserManagement) updateUser(name string, ui services.HostUsersInfo) error { + existingUser, err := u.backend.Lookup(name) if err != nil { return trace.Wrap(err) @@ -247,9 +251,35 @@ func (u *HostUserManagement) updateUser(name string, ui services.HostUsersInfo) currentGroups[group.Name] = struct{}{} } - _, hasSystemGroup := currentGroups[types.TeleportServiceGroup] - if hasSystemGroup && ui.Mode == types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP { - ui.Groups = append(ui.Groups, types.TeleportServiceGroup) + // allow for explicit assignment of teleport-keep group in order to facilitate migrating KEEP users that existed before we added + // the teleport-keep group + migrateKeepUser := slices.Contains(ui.Groups, types.TeleportKeepGroup) + + _, hasDropGroup := currentGroups[types.TeleportDropGroup] + _, hasKeepGroup := currentGroups[types.TeleportKeepGroup] + if !hasDropGroup && !hasKeepGroup && !migrateKeepUser { + return trace.Errorf("%q %w", name, unmanagedUserErr) + } + + switch ui.Mode { + case types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP: + ui.Groups = append(ui.Groups, types.TeleportDropGroup) + case types.CreateHostUserMode_HOST_USER_MODE_KEEP: + if !hasKeepGroup { + home, err := u.backend.GetDefaultHomeDirectory(name) + if err != nil { + return trace.Wrap(err) + } + + if err := u.backend.CreateHomeDirectory(home, existingUser.Uid, existingUser.Gid); err != nil { + return trace.Wrap(err) + } + } + + // no need to duplicate the group if it's already there + if !migrateKeepUser { + ui.Groups = append(ui.Groups, types.TeleportKeepGroup) + } } finalGroups := make(map[string]struct{}, len(ui.Groups)) @@ -275,9 +305,12 @@ func (u *HostUserManagement) updateUser(name string, ui services.HostUsersInfo) func (u *HostUserManagement) createUser(name string, ui services.HostUsersInfo) error { var home string var err error - if ui.Mode == types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP { - ui.Groups = append(ui.Groups, types.TeleportServiceGroup) - } else { + + switch ui.Mode { + case types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP: + ui.Groups = append(ui.Groups, types.TeleportDropGroup) + case types.CreateHostUserMode_HOST_USER_MODE_KEEP: + ui.Groups = append(ui.Groups, types.TeleportKeepGroup) home, err = u.backend.GetDefaultHomeDirectory(name) if err != nil { return trace.Wrap(err) @@ -319,19 +352,54 @@ func (u *HostUserManagement) createUser(name string, ui services.HostUsersInfo) })) } -// UpsertUser creates a temporary Teleport user in the TeleportServiceGroup -func (u *HostUserManagement) UpsertUser(name string, ui services.HostUsersInfo) (io.Closer, error) { - var groupErrs []error - // cloning to prevent unintended mutation of passed in Groups slice - ui.Groups = slices.Clone(ui.Groups) - for _, group := range append(ui.Groups, types.TeleportServiceGroup) { +func (u *HostUserManagement) ensureGroupsExist(groups ...string) error { + var errs []error + + for _, group := range groups { if err := u.createGroupIfNotExist(group); err != nil { - groupErrs = append(groupErrs, err) + errs = append(errs, err) } } - if err := trace.NewAggregate(groupErrs...); err != nil { - return nil, trace.WrapWithMessage(err, "error while creating groups") + return trace.NewAggregate(errs...) +} + +// UpsertUser creates a temporary Teleport user in the TeleportDropGroup +func (u *HostUserManagement) UpsertUser(name string, ui services.HostUsersInfo) (io.Closer, error) { + // allow for explicit assignment of teleport-keep group in order to facilitate migrating KEEP users that existed before we added + // the teleport-keep group + migrateKeepUser := slices.Contains(ui.Groups, types.TeleportKeepGroup) + skipKeepGroup := migrateKeepUser && ui.Mode == types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP + + if skipKeepGroup { + log.Warnf("explicit assignment of %q group is not possible in 'insecure-drop' mode", types.TeleportKeepGroup) + } + + groupSet := make(map[string]struct{}, len(ui.Groups)) + groups := make([]string, 0, len(ui.Groups)) + for _, group := range ui.Groups { + // the TeleportDropGroup is managed automatically and should not be allowed direct assignment + if group == types.TeleportDropGroup { + continue + } + + if skipKeepGroup && group == types.TeleportKeepGroup { + continue + } + + if _, ok := groupSet[group]; !ok { + groupSet[group] = struct{}{} + groups = append(groups, group) + } + } + ui.Groups = groups + + if err := u.ensureGroupsExist(types.TeleportDropGroup, types.TeleportKeepGroup); err != nil { + return nil, trace.WrapWithMessage(err, "creating teleport system groups") + } + + if err := u.ensureGroupsExist(groups...); err != nil { + return nil, trace.WrapWithMessage(err, "creating configured groups") } var closer io.Closer @@ -409,10 +477,10 @@ func (u *HostUserManagement) DeleteAllUsers() error { if err != nil { return trace.Wrap(err) } - teleportGroup, err := u.backend.LookupGroup(types.TeleportServiceGroup) + teleportGroup, err := u.backend.LookupGroup(types.TeleportDropGroup) if err != nil { - if isUnknownGroupError(err, types.TeleportServiceGroup) { - log.Debugf("%q group not found, not deleting users", types.TeleportServiceGroup) + if isUnknownGroupError(err, types.TeleportDropGroup) { + log.Debugf("%q group not found, not deleting users", types.TeleportDropGroup) return nil } return trace.Wrap(err) diff --git a/lib/srv/usermgmt_test.go b/lib/srv/usermgmt_test.go index 4ed366019f9ba..753e409454d28 100644 --- a/lib/srv/usermgmt_test.go +++ b/lib/srv/usermgmt_test.go @@ -48,7 +48,8 @@ type testHostUserBackend struct { // userGID: user -> gid userGID map[string]string - setUserGroupsCalls int + setUserGroupsCalls int + createHomeDirectoryCalls int } func newTestUserMgmt() *testHostUserBackend { @@ -159,6 +160,7 @@ func (tm *testHostUserBackend) DeleteUser(user string) error { } func (tm *testHostUserBackend) CreateHomeDirectory(user, uid, gid string) error { + tm.createHomeDirectoryCalls++ return nil } @@ -226,7 +228,7 @@ func TestUserMgmt_CreateTemporaryUser(t *testing.T) { // temproary users must always include the teleport-service group require.Equal(t, []string{ - "hello", "sudo", types.TeleportServiceGroup, + "hello", "sudo", types.TeleportDropGroup, }, backend.users["bob"]) // try create the same user again @@ -241,10 +243,10 @@ func TestUserMgmt_CreateTemporaryUser(t *testing.T) { backend.CreateGroup("testgroup", "") backend.CreateUser("simon", []string{}, "", "", "") - // try to create a temporary user for simon + // an existing, unmanaged user should not be changed closer, err = users.UpsertUser("simon", userinfo) - require.NoError(t, err) - require.NotEqual(t, nil, closer) + require.ErrorIs(t, err, unmanagedUserErr) + require.Equal(t, nil, closer) } func TestUserMgmtSudoers_CreateTemporaryUser(t *testing.T) { @@ -289,12 +291,12 @@ func TestUserMgmtSudoers_CreateTemporaryUser(t *testing.T) { _, err := users.UpsertUser("testuser", services.HostUsersInfo{ Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP, }) - require.NoError(t, err) - backend.CreateGroup(types.TeleportServiceGroup, "") + require.ErrorIs(t, err, unmanagedUserErr) + backend.CreateGroup(types.TeleportDropGroup, "") _, err = users.UpsertUser("testuser", services.HostUsersInfo{ Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP, }) - require.NoError(t, err) + require.ErrorIs(t, err, unmanagedUserErr) }) } @@ -307,8 +309,8 @@ func TestUserMgmt_DeleteAllTeleportSystemUsers(t *testing.T) { } usersDB := []userAndGroups{ - {"fgh", []string{types.TeleportServiceGroup}}, - {"xyz", []string{types.TeleportServiceGroup}}, + {"fgh", []string{types.TeleportDropGroup}}, + {"xyz", []string{types.TeleportDropGroup}}, {"pqr", []string{"not-deleted"}}, {"abc", []string{"not-deleted"}}, } @@ -329,7 +331,7 @@ func TestUserMgmt_DeleteAllTeleportSystemUsers(t *testing.T) { for _, group := range user.groups { mgmt.CreateGroup(group, "") } - if slices.Contains(user.groups, types.TeleportServiceGroup) { + if slices.Contains(user.groups, types.TeleportDropGroup) { users.UpsertUser(user.user, services.HostUsersInfo{ Groups: user.groups, Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP, @@ -399,9 +401,8 @@ func TestIsUnknownGroupError(t *testing.T) { } } -func TestUpdateUserGroups(t *testing.T) { +func Test_UpdateUserGroups_Keep(t *testing.T) { t.Parallel() - backend := newTestUserMgmt() bk, err := memory.New(memory.Config{}) require.NoError(t, err) @@ -417,35 +418,107 @@ func TestUpdateUserGroups(t *testing.T) { } userinfo := services.HostUsersInfo{ - Groups: allGroups[:2], + Groups: slices.Clone(allGroups[:2]), Mode: types.CreateHostUserMode_HOST_USER_MODE_KEEP, } - // Create a user with some groups. + // Create user closer, err := users.UpsertUser("alice", userinfo) assert.NoError(t, err) assert.Equal(t, nil, closer) assert.Zero(t, backend.setUserGroupsCalls) - assert.ElementsMatch(t, userinfo.Groups, backend.users["alice"]) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportKeepGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportDropGroup) // Update user with new groups. - userinfo.Groups = allGroups[2:] + userinfo.Groups = slices.Clone(allGroups[2:]) closer, err = users.UpsertUser("alice", userinfo) assert.NoError(t, err) assert.Equal(t, nil, closer) assert.Equal(t, 1, backend.setUserGroupsCalls) - assert.ElementsMatch(t, userinfo.Groups, backend.users["alice"]) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportKeepGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportDropGroup) // Upsert again with same groups should not call SetUserGroups. closer, err = users.UpsertUser("alice", userinfo) assert.NoError(t, err) assert.Equal(t, nil, closer) assert.Equal(t, 1, backend.setUserGroupsCalls) - assert.ElementsMatch(t, userinfo.Groups, backend.users["alice"]) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportKeepGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportDropGroup) + + // Updates with INSECURE_DROP mode should convert the managed user + userinfo.Mode = types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP + userinfo.Groups = slices.Clone(allGroups[:2]) + closer, err = users.UpsertUser("alice", userinfo) + assert.NoError(t, err) + assert.NotEqual(t, nil, closer) + assert.Equal(t, 2, backend.setUserGroupsCalls) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportDropGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportKeepGroup) } -func Test_DontDropExistingUser(t *testing.T) { +func Test_UpdateUserGroups_Drop(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, + } + + allGroups := []string{"foo", "bar", "baz", "quux"} + for _, group := range allGroups { + require.NoError(t, backend.CreateGroup(group, "")) + } + + userinfo := services.HostUsersInfo{ + Groups: slices.Clone(allGroups[:2]), + Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP, + } + + // Create user + closer, err := users.UpsertUser("alice", userinfo) + assert.NoError(t, err) + assert.NotEqual(t, nil, closer) + assert.Zero(t, backend.setUserGroupsCalls) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportDropGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportKeepGroup) + + // Update user with new groups. + userinfo.Groups = slices.Clone(allGroups[2:]) + + closer, err = users.UpsertUser("alice", userinfo) + assert.NoError(t, err) + assert.NotEqual(t, nil, closer) + assert.Equal(t, 1, backend.setUserGroupsCalls) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportDropGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportKeepGroup) + + // Upsert again with same groups should not call SetUserGroups. + closer, err = users.UpsertUser("alice", userinfo) + assert.NoError(t, err) + assert.NotEqual(t, nil, closer) + assert.Equal(t, 1, backend.setUserGroupsCalls) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportDropGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportKeepGroup) + + // Updates with KEEP mode should convert the ephemeral user + userinfo.Mode = types.CreateHostUserMode_HOST_USER_MODE_KEEP + userinfo.Groups = slices.Clone(allGroups[:2]) + closer, err = users.UpsertUser("alice", userinfo) + assert.NoError(t, err) + assert.Equal(t, nil, closer) + assert.Equal(t, 2, backend.setUserGroupsCalls) + assert.Equal(t, 1, backend.createHomeDirectoryCalls) + assert.ElementsMatch(t, append(userinfo.Groups, types.TeleportKeepGroup), backend.users["alice"]) + assert.NotContains(t, backend.users["alice"], types.TeleportDropGroup) +} + +func Test_DontManageExistingUser(t *testing.T) { t.Parallel() backend := newTestUserMgmt() @@ -462,25 +535,122 @@ func Test_DontDropExistingUser(t *testing.T) { require.NoError(t, backend.CreateGroup(group, "")) } + assert.NoError(t, backend.CreateUser("alice", allGroups, "", "", "")) + + userinfo := services.HostUsersInfo{ + Groups: allGroups[:2], + Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP, + } + + // Update user in DROP mode + closer, err := users.UpsertUser("alice", userinfo) + assert.ErrorIs(t, err, unmanagedUserErr) + assert.Equal(t, nil, closer) + assert.Zero(t, backend.setUserGroupsCalls) + assert.ElementsMatch(t, allGroups, backend.users["alice"]) + + // Update user in KEEP mode + userinfo.Mode = types.CreateHostUserMode_HOST_USER_MODE_KEEP + closer, err = users.UpsertUser("alice", userinfo) + assert.ErrorIs(t, err, unmanagedUserErr) + assert.Equal(t, nil, closer) + assert.Zero(t, backend.setUserGroupsCalls) + assert.ElementsMatch(t, allGroups, backend.users["alice"]) +} + +func Test_DontUpdateUnmanagedUsers(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, + } + + allGroups := []string{"foo", "bar", "baz", "quux"} + for _, group := range allGroups { + require.NoError(t, backend.CreateGroup(group, "")) + } + + assert.NoError(t, backend.CreateUser("alice", allGroups[2:], "", "", "")) userinfo := services.HostUsersInfo{ Groups: allGroups[:2], Mode: types.CreateHostUserMode_HOST_USER_MODE_KEEP, } - // Create a user with some groups. + // Try to update existing, unmanaged user in KEEP mode closer, err := users.UpsertUser("alice", userinfo) - assert.NoError(t, err) + assert.ErrorIs(t, err, unmanagedUserErr) assert.Equal(t, nil, closer) assert.Zero(t, backend.setUserGroupsCalls) - assert.ElementsMatch(t, userinfo.Groups, backend.users["alice"]) + assert.ElementsMatch(t, allGroups[2:], backend.users["alice"]) - // Upserting an existing user in INSECURE_DROP mode shouldn't make them ephemeral - userinfo.Mode = types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP - userinfo.Groups = allGroups[2:] + userinfo = services.HostUsersInfo{ + Groups: allGroups[:2], + Mode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP, + } + + // Try to update existing, unmanaged user in DROP mode closer, err = users.UpsertUser("alice", userinfo) + assert.ErrorIs(t, err, unmanagedUserErr) + assert.Equal(t, nil, closer) + assert.Zero(t, backend.setUserGroupsCalls) + assert.ElementsMatch(t, allGroups[2:], backend.users["alice"]) +} + +// teleport-keep can be included explicitly in the Groups slice in order to flag an +// existing user as being managed by teleport +func Test_AllowExplicitlyManageExistingUsers(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, + } + + allGroups := []string{"foo", types.TeleportKeepGroup, types.TeleportDropGroup} + for _, group := range allGroups { + require.NoError(t, backend.CreateGroup(group, "")) + } + + assert.NoError(t, backend.CreateUser("alice-keep", []string{}, "", "", "")) + assert.NoError(t, backend.CreateUser("alice-drop", []string{}, "", "", "")) + userinfo := services.HostUsersInfo{ + Groups: slices.Clone(allGroups), + Mode: types.CreateHostUserMode_HOST_USER_MODE_KEEP, + } + + // Take ownership of existing user when in KEEP mode + closer, err := users.UpsertUser("alice-keep", userinfo) + assert.NoError(t, err) + assert.Equal(t, nil, closer) + assert.Equal(t, 1, backend.setUserGroupsCalls) + // slice off the end because teleport-system should be explicitly excluded + assert.ElementsMatch(t, allGroups[:2], backend.users["alice-keep"]) + assert.NotContains(t, backend.users["alice-keep"], types.TeleportDropGroup) + + // Don't take ownership of existing user when in DROP mode + userinfo.Mode = types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP + closer, err = users.UpsertUser("alice-drop", userinfo) + assert.ErrorIs(t, err, unmanagedUserErr) + assert.Equal(t, nil, closer) + assert.Equal(t, 1, backend.setUserGroupsCalls) + assert.Empty(t, backend.users["alice-drop"]) + + // Don't assign teleport-keep to users created in DROP mode + userinfo.Mode = types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP + closer, err = users.UpsertUser("bob", userinfo) assert.NoError(t, err) assert.NotEqual(t, nil, closer) assert.Equal(t, 1, backend.setUserGroupsCalls) - assert.ElementsMatch(t, userinfo.Groups, backend.users["alice"]) - assert.NotContains(t, backend.users["alice"], types.TeleportServiceGroup) + assert.ElementsMatch(t, []string{"foo", types.TeleportDropGroup}, backend.users["bob"]) + assert.NotContains(t, backend.users["bob"], types.TeleportKeepGroup) + }