diff --git a/server/authz/policy.rego b/server/authz/policy.rego index ca7e0268de..d7c49d22ad 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -23,9 +23,6 @@ run := "run" # Action used on object "query" used for running "new" live queries. run_new := "run_new" -# MDM specific actions -mdm_command := "mdm_command" - # Roles admin := "admin" maintainer := "maintainer" @@ -599,21 +596,6 @@ allow { action == write } -# Global admins and maintainers can issue MDM commands to all hosts. -allow { - object.type == "host" - subject.global_role == [admin, maintainer][_] - action == mdm_command -} - -# Team admins and maintainers can issue MDM commands to hosts on their teams. -allow { - not is_null(object.team_id) - object.type == "host" - team_role(subject, object.team_id) == [admin, maintainer][_] - action == mdm_command -} - # Global admins can read and write MDM apple information. allow { object.type == "mdm_apple" diff --git a/server/authz/policy_test.go b/server/authz/policy_test.go index 71ed444193..a0e80da345 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -14,14 +14,13 @@ import ( ) const ( - read = fleet.ActionRead - list = fleet.ActionList - write = fleet.ActionWrite - writeRole = fleet.ActionWriteRole - run = fleet.ActionRun - runNew = fleet.ActionRunNew - changePwd = fleet.ActionChangePassword - mdmCommand = fleet.ActionMDMCommand + read = fleet.ActionRead + list = fleet.ActionList + write = fleet.ActionWrite + writeRole = fleet.ActionWriteRole + run = fleet.ActionRun + runNew = fleet.ActionRunNew + changePwd = fleet.ActionChangePassword ) var auth *Authorizer @@ -535,145 +534,109 @@ func TestAuthorizeHost(t *testing.T) { {user: nil, object: host, action: read, allow: false}, {user: nil, object: host, action: write, allow: false}, {user: nil, object: host, action: list, allow: false}, - {user: nil, object: host, action: mdmCommand, allow: false}, {user: nil, object: hostTeam1, action: read, allow: false}, {user: nil, object: hostTeam1, action: write, allow: false}, - {user: nil, object: hostTeam1, action: mdmCommand, allow: false}, {user: nil, object: hostTeam2, action: read, allow: false}, {user: nil, object: hostTeam2, action: write, allow: false}, - {user: nil, object: hostTeam2, action: mdmCommand, allow: false}, // No host access if the user has no roles. {user: test.UserNoRoles, object: host, action: read, allow: false}, {user: test.UserNoRoles, object: host, action: write, allow: false}, {user: test.UserNoRoles, object: host, action: list, allow: false}, - {user: test.UserNoRoles, object: host, action: mdmCommand, allow: false}, {user: test.UserNoRoles, object: hostTeam1, action: read, allow: false}, {user: test.UserNoRoles, object: hostTeam1, action: write, allow: false}, - {user: test.UserNoRoles, object: hostTeam1, action: mdmCommand, allow: false}, {user: test.UserNoRoles, object: hostTeam2, action: read, allow: false}, {user: test.UserNoRoles, object: hostTeam2, action: write, allow: false}, - {user: test.UserNoRoles, object: hostTeam2, action: mdmCommand, allow: false}, // Global observer can read all {user: test.UserObserver, object: host, action: read, allow: true}, {user: test.UserObserver, object: host, action: write, allow: false}, {user: test.UserObserver, object: host, action: list, allow: true}, - {user: test.UserObserver, object: host, action: mdmCommand, allow: false}, {user: test.UserObserver, object: hostTeam1, action: read, allow: true}, {user: test.UserObserver, object: hostTeam1, action: write, allow: false}, - {user: test.UserObserver, object: hostTeam1, action: mdmCommand, allow: false}, {user: test.UserObserver, object: hostTeam2, action: read, allow: true}, {user: test.UserObserver, object: hostTeam2, action: write, allow: false}, - {user: test.UserObserver, object: hostTeam2, action: mdmCommand, allow: false}, // Global observer+ can read all {user: test.UserObserverPlus, object: host, action: read, allow: true}, {user: test.UserObserverPlus, object: host, action: write, allow: false}, {user: test.UserObserverPlus, object: host, action: list, allow: true}, - {user: test.UserObserverPlus, object: host, action: mdmCommand, allow: false}, {user: test.UserObserverPlus, object: hostTeam1, action: read, allow: true}, {user: test.UserObserverPlus, object: hostTeam1, action: write, allow: false}, - {user: test.UserObserverPlus, object: hostTeam1, action: mdmCommand, allow: false}, {user: test.UserObserverPlus, object: hostTeam2, action: read, allow: true}, {user: test.UserObserverPlus, object: hostTeam2, action: write, allow: false}, - {user: test.UserObserverPlus, object: hostTeam2, action: mdmCommand, allow: false}, // Global admin can read/write all {user: test.UserAdmin, object: host, action: read, allow: true}, {user: test.UserAdmin, object: host, action: write, allow: true}, {user: test.UserAdmin, object: host, action: list, allow: true}, - {user: test.UserAdmin, object: host, action: mdmCommand, allow: true}, {user: test.UserAdmin, object: hostTeam1, action: read, allow: true}, {user: test.UserAdmin, object: hostTeam1, action: write, allow: true}, - {user: test.UserAdmin, object: hostTeam1, action: mdmCommand, allow: true}, {user: test.UserAdmin, object: hostTeam2, action: read, allow: true}, {user: test.UserAdmin, object: hostTeam2, action: write, allow: true}, - {user: test.UserAdmin, object: hostTeam2, action: mdmCommand, allow: true}, // Global maintainer can read/write all {user: test.UserMaintainer, object: host, action: read, allow: true}, {user: test.UserMaintainer, object: host, action: write, allow: true}, {user: test.UserMaintainer, object: host, action: list, allow: true}, - {user: test.UserMaintainer, object: host, action: mdmCommand, allow: true}, {user: test.UserMaintainer, object: hostTeam1, action: read, allow: true}, {user: test.UserMaintainer, object: hostTeam1, action: write, allow: true}, - {user: test.UserMaintainer, object: hostTeam1, action: mdmCommand, allow: true}, {user: test.UserMaintainer, object: hostTeam2, action: read, allow: true}, {user: test.UserMaintainer, object: hostTeam2, action: write, allow: true}, - {user: test.UserMaintainer, object: hostTeam2, action: mdmCommand, allow: true}, // Global GitOps can write (not read) all. {user: test.UserGitOps, object: host, action: read, allow: false}, {user: test.UserGitOps, object: host, action: write, allow: true}, {user: test.UserGitOps, object: host, action: list, allow: false}, - {user: test.UserGitOps, object: host, action: mdmCommand, allow: false}, {user: test.UserGitOps, object: hostTeam1, action: read, allow: false}, {user: test.UserGitOps, object: hostTeam1, action: write, allow: true}, - {user: test.UserGitOps, object: hostTeam1, action: mdmCommand, allow: false}, {user: test.UserGitOps, object: hostTeam2, action: read, allow: false}, {user: test.UserGitOps, object: hostTeam2, action: write, allow: true}, - {user: test.UserGitOps, object: hostTeam2, action: mdmCommand, allow: false}, // Team observer can read only on appropriate team {user: teamObserver, object: host, action: read, allow: false}, {user: teamObserver, object: host, action: write, allow: false}, {user: teamObserver, object: host, action: list, allow: true}, - {user: teamObserver, object: host, action: mdmCommand, allow: false}, {user: teamObserver, object: hostTeam1, action: read, allow: true}, {user: teamObserver, object: hostTeam1, action: write, allow: false}, - {user: teamObserver, object: hostTeam1, action: mdmCommand, allow: false}, {user: teamObserver, object: hostTeam2, action: read, allow: false}, {user: teamObserver, object: hostTeam2, action: write, allow: false}, - {user: teamObserver, object: hostTeam2, action: mdmCommand, allow: false}, // Team observer+ can read only on appropriate team {user: teamObserverPlus, object: host, action: read, allow: false}, {user: teamObserverPlus, object: host, action: write, allow: false}, {user: teamObserverPlus, object: host, action: list, allow: true}, - {user: teamObserverPlus, object: host, action: mdmCommand, allow: false}, {user: teamObserverPlus, object: hostTeam1, action: read, allow: true}, {user: teamObserverPlus, object: hostTeam1, action: write, allow: false}, - {user: teamObserverPlus, object: hostTeam1, action: mdmCommand, allow: false}, {user: teamObserverPlus, object: hostTeam2, action: read, allow: false}, {user: teamObserverPlus, object: hostTeam2, action: write, allow: false}, - {user: teamObserverPlus, object: hostTeam2, action: mdmCommand, allow: false}, // Team maintainer can read/write only on appropriate team {user: teamMaintainer, object: host, action: read, allow: false}, {user: teamMaintainer, object: host, action: write, allow: false}, {user: teamMaintainer, object: host, action: list, allow: true}, - {user: teamMaintainer, object: host, action: mdmCommand, allow: false}, {user: teamMaintainer, object: hostTeam1, action: read, allow: true}, {user: teamMaintainer, object: hostTeam1, action: write, allow: true}, - {user: teamMaintainer, object: hostTeam1, action: mdmCommand, allow: true}, {user: teamMaintainer, object: hostTeam2, action: read, allow: false}, {user: teamMaintainer, object: hostTeam2, action: write, allow: false}, - {user: teamMaintainer, object: hostTeam2, action: mdmCommand, allow: false}, // Team admin can read/write only on appropriate team {user: teamAdmin, object: host, action: read, allow: false}, {user: teamAdmin, object: host, action: write, allow: false}, {user: teamAdmin, object: host, action: list, allow: true}, - {user: teamAdmin, object: host, action: mdmCommand, allow: false}, {user: teamAdmin, object: hostTeam1, action: read, allow: true}, {user: teamAdmin, object: hostTeam1, action: write, allow: true}, - {user: teamAdmin, object: hostTeam1, action: mdmCommand, allow: true}, {user: teamAdmin, object: hostTeam2, action: read, allow: false}, {user: teamAdmin, object: hostTeam2, action: write, allow: false}, - {user: teamAdmin, object: hostTeam2, action: mdmCommand, allow: false}, // Team GitOps can cannot read/write hosts. {user: teamGitOps, object: host, action: read, allow: false}, {user: teamGitOps, object: host, action: write, allow: false}, {user: teamGitOps, object: host, action: list, allow: false}, - {user: teamGitOps, object: host, action: mdmCommand, allow: false}, {user: teamGitOps, object: hostTeam1, action: read, allow: false}, {user: teamGitOps, object: hostTeam1, action: write, allow: false}, - {user: teamGitOps, object: hostTeam1, action: mdmCommand, allow: false}, {user: teamGitOps, object: hostTeam2, action: read, allow: false}, {user: teamGitOps, object: hostTeam2, action: write, allow: false}, - {user: teamGitOps, object: hostTeam2, action: mdmCommand, allow: false}, }) } @@ -1480,6 +1443,96 @@ func runTestCases(t *testing.T, testCases []authTestCase) { } } +func TestAuthorizeMDMAppleCommand(t *testing.T) { + t.Parallel() + + globalCommand := &fleet.MDMAppleCommandAuthz{} + team1Command := &fleet.MDMAppleCommandAuthz{ + TeamID: ptr.Uint(1), + } + runTestCases(t, []authTestCase{ + {user: test.UserNoRoles, object: globalCommand, action: write, allow: false}, + {user: test.UserNoRoles, object: globalCommand, action: read, allow: false}, + {user: test.UserNoRoles, object: team1Command, action: write, allow: false}, + {user: test.UserNoRoles, object: team1Command, action: read, allow: false}, + + {user: test.UserAdmin, object: globalCommand, action: write, allow: true}, + {user: test.UserAdmin, object: globalCommand, action: read, allow: true}, + {user: test.UserAdmin, object: team1Command, action: write, allow: true}, + {user: test.UserAdmin, object: team1Command, action: read, allow: true}, + + {user: test.UserMaintainer, object: globalCommand, action: write, allow: true}, + {user: test.UserMaintainer, object: globalCommand, action: read, allow: true}, + {user: test.UserMaintainer, object: team1Command, action: write, allow: true}, + {user: test.UserMaintainer, object: team1Command, action: read, allow: true}, + + {user: test.UserObserver, object: globalCommand, action: write, allow: false}, + {user: test.UserObserver, object: globalCommand, action: read, allow: true}, + {user: test.UserObserver, object: team1Command, action: write, allow: false}, + {user: test.UserObserver, object: team1Command, action: read, allow: true}, + + {user: test.UserObserverPlus, object: globalCommand, action: write, allow: false}, + {user: test.UserObserverPlus, object: globalCommand, action: read, allow: true}, + {user: test.UserObserverPlus, object: team1Command, action: write, allow: false}, + {user: test.UserObserverPlus, object: team1Command, action: read, allow: true}, + + {user: test.UserGitOps, object: globalCommand, action: write, allow: false}, + {user: test.UserGitOps, object: globalCommand, action: read, allow: false}, + {user: test.UserGitOps, object: team1Command, action: write, allow: false}, + {user: test.UserGitOps, object: team1Command, action: read, allow: false}, + + {user: test.UserTeamAdminTeam1, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamAdminTeam1, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamAdminTeam1, object: team1Command, action: write, allow: true}, + {user: test.UserTeamAdminTeam1, object: team1Command, action: read, allow: true}, + + {user: test.UserTeamAdminTeam2, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamAdminTeam2, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamAdminTeam2, object: team1Command, action: write, allow: false}, + {user: test.UserTeamAdminTeam2, object: team1Command, action: read, allow: false}, + + {user: test.UserTeamMaintainerTeam1, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamMaintainerTeam1, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamMaintainerTeam1, object: team1Command, action: write, allow: true}, + {user: test.UserTeamMaintainerTeam1, object: team1Command, action: read, allow: true}, + + {user: test.UserTeamMaintainerTeam2, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamMaintainerTeam2, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamMaintainerTeam2, object: team1Command, action: write, allow: false}, + {user: test.UserTeamMaintainerTeam2, object: team1Command, action: read, allow: false}, + + {user: test.UserTeamObserverTeam1, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamObserverTeam1, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamObserverTeam1, object: team1Command, action: write, allow: false}, + {user: test.UserTeamObserverTeam1, object: team1Command, action: read, allow: true}, + + {user: test.UserTeamObserverTeam2, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamObserverTeam2, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamObserverTeam2, object: team1Command, action: write, allow: false}, + {user: test.UserTeamObserverTeam2, object: team1Command, action: read, allow: false}, + + {user: test.UserTeamObserverPlusTeam1, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam1, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamObserverPlusTeam1, object: team1Command, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam1, object: team1Command, action: read, allow: true}, + + {user: test.UserTeamObserverPlusTeam2, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam2, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamObserverPlusTeam2, object: team1Command, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam2, object: team1Command, action: read, allow: false}, + + {user: test.UserTeamGitOpsTeam1, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: team1Command, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: team1Command, action: read, allow: false}, + + {user: test.UserTeamGitOpsTeam2, object: globalCommand, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam2, object: globalCommand, action: read, allow: false}, + {user: test.UserTeamGitOpsTeam2, object: team1Command, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam2, object: team1Command, action: read, allow: false}, + }) +} + func TestJSONToInterfaceUser(t *testing.T) { t.Parallel() diff --git a/server/fleet/authz.go b/server/fleet/authz.go index 414cc35d16..f73967aced 100644 --- a/server/fleet/authz.go +++ b/server/fleet/authz.go @@ -31,11 +31,4 @@ const ( ActionRun = "run" // ActionRunNew is the action for running a new live query. ActionRunNew = "run_new" - - // - // MDM-specific actions - // - - // ActionMDMCommand is the action for executing an MDM command - ActionMDMCommand = "mdm_command" ) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 602fbb93ca..1eefc27a14 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -1079,7 +1079,7 @@ func (svc *Service) EnqueueMDMAppleCommand( "DeviceLock": true, } - // load hosts (lite) by uuids, check that the user has the rigts to run + // load hosts (lite) by uuids, check that the user has the rights to run // commands for every affected team. if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { return 0, nil, ctxerr.Wrap(ctx, err) @@ -1277,8 +1277,10 @@ func (svc *Service) EnqueueMDMAppleCommandRemoveEnrollmentProfile(ctx context.Co return ctxerr.Wrap(ctx, err, "getting mdm checkin info for mdm apple remove profile command") } - // check authorization again based on host info for team-based permissions - if err := svc.authz.Authorize(ctx, h, fleet.ActionMDMCommand); err != nil { + // Check authorization again based on host info for team-based permissions. + if err := svc.authz.Authorize(ctx, fleet.MDMAppleCommandAuthz{ + TeamID: h.TeamID, + }, fleet.ActionWrite); err != nil { return err }