exclude settings except in dedicated ds method

fleetdm · Jan 8, 2025 · b842a87 · b842a87
1 parent 394c89b
commit b842a87
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 9 deletions.
diff --git a/server/datastore/mysql/users.go b/server/datastore/mysql/users.go
@@ -75,7 +75,8 @@ func (ds *Datastore) NewUser(ctx context.Context, user *fleet.User) (*fleet.User
 
 func (ds *Datastore) findUser(ctx context.Context, searchCol string, searchVal interface{}) (*fleet.User, error) {
 	sqlStatement := fmt.Sprintf(
-		"SELECT * FROM users "+
+		// everything except `settings`
+		"SELECT id, created_at, updated_at, password, salt, name, email, admin_forced_password_reset, gravatar_url, position, sso_enabled, global_role, api_only, mfa_enabled FROM users "+
 			"WHERE %s = ? LIMIT 1",
 		searchCol,
 	)

diff --git a/server/datastore/mysql/users_test.go b/server/datastore/mysql/users_test.go
@@ -169,15 +169,17 @@ func testSettingsAttribute(t *testing.T, ds fleet.Datastore, users []*fleet.User
 
 		verify, err := ds.UserByID(context.Background(), user.ID)
 		assert.Nil(t, err)
-		assert.Empty(t, verify.Settings.HiddenHostColumns)
+		// settings should only be returned via dedicated method
+		assert.Nil(t, verify.Settings)
 
 		user.Settings.HiddenHostColumns = []string{"osquery_version"}
 		err = ds.SaveUser(context.Background(), user)
 		assert.Nil(t, err)
 
-		verify, err = ds.UserByID(context.Background(), user.ID)
+		// call the settings db method here
+		settings, err := ds.UserSettings(context.Background(), user.ID)
 		assert.Nil(t, err)
-		assert.Equal(t, verify.Settings.HiddenHostColumns, user.Settings.HiddenHostColumns)
+		assert.Equal(t, settings.HiddenHostColumns, user.Settings.HiddenHostColumns)
 	}
 }
 

diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go
@@ -4671,7 +4671,9 @@ func (s *integrationTestSuite) TestUsers() {
 	// session user id 1
 	assert.Equal(t, uint(1), getMeResp.User.ID)
 	assert.NotNil(t, getMeResp.User.GlobalRole)
-	assert.Empty(t, getMeResp.User.Settings)
+	// settings should only be present in dedicated settings field, not in user object
+	assert.Nil(t, getMeResp.User.Settings)
+	assert.Empty(t, getMeResp.Settings)
 
 	// modify session user - add ui setting
 	var modResp modifyUserResponse
@@ -4683,7 +4685,8 @@ func (s *integrationTestSuite) TestUsers() {
 	// get session user with ui settings, should now be present, two endpoints
 	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/users/%d", 1), nil, http.StatusOK, &getResp, "include_ui_settings", "true")
 	assert.Equal(t, uint(1), getResp.User.ID)
-	assert.Equal(t, getResp.User.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"osquery_version"}})
+	assert.Nil(t, getMeResp.User.Settings)
+	assert.Equal(t, getResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"osquery_version"}})
 
 	resp = s.DoRawWithHeaders("GET", "/api/latest/fleet/me", []byte(""), http.StatusOK, map[string]string{
 		"Authorization": fmt.Sprintf("Bearer %s", ssn.Key),
@@ -4692,7 +4695,8 @@ func (s *integrationTestSuite) TestUsers() {
 	require.NoError(t, err)
 	assert.Equal(t, uint(1), getMeResp.User.ID)
 	assert.NotNil(t, getMeResp.User.GlobalRole)
-	assert.Equal(t, getResp.User.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"osquery_version"}})
+	assert.Nil(t, getMeResp.User.Settings)
+	assert.Equal(t, getResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"osquery_version"}})
 
 	// modify user ui settings, check they are returned modified
 	s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/users/%d", 1), json.RawMessage(`{
@@ -4703,7 +4707,8 @@ func (s *integrationTestSuite) TestUsers() {
 	// get session user with ui settings, should now be present, two endpoints
 	s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/users/%d", 1), nil, http.StatusOK, &getResp, "include_ui_settings", "true")
 	assert.Equal(t, uint(1), getResp.User.ID)
-	assert.Equal(t, getResp.User.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"hostname", "osquery_version"}})
+	assert.Nil(t, getResp.User.Settings)
+	assert.Equal(t, getResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"hostname", "osquery_version"}})
 
 	resp = s.DoRawWithHeaders("GET", "/api/latest/fleet/me", []byte(""), http.StatusOK, map[string]string{
 		"Authorization": fmt.Sprintf("Bearer %s", ssn.Key),
@@ -4712,7 +4717,8 @@ func (s *integrationTestSuite) TestUsers() {
 	require.NoError(t, err)
 	assert.Equal(t, uint(1), getMeResp.User.ID)
 	assert.NotNil(t, getMeResp.User.GlobalRole)
-	assert.Equal(t, getResp.User.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"hostname", "osquery_version"}})
+	assert.Nil(t, getResp.User.Settings)
+	assert.Equal(t, getResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"hostname", "osquery_version"}})
 
 	// create a new user
 	var createResp createUserResponse