`securityPolicyProjectAssign` mutation does not authorize security policy project ID

The securityPolicyProjectAssign mutation does not authorize its securityPolicyProjectId parameter. This lets any EE-licensed user link any security policy project by its ID to projects/groups the user has access to, revealing the security project's configured security policies.

Steps to reproduce

  • Have an EE license
  • Impersonate User A
  • Create a new group demo-corp
  • On the group-level, navigate to Secure > Policies and create the following example policy (adjust the user_approvers):
type: scan_result_policy
name: Container Scanning
description: ''
enabled: true
rules:
  - type: scan_finding
    branches: []
    scanners:
      - container_scanning
    vulnerabilities_allowed: 0
    severity_levels: []
    vulnerability_states: []
actions:
  - type: require_approval
    approvals_required: 1
    user_approvers: [toya.bauch]
  • note the ID of the newly created demo-corp/demo-corp-security-policy-project (say 41)
  • Impersonate User B
  • Create a new group other-corp
  • Execute the following GraphQL mutation with the security policy project ID replaced:
mutation {
  securityPolicyProjectAssign(input: { fullPath: "other-corp", securityPolicyProjectId: "gid://gitlab/Project/41" }) {
    errors
  }
}

Implementation Plan

Since we already have a find_object method that loads the project or group, using authorized_find! to get the security_project_policy didn't work. I tried to use the authorized_find! and noticed our find_object method is called and requires a project_path full_path. I believe the easiest way to accomplish what we need is to call the authorize! method passing the policy_project.

===================================================================
diff --git a/ee/app/graphql/mutations/security_policy/assign_security_policy_project.rb b/ee/app/graphql/mutations/security_policy/assign_security_policy_project.rb
--- a/ee/app/graphql/mutations/security_policy/assign_security_policy_project.rb	(revision 0616042ba9e403381ea9fe28ecf6dead16fd67e1)
+++ b/ee/app/graphql/mutations/security_policy/assign_security_policy_project.rb	(date 1689711403440)
@@ -30,6 +30,7 @@
 
         policy_project = find_policy_project(args[:security_policy_project_id])
         raise_resource_not_available_error! unless policy_project.present?
+        authorize!(policy_project)
 
         result = assign_project(project_or_group, policy_project)
         {

This patch seems to work as expected using the GraphQL explorer, but it did break some tests.

I believe we should fix the specs since the owner context seems not to work as expected. Debugging the spec, I found the current_user was different from the subject owner as shown below:

[3] pry(#<Gitlab::Graphql::Authorize::ObjectAuthorization>)> current_user
=> #<User id:31 @user1>
[4] pry(#<Gitlab::Graphql::Authorize::ObjectAuthorization>)> subject
=> #<Project id:22 namespace1/project-2>>
[5] pry(#<Gitlab::Graphql::Authorize::ObjectAuthorization>)> subject.owner
=> #<User id:33 @namespace1>
[6] pry(#<Gitlab::Graphql::Authorize::ObjectAuthorization>)>

Verification steps

Follow the steps described here and verify an error message is returned when userB tries to assign the security project created by userA.

Edited by Marcos Rocha