From 4eda233e09a782df5a935859346a04c10935491e Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 6 Jun 2023 10:42:47 +0200 Subject: [PATCH] Add webhook templating (#23289) Co-authored-by: Claire --- app/controllers/admin/webhooks_controller.rb | 2 +- app/lib/webhooks/payload_renderer.rb | 67 +++++++++++++++++++ app/models/webhook.rb | 15 ++++- app/views/admin/webhooks/_form.html.haml | 3 + app/views/admin/webhooks/show.html.haml | 16 ++--- app/workers/webhooks/delivery_worker.rb | 2 +- config/locales/simple_form.en.yml | 2 + ...20230129023109_add_template_to_webhooks.rb | 7 ++ db/schema.rb | 1 + spec/lib/webhooks/payload_renderer_spec.rb | 30 +++++++++ 10 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 app/lib/webhooks/payload_renderer.rb create mode 100644 db/migrate/20230129023109_add_template_to_webhooks.rb create mode 100644 spec/lib/webhooks/payload_renderer_spec.rb diff --git a/app/controllers/admin/webhooks_controller.rb b/app/controllers/admin/webhooks_controller.rb index 1ed3fd18a..01d9ba8ce 100644 --- a/app/controllers/admin/webhooks_controller.rb +++ b/app/controllers/admin/webhooks_controller.rb @@ -71,7 +71,7 @@ module Admin end def resource_params - params.require(:webhook).permit(:url, events: []) + params.require(:webhook).permit(:url, :template, events: []) end end end diff --git a/app/lib/webhooks/payload_renderer.rb b/app/lib/webhooks/payload_renderer.rb new file mode 100644 index 000000000..3d2731f6d --- /dev/null +++ b/app/lib/webhooks/payload_renderer.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +class Webhooks::PayloadRenderer + class DocumentTraverser + INT_REGEX = /[0-9]+/ + + def initialize(document) + @document = document.with_indifferent_access + end + + def get(path) + value = @document.dig(*parse_path(path)) + string = Oj.dump(value) + + # We want to make sure people can use the variable inside + # other strings, so it can't be wrapped in quotes. + if value.is_a?(String) + string[1...-1] + else + string + end + end + + private + + def parse_path(path) + path.split('.').filter_map do |segment| + if segment.match(INT_REGEX) + segment.to_i + else + segment.presence + end + end + end + end + + class TemplateParser < Parslet::Parser + rule(:dot) { str('.') } + rule(:digit) { match('[0-9]') } + rule(:property_name) { match('[a-z_]').repeat(1) } + rule(:array_index) { digit.repeat(1) } + rule(:segment) { (property_name | array_index) } + rule(:path) { property_name >> (dot >> segment).repeat } + rule(:variable) { (str('}}').absent? >> path).repeat.as(:variable) } + rule(:expression) { str('{{') >> variable >> str('}}') } + rule(:text) { (str('{{').absent? >> any).repeat(1) } + rule(:text_with_expressions) { (text.as(:text) | expression).repeat.as(:text) } + root(:text_with_expressions) + end + + EXPRESSION_REGEXP = / + \{\{ + [a-z_]+ + (\. + ([a-z_]+|[0-9]+) + )* + \}\} + /iox + + def initialize(json) + @document = DocumentTraverser.new(Oj.load(json)) + end + + def render(template) + template.gsub(EXPRESSION_REGEXP) { |match| @document.get(match[2...-2]) } + end +end diff --git a/app/models/webhook.rb b/app/models/webhook.rb index c556bcc2b..c46fce743 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -11,6 +11,7 @@ # enabled :boolean default(TRUE), not null # created_at :datetime not null # updated_at :datetime not null +# template :text # class Webhook < ApplicationRecord @@ -30,6 +31,7 @@ class Webhook < ApplicationRecord validates :events, presence: true validate :validate_events + validate :validate_template before_validation :strip_events before_validation :generate_secret @@ -49,7 +51,18 @@ class Webhook < ApplicationRecord private def validate_events - errors.add(:events, :invalid) if events.any? { |e| !EVENTS.include?(e) } + errors.add(:events, :invalid) if events.any? { |e| EVENTS.exclude?(e) } + end + + def validate_template + return if template.blank? + + begin + parser = Webhooks::PayloadRenderer::TemplateParser.new + parser.parse(template) + rescue Parslet::ParseFailed + errors.add(:template, :invalid) + end end def strip_events diff --git a/app/views/admin/webhooks/_form.html.haml b/app/views/admin/webhooks/_form.html.haml index c1e8f8979..8d019ff43 100644 --- a/app/views/admin/webhooks/_form.html.haml +++ b/app/views/admin/webhooks/_form.html.haml @@ -7,5 +7,8 @@ .fields-group = f.input :events, collection: Webhook::EVENTS, wrapper: :with_block_label, include_blank: false, as: :check_boxes, collection_wrapper_tag: 'ul', item_wrapper_tag: 'li' + .fields-group + = f.input :template, wrapper: :with_block_label, input_html: { placeholder: '{ "content": "Hello {{object.username}}" }' } + .actions = f.button :button, @webhook.new_record? ? t('admin.webhooks.add_new') : t('generic.save_changes'), type: :submit diff --git a/app/views/admin/webhooks/show.html.haml b/app/views/admin/webhooks/show.html.haml index cc450de26..5ac809efc 100644 --- a/app/views/admin/webhooks/show.html.haml +++ b/app/views/admin/webhooks/show.html.haml @@ -2,14 +2,14 @@ = t('admin.webhooks.title') - content_for :heading do - %h2 - %small - = fa_icon 'inbox' - = t('admin.webhooks.webhook') - = @webhook.url - -- content_for :heading_actions do - = link_to t('admin.webhooks.edit'), edit_admin_webhook_path, class: 'button' if can?(:update, @webhook) + .content__heading__row + %h2 + %small + = fa_icon 'inbox' + = t('admin.webhooks.webhook') + = @webhook.url + .content__heading__actions + = link_to t('admin.webhooks.edit'), edit_admin_webhook_path, class: 'button' if can?(:update, @webhook) .table-wrapper %table.table.horizontal-table diff --git a/app/workers/webhooks/delivery_worker.rb b/app/workers/webhooks/delivery_worker.rb index f8ed669fb..3bbc8e225 100644 --- a/app/workers/webhooks/delivery_worker.rb +++ b/app/workers/webhooks/delivery_worker.rb @@ -8,7 +8,7 @@ class Webhooks::DeliveryWorker def perform(webhook_id, body) @webhook = Webhook.find(webhook_id) - @body = body + @body = @webhook.template.blank? ? body : Webhooks::PayloadRenderer.new(body).render(@webhook.template) @response = nil perform_request diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index 9c747e595..f92e66e55 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -131,6 +131,7 @@ en: position: Higher role decides conflict resolution in certain situations. Certain actions can only be performed on roles with a lower priority webhook: events: Select events to send + template: Compose your own JSON payload using variable interpolation. Leave blank for default JSON. url: Where events will be sent to labels: account: @@ -304,6 +305,7 @@ en: position: Priority webhook: events: Enabled events + template: Payload template url: Endpoint URL 'no': 'No' not_recommended: Not recommended diff --git a/db/migrate/20230129023109_add_template_to_webhooks.rb b/db/migrate/20230129023109_add_template_to_webhooks.rb new file mode 100644 index 000000000..73bdd73ff --- /dev/null +++ b/db/migrate/20230129023109_add_template_to_webhooks.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddTemplateToWebhooks < ActiveRecord::Migration[6.1] + def change + add_column :webhooks, :template, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 35fbb8d2e..fd708b44a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1136,6 +1136,7 @@ ActiveRecord::Schema.define(version: 2023_06_05_085710) do t.boolean "enabled", default: true, null: false t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false + t.text "template" t.index ["url"], name: "index_webhooks_on_url", unique: true end diff --git a/spec/lib/webhooks/payload_renderer_spec.rb b/spec/lib/webhooks/payload_renderer_spec.rb new file mode 100644 index 000000000..074847c74 --- /dev/null +++ b/spec/lib/webhooks/payload_renderer_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Webhooks::PayloadRenderer do + subject(:renderer) { described_class.new(json) } + + let(:event) { Webhooks::EventPresenter.new(type, object) } + let(:payload) { ActiveModelSerializers::SerializableResource.new(event, serializer: REST::Admin::WebhookEventSerializer, scope: nil, scope_name: :current_user).as_json } + let(:json) { Oj.dump(payload) } + + describe '#render' do + context 'when event is account.approved' do + let(:type) { 'account.approved' } + let(:object) { Fabricate(:account, display_name: 'Foo"') } + + it 'renders event-related variables into template' do + expect(renderer.render('foo={{event}}')).to eq 'foo=account.approved' + end + + it 'renders event-specific variables into template' do + expect(renderer.render('foo={{object.username}}')).to eq "foo=#{object.username}" + end + + it 'escapes values for use in JSON' do + expect(renderer.render('foo={{object.account.display_name}}')).to eq 'foo=Foo\\"' + end + end + end +end