Skip to content

Commit

Permalink
Don't call hook methods for 'ping' requests
Browse files Browse the repository at this point in the history
Signed-off-by: Max Brunsfeld <mbrunsfeld@pivotallabs.com>
  • Loading branch information
caleb miles authored and Max Brunsfeld committed Jun 2, 2014
1 parent ba75eaa commit dc5b1ce
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
3 changes: 3 additions & 0 deletions lib/right_hook/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class App < Sinatra::Base
halt 404, "Unknown event type" unless Event::KNOWN_TYPES.include?(event_type)
halt 501, "Event type not implemented" unless respond_to?("on_#{event_type}")

return 200 if request.env['HTTP_X_GITHUB_EVENT'] == Event::PING

require_valid_signature(content, owner, repo_name, event_type)

json = JSON.parse(params['payload'])
Expand All @@ -39,6 +41,7 @@ def secret(owner, repo_name, event_type)
end

private

def require_valid_signature(content, owner, repo_name, event_type)
s = secret(owner, repo_name, event_type)
expected_signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('sha1'), s, content)
Expand Down
1 change: 1 addition & 0 deletions lib/right_hook/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def github_name(event_type)
ISSUE = 'issue'.freeze
PULL_REQUEST = 'pull_request'.freeze
ISSUE_COMMENT = 'issue_comment'.freeze
PING = 'ping'.freeze

KNOWN_TYPES = [
ISSUE,
Expand Down
24 changes: 19 additions & 5 deletions spec/app_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe RightHook::App do
class BareApp < RightHook::App
class BareApp < RightHook::DebugApp
def secret(owner, repo_name, event_type)
'secret'
end
Expand All @@ -25,7 +25,7 @@ def secret(owner, repo_name, event_type)
expect { RightHook::App.new!.secret('owner', 'repo', 'issue') }.to raise_error(NotImplementedError)
end

describe 'the status of a request' do
describe 'when an app implements the hook method' do
class WeirdApp < BareApp
def on_issue(owner, repo_name, action, issue_json)
302
Expand All @@ -34,11 +34,25 @@ def on_issue(owner, repo_name, action, issue_json)

let(:app) { WeirdApp }

it 'is 200 regardless of what the hook method returns' do
payload = '{}'
WeirdApp.any_instance.should_receive(:on_issue).and_call_original
let(:payload) { '{}' }

it 'responds with a 200 regardless of what the hook method returns' do
expect_any_instance_of(WeirdApp).to receive(:on_issue).and_call_original

post_with_signature(path: '/hook/mark-rushakoff/right_hook/issue', payload: payload, secret: 'secret')
expect(last_response.status).to eq(200)
end

it 'does not call the hook method when a ping event is received' do
expect_any_instance_of(WeirdApp).not_to receive(:on_issue)

post(
'/hook/mark-rushakoff/right_hook/issue',
{payload: payload},
generate_secret_header('secret', URI.encode_www_form(payload: payload)).merge(
'HTTP_X_GITHUB_EVENT' => 'ping'
),
)
end
end
end

0 comments on commit dc5b1ce

Please sign in to comment.