From d90d192f48b45bbfd4e3c9492372875867142a69 Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Tue, 7 Oct 2025 06:49:14 +0000 Subject: [PATCH] Introduces issue webhooks (#29664): * users can set up hooks for issue creation, update and deletion events, for any number of projects * hooks run in the context of the creating user, and only if the object in question is visible to that user * the actual HTTP call is done in ActiveJob * webhook calls are optionally signed the same way GitHub does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch by Jens Krämer (user:jkraemer). git-svn-id: https://svn.redmine.org/redmine/trunk@24034 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- Gemfile | 3 + app/assets/images/icons.svg | 5 + app/controllers/webhooks_controller.rb | 59 ++++++ app/jobs/webhook_job.rb | 16 ++ app/models/issue.rb | 4 + app/models/project.rb | 2 + app/models/user.rb | 2 + app/models/webhook.rb | 111 +++++++++++ app/models/webhook_payload.rb | 101 ++++++++++ app/views/webhooks/_form.html.erb | 44 +++++ app/views/webhooks/edit.html.erb | 6 + app/views/webhooks/index.html.erb | 35 ++++ app/views/webhooks/new.html.erb | 7 + config/configuration.yml.example | 13 ++ config/icon_source.yml | 4 +- config/locales/de.yml | 11 ++ config/locales/en.yml | 11 ++ config/routes.rb | 2 + db/migrate/20251007073256_create_webhooks.rb | 17 ++ lib/webhook_endpoint_validator.rb | 172 ++++++++++++++++++ test/functional/webhooks_controller_test.rb | 73 ++++++++ .../lib/webhook_endpoint_validator_test.rb | 82 +++++++++ test/unit/webhook_payload_test.rb | 40 ++++ test/unit/webhook_test.rb | 167 +++++++++++++++++ 24 files changed, 986 insertions(+), 1 deletion(-) create mode 100644 app/controllers/webhooks_controller.rb create mode 100644 app/jobs/webhook_job.rb create mode 100644 app/models/webhook.rb create mode 100644 app/models/webhook_payload.rb create mode 100644 app/views/webhooks/_form.html.erb create mode 100644 app/views/webhooks/edit.html.erb create mode 100644 app/views/webhooks/index.html.erb create mode 100644 app/views/webhooks/new.html.erb create mode 100644 db/migrate/20251007073256_create_webhooks.rb create mode 100644 lib/webhook_endpoint_validator.rb create mode 100644 test/functional/webhooks_controller_test.rb create mode 100644 test/unit/lib/webhook_endpoint_validator_test.rb create mode 100644 test/unit/webhook_payload_test.rb create mode 100644 test/unit/webhook_test.rb diff --git a/Gemfile b/Gemfile index 53c501c4b..e42693f30 100644 --- a/Gemfile +++ b/Gemfile @@ -41,6 +41,9 @@ gem 'rqrcode' gem "html-pipeline", "~> 2.13.2" gem "sanitize", "~> 6.0" +# Triggering of Webhooks +gem "rest-client", "~> 2.1" + # Optional gem for LDAP authentication group :ldap do gem 'net-ldap', '~> 0.17.0' diff --git a/app/assets/images/icons.svg b/app/assets/images/icons.svg index f100919e3..62384e3f1 100644 --- a/app/assets/images/icons.svg +++ b/app/assets/images/icons.svg @@ -547,6 +547,11 @@ + + + + + diff --git a/app/controllers/webhooks_controller.rb b/app/controllers/webhooks_controller.rb new file mode 100644 index 000000000..d5eb98581 --- /dev/null +++ b/app/controllers/webhooks_controller.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +class WebhooksController < ApplicationController + self.main_menu = false + + before_action :require_login + before_action :find_webhook, only: [:edit, :update, :destroy] + + require_sudo_mode :create, :update, :destroy + + def index + @webhooks = webhooks.order(:url) + end + + def new + @webhook = Webhook.new + end + + def edit + end + + def create + @webhook = webhooks.build(webhook_params) + if @webhook.save + redirect_to webhooks_path + else + render :new + end + end + + def update + if @webhook.update(webhook_params) + redirect_to webhooks_path + else + render :edit + end + end + + def destroy + @webhook.destroy + redirect_to webhooks_path + end + + private + + def webhook_params + params.require(:webhook).permit(:url, :secret, :active, events: [], project_ids: []) + end + + def find_webhook + @webhook = webhooks.find(params[:id]) + rescue ActiveRecord::RecordNotFound + render_404 + end + + def webhooks + User.current.webhooks + end +end diff --git a/app/jobs/webhook_job.rb b/app/jobs/webhook_job.rb new file mode 100644 index 000000000..a3be1a4ce --- /dev/null +++ b/app/jobs/webhook_job.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class WebhookJob < ApplicationJob + def perform(hook_id, payload_json) + if hook = Webhook.find_by_id(hook_id) + if hook.user&.active? + User.current = hook.user + hook.call payload_json + else + Rails.logger.debug { "WebhookJob: user with id=#{hook.user_id} is not active" } + end + else + Rails.logger.debug { "WebhookJob: couldn't find hook with id=#{hook_id}" } + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index 34f02b300..ee317b006 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -130,6 +130,10 @@ class Issue < ApplicationRecord after_create_commit :add_auto_watcher after_commit :create_parent_issue_journal + after_create_commit ->{ Webhook.trigger('issue.created', self) } + after_update_commit ->{ Webhook.trigger('issue.updated', self) } + after_destroy_commit ->{ Webhook.trigger('issue.deleted', self) } + # Returns a SQL conditions string used to find all issues visible by the specified user def self.visible_condition(user, options={}) Project.allowed_to_condition(user, :view_issues, options) do |role, user| diff --git a/app/models/project.rb b/app/models/project.rb index 89a83b202..e13f55a55 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -60,6 +60,8 @@ class Project < ApplicationRecord :class_name => 'IssueCustomField', :join_table => "#{table_name_prefix}custom_fields_projects#{table_name_suffix}", :association_foreign_key => 'custom_field_id' + has_and_belongs_to_many :webhooks + # Default Custom Query belongs_to :default_issue_query, :class_name => 'IssueQuery' diff --git a/app/models/user.rb b/app/models/user.rb index 496084ceb..f41f654e1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -102,6 +102,8 @@ class User < Principal has_one :api_token, lambda {where "#{table.name}.action='api'"}, :class_name => 'Token' has_many :email_addresses, :dependent => :delete_all has_many :reactions, dependent: :delete_all + has_many :webhooks, dependent: :destroy + belongs_to :auth_source scope :logged, lambda {where("#{User.table_name}.status <> #{STATUS_ANONYMOUS}")} diff --git a/app/models/webhook.rb b/app/models/webhook.rb new file mode 100644 index 000000000..b36abdd4c --- /dev/null +++ b/app/models/webhook.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'rest-client' + +class Webhook < ApplicationRecord + Executor = Struct.new(:url, :payload, :secret) do + # @return [RestClient::Response] if the POST request was successful + # @raise [RestClient::Exception, Exception] a `RestClient::Exception` if an + # unexpected (i.e. non-successful) response status was set; it may contain + # the server response. For connection errors, we may raise any other + # exception. + def call + # DNS and therefore destination IPs might have changed since the record was saved, so check the URL, again. + raise URI::BadURIError unless WebhookEndpointValidator.safe_webhook_uri?(url) + + headers = { accept: '*/*', content_type: :json, user_agent: 'Redmine' } + if secret.present? + headers['X-Redmine-Signature-256'] = compute_signature + end + Rails.logger.debug { "Webhook: POST #{url}" } + RestClient.post url, payload, headers + end + + # Computes the HMAC signature for the given payload and secret. + # https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries + def compute_signature + 'sha256=' + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha256'), secret, payload) + end + end + + belongs_to :user + has_and_belongs_to_many :projects + + validates :url, presence: true, webhook_endpoint: true, length: { maximum: 2000 } + validates :secret, length: { maximum: 255 }, allow_blank: true + validate :check_events_array + + serialize :events, coder: YAML, type: Array + + scope :active, -> { where(active: true) } + + before_validation ->(hook){ hook.projects = hook.projects.to_a.select{|p| p.visible?(hook.user) } } + + # Triggers the given event for the given object, scheduling qualifying hooks + # to be called. + def self.trigger(event, object) + hooks_for(event, object).each do |hook| + payload = hook.payload(event, object) + WebhookJob.perform_later(hook.id, payload.to_json) + end + end + + # Finds hooks for the given event and object. + # Returns an array of hooks that are active, have the given event in their list + # of events, and whose user can see the object. + # + # Object must have a project_id and respond to visible?(user) + def self.hooks_for(event, object) + Webhook.active + .joins("INNER JOIN projects_webhooks on projects_webhooks.webhook_id = webhooks.id") + .eager_load(:user) + .where(users: { status: User::STATUS_ACTIVE }, projects_webhooks: { project_id: object.project_id }) + .to_a.select do |hook| + hook.events.include?(event) && object.visible?(hook.user) + end + end + + def setable_projects + Project.visible + end + + def setable_events + WebhookPayload::EVENTS + end + + def setable_event_names + setable_events.map{|type, actions| actions.map{|action| "#{type}.#{action}"}}.flatten + end + + # computes the payload. this happens when the hook is triggered, and the + # payload is stored as part of the hook job definition. + # event must be of the form 'type.action' (like 'issue.created') + def payload(event, object) + WebhookPayload.new(event, object, user).to_h + end + + # POSTs the given payload to the hook URL, returns true if successful, false otherwise. + # + # logs any unsuccessful hook calls, but does not raise + def call(payload_json) + Executor.new(url, payload_json, secret).call + true + rescue => e + Rails.logger.warn { "Webhook Error: #{e.message} (#{e.class})\n#{e.backtrace.join "\n"}" } + false + end + + private + + def check_events_array + unless events.is_a?(Array) + errors.add(:events, :invalid) + return + end + + events.reject!(&:blank?) + if (events - setable_event_names).any? + errors.add(:events, :invalid) + end + end +end diff --git a/app/models/webhook_payload.rb b/app/models/webhook_payload.rb new file mode 100644 index 000000000..fd0af9171 --- /dev/null +++ b/app/models/webhook_payload.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +# Webhook payload +class WebhookPayload + attr_accessor :event, :object, :user + + def initialize(event, object, user) + self.event = event + self.object = object + self.user = user + end + + EVENTS = { + issue: %w[created updated deleted] + } + + def to_h + type, action = event.split('.') + if EVENTS[type.to_sym].include?(action) + send("#{type}_payload", action) + else + raise ArgumentError, "invalid event: #{event}" + end + end + + private + + def issue_payload(action) + issue = object + if issue.current_journal.present? + journal = issue.journals.visible(user).find_by_id(issue.current_journal.id) + end + ts = case action + when 'created' + issue.created_on + when 'deleted' + Time.now + else + journal&.created_on || issue.updated_on + end + h = { + type: event, + timestamp: ts.iso8601, + data: { + issue: ApiRenderer.new("app/views/issues/show.api.rsb", user).to_h(issue: issue) + } + } + if action == 'updated' && journal.present? + h[:data][:journal] = journal_payload(journal) + end + h + end + + def journal_payload(journal) + { + id: journal.id, + created_on: journal.created_on.iso8601, + notes: journal.notes, + user: { + id: journal.user.id, + name: journal.user.name, + }, + details: journal.visible_details(user).map do |d| + { + property: d.property, + prop_key: d.prop_key, + old_value: d.old_value, + value: d.value, + } + end + } + end + + # given a path to an API template (relative to RAILS_ROOT), renders it and returns the resulting hash + class ApiRenderer + include ApplicationHelper + include CustomFieldsHelper + attr_accessor :path, :params, :user + + DummyRequest = Struct.new(:params) + + def initialize(path, user, params = nil) + self.path = path + self.user = user + self.params = params || {} + end + + def to_h(**ivars) + req = DummyRequest.new(params) + api = Redmine::Views::Builders::Json.new(req, nil) + ivars.each { |k, v| instance_variable_set :"@#{k}", v } + original_user = User.current + begin + User.current = self.user + instance_eval(File.read(Rails.root.join(path)), path, 1) + ensure + User.current = original_user + end + end + end +end diff --git a/app/views/webhooks/_form.html.erb b/app/views/webhooks/_form.html.erb new file mode 100644 index 000000000..ab927f7f9 --- /dev/null +++ b/app/views/webhooks/_form.html.erb @@ -0,0 +1,44 @@ +<%= error_messages_for @webhook %> + +
+
+
+

<%= f.text_field :url, required: true, size: 60 %> + <%= l :webhook_url_info %> +

+

+ <%= f.text_field :secret %> + <%= raw l :webhook_secret_info_html %> +

+

<%= f.check_box :active %>

+
+ +

<%= l :label_webhook_events %>

+
+ <% @webhook.setable_events.keys.sort.each do |type| %> +
<%= toggle_checkboxes_link("##{type}_events\ input") %><%= l_or_humanize(type, prefix: 'webhook_events_') %> + <% @webhook.setable_events[type].each do |action| %> + <% name = "#{type}.#{action}" %> + + <% end %> +
+ <% end %> +
<%= check_all_links 'events' %> + <%= hidden_field_tag 'webhook[events][]', '' %> +
+
+ +
+
<%= toggle_checkboxes_link("#webhook_project_ids input[type=checkbox]") %><%= l(:label_project_plural) %> + <% project_ids = @webhook.project_ids.to_a %> + <%= render_project_nested_lists(@webhook.setable_projects) do |p| + content_tag('label', check_box_tag('webhook[project_ids][]', p.id, project_ids.include?(p.id), :id => nil) + ' ' + h(p)) + end %> + <%= hidden_field_tag('webhook[project_ids][]', '', :id => nil) %> +
+ +
+
diff --git a/app/views/webhooks/edit.html.erb b/app/views/webhooks/edit.html.erb new file mode 100644 index 000000000..4afb7b0c3 --- /dev/null +++ b/app/views/webhooks/edit.html.erb @@ -0,0 +1,6 @@ +

<%= l :label_webhook_edit %>

+ +<%= labelled_form_for @webhook, html: { method: :patch } do |f| %> + <%= render :partial => 'form', :locals => { :f => f } %> + <%= submit_tag l(:button_save) %> +<% end %> diff --git a/app/views/webhooks/index.html.erb b/app/views/webhooks/index.html.erb new file mode 100644 index 000000000..9da188a9c --- /dev/null +++ b/app/views/webhooks/index.html.erb @@ -0,0 +1,35 @@ +
+ <%= link_to sprite_icon('add', l(:label_webhook_new)), new_webhook_path, class: 'icon icon-add' %> +
+ +<%= title l :label_webhook_plural %> + +<% if @webhooks.any? %> +
+ + + + + + + + + + <% @webhooks.each do |webhook| %> + "> + + + + + + + <% end %> + +
<%= l :field_active %><%= l :label_url %><%= l :label_webhook_events %><%= l :label_project_plural %>
<%= webhook.active ? l(:general_text_Yes) : l(:general_text_No) %><%= truncate webhook.url, length: 40 %><%= safe_join webhook.events.map{|e| content_tag :code, e }, ', ' %><%= safe_join webhook.projects.visible.map{|p| link_to_project(p) }, ', ' %> + <%= link_to sprite_icon('edit', l(:button_edit)), edit_webhook_path(webhook), class: 'icon icon-edit' %> + <%= link_to sprite_icon('del', l(:button_delete)), webhook_path(webhook), :data => {:confirm => l(:text_are_you_sure)}, :method => :delete, :class => 'icon icon-del' %> +
+
+<% else %> +

<%= l(:label_no_data) %>

+<% end %> diff --git a/app/views/webhooks/new.html.erb b/app/views/webhooks/new.html.erb new file mode 100644 index 000000000..50359f320 --- /dev/null +++ b/app/views/webhooks/new.html.erb @@ -0,0 +1,7 @@ +

<%= l :label_webhook_new %>

+ +<%= labelled_form_for @webhook, url: webhooks_path do |f| %> + <%= render :partial => 'webhooks/form', locals: { f: f } %> + <%= submit_tag l(:button_create) %> + <%= link_to l(:button_cancel), webhooks_path %> +<% end %> diff --git a/config/configuration.yml.example b/config/configuration.yml.example index c528d764e..4e8444ca9 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -224,6 +224,19 @@ default: # false: switches to default common mark where two or more spaces are required # common_mark_enable_hardbreaks: true + # Webhooks + # + # An optional list of hosts and/or IP addresses and/or IP networks which + # should NOT be valid as webhook targets. You can add your internal IPs and + # hostnames here to avoid possible SSRF attacks. + # webhook_blocklist: + # - 10.0.0.0/8 + # - 172.16.0.0/12 + # - 192.168.0.0/16 + # - fc00::/7 + # - example.org + # - "*.example.com" + # specific configuration options for production environment # that overrides the default ones production: diff --git a/config/icon_source.yml b/config/icon_source.yml index 1e4c2ae09..c3edb9beb 100644 --- a/config/icon_source.yml +++ b/config/icon_source.yml @@ -241,4 +241,6 @@ - name: loader svg: loader-2 - name: hourglass - svg: hourglass \ No newline at end of file + svg: hourglass +- name: webhook + svg: webhook \ No newline at end of file diff --git a/config/locales/de.yml b/config/locales/de.yml index c4ea072d6..7aeae1768 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -845,6 +845,17 @@ de: label_yesterday: gestern label_default_query: Standardabfrage label_progressbar: Fortschrittsbalken + label_webhook_plural: Webhooks + label_webhook_new: Neuer Webhook + label_webhook_edit: Webhook bearbeiten + label_webhook_events: Ereignisse + + webhook_events_issue: Aufgaben + webhook_events_issue_created: Aufgabe angelegt + webhook_events_issue_updated: Aufgabe bearbeitet + webhook_events_issue_deleted: Aufgabe gelöscht + webhook_url_info: Redmine sendet einen POST-Request an diese URL, wenn eines der gewählten Ereignisse in einem der ausgewählten Projekte eintritt. + webhook_secret_info_html: Wenn gesetzt, wird Redmine mit jedem Request eine Hash-Signatur im X-Redmine-Signature-256 header übermitteln, die zur Authentifizierung des Requests herangezogen werden kann. mail_body_account_activation_request: "Ein neuer Benutzer (%{value}) hat sich registriert. Sein Konto wartet auf Ihre Genehmigung:" mail_body_account_information: Ihre Konto-Informationen diff --git a/config/locales/en.yml b/config/locales/en.yml index f67d2d2f3..5385ad02d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1171,6 +1171,17 @@ en: label_oauth_admin_access: Administration label_oauth_application_plural: Applications label_oauth_authorized_application_plural: Authorized applications + label_webhook_plural: Webhooks + label_webhook_new: New webhook + label_webhook_edit: Edit webhook + label_webhook_events: Events + + webhook_events_issue: Issues + webhook_events_issue_created: Issue created + webhook_events_issue_updated: Issue updated + webhook_events_issue_deleted: Issue deleted + webhook_url_info: Redmine will send a POST request to this URL whenever one of the selected events occurs in one of the selected projects. + webhook_secret_info_html: If provided, Redmine will use this to create a hash signature that is sent with each delivery as the value of the X-Redmine-Signature-256 header. button_login: Login button_submit: Submit diff --git a/config/routes.rb b/config/routes.rb index 7a683c938..df48e19e9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -35,6 +35,8 @@ Rails.application.routes.draw do match 'account/activate', :to => 'account#activate', :via => :get get 'account/activation_email', :to => 'account#activation_email', :as => 'activation_email' + resources :webhooks, only: [:index, :new, :create, :edit, :update, :destroy] + match '/news/preview', :controller => 'previews', :action => 'news', :as => 'preview_news', :via => [:get, :post, :put, :patch] match '/issues/preview', :to => 'previews#issue', :as => 'preview_issue', :via => [:get, :post, :put, :patch] match '/preview/text', :to => 'previews#text', :as => 'preview_text', :via => [:get, :post, :put, :patch] diff --git a/db/migrate/20251007073256_create_webhooks.rb b/db/migrate/20251007073256_create_webhooks.rb new file mode 100644 index 000000000..07bed0684 --- /dev/null +++ b/db/migrate/20251007073256_create_webhooks.rb @@ -0,0 +1,17 @@ +class CreateWebhooks < ActiveRecord::Migration[8.0] + def change + create_table :webhooks do |t| + t.string :url, null: false, limit: 2000 + t.string :secret + t.text :events + t.integer :user_id, null: false, index: true + t.boolean :active, null: false, default: false, index: true + t.timestamps + end + + create_table :projects_webhooks do |t| + t.integer :project_id, null: false, index: true + t.integer :webhook_id, null: false, index: true + end + end +end diff --git a/lib/webhook_endpoint_validator.rb b/lib/webhook_endpoint_validator.rb new file mode 100644 index 000000000..860a0af6a --- /dev/null +++ b/lib/webhook_endpoint_validator.rb @@ -0,0 +1,172 @@ +# frozen_string_literal: true + +require 'uri' + +class WebhookEndpointValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + return if value.blank? + + unless self.class.safe_webhook_uri?(value) + record.errors.add attribute, :invalid + end + end + + def self.safe_webhook_uri?(value) + uri = value.is_a?(URI) ? value : URI.parse(value) + return false if uri.nil? + + return false unless valid_scheme?(uri.scheme) + return false unless valid_host?(uri.host) + return false unless valid_port?(uri.port) + + true + rescue + Rails.logger.warn { "URI failed webhook safety checks: #{uri}" } + false + end + + def self.valid_port?(port) + !BAD_PORTS.include?(port) + end + + def self.valid_scheme?(scheme) + %w[http https].include?(scheme) + end + + def self.blocked_hosts + @blocked_hosts ||= begin + ips = [] + wildcards = [] + hosts = [] + + Array(Redmine::Configuration['webhook_blocklist']).map(&:to_s).each do |block| + # We try to parse the block as an IP address first... + ips << IPAddr.new(block) + rescue IPAddr::Error + # If that failed, we assume it is a (wildcard) hostname + if block.start_with?('*.') + wildcards << Regexp.escape(block[2..]) + else + hosts << Regexp.escape(block) + end + end + + regex_parts = [] + regex_parts << "(?:#{hosts.join('|')})" if hosts.any? + regex_parts << "(?:.*\\.)?(?:#{wildcards.join('|')})" if wildcards.any? + + { + ips: ips.freeze, + host: regex_parts.any? ? /\A(?:#{regex_parts.join('|')})\z/i : nil + }.freeze + end + end + + def self.valid_host?(host) + return false if host.blank? + + return false if blocked_hosts[:host]&.match?(host) + + Resolv.each_address(host) do |ip| + ipaddr = IPAddr.new(ip) + return false if ipaddr.link_local? || ipaddr.loopback? + return false if IPAddr.new('224.0.0.0/24').include?(ipaddr) # multicast + return false if blocked_hosts[:ips].any? { |net| net.include?(ipaddr) } + end + + true + end + + # A general port blacklist. Connections to these ports will not be allowed + # unless the protocol overrides. + # + # This list is to be kept in sync with "bad ports" as defined in the + # WHATWG Fetch standard at https://fetch.spec.whatwg.org/#port-blocking + # + # see also: https://github.com/mozilla/gecko-dev/blob/d55e89d48a8053ce45a74b0ec92c0ff6a9dcc43d/netwerk/base/nsIOService.cpp#L109-L199 + # + BAD_PORTS = Set[ + 1, # tcpmux + 7, # echo + 9, # discard + 11, # systat + 13, # daytime + 15, # netstat + 17, # qotd + 19, # chargen + 20, # ftp-data + 21, # ftp + 22, # ssh + 23, # telnet + 25, # smtp + 37, # time + 42, # name + 43, # nicname + 53, # domain + 69, # tftp + 77, # priv-rjs + 79, # finger + 87, # ttylink + 95, # supdup + 101, # hostriame + 102, # iso-tsap + 103, # gppitnp + 104, # acr-nema + 109, # pop2 + 110, # pop3 + 111, # sunrpc + 113, # auth + 115, # sftp + 117, # uucp-path + 119, # nntp + 123, # ntp + 135, # loc-srv / epmap + 137, # netbios + 139, # netbios + 143, # imap2 + 161, # snmp + 179, # bgp + 389, # ldap + 427, # afp (alternate) + 465, # smtp (alternate) + 512, # print / exec + 513, # login + 514, # shell + 515, # printer + 526, # tempo + 530, # courier + 531, # chat + 532, # netnews + 540, # uucp + 548, # afp + 554, # rtsp + 556, # remotefs + 563, # nntp+ssl + 587, # smtp (outgoing) + 601, # syslog-conn + 636, # ldap+ssl + 989, # ftps-data + 990, # ftps + 993, # imap+ssl + 995, # pop3+ssl + 1719, # h323gatestat + 1720, # h323hostcall + 1723, # pptp + 2049, # nfs + 3659, # apple-sasl + 4045, # lockd + 4190, # sieve + 5060, # sip + 5061, # sips + 6000, # x11 + 6566, # sane-port + 6665, # irc (alternate) + 6666, # irc (alternate) + 6667, # irc (default) + 6668, # irc (alternate) + 6669, # irc (alternate) + 6679, # osaut + 6697, # irc+tls + 10080 # amanda + ].freeze +end diff --git a/test/functional/webhooks_controller_test.rb b/test/functional/webhooks_controller_test.rb new file mode 100644 index 000000000..418eb76d6 --- /dev/null +++ b/test/functional/webhooks_controller_test.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'test_helper' + +class WebhooksControllerTest < Redmine::ControllerTest + fixtures :projects, :users, :email_addresses, :user_preferences, :members, :member_roles, :roles, + :groups_users, + :trackers, :projects_trackers, + :enabled_modules, + :versions, + :issue_statuses, :issue_categories, :issue_relations, :workflows, + :enumerations, + :issues, :journals, :journal_details + + setup do + @project = Project.find 'ecookbook' + @dlopper = User.find_by_login 'dlopper' + @issue = @project.issues.first + @hook = create_hook + @other_hook = create_hook user: User.find_by_login('admin'), url: 'https://example.com/other/hook' + @request.session[:user_id] = @dlopper.id + end + + test "should require login" do + @request.session[:user_id] = nil + get :index + assert_redirected_to '/login?back_url=http%3A%2F%2Ftest.host%2Fwebhooks' + end + + test "should get index" do + get :index + assert_response :success + assert_select 'td', text: @hook.url + assert_select 'td', text: @other_hook.url, count: 0 + end + + test "should get new" do + get :new + assert_response :success + end + + test "should create webhook" do + assert_difference 'Webhook.count' do + post :create, params: { webhook: { url: 'https://example.com/new/hook', events: %w(issue.created), project_ids: [@project.id] } } + end + assert_redirected_to webhooks_path + end + + test "should get edit" do + get :edit, params: { id: @hook.id } + assert_response :success + end + + test "should update webhook" do + patch :update, params: { id: @hook.id, webhook: { url: 'https://example.com/updated/hook' } } + assert_redirected_to webhooks_path + assert_equal 'https://example.com/updated/hook', @hook.reload.url + end + + test 'edit should not find hook of other user' do + get :edit, params: { id: @other_hook.id } + assert_response :not_found + end + + private + + def create_hook(url: 'https://example.com/some/hook', + user: User.find_by_login('dlopper'), + events: %w(issue.created issue.updated), + projects: [Project.find('ecookbook')]) + Webhook.create!(url: url, user: user, events: events, projects: projects) + end +end diff --git a/test/unit/lib/webhook_endpoint_validator_test.rb b/test/unit/lib/webhook_endpoint_validator_test.rb new file mode 100644 index 000000000..426316567 --- /dev/null +++ b/test/unit/lib/webhook_endpoint_validator_test.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'test_helper' + +class WebhookEndpointValidatorTest < ActiveSupport::TestCase + class TestModel + include ActiveModel::Validations + attr_accessor :url + + def initialize(url) + self.url = url + end + + validates :url, webhook_endpoint: true + end + + setup do + WebhookEndpointValidator.class_eval do + @blocked_hosts = nil + end + end + + test "should validate url" do + Redmine::Configuration.with('webhook_blocklist' => ['*.example.org', '10.0.0.0/8', '192.168.0.0/16']) do + %w[ + mailto:user@example.com + foobar + example.com + file://example.com + https://x.example.org/ + http://x.example.org/ + ].each do |url| + assert_not WebhookEndpointValidator.safe_webhook_uri?(url), "#{url} should be invalid" + record = TestModel.new url + assert_not record.valid? + assert record.errors[:url].any? + end + + assert WebhookEndpointValidator.safe_webhook_uri? 'https://acme.com/some/webhook?foo=bar' + record = TestModel.new 'https://acme.com/some/webhook?foo=bar' + assert record.valid?, record.errors.inspect + end + end + + test "should validate ports" do + %w[ + http://example.com:22 + http://example.com:1 + ].each do |url| + assert_not WebhookEndpointValidator.safe_webhook_uri?(url), "#{url} should be invalid" + end + %w[ + http://example.com + http://example.com:80 + http://example.com:443 + http://example.com:8080 + ].each do |url| + assert WebhookEndpointValidator.safe_webhook_uri? url + end + end + + test "should validate ip addresses" do + Redmine::Configuration.with('webhook_blocklist' => ['*.example.org', '10.0.0.0/8', '192.168.0.0/16']) do + %w[ + 127.0.0.0 + 127.0.0.1 + 10.0.0.0 + 10.0.1.0 + 169.254.1.9 + 192.168.2.1 + 224.0.0.1 + ::1/128 + fe80::/10 + ].each do |ip| + assert_not WebhookEndpointValidator.safe_webhook_uri? ip + h = TestModel.new "http://#{ip}" + assert_not h.valid?, "IP #{ip} should be invalid" + assert h.errors[:url].any? + end + end + end +end diff --git a/test/unit/webhook_payload_test.rb b/test/unit/webhook_payload_test.rb new file mode 100644 index 000000000..feaa501df --- /dev/null +++ b/test/unit/webhook_payload_test.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'test_helper' + +class WebhookPayloadTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + fixtures :projects, :users, :trackers, :projects_trackers, :versions, + :issue_statuses, :issue_categories, :issue_relations, + :enumerations, :issues, :journals, :journal_details + + setup do + @dlopper = User.find_by_login 'dlopper' + @project = Project.find 'ecookbook' + @issue = @project.issues.first + end + + test "issue update payload should contain journal" do + @issue.init_journal(@dlopper) + @issue.subject = "new subject" + @issue.save + p = WebhookPayload.new('issue.updated', @issue, @dlopper) + assert h = p.to_h + assert_equal 'issue.updated', h[:type] + assert j = h.dig(:data, :journal) + assert_equal 'Dave Lopper', j[:user][:name] + assert i = h.dig(:data, :issue) + assert_equal 'new subject', i[:subject], i.inspect + end + + test "should compute payload of deleted issue" do + @issue.destroy + p = WebhookPayload.new('issue.deleted', @issue, @dlopper) + assert h = p.to_h + assert_equal 'issue.deleted', h[:type] + assert_nil h.dig(:data, :journal) + assert i = h.dig(:data, :issue) + assert_equal @issue.subject, i[:subject], i.inspect + end +end diff --git a/test/unit/webhook_test.rb b/test/unit/webhook_test.rb new file mode 100644 index 000000000..05c4729a4 --- /dev/null +++ b/test/unit/webhook_test.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'test_helper' +require 'pp' + +class WebhookTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + fixtures :projects, :users, :email_addresses, :user_preferences, :members, :member_roles, :roles, + :groups_users, + :trackers, :projects_trackers, + :enabled_modules, + :versions, + :issue_statuses, :issue_categories, :issue_relations, :workflows, + :enumerations, + :issues, :journals, :journal_details + + setup do + # Set ActiveJob to use the test adapter + @original_adapter = ActiveJob::Base.queue_adapter + ActiveJob::Base.queue_adapter = :test + + @project = Project.find 'ecookbook' + @dlopper = User.find_by_login 'dlopper' + @issue = @project.issues.first + WebhookEndpointValidator.class_eval do + @blocked_hosts = nil + end + end + + teardown do + # Restore the original adapter + ActiveJob::Base.queue_adapter = @original_adapter + end + + test "should validate url" do + Redmine::Configuration.with('webhook_blocklist' => ['*.example.org', '10.0.0.0/8', '192.168.0.0/16']) do + %w[ + mailto:user@example.com + https://x.example.org/ + https://example.org/ + https://x.example.org/foo/bar?a=b + foobar + example.com + https://10.1.0.12/ + ].each do |url| + hook = Webhook.new(url: url) + assert_not hook.valid?, "URL '#{url}' should be invalid" + assert hook.errors[:url].any? + end + end + end + + test "should validate secret length" do + hook = Webhook.new secret: 'abdc' * 100 + assert_not hook.valid? + assert hook.errors[:secret].any? + end + + test "should validate events" do + Webhook.new.setable_event_names.each do |event| + h = create_hook events: [event] + assert h.persisted? + end + hook = Webhook.new(events: ['issue.created', 'invalid.event']) + assert_not hook.valid? + assert hook.errors[:events].any? + assert_raise(ActiveRecord::SerializationTypeMismatch){ Webhook.new(events: 'issue.created') } + end + + test "should clean up project list on save" do + h = create_hook + assert_equal [@project], h.projects + @project.memberships.destroy_all + @project.update is_public: false + + h.reload + h.save + h.reload + assert_equal [], h.projects + end + + test "should check ip address at run time" do + Redmine::Configuration.with('webhook_blocklist' => ['*.example.org', '10.0.0.0/8', '192.168.0.0/16']) do + %w[ + 127.0.0.0 + 127.0.0.1 + 10.0.0.0 + 10.0.1.0 + 169.254.1.9 + 192.168.2.1 + 224.0.0.1 + ::1/128 + fe80::/10 + ].each do |ip| + h = Webhook.new url: "http://#{ip}" + assert_not h.valid?, "IP #{ip} should be invalid" + assert h.errors[:url].any? + end + end + end + + test "should find hooks for issue" do + hook = create_hook + assert @issue.visible?(hook.user) + assert_equal [hook], Webhook.hooks_for('issue.created', @issue) + assert_equal [], Webhook.hooks_for('issue.deleted', @issue) + @issue.update_column :project_id, 99 + assert_equal [], Webhook.hooks_for('issue.created', @issue) + end + + test "should not find inactive hook" do + hook = create_hook active: false + assert @issue.visible?(hook.user) + assert_equal [], Webhook.hooks_for('issue.created', @issue) + end + + test "should not find hook of inactive user" do + admin = User.find_by_login 'admin' + hook = create_hook user: admin + assert_equal [hook], Webhook.hooks_for('issue.created', @issue) + admin.update_column :status, 3 + assert_equal [], Webhook.hooks_for('issue.created', @issue) + end + + test "should find hook for deleted issue" do + hook = create_hook events: ['issue.deleted'] + @issue.destroy + assert_equal [hook], Webhook.hooks_for('issue.deleted', @issue) + end + + test "schedule should enqueue jobs for hooks" do + hook = create_hook + assert_enqueued_jobs 1 do + assert_enqueued_with(job: WebhookJob) do + Webhook.trigger('issue.created', @issue) + end + end + end + + test "should not enqueue job for inactive hook" do + hook = create_hook active: false + assert_no_enqueued_jobs do + Webhook.trigger('issue.created', @issue) + end + end + + test "should compute payload" do + hook = create_hook + payload = hook.payload('issue.created', @issue) + assert_equal 'issue.created', payload[:type] + assert_equal @issue.id, payload.dig(:data, :issue, :id) + end + + test "should compute correct signature" do + # we're implementing the same signature mechanism as GitHub, so might as well re-use their + # example. https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries + e = Webhook::Executor.new('https://example.com', 'Hello, World!', "It's a Secret to Everybody") + assert_equal "sha256=757107ea0eb2509fc211221cce984b8a37570b6d7586c22c46f4379c8b043e17", e.compute_signature + end + + private + + def create_hook(url: 'https://example.com/some/hook', user: User.find_by_login('dlopper'), projects: [Project.find('ecookbook')], events: ['issue.created'], active: true) + Webhook.create!(url: url, user: user, projects: projects, events: events, active: active) + end +end