Maintenance: Mentions permissions handling enhancements

app/controllers/mentions_controller.rb

@@ -1,24 +1,21 @@
 # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
 
 class MentionsController < ApplicationController
-  prepend_before_action -> { authorize! }
-  prepend_before_action { authentication_check }
+  prepend_before_action :authorize!
+  prepend_before_action :authentication_check
 
   # GET /api/v1/mentions
-  def list
-    list = Mention.where(condition).order(created_at: :desc)
+  def index
+    list = mentionable_object.mentions
 
     if response_full?
-      assets = {}
-      item_ids = []
-      list.each do |item|
-        item_ids.push item.id
-        assets = item.assets(assets)
-      end
+      item_ids = list.map(&:id)
+      assets   = ApplicationModel::CanAssets.reduce list
+
       render json: {
         record_ids: item_ids,
         assets:     assets,
-      }, status: :ok
+      }
       return
     end
 
@@ -30,71 +27,26 @@ class MentionsController < ApplicationController
 
   # POST /api/v1/mentions
   def create
-    success = Mention.create!(
-      mentionable: mentionable!,
-      user:        current_user,
-    )
-    if success
-      render json: success, status: :created
-    else
-      render json: success.errors, status: :unprocessable_entity
-    end
+    Mention.subscribe! mentionable_object, current_user
+
+    render json: true, status: :created
   end
 
   # DELETE /api/v1/mentions
   def destroy
-    success = Mention.find_by(user: current_user, id: params[:id]).destroy
-    if success
-      render json: success, status: :ok
-    else
-      render json: success.errors, status: :unprocessable_entity
-    end
-  end
-
-  private
-
-  def mentionable_type!
-    @mentionable_type ||= begin
-      raise __("The parameter 'mentionable_type' is invalid.") if 'Ticket'.freeze != params[:mentionable_type]
-
-      params[:mentionable_type]
-    end
-  end
-
-  def mentionable!
-    object = mentionable_type!.constantize.find(params[:mentionable_id])
-    authorize!(object, :agent_read_access?)
-    object
-  end
-
-  def fill_condition_mentionable(condition)
-    condition[:mentionable_type] = mentionable_type!
-    return if params[:mentionable_id].blank?
-
-    condition[:mentionable_id] = params[:mentionable_id]
-  end
-
-  def fill_condition_id(condition)
-    return if params[:id].blank?
-
-    condition[:id] = params[:id]
-  end
-
-  def fill_condition_user(condition)
-    return if params[:user_id].blank?
+    Mention.where(user: current_user, id: params[:id]).destroy_all
 
-    condition[:user] = User.find(params[:user_id])
+    render json: true, status: :ok
   end
 
-  def condition
-    condition = {}
-    fill_condition_id(condition)
-    fill_condition_user(condition)
-
-    return condition if params[:mentionable_type].blank?
-
-    mentionable!
-    fill_condition_mentionable(condition)
-    condition
+  def mentionable_object
+    @mentionable_object ||= begin
+      case params[:mentionable_type]
+      when 'Ticket'
+        Ticket.find_by id: params[:mentionable_id]
+      else
+        raise Exceptions::UnprocessableEntity, __("The parameter 'mentionable_type' is invalid.")
+      end
+    end
   end
 end

app/controllers/tickets_controller.rb

@@ -174,7 +174,7 @@ class TicketsController < ApplicationController
 
       # create mentions if given
       if params[:mentions].present?
-        authorize!(Mention.new, :create?)
+        authorize!(ticket, :create_mentions?)
         Array(params[:mentions]).each do |user_id|
           Mention.where(mentionable: ticket, user_id: user_id).first_or_create(mentionable: ticket, user_id: user_id)
         end

app/models/mention.rb

@@ -85,4 +85,17 @@ class Mention < ApplicationModel
 
     true
   end
+
+  # Check if given user is able to subscribe to a given object
+  # @param object to subscribe to
+  # @param mentioned user
+  # @return Boolean
+  def self.mentionable?(object, user)
+    case object
+    when Ticket
+      TicketPolicy.new(user, object).agent_read_access?
+    else
+      false
+    end
+  end
 end

app/models/mention/validation.rb

@@ -1,22 +1,9 @@
 # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
 
 class Mention::Validation < ActiveModel::Validator
-  attr_reader :record
-
   def validate(record)
-    @record = record
-    check_user_permission
-  end
-
-  private
-
-  def check_user_permission
-    return if MentionPolicy.new(record.user, record).create?
-
-    invalid_because(:user, 'has no ticket.agent permissions')
-  end
+    return if Mention.mentionable? record.mentionable, record.user
 
-  def invalid_because(attribute, message)
-    record.errors.add attribute, message
+    record.errors.add :user, 'has no agent access to this ticket'
   end
 end

app/models/ticket/article.rb

@@ -341,7 +341,7 @@ returns
 
     return if mention_user_ids.blank?
 
-    if !MentionPolicy.new(updated_by, Mention.new).create?
+    if !TicketPolicy.new(updated_by, ticket).create_mentions?
       return if ApplicationHandleInfo.postmaster?
       return if updated_by.id == 1
 

app/policies/controllers/mentions_controller_policy.rb

@@ -1,5 +1,29 @@
 # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
 
 class Controllers::MentionsControllerPolicy < Controllers::ApplicationControllerPolicy
-  default_permit!('ticket.agent')
+  def index?
+    object_accessible?
+  end
+
+  def create?
+    object_accessible?
+  end
+
+  def destroy?
+    mentioned_user?
+  end
+
+  private
+
+  def object_accessible?
+    Mention.mentionable? record.mentionable_object, user
+  rescue Exceptions::UnprocessableEntity => e
+    not_authorized(e)
+  end
+
+  def mentioned_user?
+    mention = Mention.find_by id: record.params[:id]
+
+    mention&.user_id == user.id
+  end
 end

app/policies/mention_policy.rb

@@ -1,7 +0,0 @@
-# Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
-
-class MentionPolicy < ApplicationPolicy
-  def create?
-    user.permissions?('ticket.agent')
-  end
-end

app/policies/ticket_policy.rb

@@ -57,6 +57,10 @@ class TicketPolicy < ApplicationPolicy
     agent_access?('change')
   end
 
+  def create_mentions?
+    agent_read_access?
+  end
+
   private
 
   def follow_up_possible?

config/routes/mention.rb

@@ -1,9 +1,7 @@
 # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
 
 Zammad::Application.routes.draw do
-  api_path = Rails.configuration.api_path
-
-  match api_path + '/mentions',           to: 'mentions#list',         via: :get
-  match api_path + '/mentions',           to: 'mentions#create',       via: :post
-  match api_path + '/mentions/:id',       to: 'mentions#destroy',      via: :delete
+  scope Rails.configuration.api_path do
+    resources :mentions, only: %i[index create destroy]
+  end
 end

spec/lib/search_index_backend_spec.rb

@@ -248,7 +248,7 @@ RSpec.describe SearchIndexBackend do
     let(:agent1)        { create(:agent, organization: organization1, groups: [group1]) }
     let(:customer1)     { create(:customer, organization: organization1, firstname: 'special-first-name') }
     let(:ticket1) do
-      ticket = create(:ticket, title: 'some-title1', state_id: 1, created_by: agent1)
+      ticket = create(:ticket, title: 'some-title1', state_id: 1, created_by: agent1, group: group1)
       ticket.tag_add('t1', 1)
       ticket
     end