Merge branch 'feature/backport-safewebhooks' into 'master'

app/controllers/admin/hooks_controller.rb

@@ -39,6 +39,12 @@ class Admin::HooksController < Admin::ApplicationController
   end
 
   def hook_params
-    params.require(:hook).permit(:url, :enable_ssl_verification, :push_events, :tag_push_events)
+    params.require(:hook).permit(
+      :enable_ssl_verification,
+      :push_events,
+      :tag_push_events,
+      :token,
+      :url
+    )
   end
 end

app/controllers/projects/hooks_controller.rb

@@ -52,8 +52,16 @@ class Projects::HooksController < Projects::ApplicationController
   end
 
   def hook_params
-    params.require(:hook).permit(:url, :push_events, :issues_events,
-      :merge_requests_events, :tag_push_events, :note_events,
-      :build_events, :enable_ssl_verification)
+    params.require(:hook).permit(
+      :build_events,
+      :enable_ssl_verification,
+      :issues_events,
+      :merge_requests_events,
+      :note_events,
+      :push_events,
+      :tag_push_events,
+      :token,
+      :url
+    )
   end
 end

app/models/hooks/project_hook.rb

@@ -16,6 +16,7 @@
 #  note_events             :boolean          default(FALSE), not null
 #  enable_ssl_verification :boolean          default(TRUE)
 #  build_events            :boolean          default(FALSE), not null
+#  token                   :string
 #
 
 class ProjectHook < WebHook

app/models/hooks/service_hook.rb

@@ -16,6 +16,7 @@
 #  note_events             :boolean          default(FALSE), not null
 #  enable_ssl_verification :boolean          default(TRUE)
 #  build_events            :boolean          default(FALSE), not null
+#  token                   :string
 #
 
 class ServiceHook < WebHook

app/models/hooks/system_hook.rb

@@ -16,6 +16,7 @@
 #  note_events             :boolean          default(FALSE), not null
 #  enable_ssl_verification :boolean          default(TRUE)
 #  build_events            :boolean          default(FALSE), not null
+#  token                   :string
 #
 
 class SystemHook < WebHook

app/models/hooks/web_hook.rb

@@ -16,6 +16,7 @@
 #  note_events             :boolean          default(FALSE), not null
 #  enable_ssl_verification :boolean          default(TRUE)
 #  build_events            :boolean          default(FALSE), not null
+#  token                   :string
 #
 
 class WebHook < ActiveRecord::Base
@@ -43,23 +44,17 @@ class WebHook < ActiveRecord::Base
     if parsed_url.userinfo.blank?
       response = WebHook.post(url,
                               body: data.to_json,
-                              headers: {
-                                  "Content-Type" => "application/json",
-                                  "X-Gitlab-Event" => hook_name.singularize.titleize
-                              },
+                              headers: build_headers(hook_name),
                               verify: enable_ssl_verification)
     else
-      post_url = url.gsub("#{parsed_url.userinfo}@", "")
+      post_url = url.gsub("#{parsed_url.userinfo}@", '')
       auth = {
         username: CGI.unescape(parsed_url.user),
         password: CGI.unescape(parsed_url.password),
       }
       response = WebHook.post(post_url,
                               body: data.to_json,
-                              headers: {
-                                  "Content-Type" => "application/json",
-                                  "X-Gitlab-Event" => hook_name.singularize.titleize
-                              },
+                              headers: build_headers(hook_name),
                               verify: enable_ssl_verification,
                               basic_auth: auth)
     end
@@ -73,4 +68,15 @@ class WebHook < ActiveRecord::Base
   def async_execute(data, hook_name)
     Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data, hook_name)
   end
+
+  private
+
+  def build_headers(hook_name)
+    headers = {
+      'Content-Type' => 'application/json',
+      'X-Gitlab-Event' => hook_name.singularize.titleize
+    }
+    headers['X-Gitlab-Token'] = token if token.present?
+    headers
+  end
 end

app/views/admin/hooks/index.html.haml

@@ -13,9 +13,15 @@
   = form_errors(@hook)
 
   .form-group
-    = f.label :url, "URL:", class: 'control-label'
+    = f.label :url, 'URL', class: 'control-label'
     .col-sm-10
-      = f.text_field :url, class: "form-control"
+      = f.text_field :url, class: 'form-control'
+  .form-group
+    = f.label :token, 'Secret Token', class: 'control-label'
+    .col-sm-10
+      = f.text_field :token, class: 'form-control'
+      %p.help-block
+        Use this token to validate received payloads
   .form-group
     = f.label :url, "Trigger", class: 'control-label'
     .col-sm-10.prepend-top-10

