Authorize branch rules on read_branch_rule

What does this MR do and why?

Fix branchRules returning the "All branches" and "All protected branches" pseudo-rules to users who can't read them.

Types::Projects::BranchRuleType authorized on read_protected_branch, which passes through to the project. Users with the admin_protected_branch custom role have read_protected_branch, so they could see both pseudo-rules. The policies for those pseudo-rules deny read_branch_rule to that role. Only Maintainers and the manage_merge_request_settings custom role can read them.

This MR makes two changes:

  • BranchRuleType now authorizes on read_branch_rule. approvalRules and externalStatusChecks keep their own field-level permissions (manage_merge_request_settings and read_external_status_check).
  • The branchRules resolver checks read_branch_rule on each pseudo-rule before it builds the page and drops any the user can't read. CE handles "All branches" and EE handles "All protected branches".
Why filter in the resolver as well as the type

GraphQL type authorization runs after the page is built. If we only filtered at type level we'd return short or empty pages. For example, first: 1 for an admin_protected_branch user returned an empty page because the only item was "All branches", and that item was then filtered out.

The resolver uses Ability.allowed? so a signed-out (nil) user is handled.

Request specs cover both custom roles (admin_protected_branch and manage_merge_request_settings) across every combination of persisted/unpersisted "All protected branches" rule and buildMissing: true/false. They also cover page size with first: 1 and a signed-out user.

This MR depends on !258973 (merged), which must merge first.

References

Screenshots or screen recordings

N/A, backend only change.

Before After

How to set up and validate locally

1. Set up the project

  1. Create a group and a project inside it.
  2. Protect one branch in the project.
  3. Go to Settings > Merge requests and add an approval rule that targets all protected branches.

2. Create the custom role

  1. Create a custom role based on Guest with Manage Protected Branches (admin_protected_branch).
  2. Add a user to the project with that role.

3. Query as the admin_protected_branch user

  1. Sign in as that user and open /-/graphql-explorer.

  2. Run:

    query {
      project(fullPath: "<path>") {
        branchRules(buildMissing: true) {
          nodes {
            name
            approvalRules { nodes { name } }
            externalStatusChecks { nodes { name } }
          }
        }
      }
    }

Expected: Only the protected branch rule is returned. approvalRules and externalStatusChecks are null.

4. Check page size

  1. Run the same query with branchRules(buildMissing: true, first: 1).

Expected: One item is returned and it's the protected branch rule, not an empty page.

5. Query as a manage_merge_request_settings user

  1. Create a second custom role based on Guest with Manage merge request approvals and settings (manage_merge_request_settings), and assign it to the user instead.
  2. Run the query from step 3 again.

Expected: "All branches", "All protected branches" and the protected branch are all returned. approvalRules and externalStatusChecks return data instead of null, and "All protected branches" lists the approval rule from step 1.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Joe Woodward

Merge request reports

Loading
Loading