Added association between webhook_endpoints and oauth apps (#4060)

* Added association between webhook_endpoints and oauth apps

* Limit webhooks access by oauth_app_id when authorized by doorkeeper

* Use rails route helpers in webhooks api test

app/controllers/api/v0/webhooks_controller.rb

@@ -7,26 +7,36 @@ module Api
       skip_before_action :verify_authenticity_token, only: %w[create destroy]
 
       def index
-        @webhooks = @user.webhook_endpoints.order(:id)
+        @webhooks = webhooks_scope.order(:id)
       end
 
       def create
-        @webhook = @user.webhook_endpoints.create!(webhook_params)
+        @webhook = @user.webhook_endpoints.new(webhook_params)
+        @webhook.oauth_application_id = doorkeeper_token.application_id if doorkeeper_token
+        @webhook.save!
         render "show", status: :created
       end
 
       def show
-        @webhook = @user.webhook_endpoints.find(params[:id])
+        @webhook = webhooks_scope.find(params[:id])
       end
 
       def destroy
-        webhook = @user.webhook_endpoints.find(params[:id])
+        webhook = webhooks_scope.find(params[:id])
         webhook.destroy!
         head :no_content
       end
 
       private
 
+      def webhooks_scope
+        if doorkeeper_token
+          @user.webhook_endpoints.for_app(doorkeeper_token.application_id)
+        else
+          @user.webhook_endpoints
+        end
+      end
+
       def webhook_params
         params.require(:webhook_endpoint).permit(:target_url, :source, events: [])
       end

app/models/webhook/endpoint.rb

@@ -1,6 +1,11 @@
 module Webhook
   class Endpoint < ApplicationRecord
     belongs_to :user, inverse_of: :webhook_endpoints
+    # rubocop:disable Rails/InverseOf
+    belongs_to :oauth_application, optional: true,
+                                   class_name: "Doorkeeper::Application",
+                                   foreign_key: :oauth_application_id
+    # rubocop:enable Rails/InverseOf
 
     validates :target_url, presence: true, uniqueness: true, url: { schemes: %w[https] }
     validates :source, :events, presence: true
@@ -8,6 +13,7 @@ module Webhook
     attribute :events, :string, array: true, default: []
 
     scope :for_events, ->(events) { where("events @> ARRAY[?]::varchar[]", Array(events)) }
+    scope :for_app, ->(app_id) { where(oauth_application_id: app_id) }
 
     def self.table_name_prefix
       "webhook_"

db/migrate/20190918104106_add_oauth_application_id_to_webhook_endpoints.rb

@@ -0,0 +1,7 @@
+class AddOauthApplicationIdToWebhookEndpoints < ActiveRecord::Migration[5.2]
+  def change
+    change_table :webhook_endpoints do |t|
+      t.references :oauth_application, foreign_key: true
+    end
+  end
+end

db/schema.rb

@@ -12,7 +12,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_09_10_153845) do
+ActiveRecord::Schema.define(version: 2019_09_18_104106) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -1153,11 +1153,13 @@ ActiveRecord::Schema.define(version: 2019_09_10_153845) do
   create_table "webhook_endpoints", force: :cascade do |t|
     t.datetime "created_at", null: false
     t.string "events", null: false, array: true
+    t.bigint "oauth_application_id"
     t.string "source"
     t.string "target_url", null: false
     t.datetime "updated_at", null: false
     t.bigint "user_id", null: false
     t.index ["events"], name: "index_webhook_endpoints_on_events"
+    t.index ["oauth_application_id"], name: "index_webhook_endpoints_on_oauth_application_id"
     t.index ["user_id"], name: "index_webhook_endpoints_on_user_id"
   end
 
@@ -1174,5 +1176,6 @@ ActiveRecord::Schema.define(version: 2019_09_10_153845) do
   add_foreign_key "push_notification_subscriptions", "users"
   add_foreign_key "sponsorships", "organizations"
   add_foreign_key "sponsorships", "users"
+  add_foreign_key "webhook_endpoints", "oauth_applications"
   add_foreign_key "webhook_endpoints", "users"
 end

spec/requests/api/v0/webhooks_spec.rb

@@ -2,26 +2,23 @@ require "rails_helper"
 
 RSpec.describe "Api::V0::Webhooks", type: :request do
   let(:user) { create(:user) }
-  let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") }
-
-  before do
-    sign_in user
-  end
 
   describe "GET /api/v0/webhooks" do
+    let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") }
     let!(:webhook2) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/webhook") }
 
     before do
+      sign_in user
       create(:webhook_endpoint)
     end
 
     it "returns 200 on success" do
-      get "/api/webhooks"
+      get api_webhooks_path
       expect(response).to have_http_status(:ok)
     end
 
     it "returns json on success" do
-      get "/api/webhooks"
+      get api_webhooks_path
       json = JSON.parse(response.body)
       ids = json.map { |item| item["id"] }
       urls = json.map { |item| item["target_url"] }
@@ -31,24 +28,30 @@ RSpec.describe "Api::V0::Webhooks", type: :request do
   end
 
   describe "GET /api/v0/webhooks/:id" do
+    let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") }
+
+    before do
+      sign_in user
+    end
+
     it "returns 200 on success" do
