diff --git a/changes/9072-new-activity-role-users b/changes/9072-new-activity-role-users new file mode 100644 index 0000000000..4e848aa94b --- /dev/null +++ b/changes/9072-new-activity-role-users @@ -0,0 +1 @@ +- Add new activity that records create/edit/delete user roles. \ No newline at end of file diff --git a/cmd/fleetctl/users_test.go b/cmd/fleetctl/users_test.go index 2a2c975065..d7d6bf659c 100644 --- a/cmd/fleetctl/users_test.go +++ b/cmd/fleetctl/users_test.go @@ -34,6 +34,10 @@ func TestUserDelete(t *testing.T) { deletedUser = id return nil } + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { + assert.Equal(t, fleet.ActivityTypeDeletedUser, activityType) + return nil + } assert.Equal(t, "", runAppForTest(t, []string{"user", "delete", "--email", "user1@test.com"})) assert.Equal(t, uint(42), deletedUser) @@ -62,6 +66,9 @@ func TestUserCreateForcePasswordReset(t *testing.T) { ds.InviteByEmailFunc = func(ctx context.Context, email string) (*fleet.Invite, error) { return nil, ¬FoundError{} } + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { + return nil + } for _, tc := range []struct { name string @@ -117,6 +124,9 @@ func TestCreateBulkUsers(t *testing.T) { ds.InviteByEmailFunc = func(ctx context.Context, email string) (*fleet.Invite, error) { return nil, nil } + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { + return nil + } csvFile := writeTmpCsv(t, `Name,Email,SSO,API Only,Global Role,Teams @@ -137,6 +147,9 @@ func TestCreateBulkUsers(t *testing.T) { func TestDeleteBulkUsers(t *testing.T) { _, ds := runServerWithMockedDS(t) + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { + return nil + } csvFilePath := writeTmpCsv(t, `Email user11@example.com diff --git a/server/fleet/activities.go b/server/fleet/activities.go index 0ccd5746af..f6c8a38596 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -47,6 +47,18 @@ const ( ActivityTypeEditedAgentOptions = "edited_agent_options" // ActivityTypeAppliedSpecTeam is the activity type for a team spec applied ActivityTypeAppliedSpecTeam = "applied_spec_team" + // ActivityTypeCreatedUser is the activity type for created users. + ActivityTypeCreatedUser = "created_user" + // ActivityTypeDeletedUser is the activity type for deleted users. + ActivityTypeDeletedUser = "deleted_user" + // ActivityTypeChangedUserGlobalRole is the activity type for changed user global role. + ActivityTypeChangedUserGlobalRole = "changed_user_global_role" + // ActivityTypeDeletedUserGlobalRole is the activity type for deleted user global role. + ActivityTypeDeletedUserGlobalRole = "deleted_user_global_role" + // ActivityTypeChangedUserTeamRole is the activity type for changed user team role. + ActivityTypeChangedUserTeamRole = "changed_user_team_role" + // ActivityTypeDeletedUserTeamRole is the activity type for deleted user team role. + ActivityTypeDeletedUserTeamRole = "deleted_user_team_role" ) type Activity struct { diff --git a/server/service/activities.go b/server/service/activities.go index 8dae97ab13..7a7825fd9b 100644 --- a/server/service/activities.go +++ b/server/service/activities.go @@ -38,3 +38,62 @@ func (svc *Service) ListActivities(ctx context.Context, opt fleet.ListOptions) ( } return svc.ds.ListActivities(ctx, opt) } + +// logRoleChangeActivities stores the activities for role changes, globally and in teams. +func logRoleChangeActivities(ctx context.Context, ds fleet.Datastore, adminUser *fleet.User, oldRole *string, oldTeams []fleet.UserTeam, user *fleet.User) error { + if user.GlobalRole != nil && (oldRole == nil || *oldRole != *user.GlobalRole) { + if err := ds.NewActivity( + ctx, + adminUser, + fleet.ActivityTypeChangedUserGlobalRole, + &map[string]interface{}{"user_name": user.Name, "user_id": user.ID, "user_email": user.Email, "role": *user.GlobalRole}, + ); err != nil { + return err + } + } + if user.GlobalRole == nil && oldRole != nil { + if err := ds.NewActivity( + ctx, + adminUser, + fleet.ActivityTypeDeletedUserGlobalRole, + &map[string]interface{}{"user_name": user.Name, "user_id": user.ID, "user_email": user.Email, "role": *oldRole}, + ); err != nil { + return err + } + } + oldTeamsLookup := make(map[uint]fleet.UserTeam, len(oldTeams)) + for _, t := range oldTeams { + oldTeamsLookup[t.ID] = t + } + + newTeamLookup := make(map[uint]struct{}, len(user.Teams)) + for _, t := range user.Teams { + newTeamLookup[t.ID] = struct{}{} + o, ok := oldTeamsLookup[t.ID] + if ok && o.Role == t.Role { + continue + } + if err := ds.NewActivity( + ctx, + adminUser, + fleet.ActivityTypeChangedUserTeamRole, + &map[string]interface{}{"user_name": user.Name, "user_id": user.ID, "user_email": user.Email, "team_name": t.Name, "team_id": t.ID, "role": t.Role}, + ); err != nil { + return err + } + } + for _, o := range oldTeams { + if _, ok := newTeamLookup[o.ID]; ok { + continue + } + if err := ds.NewActivity( + ctx, + adminUser, + fleet.ActivityTypeDeletedUserTeamRole, + &map[string]interface{}{"user_name": user.Name, "user_id": user.ID, "user_email": user.Email, "team_name": o.Name, "team_id": o.ID, "role": o.Role}, + ); err != nil { + return err + } + } + return nil +} diff --git a/server/service/activities_test.go b/server/service/activities_test.go index 85f1196442..0eab05caf2 100644 --- a/server/service/activities_test.go +++ b/server/service/activities_test.go @@ -7,6 +7,7 @@ import ( "github.com/fleetdm/fleet/v4/server/authz" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mock" + "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" "github.com/stretchr/testify/require" ) @@ -49,3 +50,90 @@ func TestListActivities(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), authz.ForbiddenErrorMessage) } + +func Test_logRoleChangeActivities(t *testing.T) { + tests := []struct { + name string + oldRole *string + newRole *string + oldTeamRoles map[uint]string + newTeamRoles map[uint]string + expectActivities []string + }{ + { + name: "Empty", + }, { + name: "AddGlobal", + newRole: ptr.String("role"), + expectActivities: []string{"changed_user_global_role"}, + }, { + name: "NoChangeGlobal", + oldRole: ptr.String("role"), + newRole: ptr.String("role"), + expectActivities: []string{}, + }, { + name: "ChangeGlobal", + oldRole: ptr.String("old"), + newRole: ptr.String("role"), + expectActivities: []string{"changed_user_global_role"}, + }, { + name: "Delete", + oldRole: ptr.String("old"), + newRole: nil, + expectActivities: []string{"deleted_user_global_role"}, + }, { + name: "SwitchGlobalToTeams", + oldRole: ptr.String("old"), + newTeamRoles: map[uint]string{ + 1: "foo", + 2: "bar", + 3: "baz", + }, + expectActivities: []string{"deleted_user_global_role", "changed_user_team_role", "changed_user_team_role", "changed_user_team_role"}, + }, { + name: "DeleteModifyTeam", + oldTeamRoles: map[uint]string{ + 1: "foo", + 2: "bar", + 3: "baz", + }, + newTeamRoles: map[uint]string{ + 2: "newRole", + 3: "baz", + }, + expectActivities: []string{"changed_user_team_role", "deleted_user_team_role"}, + }, + } + ctx := context.Background() + ds := new(mock.Store) + var activities []string + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { + activities = append(activities, activityType) + return nil + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + activities = activities[:0] + oldTeams := make([]fleet.UserTeam, 0, len(tt.oldTeamRoles)) + for id, r := range tt.oldTeamRoles { + oldTeams = append(oldTeams, fleet.UserTeam{ + Team: fleet.Team{ID: id}, + Role: r, + }) + } + newTeams := make([]fleet.UserTeam, 0, len(tt.newTeamRoles)) + for id, r := range tt.newTeamRoles { + newTeams = append(newTeams, fleet.UserTeam{ + Team: fleet.Team{ID: id}, + Role: r, + }) + } + newUser := &fleet.User{ + GlobalRole: tt.newRole, + Teams: newTeams, + } + require.NoError(t, logRoleChangeActivities(ctx, ds, &fleet.User{}, tt.oldRole, oldTeams, newUser)) + require.Equal(t, tt.expectActivities, activities) + }) + } +} diff --git a/server/service/service_users.go b/server/service/service_users.go index 0a871a9e9f..c53d8cfa03 100644 --- a/server/service/service_users.go +++ b/server/service/service_users.go @@ -3,6 +3,7 @@ package service import ( "context" + "github.com/fleetdm/fleet/v4/server/authz" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" @@ -38,6 +39,24 @@ func (svc *Service) NewUser(ctx context.Context, p fleet.UserPayload) (*fleet.Us if err != nil { return nil, err } + + adminUser := authz.UserFromContext(ctx) + if adminUser == nil { + // In case of invites the user created herself. + adminUser = user + } + if err := svc.ds.NewActivity( + ctx, + adminUser, + fleet.ActivityTypeCreatedUser, + &map[string]interface{}{"user_name": user.Name, "user_id": user.ID, "user_email": user.Email}, + ); err != nil { + return nil, err + } + if err := logRoleChangeActivities(ctx, svc.ds, adminUser, nil, nil, user); err != nil { + return nil, err + } + return user, nil } diff --git a/server/service/users.go b/server/service/users.go index 0d23fc7e34..c71554b091 100644 --- a/server/service/users.go +++ b/server/service/users.go @@ -277,6 +277,9 @@ func (svc *Service) ModifyUser(ctx context.Context, userID uint, p fleet.UserPay return nil, err } + oldGlobalRole := user.GlobalRole + oldTeams := user.Teams + if err := svc.authz.Authorize(ctx, user, fleet.ActionWrite); err != nil { return nil, err } @@ -375,6 +378,16 @@ func (svc *Service) ModifyUser(ctx context.Context, userID uint, p fleet.UserPay return nil, err } + // load user again to get team-details like names. + user, err = svc.User(ctx, userID) + if err != nil { + return nil, err + } + adminUser := authz.UserFromContext(ctx) + if err := logRoleChangeActivities(ctx, svc.ds, adminUser, oldGlobalRole, oldTeams, user); err != nil { + return nil, err + } + return user, nil } @@ -410,7 +423,21 @@ func (svc *Service) DeleteUser(ctx context.Context, id uint) error { if err := svc.authz.Authorize(ctx, user, fleet.ActionWrite); err != nil { return err } - return svc.ds.DeleteUser(ctx, id) + if err := svc.ds.DeleteUser(ctx, id); err != nil { + return err + } + + adminUser := authz.UserFromContext(ctx) + if err := svc.ds.NewActivity( + ctx, + adminUser, + fleet.ActivityTypeDeletedUser, + &map[string]interface{}{"user_name": user.Name, "user_id": user.ID, "user_email": user.Email}, + ); err != nil { + return err + } + + return nil } //////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/users_test.go b/server/service/users_test.go index b99145f4b9..f16c678e54 100644 --- a/server/service/users_test.go +++ b/server/service/users_test.go @@ -77,6 +77,9 @@ func TestUserAuth(t *testing.T) { ds.ListSessionsForUserFunc = func(ctx context.Context, id uint) ([]*fleet.Session, error) { return nil, nil } + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { + return nil + } testCases := []struct { name string