Add finer permission requirements for managing webhooks (#25463)

app/controllers/admin/webhooks_controller.rb

@@ -28,6 +28,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

app/models/webhook.rb

@@ -24,6 +24,8 @@ class Webhook < ApplicationRecord
     status.updated
   ).freeze
 
+  attr_writer :current_account
+
   scope :enabled, -> { where(enabled: true) }
 
   validates :url, presence: true, url: true
@@ -31,6 +33,7 @@ class Webhook < ApplicationRecord
   validates :events, presence: true
 
   validate :validate_events
+  validate :validate_permissions
   validate :validate_template
 
   before_validation :strip_events
@@ -48,12 +51,31 @@ 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
+    when 'status.created', 'status.updated'
+      :view_devops
+    end
+  end
+
   private
 
   def validate_events
     errors.add(:events, :invalid) if events.any? { |e| EVENTS.exclude?(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 validate_template
     return if template.blank?
 

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

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)) }
 
   .fields-group
     = f.input :template, wrapper: :with_block_label, input_html: { placeholder: '{ "content": "Hello {{object.username}}" }' }

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

spec/controllers/admin/webhooks_controller_spec.rb

@@ -48,7 +48,7 @@ describe Admin::WebhooksController do
   end
 
   context 'with an existing record' do
-    let!(:webhook) { Fabricate :webhook }
+    let!(:webhook) { Fabricate(:webhook, events: ['account.created', 'report.created']) }
 
     describe 'GET #show' do
       it 'returns http success and renders view' do

spec/policies/webhook_policy_spec.rb

@@ -8,16 +8,32 @@ describe WebhookPolicy do
   let(:admin)   { Fabricate(:user, role: UserRole.find_by(name: 'Admin')).account }
   let(:john)    { Fabricate(:account) }
 
-  permissions :index?, :create?, :show?, :update?, :enable?, :disable?, :rotate_secret?, :destroy? do
+  permissions :index?, :create? do
     context 'with an admin' do
       it 'permits' do
-        expect(policy).to permit(admin, Tag)
+        expect(policy).to permit(admin, Webhook)
       end
     end
 
     context 'with a non-admin' do
       it 'denies' do
-        expect(policy).to_not permit(john, Tag)
+        expect(policy).to_not permit(john, Webhook)
+      end
+    end
+  end
+
+  permissions :show?, :update?, :enable?, :disable?, :rotate_secret?, :destroy? do
+    let(:webhook) { Fabricate(:webhook, events: ['account.created', 'report.created']) }
+
+    context 'with an admin' do
+      it 'permits' do
+        expect(policy).to permit(admin, webhook)
+      end
+    end
+
+    context 'with a non-admin' do
+      it 'denies' do
+        expect(policy).to_not permit(john, webhook)
       end
     end
   end