diff --git a/changes/15337-gitops-puppet b/changes/15337-gitops-puppet new file mode 100644 index 0000000000..7d49948b22 --- /dev/null +++ b/changes/15337-gitops-puppet @@ -0,0 +1,3 @@ +* Allow GitOps user to access the following endpoints: + - GET /api/latest/fleet/mdm/hosts/:host_id/profiles + - GET /api/latest/fleet/hosts/identifier/:identifier diff --git a/ee/server/service/mdm.go b/ee/server/service/mdm.go index dd54773140..2d80af3dbc 100644 --- a/ee/server/service/mdm.go +++ b/ee/server/service/mdm.go @@ -147,7 +147,7 @@ func (svc *Service) MDMAppleEraseDevice(ctx context.Context, hostID uint) error } func (svc *Service) MDMListHostConfigurationProfiles(ctx context.Context, hostID uint) ([]*fleet.MDMAppleConfigProfile, error) { - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionSelectiveList); err != nil { return nil, err } diff --git a/ee/tools/puppet/fleetdm/spec/functions/preassign_profile_spec.rb b/ee/tools/puppet/fleetdm/spec/functions/preassign_profile_spec.rb index 920add8432..ba04da52bc 100644 --- a/ee/tools/puppet/fleetdm/spec/functions/preassign_profile_spec.rb +++ b/ee/tools/puppet/fleetdm/spec/functions/preassign_profile_spec.rb @@ -57,4 +57,90 @@ describe 'fleetdm::preassign_profile' do end # TODO: add coverage for early exits, error handling, and resource_changed + it 'gracefully handles preassign_profile failures' do + expect(fleet_client_mock) + .to receive(:preassign_profile) + .with(run_identifier, device_uuid, template, 'default', 'present', rspec_puppet_env) + .and_return({ 'error' => 'preassign error' }) + is_expected.to run.with_params(profile_identifier, device_uuid, template).and_return({ 'error' => 'preassign error', 'resource_changed' => false }) + end + + it 'gracefully handles get_host_by_identifier failures' do + expect(fleet_client_mock) + .to receive(:preassign_profile) + .with(run_identifier, device_uuid, template, 'default', 'present', rspec_puppet_env) + .and_return({ 'error' => '' }) + expect(fleet_client_mock) + .to receive(:get_host_by_identifier) + .with(device_uuid, rspec_puppet_env) + .and_return({ 'error' => 'get host by identifier error' }) + is_expected.to run.with_params(profile_identifier, device_uuid, template).and_return({ 'error' => 'get host by identifier error', 'resource_changed' => false }) + end + + it 'gracefully handles get_host_profiles failures' do + expect(fleet_client_mock) + .to receive(:preassign_profile) + .with(run_identifier, device_uuid, template, 'default', 'present', rspec_puppet_env) + .and_return({ 'error' => '' }) + expect(fleet_client_mock) + .to receive(:get_host_by_identifier) + .with(device_uuid, rspec_puppet_env) + .and_return({ 'error' => '', 'body' => host_response }) + expect(fleet_client_mock) + .to receive(:get_host_profiles) + .with(host_response['host']['id'], rspec_puppet_env) + .and_return({ 'error' => 'get host profiles error' }) + is_expected.to run.with_params(profile_identifier, device_uuid, template).and_return({ 'error' => 'get host profiles error', 'resource_changed' => false }) + end + + it 'handles the case where no host is found' do + expect(fleet_client_mock) + .to receive(:preassign_profile) + .with(run_identifier, device_uuid, template, 'default', 'present', rspec_puppet_env) + .and_return({ 'error' => '' }) + expect(fleet_client_mock) + .to receive(:get_host_by_identifier) + .with(device_uuid, rspec_puppet_env) + .and_return({ 'error' => '', 'body' => {} }) # Simulating no host found + is_expected.to run.with_params(profile_identifier, device_uuid, template).and_return({ 'error' => 'No host found', 'resource_changed' => false }) + end + + it 'does not change resource when profile is already present and ensure is present' do + host_profiles = { 'profiles' => [{ 'checksum' => Digest::MD5.base64digest(template) }] } + expect_successful_calls_with_host_profiles(host_profiles, 'present') + is_expected.to run.with_params(profile_identifier, device_uuid, template).and_return({ 'error' => '', 'resource_changed' => false }) + end + + it 'does not change resource when profile is absent and ensure is absent' do + host_profiles = { 'profiles' => [] } # no profiles assigned + expect_successful_calls_with_host_profiles(host_profiles, 'absent') + is_expected.to run.with_params(profile_identifier, device_uuid, template, 'default', 'absent').and_return({ 'error' => '', 'resource_changed' => false }) + end + + it 'changes resource when profile is present and ensure is absent' do + host_profiles = { 'profiles' => [{ 'checksum' => Digest::MD5.base64digest(template) }] } + expect_successful_calls_with_host_profiles(host_profiles, 'absent') + is_expected.to run.with_params(profile_identifier, device_uuid, template, 'default', 'absent').and_return({ 'error' => '', 'resource_changed' => true }) + end + + it 'changes resource when profile is absent and ensure is present' do + host_profiles = { 'profiles' => [] } # No profiles assigned + expect_successful_calls_with_host_profiles(host_profiles, 'present') + is_expected.to run.with_params(profile_identifier, device_uuid, template, 'default', 'present').and_return({ 'error' => '', 'resource_changed' => true }) + end + + def expect_successful_calls_with_host_profiles(host_profiles, ensure_profile) + expect(fleet_client_mock) + .to receive(:preassign_profile) + .with(run_identifier, device_uuid, template, 'default', ensure_profile, rspec_puppet_env) + .and_return({ 'error' => '' }) + expect(fleet_client_mock) + .to receive(:get_host_by_identifier) + .with(device_uuid, rspec_puppet_env) + .and_return({ 'error' => '', 'body' => host_response }) + expect(fleet_client_mock) + .to receive(:get_host_profiles) + .with(host_response['host']['id'], rspec_puppet_env) + .and_return({ 'error' => '', 'body' => host_profiles }) + end end diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 3e3c9ce6ef..b978709d19 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -23,6 +23,11 @@ run := "run" # Action used on object "query" used for running "new" live queries. run_new := "run_new" +# Selective prefixes over actions mean that they can be allowed in specific +# cases for roles that usually aren't allowed to perform them. +selective_read := "selective_read" +selective_list := "selective_list" + # Roles admin := "admin" maintainer := "maintainer" @@ -211,47 +216,76 @@ allow { # Hosts ## +# allowed_read_roles evaulates which roles are allowed for read based on the given action. +allowed_read_roles(action, base_roles, extra_roles) = result { + action == selective_read + result := base_roles | extra_roles +} else = result { + action == read + result := base_roles +} else = result { + result := null +} + +# allowed_list_roles evaulates which roles are allowed for list based on the given action. +allowed_list_roles(action, base_roles, extra_roles) = result { + action == "selective_list" + result := base_roles | extra_roles +} else = result { + action == "list" + result := base_roles +} else = result { + result := null +} + # Global admins, maintainers, observer_plus and observers can list hosts. allow { - object.type == "host" - subject.global_role == [admin, maintainer, observer_plus, observer][_] - action == list + object.type == "host" + base_roles := {admin, maintainer, observer_plus, observer} + extra_roles := {gitops} + allowed_list_roles(action, base_roles, extra_roles)[_] == subject.global_role } -# Team admins, maintainers, observer_plus and observers can list hosts. +# Team admins, maintainers, observer_plus and observers can list and selective_list hosts. +# Gitops can selective_list hosts allow { object.type == "host" - # If role is admin, maintainer, observer_plus or observer on any team. - team_role(subject, subject.teams[_].id) == [admin, maintainer, observer_plus, observer][_] - action == list + # If role is admin, maintainer, observer_plus or observer on any team. + base_roles := {admin, maintainer, observer_plus, observer} + # Or gitops for selective reads + extra_roles := {gitops} + allowed_list_roles(action, base_roles, extra_roles)[_] == team_role(subject, subject.teams[_].id) } -# Allow read/write for global admin/maintainer. +# Allow read for global admin/maintainer, selective_read for gitops. allow { object.type == "host" - subject.global_role == [admin, maintainer][_] - action == [read, write][_] + base_roles := {admin, maintainer} + extra_roles := {gitops} + allowed_read_roles(action, base_roles, extra_roles)[_] == subject.global_role } -# Global gitops can write hosts. +# Global gitops, admin and mantainers can write hosts. allow { object.type == "host" - subject.global_role == gitops + subject.global_role == [admin, maintainer, gitops][_] action == write } -# Allow read for global observer and observer_plus. +# Allow read for global observer and observer_plus, selective_read for gitops. allow { object.type == "host" - subject.global_role == [observer, observer_plus][_] - action == read + base_roles := {observer_plus, observer} + extra_roles := {gitops} + allowed_read_roles(action, base_roles, extra_roles)[_] == subject.global_role } -# Allow read for matching team admin/maintainer/observer/observer_plus. +# Allow read for matching team admin/maintainer/observer/observer_plus, selective read for gitops. allow { object.type == "host" - team_role(subject, object.team_id) == [admin, maintainer, observer, observer_plus][_] - action == read + base_roles := {admin, maintainer, observer, observer_plus} + extra_roles := {gitops} + allowed_read_roles(action, base_roles, extra_roles)[_] == team_role(subject, object.team_id) } # Team admins and maintainers can write to hosts of their own team @@ -598,38 +632,22 @@ allow { # Apple and Windows MDM ## -# Global admins and maintainers can read and write MDM config profiles. +# Global admins, maintainers and gitops can read and write MDM config profiles. allow { object.type == "mdm_config_profile" - subject.global_role == [admin, maintainer][_] + subject.global_role == [admin, maintainer, gitops][_] action == [read, write][_] } -# Global gitops can write MDM config profiles. -allow { - object.type == "mdm_config_profile" - subject.global_role == gitops - action == write -} - -# Team admins and maintainers can read and write MDM config profiles on their teams. +# Team admins, maintainers and gitops can read and write MDM config profiles on their teams. allow { not is_null(object.team_id) object.team_id != 0 object.type == "mdm_config_profile" - team_role(subject, object.team_id) == [admin, maintainer][_] + team_role(subject, object.team_id) == [admin, maintainer, gitops][_] action == [read, write][_] } -# Team gitops can write MDM config profiles on their teams. -allow { - not is_null(object.team_id) - object.team_id != 0 - object.type == "mdm_config_profile" - team_role(subject, object.team_id) == gitops - action == write -} - # 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 0c404068f0..6948cf537b 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -14,13 +14,15 @@ import ( ) const ( - read = fleet.ActionRead - list = fleet.ActionList - write = fleet.ActionWrite - writeRole = fleet.ActionWriteRole - run = fleet.ActionRun - runNew = fleet.ActionRunNew - changePwd = fleet.ActionChangePassword + read = fleet.ActionRead + list = fleet.ActionList + write = fleet.ActionWrite + writeRole = fleet.ActionWriteRole + run = fleet.ActionRun + runNew = fleet.ActionRunNew + changePwd = fleet.ActionChangePassword + selectiveRead = fleet.ActionSelectiveRead + selectiveList = fleet.ActionSelectiveList ) var auth *Authorizer @@ -534,109 +536,154 @@ 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: selectiveList, allow: false}, + {user: nil, object: host, action: selectiveRead, allow: false}, {user: nil, object: hostTeam1, action: read, allow: false}, {user: nil, object: hostTeam1, action: write, allow: false}, + {user: nil, object: hostTeam1, action: selectiveRead, allow: false}, {user: nil, object: hostTeam2, action: read, allow: false}, {user: nil, object: hostTeam2, action: write, allow: false}, + {user: nil, object: hostTeam2, action: selectiveRead, 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: selectiveList, allow: false}, + {user: test.UserNoRoles, object: host, action: selectiveRead, 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: selectiveRead, 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: selectiveRead, 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: selectiveList, allow: true}, + {user: test.UserObserver, object: host, action: selectiveRead, allow: true}, {user: test.UserObserver, object: hostTeam1, action: read, allow: true}, + {user: test.UserObserver, object: hostTeam1, action: selectiveRead, allow: true}, {user: test.UserObserver, object: hostTeam1, action: write, allow: false}, {user: test.UserObserver, object: hostTeam2, action: read, allow: true}, + {user: test.UserObserver, object: hostTeam2, action: selectiveRead, allow: true}, {user: test.UserObserver, object: hostTeam2, action: write, 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: selectiveList, allow: true}, + {user: test.UserObserverPlus, object: host, action: selectiveRead, allow: true}, {user: test.UserObserverPlus, object: hostTeam1, action: read, allow: true}, + {user: test.UserObserverPlus, object: hostTeam1, action: selectiveRead, allow: true}, {user: test.UserObserverPlus, object: hostTeam1, action: write, allow: false}, {user: test.UserObserverPlus, object: hostTeam2, action: read, allow: true}, + {user: test.UserObserverPlus, object: hostTeam2, action: selectiveRead, allow: true}, {user: test.UserObserverPlus, object: hostTeam2, action: write, allow: false}, // Global admin can read/write all {user: test.UserAdmin, object: host, action: read, allow: true}, + {user: test.UserAdmin, object: host, action: selectiveRead, 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: selectiveList, allow: true}, {user: test.UserAdmin, object: hostTeam1, action: read, allow: true}, + {user: test.UserAdmin, object: hostTeam1, action: selectiveRead, allow: true}, {user: test.UserAdmin, object: hostTeam1, action: write, allow: true}, {user: test.UserAdmin, object: hostTeam2, action: read, allow: true}, + {user: test.UserAdmin, object: hostTeam2, action: selectiveRead, allow: true}, {user: test.UserAdmin, object: hostTeam2, action: write, allow: true}, // Global maintainer can read/write all {user: test.UserMaintainer, object: host, action: read, allow: true}, + {user: test.UserMaintainer, object: host, action: selectiveRead, 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: selectiveList, allow: true}, {user: test.UserMaintainer, object: hostTeam1, action: read, allow: true}, + {user: test.UserMaintainer, object: hostTeam1, action: selectiveRead, allow: true}, {user: test.UserMaintainer, object: hostTeam1, action: write, allow: true}, {user: test.UserMaintainer, object: hostTeam2, action: read, allow: true}, + {user: test.UserMaintainer, object: hostTeam2, action: selectiveRead, allow: true}, {user: test.UserMaintainer, object: hostTeam2, action: write, allow: true}, - // Global GitOps can write (not read) all. + // Global GitOps can write and selectively 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: selectiveRead, allow: true}, {user: test.UserGitOps, object: host, action: list, allow: false}, + {user: test.UserGitOps, object: host, action: selectiveList, allow: true}, {user: test.UserGitOps, object: hostTeam1, action: read, allow: false}, {user: test.UserGitOps, object: hostTeam1, action: write, allow: true}, + {user: test.UserGitOps, object: hostTeam1, action: selectiveRead, allow: true}, {user: test.UserGitOps, object: hostTeam2, action: read, allow: false}, {user: test.UserGitOps, object: hostTeam2, action: write, allow: true}, + {user: test.UserGitOps, object: hostTeam2, action: selectiveRead, allow: true}, // Team observer can read only on appropriate team {user: teamObserver, object: host, action: read, allow: false}, + {user: teamObserver, object: host, action: selectiveRead, allow: false}, {user: teamObserver, object: host, action: write, allow: false}, {user: teamObserver, object: host, action: list, allow: true}, + {user: teamObserver, object: host, action: selectiveList, allow: true}, {user: teamObserver, object: hostTeam1, action: read, allow: true}, + {user: teamObserver, object: hostTeam1, action: selectiveRead, allow: true}, {user: teamObserver, object: hostTeam1, action: write, allow: false}, {user: teamObserver, object: hostTeam2, action: read, allow: false}, + {user: teamObserver, object: hostTeam2, action: selectiveRead, allow: false}, {user: teamObserver, object: hostTeam2, action: write, allow: false}, // Team observer+ can read only on appropriate team {user: teamObserverPlus, object: host, action: read, allow: false}, + {user: teamObserverPlus, object: host, action: selectiveRead, allow: false}, {user: teamObserverPlus, object: host, action: write, allow: false}, {user: teamObserverPlus, object: host, action: list, allow: true}, + {user: teamObserverPlus, object: host, action: selectiveList, allow: true}, {user: teamObserverPlus, object: hostTeam1, action: read, allow: true}, + {user: teamObserverPlus, object: hostTeam1, action: selectiveRead, allow: true}, {user: teamObserverPlus, object: hostTeam1, action: write, allow: false}, {user: teamObserverPlus, object: hostTeam2, action: read, allow: false}, + {user: teamObserverPlus, object: hostTeam2, action: selectiveRead, allow: false}, {user: teamObserverPlus, object: hostTeam2, action: write, allow: false}, // Team maintainer can read/write only on appropriate team {user: teamMaintainer, object: host, action: read, allow: false}, + {user: teamMaintainer, object: host, action: selectiveRead, allow: false}, {user: teamMaintainer, object: host, action: write, allow: false}, {user: teamMaintainer, object: host, action: list, allow: true}, + {user: teamMaintainer, object: host, action: selectiveList, allow: true}, {user: teamMaintainer, object: hostTeam1, action: read, allow: true}, + {user: teamMaintainer, object: hostTeam1, action: selectiveRead, allow: true}, {user: teamMaintainer, object: hostTeam1, action: write, allow: true}, {user: teamMaintainer, object: hostTeam2, action: read, allow: false}, {user: teamMaintainer, object: hostTeam2, action: write, allow: false}, // Team admin can read/write only on appropriate team {user: teamAdmin, object: host, action: read, allow: false}, + {user: teamAdmin, object: host, action: selectiveRead, allow: false}, {user: teamAdmin, object: host, action: write, allow: false}, {user: teamAdmin, object: host, action: list, allow: true}, + {user: teamAdmin, object: host, action: selectiveList, allow: true}, {user: teamAdmin, object: hostTeam1, action: read, allow: true}, {user: teamAdmin, object: hostTeam1, action: write, allow: true}, {user: teamAdmin, object: hostTeam2, action: read, allow: false}, {user: teamAdmin, object: hostTeam2, action: write, allow: false}, - // Team GitOps can cannot read/write hosts. + // Team GitOps can cannot read hosts, but it can write and selectively read them. {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: selectiveRead, allow: false}, {user: teamGitOps, object: hostTeam1, action: read, allow: false}, + {user: teamGitOps, object: hostTeam1, action: list, allow: false}, + {user: teamGitOps, object: hostTeam1, action: selectiveList, allow: true}, + {user: teamGitOps, object: hostTeam1, action: selectiveRead, allow: true}, {user: teamGitOps, object: hostTeam1, action: write, allow: false}, {user: teamGitOps, object: hostTeam2, action: read, allow: false}, {user: teamGitOps, object: hostTeam2, action: write, allow: false}, + {user: teamGitOps, object: hostTeam2, action: selectiveRead, allow: false}, }) } @@ -1321,9 +1368,9 @@ func TestAuthorizeMDMConfigProfile(t *testing.T) { {user: test.UserObserverPlus, object: team1Profile, action: read, allow: false}, {user: test.UserGitOps, object: globalProfile, action: write, allow: true}, - {user: test.UserGitOps, object: globalProfile, action: read, allow: false}, + {user: test.UserGitOps, object: globalProfile, action: read, allow: true}, {user: test.UserGitOps, object: team1Profile, action: write, allow: true}, - {user: test.UserGitOps, object: team1Profile, action: read, allow: false}, + {user: test.UserGitOps, object: team1Profile, action: read, allow: true}, {user: test.UserTeamAdminTeam1, object: globalProfile, action: write, allow: false}, {user: test.UserTeamAdminTeam1, object: globalProfile, action: read, allow: false}, @@ -1368,7 +1415,7 @@ func TestAuthorizeMDMConfigProfile(t *testing.T) { {user: test.UserTeamGitOpsTeam1, object: globalProfile, action: write, allow: false}, {user: test.UserTeamGitOpsTeam1, object: globalProfile, action: read, allow: false}, {user: test.UserTeamGitOpsTeam1, object: team1Profile, action: write, allow: true}, - {user: test.UserTeamGitOpsTeam1, object: team1Profile, action: read, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: team1Profile, action: read, allow: true}, {user: test.UserTeamGitOpsTeam2, object: globalProfile, action: write, allow: false}, {user: test.UserTeamGitOpsTeam2, object: globalProfile, action: read, allow: false}, diff --git a/server/fleet/authz.go b/server/fleet/authz.go index f73967aced..e94f902202 100644 --- a/server/fleet/authz.go +++ b/server/fleet/authz.go @@ -31,4 +31,15 @@ const ( ActionRun = "run" // ActionRunNew is the action for running a new live query. ActionRunNew = "run_new" + + // + // Selective prefixes over actions mean that they can be allowed in + // specific cases for roles that usually aren't allowed to perform + // them. + // + + // ActionSelectiveRead allows targeted read access of an entity. + ActionSelectiveRead = "selective_read" + // ActionSelectiveList allows targeted list access of an entity. + ActionSelectiveList = "selective_list" ) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 534be43501..b6ef18f0ac 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -1841,6 +1841,10 @@ type bootstrapPackageMetadataRequest struct { // "write" instead of a "read", this is needed specifically for the gitops // user which is a write-only user, but needs to call this endpoint to check // if it needs to upload the bootstrap package (if the hashes are different). + // + // NOTE: this parameter is going to be removed in a future version. + // Prefer other ways to allow gitops read access. + // For context, see: https://github.com/fleetdm/fleet/issues/15337#issuecomment-1932878997 ForUpdate bool `query:"for_update,optional"` } diff --git a/server/service/hosts.go b/server/service/hosts.go index 8142be8614..7d030d0d00 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -667,7 +667,7 @@ func hostByIdentifierEndpoint(ctx context.Context, request interface{}, svc flee } func (svc *Service) HostByIdentifier(ctx context.Context, identifier string, opts fleet.HostDetailOptions) (*fleet.HostDetail, error) { - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionSelectiveList); err != nil { return nil, err } @@ -677,7 +677,7 @@ func (svc *Service) HostByIdentifier(ctx context.Context, identifier string, opt } // Authorize again with team loaded now that we have team_id - if err := svc.authz.Authorize(ctx, host, fleet.ActionRead); err != nil { + if err := svc.authz.Authorize(ctx, host, fleet.ActionSelectiveRead); err != nil { return nil, err } diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 55b475134f..a2773a5333 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -3895,7 +3895,7 @@ func (s *integrationEnterpriseTestSuite) TestListSoftware() { require.Equal(t, barPayload.Vulnerabilities[0].ResolvedInVersion, ptr.StringPtr("1.2.3")) } -// TestGitOpsUserActions tests the permissions listed in ../../docs/Using-Fleet/Permissions.md. +// TestGitOpsUserActions tests the MDM permissions listed in ../../docs/Using\ Fleet/manage-access.md func (s *integrationEnterpriseTestSuite) TestGitOpsUserActions() { t := s.T() ctx := context.Background() @@ -3910,7 +3910,7 @@ func (s *integrationEnterpriseTestSuite) TestGitOpsUserActions() { h1, err := s.ds.NewHost(ctx, &fleet.Host{ NodeKey: ptr.String(t.Name() + "1"), UUID: t.Name() + "1", - Hostname: t.Name() + "foo.local", + Hostname: strings.Replace(t.Name()+"foo.local", "/", "_", -1), }) require.NoError(t, err) t1, err := s.ds.NewTeam(ctx, &fleet.Team{ @@ -4043,6 +4043,9 @@ func (s *integrationEnterpriseTestSuite) TestGitOpsUserActions() { // Attempt to retrieve hosts, should fail. s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusForbidden, &listHostsResponse{}) + // Attempt to retrieve a host by identifier should succeed + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/identifier/%s", h1.Hostname), hostByIdentifierRequest{}, http.StatusOK, &getHostResponse{}) + // Attempt to filter hosts using labels, should fail (label ID 6 is the builtin label "All Hosts") s.DoJSON("GET", "/api/latest/fleet/labels/6/hosts", nil, http.StatusForbidden, &listHostsResponse{}) diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index f128fc9805..10afb258aa 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -1315,6 +1315,24 @@ func (s *integrationMDMTestSuite) TestPuppetMatchPreassignProfiles() { ctx := context.Background() t := s.T() + // Use a gitops user for all Puppet actions + u := &fleet.User{ + Name: "GitOps", + Email: "gitops-TestPuppetMatchPreassignProfiles@example.com", + GlobalRole: ptr.String(fleet.RoleGitOps), + } + require.NoError(t, u.SetPassword(test.GoodPassword, 10, 10)) + _, err := s.ds.NewUser(context.Background(), u) + require.NoError(t, err) + s.setTokenForTest(t, "gitops-TestPuppetMatchPreassignProfiles@example.com", test.GoodPassword) + + runWithAdminToken := func(cb func()) { + s.token = s.getTestAdminToken() + cb() + s.token = s.getCachedUserToken("gitops-TestPuppetMatchPreassignProfiles@example.com", test.GoodPassword) + + } + // create a host enrolled in fleet mdmHost, _ := createHostThenEnrollMDM(s.ds, s.server.URL, t) s.runWorker() @@ -1380,26 +1398,28 @@ func (s *integrationMDMTestSuite) TestPuppetMatchPreassignProfiles() { require.NoError(t, err) require.Equal(t, "g1", tm1.Name) - // it create activities for the new team, the profiles assigned to it, - // the host moved to it, and setup assistant - s.lastActivityOfTypeMatches( - fleet.ActivityTypeCreatedTeam{}.ActivityName(), - fmt.Sprintf(`{"team_id": %d, "team_name": %q}`, tm1.ID, tm1.Name), - 0) - s.lastActivityOfTypeMatches( - fleet.ActivityTypeEditedMacosProfile{}.ActivityName(), - fmt.Sprintf(`{"team_id": %d, "team_name": %q}`, tm1.ID, tm1.Name), - 0) - s.lastActivityOfTypeMatches( - fleet.ActivityTypeTransferredHostsToTeam{}.ActivityName(), - fmt.Sprintf(`{"team_id": %d, "team_name": %q, "host_ids": [%d], "host_display_names": [%q]}`, - tm1.ID, tm1.Name, h.ID, h.DisplayName()), - 0) - s.lastActivityOfTypeMatches( - fleet.ActivityTypeChangedMacosSetupAssistant{}.ActivityName(), - fmt.Sprintf(`{"team_id": %d, "name": %q, "team_name": %q}`, - tm1.ID, globalAsstResp.Name, tm1.Name), - 0) + runWithAdminToken(func() { + // it create activities for the new team, the profiles assigned to it, + // the host moved to it, and setup assistant + s.lastActivityOfTypeMatches( + fleet.ActivityTypeCreatedTeam{}.ActivityName(), + fmt.Sprintf(`{"team_id": %d, "team_name": %q}`, tm1.ID, tm1.Name), + 0) + s.lastActivityOfTypeMatches( + fleet.ActivityTypeEditedMacosProfile{}.ActivityName(), + fmt.Sprintf(`{"team_id": %d, "team_name": %q}`, tm1.ID, tm1.Name), + 0) + s.lastActivityOfTypeMatches( + fleet.ActivityTypeTransferredHostsToTeam{}.ActivityName(), + fmt.Sprintf(`{"team_id": %d, "team_name": %q, "host_ids": [%d], "host_display_names": [%q]}`, + tm1.ID, tm1.Name, h.ID, h.DisplayName()), + 0) + s.lastActivityOfTypeMatches( + fleet.ActivityTypeChangedMacosSetupAssistant{}.ActivityName(), + fmt.Sprintf(`{"team_id": %d, "name": %q, "team_name": %q}`, + tm1.ID, globalAsstResp.Name, tm1.Name), + 0) + }) // and the team has the expected profiles profs, err := s.ds.ListMDMAppleConfigProfiles(ctx, &tm1.ID) @@ -1567,6 +1587,17 @@ func (s *integrationMDMTestSuite) TestPuppetRun() { host3, _ := createHostThenEnrollMDM(s.ds, s.server.URL, t) s.runWorker() + // Use a gitops user for all Puppet actions + u := &fleet.User{ + Name: "GitOps", + Email: "gitops-TestPuppetRun@example.com", + GlobalRole: ptr.String(fleet.RoleGitOps), + } + require.NoError(t, u.SetPassword(test.GoodPassword, 10, 10)) + _, err := s.ds.NewUser(context.Background(), u) + require.NoError(t, err) + s.setTokenForTest(t, "gitops-TestPuppetRun@example.com", test.GoodPassword) + // preassignAndMatch simulates the puppet module doing all the // preassign/match calls for a given set of profiles. preassignAndMatch := func(profs []fleet.MDMApplePreassignProfilePayload) { @@ -6798,7 +6829,7 @@ var testBMToken = &nanodep_client.OAuth1Tokens{ AccessTokenExpiry: time.Date(2999, 1, 1, 0, 0, 0, 0, time.UTC), } -// TestGitOpsUserActions tests the MDM permissions listed in ../../docs/Using-Fleet/Permissions.md. +// TestGitOpsUserActions tests the MDM permissions listed in ../../docs/Using\ Fleet/manage-access.md func (s *integrationMDMTestSuite) TestGitOpsUserActions() { t := s.T() ctx := context.Background() @@ -6935,6 +6966,20 @@ func (s *integrationMDMTestSuite) TestGitOpsUserActions() { s.Do("POST", "/api/v1/fleet/mdm/apple/profiles/batch", batchSetMDMAppleProfilesRequest{ Profiles: teamProfiles, }, http.StatusForbidden, "team_id", strconv.Itoa(int(t2.ID))) + + // Attempt to retrieve host profiles fails if the host doesn't belong to the team + h1, err := s.ds.NewHost(ctx, &fleet.Host{ + NodeKey: ptr.String(t.Name() + "1"), + UUID: t.Name() + "1", + Hostname: t.Name() + "foo.local", + }) + require.NoError(t, err) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/mdm/hosts/%d/profiles", h1.ID), getHostRequest{}, http.StatusForbidden, &getHostResponse{}) + + err = s.ds.AddHostsToTeam(ctx, &t1.ID, []uint{h1.ID}) + require.NoError(t, err) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/mdm/hosts/%d/profiles", h1.ID), getHostRequest{}, http.StatusOK, &getHostResponse{}) + } func (s *integrationMDMTestSuite) TestOrgLogo() { diff --git a/server/service/mdm_test.go b/server/service/mdm_test.go index f566b78be8..04bcb4e8d2 100644 --- a/server/service/mdm_test.go +++ b/server/service/mdm_test.go @@ -805,8 +805,8 @@ func TestMDMWindowsConfigProfileAuthz(t *testing.T) { // profiles. "global gitops", &fleet.User{GlobalRole: ptr.String(fleet.RoleGitOps)}, - true, - true, + false, + false, false, false, }, @@ -881,7 +881,7 @@ func TestMDMWindowsConfigProfileAuthz(t *testing.T) { "team gitops, belongs to team", &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleGitOps}}}, true, - true, + false, true, false, },