You need to sign in or sign up before continuing.
Fix effective access level of group members
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.
This bug will be fixed by this MR.
/cc @bufferoverflow
Screenshots
Steps to reproduce (#249523 (closed)):
- create a group
parent- create a group
subin theparentnamespace- in the group
subadd a member with anaccess_levelof20(Reporter)- in the group
parentadd same member with anaccess_levelof30(Developer)Note that
3.and4.steps should not be swapped because this would show a validation error.
30 (Developer) in group sub.
| before MR | after MR |
|---|---|
 |
 |
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) membersGroupMembersFinder.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" != 5GroupMembersFinder.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) membersGroupMembersFinder.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) membersGroupMembersFinder.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) membersGroupMembersFinder.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) membersGroupMembersFinder.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) membersGroupMembersFinder.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) membersDoes this MR meet the acceptance criteria?
Conformity
-
📋 Does this MR need a changelog?- I have included a changelog entry.
- Documentation (if required)
- Code review guidelines
- Merge request performance guidelines
- Style guides
- Database guides
- [-] Separation of EE specific content
Availability and Testing
- Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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