summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-05-02 15:13:42 +0000
committerRobert Speicher <robert@gitlab.com>2016-05-02 15:13:42 +0000
commitec2710d0f545864489c0fb6525b722a6cba7ab33 (patch)
treecbf6348184dc2491a2a598a00214f08c91c407a2
parent15a2c558022908e23f0ff6ac80a135899aa0899a (diff)
parent00ced598ea65f1b957c43576bc1564ed3f67d749 (diff)
downloadgitlab-ce-ec2710d0f545864489c0fb6525b722a6cba7ab33.tar.gz
Merge branch 'feature/backport-safewebhooks' into 'master'
Backported minimal safewebhook implementation to GitLab CE This brings a minimal implementation for gitlab-org/gitlab-ce#13478 backported from EE (gitlab-org/gitlab-ee!334). Also added UI to configure Secret Token Fixes #15365. See merge request !3940
-rw-r--r--app/controllers/admin/hooks_controller.rb8
-rw-r--r--app/controllers/projects/hooks_controller.rb14
-rw-r--r--app/models/hooks/project_hook.rb1
-rw-r--r--app/models/hooks/service_hook.rb1
-rw-r--r--app/models/hooks/system_hook.rb1
-rw-r--r--app/models/hooks/web_hook.rb24
-rw-r--r--app/views/admin/hooks/index.html.haml10
-rw-r--r--app/views/projects/hooks/index.html.haml5
-rw-r--r--db/migrate/20160413115152_add_token_to_web_hooks.rb5
-rw-r--r--db/schema.rb1
-rw-r--r--spec/factories/project_hooks.rb4
-rw-r--r--spec/models/hooks/web_hook_spec.rb46
12 files changed, 89 insertions, 31 deletions
diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index 93c4894ea0f..4e85b6b4cf2 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/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
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 5fd4f855dec..dfa9bd259e8 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/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
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index bc6e0f98c3c..d149511b868 100644
--- a/app/models/hooks/project_hook.rb
+++ b/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
diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb
index 80962264ba2..f45145eeb3a 100644
--- a/app/models/hooks/service_hook.rb
+++ b/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
diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb
index 15dddcc2447..012cc8ec005 100644
--- a/app/models/hooks/system_hook.rb
+++ b/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
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 3a2e4f546f7..1e3b4815596 100644
--- a/app/models/hooks/web_hook.rb
+++ b/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
diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml
index 67d23c80233..7b388cf7862 100644
--- a/app/views/admin/hooks/index.html.haml
+++ b/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
diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml
index 6f1ee209430..36c1d69f060 100644
--- a/app/views/projects/hooks/index.html.haml
+++ b/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"
diff --git a/db/migrate/20160413115152_add_token_to_web_hooks.rb b/db/migrate/20160413115152_add_token_to_web_hooks.rb
new file mode 100644
index 00000000000..f04225068cd
--- /dev/null
+++ b/db/migrate/20160413115152_add_token_to_web_hooks.rb
@@ -0,0 +1,5 @@
+class AddTokenToWebHooks < ActiveRecord::Migration
+ def change
+ add_column :web_hooks, :token, :string
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 42457d92353..04aee737e4c 100644
--- a/db/schema.rb
+++ b/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
diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb
index 94dd935a039..3195fb3ddcc 100644
--- a/spec/factories/project_hooks.rb
+++ b/spec/factories/project_hooks.rb
@@ -1,5 +1,9 @@
FactoryGirl.define do
factory :project_hook do
url { FFaker::Internet.uri('http') }
+
+ trait :token do
+ token { SecureRandom.hex(10) }
+ end
end
end
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 04bc2dcfb16..37a27d73aab 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/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