Remove old mdm_command action (do we really need it?) (#11222)

A question in form of PR:

Do we really need the following two entities in our
[policy.rego](https://github.com/fleetdm/fleet/blob/main/server/authz/policy.rego)
`1. (object=mdm_apple_command, action=read/write)` and `2. (object=host,
action=mdm_command)`? (Maybe mdm_command is a leftover action from the
PoC?)

Guess: `mdm_apple_command` (`fleet.MDMAppleCommandAuthz`) is what we
want: `action=write` means you can enqueue, `action=read` means you can
list commands and read their results.

PS: Found this while trying to add command execution permissions to the
new `GitOps` role.
This commit is contained in:
Lucas Manuel Rodriguez 2023-04-18 07:53:33 -03:00 committed by GitHub
parent 9c594fba21
commit ed4f6e4178
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 102 additions and 72 deletions

View file

@ -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"

View file

@ -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()

View file

@ -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"
)

View file

@ -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
}