From fdee411b589e64759367d7599ecb28ba58280149 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Tue, 25 Jun 2024 16:22:51 -0400 Subject: [PATCH] fix: don't allow observer and observer+ to download software installers (#19938) > Related issue: https://github.com/fleetdm/confidential/issues/6979 # 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/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/6979-observer-software | 1 + server/authz/policy.rego | 8 ++++---- server/authz/policy_test.go | 16 ++++++++-------- server/service/software_installers_test.go | 12 ++++++------ 4 files changed, 19 insertions(+), 18 deletions(-) create mode 100644 changes/6979-observer-software diff --git a/changes/6979-observer-software b/changes/6979-observer-software new file mode 100644 index 0000000000..005d4a8eaf --- /dev/null +++ b/changes/6979-observer-software @@ -0,0 +1 @@ +- Bug fix: do not allow Observer and Observer+ roles to download software installers. \ No newline at end of file diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 1836162200..8f9053472c 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -643,10 +643,10 @@ allow { action == read } -# Global admins, maintainers, observers, and observer_plus can read any software installer. +# Global admins and maintainers can read any software installer. allow { object.type == "software_installer" - subject.global_role == [admin, maintainer, observer, observer_plus][_] + subject.global_role == [admin, maintainer][_] action == read } @@ -657,11 +657,11 @@ allow { action == write } -# Team admins, maintainers, observers, and observer_plus can read any software installer in their teams. +# Team admins and maintainers can read any software installer in their teams. allow { not is_null(object.team_id) object.type == "software_installer" - team_role(subject, object.team_id) == [admin, maintainer, observer, observer_plus][_] + team_role(subject, object.team_id) == [admin, maintainer][_] action == read } diff --git a/server/authz/policy_test.go b/server/authz/policy_test.go index 8f81c482b5..2ab7980d13 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -535,18 +535,18 @@ func TestAuthorizeSoftwareInstaller(t *testing.T) { {user: test.UserMaintainer, object: team2Installer, action: read, allow: true}, {user: test.UserMaintainer, object: team2Installer, action: write, allow: true}, - {user: test.UserObserver, object: noTeamInstaller, action: read, allow: true}, + {user: test.UserObserver, object: noTeamInstaller, action: read, allow: false}, {user: test.UserObserver, object: noTeamInstaller, action: write, allow: false}, - {user: test.UserObserver, object: team1Installer, action: read, allow: true}, + {user: test.UserObserver, object: team1Installer, action: read, allow: false}, {user: test.UserObserver, object: team1Installer, action: write, allow: false}, - {user: test.UserObserver, object: team2Installer, action: read, allow: true}, + {user: test.UserObserver, object: team2Installer, action: read, allow: false}, {user: test.UserObserver, object: team2Installer, action: write, allow: false}, - {user: test.UserObserverPlus, object: noTeamInstaller, action: read, allow: true}, + {user: test.UserObserverPlus, object: noTeamInstaller, action: read, allow: false}, {user: test.UserObserverPlus, object: noTeamInstaller, action: write, allow: false}, - {user: test.UserObserverPlus, object: team1Installer, action: read, allow: true}, + {user: test.UserObserverPlus, object: team1Installer, action: read, allow: false}, {user: test.UserObserverPlus, object: team1Installer, action: write, allow: false}, - {user: test.UserObserverPlus, object: team2Installer, action: read, allow: true}, + {user: test.UserObserverPlus, object: team2Installer, action: read, allow: false}, {user: test.UserObserverPlus, object: team2Installer, action: write, allow: false}, // TODO: confirm gitops permissions @@ -581,14 +581,14 @@ func TestAuthorizeSoftwareInstaller(t *testing.T) { {user: test.UserTeamObserverTeam1, object: noTeamInstaller, action: read, allow: false}, {user: test.UserTeamObserverTeam1, object: noTeamInstaller, action: write, allow: false}, - {user: test.UserTeamObserverTeam1, object: team1Installer, action: read, allow: true}, + {user: test.UserTeamObserverTeam1, object: team1Installer, action: read, allow: false}, {user: test.UserTeamObserverTeam1, object: team1Installer, action: write, allow: false}, {user: test.UserTeamObserverTeam1, object: team2Installer, action: read, allow: false}, {user: test.UserTeamObserverTeam1, object: team2Installer, action: write, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: noTeamInstaller, action: read, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: noTeamInstaller, action: write, allow: false}, - {user: test.UserTeamObserverPlusTeam1, object: team1Installer, action: read, allow: true}, + {user: test.UserTeamObserverPlusTeam1, object: team1Installer, action: read, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: team1Installer, action: write, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: team2Installer, action: read, allow: false}, {user: test.UserTeamObserverPlusTeam1, object: team2Installer, action: write, allow: false}, diff --git a/server/service/software_installers_test.go b/server/service/software_installers_test.go index e70050b766..3023773cf7 100644 --- a/server/service/software_installers_test.go +++ b/server/service/software_installers_test.go @@ -33,10 +33,10 @@ func TestSoftwareInstallersAuth(t *testing.T) { {"global admin team", test.UserAdmin, ptr.Uint(1), false, false}, {"global maintainer no team", test.UserMaintainer, nil, false, false}, {"global maintainer team", test.UserMaintainer, ptr.Uint(1), false, false}, - {"global observer no team", test.UserObserver, nil, false, true}, - {"global observer team", test.UserObserver, ptr.Uint(1), false, true}, - {"global observer+ no team", test.UserObserverPlus, nil, false, true}, - {"global observer+ team", test.UserObserverPlus, ptr.Uint(1), false, true}, + {"global observer no team", test.UserObserver, nil, true, true}, + {"global observer team", test.UserObserver, ptr.Uint(1), true, true}, + {"global observer+ no team", test.UserObserverPlus, nil, true, true}, + {"global observer+ team", test.UserObserverPlus, ptr.Uint(1), true, true}, {"global gitops no team", test.UserGitOps, nil, true, false}, {"global gitops team", test.UserGitOps, ptr.Uint(1), true, false}, {"team admin no team", test.UserTeamAdminTeam1, nil, true, true}, @@ -46,10 +46,10 @@ func TestSoftwareInstallersAuth(t *testing.T) { {"team maintainer team", test.UserTeamMaintainerTeam1, ptr.Uint(1), false, false}, {"team maintainer other team", test.UserTeamMaintainerTeam2, ptr.Uint(1), true, true}, {"team observer no team", test.UserTeamObserverTeam1, nil, true, true}, - {"team observer team", test.UserTeamObserverTeam1, ptr.Uint(1), false, true}, + {"team observer team", test.UserTeamObserverTeam1, ptr.Uint(1), true, true}, {"team observer other team", test.UserTeamObserverTeam2, ptr.Uint(1), true, true}, {"team observer+ no team", test.UserTeamObserverPlusTeam1, nil, true, true}, - {"team observer+ team", test.UserTeamObserverPlusTeam1, ptr.Uint(1), false, true}, + {"team observer+ team", test.UserTeamObserverPlusTeam1, ptr.Uint(1), true, true}, {"team observer+ other team", test.UserTeamObserverPlusTeam2, ptr.Uint(1), true, true}, {"team gitops no team", test.UserTeamGitOpsTeam1, nil, true, true}, {"team gitops team", test.UserTeamGitOpsTeam1, ptr.Uint(1), true, false},