Allow administrators to disable webhooks from settings (#29664).

Patch by Katsuya HIDAKA (user:hidakatsuya).


git-svn-id: https://svn.redmine.org/redmine/trunk@24073 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/webhooks_controller.rb

@@ -4,6 +4,7 @@ class WebhooksController < ApplicationController
   self.main_menu = false
 
   before_action :require_login
+  before_action :check_enabled
   before_action :authorize
 
   before_action :find_webhook, only: [:edit, :update, :destroy]
@@ -62,4 +63,8 @@ class WebhooksController < ApplicationController
   def authorize
     deny_access unless User.current.allowed_to?(:use_webhooks, nil, global: true)
   end
+
+  def check_enabled
+    render_403 unless Webhook.enabled?
+  end
 end

app/helpers/settings_helper.rb

@@ -25,7 +25,7 @@ module SettingsHelper
         {:name => 'display', :partial => 'settings/display', :label => :label_display},
         {:name => 'authentication', :partial => 'settings/authentication',
          :label => :label_authentication},
-        {:name => 'api', :partial => 'settings/api', :label => :label_api},
+        {:name => 'integrations', :partial => 'settings/api', :label => :label_integrations},
         {:name => 'projects', :partial => 'settings/projects', :label => :label_project_plural},
         {:name => 'users', :partial => 'settings/users', :label => :label_user_plural},
         {:name => 'issues', :partial => 'settings/issues', :label => :label_issue_tracking},

app/models/webhook.rb

@@ -42,9 +42,15 @@ class Webhook < ApplicationRecord
 
   before_validation ->(hook){ hook.projects = hook.projects.to_a & hook.setable_projects }
 
+  def self.enabled?
+    Setting.webhooks_enabled?
+  end
+
   # Triggers the given event for the given object, scheduling qualifying hooks
   # to be called.
   def self.trigger(event, object)
+    return unless enabled?
+
     hooks_for(event, object).each do |hook|
       payload = hook.payload(event, object)
       WebhookJob.perform_later(hook.id, payload.to_json)

app/views/my/account.html.erb

@@ -1,7 +1,7 @@
 <div class="contextual">
 <%= additional_emails_link(@user) %>
 <%= link_to(sprite_icon('key', l(:button_change_password)), { :action => 'password'}, :class => 'icon icon-passwd') if @user.change_password_allowed? %>
-<%= link_to sprite_icon('webhook', l(:label_webhook_plural)), webhooks_path, class: 'icon icon-webhook' if @user.allowed_to?(:use_webhooks, nil, global: true) %>
+<%= link_to sprite_icon('webhook', l(:label_webhook_plural)), webhooks_path, class: 'icon icon-webhook' if Webhook.enabled? && @user.allowed_to?(:use_webhooks, nil, global: true) %>
 <%= link_to(sprite_icon('apps', l('label_oauth_authorized_application_plural')), oauth_authorized_applications_path, :class => 'icon icon-applications') if Setting.rest_api_enabled? %>
 <%= call_hook(:view_my_account_contextual, :user => @user)%>
 </div>

app/views/settings/_api.html.erb

@@ -1,9 +1,11 @@
-<%= form_tag({:action => 'edit', :tab => 'api'}) do %>
+<%= form_tag({:action => 'edit', :tab => 'integrations'}) do %>
 
 <div class="box tabular settings">
 <p><%= setting_check_box :rest_api_enabled %></p>
 
 <p><%= setting_check_box :jsonp_enabled %></p>
+
+<p><%= setting_check_box :webhooks_enabled %></p>
 </div>
 
 <%= submit_tag l(:button_save) %>

config/locales/en.yml

@@ -505,6 +505,7 @@ en:
   setting_thumbnails_size: Thumbnails size (in pixels)
   setting_non_working_week_days: Non-working days
   setting_jsonp_enabled: Enable JSONP support
+  setting_webhooks_enabled: Enable webhooks
   setting_default_projects_tracker_ids: Default trackers for new projects
   setting_mail_handler_excluded_filenames: Exclude attachments by name
   setting_force_default_language_for_anonymous: Force default language for anonymous users
@@ -1176,6 +1177,7 @@ en:
   label_webhook_new: New webhook
   label_webhook_edit: Edit webhook
   label_webhook_events: Events
+  label_integrations: Integrations
 
   webhook_events_issue: Issues
   webhook_events_issue_created: Issue created

config/settings.yml

@@ -336,6 +336,8 @@ rest_api_enabled:
 jsonp_enabled:
   default: 0
   security_notifications: 1
+webhooks_enabled:
+  default: 1
 default_notification_option:
   default: 'only_assigned'
 emails_header:

test/functional/my_controller_test.rb

@@ -398,6 +398,20 @@ class MyControllerTest < Redmine::ControllerTest
     assert_select 'select[name=?]', 'user[language]'
   end
 
+  def test_my_account_should_toggle_webhook_link_with_setting
+    User.find(2).roles.first.add_permission!(:use_webhooks)
+
+    get :account
+    assert_response :success
+    assert_select 'a.icon-webhook', 1
+
+    with_settings webhooks_enabled: '0' do
+      get :account
+      assert_response :success
+      assert_select 'a.icon-webhook', 0
+    end
+  end
+
   def test_my_account_with_avatar_enabled_should_link_to_edit_avatar
     with_settings :gravatar_enabled => '1' do
       Redmine::Configuration.with 'avatar_server_url' => 'https://gravatar.com' do

test/functional/webhooks_controller_test.rb

@@ -27,6 +27,16 @@ class WebhooksControllerTest < Redmine::ControllerTest
     assert_select 'td', text: @other_hook.url, count: 0
   end
 
+  test "should return not found when disabled" do
+    with_settings webhooks_enabled: '0' do
+      get :index
+      assert_response :forbidden
+
+      get :new
+      assert_response :forbidden
+    end
+  end
+
   test "should get new" do
     get :new
     assert_response :success

test/unit/webhook_test.rb

@@ -168,6 +168,28 @@ class WebhookTest < ActiveSupport::TestCase
     end
   end
 
+  test "enabled? should follow setting flag" do
+    assert Webhook.enabled?
+
+    with_settings webhooks_enabled: '0' do
+      assert_not Webhook.enabled?
+    end
+
+    with_settings webhooks_enabled: '1' do
+      assert Webhook.enabled?
+    end
+  end
+
+  test "trigger should not enqueue jobs when disabled" do
+    create_hook
+
+    with_settings webhooks_enabled: '0' do
+      assert_no_enqueued_jobs do
+        Webhook.trigger('issue.created', @issue)
+      end
+    end
+  end
+
   test "should compute payload" do
     hook = create_hook
     payload = hook.payload('issue.created', @issue)