Commit 0051f58f authored by Douwe Maan's avatar Douwe Maan

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
parents 43ddf0ca 6a18b3a5
...@@ -45,6 +45,9 @@ v 8.12.0 (unreleased) ...@@ -45,6 +45,9 @@ v 8.12.0 (unreleased)
v 8.11.4 (unreleased) v 8.11.4 (unreleased)
- Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner)
v 8.11.4 (unreleased)
- Fix issue boards leak private label names and descriptions
v 8.11.3 (unreleased) v 8.11.3 (unreleased)
- Allow system info page to handle case where info is unavailable - Allow system info page to handle case where info is unavailable
- Label list shows all issues (opened or closed) with that label - Label list shows all issues (opened or closed) with that label
......
...@@ -3,7 +3,10 @@ module Boards ...@@ -3,7 +3,10 @@ module Boards
class CreateService < Boards::BaseService class CreateService < Boards::BaseService
def execute def execute
List.transaction do List.transaction do
create_list_at(next_position) label = project.labels.find(params[:label_id])
position = next_position
create_list(label, position)
end end
end end
...@@ -14,8 +17,8 @@ module Boards ...@@ -14,8 +17,8 @@ module Boards
max_position.nil? ? 0 : max_position.succ max_position.nil? ? 0 : max_position.succ
end end
def create_list_at(position) def create_list(label, position)
board.lists.create(params.merge(list_type: :label, position: position)) board.lists.create(label: label, list_type: :label, position: position)
end end
end end
end end
......
...@@ -39,7 +39,7 @@ describe Projects::Boards::ListsController do ...@@ -39,7 +39,7 @@ describe Projects::Boards::ListsController do
allow(Ability.abilities).to receive(:allowed?).with(user, :read_list, project).and_return(false) allow(Ability.abilities).to receive(:allowed?).with(user, :read_list, project).and_return(false)
end end
it 'returns a successful 403 response' do it 'returns a forbidden 403 response' do
read_board_list user: user read_board_list user: user
expect(response).to have_http_status(403) expect(response).to have_http_status(403)
...@@ -56,9 +56,9 @@ describe Projects::Boards::ListsController do ...@@ -56,9 +56,9 @@ describe Projects::Boards::ListsController do
end end
describe 'POST create' do describe 'POST create' do
context 'with valid params' do
let(:label) { create(:label, project: project, name: 'Development') } let(:label) { create(:label, project: project, name: 'Development') }
context 'with valid params' do
it 'returns a successful 200 response' do it 'returns a successful 200 response' do
create_board_list user: user, label_id: label.id create_board_list user: user, label_id: label.id
...@@ -73,20 +73,29 @@ describe Projects::Boards::ListsController do ...@@ -73,20 +73,29 @@ describe Projects::Boards::ListsController do
end end
context 'with invalid params' do context 'with invalid params' do
it 'returns an error' do context 'when label is nil' do
it 'returns a not found 404 response' do
create_board_list user: user, label_id: nil create_board_list user: user, label_id: nil
parsed_response = JSON.parse(response.body) expect(response).to have_http_status(404)
end
end
expect(parsed_response['label']).to contain_exactly "can't be blank" context 'when label that does not belongs to project' do
expect(response).to have_http_status(422) it 'returns a not found 404 response' do
label = create(:label, name: 'Development')
create_board_list user: user, label_id: label.id
expect(response).to have_http_status(404)
end
end end
end end
context 'with unauthorized user' do 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 create_board_list user: guest, label_id: label.id
expect(response).to have_http_status(403) expect(response).to have_http_status(403)
...@@ -122,7 +131,7 @@ describe Projects::Boards::ListsController do ...@@ -122,7 +131,7 @@ describe Projects::Boards::ListsController do
end end
context 'with invalid position' do 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 move user: user, list: planning, position: 6
expect(response).to have_http_status(422) expect(response).to have_http_status(422)
...@@ -138,7 +147,7 @@ describe Projects::Boards::ListsController do ...@@ -138,7 +147,7 @@ describe Projects::Boards::ListsController do
end end
context 'with unauthorized user' do 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 move user: guest, list: planning, position: 6
expect(response).to have_http_status(403) expect(response).to have_http_status(403)
...@@ -180,7 +189,7 @@ describe Projects::Boards::ListsController do ...@@ -180,7 +189,7 @@ describe Projects::Boards::ListsController do
end end
context 'with unauthorized user' do 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 remove_board_list user: guest, list: planning
expect(response).to have_http_status(403) expect(response).to have_http_status(403)
...@@ -213,7 +222,7 @@ describe Projects::Boards::ListsController do ...@@ -213,7 +222,7 @@ describe Projects::Boards::ListsController do
end end
context 'when board lists is not empty' do 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) create(:list, board: board)
generate_default_board_lists user: user generate_default_board_lists user: user
...@@ -223,7 +232,7 @@ describe Projects::Boards::ListsController do ...@@ -223,7 +232,7 @@ describe Projects::Boards::ListsController do
end end
context 'with unauthorized user' do 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 generate_default_board_lists user: guest
expect(response).to have_http_status(403) expect(response).to have_http_status(403)
......
...@@ -5,7 +5,7 @@ describe Boards::Lists::CreateService, services: true do ...@@ -5,7 +5,7 @@ describe Boards::Lists::CreateService, services: true do
let(:project) { create(:project_with_board) } let(:project) { create(:project_with_board) }
let(:board) { project.board } let(:board) { project.board }
let(:user) { create(:user) } 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) } subject(:service) { described_class.new(project, user, label_id: label.id) }
...@@ -50,5 +50,14 @@ describe Boards::Lists::CreateService, services: true do ...@@ -50,5 +50,14 @@ describe Boards::Lists::CreateService, services: true do
expect(list2.reload.position).to eq 1 expect(list2.reload.position).to eq 1
end end
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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment