From 1d5705a84ced01bfc79b48a38ac16e769d9cc7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Wed, 20 Mar 2019 14:39:07 +0100 Subject: Disallow changing namespace of a project in update method --- app/controllers/projects_controller.rb | 13 ++++++++----- .../security-mass-assignment-on-project-update.yml | 5 +++++ spec/controllers/projects_controller_spec.rb | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/security-mass-assignment-on-project-update.yml diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 33c6608d321..b419d669544 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -47,7 +47,7 @@ class ProjectsController < Projects::ApplicationController end def create - @project = ::Projects::CreateService.new(current_user, project_params).execute + @project = ::Projects::CreateService.new(current_user, project_params(attributes: project_params_create_attributes)).execute if @project.saved? cookies[:issue_board_welcome_hidden] = { path: project_path(@project), value: nil, expires: Time.at(0) } @@ -328,9 +328,9 @@ class ProjectsController < Projects::ApplicationController end # rubocop: enable CodeReuse/ActiveRecord - def project_params + def project_params(attributes: project_params_attributes) params.require(:project) - .permit(project_params_attributes) + .permit(attributes) end def project_params_attributes @@ -349,11 +349,10 @@ class ProjectsController < Projects::ApplicationController :last_activity_at, :lfs_enabled, :name, - :namespace_id, :only_allow_merge_if_all_discussions_are_resolved, :only_allow_merge_if_pipeline_succeeds, - :printing_merge_request_link_enabled, :path, + :printing_merge_request_link_enabled, :public_builds, :request_access_enabled, :runners_token, @@ -375,6 +374,10 @@ class ProjectsController < Projects::ApplicationController ] end + def project_params_create_attributes + project_params_attributes << :namespace_id + end + def custom_import_params {} end diff --git a/changelogs/unreleased/security-mass-assignment-on-project-update.yml b/changelogs/unreleased/security-mass-assignment-on-project-update.yml new file mode 100644 index 00000000000..8657dcdd135 --- /dev/null +++ b/changelogs/unreleased/security-mass-assignment-on-project-update.yml @@ -0,0 +1,5 @@ +--- +title: Disallow updating namespace during updating project +merge_request: +author: +type: security diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index a1662658ade..41380e79305 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -369,6 +369,23 @@ describe ProjectsController do end end + it 'does not update namespace' do + controller.instance_variable_set(:@project, project) + + params = { + namespace_id: 'test' + } + + expect do + put :update, + params: { + namespace_id: project.namespace, + id: project.id, + project: params + } + end.not_to change {project.namespace} + end + def update_project(**parameters) put :update, params: { -- cgit v1.2.3 From 3a3748929b37a6b86e39026368c6883e8f5ef451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Mon, 25 Mar 2019 11:08:20 +0100 Subject: Add cr remarks --- app/controllers/projects_controller.rb | 6 +++--- .../unreleased/security-mass-assignment-on-project-update.yml | 2 +- spec/controllers/projects_controller_spec.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b419d669544..f76e6663995 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -328,9 +328,9 @@ class ProjectsController < Projects::ApplicationController end # rubocop: enable CodeReuse/ActiveRecord - def project_params(attributes: project_params_attributes) + def project_params(attributes: []) params.require(:project) - .permit(attributes) + .permit(project_params_attributes + attributes) end def project_params_attributes @@ -375,7 +375,7 @@ class ProjectsController < Projects::ApplicationController end def project_params_create_attributes - project_params_attributes << :namespace_id + [:namespace_id] end def custom_import_params diff --git a/changelogs/unreleased/security-mass-assignment-on-project-update.yml b/changelogs/unreleased/security-mass-assignment-on-project-update.yml index 8657dcdd135..93561cd91b3 100644 --- a/changelogs/unreleased/security-mass-assignment-on-project-update.yml +++ b/changelogs/unreleased/security-mass-assignment-on-project-update.yml @@ -1,5 +1,5 @@ --- -title: Disallow updating namespace during updating project +title: Disallow updating namespace when updating a project merge_request: author: type: security diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 41380e79305..717675d1d30 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -383,7 +383,7 @@ describe ProjectsController do id: project.id, project: params } - end.not_to change {project.namespace} + end.not_to change {project.reload.namespace} end def update_project(**parameters) -- cgit v1.2.3 From 148584715838cfe918f830ce2c07d7d428e614db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C5=82gorzata=20Ksionek?= Date: Tue, 26 Mar 2019 09:30:16 +0100 Subject: Refactor specs according to the code review --- spec/controllers/projects_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 717675d1d30..356d606d5c5 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -383,7 +383,7 @@ describe ProjectsController do id: project.id, project: params } - end.not_to change {project.reload.namespace} + end.not_to change { project.namespace.reload } end def update_project(**parameters) -- cgit v1.2.3