From a524967fbc4b2b2154f71c140680fb7b5ea19190 Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Tue, 7 Oct 2025 06:51:33 +0000 Subject: [PATCH] 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). 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@24035 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- Gemfile | 1 - app/controllers/webhooks_controller.rb | 6 ++++ app/models/webhook.rb | 7 +++-- app/views/my/account.html.erb | 1 + config/locales/de.yml | 1 + config/locales/en.yml | 1 + lib/redmine/preparation.rb | 3 ++ test/functional/webhooks_controller_test.rb | 2 ++ test/unit/webhook_test.rb | 34 ++++++++++++++++++++- 9 files changed, 51 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index e42693f30..13bbb6c71 100644 --- a/Gemfile +++ b/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") diff --git a/app/controllers/webhooks_controller.rb b/app/controllers/webhooks_controller.rb index d5eb98581..98ebe07fa 100644 --- a/app/controllers/webhooks_controller.rb +++ b/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 diff --git a/app/models/webhook.rb b/app/models/webhook.rb index b36abdd4c..11dafd3a2 100644 --- a/app/models/webhook.rb +++ b/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 diff --git a/app/views/my/account.html.erb b/app/views/my/account.html.erb index 95afbabac..b516dd842 100644 --- a/app/views/my/account.html.erb +++ b/app/views/my/account.html.erb @@ -1,6 +1,7 @@
<%= 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)%>
diff --git a/config/locales/de.yml b/config/locales/de.yml index 7aeae1768..ac4d24971 100644 --- a/config/locales/de.yml +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 5385ad02d..e8ab8cd25 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index a7387f5dc..fec55eb46 100644 --- a/lib/redmine/preparation.rb +++ b/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], diff --git a/test/functional/webhooks_controller_test.rb b/test/functional/webhooks_controller_test.rb index 418eb76d6..0a3cdc1a5 100644 --- a/test/functional/webhooks_controller_test.rb +++ b/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 diff --git a/test/unit/webhook_test.rb b/test/unit/webhook_test.rb index 05c4729a4..ee6b6f28c 100644 --- a/test/unit/webhook_test.rb +++ b/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)