Fixes #3372 - Reasoning about Webhooks activity.

This commit is contained in:
Martin Edenhofer 2021-02-23 15:52:16 +01:00 committed by Thorsten Eckel
parent 1ac83882f7
commit 02417225f7
24 changed files with 495 additions and 84 deletions

View file

@ -2,6 +2,7 @@ class App.ControllerGenericIndex extends App.Controller
events: events:
'click [data-type=edit]': 'edit' 'click [data-type=edit]': 'edit'
'click [data-type=new]': 'new' 'click [data-type=new]': 'new'
'click [data-type=payload]': 'payload'
'click [data-type=import]': 'import' 'click [data-type=import]': 'import'
'click .js-description': 'description' 'click .js-description': 'description'
@ -152,6 +153,12 @@ class App.ControllerGenericIndex extends App.Controller
else else
@table.update(objects: objects, pagerSelected: @pageData.pagerSelected, pagerTotalCount: @pageData.pagerTotalCount) @table.update(objects: objects, pagerSelected: @pageData.pagerSelected, pagerTotalCount: @pageData.pagerTotalCount)
if @pageData.logFacility
new App.HttpLog(
el: @$('.page-footer')
facility: @pageData.logFacility
)
edit: (id, e) => edit: (id, e) =>
e.preventDefault() e.preventDefault()
item = App[ @genericObject ].find(id) item = App[ @genericObject ].find(id)
@ -181,6 +188,13 @@ class App.ControllerGenericIndex extends App.Controller
veryLarge: @veryLarge veryLarge: @veryLarge
) )
payload: (e) ->
e.preventDefault()
new App.WidgetPayloadExample(
baseUrl: @payloadExampleUrl
container: @el.closest('.content')
)
import: (e) -> import: (e) ->
e.preventDefault() e.preventDefault()
@importCallback() @importCallback()

View file

