Follow-up: 276c45b2e9 - Improved ticket policy scope handling.

app/policies/ticket_policy/base_scope.rb

@@ -13,7 +13,7 @@ class TicketPolicy < ApplicationPolicy
       super
     end
 
-    def resolve # rubocop:disable Metrics/AbcSize
+    def resolve # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
       raise NoMethodError, <<~ERR.chomp if instance_of?(TicketPolicy::BaseScope)
         specify an access type using a subclass of TicketPolicy::BaseScope
       ERR
@@ -26,12 +26,19 @@ class TicketPolicy < ApplicationPolicy
         bind.push(user.group_ids_access(self.class::ACCESS_TYPE))
       end
 
-      if user.organization&.shared
+      if user.permissions?('ticket.customer')
-        sql.push('(tickets.customer_id = ? OR tickets.organization_id = ?)')
+        if user.organization&.shared
-        bind.push(user.id, user.organization.id)
+          sql.push('(tickets.customer_id = ? OR tickets.organization_id = ?)')
-      else
+          bind.push(user.id, user.organization.id)
-        sql.push('tickets.customer_id = ?')
+        else
-        bind.push(user.id)
+          sql.push('tickets.customer_id = ?')
+          bind.push(user.id)
+        end
+      end
+
+      # The report permission can access all tickets.
+      if sql.empty? && !user.permissions?('report')
+        sql.push '0 = 1' # Forbid unlimited access for all other permissions.
       end
 
       scope.where sql.join(' OR '), *bind

spec/policies/ticket_policy/shared_examples.rb

@@ -12,15 +12,13 @@ RSpec.shared_examples 'for agent user' do |access_type|
 
   shared_examples 'shown' do
     it 'returns its groups’ tickets' do
-      expect(scope.resolve)
+      expect(scope.resolve).to match_array(Ticket.where(group: member_groups))
-        .to match_array(Ticket.where(group: member_groups))
     end
   end
 
   shared_examples 'hidden' do
     it 'does not return its groups’ tickets' do
-      expect(scope.resolve)
+      expect(scope.resolve).to be_empty
-        .to be_empty
     end
   end
 
@@ -47,6 +45,62 @@ RSpec.shared_examples 'for agent user' do |access_type|
     end
   end
 
+  context 'without any role permission' do
+
+    let(:role_without_rights) { create(:role) }
+    let(:user) { create(:agent, groups: member_groups, role_ids: [ role_without_rights.id ]) }
+
+    context 'when checking for "full" access' do
+      # this is already true by default, but it doesn't hurt to be explicit
+      before { user.user_groups.each { |ug| ug.update_columns(access: 'full') } }
+
+      include_examples 'hidden'
+    end
+
+    context 'when limited to "read" access' do
+      before { user.user_groups.each { |ug| ug.update_columns(access: 'read') } }
+
+      include_examples 'hidden'
+    end
+
+    context 'when limited to "overview" access' do
+      before { user.user_groups.each { |ug| ug.update_columns(access: 'overview') } }
+
+      include_examples 'hidden'
+    end
+  end
+
+  context 'with report permission' do
+
+    let(:report_role) { create(:role).tap { |role| role.permission_grant('report') } }
+    let(:user) { create(:agent, groups: member_groups, role_ids: [ report_role.id ]) }
+
+    context 'when checking for "full" access' do
+      # this is already true by default, but it doesn't hurt to be explicit
+      before { user.user_groups.each { |ug| ug.update_columns(access: 'full') } }
+
+      it 'grants access to all tickets' do
+        expect(scope.resolve).to match_array(Ticket.all)
+      end
+    end
+
+    context 'when limited to "read" access' do
+      before { user.user_groups.each { |ug| ug.update_columns(access: 'read') } }
+
+      it 'grants access to all tickets' do
+        expect(scope.resolve).to match_array(Ticket.all)
+      end
+    end
+
+    context 'when limited to "overview" access' do
+      before { user.user_groups.each { |ug| ug.update_columns(access: 'overview') } }
+
+      it 'grants access to all tickets' do
+        expect(scope.resolve).to match_array(Ticket.all)
+      end
+    end
+  end
+
   context 'with indirect access via Role#groups' do
     let(:user) { create(:agent).tap { |u| u.roles << role } }
     let(:role) { create(:role, groups: member_groups) }
@@ -70,6 +124,41 @@ RSpec.shared_examples 'for agent user' do |access_type|
       include_examples access_type == 'overview' ? 'shown' : 'hidden'
     end
   end
+
+  context 'when checking access via customer organization but no customer permissions' do
+    let(:user) { create(:agent, organization: organization) }
+
+    let(:teammate) { create(:agent, organization: organization) }
+
+    before do
+      create_list(:ticket, 2, customer: user)
+      create_list(:ticket, 2, customer: teammate)
+    end
+
+    context 'with no #organization' do
+      let(:organization) { nil }
+
+      it 'returns no tickets' do
+        expect(scope.resolve).to be_empty
+      end
+    end
+
+    context 'with a non-shared #organization' do
+      let(:organization) { create(:organization, shared: false) }
+
+      it 'returns no tickets' do
+        expect(scope.resolve).to be_empty
+      end
+    end
+
+    context 'with a shared #organization (default)' do
+      let(:organization) { create(:organization, shared: true) }
+
+      it 'returns no tickets' do
+        expect(scope.resolve).to be_empty
+      end
+    end
+  end
 end
 
 RSpec.shared_examples 'for agent user with predefined but impossible context' do