Filter out non-observer_can_run queries for observers in fleetctl get queries command to match the UI. (#11251)

#11089 - [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. - ~[ ] Documented any API changes (docs/Using-Fleet/REST-API.md or docs/Contributing/API-for-contributors.md)~ - [X] Documented any permissions changes - ~[ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements)~ - ~[ ] Added support on fleet's osquery simulator `cmd/osquery-perf` for new osquery data ingestion features.~ - [x] Added/updated tests - [X] Manual QA for all new/changed functionality - ~For Orbit and Fleet Desktop changes:~ - ~[ ] Manual QA must be performed in the three main OSs, macOS, Windows and Linux.~ - ~[ ] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)).~
2024-11-06 00:45:19 +00:00 · 2023-04-26 11:38:20 -03:00 · 2023-04-26 11:38:20 -03:00 · b9e6a84f24
commit b9e6a84f24
parent 77855a5e1d
6 changed files with 396 additions and 12 deletions
--- a/changes/11089-filter-fleetctl-get-queries-for-observers
+++ b/changes/11089-filter-fleetctl-get-queries-for-observers
@ -0,0 +1 @@
+* Filter out non-`observer_can_run` queries for observers in `fleetctl get queries` (to match the UI behavior).
--- a/cmd/fleetctl/get.go
+++ b/cmd/fleetctl/get.go
@ -324,6 +324,30 @@ func getQueriesCommand() *cli.Command {
 					return fmt.Errorf("could not list queries: %w", err)
 				}

+				me, err := client.Me()
+				if err != nil {
+					return err
+				}
+				if me == nil {
+					return errors.New("/api/latest/fleet/me returned an empty user")
+				}
+				ok, err := userIsObserver(*me)
+				if err != nil {
+					return err
+				}
+				if ok {
+					// Filter out queries (in-place) that a observer user
+					// cannot execute (this behavior matches the UI).
+					n := 0
+					for _, query := range queries {
+						if query.ObserverCanRun {
+							queries[n] = query
+							n++
+						}
+					}
+					queries = queries[:n]
+				}
+
 				if len(queries) == 0 {
 					fmt.Println("No queries found")
 					return nil
@ -331,7 +355,11 @@ func getQueriesCommand() *cli.Command {

 				if c.Bool(yamlFlagName) || c.Bool(jsonFlagName) {
 					for _, query := range queries {
-						if err := printQuery(c, query); err != nil {
+						if err := printQuery(c, &fleet.QuerySpec{
+							Name:        query.Name,
+							Description: query.Description,
+							Query:       query.Query,
+						}); err != nil {
 							return fmt.Errorf("unable to print query: %w", err)
 						}
 					}
@ -367,6 +395,28 @@ func getQueriesCommand() *cli.Command {
 	}
 }

+var errUserNoRoles = errors.New("user does not have roles")
+
+// userIsObserver returns whether the user is a global/team observer/observer+.
+// In the case of user belonging to multiple teams, a user is considered observer
+// if it is observer of all teams.
+//
+// Returns errUserNoRoles if the user does not have any roles.
+func userIsObserver(user fleet.User) (bool, error) {
+	if user.GlobalRole != nil {
+		return *user.GlobalRole == fleet.RoleObserver || *user.GlobalRole == fleet.RoleObserverPlus, nil
+	} // Team user
+	if len(user.Teams) == 0 {
+		return false, errUserNoRoles
+	}
+	for _, team := range user.Teams {
+		if team.Role != fleet.RoleObserver && team.Role != fleet.RoleObserverPlus {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
 func getPacksCommand() *cli.Command {
 	return &cli.Command{
 		Name:    "packs",
@ -418,7 +468,11 @@ func getPacksCommand() *cli.Command {
 						continue
 					}

-					if err := printQuery(c, query); err != nil {
+					if err := printQuery(c, &fleet.QuerySpec{
+						Name:        query.Name,
+						Description: query.Description,
+						Query:       query.Query,
+					}); err != nil {
 						return fmt.Errorf("unable to print query: %w", err)
 					}
 				}
--- a/cmd/fleetctl/get_test.go
+++ b/cmd/fleetctl/get_test.go
@ -1053,6 +1053,245 @@ spec:
 	assert.Equal(t, expectedJson, runAppForTest(t, []string{"get", "query", "--json", "query1"}))
 }

+// TestGetQueriesAsObservers tests that when observers run `fleectl get queries` they
+// only get queries that they can execute.
+func TestGetQueriesAsObserver(t *testing.T) {
+	_, ds := runServerWithMockedDS(t)
+
+	setCurrentUserSession := func(user *fleet.User) {
+		user, err := ds.NewUser(context.Background(), user)
+		require.NoError(t, err)
+		ds.SessionByKeyFunc = func(ctx context.Context, key string) (*fleet.Session, error) {
+			return &fleet.Session{
+				CreateTimestamp: fleet.CreateTimestamp{CreatedAt: time.Now()},
+				ID:              1,
+				AccessedAt:      time.Now(),
+				UserID:          user.ID,
+				Key:             key,
+			}, nil
+		}
+		ds.UserByIDFunc = func(ctx context.Context, id uint) (*fleet.User, error) {
+			return user, nil
+		}
+	}
+
+	ds.ListQueriesFunc = func(ctx context.Context, opt fleet.ListQueryOptions) ([]*fleet.Query, error) {
+		return []*fleet.Query{
+			{
+				ID:             42,
+				Name:           "query1",
+				Description:    "some desc",
+				Query:          "select 1;",
+				ObserverCanRun: false,
+			},
+			{
+				ID:             43,
+				Name:           "query2",
+				Description:    "some desc 2",
+				Query:          "select 2;",
+				ObserverCanRun: true,
+			},
+			{
+				ID:             44,
+				Name:           "query3",
+				Description:    "some desc 3",
+				Query:          "select 3;",
+				ObserverCanRun: false,
+			},
+		}, nil
+	}
+
+	for _, tc := range []struct {
+		name string
+		user *fleet.User
+	}{
+		{
+			name: "global observer",
+			user: &fleet.User{
+				ID:         1,
+				Name:       "Global observer",
+				Password:   []byte("p4ssw0rd.123"),
+				Email:      "go@example.com",
+				GlobalRole: ptr.String(fleet.RoleObserverPlus),
+			},
+		},
+		{
+			name: "team observer",
+			user: &fleet.User{
+				ID:         2,
+				Name:       "Team observer",
+				Password:   []byte("p4ssw0rd.123"),
+				Email:      "tm@example.com",
+				GlobalRole: nil,
+				Teams:      []fleet.UserTeam{{Role: fleet.RoleObserver}},
+			},
+		},
+		{
+			name: "observer of multiple teams",
+			user: &fleet.User{
+				ID:         3,
+				Name:       "Observer of multiple teams",
+				Password:   []byte("p4ssw0rd.123"),
+				Email:      "omt@example.com",
+				GlobalRole: nil,
+				Teams: []fleet.UserTeam{
+					{
+						Team: fleet.Team{ID: 1},
+						Role: fleet.RoleObserver,
+					},
+					{
+						Team: fleet.Team{ID: 2},
+						Role: fleet.RoleObserverPlus,
+					},
+				},
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			setCurrentUserSession(tc.user)
+
+			expected := `+--------+-------------+-----------+
+|  NAME  | DESCRIPTION |   QUERY   |
+--------+-------------+-----------+
+| query2 | some desc 2 | select 2; |
+--------+-------------+-----------+
+`
+			expectedYaml := `---
+apiVersion: v1
+kind: query
+spec:
+  description: some desc 2
+  name: query2
+  query: select 2;
+`
+			expectedJson := `{"kind":"query","apiVersion":"v1","spec":{"name":"query2","description":"some desc 2","query":"select 2;"}}
+`
+
+			assert.Equal(t, expected, runAppForTest(t, []string{"get", "queries"}))
+			assert.Equal(t, expectedYaml, runAppForTest(t, []string{"get", "queries", "--yaml"}))
+			assert.Equal(t, expectedJson, runAppForTest(t, []string{"get", "queries", "--json"}))
+		})
+	}
+
+	// Test with a user that is observer of a team, but maintainer of another team (should not filter the queries).
+	setCurrentUserSession(&fleet.User{
+		ID:         4,
+		Name:       "Not observer of all teams",
+		Password:   []byte("p4ssw0rd.123"),
+		Email:      "omt2@example.com",
+		GlobalRole: nil,
+		Teams: []fleet.UserTeam{
+			{
+				Team: fleet.Team{ID: 1},
+				Role: fleet.RoleObserver,
+			},
+			{
+				Team: fleet.Team{ID: 2},
+				Role: fleet.RoleMaintainer,
+			},
+		},
+	})
+
+	expected := `+--------+-------------+-----------+
+|  NAME  | DESCRIPTION |   QUERY   |
+--------+-------------+-----------+
+| query1 | some desc   | select 1; |
+--------+-------------+-----------+
+| query2 | some desc 2 | select 2; |
+--------+-------------+-----------+
+| query3 | some desc 3 | select 3; |
+--------+-------------+-----------+
+`
+	expectedYaml := `---
+apiVersion: v1
+kind: query
+spec:
+  description: some desc
+  name: query1
+  query: select 1;
+---
+apiVersion: v1
+kind: query
+spec:
+  description: some desc 2
+  name: query2
+  query: select 2;
+---
+apiVersion: v1
+kind: query
+spec:
+  description: some desc 3
+  name: query3
+  query: select 3;
+`
+	expectedJson := `{"kind":"query","apiVersion":"v1","spec":{"name":"query1","description":"some desc","query":"select 1;"}}
+{"kind":"query","apiVersion":"v1","spec":{"name":"query2","description":"some desc 2","query":"select 2;"}}
+{"kind":"query","apiVersion":"v1","spec":{"name":"query3","description":"some desc 3","query":"select 3;"}}
+`
+
+	assert.Equal(t, expected, runAppForTest(t, []string{"get", "queries"}))
+	assert.Equal(t, expectedYaml, runAppForTest(t, []string{"get", "queries", "--yaml"}))
+	assert.Equal(t, expectedJson, runAppForTest(t, []string{"get", "queries", "--json"}))
+
+	// No queries are returned if none is observer_can_run.
+	setCurrentUserSession(&fleet.User{
+		ID:         2,
+		Name:       "Team observer",
+		Password:   []byte("p4ssw0rd.123"),
+		Email:      "tm@example.com",
+		GlobalRole: nil,
+		Teams:      []fleet.UserTeam{{Role: fleet.RoleObserver}},
+	})
+	ds.ListQueriesFunc = func(ctx context.Context, opt fleet.ListQueryOptions) ([]*fleet.Query, error) {
+		return []*fleet.Query{
+			{
+				ID:             42,
+				Name:           "query1",
+				Description:    "some desc",
+				Query:          "select 1;",
+				ObserverCanRun: false,
+			},
+			{
+				ID:             43,
+				Name:           "query2",
+				Description:    "some desc 2",
+				Query:          "select 2;",
+				ObserverCanRun: false,
+			},
+		}, nil
+	}
+	assert.Equal(t, "", runAppForTest(t, []string{"get", "queries"}))
+
+	// No filtering is performed if all are observer_can_run.
+	ds.ListQueriesFunc = func(ctx context.Context, opt fleet.ListQueryOptions) ([]*fleet.Query, error) {
+		return []*fleet.Query{
+			{
+				ID:             42,
+				Name:           "query1",
+				Description:    "some desc",
+				Query:          "select 1;",
+				ObserverCanRun: true,
+			},
+			{
+				ID:             43,
+				Name:           "query2",
+				Description:    "some desc 2",
+				Query:          "select 2;",
+				ObserverCanRun: true,
+			},
+		}, nil
+	}
+	expected = `+--------+-------------+-----------+
+|  NAME  | DESCRIPTION |   QUERY   |
+--------+-------------+-----------+
+| query1 | some desc   | select 1; |
+--------+-------------+-----------+
+| query2 | some desc 2 | select 2; |
+--------+-------------+-----------+
+`
+	assert.Equal(t, expected, runAppForTest(t, []string{"get", "queries"}))
+}
+
 func TestEnrichedAppConfig(t *testing.T) {
 	t.Run("deprecated fields", func(t *testing.T) {
 		resp := []byte(`
@ -1616,3 +1855,80 @@ func TestGetMDMCommands(t *testing.T) {
 +----+----------------------+-------------+--------------+----------+
 `))
 }
+
+func TestUserIsObserver(t *testing.T) {
+	for _, tc := range []struct {
+		name        string
+		user        fleet.User
+		expectedVal bool
+		expectedErr error
+	}{
+		{
+			name:        "user without roles",
+			user:        fleet.User{},
+			expectedErr: errUserNoRoles,
+		},
+		{
+			name:        "global observer",
+			user:        fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)},
+			expectedVal: true,
+		},
+		{
+			name:        "global observer+",
+			user:        fleet.User{GlobalRole: ptr.String(fleet.RoleObserverPlus)},
+			expectedVal: true,
+		},
+		{
+			name:        "global maintainer",
+			user:        fleet.User{GlobalRole: ptr.String(fleet.RoleMaintainer)},
+			expectedVal: false,
+		},
+		{
+			name: "team observer",
+			user: fleet.User{
+				GlobalRole: nil,
+				Teams: []fleet.UserTeam{
+					{Role: fleet.RoleObserver},
+				},
+			},
+			expectedVal: true,
+		},
+		{
+			name: "team observer+",
+			user: fleet.User{
+				GlobalRole: nil,
+				Teams: []fleet.UserTeam{
+					{Role: fleet.RoleObserverPlus},
+				},
+			},
+			expectedVal: true,
+		},
+		{
+			name: "team maintainer",
+			user: fleet.User{
+				GlobalRole: nil,
+				Teams: []fleet.UserTeam{
+					{Role: fleet.RoleMaintainer},
+				},
+			},
+			expectedVal: false,
+		},
+		{
+			name: "team observer and maintainer",
+			user: fleet.User{
+				GlobalRole: nil,
+				Teams: []fleet.UserTeam{
+					{Role: fleet.RoleObserver},
+					{Role: fleet.RoleMaintainer},
+				},
+			},
+			expectedVal: false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := userIsObserver(tc.user)
+			require.Equal(t, tc.expectedErr, err)
+			require.Equal(t, tc.expectedVal, actual)
+		})
+	}
+}
--- a/docs/Using-Fleet/Permissions.md
+++ b/docs/Using-Fleet/Permissions.md
@ -47,10 +47,10 @@ GitOps is an API-only and write-only role that can be used on CI/CD pipelines.
 | Filter hosts by software                                                                                                                   | ✅        | ✅         | ✅          | ✅     |        |
 | Filter software by team\*                                                                                                                  | ✅        | ✅         | ✅          | ✅     |        |
 | Manage [vulnerability automations](https://fleetdm.com/docs/using-fleet/automations#vulnerability-automations)                             |          |           |            | ✅     | ✅      |
-| Run only designated, **observer can run** ,queries as live queries against all hosts                                                       | ✅        | ✅         | ✅          | ✅     |        |
+| Run only designated, **observer can run**, queries as live queries against all hosts                                                       | ✅        | ✅         | ✅          | ✅     |        |
 | Run any query as [live query](https://fleetdm.com/docs/using-fleet/fleet-ui#run-a-query) against all hosts                                 |          | ✅         | ✅          | ✅     |        |
 | Create, edit, and delete queries                                                                                                           |          |           | ✅          | ✅     | ✅      |
-| View all queries                                                                                                                           | ✅        | ✅         | ✅          | ✅     |        |
+| View all queries\**                                                                                                                        | ✅        | ✅         | ✅          | ✅     |        |
 | Add, edit, and remove queries from all schedules                                                                                           |          |           | ✅          | ✅     | ✅      |
 | Create, edit, view, and delete packs                                                                                                       |          |           | ✅          | ✅     | ✅      |
 | View all policies                                                                                                                          | ✅        | ✅         | ✅          | ✅     |        |
@ -63,7 +63,7 @@ GitOps is an API-only and write-only role that can be used on CI/CD pipelines.
 | Create, edit, and delete teams\*                                                                                                           |          |           |            | ✅     | ✅      |
 | Create, edit, and delete [enroll secrets](https://fleetdm.com/docs/deploying/faq#when-do-i-need-to-deploy-a-new-enroll-secret-to-my-hosts) |          |           | ✅          | ✅     | ✅      |
 | Create, edit, and delete [enroll secrets for teams](https://fleetdm.com/docs/using-fleet/rest-api#get-enroll-secrets-for-a-team)\*         |          |           | ✅          | ✅     |        |
-| Read organization settings and agent options\**                                                                                            | ✅        | ✅         | ✅          | ✅     |        |
+| Read organization settings and agent options\***                                                                                           | ✅        | ✅         | ✅          | ✅     |        |
 | Edit [organization settings](https://fleetdm.com/docs/using-fleet/configuration-files#organization-settings)                               |          |           |            | ✅     | ✅      |
 | Edit [agent options](https://fleetdm.com/docs/using-fleet/configuration-files#agent-options)                                               |          |           |            | ✅     | ✅      |
 | Edit [agent options for hosts assigned to teams](https://fleetdm.com/docs/using-fleet/configuration-files#team-agent-options)\*            |          |           |            | ✅     | ✅      |
@ -81,9 +81,11 @@ GitOps is an API-only and write-only role that can be used on CI/CD pipelines.
 | View/download MDM macOS setup assistant\*                                                                                                  |          |           | ✅          | ✅     |        |
 | Edit/upload MDM macOS setup assistant\*                                                                                                    |          |           | ✅          | ✅     | ✅     |

-\*Applies only to Fleet Premium
+\* Applies only to Fleet Premium

-\** Applies only to [Fleet REST API](https://fleetdm.com/docs/using-fleet/rest-api)
+\** Global observers can view all queries but the UI and fleetctl only list the ones they can run (**observer can run**).
+
+\*** Applies only to [Fleet REST API](https://fleetdm.com/docs/using-fleet/rest-api)

 ## Team member permissions

@ -111,9 +113,10 @@ Users that are members of multiple teams can be assigned different roles for eac
 | Filter software by [vulnerabilities](<(https://fleetdm.com/docs/using-fleet/vulnerability-processing#vulnerability-processing)>) | ✅             | ✅              | ✅               | ✅          |             |
 | Filter hosts by software                                                                                                         | ✅             | ✅              | ✅               | ✅          |             |
 | Filter software                                                                                                                  | ✅             | ✅              | ✅               | ✅          |             |
-| Run only designated, **observer can run** ,queries as live queries against all hosts                                             | ✅             | ✅              | ✅               | ✅          |             |
+| Run only designated, **observer can run**, queries as live queries against all hosts                                             | ✅             | ✅              | ✅               | ✅          |             |
 | Run any query as [live query](https://fleetdm.com/docs/using-fleet/fleet-ui#run-a-query)                                         |               | ✅              | ✅               | ✅          |             |
 | Create, edit, and delete only **self authored** queries                                                                          |               |                | ✅               | ✅          | ✅           |
+| View all queries\**                                                                                                              | ✅             | ✅              | ✅               | ✅          |             |
 | Add, edit, and remove queries from the schedule                                                                                  |               |                | ✅               | ✅          | ✅           |
 | View policies                                                                                                                    | ✅             | ✅              | ✅               | ✅          |             |
 | View global (inherited) policies                                                                                                 | ✅             | ✅              | ✅               | ✅          |             |
@ -137,4 +140,6 @@ Users that are members of multiple teams can be assigned different roles for eac

 \* Applies only to [Fleet REST API](https://fleetdm.com/docs/using-fleet/rest-api)

+\** Team observers can view all queries but the UI and fleetctl only list the ones they can run (**observer can run**).
+
 <meta name="pageOrderInSection" value="900">
--- a/server/service/client_queries.go
+++ b/server/service/client_queries.go
@ -24,11 +24,11 @@ func (c *Client) GetQuery(name string) (*fleet.QuerySpec, error) {
 }

 // GetQueries retrieves the list of all Queries.
-func (c *Client) GetQueries() ([]*fleet.QuerySpec, error) {
-	verb, path := "GET", "/api/latest/fleet/spec/queries"
-	var responseBody getQuerySpecsResponse
+func (c *Client) GetQueries() ([]fleet.Query, error) {
+	verb, path := "GET", "/api/latest/fleet/queries"
+	var responseBody listQueriesResponse
 	err := c.authenticatedRequest(nil, verb, path, &responseBody)
-	return responseBody.Specs, err
+	return responseBody.Queries, err
 }

 // DeleteQuery deletes the query with the matching name.
--- a/server/service/client_users.go
+++ b/server/service/client_users.go
@ -67,3 +67,11 @@ func (c *Client) DeleteUser(email string) error {
 	var responseBody deleteUserResponse
 	return c.authenticatedRequest(nil, verb, path, &responseBody)
 }
+
+// Me returns the user associated with the current session.
+func (c *Client) Me() (*fleet.User, error) {
+	verb, path := "GET", "/api/latest/fleet/me"
+	var responseBody getUserResponse
+	err := c.authenticatedRequest(nil, verb, path, &responseBody)
+	return responseBody.User, err
+}
				`@ -0,0 +1 @@`
				* Filter out non-`observer_can_run` queries for observers in `fleetctl get queries` (to match the UI behavior).