Check if labels are available for target issuable

- labels have to be in the same project/group
as an issuable

app/models/label.rb

@@ -133,6 +133,10 @@ class Label < ActiveRecord::Base
     1
   end
 
+  def self.by_ids(ids)
+    where(id: ids)
+  end
+
   def open_issues_count(user = nil)
     issues_count(user, state: 'opened')
   end

app/services/issuable_base_service.rb

@@ -70,10 +70,14 @@ class IssuableBaseService < BaseService
   end
 
   def filter_labels
-    filter_labels_in_param(:add_label_ids)
-    filter_labels_in_param(:remove_label_ids)
-    filter_labels_in_param(:label_ids)
-    find_or_create_label_ids
+    params[:add_label_ids] = labels_service.filter_labels_ids_in_param(:add_label_ids) if params[:add_label_ids]
+    params[:remove_label_ids] = labels_service.filter_labels_ids_in_param(:remove_label_ids) if params[:remove_label_ids]
+
+    if params[:label_ids]
+      params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids)
+    elsif params[:labels]
+      params[:label_ids] = labels_service.find_or_create_by_titles.map(&:id)
+    end
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
@@ -101,6 +105,10 @@ class IssuableBaseService < BaseService
     end.compact
   end
 
+  def labels_service
+    @labels_service ||= ::Labels::AvailableLabelsService.new(current_user, parent, params)
+  end
+
   def process_label_ids(attributes, existing_label_ids: nil)
     label_ids = attributes.delete(:label_ids)
     add_label_ids = attributes.delete(:add_label_ids)
@@ -118,10 +126,6 @@ class IssuableBaseService < BaseService
     new_label_ids
   end
 
-  def available_labels
-    @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: true).execute
-  end
-
   def handle_quick_actions_on_create(issuable)
     merge_quick_actions_into_params!(issuable)
   end

app/services/labels/available_labels_service.rb

+# frozen_string_literal: true
+module Labels
+  class AvailableLabelsService
+    attr_reader :current_user, :parent, :params
+
+    def initialize(current_user, parent, params)
+      @current_user = current_user
+      @parent = parent
+      @params = params
+    end
+
+    def find_or_create_by_titles
+      labels = params.delete(:labels)
+
+      return [] unless labels
+
+      labels = labels.split(',') if labels.is_a?(String)
+
+      labels.map do |label_name|
+        label = Labels::FindOrCreateService.new(
+          current_user,
+          parent,
+          include_ancestor_groups: true,
+          title: label_name.strip,
+          available_labels: available_labels
+        ).execute
+
+        label
+      end.compact
+    end
+
+    def filter_labels_ids_in_param(key)
+      return [] if params[key].to_a.empty?
+
+      # rubocop:disable CodeReuse/ActiveRecord
+      available_labels.by_ids(params[key]).pluck(:id)
+      # rubocop:enable CodeReuse/ActiveRecord
+    end
+
+    private
+
+    def available_labels
+      @available_labels ||= LabelsFinder.new(current_user, finder_params).execute
+    end
+
+    def finder_params
+      params = { include_ancestor_groups: true }
+
+      case parent
+      when Group
+        params[:group_id] = parent.id
+        params[:only_group_labels] = true
+      when Project
+        params[:project_id] = parent.id
+      end
+
+      params
+    end
+  end
+end

ee/app/services/ee/boards/base_service.rb

@@ -35,20 +35,15 @@ module EE
       # rubocop: enable CodeReuse/ActiveRecord
 
       def set_labels
-        labels = params.delete(:labels)
-
-        return unless labels
-
-        params[:label_ids] = labels.split(",").map do |label_name|
-          label = Labels::FindOrCreateService.new(
-            current_user,
-            parent,
-            title: label_name.strip,
-            include_ancestor_groups: true
-          ).execute
+        if params[:label_ids]
+          params[:label_ids] = labels_service.filter_labels_ids_in_param(:label_ids)
+        elsif params[:labels]
+          params[:label_ids] = labels_service.find_or_create_by_titles.map(&:id)
+        end
+      end
 
-          label.try(:id)
-        end.compact
+      def labels_service
+        @labels_service ||= ::Labels::AvailableLabelsService.new(current_user, parent, params)
       end
     end
   end

ee/changelogs/unreleased/security-milestone-labels.yml

+---
+title: Check label_ids parent when updating issue board
+merge_request:
+author:
+type: security

ee/spec/controllers/projects/boards_controller_spec.rb

