Adds the :use_webhooks permission in order to allow users to use webhooks only in projects where they have this permission. This is checked when a hook is saved, and before a hook runs (#29664).

Patch by Jens Krämer (user:jkraemer).



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

Gemfile

@@ -124,7 +124,6 @@ group :test do
   gem 'bundle-audit', require: false
   # for testing oauth provider capabilities
   gem 'oauth2'
-  gem 'rest-client'
 end
 
 local_gemfile = File.join(File.dirname(__FILE__), "Gemfile.local")

app/controllers/webhooks_controller.rb

@@ -4,6 +4,8 @@ class WebhooksController < ApplicationController
   self.main_menu = false
 
   before_action :require_login
+  before_action :authorize
+
   before_action :find_webhook, only: [:edit, :update, :destroy]
 
   require_sudo_mode :create, :update, :destroy
@@ -56,4 +58,8 @@ class WebhooksController < ApplicationController
   def webhooks
     User.current.webhooks
   end
+
+  def authorize
+    deny_access unless User.current.allowed_to?(:use_webhooks, nil, global: true)
+  end
 end

app/models/webhook.rb

@@ -39,7 +39,7 @@ class Webhook < ApplicationRecord
 
   scope :active, -> { where(active: true) }
 
-  before_validation ->(hook){ hook.projects = hook.projects.to_a.select{|p| p.visible?(hook.user) } }
+  before_validation ->(hook){ hook.projects = hook.projects.to_a & hook.setable_projects }
 
   # Triggers the given event for the given object, scheduling qualifying hooks
   # to be called.
@@ -61,12 +61,13 @@ class Webhook < ApplicationRecord
       .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)
+      hook.events.include?(event) && object.visible?(hook.user) && hook.user.allowed_to?(:use_webhooks, object.project)
     end
   end
 
   def setable_projects
-    Project.visible
+    user = self.user || User.current
+    Project.visible(user).to_a.select{|p| user.allowed_to?(:use_webhooks, p)}
   end
 
   def setable_events

app/views/my/account.html.erb

@@ -1,6 +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('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>

config/locales/de.yml

@@ -970,6 +970,7 @@ de:
   permission_set_issues_private: Tickets als privat oder öffentlich markieren
   permission_set_notes_private: Kommentar als privat markieren
   permission_set_own_issues_private: Eigene Tickets als privat oder öffentlich markieren
+  permission_use_webhooks: Webhooks verwenden
   permission_view_calendar: Kalender ansehen
   permission_view_changesets: Changesets ansehen
   permission_view_documents: Dokumente ansehen

config/locales/en.yml

@@ -611,6 +611,7 @@ en:
   permission_view_project: View projects
   permission_search_project: Search projects
   permission_view_members: View project members
+  permission_use_webhooks: Use webhooks
 
 
   project_module_issue_tracking: Issue tracking

lib/redmine/preparation.rb

@@ -49,6 +49,9 @@ module Redmine
         map.permission :manage_public_queries, {:queries => [:new, :create, :edit, :update, :destroy]}, :require => :member
         map.permission :save_queries, {:queries => [:new, :create, :edit, :update, :destroy]}, :require => :loggedin
 
+        # Webhooks
+        map.permission :use_webhooks, {}, :require => :member
+
         map.project_module :issue_tracking do |map|
           # Issues
           map.permission :view_issues, {:issues => [:index, :show, :issue_tab],

test/functional/webhooks_controller_test.rb

@@ -16,6 +16,8 @@ class WebhooksControllerTest < Redmine::ControllerTest
     @project = Project.find 'ecookbook'
     @dlopper = User.find_by_login 'dlopper'
     @issue = @project.issues.first
+    @role = Role.find_by_name 'Developer'
+    @role.permissions << :use_webhooks; @role.save!
     @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

test/unit/webhook_test.rb

@@ -22,6 +22,8 @@ class WebhookTest < ActiveSupport::TestCase
 
     @project = Project.find 'ecookbook'
     @dlopper = User.find_by_login 'dlopper'
+    @role = Role.find_by_name 'Developer'
+    @role.permissions << :use_webhooks; @role.save!
     @issue = @project.issues.first
     WebhookEndpointValidator.class_eval do
       @blocked_hosts = nil
@@ -68,7 +70,19 @@ class WebhookTest < ActiveSupport::TestCase
     assert_raise(ActiveRecord::SerializationTypeMismatch){ Webhook.new(events: 'issue.created') }
   end
 
-  test "should clean up project list on save" do
+  test "should clean up project list based on permissions on save" do
+    h = create_hook
+    assert_equal [@project], h.projects
+    @role.permissions.delete :use_webhooks
+    @role.save!
+
+    h.reload
+    h.save
+    h.reload
+    assert_equal [], h.projects
+  end
+
+  test "should clean up project list based on project visibility on save" do
     h = create_hook
     assert_equal [@project], h.projects
     @project.memberships.destroy_all
@@ -80,6 +94,15 @@ class WebhookTest < ActiveSupport::TestCase
     assert_equal [], h.projects
   end
 
+  test "should filter setable projects" do
+    assert_equal [@project], Webhook.new(user: @dlopper).setable_projects
+
+    @role.permissions.delete :use_webhooks
+    @role.save!
+    @dlopper.reload
+    assert_equal [], Webhook.new(user: @dlopper).setable_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[
@@ -109,6 +132,15 @@ class WebhookTest < ActiveSupport::TestCase
     assert_equal [], Webhook.hooks_for('issue.created', @issue)
   end
 
+  test "should check permission when looking for hooks" do
+    hook = create_hook
+    assert @issue.visible?(hook.user)
+    assert_equal [hook], Webhook.hooks_for('issue.created', @issue)
+    @role.permissions.delete :use_webhooks
+    @role.save!
+    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)