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

Minimal implementation for Safe Webhooks

This will be used by Geo #76, and will be improved for 8.8 in gitlab-org/gitlab-ce#13478

See merge request !334

app/models/hooks/group_hook.rb

@@ -3,7 +3,7 @@
 # Table name: web_hooks
 #
 #  id                      :integer          not null, primary key
-#  url                   :string(255)
+#  url                     :string(2000)
 #  project_id              :integer
 #  created_at              :datetime
 #  updated_at              :datetime
@@ -13,6 +13,11 @@
 #  issues_events           :boolean          default(FALSE), not null
 #  merge_requests_events   :boolean          default(FALSE), not null
 #  tag_push_events         :boolean          default(FALSE)
+#  group_id                :integer
+#  note_events             :boolean          default(FALSE), not null
+#  enable_ssl_verification :boolean          default(TRUE)
+#  build_events            :boolean          default(FALSE), not null
+#  token                   :string
 #
 
 class GroupHook < ProjectHook

app/models/hooks/project_hook.rb

@@ -7,15 +7,17 @@
 #  project_id              :integer
 #  created_at              :datetime
 #  updated_at              :datetime
-#  type                    :string           default("ProjectHook")
+#  type                    :string(255)      default("ProjectHook")
 #  service_id              :integer
 #  push_events             :boolean          default(TRUE), not null
 #  issues_events           :boolean          default(FALSE), not null
 #  merge_requests_events   :boolean          default(FALSE), not null
 #  tag_push_events         :boolean          default(FALSE)
+#  group_id                :integer
 #  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

@@ -7,15 +7,17 @@
 #  project_id              :integer
 #  created_at              :datetime
 #  updated_at              :datetime
-#  type                    :string           default("ProjectHook")
+#  type                    :string(255)      default("ProjectHook")
 #  service_id              :integer
 #  push_events             :boolean          default(TRUE), not null
 #  issues_events           :boolean          default(FALSE), not null
 #  merge_requests_events   :boolean          default(FALSE), not null
 #  tag_push_events         :boolean          default(FALSE)
+#  group_id                :integer
 #  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

@@ -7,15 +7,17 @@
 #  project_id              :integer
 #  created_at              :datetime
 #  updated_at              :datetime
-#  type                    :string           default("ProjectHook")
+#  type                    :string(255)      default("ProjectHook")
 #  service_id              :integer
 #  push_events             :boolean          default(TRUE), not null
 #  issues_events           :boolean          default(FALSE), not null
 #  merge_requests_events   :boolean          default(FALSE), not null
 #  tag_push_events         :boolean          default(FALSE)
+#  group_id                :integer
 #  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

@@ -7,15 +7,17 @@
 #  project_id              :integer
 #  created_at              :datetime
 #  updated_at              :datetime
-#  type                    :string           default("ProjectHook")
+#  type                    :string(255)      default("ProjectHook")
 #  service_id              :integer
 #  push_events             :boolean          default(TRUE), not null
 #  issues_events           :boolean          default(FALSE), not null
 #  merge_requests_events   :boolean          default(FALSE), not null
 #  tag_push_events         :boolean          default(FALSE)
+#  group_id                :integer
 #  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
@@ -40,23 +42,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
@@ -70,4 +66,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

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

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160331223143) do
+ActiveRecord::Schema.define(version: 20160413115152) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1126,6 +1126,7 @@ ActiveRecord::Schema.define(version: 20160331223143) do
     t.boolean  "note_events",                          default: false,         null: false
     t.boolean  "enable_ssl_verification",              default: true
     t.boolean  "build_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