Limit oncall projects shown to scope of source

Changelog: security
EE: true

app/serializers/member_entity.rb

@@ -40,7 +40,9 @@ class MemberEntity < Grape::Entity
 
   expose :valid_level_roles, as: :valid_roles
 
-  expose :user, if: -> (member) { member.user.present? }, using: MemberUserEntity
+  expose :user, if: -> (member) { member.user.present? } do |member, options|
+    MemberUserEntity.represent(member.user, source: options[:source])
+  end
 
   expose :invite, if: -> (member) { member.invite? } do
     expose :email do |member|

ee/app/serializers/ee/member_user_entity.rb

@@ -9,8 +9,14 @@ module EE
       unexpose :email
       expose :oncall_schedules, with: ::IncidentManagement::OncallScheduleEntity
 
+      # options[:source] is required to scope the schedules
+      # It should be either a Group or Project
       def oncall_schedules
-        object.oncall_schedules.uniq
+        return [] unless options[:source].present?
+
+        project_ids = options[:source].is_a?(Group) ? options[:source].project_ids : [options[:source].id]
+
+        object.oncall_schedules.select { |schedule| project_ids.include?(schedule.project_id) }
       end
     end
   end

ee/spec/helpers/projects/project_members_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Projects::ProjectMembersHelper do
+  include OncallHelpers
+
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+  let(:current_user) { user }
+
+  before do
+    allow(helper).to receive(:current_user).and_return(current_user)
+    allow(helper).to receive(:can?).and_return(true)
+  end
+
+  describe '#project_members_app_data_json' do
+    before do
+      project.add_developer(user)
+      create_schedule_with_user(project, user)
+    end
+
+    it 'does not execute N+1' do
+      control_count = ActiveRecord::QueryRecorder.new do
+        call_project_members_app_data_json
+      end.count
+
+      expect(project.members.count).to eq(2)
+
+      user_2 = create(:user)
+      project.add_developer(user_2)
+      create_schedule_with_user(project, user_2)
+
+      expect(project.members.count).to eq(3)
+
+      expect { call_project_members_app_data_json }.not_to exceed_query_limit(control_count).with_threshold(6) # existing n+1
+    end
+  end
+
+  def call_project_members_app_data_json
+    helper.project_members_app_data_json(project, members: preloaded_members, group_links: [], invited: [], access_requests: [])
+  end
+
+  # Simulates the behaviour in ProjectMembersController
+  def preloaded_members
+    klass = Class.new do
+      include MembersPresentation
+
+      def initialize(user)
+        @current_user = user
+      end
+
+      attr_reader :current_user
+    end
+
+    klass.new(current_user).present_members(project.members.reload)
+  end
+end

ee/spec/serializers/member_user_entity_spec.rb

@@ -3,35 +3,54 @@
 require 'spec_helper'
 
 RSpec.describe MemberUserEntity do
+  include OncallHelpers
+
   let_it_be_with_reload(:user) { create(:user) }
 
-  let(:entity) { described_class.new(user) }
+  let(:entity) { described_class.new(user, source: source) }
   let(:entity_hash) { entity.as_json }
+  let(:source) { nil }
 
   it 'matches json schema' do
     expect(entity.to_json).to match_schema('entities/member_user')
   end
 
   context 'with oncall schedules' do
-    let_it_be(:oncall_schedule) { create(:incident_management_oncall_participant, user: user).rotation.schedule }
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project_1) { create(:project, group: group )}
+    let_it_be(:project_2) { create(:project, group: group )}
+
+    let_it_be(:oncall_schedule_1) { create_schedule_with_user(project_1, user) }
+    let_it_be(:oncall_schedule_2) { create_schedule_with_user(project_2, user) }
+
+    it 'returns an empty array if no source option is given' do
+      expect(entity_hash[:oncall_schedules]).to eq []
+    end
+
+    context 'source is project' do
+      let(:source) { project_1 }
 
-    it 'correctly exposes `oncall_schedules`' do
-      expect(entity_hash[:oncall_schedules]).to include(schedule_hash(oncall_schedule))
+      it 'correctly exposes `oncall_schedules`' do
+        expect(entity_hash[:oncall_schedules]).to contain_exactly(schedule_hash(oncall_schedule_1))
+      end
     end
 
-    it 'exposed and de-dupes the schedules' do
-      allow(user).to receive(:oncall_schedules).and_return([oncall_schedule, oncall_schedule])
+    context 'source is group' do
+      let(:source) { group }
 
-      expect(entity_hash[:oncall_schedules].size).to eq(1)
-      expect(entity_hash[:oncall_schedules]).to include(schedule_hash(oncall_schedule))
+      it 'correctly exposes `oncall_schedules`' do
+        expect(entity_hash[:oncall_schedules]).to contain_exactly(schedule_hash(oncall_schedule_1), schedule_hash(oncall_schedule_2))
+      end
     end
 
+    private
+
     def schedule_hash(schedule)
       schedule_url = Gitlab::Routing.url_helpers.project_incident_management_oncall_schedules_url(schedule.project)
       project_url = Gitlab::Routing.url_helpers.project_url(schedule.project)
       {
-        name: oncall_schedule.name,
-        project_name: oncall_schedule.project.name,
+        name: schedule.name,
+        project_name: schedule.project.name,
         schedule_url: schedule_url,
         project_url: project_url
       }

ee/spec/support/helpers/oncall_helpers.rb

@@ -6,4 +6,12 @@ module OncallHelpers
 
     rotation.active_period.for_date(date)
   end
+
+  def create_schedule_with_user(project, user)
+    create(:incident_management_oncall_schedule, project: project) do |schedule|
+      create(:incident_management_oncall_rotation, schedule: schedule) do |rotation|
+        create(:incident_management_oncall_participant, rotation: rotation, user: user)
+      end
+    end
+  end
 end