feat: don't remove user email from activity feed when user deleted

changes/12634-keep-user-email

@@ -0,0 +1,2 @@
+- Fixes bug where a deleted user's email would no longer show in the Activity feed for actions
+  they'd taken.

server/datastore/mysql/activities.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"fmt"
+	"strings"
 
 	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/fleet"
@@ -19,17 +21,29 @@ func (ds *Datastore) NewActivity(ctx context.Context, user *fleet.User, activity
 
 	var userID *uint
 	var userName *string
+	var userEmail *string
 	if user != nil {
 		userID = &user.ID
 		userName = &user.Name
+		userEmail = &user.Email
 	}
 
-	_, err = ds.writer(ctx).ExecContext(ctx,
-		`INSERT INTO activities (user_id, user_name, activity_type, details) VALUES(?,?,?,?)`,
+	cols := []string{"user_id", "user_name", "activity_type", "details"}
+	insertStmt := `INSERT INTO activities (%s) VALUES (%s)`
+	args := []any{
 		userID,
 		userName,
 		activity.ActivityName(),
 		detailsBytes,
+	}
+	if userEmail != nil {
+		args = append(args, userEmail)
+		cols = append(cols, "user_email")
+	}
+	sql := fmt.Sprintf(insertStmt, strings.Join(cols, ","), strings.Repeat("?,", len(cols)-1)+"?")
+	_, err = ds.writer(ctx).ExecContext(ctx,
+		sql,
+		args...,
 	)
 	if err != nil {
 		return ctxerr.Wrap(ctx, err, "new activity")
@@ -50,7 +64,8 @@ func (ds *Datastore) ListActivities(ctx context.Context, opt fleet.ListActivitie
 			a.activity_type,
 			a.details,
 			a.user_name as name,
-			a.streamed
+			a.streamed,
+			a.user_email
 		FROM activities a
 		WHERE true`
 

server/datastore/mysql/migrations/tables/20231107115838_AddUserEmailToActivities.go

@@ -0,0 +1,36 @@
+package tables
+
+import (
+	"database/sql"
+	"fmt"
+)
+
+func init() {
+	MigrationClient.AddMigration(Up_20231107115838, Down_20231107115838)
+}
+
+func Up_20231107115838(tx *sql.Tx) error {
+	stmt := `
+		ALTER TABLE activities
+		ADD COLUMN user_email varchar(255) NOT NULL DEFAULT '';
+	`
+	if _, err := tx.Exec(stmt); err != nil {
+		return fmt.Errorf("add user_email to activities: %w", err)
+	}
+
+	stmt = `
+		UPDATE activities t1
+		INNER JOIN users t2 ON t1.user_id = t2.id 
+		SET t1.user_email = t2.email;
+	`
+
+	if _, err := tx.Exec(stmt); err != nil {
+		return fmt.Errorf("insert existing emails into activities: %w", err)
+	}
+
+	return nil
+}
+
+func Down_20231107115838(tx *sql.Tx) error {
+	return nil
+}

server/datastore/mysql/migrations/tables/20231107115838_AddUserEmailToActivities_test.go

@@ -0,0 +1,48 @@
+package tables
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUp_20231107115838(t *testing.T) {
+	db := applyUpToPrev(t)
+
+	//
+	// Insert data to test the migration
+	//
+	// ...
+
+	userEmails := map[uint]string{1: "admin@email.com", 2: "user1@email.com"}
+
+	setupStmts := `
+		INSERT INTO users VALUES 
+			(1,'2023-11-03 20:32:32','2023-11-03 20:32:32',_binary '$2a$12$n6hwsD7OU2bAXX94551DQOBcNNhfsEPS3Y6JEuLDjsLNvry3lgJjy','0fF81xRQIriYzm5fdXouk3V3tRwsZJhV','admin','admin@email.com',0,'','',0,'admin',0),
+			(2,'2023-11-03 20:33:13','2023-11-03 20:35:26',_binary '$2a$12$YxPPOd5TOmYhDlH5CfGIfuxBe4GJ78gbwvtxoBHTTw.symxpVcEZS','JPDLcBcv4j1QwIU+rHoRWBt3HVJC8hnf','User 1','user1@email.com',0,'','',0,NULL,0);
+		INSERT INTO activities VALUES
+			(1,'2023-11-04 20:32:32',1,'admin','user_logged_in','{"public_ip": "[::1]"}',0),
+			(2,'2023-11-03 20:32:32',2,'User 1','user_logged_in','{"public_ip": "[::1]"}',0);
+	`
+
+	_, err := db.Exec(setupStmts)
+	require.NoError(t, err)
+	// Apply current migration.
+	applyNext(t, db)
+
+	stmt := `
+		SELECT user_id, user_email FROM activities;
+	`
+	rows, err := db.Query(stmt)
+	require.NoError(t, rows.Err())
+	require.NoError(t, err)
+	defer rows.Close()
+
+	for rows.Next() {
+		var userEmail string
+		var id uint
+		err := rows.Scan(&id, &userEmail)
+		require.NoError(t, err)
+		require.Equal(t, userEmails[id], userEmail)
+	}
+}

server/fleet/activities.go

@@ -491,7 +491,7 @@ type Activity struct {
 	ActorFullName *string          `json:"actor_full_name,omitempty" db:"name"`
 	ActorID       *uint            `json:"actor_id,omitempty" db:"user_id"`
 	ActorGravatar *string          `json:"actor_gravatar,omitempty" db:"gravatar_url"`
-	ActorEmail    *string          `json:"actor_email,omitempty" db:"email"`
+	ActorEmail    *string          `json:"actor_email,omitempty" db:"user_email"`
 	Type          string           `json:"type" db:"activity_type"`
 	Details       *json.RawMessage `json:"details" db:"details"`
 	Streamed      *bool            `json:"-" db:"streamed"`

server/service/integration_core_test.go

@@ -268,6 +268,60 @@ func (s *integrationTestSuite) TestQueryCreationLogsActivity() {
 	require.True(t, found)
 }
 
+func (s *integrationTestSuite) TestActivityUserEmailPersistsAfterDeletion() {
+	t := s.T()
+
+	// create a new user
+	var createResp createUserResponse
+	userRawPwd := test.GoodPassword
+	params := fleet.UserPayload{
+		Name:       ptr.String("Gonna B Deleted"),
+		Email:      ptr.String("goingto@delete.com"),
+		Password:   ptr.String(userRawPwd),
+		GlobalRole: ptr.String(fleet.RoleObserver),
+	}
+	s.DoJSON("POST", "/api/latest/fleet/users/admin", params, http.StatusOK, &createResp)
+	assert.NotZero(t, createResp.User.ID)
+	assert.True(t, createResp.User.AdminForcedPasswordReset)
+	u := *createResp.User
+
+	var loginResp loginResponse
+	s.DoJSON("POST", "/api/latest/fleet/login", params, http.StatusOK, &loginResp)
+	require.Equal(t, loginResp.User.ID, u.ID)
+
+	activities := listActivitiesResponse{}
+	s.DoJSON("GET", "/api/latest/fleet/activities", nil, http.StatusOK, &activities)
+
+	assert.GreaterOrEqual(t, len(activities.Activities), 1)
+	found := false
+	for _, activity := range activities.Activities {
+		if activity.Type == "user_logged_in" && *activity.ActorFullName == u.Name {
+			found = true
+			assert.Equal(t, u.Email, *activity.ActorEmail)
+		}
+	}
+	require.True(t, found)
+
+	err := s.ds.DeleteUser(context.Background(), u.ID)
+	require.NoError(t, err)
+
+	activities = listActivitiesResponse{}
+	s.DoJSON("GET", "/api/latest/fleet/activities", nil, http.StatusOK, &activities)
+
+	assert.GreaterOrEqual(t, len(activities.Activities), 1)
+	found = false
+	for _, activity := range activities.Activities {
+		if activity.Type == "user_logged_in" && *activity.ActorFullName == u.Name {
+			found = true
+			assert.Equal(t, u.Email, *activity.ActorEmail)
+		}
+	}
+	require.True(t, found)
+
+	// ensure that on exit, the admin token is used
+	s.token = s.getTestAdminToken()
+}
+
 func (s *integrationTestSuite) TestPolicyDeletionLogsActivity() {
 	t := s.T()