From e65e3a6d14174378b8bf58f5997cde3de40c3ca7 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 22 Jun 2023 14:52:25 +0200 Subject: [PATCH] Add finer permission requirements for managing webhooks (#25463) --- app/controllers/admin/webhooks_controller.rb | 3 +++ app/models/webhook.rb | 20 ++++++++++++++++++++ app/policies/webhook_policy.rb | 4 ++-- app/views/admin/webhooks/_form.html.haml | 2 +- config/locales/activerecord.en.yml | 4 ++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/webhooks_controller.rb b/app/controllers/admin/webhooks_controller.rb index 87d0c65709..76062ddf7f 100644 --- a/app/controllers/admin/webhooks_controller.rb +++ b/app/controllers/admin/webhooks_controller.rb @@ -20,6 +20,7 @@ module Admin authorize :webhook, :create? @webhook = Webhook.new(resource_params) + @webhook.current_account = current_account if @webhook.save redirect_to admin_webhook_path(@webhook) @@ -39,6 +40,8 @@ module Admin def update authorize @webhook, :update? + @webhook.current_account = current_account + if @webhook.update(resource_params) redirect_to admin_webhook_path(@webhook) else diff --git a/app/models/webhook.rb b/app/models/webhook.rb index 4aafb1257b..bc1c027d50 100644 --- a/app/models/webhook.rb +++ b/app/models/webhook.rb @@ -20,6 +20,8 @@ class Webhook < ApplicationRecord report.created ).freeze + attr_writer :current_account + scope :enabled, -> { where(enabled: true) } validates :url, presence: true, url: true @@ -27,6 +29,7 @@ class Webhook < ApplicationRecord validates :events, presence: true validate :validate_events + validate :validate_permissions before_validation :strip_events before_validation :generate_secret @@ -43,12 +46,29 @@ class Webhook < ApplicationRecord update!(enabled: false) end + def required_permissions + events.map { |event| Webhook.permission_for_event(event) } + end + + def self.permission_for_event(event) + case event + when 'account.approved', 'account.created', 'account.updated' + :manage_users + when 'report.created' + :manage_reports + end + end + private def validate_events errors.add(:events, :invalid) if events.any? { |e| !EVENTS.include?(e) } end + def validate_permissions + errors.add(:events, :invalid_permissions) if defined?(@current_account) && required_permissions.any? { |permission| !@current_account.user_role.can?(permission) } + end + def strip_events self.events = events.map { |str| str.strip.presence }.compact if events.present? end diff --git a/app/policies/webhook_policy.rb b/app/policies/webhook_policy.rb index a2199a333f..577e891b66 100644 --- a/app/policies/webhook_policy.rb +++ b/app/policies/webhook_policy.rb @@ -14,7 +14,7 @@ class WebhookPolicy < ApplicationPolicy end def update? - role.can?(:manage_webhooks) + role.can?(:manage_webhooks) && record.required_permissions.all? { |permission| role.can?(permission) } end def enable? @@ -30,6 +30,6 @@ class WebhookPolicy < ApplicationPolicy end def destroy? - role.can?(:manage_webhooks) + role.can?(:manage_webhooks) && record.required_permissions.all? { |permission| role.can?(permission) } end end diff --git a/app/views/admin/webhooks/_form.html.haml b/app/views/admin/webhooks/_form.html.haml index c1e8f8979b..6c5ff03dd5 100644 --- a/app/views/admin/webhooks/_form.html.haml +++ b/app/views/admin/webhooks/_form.html.haml @@ -5,7 +5,7 @@ = f.input :url, wrapper: :with_block_label, input_html: { placeholder: 'https://' } .fields-group - = f.input :events, collection: Webhook::EVENTS, wrapper: :with_block_label, include_blank: false, as: :check_boxes, collection_wrapper_tag: 'ul', item_wrapper_tag: 'li' + = f.input :events, collection: Webhook::EVENTS, wrapper: :with_block_label, include_blank: false, as: :check_boxes, collection_wrapper_tag: 'ul', item_wrapper_tag: 'li', disabled: Webhook::EVENTS.filter { |event| !current_user.role.can?(Webhook.permission_for_event(event)) } .actions = f.button :button, @webhook.new_record? ? t('admin.webhooks.add_new') : t('generic.save_changes'), type: :submit diff --git a/config/locales/activerecord.en.yml b/config/locales/activerecord.en.yml index 8aee15659f..a53c7c6e9e 100644 --- a/config/locales/activerecord.en.yml +++ b/config/locales/activerecord.en.yml @@ -53,3 +53,7 @@ en: position: elevated: cannot be higher than your current role own_role: cannot be changed with your current role + webhook: + attributes: + events: + invalid_permissions: cannot include events you don't have the rights to