Fixes #5193 - Information about remote images is lost/not displayed in Zammad.

Co-authored-by: Dusan Vuckovic <dv@zammad.com>
Co-authored-by: Florian Liebe <fl@zammad.com>

app/assets/javascripts/app/controllers/article_view/item.coffee

@@ -15,15 +15,16 @@ class App.ArticleViewItem extends App.ControllerObserver
     '.textBubble-overflowContainer': 'textBubbleOverflowContainer'
 
   events:
-    'click .article-meta-permanent':  'toggleMetaWithDelay'
-    'click .textBubble':              'toggleMetaWithDelay'
-    'click .textBubble a':            'stopPropagation'
-    'click .js-toggleFold':           'toggleFold'
-    'click .richtext-content img':    'imageView'
-    'click .attachments img':         'imageView'
-    'click .file-calendar .js-preview':  'calendarView'
-    'click .js-securityRetryProcess': 'retrySecurityProcess'
+    'click .article-meta-permanent':             'toggleMetaWithDelay'
+    'click .textBubble':                         'toggleMetaWithDelay'
+    'click .textBubble a':                       'stopPropagation'
+    'click .js-toggleFold':                      'toggleFold'
+    'click .richtext-content img':               'imageView'
+    'click .attachments img':                    'imageView'
+    'click .file-calendar .js-preview':          'calendarView'
+    'click .js-securityRetryProcess':            'retrySecurityProcess'
     'click .js-retryWhatsAppAttachmentDownload': 'retryWhatsAppAttachmentDownload'
+    'click .js-fetchOriginalFormatting':         'fetchOriginalFormatting'
 
   constructor: ->
     super
@@ -82,8 +83,9 @@ class App.ArticleViewItem extends App.ControllerObserver
         attachment.preview_url = "#{App.Config.get('api_path')}/ticket_attachment/#{article.ticket_id}/#{article.id}/#{attachment.id}?view=preview"
 
         if attachment && attachment.preferences && attachment.preferences['original-format'] is true
+          @originalFormattingURL = "#{App.Config.get('api_path')}/ticket_attachment/#{article.ticket_id}/#{article.id}/#{attachment.id}?disposition=attachment"
           link =
-              url: "#{App.Config.get('api_path')}/ticket_attachment/#{article.ticket_id}/#{article.id}/#{attachment.id}?disposition=attachment"
+              url: @originalFormattingURL
               name: __('Original Formatting')
               target: '_blank'
           links.push link
@@ -320,6 +322,18 @@ class App.ArticleViewItem extends App.ControllerObserver
           msg:  App.i18n.translateContent(details.error)
     )
 
+  fetchOriginalFormatting: (e) ->
+    return if not @originalFormattingURL
+
+    e.preventDefault()
+    e.stopPropagation()
+
+    originalFormattingLink = document.createElement('a')
+    originalFormattingLink.href = @originalFormattingURL
+    originalFormattingLink.target = '_blank'
+    originalFormattingLink.click()
+    originalFormattingLink.remove()
+
   stopPropagation: (e) ->
     e.stopPropagation()
 

app/assets/javascripts/app/views/ticket_zoom/article_view.jst.eco

@@ -83,7 +83,7 @@
     </div>
   </div>
 </div>
-<% if encryptionSuccess || signSuccess || encryptionWarning || signWarning || @article.preferences?.whatsapp?.media_error: %>
+<% if encryptionSuccess || signSuccess || encryptionWarning || signWarning || @article.preferences?.whatsapp?.media_error || @article.preferences?.remote_content_removed: %>
   <div class="article-meta-permanent">
     <% if encryptionWarning || signWarning: %>
       <div class="alert alert--warning">
@@ -113,6 +113,15 @@
       </div>
     </div>
   <% end %>
+
+  <% if @article.preferences?.remote_content_removed: %>
+    <div class="alert alert--warning">
+      <div class="alert-row">
+        <%- @Icon('danger') %> <%- @T('For privacy reasons, Zammad has blocked remote content in this message. You can retrieve it from the original formatting.') %>
+      </div>
+      <div class="btn btn--action btn--small js-fetchOriginalFormatting"><%- @T('Original Formatting') %></div>
+    </div>
+  <% end %>
   </div>
 <% end %>
 <div class="article-content">

app/assets/stylesheets/zammad.scss

