Skip to content

Prevent group owners from deleting certain project runners

Pedro Pombeiro requested to merge pedropombeiro/354004/prevent-group-owners-from-deleting-other-project-runners into master

What does this MR do and why?

Describe in detail what your merge request does and why.

This MR makes a subtle change to the ability to delete a project runner:

  • Currently, no one is allowed to delete a project runner from a group runners page, as long as the runner is associated with more than one project.
  • With this MR, an admin will be able to delete a project runner even when it is associated with multiple projects, but the group owner will not.
  • The logic to determine whether a runner can be deleted will be moved to the RunnerPolicy so that the GraphQL userPermissions.deleteRunner query returns the right value.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

Scenario http://gdk.test:3000/-/graphql-explorer http://gdk.test:3000/groups/h5bp/-/settings/ci_cd#runners-settings
Signed in as group owner 2022-03-09_at_17.15
Signed in as admin

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. Sign in one browser as root in http://gdk.test:3000

  2. Sign in on a second browser window (InPrivate mode so that cookies aren't shared) as a second user. Make sure that this user is an owner of the target group (e.g. http://gdk.test:3000/groups/h5bp)

  3. Register a GitLab runner against a project inside that group (e.g. http://gdk.test:3000/h5bp/html5-boilerplate/-/settings/ci_cd#js-runners-settings):

    gitlab-runner register -config ~/.gitlab-runner/config.gdk.toml \
        --executor "shell" \
        --url "http://gdk.test:3000/" \
        --description "Project test runner" \
        --tag-list "shell,gdk,test" \
        --run-untagged="false" \
        --locked="false" \
        --access-level="not_protected" --non-interactive \
        --registration-token="$REGISTRATION_TOKEN"

  4. Enable that project runner in a second, unrelated project (e.g. http://gdk.test:3000/gitlab-org/playground/-/settings/ci_cd):

  5. Visit http://gdk.test:3000/groups/h5bp/-/settings/ci_cd#runners-settings in each of the browser windows

    • The session where the admin is signed in should be allowed to delete the runner
    • The session where the group owner is signed in should NOT be allowed to delete the runner
GraphQL query 
query {
  currentUser {
    username
  }
  group(fullPath:"h5bp") {
    name
    runners(membership: DESCENDANTS) {
      nodes {
        shortSha
        description
        userPermissions {
          deleteRunner
        }
      }
    }
  }
}

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Closes Prevent group owners from deleting project runn... (#354004 - closed)

Edited by Pedro Pombeiro

Merge request reports