Refactor webhook handling to accept all HTTP verbs

Gemfile

@@ -48,12 +48,12 @@ platforms :ruby_18 do
 end
 
 group :development do
-  gem 'pry'
   gem 'binding_of_caller'
   gem 'better_errors'
 end
 
 group :development, :test do
+  gem 'pry'
   gem 'rspec-rails'
   gem 'rspec'
   gem 'shoulda-matchers'

app/controllers/webhooks_controller.rb

@@ -1,39 +1,39 @@
-# This controller is designed to allow your Agents to receive cross-site Webhooks (posts).  When POSTed, your Agent will
-# have #receive_webhook called on itself with the POST params.
+# This controller is designed to allow your Agents to receive cross-site Webhooks (POSTs), or to output data streams.
+# When a POST or GET is received, your Agent will have #receive_webhook called on itself with the incoming params.
 #
-# Make POSTs to the following URL:
+# To implement webhooks, make POSTs to the following URL:
 #   http://yourserver.com/users/:user_id/webhooks/:agent_id/:secret
 # where :user_id is your User's id, :agent_id is an Agent's id, and :secret is a token that should be
 # user-specifiable in your Agent.  It is highly recommended that you verify this token whenever #receive_webhook
 # is called.  For example, one of your Agent's options could be :secret and you could compare this value
 # to params[:secret] whenever #receive_webhook is called on your Agent, rejecting invalid requests.
 #
-# Your Agent's #receive_webhook method should return an Array of [json_or_string_response, status_code].  For example:
+# Your Agent's #receive_webhook method should return an Array of [json_or_string_response, status_code, optional mime type].  For example:
 #   [{status: "success"}, 200]
 # or
-#   ["not found", 404]
+#   ["not found", 404, 'text/plain']
 
 class WebhooksController < ApplicationController
   skip_before_filter :authenticate_user!
 
-  def create
+  def handle_request
     user = User.find_by_id(params[:user_id])
     if user
       agent = user.agents.find_by_id(params[:agent_id])
       if agent
-        response, status = agent.trigger_webhook(params.except(:action, :controller, :agent_id, :user_id))
-        if response.is_a?(String)
-          render :text => response, :status => status || 200
-        elsif response.is_a?(Hash)
-          render :json => response, :status => status || 200
+        content, status, content_type = agent.trigger_webhook(params.except(:action, :controller, :agent_id, :user_id, :format), request.method_symbol.to_s, request.format.to_s)
+        if content.is_a?(String)
+          render :text => content, :status => status || 200, :content_type => content_type || 'text/plain'
+        elsif content.is_a?(Hash)
+          render :json => content, :status => status || 200
         else
-          head :ok
+          head(status || 200)
         end
       else
-        render :text => "agent not found", :status => :not_found
+        render :text => "agent not found", :status => 404
       end
     else
-      render :text => "user not found", :status => :not_found
+      render :text => "user not found", :status => 404
     end
   end
 end

app/models/agent.rb

@@ -73,7 +73,7 @@ class Agent < ActiveRecord::Base
     # Implement me in your subclass of Agent.
   end
 
-  def receive_webhook(params)
+  def receive_webhook(params, method, format)
     # Implement me in your subclass of Agent.
     ["not implemented", 404]
   end
@@ -136,8 +136,8 @@ class Agent < ActiveRecord::Base
     message.gsub(/<([^>]+)>/) { Utils.value_at(payload, $1) || "??" }
   end
 
-  def trigger_webhook(params)
-    receive_webhook(params).tap do
+  def trigger_webhook(params, method, format)
+    receive_webhook(params, method, format).tap do
       self.last_webhook_at = Time.now
       save!
     end

app/models/agents/twilio_agent.rb

@@ -78,7 +78,7 @@ module Agents
       "#{server_url}/users/#{self.user.id}/webhooks/#{self.id}/#{secret}"
     end
 
-    def receive_webhook(params)
+    def receive_webhook(params, method, format)
       if memory['pending_calls'].has_key? params['secret']
         response = Twilio::TwiML::Response.new {|r| r.Say memory['pending_calls'][params['secret']], :voice => 'woman'}
         memory['pending_calls'].delete params['secret']

app/models/agents/webhook_agent.rb

@@ -36,9 +36,9 @@ module Agents
         "payload_path" => "payload"}
     end
 
-    def receive_webhook(params)
+    def receive_webhook(params, method, format)
       secret = params.delete('secret')
-      return ["Not Authorized", 401] unless secret == options['secret']
+      return ["Not Authorized", 401] unless secret == options['secret'] && method == "post"
 
       create_event(:payload => payload_for(params))
 

config/routes.rb

@@ -31,7 +31,7 @@ Huginn::Application.routes.draw do
   match "/worker_status" => "worker_status#show"
 
   post "/users/:user_id/update_location/:secret" => "user_location_updates#create"
-  post "/users/:user_id/webhooks/:agent_id/:secret" => "webhooks#create"
+  match "/users/:user_id/webhooks/:agent_id/:secret" => "webhooks#handle_request"
 
 #  match "/delayed_job" => DelayedJobWeb, :anchor => false
   devise_for :users, :sign_out_via => [ :post, :delete ]

spec/controllers/webhooks_controller_spec.rb

@@ -5,10 +5,12 @@ describe WebhooksController do
     cannot_receive_events!
     cannot_be_scheduled!
 
-    def receive_webhook(params)
+    def receive_webhook(params, method, format)
       if params.delete(:secret) == options[:secret]
         memory[:webhook_values] = params
-        ["success", 200]
+        memory[:webhook_format] = format
+        memory[:webhook_method] = method
+        ["success", 200, memory['content_type']]
       else
         ["failure", 404]
       end