@ -406,11 +406,29 @@ class App.UiElement.ticket_perform_action
selectionRecipient = columnSelectRecipient.element() selectionRecipient = columnSelectRecipient.element()
elementTemplate = 'notification'
if notificationType is 'webhook' if notificationType is 'webhook'
elementTemplate = 'webhook' notificationElement = $( App.view('generic/ticket_perform_action/webhook')(
attribute: attribute
name: name
notificationType: notificationType
meta: meta || {}
))
notificationElement = $( App.view("generic/ticket_perform_action/#{elementTemplate}")( notificationElement.find('.js-recipient select').replaceWith(selectionRecipient)
webhookSelection = App.UiElement.select.render(
name: "#{name}::webhook_id"
multiple: false
null: false
relation: 'Webhook'
value: meta.webhook_id
translate: false
)
notificationElement.find('.js-webhooks').html(webhookSelection)
else
notificationElement = $( App.view('generic/ticket_perform_action/notification')(
attribute: attribute attribute: attribute
name: name name: name
notificationType: notificationType notificationType: notificationType

View file

@ -0,0 +1,41 @@
class Index extends App.ControllerSubContent
requiredPermission: 'admin.webhook'
header: 'Webhooks'
constructor: ->
super
@genericController = new App.ControllerGenericIndex(
el: @el
id: @id
genericObject: 'Webhook'
defaultSortBy: 'name'
pageData:
home: 'webhooks'
object: 'Webhook'
objects: 'Webhooks'
pagerAjax: true
pagerBaseUrl: '#manage/webhook/'
pagerSelected: ( @page || 1 )
pagerPerPage: 150
navupdate: '#webhooks'
notes: [
'Webhooks are ...'
]
buttons: [
{ name: 'Example Payload', 'data-type': 'payload', class: 'btn' }
{ name: 'New Webhook', 'data-type': 'new', class: 'btn--success' }
]
logFacility: 'webhook'
payloadExampleUrl: '/api/v1/webhooks/preview'
container: @el.closest('.content')
veryLarge: true
)
show: (params) =>
for key, value of params
if key isnt 'el' && key isnt 'shown' && key isnt 'match'
@[key] = value
@genericController.paginate( @page || 1 )
App.Config.set('Webhook', { prio: 3350, name: 'Webhook', parent: '#manage', target: '#manage/webhook', controller: Index, permission: ['admin.webhook'] }, 'NavBarAdmin')

View file

@ -0,0 +1,37 @@
class App.WidgetPayloadExample extends App.ControllerModal
buttonClose: true
buttonCancel: true
buttonSubmit: false
head: 'Example Payload'
large: true
content: =>
if !@payloadExample
@load()
return
@payloadExample
load: =>
@ajax(
id: 'example_payload'
type: 'get'
url: @baseUrl
processData: false
contentType: 'text/plain'
dataType: 'text'
cache: false
success: (data, status, xhr) =>
@payloadExample = $(App.view('widget/payload_example')(
payload: data
))
@update()
error: (data) =>
details = data.responseJSON || {}
@notify
type: 'error'
msg: App.i18n.translateContent(details.error_human || details.error || 'Unable to load example payload!')
timeout: 6000
)

View file

@ -0,0 +1,29 @@
class App.Webhook extends App.Model
@configure 'Webhook', 'name', 'endpoint', 'signature_token', 'ssl_verify', 'note', 'active'
@extend Spine.Model.Ajax
@url: @apiPath + '/webhooks'
@configure_attributes = [
{ name: 'name', display: 'Name', tag: 'input', type: 'text', limit: 100, null: false },
{ name: 'endpoint', display: 'Endpoint', tag: 'input', type: 'text', limit: 300, null: false, placeholder: 'https://target.example.com/webhook' },
{ name: 'signature_token', display: 'HMAC SHA1 Signature Token', tag: 'input', type: 'text', limit: 100, null: true },
{ name: 'ssl_verify', display: 'SSL Verify', tag: 'boolean', null: true, options: { true: 'yes', false: 'no' }, default: true },
{ name: 'note', display: 'Note', tag: 'textarea', note: '', limit: 250, null: true },
{ name: 'active', display: 'Active', tag: 'active', default: true },
{ name: 'updated_at', display: 'Updated', tag: 'datetime', readonly: 1 },
]
@configure_delete = true
@configure_clone = true
@configure_overview = [
'name',
'endpoint',
]
@description = '''
Webhooks make it easy to send information about events within Zammad to third party systems via HTTP(S).
You can use Webhooks in Zammad to send Ticket, Article and Attachment data whenever a Trigger is performed. Just create and configure your Webhook with an HTTP(S) endpoint and relevant security settings, configure a Trigger to perform it.
'''
displayName: ->
return @name if !@endpoint
"#{@name} (#{@endpoint})"

View file

@ -17,3 +17,5 @@
<div class="page-content"> <div class="page-content">
<div class="table-overview"></div> <div class="table-overview"></div>
</div> </div>
<div class="page-footer"></div>

View file

@ -1,24 +1,6 @@
<div class="form-group"> <div class="form-group">
<div class="formGroup-label"> <div class="formGroup-label">
<label><%- @T('Endpoint') %></label> <label><%- @T('Webhooks') %></label>
</div>
<div class="controls">
<input type="url" name="<%= @name %>::endpoint" value="<%= @meta.endpoint %>" class="form-control" style="width: 100%;" placeholder="https://target.example.com/webhook">
</div>
</div>
<div class="form-group">
<div class="formGroup-label">
<label><%- @T('%s Signature Token', 'HMAC-SHA1')%></label>
</div>
<div class="controls">
<input type="text" name="<%= @name %>::token" value="<%= @meta.token %>" class="form-control" style="width: 100%;" placeholder="<%- @T('some token') %>">
</div>
</div>
<div class="form-group">
<div class="formGroup-label">
<label><%- @T('Verify SSL')%></label>
</div>
<div class="controls">
<input type="checkbox" name="<%= @name %>::verify_ssl" <% if @meta.verify_ssl: %>checked<% end %>>
</div> </div>
<div class="controls js-webhooks"></div>
</div> </div>

View file

@ -0,0 +1,10 @@
<div>
<%- @T('Header') %>
<pre>
X-Zammad-Trigger: Name of the Trigger
X-Zammad-Delivery: 6d600811-06a3-40af-aebd-a2d8213e85aa
X-Hub-Signature: sha1=06007ef23c38e435f49091cdfa3c770b3d85d7be
</pre>
<%- @T('Body') %>
<pre><%= @payload %></pre>
</div>

View file

@ -0,0 +1,37 @@
# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
class WebhooksController < ApplicationController
prepend_before_action { authentication_check && authorize! }
def preview
access_condition = Ticket.access_condition(current_user, 'read')
ticket = Ticket.where(access_condition).last
render json: JSON.pretty_generate({
ticket: TriggerWebhookJob::RecordPayload.generate(ticket),
article: TriggerWebhookJob::RecordPayload.generate(ticket.articles.last),
}),
status: :ok
end
def index
model_index_render(Webhook, params)
end
def show
model_show_render(Webhook, params)
end
def create
model_create_render(Webhook, params)
end
def update
model_update_render(Webhook, params)
end
def destroy
model_destroy_render(Webhook, params)
end
end

View file

@ -27,6 +27,7 @@ class TriggerWebhookJob < ApplicationJob
@ticket = ticket @ticket = ticket
@article = article @article = article
return if abort?
return if request.success? return if request.success?
raise TriggerWebhookJob::RequestError raise TriggerWebhookJob::RequestError
@ -34,9 +35,42 @@ class TriggerWebhookJob < ApplicationJob
private private
def abort?
if webhook_id.blank?
log_wrong_trigger_config
return true
elsif webhook.blank?
log_not_existing_webhook
return true
end
false
end
def webhook_id
@webhook_id ||= trigger.perform.dig('notification.webhook', 'webhook_id')
end
def webhook
@webhook ||= begin
Webhook.find_by(
id: webhook_id,
active: true
)
end
end
def log_wrong_trigger_config
Rails.logger.error "Can't find webhook_id for Trigger '#{trigger.name}' with ID #{trigger.id}"
end
def log_not_existing_webhook
Rails.logger.error "Can't find Webhook for ID #{webhook_id} configured in Trigger '#{trigger.name}' with ID #{trigger.id}"
end
def request def request
UserAgent.post( UserAgent.post(
config['endpoint'], webhook.endpoint,
payload, payload,
{ {
json: true, json: true,
@ -45,8 +79,8 @@ class TriggerWebhookJob < ApplicationJob
read_timeout: 30, read_timeout: 30,
total_timeout: 60, total_timeout: 60,
headers: headers, headers: headers,
signature_token: config['token'], signature_token: webhook.signature_token,
verify_ssl: verify_ssl?, verify_ssl: webhook.ssl_verify,
log: { log: {
facility: 'webhook', facility: 'webhook',
}, },
@ -54,14 +88,6 @@ class TriggerWebhookJob < ApplicationJob
) )
end end
def config
@config ||= trigger.perform['notification.webhook']
end
def verify_ssl?
config.fetch('verify_ssl', false).present?
end
def headers def headers
{ {
'X-Zammad-Trigger' => trigger.name, 'X-Zammad-Trigger' => trigger.name,

View file

@ -15,7 +15,7 @@ module ChecksPerformValidation
'article.note' => %w[body subject internal], 'article.note' => %w[body subject internal],
'notification.email' => %w[body recipient subject], 'notification.email' => %w[body recipient subject],
'notification.sms' => %w[body recipient], 'notification.sms' => %w[body recipient],
'notification.webhook' => %w[endpoint], 'notification.webhook' => %w[webhook_id],
} }
check_present.each do |key, values| check_present.each do |key, values|

View file

@ -39,6 +39,12 @@ returns
data = calendar.assets(data) data = calendar.assets(data)
end end
app_model_webhook = Webhook.to_app_model
data[ app_model_webhook ] ||= {}
Webhook.find_each do |webhook|
data = webhook.assets(data)
end
app_model_user = User.to_app_model app_model_user = User.to_app_model
data[ app_model_user ] ||= {} data[ app_model_user ] ||= {}

23
app/models/webhook.rb Normal file
View file

@ -0,0 +1,23 @@
# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
class Webhook < ApplicationModel
include ChecksClientNotification
include ChecksLatestChangeObserved
include HasCollectionUpdate
before_create :validate_endpoint
before_update :validate_endpoint
validates :name, presence: true
private
def validate_endpoint
uri = URI.parse(endpoint)
raise Exceptions::UnprocessableEntity, 'Invalid endpoint (no http/https)!' if !uri.is_a?(URI::HTTP)
raise Exceptions::UnprocessableEntity, 'Invalid endpoint (no hostname)!' if uri.host.nil?
rescue URI::InvalidURIError
raise Exceptions::UnprocessableEntity, 'Invalid endpoint!'
end
end

View file

@ -0,0 +1,3 @@
class Controllers::WebhooksControllerPolicy < Controllers::ApplicationControllerPolicy
default_permit!('admin.webhook')
end

12
config/routes/webhook.rb Normal file
View file

@ -0,0 +1,12 @@
Zammad::Application.routes.draw do
api_path = Rails.configuration.api_path
# webhooks
match api_path + '/webhooks/preview', to: 'webhooks#preview', via: :get
match api_path + '/webhooks', to: 'webhooks#index', via: :get
match api_path + '/webhooks/:id', to: 'webhooks#show', via: :get
match api_path + '/webhooks', to: 'webhooks#create', via: :post
match api_path + '/webhooks/:id', to: 'webhooks#update', via: :put
match api_path + '/webhooks/:id', to: 'webhooks#destroy', via: :delete
end

View file

@ -586,6 +586,19 @@ class CreateTicket < ActiveRecord::Migration[4.2]
add_index :karma_activity_logs, %i[o_id object_lookup_id] add_index :karma_activity_logs, %i[o_id object_lookup_id]
add_foreign_key :karma_activity_logs, :users add_foreign_key :karma_activity_logs, :users
add_foreign_key :karma_activity_logs, :karma_activities, column: :activity_id add_foreign_key :karma_activity_logs, :karma_activities, column: :activity_id
create_table :webhooks do |t|
t.column :name, :string, limit: 250, null: false
t.column :endpoint, :string, limit: 300, null: false
t.column :signature_token, :string, limit: 200, null: true
t.column :ssl_verify, :boolean, null: false, default: true
t.column :note, :string, limit: 500, null: true
t.column :active, :boolean, null: false, default: true
t.column :updated_by_id, :integer, null: false
t.column :created_by_id, :integer, null: false
t.timestamps limit: 3, null: false
end
end end
def self.down def self.down
@ -623,5 +636,6 @@ class CreateTicket < ActiveRecord::Migration[4.2]
drop_table :ticket_priorities drop_table :ticket_priorities
drop_table :ticket_states drop_table :ticket_states
drop_table :ticket_state_types drop_table :ticket_state_types
drop_table :webhooks
end end
end end

View file

@ -0,0 +1,58 @@
class Issue3372WebhooksAdminView < ActiveRecord::Migration[5.2]
def up
return if !Setting.exists?(name: 'system_init_done')
create_webhooks_table
record_upgrade
Permission.create_if_not_exists(
name: 'admin.webhook',
note: 'Manage %s',
preferences: {
translations: ['Webhooks']
},
)
end
def create_webhooks_table
create_table :webhooks do |t|
t.column :name, :string, limit: 250, null: false
t.column :endpoint, :string, limit: 300, null: false
t.column :signature_token, :string, limit: 200, null: true
t.column :ssl_verify, :boolean, null: false, default: true
t.column :note, :string, limit: 500, null: true
t.column :active, :boolean, null: false, default: true
t.column :updated_by_id, :integer, null: false
t.column :created_by_id, :integer, null: false
t.timestamps limit: 3, null: false
end
end
def record_upgrade
Trigger.all.find_each do |trigger|
next if trigger.perform.dig('notification.webhook', 'endpoint').blank?
webhook = webhook_create(
source: trigger.name,
config: trigger.perform['notification.webhook'],
)
trigger.perform['notification.webhook'] = { webhook_id: webhook.id }
trigger.save!
end
end
def webhook_create(source:, config:)
Webhook.create!(
name: "Webhook '#{source}'",
endpoint: config['endpoint'],
signature_token: config['token'],
ssl_verify: config['verify_ssl'],
active: true,
created_by_id: 1,
updated_by_id: 1,
)
end
end

View file

@ -262,6 +262,13 @@ Permission.create_if_not_exists(
translations: ['Sessions'] translations: ['Sessions']
}, },
) )
Permission.create_if_not_exists(
name: 'admin.webhook',
note: 'Manage %s',
preferences: {
translations: ['Webhooks']
},
)
Permission.create_if_not_exists( Permission.create_if_not_exists(
name: 'user_preferences', name: 'user_preferences',
note: 'User Preferences', note: 'User Preferences',

View file

@ -0,0 +1,44 @@
require 'rails_helper'
RSpec.describe Issue3372WebhooksAdminView, type: :db_migration do
let(:trigger_webhook_config) do
{
'endpoint' => 'https://example.com/webhook',
'token' => '53Cr3T',
'verify_ssl' => false,
}
end
let(:webhook_attributes) do
{
endpoint: trigger_webhook_config['endpoint'],
signature_token: trigger_webhook_config['token'],
ssl_verify: trigger_webhook_config['verify_ssl'],
}
end
let!(:trigger) do
Trigger.without_callback(:create, :before, :validate_perform) do
create(:trigger, perform: {
'notification.webhook' => trigger_webhook_config
})
end
end
it 'Creates Webhook object from mapped Trigger configuration' do
migrate do |migration|
allow(migration).to receive(:create_webhooks_table)
end
expect(Webhook.last).to have_attributes(**webhook_attributes)
end
it 'Migrates Trigger#perform Webhook configuration to new structure' do
migrate do |migration|
allow(migration).to receive(:create_webhooks_table)
end
expect(trigger.reload.perform['notification.webhook']['webhook_id']).to eq(Webhook.last.id)
end
end

View file

@ -0,0 +1,9 @@
FactoryBot.define do
factory :webhook do
sequence(:name) { |n| "Test webhook #{n}" }
ssl_verify { true }
active { true }
created_by_id { 1 }
updated_by_id { 1 }
end
end

View file

@ -62,13 +62,12 @@ RSpec.describe TriggerWebhookJob, type: :job do
let!(:ticket) { create(:ticket) } let!(:ticket) { create(:ticket) }
let!(:article) { create(:'ticket/article') } let!(:article) { create(:'ticket/article') }
let(:webhook) { create(:webhook, endpoint: endpoint, signature_token: token) }
let(:trigger) do let(:trigger) do
create(:trigger, create(:trigger,
perform: { perform: {
'notification.webhook' => { 'notification.webhook' => { 'webhook_id' => webhook.id }
endpoint: endpoint,
token: token
}
}) })
end end

View file

@ -502,13 +502,11 @@ RSpec.describe Ticket, type: :model do
end end
context 'with a "notification.webhook" trigger', performs_jobs: true do context 'with a "notification.webhook" trigger', performs_jobs: true do
let(:webhook) { create(:webhook, endpoint: 'http://api.example.com/webhook', signature_token: '53CR3t') }
let(:trigger) do let(:trigger) do
create(:trigger, create(:trigger,
perform: { perform: {
'notification.webhook' => { 'notification.webhook' => { 'webhook_id' => webhook.id }
endpoint: 'http://api.example.com/webhook',
token: '53CR3t'
}
}) })
end end

View file

@ -870,6 +870,7 @@ RSpec.describe User, type: :model do
'Channel' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, 'Channel' => { 'created_by_id' => 0, 'updated_by_id' => 0 },
'Role' => { 'created_by_id' => 0, 'updated_by_id' => 0 }, 'Role' => { 'created_by_id' => 0, 'updated_by_id' => 0 },
'History' => { 'created_by_id' => 1 }, 'History' => { 'created_by_id' => 1 },
'Webhook' => { 'created_by_id' => 0, 'updated_by_id' => 0 },
'Overview' => { 'created_by_id' => 1, 'updated_by_id' => 0 }, 'Overview' => { 'created_by_id' => 1, 'updated_by_id' => 0 },
'ActivityStream' => { 'created_by_id' => 0 }, 'ActivityStream' => { 'created_by_id' => 0 },
'StatsStore' => { 'created_by_id' => 0 }, 'StatsStore' => { 'created_by_id' => 0 },

View file

@ -0,0 +1,41 @@
require 'rails_helper'
RSpec.describe Webhook, type: :model do
describe 'check endpoint' do
subject(:webhook) { create(:webhook, endpoint: endpoint) }
let(:endpoint) { 'example.com' }
context 'with missing http type' do
it 'raise an error' do
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint (no http/https)!')
end
end
context 'with spaces in invalid hostname' do
let(:endpoint) { 'http:// example.com' }
it 'raise an error' do
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint!')
end
end
context 'with ? in hostname' do
let(:endpoint) { 'http://?example.com' }
it 'raise an error' do
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint (no hostname)!')
end
end
context 'with nil in endpoint' do
let(:endpoint) { nil }
it 'raise an error' do
expect { webhook }.to raise_error(Exceptions::UnprocessableEntity, 'Invalid endpoint!')
end
end
end
end