Browse Source

Maintenance: Improved test coverage of link requests.

Thorsten Eckel 3 years ago
parent
commit
4f6aed1856

+ 4 - 0
.rubocop/todo.rspec.yml

@@ -285,6 +285,7 @@ RSpec/ExampleLength:
     - 'spec/requests/integration/twilio_sms_spec.rb'
     - 'spec/requests/integration/user_device_spec.rb'
     - 'spec/requests/knowledge_base/answer_attachments_cloning_spec.rb'
+    - 'spec/requests/links_spec.rb'
     - 'spec/requests/long_polling_spec.rb'
     - 'spec/requests/o_auth_spec.rb'
     - 'spec/requests/organization_spec.rb'
@@ -567,6 +568,7 @@ RSpec/MultipleExpectations:
     - 'spec/requests/integration/telegram_spec.rb'
     - 'spec/requests/integration/twilio_sms_spec.rb'
     - 'spec/requests/integration/user_device_spec.rb'
+    - 'spec/requests/links_spec.rb'
     - 'spec/requests/long_polling_spec.rb'
     - 'spec/requests/o_auth_spec.rb'
     - 'spec/requests/organization_spec.rb'
@@ -610,6 +612,7 @@ RSpec/NamedSubject:
 
 RSpec/NestedGroups:
   Exclude:
+    - 'app/models/user/avatar.rb'
     - 'spec/lib/secure_mailing/smime_spec.rb'
     - 'spec/models/channel/driver/twitter_spec.rb'
     - 'spec/models/channel/email_parser_spec.rb'
@@ -618,6 +621,7 @@ RSpec/NestedGroups:
     - 'spec/models/trigger_spec.rb'
     - 'spec/models/user/has_ticket_create_screen_impact_examples.rb'
     - 'spec/models/user_spec.rb'
+    - 'spec/requests/links_spec.rb'
     - 'spec/system/ticket/create_spec.rb'
 
 RSpec/RepeatedDescription:

+ 1 - 0
app/controllers/links_controller.rb

@@ -8,6 +8,7 @@ class LinksController < ApplicationController
     links = Link.list(
       link_object:       params[:link_object],
       link_object_value: params[:link_object_value],
+      user:              current_user,
     )
 
     linked_objects = links

+ 1 - 0
app/controllers/tickets_controller.rb

@@ -703,6 +703,7 @@ class TicketsController < ApplicationController
     links = Link.list(
       link_object:       'Ticket',
       link_object_value: ticket.id,
+      user:              current_user,
     )
 
     assets = Link.reduce_assets(assets, links)

+ 19 - 1
app/models/link.rb

@@ -53,7 +53,11 @@ class Link < ApplicationModel
       items.push link
     end
 
-    items
+    return items if data[:user].blank?
+
+    items.select do |item|
+      authorized_item?(data[:user], item)
+    end
   end
 
 =begin
@@ -272,4 +276,18 @@ class Link < ApplicationModel
     ', object1_id, object2_id, object1_value, object2_value, object1_id, object2_id, object1_value, object2_value)
   end
 
+  def self.authorized_item?(user, item)
+    record = item['link_object'].constantize.lookup(id: item['link_object_value'])
+
+    # non-ID records are not checked for authorization
+    return true if record.blank?
+
+    Pundit.authorize(user, record, :show?).present?
+  rescue Pundit::NotAuthorizedError
+    false
+  rescue NameError, Pundit::NotDefinedError
+    # NameError: no Model means no authorization check possible
+    # Pundit::NotDefinedError: no Policy means no authorization check necessary
+    true
+  end
 end

+ 2 - 2
spec/factories/link.rb

@@ -4,8 +4,8 @@ FactoryBot.define do
   factory :link do
     transient do
       link_type { 'normal' }
-      link_object_source { 'Ticket' }
-      link_object_target { 'Ticket' }
+      link_object_source { from.class.name }
+      link_object_target { to.class.name }
       from { Ticket.first }
       to   { Ticket.last }
     end

+ 49 - 0
spec/requests/links_spec.rb

@@ -0,0 +1,49 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Link', type: :request do
+
+  describe 'GET /api/v1/links' do
+
+    context 'when requesting links of Ticket', authenticated_as: -> { agent } do
+
+      subject!(:ticket) { create(:ticket) }
+
+      let(:agent) { create(:agent, groups: [ticket.group]) }
+
+      let(:params) do
+        {
+          link_object:       ticket.class.name,
+          link_object_value: ticket.id,
+        }
+      end
+      let(:linked) { create(:ticket, group: ticket.group) }
+
+      before do
+        create(:link, from: ticket, to: linked)
+        get '/api/v1/links', params: params, as: :json
+      end
+
+      it 'is present in response' do
+        expect(response).to have_http_status(:ok)
+        expect(json_response['links']).to eq([
+                                               {
+                                                 'link_type'         => 'normal',
+                                                 'link_object'       => 'Ticket',
+                                                 'link_object_value' => linked.id
+                                               }
+                                             ])
+      end
+
+      context 'without permission to linked Ticket Group' do
+        let(:linked) { create(:ticket) }
+
+        it 'is not present in response' do
+          expect(response).to have_http_status(:ok)
+          expect(json_response['links']).to be_blank
+        end
+      end
+    end
+  end
+end

+ 37 - 0
spec/requests/ticket_spec.rb

@@ -2339,6 +2339,43 @@ RSpec.describe 'Ticket', type: :request do
     end
   end
 
+  describe 'GET /api/v1/tickets/:id' do
+
+    subject!(:ticket) { create(:ticket) }
+
+    let(:agent) { create(:agent, groups: [ticket.group]) }
+
+    context 'links present', authenticated_as: -> { agent } do
+
+      before do
+        create(:link, from: ticket, to: linked)
+        get "/api/v1/tickets/#{ticket.id}", params: { all: 'true' }, as: :json
+      end
+
+      let(:linked) { create(:ticket, group: ticket.group) }
+
+      it 'is present in response' do
+        expect(response).to have_http_status(:ok)
+        expect(json_response['links']).to eq([
+                                               {
+                                                 'link_type'         => 'normal',
+                                                 'link_object'       => 'Ticket',
+                                                 'link_object_value' => linked.id
+                                               }
+                                             ])
+      end
+
+      context 'no permission to linked Ticket Group' do
+        let(:linked) { create(:ticket) }
+
+        it 'is not present in response' do
+          expect(response).to have_http_status(:ok)
+          expect(json_response['links']).to be_blank
+        end
+      end
+    end
+  end
+
   describe 'GET /api/v1/ticket_customer' do
 
     subject(:ticket) { create(:ticket, customer: customer_authorized) }