From bd66898d38863c0b2d6946bdf1d77684c00d9177 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Thu, 15 Feb 2024 12:40:36 -0500 Subject: [PATCH] fix: add observer and observer plus to lock/unlock permissions (#16886) > Related issue: #16878 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [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 permissions changes (docs/Using Fleet/manage-access.md) - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/16878-action-perms | 2 ++ ee/server/service/hosts.go | 4 ++-- server/authz/policy.rego | 21 +++++++++++++++++++++ server/fleet/mdm.go | 10 ++++++++++ server/service/hosts_test.go | 24 ++++++++++++++++++------ 5 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 changes/16878-action-perms diff --git a/changes/16878-action-perms b/changes/16878-action-perms new file mode 100644 index 0000000000..d58de6b13f --- /dev/null +++ b/changes/16878-action-perms @@ -0,0 +1,2 @@ +- Fixes permission issues with lock/unlock functionality. Observer and Observer Plus are now able to + lock/unlock hosts. \ No newline at end of file diff --git a/ee/server/service/hosts.go b/ee/server/service/hosts.go index 57b5549ff7..ddb05cce10 100644 --- a/ee/server/service/hosts.go +++ b/ee/server/service/hosts.go @@ -51,7 +51,7 @@ func (svc *Service) LockHost(ctx context.Context, hostID uint) error { // Authorize again with team loaded now that we have the host's team_id. // Authorize as "execute mdm_command", which is the correct access // requirement and is what happens for macOS platforms. - if err := svc.authz.Authorize(ctx, fleet.MDMCommandAuthz{TeamID: host.TeamID}, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, fleet.MDMHostActionAuthz{TeamID: host.TeamID}, fleet.ActionWrite); err != nil { return err } @@ -133,7 +133,7 @@ func (svc *Service) UnlockHost(ctx context.Context, hostID uint) (string, error) // Authorize again with team loaded now that we have the host's team_id. // Authorize as "execute mdm_command", which is the correct access // requirement. - if err := svc.authz.Authorize(ctx, fleet.MDMCommandAuthz{TeamID: host.TeamID}, fleet.ActionWrite); err != nil { + if err := svc.authz.Authorize(ctx, fleet.MDMHostActionAuthz{TeamID: host.TeamID}, fleet.ActionWrite); err != nil { return "", err } diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 3e3c9ce6ef..9c5fe6cf3c 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -800,6 +800,27 @@ allow { action == read } + +## +# MDM Actions (lock, unlock, wipe, etc.) +## + +# Team admins, maintainers, observers, and observer_plus can write (execute) MDM actions on hosts of their teams. +allow { + not is_null(object.team_id) + object.type == "mdm_action" + team_role(subject, object.team_id) == [admin, maintainer, observer, observer_plus][_] + action == write +} + +# Global admins, maintainers, observers, and observer_plus can write (execute) MDM actions on hosts. +allow { + object.type == "mdm_action" + subject.global_role == [admin, maintainer, observer, observer_plus][_] + action == write +} + + ## # Cron schedules ## diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go index a4de548c3b..7e24c1fd7e 100644 --- a/server/fleet/mdm.go +++ b/server/fleet/mdm.go @@ -513,3 +513,13 @@ func MDMProfileSpecsMatch(a, b []MDMProfileSpec) bool { return len(pathLabelCounts) == 0 } + +// MDMHostActionAuthz is used to check authorization for executing MDM actions on a host, e.g. lock, +// unlock, wipe, etc. +type MDMHostActionAuthz struct { + TeamID *uint `json:"team_id"` // required for authorization by team +} + +func (m MDMHostActionAuthz) AuthzType() string { + return "mdm_action" +} diff --git a/server/service/hosts_test.go b/server/service/hosts_test.go index 98c936bcfb..9381c9d78b 100644 --- a/server/service/hosts_test.go +++ b/server/service/hosts_test.go @@ -1518,26 +1518,26 @@ func TestLockUnlockHostAuth(t *testing.T) { { name: "global observer", user: &fleet.User{GlobalRole: ptr.String(fleet.RoleObserver)}, - shouldFailGlobalWrite: true, - shouldFailTeamWrite: true, + shouldFailGlobalWrite: false, + shouldFailTeamWrite: false, }, { name: "team observer", user: &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}}, shouldFailGlobalWrite: true, - shouldFailTeamWrite: true, + shouldFailTeamWrite: false, }, { name: "global observer plus", user: &fleet.User{GlobalRole: ptr.String(fleet.RoleObserverPlus)}, - shouldFailGlobalWrite: true, - shouldFailTeamWrite: true, + shouldFailGlobalWrite: false, + shouldFailTeamWrite: false, }, { name: "team observer plus", user: &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserverPlus}}}, shouldFailGlobalWrite: true, - shouldFailTeamWrite: true, + shouldFailTeamWrite: false, }, { name: "global admin", @@ -1575,6 +1575,18 @@ func TestLockUnlockHostAuth(t *testing.T) { shouldFailGlobalWrite: true, shouldFailTeamWrite: true, }, + { + name: "team observer wrong team", + user: &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 42}, Role: fleet.RoleObserver}}}, + shouldFailGlobalWrite: true, + shouldFailTeamWrite: true, + }, + { + name: "team observer plus wrong team", + user: &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 42}, Role: fleet.RoleObserverPlus}}}, + shouldFailGlobalWrite: true, + shouldFailTeamWrite: true, + }, { name: "global gitops", user: &fleet.User{GlobalRole: ptr.String(fleet.RoleGitOps)},