diff --git a/changes/issue-policy-rego-bug-in-fleet-policies b/changes/issue-policy-rego-bug-in-fleet-policies new file mode 100644 index 0000000000..c616a928aa --- /dev/null +++ b/changes/issue-policy-rego-bug-in-fleet-policies @@ -0,0 +1 @@ +* Fix bug in rego policy for fleet policies. diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 4e5c248385..6dee043e7a 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -405,14 +405,14 @@ allow { # Policies ## -# Global Admin and Maintainer users can read and write policies +# Global Admin can read and write policies allow { object.type == "policy" subject.global_role == admin action == [read, write][_] } -# Global maintainer can read and write global policies +# Global Maintainer can read and write global policies allow { is_null(object.team_id) object.type == "policy" @@ -430,9 +430,8 @@ allow { # Team admin and maintainers can read and write policies for their teams allow { not is_null(object.team_id) - object.team_id == subject.teams[_].id object.type == "policy" - team_role(subject, subject.teams[_].id) == [admin,maintainer][_] + team_role(subject, object.team_id) == [admin,maintainer][_] action == [read, write][_] } @@ -444,13 +443,12 @@ allow { action == read } -# Team Observer can read policies +# Team Observer can read policies for their teams allow { not is_null(object.team_id) - object.team_id == subject.teams[_].id object.type == "policy" - team_role(subject, subject.teams[_].id) == observer - action == [read][_] + team_role(subject, object.team_id) == observer + action == read } ## diff --git a/server/service/global_policies_test.go b/server/service/global_policies_test.go index 158be716c8..288f06c90f 100644 --- a/server/service/global_policies_test.go +++ b/server/service/global_policies_test.go @@ -21,7 +21,11 @@ func TestGlobalPoliciesAuth(t *testing.T) { return nil, nil } ds.PolicyFunc = func(ctx context.Context, id uint) (*fleet.Policy, error) { - return nil, nil + return &fleet.Policy{ + PolicyData: fleet.PolicyData{ + ID: id, + }, + }, nil } ds.DeleteGlobalPoliciesFunc = func(ctx context.Context, ids []uint) ([]uint, error) { return nil, nil @@ -35,6 +39,9 @@ func TestGlobalPoliciesAuth(t *testing.T) { ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activityType string, details *map[string]interface{}) error { return nil } + ds.SavePolicyFunc = func(ctx context.Context, p *fleet.Policy) error { + return nil + } testCases := []struct { name string @@ -95,6 +102,9 @@ func TestGlobalPoliciesAuth(t *testing.T) { _, err = svc.GetPolicyByIDQueries(ctx, 1) checkAuthErr(t, tt.shouldFailRead, err) + _, err = svc.ModifyGlobalPolicy(ctx, 1, fleet.ModifyPolicyPayload{}) + checkAuthErr(t, tt.shouldFailWrite, err) + _, err = svc.DeleteGlobalPolicies(ctx, []uint{1}) checkAuthErr(t, tt.shouldFailWrite, err) diff --git a/server/service/team_policies_test.go b/server/service/team_policies_test.go index 48ec3d5650..22668c39d9 100644 --- a/server/service/team_policies_test.go +++ b/server/service/team_policies_test.go @@ -17,7 +17,12 @@ func TestTeamPoliciesAuth(t *testing.T) { svc := newTestService(ds, nil, nil) ds.NewTeamPolicyFunc = func(ctx context.Context, teamID uint, authorID *uint, args fleet.PolicyPayload) (*fleet.Policy, error) { - return &fleet.Policy{}, nil + return &fleet.Policy{ + PolicyData: fleet.PolicyData{ + ID: 1, + TeamID: ptr.Uint(1), + }, + }, nil } ds.ListTeamPoliciesFunc = func(ctx context.Context, teamID uint) ([]*fleet.Policy, error) { return nil, nil @@ -25,6 +30,20 @@ func TestTeamPoliciesAuth(t *testing.T) { ds.TeamPolicyFunc = func(ctx context.Context, teamID uint, policyID uint) (*fleet.Policy, error) { return nil, nil } + ds.PolicyFunc = func(ctx context.Context, id uint) (*fleet.Policy, error) { + if id == 1 { + return &fleet.Policy{ + PolicyData: fleet.PolicyData{ + ID: 1, + TeamID: ptr.Uint(1), + }, + }, nil + } + return nil, nil + } + ds.SavePolicyFunc = func(ctx context.Context, p *fleet.Policy) error { + return nil + } ds.DeleteTeamPoliciesFunc = func(ctx context.Context, teamID uint, ids []uint) ([]uint, error) { return nil, nil } @@ -86,6 +105,21 @@ func TestTeamPoliciesAuth(t *testing.T) { true, true, }, + { + "team observer, and team admin of another team", + &fleet.User{Teams: []fleet.UserTeam{ + { + Team: fleet.Team{ID: 1}, + Role: fleet.RoleObserver, + }, + { + Team: fleet.Team{ID: 2}, + Role: fleet.RoleAdmin, + }, + }}, + true, + false, + }, { "team maintainer, DOES NOT belong to team", &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 2}, Role: fleet.RoleMaintainer}}}, @@ -115,6 +149,9 @@ func TestTeamPoliciesAuth(t *testing.T) { _, err = svc.GetTeamPolicyByIDQueries(ctx, 1, 1) checkAuthErr(t, tt.shouldFailRead, err) + _, err = svc.ModifyTeamPolicy(ctx, 1, 1, fleet.ModifyPolicyPayload{}) + checkAuthErr(t, tt.shouldFailWrite, err) + _, err = svc.DeleteTeamPolicies(ctx, 1, []uint{1}) checkAuthErr(t, tt.shouldFailWrite, err)