diff options
author | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2016-08-22 11:50:41 -0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2016-08-29 16:04:28 -0300 |
commit | 4cccfc0f171944cd6f2ffaf49e48cf005dcf985e (patch) | |
tree | 1adc2b594dea43b8be009c7ff2c5d252872e0169 | |
parent | 68b3c8c2973a8fe1ee3f51091394acef2a25789d (diff) | |
download | gitlab-ce-4cccfc0f171944cd6f2ffaf49e48cf005dcf985e.tar.gz |
Fix issue boards leak private label names and descriptions
-rw-r--r-- | app/services/boards/lists/create_service.rb | 9 | ||||
-rw-r--r-- | spec/controllers/projects/boards/lists_controller_spec.rb | 39 | ||||
-rw-r--r-- | spec/services/boards/lists/create_service_spec.rb | 11 |
3 files changed, 40 insertions, 19 deletions
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 |