Skip to content

Reduce number of permission checks for subgroups in EpicsFinder [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Jan Provaznik requested to merge epics-sgroup-check into master

What does this MR do?

Skips checking of groups permission one-by-one in these cases:

  • if user is not logged in - in that case we return just public groups
  • if user is logged but not member of parent group - in that case we check if user is member of any related groups - if not, we return only public (and internal) groups

Related to #327454 (closed)

DB query

query plan (cold cache) which checks if there is any group (in related groups) which user is member of: https://console.postgres.ai/gitlab/gitlab-production-tunnel/sessions/3608/commands/12062 (depesz: https://explain.depesz.com/s/HhkV)
WITH RECURSIVE "base_and_descendants" AS (
                                            (SELECT "namespaces".*
                                             FROM "namespaces"
                                             WHERE "namespaces"."type" = 'Group'
                                               AND "namespaces"."id" = 9970)
                                          UNION
                                            (SELECT "namespaces".*
                                             FROM "namespaces",
                                                  "base_and_descendants"
                                             WHERE "namespaces"."type" = 'Group'
                                               AND "namespaces"."parent_id" = "base_and_descendants"."id"))
SELECT 1 AS one
FROM "base_and_descendants" AS "namespaces"
WHERE "namespaces"."id" IN
    (SELECT "namespaces"."id"
     FROM (
             (WITH "direct_groups" AS
                (SELECT "namespaces".*
                 FROM (
                         (SELECT "namespaces".*
                          FROM "namespaces"
                          INNER JOIN "members" ON "namespaces"."id" = "members"."source_id"
                          WHERE "members"."type" = 'GroupMember'
                            AND "members"."source_type" = 'Namespace'
                            AND "namespaces"."type" = 'Group'
                            AND "members"."user_id" = 1642716
                            AND "members"."requested_at" IS NULL
                            AND (access_level >= 10))
                       UNION
                         (SELECT namespaces.*
                          FROM "projects"
                          INNER JOIN "project_authorizations" ON "projects"."id" = "project_authorizations"."project_id"
                          INNER JOIN "namespaces" ON "namespaces"."id" = "projects"."namespace_id"
                          WHERE "project_authorizations"."user_id" = 1642716)) namespaces
                 WHERE "namespaces"."type" = 'Group') SELECT "namespaces".*
              FROM (
                      (SELECT "namespaces".*
                       FROM "direct_groups" "namespaces"
                       WHERE "namespaces"."type" = 'Group')
                    UNION
                      (SELECT "namespaces".*
                       FROM "namespaces"
                       INNER JOIN "group_group_links" ON "group_group_links"."shared_group_id" = "namespaces"."id"
                       WHERE "namespaces"."type" = 'Group'
                         AND "group_group_links"."shared_with_group_id" IN
                           (SELECT "namespaces"."id"
                            FROM "direct_groups" "namespaces"
                            WHERE "namespaces"."type" = 'Group'))) namespaces
              WHERE "namespaces"."type" = 'Group')
           UNION
             (SELECT "namespaces".*
              FROM "namespaces"
              INNER JOIN "members" ON "namespaces"."id" = "members"."source_id"
              WHERE "members"."type" = 'GroupMember'
                AND "members"."source_type" = 'Namespace'
                AND "namespaces"."type" = 'Group'
                AND "members"."user_id" = 1642716
                AND "members"."access_level" = 5)) namespaces
     WHERE "namespaces"."type" = 'Group')
LIMIT 1

Screenshots (strongly suggested)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Jan Provaznik

Merge request reports

Loading