diff options
| -rw-r--r-- | app/services/base_project_service.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/json_cache.rb | 4 | ||||
| -rw-r--r-- | spec/controllers/boards/issues_controller_spec.rb | 20 | ||||
| -rw-r--r-- | spec/services/issues/create_service_spec.rb | 13 | ||||
| -rw-r--r-- | spec/services/issues/update_service_spec.rb | 17 |
5 files changed, 52 insertions, 6 deletions
diff --git a/app/services/base_project_service.rb b/app/services/base_project_service.rb index 1bf4a235a79..8cb6b632a9e 100644 --- a/app/services/base_project_service.rb +++ b/app/services/base_project_service.rb @@ -7,7 +7,9 @@ class BaseProjectService < ::BaseContainerService attr_accessor :project def initialize(project:, current_user: nil, params: {}) - super(container: project, current_user: current_user, params: params) + # we need to exclude project params since they may come from external requests. project should always + # be passed as part of the service's initializer + super(container: project, current_user: current_user, params: params.except(:project, :project_id)) @project = project end diff --git a/lib/gitlab/json_cache.rb b/lib/gitlab/json_cache.rb index d5c018cfc68..d2916a01809 100644 --- a/lib/gitlab/json_cache.rb +++ b/lib/gitlab/json_cache.rb @@ -43,9 +43,7 @@ module Gitlab end def write(key, value, options = nil) - # As we use json as the serialization format, return everything from - # ActiveModel objects included encrypted values. - backend.write(cache_key(key), value.to_json(unsafe_serialization_hash: true), options) + backend.write(cache_key(key), value.to_json, options) end def fetch(key, options = {}, &block) diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index 1fd249eba69..3e1cdfccc61 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -427,6 +427,22 @@ RSpec.describe Boards::IssuesController do end describe 'POST create' do + context 'when trying to create issue on an unauthorized project' do + let(:unauthorized_project) { create(:project, :private) } + let(:issue_params) { { project_id: unauthorized_project.id } } + + it 'creates the issue on the board\'s project' do + expect do + create_issue user: user, board: board, list: list1, title: 'New issue', additional_issue_params: issue_params + end.to change(Issue, :count).by(1) + + created_issue = Issue.last + + expect(created_issue.project).to eq(project) + expect(unauthorized_project.reload.issues.count).to eq(0) + end + end + context 'with valid params' do before do create_issue user: user, board: board, list: list1, title: 'New issue' @@ -500,13 +516,13 @@ RSpec.describe Boards::IssuesController do end end - def create_issue(user:, board:, list:, title:) + def create_issue(user:, board:, list:, title:, additional_issue_params: {}) sign_in(user) post :create, params: { board_id: board.to_param, list_id: list.to_param, - issue: { title: title, project_id: project.id } + issue: { title: title, project_id: project.id }.merge(additional_issue_params) }, format: :json end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 4a84862b9d5..3d52dc07c4f 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -47,6 +47,19 @@ RSpec.describe Issues::CreateService do due_date: Date.tomorrow } end + context 'when an unauthorized project_id is provided' do + let(:unauthorized_project) { create(:project) } + + before do + opts[:project_id] = unauthorized_project.id + end + + it 'ignores the project_id param and creates issue in the given project' do + expect(issue.project).to eq(project) + expect(unauthorized_project.reload.issues.count).to eq(0) + end + end + it 'works if base work item types were not created yet' do WorkItems::Type.delete_all diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 8a2e9ed74f7..634a4206d48 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -69,6 +69,23 @@ RSpec.describe Issues::UpdateService, :mailer do } end + context 'when an unauthorized project_id is provided' do + let(:unauthorized_project) { create(:project) } + + before do + opts[:project_id] = unauthorized_project.id + end + + it 'ignores the project_id param and does not update the issue\'s project' do + expect do + update_issue(opts) + unauthorized_project.reload + end.to not_change { unauthorized_project.issues.count } + + expect(issue.project).to eq(project) + end + end + it 'updates the issue with the given params' do expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) |