Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API, datastore, migration for new "user settings", with `"hidden_hosts_table_columns" setting #25184

Merged
merged 25 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package tables

import (
"database/sql"
"fmt"
)

func init() {
MigrationClient.AddMigration(Up_20250107165731, Down_20250107165731)
}

func Up_20250107165731(tx *sql.Tx) error {
_, err := tx.Exec(`ALTER TABLE users ADD COLUMN settings json NOT NULL DEFAULT (JSON_OBJECT())`)
if err != nil {
return fmt.Errorf("failed to add settings to users: %w", err)
}
return nil
}

func Down_20250107165731(tx *sql.Tx) error {
return nil
}
5 changes: 3 additions & 2 deletions server/datastore/mysql/schema.sql

Large diffs are not rendered by default.

38 changes: 37 additions & 1 deletion server/datastore/mysql/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package mysql
import (
"context"
"database/sql"
"encoding/json"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -74,7 +75,11 @@ 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`. Since we only want to include user settings on an opt-in basis
// from the API perspective (see `include_ui_settings` query param on `GET` `/me` and `GET` `/users/:id`), excluding it here ensures it's only included in API responses
// when explicitly coded to be, via calling the dedicated UserSettings method. Otherwise,
// `settings` would be included in `user` objects in various places, which we do not want.
"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 "+
jacobshandling marked this conversation as resolved.
Show resolved Hide resolved
"WHERE %s = ? LIMIT 1",
searchCol,
)
Expand Down Expand Up @@ -172,9 +177,14 @@ func saveUserDB(ctx context.Context, tx sqlx.ExtContext, user *fleet.User) error
sso_enabled = ?,
mfa_enabled = ?,
api_only = ?,
settings = ?,
global_role = ?
WHERE id = ?
`
settingsBytes, err := json.Marshal(user.Settings)
if err != nil {
return ctxerr.Wrap(ctx, err, "marshaling user settings")
}
result, err := tx.ExecContext(ctx, sqlStatement,
user.Password,
user.Salt,
Expand All @@ -186,6 +196,7 @@ func saveUserDB(ctx context.Context, tx sqlx.ExtContext, user *fleet.User) error
user.SSOEnabled,
user.MFAEnabled,
user.APIOnly,
settingsBytes,
user.GlobalRole,
user.ID)
if err != nil {
Expand Down Expand Up @@ -310,3 +321,28 @@ func amountActiveUsersSinceDB(ctx context.Context, db sqlx.QueryerContext, since
}
return amount, nil
}

func (ds *Datastore) UserSettings(ctx context.Context, userID uint) (*fleet.UserSettings, error) {
settings := &fleet.UserSettings{}
var bytes []byte
stmt := `
SELECT settings FROM users WHERE id = ?
`
err := sqlx.GetContext(ctx, ds.reader(ctx), &bytes, stmt, userID)
if err != nil {
if err == sql.ErrNoRows {
return nil, ctxerr.Wrap(ctx, notFound("UserSettings").WithID(userID))
}
return nil, ctxerr.Wrap(ctx, err, "selecting user settings")
}

if len(bytes) == 0 {
return settings, nil
}

err = json.Unmarshal(bytes, settings)
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "unmarshaling user settings")
}
return settings, nil
}
23 changes: 23 additions & 0 deletions server/datastore/mysql/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func testUsersSave(t *testing.T, ds *Datastore) {
testEmailAttribute(t, ds, users)
testPasswordAttribute(t, ds, users)
testMFAAttribute(t, ds, users)
testSettingsAttribute(t, ds, users)
}

func testMFAAttribute(t *testing.T, ds fleet.Datastore, users []*fleet.User) {
Expand All @@ -160,6 +161,28 @@ func testMFAAttribute(t *testing.T, ds fleet.Datastore, users []*fleet.User) {
}
}

func testSettingsAttribute(t *testing.T, ds fleet.Datastore, users []*fleet.User) {
for _, user := range users {
user.Settings = &fleet.UserSettings{}
err := ds.SaveUser(context.Background(), user)
assert.Nil(t, err)

verify, err := ds.UserByID(context.Background(), user.ID)
assert.Nil(t, err)
// 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)

// call the settings db method here
settings, err := ds.UserSettings(context.Background(), user.ID)
assert.Nil(t, err)
assert.Equal(t, settings.HiddenHostColumns, user.Settings.HiddenHostColumns)
}
}

func testPasswordAttribute(t *testing.T, ds fleet.Datastore, users []*fleet.User) {
for _, user := range users {
randomText, err := server.GenerateRandomText(8) // GenerateRandomText(8)
Expand Down
2 changes: 2 additions & 0 deletions server/fleet/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ type Datastore interface {
// written to user record. userID is the ID of the user whose e-mail is being changed.
ConfirmPendingEmailChange(ctx context.Context, userID uint, token string) (string, error)

UserSettings(ctx context.Context, userID uint) (*UserSettings, error)

///////////////////////////////////////////////////////////////////////////////
// QueryStore

Expand Down
2 changes: 2 additions & 0 deletions server/fleet/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ type Service interface {
// write the new email address to user.
ChangeUserEmail(ctx context.Context, token string) (string, error)

GetUserSettings(ctx context.Context, id uint) (settings *UserSettings, err error)

// /////////////////////////////////////////////////////////////////////////////
// Session

Expand Down
48 changes: 34 additions & 14 deletions server/fleet/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,25 @@ type User struct {

// Teams is the teams this user has roles in. For users with a global role, Teams is expected to be empty.
Teams []UserTeam `json:"teams"`

Settings *UserSettings `json:"settings,omitempty"`
}

type UserSettings struct {
HiddenHostColumns []string `json:"hidden_host_columns"`
}

// Scan implements the sql.Scanner interface, tells DB driver how to convert MySQL type (json) to
// custom Go type (UserSettings).
func (us *UserSettings) Scan(val interface{}) error {
switch v := val.(type) {
case []byte:
return json.Unmarshal(v, us)
case string:
return json.Unmarshal([]byte(v), us)
default:
return fmt.Errorf("unsupported type: %T", v)
}
}

// IsGlobalObserver returns true if user is either a Global Observer or a Global Observer+
Expand Down Expand Up @@ -149,20 +168,21 @@ type UserListOptions struct {

// UserPayload is used to modify an existing user
type UserPayload struct {
Name *string `json:"name,omitempty"`
Email *string `json:"email,omitempty"`
Password *string `json:"password,omitempty"`
GravatarURL *string `json:"gravatar_url,omitempty"`
Position *string `json:"position,omitempty"`
InviteToken *string `json:"invite_token,omitempty"`
SSOInvite *bool `json:"sso_invite,omitempty"`
MFAEnabled *bool `json:"mfa_enabled,omitempty"`
SSOEnabled *bool `json:"sso_enabled,omitempty"`
GlobalRole *string `json:"global_role,omitempty"`
AdminForcedPasswordReset *bool `json:"admin_forced_password_reset,omitempty"`
APIOnly *bool `json:"api_only,omitempty"`
Teams *[]UserTeam `json:"teams,omitempty"`
NewPassword *string `json:"new_password,omitempty"`
Name *string `json:"name,omitempty"`
Email *string `json:"email,omitempty"`
Password *string `json:"password,omitempty"`
GravatarURL *string `json:"gravatar_url,omitempty"`
Position *string `json:"position,omitempty"`
InviteToken *string `json:"invite_token,omitempty"`
SSOInvite *bool `json:"sso_invite,omitempty"`
MFAEnabled *bool `json:"mfa_enabled,omitempty"`
SSOEnabled *bool `json:"sso_enabled,omitempty"`
GlobalRole *string `json:"global_role,omitempty"`
AdminForcedPasswordReset *bool `json:"admin_forced_password_reset,omitempty"`
APIOnly *bool `json:"api_only,omitempty"`
Teams *[]UserTeam `json:"teams,omitempty"`
NewPassword *string `json:"new_password,omitempty"`
Settings *UserSettings `json:"settings,omitempty"`
}

func (p *UserPayload) VerifyInviteCreate() error {
Expand Down
12 changes: 12 additions & 0 deletions server/mock/datastore_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ type UserByEmailFunc func(ctx context.Context, email string) (*fleet.User, error

type UserByIDFunc func(ctx context.Context, id uint) (*fleet.User, error)

type UserSettingsFunc func(ctx context.Context, userID uint) (*fleet.UserSettings, error)

type SaveUserFunc func(ctx context.Context, user *fleet.User) error

type SaveUsersFunc func(ctx context.Context, users []*fleet.User) error
Expand Down Expand Up @@ -1236,6 +1238,9 @@ type DataStore struct {
UserByIDFunc UserByIDFunc
UserByIDFuncInvoked bool

UserSettingsFunc UserSettingsFunc
UserSettingsFuncInvoked bool

SaveUserFunc SaveUserFunc
SaveUserFuncInvoked bool

Expand Down Expand Up @@ -3053,6 +3058,13 @@ func (s *DataStore) UserByID(ctx context.Context, id uint) (*fleet.User, error)
return s.UserByIDFunc(ctx, id)
}

func (s *DataStore) UserSettings(ctx context.Context, userID uint) (*fleet.UserSettings, error) {
s.mu.Lock()
s.UserSettingsFuncInvoked = true
s.mu.Unlock()
return s.UserSettingsFunc(ctx, userID)
}

func (s *DataStore) SaveUser(ctx context.Context, user *fleet.User) error {
s.mu.Lock()
s.SaveUserFuncInvoked = true
Expand Down
2 changes: 1 addition & 1 deletion server/service/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func attachFleetAPIRoutes(r *mux.Router, svc fleet.Service, config config.FleetC

ue.POST("/api/_version_/fleet/trigger", triggerEndpoint, triggerRequest{})

ue.GET("/api/_version_/fleet/me", meEndpoint, nil)
ue.GET("/api/_version_/fleet/me", meEndpoint, getMeRequest{})
ue.GET("/api/_version_/fleet/sessions/{id:[0-9]+}", getInfoAboutSessionEndpoint, getInfoAboutSessionRequest{})
ue.DELETE("/api/_version_/fleet/sessions/{id:[0-9]+}", deleteSessionEndpoint, deleteSessionRequest{})

Expand Down
89 changes: 87 additions & 2 deletions server/service/integration_core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4655,6 +4655,93 @@ func (s *integrationTestSuite) TestUsers() {
assert.Len(t, getMeResp.User.Teams, 0)
assert.Len(t, getMeResp.AvailableTeams, 0)

// test user settings from 2 endpoints

// get session user with ui settings, which should be empty, two endpoints
var getResp getUserResponse
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.Empty(t, getResp.User.Settings)

resp = s.DoRawWithHeaders("GET", "/api/latest/fleet/me", []byte(""), http.StatusOK, map[string]string{
"Authorization": fmt.Sprintf("Bearer %s", ssn.Key),
}, "include_ui_settings", "true")
err = json.NewDecoder(resp.Body).Decode(&getMeResp)
require.NoError(t, err)
// session user id 1
assert.Equal(t, uint(1), getMeResp.User.ID)
assert.NotNil(t, getMeResp.User.GlobalRole)
// 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
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/users/%d", 1), json.RawMessage(`{
"settings": {
"hidden_host_columns": ["osquery_version"]}
}`), http.StatusOK, &modResp)

// 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.Nil(t, getResp.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),
}, "include_ui_settings", "true")
err = json.NewDecoder(resp.Body).Decode(&getMeResp)
require.NoError(t, err)
assert.Equal(t, uint(1), getMeResp.User.ID)
assert.NotNil(t, getMeResp.User.GlobalRole)
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(`{
"settings": {
"hidden_host_columns": ["hostname", "osquery_version"]}
}`), http.StatusOK, &modResp)

// get session user with ui settings, should now be modified, 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.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),
}, "include_ui_settings", "true")
err = json.NewDecoder(resp.Body).Decode(&getMeResp)
require.NoError(t, err)
assert.Equal(t, uint(1), getMeResp.User.ID)
assert.NotNil(t, getMeResp.User.GlobalRole)
assert.Nil(t, getMeResp.User.Settings)
assert.Equal(t, getMeResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{"hostname", "osquery_version"}})

// modify user ui settings, empty array, check they are returned correctly
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/users/%d", 1), json.RawMessage(`{
"settings": {
"hidden_host_columns": []}
}`), http.StatusOK, &modResp)

// get session user with ui settings, should now be modified, 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.Nil(t, getResp.User.Settings)
jacobshandling marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, getResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{}})

resp = s.DoRawWithHeaders("GET", "/api/latest/fleet/me", []byte(""), http.StatusOK, map[string]string{
"Authorization": fmt.Sprintf("Bearer %s", ssn.Key),
}, "include_ui_settings", "true")
err = json.NewDecoder(resp.Body).Decode(&getMeResp)
require.NoError(t, err)
assert.Equal(t, uint(1), getMeResp.User.ID)
assert.NotNil(t, getMeResp.User.GlobalRole)
assert.Nil(t, getMeResp.User.Settings)
assert.Equal(t, getMeResp.Settings, &fleet.UserSettings{HiddenHostColumns: []string{}})

jacobshandling marked this conversation as resolved.
Show resolved Hide resolved
// create a new user
var createResp createUserResponse
userRawPwd := test.GoodPassword
Expand Down Expand Up @@ -4712,7 +4799,6 @@ func (s *integrationTestSuite) TestUsers() {
s.DoJSONWithoutAuth("POST", "/api/latest/fleet/sessions", sessionCreateRequest{Token: mfaToken}, http.StatusUnauthorized, &loginResp)

// turn off MFA
var modResp modifyUserResponse
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/users/%d", u.ID), fleet.UserPayload{MFAEnabled: ptr.Bool(false)}, http.StatusOK, &modResp)
require.False(t, modResp.User.MFAEnabled)

Expand All @@ -4726,7 +4812,6 @@ func (s *integrationTestSuite) TestUsers() {
assert.Len(t, loginResp.AvailableTeams, 0)

// get that user from `/users` endpoint and check that teams info is empty
var getResp getUserResponse
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/users/%d", u.ID), nil, http.StatusOK, &getResp)
assert.Equal(t, u.ID, getResp.User.ID)
assert.Len(t, getResp.User.Teams, 0)
Expand Down
Loading
Loading