Fixes #2777: Knowledge Base - Attachments not working for internal re…

…aders.

app/controllers/attachments_controller.rb

@@ -1,5 +1,6 @@
 class AttachmentsController < ApplicationController
-  prepend_before_action :authentication_check, except: :show
+  prepend_before_action :authentication_check, except: %i[show destroy]
+  prepend_before_action :authentication_check_only, only: %i[show destroy]
   before_action :verify_object_permissions, only: %i[show destroy]
 
   def show

app/models/concerns/can_be_published.rb

@@ -10,6 +10,10 @@ def visible?
     can_be_published_aasm.published?
   end
 
+  def visible_internally?
+    can_be_published_aasm.internal? || visible?
+  end
+
   class_methods do
     def inverse_relation_name(scope_name)
       "can_be_published_#{scope_name}_#{model_name.plural}"

app/models/concerns/has_knowledge_base_attachment_permissions.rb

@@ -6,12 +6,15 @@ module HasKnowledgeBaseAttachmentPermissions
     def can_show_attachment?(file, user)
       return true if user_kb_editor?(user)
 
-      find(file.o_id)&.visible?
+      object = find(file.o_id)
+
+      return object&.visible_internally? if user_kb_reader?(user)
+
+      object&.visible?
     end
 
-    def can_destroy_attachment?(file, user)
+    def can_destroy_attachment?(_file, user)
       return if !user_kb_editor?(user)
-      return if !HasRichText.attachment_inline?(file)
 
       true
     end
@@ -21,6 +24,12 @@ def user_kb_editor?(user)
 
       user.permissions? %w[knowledge_base.editor]
     end
+
+    def user_kb_reader?(user)
+      return false if user.nil?
+
+      user.permissions? %w[knowledge_base.reader]
+    end
   end
 
   included do

app/models/knowledge_base/answer/translation/content.rb

@@ -15,6 +15,10 @@ def visible?
     translation.answer.visible?
   end
 
+  def visible_internally?
+    translation.answer.visible_internally?
+  end
+
   delegate :created_by_id, to: :translation
 
   def attributes_with_association_ids

spec/requests/knowledge_base/attachments_spec.rb

@@ -0,0 +1,206 @@
+require 'rails_helper'
+
+RSpec.describe 'KnowledgeBase attachments', type: :request do
+  include_context 'basic Knowledge Base'
+
+  let(:attachment) do
+    attachment_file = File.open 'spec/fixtures/upload/hello_world.txt'
+
+    Store.add(
+      object:        object.class.to_s,
+      o_id:          object.id,
+      data:          attachment_file.read,
+      filename:      'hello_world.txt',
+      preferences:   {},
+      created_by_id: 1,
+    )
+  end
+
+  let(:endpoint) { "/api/v1/attachments/#{attachment.id}" }
+
+  describe 'visible when attached to' do
+    shared_examples 'a visible resource' do
+      it 'and returns correct status code' do
+        get endpoint
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    shared_examples 'a non-existent resource' do
+      it 'and returns correct status code' do
+        get endpoint
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    describe 'draft answer' do
+      let(:object) { draft_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a non-existent resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-existent resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-existent resource'
+      end
+    end
+
+    describe 'internal answer' do
+      let(:object) { internal_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-existent resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-existent resource'
+      end
+    end
+
+    describe 'published answer' do
+      let(:object) { published_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a visible resource'
+      end
+    end
+
+    describe 'archived answer' do
+      let(:object) { archived_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a non-existent resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a visible resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-existent resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-existent resource'
+      end
+    end
+  end
+
+  describe 'deletable when attached to' do
+    shared_examples 'a deletable resource' do
+      it { expect { delete endpoint }.to change { Store.exists? attachment.id }.from(true).to(false) }
+    end
+
+    shared_examples 'a non-deletable resource' do
+      it { expect { delete endpoint }.not_to change { Store.exists? attachment.id }.from(true) }
+    end
+
+    describe 'draft answer' do
+      let(:object) { draft_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a deletable resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-deletable resource'
+      end
+    end
+
+    describe 'internal answer' do
+      let(:object) { internal_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a deletable resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-deletable resource'
+      end
+    end
+
+    describe 'published answer' do
+      let(:object) { published_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a deletable resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-deletable resource'
+      end
+    end
+
+    describe 'archived answer' do
+      let(:object) { archived_answer }
+
+      describe 'as agent', authenticated_as: :agent_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as admin', authenticated_as: :admin_user do
+        it_behaves_like 'a deletable resource'
+      end
+
+      context 'as customer', authenticated_as: :customer_user do
+        it_behaves_like 'a non-deletable resource'
+      end
+
+      context 'as guest' do
+        it_behaves_like 'a non-deletable resource'
+      end
+    end
+  end
+end

spec/support/knowledge_base_contexts.rb

@@ -26,4 +26,8 @@
   let :internal_answer do
     create(:knowledge_base_answer, category: category, internal_at: 1.week.ago)
   end
+
+  let :archived_answer do
+    create(:knowledge_base_answer, category: category, archived_at: 1.week.ago)
+  end
 end