Skip to content

Fix runner exhibition

Di Wu requested to merge gitlab-jh/jh-team/gitlab:fix_runner_exhibition into master

What does this MR do and why?

Hi, Gitlab

We found one issue related with runner exhibition.

Here is the background:
Firstly, UserA create GroupA and GroupB, and invite UserB join GroupA to be owner and GroupA join GroupB respectively. After that, UserA register a runner in GroupB. However, when switch account to UserB, UserB can not see and modify runner in GroupB.

after reading the code, we found the key method ci_namespace_mirrors_for_group_members(level) of authority check, which will filter member who has group owners and return these groups id. following is the ci_namespace_mirrors_for_group_members(level) method code

 # gitlab/app/models/user.rb
  def ci_namespace_mirrors_for_group_members(level)
    search_members = group_members.where('access_level >= ?', level)

    # This reduces searched prefixes to only shortest ones
    # to avoid querying descendants since they are already covered
    # by ancestor namespaces. If the FF is not available fallback to
    # inefficient search: https://gitlab.com/gitlab-org/gitlab/-/issues/336436
    unless Feature.enabled?(:use_traversal_ids)
      return Ci::NamespaceMirror.contains_any_of_namespaces(search_members.pluck(:source_id))
    end

    traversal_ids = Group.joins(:all_group_members)
      .merge(search_members)
      .shortest_traversal_ids_prefixes

    Ci::NamespaceMirror.contains_traversal_ids(traversal_ids)
  end

The variable: group_members in this method only return GroupA, so we need to add a method to check if GroupA is owner in other groups. So we need to introduce group_group_links table to search. If GroupA is also other group's ownner, we need to record other group's id. So when runner policy checks the authority, UserB has GroupA and GroupB id, and GroupB has the runner, it will authorize UserB to see and modify runner in GroupB.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests. the link of GroupA user members: https://gitlab.com/groups/groupa8414205/-/group_members image the link of GroupB group members: https://gitlab.com/groups/groupb8614205/-/group_members?tab=groups image the link of GroupB runner in account ws665544: https://gitlab.com/groups/groupb8614205/-/runners image the link of GroupB runner in account wd665544: https://gitlab.com/groups/groupb8614205/-/runners image

How to set up and validate locally

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

SQL and Query Plan

we convey two parameters(source_ids, level)

SELECT
    "group_group_links".*
FROM
    "group_group_links"
WHERE
    "group_group_links"."shared_with_group_id" = #{source_ids}
    AND (group_access >= #{level})

MR acceptance checklist

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

Edited by Di Wu

Merge request reports