Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 34 Packages Projects Releases Wiki Activity

feat: don't remove user email from activity feed when user deleted (#14975)

Browse Source 
# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [x] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
This commit is contained in:
Jahziel Villasana-Espinoza
2023-11-09 15:50:01 -05:00
committed by GitHub
parent efa9f34f22
commit 447b9e8b25
7 changed files with 163 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/12634-keep-user-email
+2
View File
@@ -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
+19 -3
View File
@@ -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,30 @@ 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"}
args := []any{
userID,
userName,
activity.ActivityName(),
detailsBytes,
}
if userEmail != nil {
args = append(args, userEmail)
cols = append(cols, "user_email")
}
insertStmt := `INSERT INTO activities (%s) VALUES (%s)`
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 +65,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/20231109115838_AddUserEmailToActivities.go
+36
View File
@@ -0,0 +1,36 @@
package tables
import (
"database/sql"
"fmt"
)
func init() {
MigrationClient.AddMigration(Up_20231109115838, Down_20231109115838)
}
func Up_20231109115838(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_20231109115838(tx *sql.Tx) error {
return nil
}
server/datastore/mysql/migrations/tables/20231109115838_AddUserEmailToActivities_test.go
+48
View File
@@ -0,0 +1,48 @@
package tables
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestUp_20231109115838(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/datastore/mysql/schema.sql
+3 -2
View File
File diff suppressed because one or more lines are too long
server/fleet/activities.go
+1 -1
View File
@@ -495,7 +495,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
+54
View File
@@ -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()