Fix permissions on GitOps user for searching hosts or count targets (#11448)

#11447

- ~[ ] 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)~
- ~[ ] 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)).~
This commit is contained in:
Lucas Manuel Rodriguez 2023-05-01 12:57:28 -03:00 committed by GitHub
parent cd5dfa23f8
commit 87709d8c95
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 219 additions and 16 deletions

View File

@ -415,11 +415,18 @@ allow {
# Targets
##
# All users can read targets (filtered appropriately based on their
# teams/roles).
# Global admin, maintainer, observer_plus and observer can read targets.
allow {
not is_null(subject)
object.type == "target"
subject.global_role == [admin, maintainer, observer_plus, observer][_]
action == read
}
# Team admin, maintainer, observer_plus and observer can read global config.
allow {
object.type == "target"
# If role is admin, maintainer, observer_plus or observer on any team.
team_role(subject, subject.teams[_].id) == [admin, maintainer, observer_plus, observer][_]
action == read
}

View File

@ -910,13 +910,18 @@ func TestAuthorizeTarget(t *testing.T) {
runTestCases(t, []authTestCase{
{user: nil, object: target, action: read, allow: false},
// Everyone logged in can retrieve target (filter appropriately for their
// access)
{user: test.UserNoRoles, object: target, action: read, allow: true},
{user: test.UserNoRoles, object: target, action: read, allow: false},
{user: test.UserAdmin, object: target, action: read, allow: true},
{user: test.UserMaintainer, object: target, action: read, allow: true},
{user: test.UserObserver, object: target, action: read, allow: true},
{user: test.UserObserverPlus, object: target, action: read, allow: true},
{user: test.UserGitOps, object: target, action: read, allow: false},
{user: test.UserTeamAdminTeam1, object: target, action: read, allow: true},
{user: test.UserTeamMaintainerTeam1, object: target, action: read, allow: true},
{user: test.UserTeamObserverTeam1, object: target, action: read, allow: true},
{user: test.UserTeamObserverPlusTeam1, object: target, action: read, allow: true},
{user: test.UserTeamGitOpsTeam1, object: target, action: read, allow: false},
})
}

View File

@ -169,13 +169,6 @@ type PackSpecQuery struct {
Denylist *bool `json:"denylist,omitempty"`
}
// PackTarget targets a pack to a host, label, or team.
type PackTarget struct {
ID uint `db:"id" json:"-"`
PackID uint `db:"pack_id" json:"-"`
Target
}
type PackStats struct {
PackID uint `json:"pack_id"`
PackName string `json:"pack_name"`

View File

@ -3153,6 +3153,22 @@ func (s *integrationEnterpriseTestSuite) TestGitOpsUserActions() {
// Attempt to get a carved file, should fail.
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/carves/%d", carveID), listCarvesRequest{}, http.StatusForbidden, &listCarvesResponse{})
// Attempt to search hosts, should fail.
s.DoJSON("POST", "/api/latest/fleet/targets", searchTargetsRequest{
MatchQuery: "foo",
QueryID: &q1.ID,
}, http.StatusForbidden, &searchTargetsResponse{})
// Attempt to count target hosts, should fail.
s.DoJSON("POST", "/api/latest/fleet/targets/count", countTargetsRequest{
Selected: fleet.HostTargets{
HostIDs: []uint{h1.ID},
LabelIDs: []uint{clr.Label.ID},
TeamIDs: []uint{t1.ID},
},
QueryID: &q1.ID,
}, http.StatusForbidden, &countTargetsResponse{})
//
// Start running permission tests with user gitops2 (which is a GitOps use for team t1).
//
@ -3331,6 +3347,22 @@ func (s *integrationEnterpriseTestSuite) TestGitOpsUserActions() {
},
},
}, http.StatusForbidden, &teamResponse{})
// Attempt to search hosts, should fail.
s.DoJSON("POST", "/api/latest/fleet/targets", searchTargetsRequest{
MatchQuery: "foo",
QueryID: &q1.ID,
}, http.StatusForbidden, &searchTargetsResponse{})
// Attempt to count target hosts, should fail.
s.DoJSON("POST", "/api/latest/fleet/targets/count", countTargetsRequest{
Selected: fleet.HostTargets{
HostIDs: []uint{h1.ID},
LabelIDs: []uint{clr.Label.ID},
TeamIDs: []uint{t1.ID},
},
QueryID: &q1.ID,
}, http.StatusForbidden, &countTargetsResponse{})
}
func (s *integrationEnterpriseTestSuite) setTokenForTest(t *testing.T, email, password string) {

View File

@ -2,10 +2,12 @@ package service
import (
"context"
"errors"
"fmt"
"github.com/fleetdm/fleet/v4/server/authz"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
"github.com/fleetdm/fleet/v4/server/contexts/viewer"
"github.com/fleetdm/fleet/v4/server/fleet"
)
@ -23,6 +25,28 @@ type packResponse struct {
TeamIDs []uint `json:"team_ids"`
}
func userIsGitOpsOnly(ctx context.Context) (bool, error) {
vc, ok := viewer.FromContext(ctx)
if !ok {
return false, fleet.ErrNoContext
}
if vc.User == nil {
return false, errors.New("missing user in context")
}
if vc.User.GlobalRole != nil {
return *vc.User.GlobalRole == fleet.RoleGitOps, nil
}
if len(vc.User.Teams) == 0 {
return false, errors.New("user has no roles")
}
for _, teamRole := range vc.User.Teams {
if teamRole.Role != fleet.RoleGitOps {
return false, nil
}
}
return true, nil
}
func packResponseForPack(ctx context.Context, svc fleet.Service, pack fleet.Pack) (*packResponse, error) {
opts := fleet.ListOptions{}
queries, err := svc.GetScheduledQueriesInPack(ctx, pack.ID, opts)
@ -30,19 +54,42 @@ func packResponseForPack(ctx context.Context, svc fleet.Service, pack fleet.Pack
return nil, err
}
totalHostsCount := uint(0)
hostMetrics, err := svc.CountHostsInTargets(
ctx,
nil,
fleet.HostTargets{HostIDs: pack.HostIDs, LabelIDs: pack.LabelIDs, TeamIDs: pack.TeamIDs},
fleet.HostTargets{
HostIDs: pack.HostIDs,
LabelIDs: pack.LabelIDs,
TeamIDs: pack.TeamIDs,
},
)
if err != nil {
return nil, err
var authErr *authz.Forbidden
if !errors.As(err, &authErr) {
return nil, err
}
// Some users (e.g. gitops) are not able to read targets, thus
// we do not fail when gathering the total host count to not fail
// write packs request.
ok, gerr := userIsGitOpsOnly(ctx)
if gerr != nil {
return nil, gerr
}
if !ok {
return nil, err
}
}
if hostMetrics != nil {
totalHostsCount = hostMetrics.TotalHosts
}
return &packResponse{
Pack: pack,
QueryCount: uint(len(queries)),
TotalHostsCount: hostMetrics.TotalHosts,
TotalHostsCount: totalHostsCount,
HostIDs: pack.HostIDs,
LabelIDs: pack.LabelIDs,
TeamIDs: pack.TeamIDs,

View File

@ -5,6 +5,7 @@ import (
"testing"
"github.com/fleetdm/fleet/v4/server/authz"
"github.com/fleetdm/fleet/v4/server/contexts/viewer"
"github.com/fleetdm/fleet/v4/server/datastore/mysql"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/mock"
@ -300,3 +301,121 @@ func testPacksApplyPackSpecs(t *testing.T, ds *mysql.Datastore) {
})
}
}
func TestUserIsGitOpsOnly(t *testing.T) {
for _, tc := range []struct {
name string
user *fleet.User
expectedFn func(value bool, err error) bool
}{
{
name: "missing user in context",
user: nil,
expectedFn: func(value bool, err error) bool {
return err != nil && !value
},
},
{
name: "no roles",
user: &fleet.User{},
expectedFn: func(value bool, err error) bool {
return err != nil && !value
},
},
{
name: "global gitops",
user: &fleet.User{
GlobalRole: ptr.String(fleet.RoleGitOps),
},
expectedFn: func(value bool, err error) bool {
return err == nil && value
},
},
{
name: "global non-gitops",
user: &fleet.User{
GlobalRole: ptr.String(fleet.RoleObserver),
},
expectedFn: func(value bool, err error) bool {
return err == nil && !value
},
},
{
name: "team gitops",
user: &fleet.User{
GlobalRole: nil,
Teams: []fleet.UserTeam{
{
Team: fleet.Team{ID: 1},
Role: fleet.RoleGitOps,
},
},
},
expectedFn: func(value bool, err error) bool {
return err == nil && value
},
},
{
name: "multiple team gitops",
user: &fleet.User{
GlobalRole: nil,
Teams: []fleet.UserTeam{
{
Team: fleet.Team{ID: 1},
Role: fleet.RoleGitOps,
},
{
Team: fleet.Team{ID: 2},
Role: fleet.RoleGitOps,
},
},
},
expectedFn: func(value bool, err error) bool {
return err == nil && value
},
},
{
name: "multiple teams, not all gitops",
user: &fleet.User{
GlobalRole: nil,
Teams: []fleet.UserTeam{
{
Team: fleet.Team{ID: 1},
Role: fleet.RoleObserver,
},
{
Team: fleet.Team{ID: 2},
Role: fleet.RoleGitOps,
},
},
},
expectedFn: func(value bool, err error) bool {
return err == nil && !value
},
},
{
name: "multiple teams, none gitops",
user: &fleet.User{
GlobalRole: nil,
Teams: []fleet.UserTeam{
{
Team: fleet.Team{ID: 1},
Role: fleet.RoleObserver,
},
{
Team: fleet.Team{ID: 2},
Role: fleet.RoleMaintainer,
},
},
},
expectedFn: func(value bool, err error) bool {
return err == nil && !value
},
},
} {
t.Run(tc.name, func(t *testing.T) {
actual, err := userIsGitOpsOnly(viewer.NewContext(context.Background(), viewer.Viewer{User: tc.user}))
require.True(t, tc.expectedFn(actual, err))
})
}
}