@@ -141,7 +141,7 @@ describe Projects::BoardsController do
     let(:board) { create(:board, project: project, name: 'Backend') }
     let(:user) { create(:user) }
     let(:milestone) { create(:milestone, project: project) }
-    let(:label) { create(:label) }
+    let(:label) { create(:label, project: project) }
 
     let(:update_params) do
       { name: 'Frontend',

ee/spec/services/boards/update_service_spec.rb

@@ -17,7 +17,7 @@ describe Boards::UpdateService, services: true do
   end
 
   describe '#execute' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, group: group) }
     let(:group) { create(:group) }
     let!(:board)  { create(:board, group: group, name: 'Backend') }
 
@@ -44,10 +44,11 @@ describe Boards::UpdateService, services: true do
     it 'updates the configuration params when scoped issue board is enabled' do
       stub_licensed_features(scoped_issue_board: true)
       assignee = create(:user)
-      milestone = create(:milestone, project: project)
-      label = create(:label, project: project)
+      milestone = create(:milestone, group: group)
+      label = create(:group_label, group: board.group)
+      user = create(:user)
 
-      service = described_class.new(project, double,
+      service = described_class.new(group, user,
                                     milestone_id: milestone.id,
                                     assignee_id: assignee.id,
                                     label_ids: [label.id])
@@ -213,6 +214,18 @@ describe Boards::UpdateService, services: true do
             expect_label_assigned(user, board, %w{group_label project_label new_label}, %w{group_label project_label})
           end
         end
+
+        context 'when label_ids param is provided' do
+          it 'updates using only labels accessible by the project board' do
+            other_project_label = create(:label, title: 'other_project_label')
+            other_group_label = create(:group_label, title: 'other_group_label')
+            label_ids = [group_label.id, label.id, other_project_label.id, other_group_label.id]
+
+            described_class.new(board.parent, user, label_ids: label_ids).execute(board)
+
+            expect(board.reload.labels).to contain_exactly(group_label, label)
+          end
+        end
       end
     end
   end

spec/services/labels/available_labels_service_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Labels::AvailableLabelsService do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public, group: group) }
+  let(:group) { create(:group) }
+
+  let(:project_label) { create(:label, project: project) }
+  let(:other_project_label) { create(:label) }
+  let(:group_label) { create(:group_label, group: group) }
+  let(:other_group_label) { create(:group_label) }
+  let(:labels) { [project_label, other_project_label, group_label, other_group_label] }
+
+  context '#find_or_create_by_titles' do
+    let(:label_titles) { labels.map(&:title).push('non existing title') }
+
+    context 'when parent is a project' do
+      context 'when a user is not a project member' do
+        it 'returns only relevant label ids' do
+          result = described_class.new(user, project, labels: label_titles).find_or_create_by_titles
+
+          expect(result).to match_array([project_label, group_label])
+        end
+      end
+
+      context 'when a user is a project member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'creates new labels for not found titles' do
+          result = described_class.new(user, project, labels: label_titles).find_or_create_by_titles
+
+          expect(result.count).to eq(5)
+          expect(result).to include(project_label, group_label)
+          expect(result).not_to include(other_project_label, other_group_label)
+        end
+      end
+    end
+
+    context 'when parent is a group' do
+      context 'when a user is not a group member' do
+        it 'returns only relevant label ids' do
+          result = described_class.new(user, group, labels: label_titles).find_or_create_by_titles
+
+          expect(result).to match_array([group_label])
+        end
+      end
+
+      context 'when a user is a group member' do
+        before do
+          group.add_developer(user)
+        end
+
+        it 'creates new labels for not found titles' do
+          result = described_class.new(user, group, labels: label_titles).find_or_create_by_titles
+
+          expect(result.count).to eq(5)
+          expect(result).to include(group_label)
+          expect(result).not_to include(project_label, other_project_label, other_group_label)
+        end
+      end
+    end
+  end
+
+  context '#filter_labels_ids_in_param' do
+    let(:label_ids) { labels.map(&:id).push(99999) }
+
+    context 'when parent is a project' do
+      it 'returns only relevant label ids' do
+        result = described_class.new(user, project, ids: label_ids).filter_labels_ids_in_param(:ids)
+
+        expect(result).to match_array([project_label.id, group_label.id])
+      end
+    end
+
+    context 'when parent is a group' do
+      it 'returns only relevant label ids' do
+        result = described_class.new(user, group, ids: label_ids).filter_labels_ids_in_param(:ids)
+
+        expect(result).to match_array([group_label.id])
+      end
+    end
+  end
+end