app/views/projects/hooks/index.html.haml

@@ -16,6 +16,11 @@
         = f.label :url, "URL", class: "label-light"
         = f.text_field :url, class: "form-control", placeholder: "http://example.com/trigger-ci.json"
       .form-group
+        = f.label :token, "Secret Token", class: 'label-light'
+        = f.text_field :token, class: "form-control", placeholder: ''
+        %p.help-block
+          Use this token to validate received payloads
+      .form-group
         = f.label :url, "Trigger", class: "label-light"
         %div
           = f.check_box :push_events, class: "pull-left"

db/migrate/20160413115152_add_token_to_web_hooks.rb

+class AddTokenToWebHooks < ActiveRecord::Migration
+  def change
+    add_column :web_hooks, :token, :string
+  end
+end

db/schema.rb

@@ -1025,6 +1025,7 @@ ActiveRecord::Schema.define(version: 20160421130527) do
     t.boolean  "enable_ssl_verification",              default: true
     t.boolean  "build_events",                         default: false,         null: false
     t.boolean  "wiki_page_events",                     default: false,         null: false
+    t.string   "token"
   end
 
   add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree

spec/factories/project_hooks.rb

 FactoryGirl.define do
   factory :project_hook do
     url { FFaker::Internet.uri('http') }
+
+    trait :token do
+      token { SecureRandom.hex(10) }
+    end
   end
 end

spec/models/hooks/web_hook_spec.rb

@@ -43,51 +43,65 @@ describe WebHook, models: true do
   end
 
   describe "execute" do
+    let(:project) { create(:project) }
+    let(:project_hook) { create(:project_hook) }
+
     before(:each) do
-      @project_hook = create(:project_hook)
-      @project = create(:project)
-      @project.hooks << [@project_hook]
+      project.hooks << [project_hook]
       @data = { before: 'oldrev', after: 'newrev', ref: 'ref' }
 
-      WebMock.stub_request(:post, @project_hook.url)
+      WebMock.stub_request(:post, project_hook.url)
+    end
+
+    context 'when token is defined' do
+      let(:project_hook) { create(:project_hook, :token) }
+
+      it 'POSTs to the webhook URL' do
+        project_hook.execute(@data, 'push_hooks')
+        expect(WebMock).to have_requested(:post, project_hook.url).with(
+          headers: { 'Content-Type' => 'application/json',
+                     'X-Gitlab-Event' => 'Push Hook',
+                     'X-Gitlab-Token' => project_hook.token }
+        ).once
+      end
     end
 
     it "POSTs to the webhook URL" do
-      @project_hook.execute(@data, 'push_hooks')
-      expect(WebMock).to have_requested(:post, @project_hook.url).with(
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook' }
+      project_hook.execute(@data, 'push_hooks')
+      expect(WebMock).to have_requested(:post, project_hook.url).with(
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Push Hook' }
       ).once
     end
 
     it "POSTs the data as JSON" do
-      @project_hook.execute(@data, 'push_hooks')
-      expect(WebMock).to have_requested(:post, @project_hook.url).with(
-        headers: { 'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook' }
+      project_hook.execute(@data, 'push_hooks')
+      expect(WebMock).to have_requested(:post, project_hook.url).with(
+        headers: { 'Content-Type' => 'application/json', 'X-Gitlab-Event' => 'Push Hook' }
       ).once
     end
 
     it "catches exceptions" do
       expect(WebHook).to receive(:post).and_raise("Some HTTP Post error")
 
-      expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError)
+      expect { project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError)
     end
 
     it "handles SSL exceptions" do
       expect(WebHook).to receive(:post).and_raise(OpenSSL::SSL::SSLError.new('SSL error'))
 
-      expect(@project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error'])
+      expect(project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error'])
     end
 
     it "handles 200 status code" do
-      WebMock.stub_request(:post, @project_hook.url).to_return(status: 200, body: "Success")
+      WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: "Success")
 
-      expect(@project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
+      expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
     end
 
     it "handles 2xx status codes" do
-      WebMock.stub_request(:post, @project_hook.url).to_return(status: 201, body: "Success")
+      WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: "Success")
 
-      expect(@project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
+      expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success'])
     end
   end
 end