Include group links in access level check
What does this MR do and why?
When adding a new member to a group, in some cases, the current user may inherit the owner role via a group link. This change ensures that the group link is recognized and allows these users to invite new members to a group.
Related to:
- #434044 (closed)
- https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/3694
- !139191 (merged)
Screenshots or screen recordings
Before | After |
---|---|
ArgumentError (comparison of Integer with nil failed):
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/ee/app/services/ee/members/groups/creator_service.rb:47:in `>'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/ee/app/services/ee/members/groups/creator_service.rb:47:in `member_role_too_high?'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:163:in `commit_member'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/ee/app/services/ee/members/creator_service.rb:42:in `commit_member'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:143:in `execute'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:64:in `block (4 levels) in add_members'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:63:in `map'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:63:in `block (3 levels) in add_members'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:46:in `each'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:46:in `flat_map'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:46:in `block (2 levels) in add_members'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/models/concerns/cross_database_modification.rb:92:in `block in transaction'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8/lib/active_record/connection_adapters/abstract/transaction.rb:319:in `block in within_new_transaction'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8/lib/active_record/connection_adapters/abstract/transaction.rb:317:in `within_new_transaction'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8/lib/active_record/connection_adapters/abstract/database_statements.rb:316:in `transaction'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `public_send'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `block in write_using_load_balancer'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/load_balancer.rb:141:in `block in read_write'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/load_balancer.rb:228:in `retry_with_backoff'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/load_balancer.rb:130:in `read_write'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:126:in `write_using_load_balancer'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/load_balancing/connection_proxy.rb:78:in `transaction'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8/lib/active_record/transactions.rb:209:in `transaction'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database.rb:359:in `block in transaction'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/notifications.rb:206:in `block in instrument'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
/home/mokhax/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8/lib/active_support/notifications.rb:206:in `instrument'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database.rb:358:in `transaction'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/models/concerns/cross_database_modification.rb:83:in `transaction'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:45:in `block in add_members'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb:29:in `temporary_ignore_tables_in_transaction'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/creator_service.rb:42:in `add_members'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/create_service.rb:86:in `add_members'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/services/members/create_service.rb:31:in `execute'
/home/mokhax/src/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/lib/api/invitations.rb:49:in `block (3 levels) in <class:Invitations>'
How to set up and validate locally
- Create two top-level groups
- Group1
- Group2
- Invite another user to
Group2
as anOwner
- Invite
Group2
toGroup1
with anOwner
role - Have the other user Invite a member
- Choose a user that is not a member of either groups
- Select a role
- Click the
Invite
button
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by mo khan