diff --git a/modules/webhooks/app/components/webhooks/outgoing/deliveries/row_component.rb b/modules/webhooks/app/components/webhooks/outgoing/deliveries/row_component.rb index d7c9b674577c..c5d4e1f9b111 100644 --- a/modules/webhooks/app/components/webhooks/outgoing/deliveries/row_component.rb +++ b/modules/webhooks/app/components/webhooks/outgoing/deliveries/row_component.rb @@ -2,7 +2,7 @@ module ::Webhooks module Outgoing module Deliveries class RowComponent < ::RowComponent - property :id, :description, :event_name, :response_code + property :id, :description, :event_name def log model @@ -12,6 +12,14 @@ def time model.updated_at.to_s # Force ISO8601 end + def response_code + if log.response_code <= 0 + I18n.t(:label_none_parentheses) + else + log.response_code + end + end + def response_body render ResponseComponent.new(log) end diff --git a/modules/webhooks/app/models/webhooks/log.rb b/modules/webhooks/app/models/webhooks/log.rb index ca0829ee60ae..beca7e17507a 100644 --- a/modules/webhooks/app/models/webhooks/log.rb +++ b/modules/webhooks/app/models/webhooks/log.rb @@ -9,7 +9,7 @@ class Log < ApplicationRecord serialize :response_headers, type: Hash serialize :request_headers, type: Hash - validates :request_headers, presence: true + validates :request_headers, presence: true, allow_blank: true validates :request_body, presence: true def self.newest(limit: 10) diff --git a/modules/webhooks/app/services/webhooks/outgoing/request_webhook_service.rb b/modules/webhooks/app/services/webhooks/outgoing/request_webhook_service.rb new file mode 100644 index 000000000000..e07ebb81ce67 --- /dev/null +++ b/modules/webhooks/app/services/webhooks/outgoing/request_webhook_service.rb @@ -0,0 +1,60 @@ +module Webhooks + module Outgoing + class RequestWebhookService + include ::OpenProjectErrorHelper + + attr_reader :current_user, :event_name, :webhook + + def initialize(webhook, event_name:, current_user:) + @current_user = current_user + @webhook = webhook + @event_name = event_name + end + + def call!(body:, headers:) + begin + response = Faraday.post( + webhook.url, + body, + headers + ) + rescue Faraday::Error => e + response = e.response + exception = e + rescue StandardError => e + op_handle_error(e.message, reference: :webhook_job) + exception = e + end + + log!(body:, headers:, response:, exception:) + + # We want to re-raise timeout exceptions + # but log the request beforehand + raise exception if exception.is_a?(Faraday::TimeoutError) + end + + def log!(body:, headers:, response:, exception:) + log = ::Webhooks::Log.new( + webhook:, + event_name:, + url: webhook.url, + request_headers: headers, + request_body: body, + **response_attributes(response:, exception:) + ) + + unless log.save + OpenProject.logger.error("Failed to save webhook log: #{log.errors.full_messages.join('. ')}") + end + end + + def response_attributes(response:, exception:) + { + response_code: response&.status || -1, + response_headers: response&.headers&.to_h&.transform_keys { |k| k.underscore.to_sym }, + response_body: response&.body || exception&.message + } + end + end + end +end diff --git a/modules/webhooks/app/workers/represented_webhook_job.rb b/modules/webhooks/app/workers/represented_webhook_job.rb index fc21267fe101..0d4ca6a21163 100644 --- a/modules/webhooks/app/workers/represented_webhook_job.rb +++ b/modules/webhooks/app/workers/represented_webhook_job.rb @@ -27,8 +27,6 @@ #++ class RepresentedWebhookJob < WebhookJob - include ::OpenProjectErrorHelper - attr_reader :resource def perform(webhook_id, resource, event_name) @@ -39,41 +37,14 @@ def perform(webhook_id, resource, event_name) body = request_body headers = request_headers - exception = nil - response = nil - if signature = request_signature(body) + if (signature = request_signature(body)) headers["X-OP-Signature"] = signature end - begin - response = Faraday.post( - webhook.url, - request_body, - headers - ) - rescue Faraday::Error => e - response = e.response - exception = e - rescue StandardError => e - op_handle_error(e.message, reference: :webhook_job) - exception = e - end - - ::Webhooks::Log.create( - webhook:, - event_name:, - url: webhook.url, - request_headers: headers, - request_body: body, - response_code: response&.status, - response_headers: response&.headers&.to_h&.transform_keys { |k| k.underscore.to_sym }, - response_body: response&.body || exception&.message - ) - - # We want to re-raise timeout exceptions - # but log the request beforehand - raise exception if exception.is_a?(Faraday::TimeoutError) + ::Webhooks::Outgoing::RequestWebhookService + .new(webhook, event_name:, current_user: User.system) + .call!(body:, headers:) end def accepted_in_project? @@ -81,7 +52,7 @@ def accepted_in_project? end def request_signature(request_body) - if secret = webhook.secret.presence + if (secret = webhook.secret.presence) "sha1=#{OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), secret, request_body)}" end end diff --git a/modules/webhooks/spec/services/webhooks/outgoing/request_webhook_service_spec.rb b/modules/webhooks/spec/services/webhooks/outgoing/request_webhook_service_spec.rb new file mode 100644 index 000000000000..e12a9c888ebe --- /dev/null +++ b/modules/webhooks/spec/services/webhooks/outgoing/request_webhook_service_spec.rb @@ -0,0 +1,52 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe Webhooks::Outgoing::RequestWebhookService, type: :model do + let(:user) { build_stubbed(:user) } + let(:instance) { described_class.new(webhook, event_name: :created, current_user: user) } + + shared_let(:webhook) { create(:webhook, all_projects: true, url: "https://example.net/test/42", secret: nil) } + + subject { instance.call!(body: "body", headers: {}) } + + describe "#call!" do + context "when request_url fails with SSL errors" do + it "still logs the exception" do + allow(Faraday) + .to receive(:post) + .with(webhook.url, *any_args) + .and_raise(Faraday::SSLError, "SSL error") + + expect { subject } + .to change(Webhooks::Log, :count).by(1) + end + end + end +end