Merge branch 'fix-issue-boards-leak-private-labels-names-descriptions' into 'master'

Fix issue boards leak private labels names descriptions

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/21167

/cc @stanhu

See merge request !1989

4 files changed, 47 insertions, 19 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2d1999315c1..ba580b3a05e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -16,6 +16,13 @@ v 8.11.4
   - Fix issue boards leak private label names and descriptions
 
 v 8.11.3
+
+v 8.11.4 (unreleased)
+  - Fix issue boards leak private label names and descriptions
+
+v 8.11.3 (unreleased)
+  - Allow system info page to handle case where info is unavailable
+  - Label list shows all issues (opened or closed) with that label
   - Don't show resolve conflicts link before MR status is updated
   - Fix IE11 fork button bug !5982
   - Don't prevent viewing the MR when git refs for conflicts can't be found on disk
diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb
index 5cb408b9d20..b1887820bd4 100644
--- a/app/services/boards/lists/create_service.rb
+++ b/app/services/boards/lists/create_service.rb
@@ -3,7 +3,10 @@ module Boards
     class CreateService < Boards::BaseService
       def execute
         List.transaction do
-          create_list_at(next_position)
+          label    = project.labels.find(params[:label_id])
+          position = next_position
+
+          create_list(label, position)
         end
       end
 
@@ -14,8 +17,8 @@ module Boards
         max_position.nil? ? 0 : max_position.succ
       end
 
-      def create_list_at(position)
-        board.lists.create(params.merge(list_type: :label, position: position))
+      def create_list(label, position)
+        board.lists.create(label: label, list_type: :label, position: position)
       end
     end
   end
diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb
index 9496636e3cc..261f35f28ed 100644
--- a/spec/controllers/projects/boards/lists_controller_spec.rb
+++ b/spec/controllers/projects/boards/lists_controller_spec.rb
@@ -39,7 +39,7 @@ describe Projects::Boards::ListsController do
         allow(Ability.abilities).to receive(:allowed?).with(user, :read_list, project).and_return(false)
       end
 
-      it 'returns a successful 403 response' do
+      it 'returns a forbidden 403 response' do
         read_board_list user: user
 
         expect(response).to have_http_status(403)
@@ -56,9 +56,9 @@ describe Projects::Boards::ListsController do
   end
 
   describe 'POST create' do
-    let(:label) { create(:label, project: project, name: 'Development') }
-
     context 'with valid params' do
+      let(:label) { create(:label, project: project, name: 'Development') }
+
       it 'returns a successful 200 response' do
         create_board_list user: user, label_id: label.id
 
@@ -73,20 +73,29 @@ describe Projects::Boards::ListsController do
     end
 
     context 'with invalid params' do
-      it 'returns an error' do
-        create_board_list user: user, label_id: nil
+      context 'when label is nil' do
+        it 'returns a not found 404 response' do
+          create_board_list user: user, label_id: nil
+
+          expect(response).to have_http_status(404)
+        end
+      end
 
-        parsed_response = JSON.parse(response.body)
+      context 'when label that does not belongs to project' do
+        it 'returns a not found 404 response' do
+          label = create(:label, name: 'Development')
 
-        expect(parsed_response['label']).to contain_exactly "can't be blank"
-        expect(response).to have_http_status(422)
+          create_board_list user: user, label_id: label.id
+
+          expect(response).to have_http_status(404)
+        end
       end
     end
 
     context 'with unauthorized user' do
-      let(:label) { create(:label, project: project, name: 'Development') }
+      it 'returns a forbidden 403 response' do
+        label = create(:label, project: project, name: 'Development')
 
-      it 'returns a successful 403 response' do
         create_board_list user: guest, label_id: label.id
 
         expect(response).to have_http_status(403)
@@ -122,7 +131,7 @@ describe Projects::Boards::ListsController do
     end
 
     context 'with invalid position' do
-      it 'returns a unprocessable entity 422 response' do
+      it 'returns an unprocessable entity 422 response' do
         move user: user, list: planning, position: 6
 
         expect(response).to have_http_status(422)
@@ -138,7 +147,7 @@ describe Projects::Boards::ListsController do
     end
 
     context 'with unauthorized user' do
-      it 'returns a successful 403 response' do
+      it 'returns a forbidden 403 response' do
         move user: guest, list: planning, position: 6
 
         expect(response).to have_http_status(403)
@@ -180,7 +189,7 @@ describe Projects::Boards::ListsController do
     end
 
     context 'with unauthorized user' do
-      it 'returns a successful 403 response' do
+      it 'returns a forbidden 403 response' do
         remove_board_list user: guest, list: planning
 
         expect(response).to have_http_status(403)
@@ -213,7 +222,7 @@ describe Projects::Boards::ListsController do
     end
 
     context 'when board lists is not empty' do
-      it 'returns a unprocessable entity 422 response' do
+      it 'returns an unprocessable entity 422 response' do
         create(:list, board: board)
 
         generate_default_board_lists user: user
@@ -223,7 +232,7 @@ describe Projects::Boards::ListsController do
     end
 
     context 'with unauthorized user' do
-      it 'returns a successful 403 response' do
+      it 'returns a forbidden 403 response' do
         generate_default_board_lists user: guest
 
         expect(response).to have_http_status(403)
diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb
index 5e7e145065e..90764b86b16 100644
--- a/spec/services/boards/lists/create_service_spec.rb
+++ b/spec/services/boards/lists/create_service_spec.rb
@@ -5,7 +5,7 @@ describe Boards::Lists::CreateService, services: true do
     let(:project) { create(:project_with_board) }
     let(:board)   { project.board }
     let(:user)    { create(:user) }
-    let(:label)   { create(:label, name: 'in-progress') }
+    let(:label)   { create(:label, project: project, name: 'in-progress') }
 
     subject(:service) { described_class.new(project, user, label_id: label.id) }
 
@@ -50,5 +50,14 @@ describe Boards::Lists::CreateService, services: true do
         expect(list2.reload.position).to eq 1
       end
     end
+
+    context 'when provided label does not belongs to the project' do
+      it 'raises an error' do
+        label = create(:label, name: 'in-development')
+        service = described_class.new(project, user, label_id: label.id)
+
+        expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end