From 2408519e08084f6d81f2628eba6868910ee7fcfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= <fjlopez@gitlab.com>
Date: Thu, 21 Jun 2018 10:38:59 +0200
Subject: [PATCH] Changing the hook test action to use POST

---
 app/helpers/hooks_helper.rb                                  | 2 +-
 .../unreleased/security-fj-missing-csrf-system-hooks.yml     | 5 +++++
 config/routes/admin.rb                                       | 2 +-
 config/routes/project.rb                                     | 2 +-
 spec/routing/admin_routing_spec.rb                           | 2 +-
 spec/routing/project_routing_spec.rb                         | 2 +-
 6 files changed, 10 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/security-fj-missing-csrf-system-hooks.yml

diff --git a/app/helpers/hooks_helper.rb b/app/helpers/hooks_helper.rb
index 551b9cca6b1..0a356ba55d2 100644
--- a/app/helpers/hooks_helper.rb
+++ b/app/helpers/hooks_helper.rb
@@ -10,7 +10,7 @@ module HooksHelper
 
     trigger_human_name = trigger.to_s.tr('_', ' ').camelize
 
-    link_to path, rel: 'nofollow' do
+    link_to path, rel: 'nofollow', method: :post do
       content_tag(:span, trigger_human_name)
     end
   end
diff --git a/changelogs/unreleased/security-fj-missing-csrf-system-hooks.yml b/changelogs/unreleased/security-fj-missing-csrf-system-hooks.yml
new file mode 100644
index 00000000000..fabf48acbbc
--- /dev/null
+++ b/changelogs/unreleased/security-fj-missing-csrf-system-hooks.yml
@@ -0,0 +1,5 @@
+---
+title: Adding CSRF protection to Hooks test action
+merge_request:
+author:
+type: security
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index ff27ceb50dc..109f00631fb 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -54,7 +54,7 @@ namespace :admin do
 
   resources :hooks, only: [:index, :create, :edit, :update, :destroy] do
     member do
-      get :test
+      post :test
     end
 
     resources :hook_logs, only: [:show] do
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 5057e937941..8e019f8c8bb 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -301,7 +301,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       resources :hooks, only: [:index, :create, :edit, :update, :destroy], constraints: { id: /\d+/ } do
         member do
-          get :test
+          post :test
         end
 
         resources :hook_logs, only: [:show] do
diff --git a/spec/routing/admin_routing_spec.rb b/spec/routing/admin_routing_spec.rb
index 179fc9733ad..98df5f787f7 100644
--- a/spec/routing/admin_routing_spec.rb
+++ b/spec/routing/admin_routing_spec.rb
@@ -79,7 +79,7 @@ end
 # edit_admin_hook GET    /admin/hooks/:id(.:format)           admin/hooks#edit
 describe Admin::HooksController, "routing" do
   it "to #test" do
-    expect(get("/admin/hooks/1/test")).to route_to('admin/hooks#test', id: '1')
+    expect(post("/admin/hooks/1/test")).to route_to('admin/hooks#test', id: '1')
   end
 
   it "to #index" do
diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb
index 56d93095a85..70a7707826e 100644
--- a/spec/routing/project_routing_spec.rb
+++ b/spec/routing/project_routing_spec.rb
@@ -389,7 +389,7 @@ describe 'project routing' do
   #                   DELETE /:project_id/hooks/:id(.:format)      hooks#destroy
   describe Projects::HooksController, 'routing' do
     it 'to #test' do
-      expect(get('/gitlab/gitlabhq/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
+      expect(post('/gitlab/gitlabhq/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
     end
 
     it_behaves_like 'RESTful project resources' do