Merge branch...

Merge branch '352777-group-access-tokens-created-in-the-top-level-namespace-are-counted-as-licensed-users' into 'master'

Resolve "Any Group Access Token bot users originating from shared groups should not count as licensed users"

See merge request gitlab-org/gitlab!80889

ee/app/models/ee/group.rb

@@ -584,8 +584,8 @@ module EE
       strong_memoize(:billed_user_ids_excluding_guests) do
         group_member_user_ids = billed_group_users(non_guests: true).distinct.pluck(:id)
         project_member_user_ids = billed_project_users(non_guests: true).distinct.pluck(:id)
-        shared_group_user_ids = billed_shared_non_guests_group_members.non_guests.distinct.pluck(:user_id)
-        shared_project_user_ids = billed_invited_non_guests_group_to_project_members.non_guests.distinct.pluck(:user_id)
+        shared_group_user_ids = billed_shared_non_guests_group_users.distinct.pluck(:id)
+        shared_project_user_ids = billed_invited_non_guests_group_to_project_users.distinct.pluck(:id)
 
         {
           user_ids: (group_member_user_ids + project_member_user_ids + shared_group_user_ids + shared_project_user_ids).to_set,
@@ -601,8 +601,8 @@ module EE
       strong_memoize(:billed_user_ids_including_guests) do
         group_member_user_ids = billed_group_users.distinct.pluck(:id)
         project_member_user_ids = billed_project_users.distinct.pluck(:id)
-        shared_group_user_ids = billed_shared_group_members.distinct.pluck(:user_id)
-        shared_project_user_ids = billed_invited_group_to_project_members.distinct.pluck(:user_id)
+        shared_group_user_ids = billed_shared_group_users.distinct.pluck(:id)
+        shared_project_user_ids = billed_invited_group_to_project_users.distinct.pluck(:id)
 
         {
           user_ids: (group_member_user_ids + project_member_user_ids + shared_group_user_ids + shared_project_user_ids).to_set,
@@ -631,9 +631,7 @@ module EE
 
       members = members.non_guests if non_guests
 
-      user_ids = members.distinct.select(:user_id)
-
-      ::User.without_project_bot.where(id: user_ids)
+      users_without_project_bots(members)
     end
 
     # Members belonging directly to Projects within Group or Projects within subgroups
@@ -642,22 +640,22 @@ module EE
 
       members = members.non_guests if non_guests
 
-      user_ids = members.where(
+      members = members.where(
         source_id: ::Project.joins(:group).where(namespace: self_and_descendants)
       )
-      .distinct
-      .select(:user_id)
 
-      ::User.with_state(:active).without_project_bot.where(id: user_ids)
+      users_without_project_bots(members).with_state(:active)
     end
 
     # Members belonging to Groups invited to collaborate with Projects
-    def billed_invited_group_to_project_members
-      invited_or_shared_group_members(invited_groups_in_projects)
+    def billed_invited_group_to_project_users
+      members = invited_or_shared_group_members(invited_groups_in_projects)
+      users_without_project_bots(members)
     end
 
-    def billed_invited_non_guests_group_to_project_members
-      invited_or_shared_group_members(invited_group_as_non_guests_in_projects)
+    def billed_invited_non_guests_group_to_project_users
+      members = invited_or_shared_group_members(invited_group_as_non_guests_in_projects).non_guests
+      users_without_project_bots(members)
     end
 
     def invited_group_as_non_guests_in_projects
@@ -670,12 +668,14 @@ module EE
     end
 
     # Members belonging to Groups invited to collaborate with Groups and Subgroups
-    def billed_shared_group_members
-      invited_or_shared_group_members(invited_group_in_groups)
+    def billed_shared_group_users
+      members = invited_or_shared_group_members(invited_group_in_groups)
+      users_without_project_bots(members)
     end
 
-    def billed_shared_non_guests_group_members
-      invited_or_shared_group_members(invited_non_guest_group_in_groups)
+    def billed_shared_non_guests_group_users
+      members = invited_or_shared_group_members(invited_non_guest_group_in_groups).non_guests
+      users_without_project_bots(members)
     end
 
     def invited_non_guest_group_in_groups
@@ -691,6 +691,10 @@ module EE
       ::GroupMember.active_without_invites_and_requests.where(source_id: groups.self_and_ancestors)
     end
 
+    def users_without_project_bots(members)
+      ::User.where(id: members.distinct.select(:user_id)).without_project_bot
+    end
+
     override :_safe_read_repository_read_only_column
     def _safe_read_repository_read_only_column
       ::NamespaceSetting.where(namespace: self).pick(:repository_read_only)

ee/spec/models/ee/namespace_spec.rb

@@ -983,6 +983,22 @@ RSpec.describe Namespace do
     end
   end
 
+  shared_context 'bot user for invited_group' do
+    let(:group_bot_in_invited_group) { create(:user, :project_bot) }
+
+    before do
+      invited_group.add_maintainer(group_bot_in_invited_group)
+    end
+  end
+
+  shared_context 'bot user for shared_group' do
+    let(:group_bot_in_shared_group) { create(:user, :project_bot) }
+
+    before do
+      add_bot_to_group.add_maintainer(group_bot_in_shared_group)
+    end
+  end
+
   describe '#billed_user_ids', :saas do
     context 'with a user namespace' do
       let(:user) { create(:user) }
@@ -1087,6 +1103,13 @@ RSpec.describe Namespace do
                   expect(billed_user_ids[:group_member_user_ids]).to match_array([developer.id])
                   expect(billed_user_ids[:project_member_user_ids]).to match_array([developer.id, project_developer.id])
                 end
+
+                context 'the invited group contains bot users' do
+                  include_context 'bot user for invited_group'
+
+                  it { expect(billed_user_ids[:user_ids]).not_to include(group_bot_in_invited_group.id) }
+                  it { expect(billed_user_ids[:shared_project_user_ids]).not_to include(group_bot_in_invited_group.id) }
+                end
               end
 
               context 'when group is invited as a guest to the project' do
@@ -1122,6 +1145,15 @@ RSpec.describe Namespace do
               expect(shared_group.billed_user_ids[:user_ids]).not_to include([developer.id])
             end
 
+            context 'the shared group contains bot users' do
+              include_context 'bot user for shared_group' do
+                let(:add_bot_to_group) { shared_group }
+              end
+
+              it { expect(billed_user_ids[:user_ids]).not_to include(group_bot_in_shared_group.id) }
+              it { expect(billed_user_ids[:shared_group_user_ids]).not_to include(group_bot_in_shared_group.id) }
+            end
+
             context 'when subgroup invited another group to collaborate' do
               let(:another_shared_group) { create(:group) }
               let(:another_shared_group_developer) { create(:user) }
@@ -1146,6 +1178,15 @@ RSpec.describe Namespace do
                   expect(shared_group.billed_user_ids[:user_ids]).not_to include([developer.id])
                   expect(another_shared_group.billed_user_ids[:user_ids]).not_to include([developer.id, shared_group_developer.id])
                 end
+
+                context 'the shared group contains bot users' do
+                  include_context 'bot user for shared_group' do
+                    let(:add_bot_to_group) { another_shared_group }
+                  end
+
+                  it { expect(billed_user_ids[:user_ids]).not_to include(group_bot_in_shared_group.id) }
+                  it { expect(billed_user_ids[:shared_group_user_ids]).not_to include(group_bot_in_shared_group.id) }
+                end
               end
 
               context 'when subgroup invites another group as guest' do
@@ -1253,6 +1294,13 @@ RSpec.describe Namespace do
                     invited_group_guest.id
                   ])
                 end
+
+                context 'the invited group contains bot users' do
+                  include_context 'bot user for invited_group'
+
+                  it { expect(billed_user_ids[:user_ids]).not_to include(group_bot_in_invited_group.id) }
+                  it { expect(billed_user_ids[:shared_project_user_ids]).not_to include(group_bot_in_invited_group.id) }
+                end
               end
             end
 
@@ -1277,6 +1325,15 @@ RSpec.describe Namespace do
                 expect(billed_user_ids[:shared_group_user_ids]).to match_array([shared_group_developer.id, shared_group_guest.id])
                 expect(shared_group.billed_user_ids[:user_ids]).to match_array([shared_group_developer.id, shared_group_guest.id])
               end
+
+              context 'the shared group contains bot users' do
+                include_context 'bot user for shared_group' do
+                  let(:add_bot_to_group) { shared_group }
+                end
+
+                it { expect(billed_user_ids[:user_ids]).not_to include(group_bot_in_shared_group.id) }
+                it { expect(billed_user_ids[:shared_group_user_ids]).not_to include(group_bot_in_shared_group.id) }
+              end
             end
           end
         end