Merge branch 'raise-on-unfiltered-params' into 'master'

Set ActionController raise_on_unfiltered_parameters to true

See merge request gitlab-org/gitlab-ce!24443

7 files changed, 28 insertions, 7 deletions

diff --git a/app/controllers/projects/lfs_locks_api_controller.rb b/app/controllers/projects/lfs_locks_api_controller.rb
index fc67cd72faa..6aacb9d9a56 100644
--- a/app/controllers/projects/lfs_locks_api_controller.rb
+++ b/app/controllers/projects/lfs_locks_api_controller.rb
@@ -4,19 +4,19 @@ class Projects::LfsLocksApiController < Projects::GitHttpClientController
   include LfsRequest
 
   def create
-    @result = Lfs::LockFileService.new(project, user, params).execute
+    @result = Lfs::LockFileService.new(project, user, lfs_params).execute
 
     render_json(@result[:lock])
   end
 
   def unlock
-    @result = Lfs::UnlockFileService.new(project, user, params).execute
+    @result = Lfs::UnlockFileService.new(project, user, lfs_params).execute
 
     render_json(@result[:lock])
   end
 
   def index
-    @result = Lfs::LocksFinderService.new(project, user, params).execute
+    @result = Lfs::LocksFinderService.new(project, user, lfs_params).execute
 
     render_json(@result[:locks])
   end
@@ -69,4 +69,8 @@ class Projects::LfsLocksApiController < Projects::GitHttpClientController
   def upload_request?
     %w(create unlock verify).include?(params[:action])
   end
+
+  def lfs_params
+    params.permit(:id, :path, :force)
+  end
 end
diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
index 5a21403bc5e..ab4a1ccc0d1 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -32,7 +32,7 @@ module MembersHelper
   end
 
   def filter_group_project_member_path(options = {})
-    options = params.slice(:search, :sort).merge(options)
+    options = params.slice(:search, :sort).merge(options).permit!
     "#{request.path}?#{options.to_param}"
   end
 end
diff --git a/app/services/projects/create_from_template_service.rb b/app/services/projects/create_from_template_service.rb
index 8306d43ca7c..678bc0d24c3 100644
--- a/app/services/projects/create_from_template_service.rb
+++ b/app/services/projects/create_from_template_service.rb
@@ -5,7 +5,7 @@ module Projects
     include Gitlab::Utils::StrongMemoize
 
     def initialize(user, params)
-      @current_user, @params = user, params.dup
+      @current_user, @params = user, params.to_h.dup
     end
 
     def execute
diff --git a/changelogs/unreleased/raise-on-unfiltered-params.yml b/changelogs/unreleased/raise-on-unfiltered-params.yml
new file mode 100644
index 00000000000..531e9ba807e
--- /dev/null
+++ b/changelogs/unreleased/raise-on-unfiltered-params.yml
@@ -0,0 +1,5 @@
+---
+title: Actually set raise_on_unfiltered_parameters to true
+merge_request: 24443
+author: Jasper Maes
+type: other
diff --git a/config/application.rb b/config/application.rb
index 349c7258852..92a3d031c63 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -162,6 +162,9 @@ module Gitlab
 
     config.action_view.sanitized_allowed_protocols = %w(smb)
 
+    # Can be removed once upgraded to Rails 5.1 or higher
+    config.action_controller.raise_on_unfiltered_parameters = true
+
     # Nokogiri is significantly faster and uses less memory than REXML
     ActiveSupport::XmlMini.backend = 'Nokogiri'
 
diff --git a/config/initializers/new_framework_defaults.rb b/config/initializers/new_framework_defaults.rb
index a1e0667bc6f..115ee08dbb6 100644
--- a/config/initializers/new_framework_defaults.rb
+++ b/config/initializers/new_framework_defaults.rb
@@ -8,8 +8,6 @@
 #
 # Read the Guide for Upgrading Ruby on Rails for more info on each option.
 
-Rails.application.config.action_controller.raise_on_unfiltered_parameters = true
-
 # Enable per-form CSRF tokens. Previous versions had false.
 Rails.application.config.action_controller.per_form_csrf_tokens = false
 
diff --git a/spec/requests/lfs_locks_api_spec.rb b/spec/requests/lfs_locks_api_spec.rb
index 28cb90e450e..c63fbcdd84e 100644
--- a/spec/requests/lfs_locks_api_spec.rb
+++ b/spec/requests/lfs_locks_api_spec.rb
@@ -132,6 +132,17 @@ describe 'Git LFS File Locking API' do
 
         expect(json_response['lock'].keys).to match_array(%w(id path locked_at owner))
       end
+
+      context 'when a maintainer uses force' do
+        let(:authorization) { authorize_user(maintainer) }
+
+        it 'deletes the lock' do
+          project.add_maintainer(maintainer)
+          post_lfs_json url, { force: true }, headers
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
     end
   end