diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index 93c4894ea0f5791d023c5d3f08eae9480abb8632..4e85b6b4cf200026c1207a68f98e14726dc16655 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -39,6 +39,12 @@ def test 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 5fd4f855dec8d4073574d53d2b6aff161a64f158..dfa9bd259e80fba381369710ba145bf59fc405ac 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -52,8 +52,16 @@ def hook 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 bc6e0f98c3c5ebb1bd24560f29463cbd88be1778..d149511b868f3fd9a17ea960867e0c23ed7c1108 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 80962264ba2c4b398a492521f5ac8dc60c636ba7..f45145eeb3a8833c3f8e5a1f640d60a53a536005 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 15dddcc2447f517894535810618dc219ac66c438..012cc8ec0058f1b6e7bfdf17bf976aa1c66be897 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 3a2e4f546f72e4c7c7ebacd7e16e4213e7a93e44..1e3b48155967a8fbdec06d025011f4fb0b20fbf5 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 @@ def execute(data, hook_name) 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 @@ def execute(data, hook_name) 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 67d23c80233e1c31c1cb4b95b6750aa7f4db4fc2..7b388cf7862dcfeecc0df872943ac156ffe49c40 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 6f1ee209430d9f7a68e6c1a7951e416d966075ff..36c1d69f060d9d5cf9f72fd66aed2eaacb4664c3 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -15,6 +15,11 @@ .form-group = 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 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 0000000000000000000000000000000000000000..f04225068cd2cf08c851b0d062d66b1f6450d19a --- /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 42457d923534680ba5c7832a8f83820d9a85fb81..04aee737e4ccacaba3db0c4066908ed18ce38774 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1025,6 +1025,7 @@ 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 94dd935a0393cd7dbde3ef9c70ccc768530b6806..3195fb3ddcc90332facafc6ccb5bfd7b80a6d7f1 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 04bc2dcfb16a80c9b8ebc062262b6c9aebdf465f..37a27d73aab89207b5ed58dc56cdf21b386272bb 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -43,51 +43,65 @@ 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