From 9a9c2ed71923c32a2ef7420f4f493bdbb18c02b6 Mon Sep 17 00:00:00 2001 From: Mantas Masalskis Date: Tue, 16 Mar 2021 07:25:55 +0000 Subject: [PATCH] Follow up 20d3c5027f52664ad09c5009b4d6cdf9e95336c1 - Fixes #342 - prevent deletion of Webhooks that are used by Job/Macro/Trigger --- app/models/webhook.rb | 6 +- .../webhook/ensure_no_related_objects.rb | 69 +++++++++++++++++++ spec/factories/webhook.rb | 1 + spec/models/webhook_spec.rb | 18 +++++ 4 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 app/models/webhook/ensure_no_related_objects.rb diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 4285ec1a3..232d05141 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -5,8 +5,9 @@ class Webhook < ApplicationModel include ChecksLatestChangeObserved include HasCollectionUpdate - before_create :validate_endpoint - before_update :validate_endpoint + before_create :validate_endpoint + before_update :validate_endpoint + before_destroy Webhook::EnsureNoRelatedObjects validates :name, presence: true @@ -19,5 +20,4 @@ class Webhook < ApplicationModel rescue URI::InvalidURIError raise Exceptions::UnprocessableEntity, 'Invalid endpoint!' end - end diff --git a/app/models/webhook/ensure_no_related_objects.rb b/app/models/webhook/ensure_no_related_objects.rb new file mode 100644 index 000000000..73cf454e6 --- /dev/null +++ b/app/models/webhook/ensure_no_related_objects.rb @@ -0,0 +1,69 @@ +# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ + +class Webhook::EnsureNoRelatedObjects + + attr_reader :record + + def self.before_destroy(record) + new(record).before_destroy + end + + def self.referencing_models + # this cache doesn't need to be cleared as the result won't change + @referencing_models ||= Models + .all + .keys + .select { |klass| klass.column_names.include? 'perform' } + end + + def initialize(record) + @record = record + end + + def before_destroy + return if record.new_record? + + ensure_no_related_objects! + end + + private + + def ensure_no_related_objects! + return if related_objects.blank? + + raise Exceptions::UnprocessableEntity, "Cannot delete! This webhook is referenced by #{references_text}" + end + + def related_objects + @related_objects ||= self.class.referencing_models.each_with_object({}) do |model, result| + performables = referencing_performables(model) + next if performables.blank? + + result[model.name] = performables + end + end + + def referencing_performables(model) + model.find_each.with_object([]) do |performable, result| + next if !webhook_referenced?(performable) + + result.push({ + id: performable.id, + name: performable.name + }) + end + end + + def webhook_referenced?(performable) + record.id == performable.perform + &.dig('notification.webhook', 'webhook_id') + &.to_i + end + + def references_text + related_objects.map do |model, performables| + performables_text = performables.map { |performable| "#{performable[:name]} (##{performable[:id]})" }.join(', ') + "#{model}: #{performables_text}" + end.join(', ') + end +end diff --git a/spec/factories/webhook.rb b/spec/factories/webhook.rb index 4230500d5..f4ed19b4c 100644 --- a/spec/factories/webhook.rb +++ b/spec/factories/webhook.rb @@ -1,6 +1,7 @@ FactoryBot.define do factory :webhook do sequence(:name) { |n| "Test webhook #{n}" } + endpoint { 'http://example.com/endpoint' } ssl_verify { true } active { true } created_by_id { 1 } diff --git a/spec/models/webhook_spec.rb b/spec/models/webhook_spec.rb index f19522ef4..0a5ccb829 100644 --- a/spec/models/webhook_spec.rb +++ b/spec/models/webhook_spec.rb @@ -38,4 +38,22 @@ RSpec.describe Webhook, type: :model do end end + + describe '#destroy' do + subject(:webhook) { create(:webhook) } + + context 'when no dependencies' do + it 'removes the object' do + expect { webhook.destroy }.to change(webhook, :destroyed?).to true + end + end + + context 'when related object exists' do + let!(:trigger) { create(:trigger, perform: { 'notification.webhook' => { 'webhook_id' => webhook.id.to_s } }) } + + it 'raises error with details' do + expect { webhook.destroy }.to raise_error(Exceptions::UnprocessableEntity, /#{Regexp.escape("Trigger: #{trigger.name} (##{trigger.id})")}/) + end + end + end end