Skip to content

Fix effective access level of group members

Jonas Wälter requested to merge siemens/gitlab:fix/group_members_max_access_level into master

What does this MR do?

Resolves #249523 (closed)

If a user is a member of both a group and its subgroup but with different access levels, the property access_level is not properly inherited from the parent group. Instead, the wrong effective access_level is shown in the subgroup (UI and API).

This bug is known described in the API documentation:

Due to an issue, the users effective access_level may actually be higher than returned value when listing group members.

https://docs.gitlab.com/ee/api/members.html#list-all-members-of-a-group-or-project-including-inherited-members

This bug will be fixed by this MR.

🛠 with ❤ at Siemens

/cc @bufferoverflow

Screenshots

Steps to reproduce (#249523 (closed)):

  1. create a group parent
  2. create a group sub in the parent namespace
  3. in the group sub add a member with an access_level of 20 (Reporter)
  4. in the group parent add same member with an access_level of 30 (Developer)

Note that 3. and 4. steps should not be swapped because this would show a validation error.

➡ Member should have the effective access level 30 (Developer) in group sub.

before MR after MR
Group_members_access_level_before Group_members_access_level_after

API

GET /api/v4/groups/:id_of_subgroup/members/all

Response: before MR 
[
    ...
    {
        "id": 55,
        "name": "Altha Huel",
        "access_level": 20,
        ...
    }
]
Response: after MR 
[
    ...
    {
        "id": 55,
        "name": "Altha Huel",
        "access_level": 30,
        ...
    }
]

Database Review

GroupMembersFinder.new(sub_group).execute:

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN ( WITH RECURSIVE "base_and_ancestors" AS (
(
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = 123)
                UNION (
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces",
                        "base_and_ancestors"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = "base_and_ancestors"."parent_id"))
                SELECT
                    "id"
                FROM
                    "base_and_ancestors" AS "namespaces")
                AND (members.access_level > 5)
            ORDER BY
                user_id,
                invite_email,
                access_level DESC,
                expires_at DESC,
                created_at ASC) members

GroupMembersFinder.new(sub_group).execute(include_relations: [:direct]):

Raw SQL 
SELECT
    "members".*
FROM
    "members"
WHERE
    "members"."type" = 'GroupMember'
    AND "members"."source_id" = 123
    AND "members"."source_type" = 'Namespace'
    AND "members"."requested_at" IS NULL
    AND "members"."access_level" != 5

GroupMembersFinder.new(sub_group).execute(include_relations: [:direct, :inherited]):

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN ( WITH RECURSIVE "base_and_ancestors" AS (
(
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = 123)
                UNION (
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces",
                        "base_and_ancestors"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = "base_and_ancestors"."parent_id"))
                SELECT
                    "id"
                FROM
                    "base_and_ancestors" AS "namespaces")
                AND (members.access_level > 5)
            ORDER BY
                user_id,
                invite_email,
                access_level DESC,
                expires_at DESC,
                created_at ASC) members

GroupMembersFinder.new(sub_group).execute(include_relations: [:direct, :descendants]):

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN ( WITH RECURSIVE "base_and_descendants" AS (
(
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = 123)
                UNION (
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces",
                        "base_and_descendants"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."parent_id" = "base_and_descendants"."id"))
                SELECT
                    "id"
                FROM
                    "base_and_descendants" AS "namespaces")
                AND (members.access_level > 5)
            ORDER BY
                user_id,
                invite_email,
                access_level DESC,
                expires_at DESC,
                created_at ASC) members

