Skip to content

Include group links in access level check

mo khan requested to merge mokhax/434044/add-member-validation into master

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:

Screenshots or screen recordings

Before After
image image
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

  1. Create two top-level groups
    • Group1
    • Group2
  2. Invite another user to Group2 as an Owner
  3. Invite Group2 to Group1 with an Owner role
  4. Have the other user Invite a member
  5. Choose a user that is not a member of either groups
  6. Select a role
  7. 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.

Edited by mo khan

Merge request reports