@@ -3936,6 +3936,10 @@ ol.tabs li {
 
 .icon-danger {
   color: hsl(41, 100%, 49%);
+
+  .alert.alert--warning & {
+    color: inherit;
+  }
 }
 
 .icon-draggable {

app/frontend/apps/mobile/pages/ticket/__tests__/ticket-detail-view.spec.ts

@@ -460,6 +460,41 @@ describe('calling API to retry encryption', () => {
   })
 })
 
+describe('remote content removal', () => {
+  it('shows blocked content badge', async () => {
+    const articlesQuery = defaultArticles()
+    const article = articlesQuery.description!.edges[0].node
+    article.preferences = {
+      remote_content_removed: true,
+    }
+    article.attachmentsWithoutInline = [
+      {
+        internalId: 1,
+        name: 'message',
+        preferences: {
+          'original-format': true,
+        },
+      },
+    ]
+
+    const { waitUntilTicketLoaded } = mockTicketDetailViewGql({
+      articles: articlesQuery,
+    })
+
+    const view = await visitView('/tickets/1')
+
+    await waitUntilTicketLoaded()
+
+    const blockedContent = view.getByRole('button', { name: 'Blocked Content' })
+
+    await view.events.click(blockedContent)
+
+    await view.events.click(view.getByText('Original Formatting'))
+
+    expect(view.queryByTestId('popupWindow')).not.toBeInTheDocument()
+  })
+})
+
 describe('ticket viewers inside a ticket', () => {
   it('displays information with newer last interaction (and without own entry)', async () => {
     const { waitUntilTicketLoaded, mockTicketLiveUsersSubscription } =

app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleBubble.vue

@@ -29,6 +29,7 @@ import { useArticleAttachments } from '../../composable/useArticleAttachments.ts
 import { useArticleSeen } from '../../composable/useArticleSeen.ts'
 import { useArticleToggleMore } from '../../composable/useArticleToggleMore.ts'
 
+import ArticleRemoteContentBadge from './ArticleRemoteContentBadge.vue'
 import ArticleSecurityBadge from './ArticleSecurityBadge.vue'
 import ArticleWhatsappMediaBadge from './ArticleWhatsappMediaBadge.vue'
 
@@ -42,6 +43,7 @@ interface Props {
   ticketInternalId: number
   articleId: string
   attachments: TicketArticleAttachment[]
+  remoteContentWarning?: string
   mediaError?: boolean | null
 }
 
@@ -329,6 +331,10 @@ const onContextClick = () => {
             :success-class="colorClasses"
             :security="security"
           />
+          <ArticleRemoteContentBadge
+            v-if="remoteContentWarning"
+            :original-formatting-url="remoteContentWarning"
+          />
           <button
             v-if="hasShowMore"
             :class="[

app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleRemoteContentBadge.vue

@@ -0,0 +1,63 @@
+<!-- Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/ -->
+
+<script setup lang="ts">
+import { computed, ref } from 'vue'
+
+import CommonSectionPopup from '#mobile/components/CommonSectionPopup/CommonSectionPopup.vue'
+
+export interface Props {
+  originalFormattingUrl?: string
+}
+
+const props = defineProps<Props>()
+
+const showPopup = ref(false)
+
+const popupItems = computed(() =>
+  props.originalFormattingUrl
+    ? [
+        {
+          type: 'link' as const,
+          label: __('Original Formatting'),
+          link: props.originalFormattingUrl,
+          attributes: {
+            'rest-api': true,
+            target: '_blank',
+          },
+        },
+      ]
+    : [],
+)
+</script>
+
+<template>
+  <button
+    v-bind="$attrs"
+    type="button"
+    class="bg-yellow inline-flex h-7 grow items-center gap-1 rounded-lg px-2 py-1 text-xs font-bold text-black"
+    @click.prevent="showPopup = !showPopup"
+    @keydown.space.prevent="showPopup = !showPopup"
+  >
+    <CommonIcon name="warning" decorative size="xs" />
+    {{ $t('Blocked Content') }}
+  </button>
+  <CommonSectionPopup v-model:state="showPopup" :messages="popupItems">
+    <template #header>
+      <div
+        class="flex flex-col items-center gap-2 border-b border-b-white/10 p-4"
+      >
+        <div class="text-yellow flex w-full items-center justify-center gap-1">
+          <CommonIcon name="warning" size="tiny" />
+          {{ $t('Blocked Content') }}
+        </div>
+        <div>
+          {{
+            $t(
+              'For privacy reasons, Zammad has blocked remote content in this message. You can retrieve it from the original formatting.',
+            )
+          }}
+        </div>
+      </div>
+    </template>
+  </CommonSectionPopup>
+</template>

app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticlesList.vue

@@ -48,6 +48,24 @@ const filterAttachments = (article: TicketArticle) => {
   )
 }
 
+const remoteContentWarning = (article: TicketArticle): string | undefined => {
+  if (!article.preferences?.remote_content_removed) return
+
+  let originalFormattingUrl
+
+  article.attachmentsWithoutInline.forEach((file) => {
+    if (file.preferences?.['original-format'] !== true) {
+      return
+    }
+    const articleInternalId = article.internalId
+    const attachmentInternalId = file.internalId
+    const ticketInternalId = props.ticket.internalId
+    originalFormattingUrl = `/ticket_attachment/${ticketInternalId}/${articleInternalId}/${attachmentInternalId}?disposition=attachment`
+  })
+
+  return originalFormattingUrl
+}
+
 const { newArticlesIds } = useTicketInformation()
 
 const markSeen = (id: string) => {
@@ -75,6 +93,7 @@ const markSeen = (id: string) => {
         :ticket-internal-id="ticket.internalId"
         :article-id="row.article.id"
         :attachments="filterAttachments(row.article)"
+        :remote-content-warning="remoteContentWarning(row.article)"
         @seen="markSeen(row.key)"
         @show-context="showArticleContext(row.article, ticket)"
       />

app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/__tests__/ArticleRemoteContentBadge.spec.ts

@@ -0,0 +1,66 @@
+// Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+import { within } from '@testing-library/vue'
+
+import { renderComponent } from '#tests/support/components/index.ts'
+import { mockApplicationConfig } from '#tests/support/mock-applicationConfig.ts'
+
+import ArticleRemoteContentBadge, {
+  type Props,
+} from '../ArticleRemoteContentBadge.vue'
+
+const renderBadge = (propsData: Props = {}) => {
+  return renderComponent(ArticleRemoteContentBadge, {
+    props: propsData,
+    router: true,
+  })
+}
+
+const originalFormattingUrl =
+  '/ticket_attachment/12/34/56?disposition=attachment'
+
+describe('rendering remote content badge', () => {
+  beforeEach(() => {
+    mockApplicationConfig({
+      api_path: '/api/v1',
+    })
+  })
+
+  it('renders the button and popup on click', async () => {
+    const view = renderBadge({ originalFormattingUrl })
+
+    const button = view.getByRole('button', { name: 'Blocked Content' })
+
+    expect(view.getByIconName('warning')).toBeInTheDocument()
+
+    await view.events.click(button)
+
+    const popup = view.getByTestId('popupWindow')
+
+    expect(within(popup).getByText('Blocked Content')).toBeInTheDocument()
+    expect(
+      within(popup).getByText(
+        'For privacy reasons, Zammad has blocked remote content in this message. You can retrieve it from the original formatting.',
+      ),
+    ).toBeInTheDocument()
+
+    const link = within(popup).getByText('Original Formatting')
+
+    expect(link).toHaveAttribute('href', `/api/v1${originalFormattingUrl}`)
+    expect(link).toHaveAttribute('target', '_blank')
+  })
+
+  it('does not show original formatting link if missing', async () => {
+    const view = renderBadge()
+
+    await view.events.click(
+      view.getByRole('button', { name: 'Blocked Content' }),
+    )
+
+    const popup = view.getByTestId('popupWindow')
+
+    expect(
+      within(popup).queryByText('Original Formatting'),
+    ).not.toBeInTheDocument()
+  })
+})

app/models/channel/email_parser.rb

@@ -301,6 +301,9 @@ returns
       # x-headers lookup
       set_attributes_by_x_headers(article, 'article', mail)
 
+      # Store additional information in preferences, e.g. if remote content got removed.
+      article.preferences.merge!(mail[:sanitized_body_info])
+
       # create article
       article.save!
 
@@ -626,7 +629,7 @@ returns
   def message_body_hash(mail)
     if mail.html_part&.body.present?
       content_type = mail.html_part.mime_type || 'text/plain'
-      body = body_text(mail.html_part, strict_html: true)
+      (body, sanitized_body_info) = body_text(mail.html_part, strict_html: true)
     elsif mail.text_part.present? && mail.all_parts.any? { |elem| elem.inline? && elem.content_type&.start_with?('image') }
       content_type = 'text/html'
 
@@ -634,7 +637,7 @@ returns
         .all_parts
         .reduce('') do |memo, part|
           if part.mime_type == 'text/plain' && !part.attachment?
-            memo += body_text(part, strict_html: false).text2html
+            memo += body_text(part, strict_html: false).first.text2html
           elsif part.inline? && part.content_type&.start_with?('image')
             memo += "<img src='cid:#{part.cid}'>"
           end
@@ -648,22 +651,23 @@ returns
         .all_parts
         .reduce('') do |memo, part|
           if part.mime_type == 'text/plain' && !part.attachment?
-            memo += body_text(part, strict_html: false)
+            memo += body_text(part, strict_html: false).first
           end
 
           memo
         end
     elsif mail&.body.present? && (mail.mime_type.nil? || mail.mime_type.match?(%r{^text/(plain|html)$}))
       content_type = mail.mime_type || 'text/plain'
-      body = body_text(mail, strict_html: content_type.eql?('text/html'))
+      (body, sanitized_body_info) = body_text(mail, strict_html: content_type.eql?('text/html'))
     end
 
     content_type = 'text/plain' if body.blank?
 
     {
-      attachments:  collect_attachments(mail),
-      content_type: content_type || 'text/plain',
-      body:         body.presence || 'no visible content'
+      attachments:         collect_attachments(mail),
+      content_type:        content_type || 'text/plain',
+      body:                body.presence || 'no visible content',
+      sanitized_body_info: sanitized_body_info || {},
     }.with_indifferent_access
   end
 
@@ -678,10 +682,10 @@ returns
     body_text = Mail::Utilities.to_lf(body_text)
 
     # plaintext body requires no processing
-    return body_text if !options[:strict_html]
+    return [body_text, {}] if !options[:strict_html]
 
     # Issue #2390 - emails with >5k HTML links should be rejected
-    return EXCESSIVE_LINKS_MSG if body_text.scan(%r{<a[[:space:]]}i).count >= 5_000
+    return [EXCESSIVE_LINKS_MSG, {}] if body_text.scan(%r{<a[[:space:]]}i).count >= 5_000
 
     body_text.html2html_strict
   end
@@ -719,7 +723,7 @@ returns
     }.compact_blank
 
     [{
-      data:        body_text(message),
+      data:        body_text(message).first,
       filename:    filename,
       preferences: headers_store
     }]

i18n/zammad.pot

@@ -2026,6 +2026,10 @@ msgstr ""
 msgid "Block caller IDs based on sender caller ID."
 msgstr ""
 
+#: app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleRemoteContentBadge.vue
+msgid "Blocked Content"
+msgstr ""
+
 #: app/assets/javascripts/app/models/chat.coffee
 msgid "Blocked IPs (separated by ;)"
 msgstr ""
@@ -5972,6 +5976,11 @@ msgstr ""
 msgid "For more details, please check out our online documentation %l."
 msgstr ""
 
+#: app/assets/javascripts/app/views/ticket_zoom/article_view.jst.eco
+#: app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleRemoteContentBadge.vue
+msgid "For privacy reasons, Zammad has blocked remote content in this message. You can retrieve it from the original formatting."
+msgstr ""
+
 #: app/assets/javascripts/app/views/profile/token_access_created.jst.eco
 #: app/frontend/apps/desktop/pages/personal-setting/components/PersonalSettingNewAccessTokenFlyout.vue
 msgid "For security reasons, the API token is shown only once. You'll need to save it somewhere secure before continuing."
@@ -9828,7 +9837,9 @@ msgid "Origin By"
 msgstr ""
 
 #: app/assets/javascripts/app/controllers/article_view/item.coffee
+#: app/assets/javascripts/app/views/ticket_zoom/article_view.jst.eco
 #: app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleMetadataDialog.vue
+#: app/frontend/apps/mobile/pages/ticket/components/TicketDetailView/ArticleRemoteContentBadge.vue
 msgid "Original Formatting"
 msgstr ""