GroupMembersFinder.new(sub_group).execute(include_relations: [:direct, :descendants, :inherited]):

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN ( WITH RECURSIVE "base_and_ancestors" AS (
(
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = 123)
                UNION (
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces",
                        "base_and_ancestors"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = "base_and_ancestors"."parent_id")),
                "base_and_descendants" AS (
(
                        SELECT
                            "namespaces".*
                        FROM
                            "namespaces"
                        WHERE
                            "namespaces"."type" = 'Group'
                            AND "namespaces"."id" = 123)
                    UNION (
                        SELECT
                            "namespaces".*
                        FROM
                            "namespaces",
                            "base_and_descendants"
                        WHERE
                            "namespaces"."type" = 'Group'
                            AND "namespaces"."parent_id" = "base_and_descendants"."id"))
                    SELECT
                        "namespaces"."id"
                    FROM ((
                            SELECT
                                "namespaces".*
                            FROM
                                "base_and_ancestors" AS "namespaces"
                            WHERE
                                "namespaces"."type" = 'Group')
                        UNION (
                            SELECT
                                "namespaces".*
                            FROM
                                "base_and_descendants" AS "namespaces"
                            WHERE
                                "namespaces"."type" = 'Group')) namespaces
                    WHERE
                        "namespaces"."type" = 'Group')
                    AND (members.access_level > 5)
                ORDER BY
                    user_id,
                    invite_email,
                    access_level DESC,
                    expires_at DESC,
                    created_at ASC) members

GroupMembersFinder.new(sub_group).execute(include_relations: [:inherited]):

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN ( WITH RECURSIVE "base_and_ancestors" AS (
(
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = 122)
                UNION (
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces",
                        "base_and_ancestors"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."id" = "base_and_ancestors"."parent_id"))
                SELECT
                    "id"
                FROM
                    "base_and_ancestors" AS "namespaces")
                AND (members.access_level > 5)
            ORDER BY
                user_id,
                invite_email,
                access_level DESC,
                expires_at DESC,
                created_at ASC) members

GroupMembersFinder.new(sub_group).execute(include_relations: [:descendants]):

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN ( WITH RECURSIVE "base_and_descendants" AS (
(
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."parent_id" = 123)
                UNION (
                    SELECT
                        "namespaces".*
                    FROM
                        "namespaces",
                        "base_and_descendants"
                    WHERE
                        "namespaces"."type" = 'Group'
                        AND "namespaces"."parent_id" = "base_and_descendants"."id"))
                SELECT
                    "id"
                FROM
                    "base_and_descendants" AS "namespaces")
                AND (members.access_level > 5)
            ORDER BY
                user_id,
                invite_email,
                access_level DESC,
                expires_at DESC,
                created_at ASC) members

GroupMembersFinder.new(sub_group).execute(include_relations: [:descendants, :inherited]):

Raw SQL 
SELECT
    "members".*
FROM ( SELECT DISTINCT ON (user_id, invite_email)
        *
    FROM
        "members"
    WHERE
        "members"."type" = 'GroupMember'
        AND "members"."source_type" = 'Namespace'
        AND "members"."requested_at" IS NULL
        AND "members"."source_id" IN (
            SELECT
                "namespaces"."id"
            FROM (( WITH RECURSIVE "base_and_ancestors" AS (
(
                            SELECT
                                "namespaces".*
                            FROM
                                "namespaces"
                            WHERE
                                "namespaces"."type" = 'Group'
                                AND "namespaces"."id" = 122)
                        UNION (
                            SELECT
                                "namespaces".*
                            FROM
                                "namespaces",
                                "base_and_ancestors"
                            WHERE
                                "namespaces"."type" = 'Group'
                                AND "namespaces"."id" = "base_and_ancestors"."parent_id"))
                        SELECT
                            "namespaces".*
                        FROM
                            "base_and_ancestors" AS "namespaces")
                    UNION ( WITH RECURSIVE "base_and_descendants" AS (
(
                                SELECT
                                    "namespaces".*
                                FROM
                                    "namespaces"
                                WHERE
                                    "namespaces"."type" = 'Group'
                                    AND "namespaces"."parent_id" = 123)
                            UNION (
                                SELECT
                                    "namespaces".*
                                FROM
                                    "namespaces",
                                    "base_and_descendants"
                                WHERE
                                    "namespaces"."type" = 'Group'
                                    AND "namespaces"."parent_id" = "base_and_descendants"."id"))
                            SELECT
                                "namespaces".*
                            FROM
                                "base_and_descendants" AS "namespaces")) namespaces
                    WHERE
                        "namespaces"."type" = 'Group')
                    AND (members.access_level > 5)
                ORDER BY
                    user_id,
                    invite_email,
                    access_level DESC,
                    expires_at DESC,
                    created_at ASC) members

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 Jonas Wälter

Merge request reports