@@ -24,31 +26,72 @@ describe WebhooksController do
 
   it "should not require login to trigger a webhook" do
     @agent.last_webhook_at.should be_nil
-    post :create, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5"
+    post :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5"
     @agent.reload.last_webhook_at.should be_within(2).of(Time.now)
     response.body.should == "success"
     response.should be_success
   end
 
   it "should call receive_webhook" do
-    post :create, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5"
-    @agent.reload.memory[:webhook_values].should == { 'key' => "value", 'another_key' => "5" }
+    post :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5"
+    @agent.reload
+    @agent.memory[:webhook_values].should == { 'key' => "value", 'another_key' => "5" }
+    @agent.memory[:webhook_format].should == "text/html"
+    @agent.memory[:webhook_method].should == "post"
     response.body.should == "success"
+    response.headers['Content-Type'].should == 'text/plain; charset=utf-8'
     response.should be_success
 
-    post :create, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "not_my_secret", :no => "go"
+    post :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "not_my_secret", :no => "go"
     @agent.reload.memory[:webhook_values].should_not == { 'no' => "go" }
     response.body.should == "failure"
     response.should be_missing
   end
 
+  it "should accept gets" do
+    get :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5"
+    @agent.reload
+    @agent.memory[:webhook_values].should == { 'key' => "value", 'another_key' => "5" }
+    @agent.memory[:webhook_format].should == "text/html"
+    @agent.memory[:webhook_method].should == "get"
+    response.body.should == "success"
+    response.should be_success
+  end
+
+  it "should pass through the received format" do
+    get :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5", :format => :json
+    @agent.reload
+    @agent.memory[:webhook_values].should == { 'key' => "value", 'another_key' => "5" }
+    @agent.memory[:webhook_format].should == "application/json"
+    @agent.memory[:webhook_method].should == "get"
+
+    post :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5", :format => :xml
+    @agent.reload
+    @agent.memory[:webhook_values].should == { 'key' => "value", 'another_key' => "5" }
+    @agent.memory[:webhook_format].should == "application/xml"
+    @agent.memory[:webhook_method].should == "post"
+
+    put :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5", :format => :atom
+    @agent.reload
+    @agent.memory[:webhook_values].should == { 'key' => "value", 'another_key' => "5" }
+    @agent.memory[:webhook_format].should == "application/atom+xml"
+    @agent.memory[:webhook_method].should == "put"
+  end
+
+  it "can accept a content-type to return" do
+    @agent.memory['content_type'] = 'application/json'
+    @agent.save!
+    get :handle_request, :user_id => users(:bob).to_param, :agent_id => @agent.id, :secret => "my_secret", :key => "value", :another_key => "5"
+    response.headers['Content-Type'].should == 'application/json; charset=utf-8'
+  end
+
   it "should fail on incorrect users" do
-    post :create, :user_id => users(:jane).to_param, :agent_id => @agent.id, :secret => "my_secret", :no => "go"
+    post :handle_request, :user_id => users(:jane).to_param, :agent_id => @agent.id, :secret => "my_secret", :no => "go"
     response.should be_missing
   end
 
   it "should fail on incorrect agents" do
-    post :create, :user_id => users(:bob).to_param, :agent_id => 454545, :secret => "my_secret", :no => "go"
+    post :handle_request, :user_id => users(:bob).to_param, :agent_id => 454545, :secret => "my_secret", :no => "go"
     response.should be_missing
   end
 end

spec/models/agents/webhook_agent_spec.rb

@@ -14,7 +14,7 @@ describe Agents::WebhookAgent do
     it 'should create event if secret matches' do
       out = nil
       lambda {
-        out = agent.receive_webhook('secret' => 'foobar', 'payload' => payload)
+        out = agent.receive_webhook({ 'secret' => 'foobar', 'payload' => payload }, "post", "text/html")
       }.should change { Event.count }.by(1)
       out.should eq(['Event Created', 201])
       Event.last.payload.should eq(payload)
@@ -23,7 +23,15 @@ describe Agents::WebhookAgent do
     it 'should not create event if secrets dont match' do
       out = nil
       lambda {
-        out = agent.receive_webhook('secret' => 'bazbat', 'payload' => payload)
+        out = agent.receive_webhook({ 'secret' => 'bazbat', 'payload' => payload }, "post", "text/html")
+      }.should change { Event.count }.by(0)
+      out.should eq(['Not Authorized', 401])
+    end
+
+    it "should only accept POSTs" do
+      out = nil
+      lambda {
+        out = agent.receive_webhook({ 'secret' => 'foobar', 'payload' => payload }, "get", "text/html")
       }.should change { Event.count }.by(0)
       out.should eq(['Not Authorized', 401])
     end

spec/routing/webhooks_controller_spec.rb

@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe "routing for webhooks" do
+  it "routes to handle_request" do
+    resulting_params = { :user_id => "6", :agent_id => "2", :secret => "foobar" }
+    get("/users/6/webhooks/2/foobar").should route_to("webhooks#handle_request", resulting_params)
+    post("/users/6/webhooks/2/foobar").should route_to("webhooks#handle_request", resulting_params)
+    put("/users/6/webhooks/2/foobar").should route_to("webhooks#handle_request", resulting_params)
+    delete("/users/6/webhooks/2/foobar").should route_to("webhooks#handle_request", resulting_params)
+  end
+
+  it "routes with format" do
+    get("/users/6/webhooks/2/foobar.json").should route_to("webhooks#handle_request",
+                                                           { :user_id => "6", :agent_id => "2", :secret => "foobar", :format => "json" })
+
+    get("/users/6/webhooks/2/foobar.atom").should route_to("webhooks#handle_request",
+                                                           { :user_id => "6", :agent_id => "2", :secret => "foobar", :format => "atom" })
+  end
+end