Incorrect Permissions Check when Creating/Editing Security Policies
Summary
Developers and Maintainers are unable to create or edit security policies, even when they have Developer or Maintainer permissions to the linked Security Policy Project. Currently the error below is being shown even when a security policy project is already linked. This error should only appear if a security policy project has not yet been linked to the development project.
Steps to reproduce
- Have a development project owner link a Security Policy Project to a Development Project.
- Give a second user Maintainer Access to both the Security Policy Project and the Development Project.
- Log in as the second user and try to create or edit a security policy in the UI.
Example Project
What is the current bug behavior?
What is the expected correct behavior?
In the case where a security policy project is already linked, ideally the code would be able to detect whether or not the logged in user has Developer or higher permissions on that security policy project. If so, then they should be able to create and edit policies. If not, then the create and edit policy buttons should not even be shown at all.
Relevant logs and/or screenshots
Output of checks
Results of GitLab environment info
Expand for output related to GitLab environment info
(For installations with omnibus-gitlab package run and paste the output of: `sudo gitlab-rake gitlab:env:info`) (For installations from source run and paste the output of: `sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)
Results of GitLab application Check
Expand for output related to the GitLab application check
(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true
)(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true
)(we will only investigate if the tests are passing)
Possible fixes
-
backend Create a new permission modify_security_policy
inee/app/policies/ee/group_policy.rb
andee/app/policies/ee/project_policy.rb
:
diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb
index f01c6346fc80..794e6721ce90 100644
--- a/ee/app/policies/ee/group_policy.rb
+++ b/ee/app/policies/ee/group_policy.rb
@@ -177,6 +177,18 @@ module GroupPolicy
@user.banned_from_namespace?(root_namespace)
end
+ condition(:security_policy_project_available) do
+ @subject.security_orchestration_policy_configuration.present?
+ end
+
+ condition(:can_access_security_policy_project) do
+ security_orchestration_policy_configuration = @subject.security_orchestration_policy_configuration
+
+ next unless security_orchestration_policy_configuration
+
+ Ability.allowed?(@user, :developer_access, security_orchestration_policy_configuration.security_policy_management_project)
+ end
+
# Owners can be banned from their own group except for top-level group
# owners. This exception is made at the service layer
# (Users::Abuse::GitAbuse::NamespaceThrottleService) where the ban record
@@ -394,6 +406,15 @@ module GroupPolicy
enable :read_security_orchestration_policies
end
+ rule { security_orchestration_policies_enabled & can?(:owner_access) }.policy do
+ enable :update_security_orchestration_policy_project
+ enable :modify_security_policy
+ end
+
+ rule { security_orchestration_policies_enabled & security_policy_project_available & can_access_security_policy_project }.policy do
+ enable :modify_security_policy
+ end
+
rule { security_dashboard_enabled & developer }.policy do
enable :read_group_security_dashboard
enable :admin_vulnerability
@@ -511,10 +532,6 @@ module GroupPolicy
rule { can?(:owner_access) & external_audit_events_available }.policy do
enable :admin_external_audit_events
end
-
- rule { security_orchestration_policies_enabled & can?(:owner_access) }.policy do
- enable :update_security_orchestration_policy_project
- end
end
override :lookup_access_level!
diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb
index cbbee3fab8e4..ce1105c48657 100644
--- a/ee/app/policies/ee/project_policy.rb
+++ b/ee/app/policies/ee/project_policy.rb
@@ -160,6 +160,20 @@ module ProjectPolicy
@subject.group && (@subject.group.membership_lock? || ::Gitlab::CurrentSettings.lock_memberships_to_ldap?)
end
+ with_scope :subject
+ condition(:security_policy_project_available) do
+ @subject.security_orchestration_policy_configuration.present?
+ end
+
+ with_scope :subject
+ condition(:can_access_security_policy_project) do
+ security_orchestration_policy_configuration = @subject.security_orchestration_policy_configuration
+
+ next unless security_orchestration_policy_configuration
+
+ Ability.allowed?(@user, :developer_access, security_orchestration_policy_configuration.security_policy_management_project)
+ end
+
rule { membership_locked_via_parent_group }.policy do
prevent :import_project_members_from_another_project
end
@@ -265,12 +279,17 @@ module ProjectPolicy
enable :read_security_orchestration_policies
end
+ rule { security_orchestration_policies_enabled & auditor }.policy do
+ enable :read_security_orchestration_policies
+ end
+
rule { security_orchestration_policies_enabled & can?(:owner_access) }.policy do
+ enable :modify_security_policy
enable :update_security_orchestration_policy_project
end
- rule { security_orchestration_policies_enabled & auditor }.policy do
- enable :read_security_orchestration_policies
+ rule { security_orchestration_policies_enabled & security_policy_project_available & can_access_security_policy_project }.policy do
+ enable :modify_security_policy
end
rule { security_dashboard_enabled & can?(:developer_access) }.policy do
-
backend Update CommitScanExecutionPolicy mutation to use the modify_security_policy
permission instead ofread_security_orchestration_policies
-
frontend Use modify_security_policy
in PolicyHeader and PolicySelection components