-      get "/api/webhooks/#{webhook.id}"
+      get api_webhook_path(webhook.id)
       expect(response).to have_http_status(:ok)
     end
 
     it "returns 404 if the webhook does not exist" do
-      get "/api/webhooks/9999"
+      get api_webhook_path(9999)
       expect(response).to have_http_status(:not_found)
     end
 
     it "returns 404 if another user webhook is accessed" do
       other_webhook = create(:webhook_endpoint, user: create(:user))
-      get "/api/webhooks/#{other_webhook.id}"
+      get api_webhook_path(other_webhook.id)
       expect(response).to have_http_status(:not_found)
     end
 
     it "returns json on success" do
-      get "/api/webhooks/#{webhook.id}"
+      get api_webhook_path(webhook.id)
       json = JSON.parse(response.body)
       expect(json["target_url"]).to eq(webhook.target_url)
       expect(json["user"]["username"]).to eq(user.username)
@@ -64,22 +67,27 @@ RSpec.describe "Api::V0::Webhooks", type: :request do
       }
     end
 
+    before do
+      sign_in user
+    end
+
     it "creates a webhook" do
       expect do
-        post "/api/webhooks", params: { webhook_endpoint: webhook_params }
+        post api_webhooks_path, params: { webhook_endpoint: webhook_params }
       end.to change(Webhook::Endpoint, :count).by(1)
     end
 
     it "creates a webhook with events and data" do
-      post "/api/webhooks", params: { webhook_endpoint: webhook_params }
+      post api_webhooks_path, params: { webhook_endpoint: webhook_params }
       created_webhook = user.webhook_endpoints.last
       expect(created_webhook.events).to eq(%w[article_created article_updated article_destroyed])
       expect(created_webhook.target_url).to eq(webhook_params[:target_url])
       expect(created_webhook.source).to eq(webhook_params[:source])
+      expect(created_webhook.oauth_application_id).to eq(nil)
     end
 
     it "returns :created and json response on success" do
-      post "/api/webhooks", params: { webhook_endpoint: webhook_params }
+      post api_webhooks_path, params: { webhook_endpoint: webhook_params }
       expect(response).to have_http_status(:created)
       expect(response.content_type).to eq("application/json")
       json = JSON.parse(response.body)
@@ -88,27 +96,72 @@ RSpec.describe "Api::V0::Webhooks", type: :request do
   end
 
   describe "DELETE /api/v0/webhooks/:id" do
+    let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") }
+
+    before do
+      sign_in user
+    end
+
     it "deletes the webhook" do
       expect do
-        delete "/api/webhooks/#{webhook.id}"
+        delete api_webhook_path(webhook.id)
       end.to change(Webhook::Endpoint, :count).by(-1)
     end
 
     it "returns 204 on success" do
-      delete "/api/webhooks/#{webhook.id}"
+      delete api_webhook_path(webhook.id)
       expect(response).to have_http_status(:no_content)
     end
 
     it "doesn't allow to destroy other user webhook" do
       other_webhook = create(:webhook_endpoint, user: create(:user))
       expect do
-        delete "/api/webhooks/#{other_webhook.id}"
+        delete api_webhook_path(other_webhook.id)
       end.not_to change(Webhook::Endpoint, :count)
     end
 
     it "returns 404 if another user webhook is accessed" do
       other_webhook = create(:webhook_endpoint, user: create(:user))
-      delete "/api/webhooks/#{other_webhook.id}"
+      delete api_webhook_path(other_webhook.id)
+      expect(response).to have_http_status(:not_found)
+    end
+  end
+
+  describe "authorized with doorkeeper" do
+    let!(:oauth_app) { create(:application) }
+    let!(:oauth_app2) { create(:application) }
+    let(:access_token) { create :doorkeeper_access_token, resource_owner: user, application: oauth_app2 }
+
+    it "renders index successfully" do
+      get api_webhooks_path, params: { access_token: access_token.token }
+      expect(response.content_type).to eq("application/json")
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "renders only corresponding webhooks" do
+      create(:webhook_endpoint, oauth_application_id: oauth_app.id, user: user)
+      webhook2 = create(:webhook_endpoint, oauth_application_id: oauth_app2.id, user: user)
+      get api_webhooks_path, params: { access_token: access_token.token }
+
+      json = JSON.parse(response.body)
+      ids = json.map { |item| item["id"] }
+      expect(ids).to eq([webhook2.id])
+    end
+
+    it "sets correct oauth app id for the webhook if needed" do
+      webhook_params = {
+        source: "stackbit",
+        target_url: Faker::Internet.url(scheme: "https"),
+        events: %w[article_created article_updated article_destroyed]
+      }
+      post api_webhooks_path, params: { access_token: access_token.token, webhook_endpoint: webhook_params }
+      webhook = user.webhook_endpoints.find_by(target_url: webhook_params[:target_url])
+      expect(webhook.oauth_application_id).to eq(oauth_app2.id)
+    end
+
+    it "doesn't allow destroying another app webhook" do
+      other_webhook = create(:webhook_endpoint, user: user)
+      delete api_webhook_path(other_webhook.id), params: { access_token: access_token.token }
       expect(response).to have_http_status(:not_found)
     end
   end