Fixes: Mobile - Align the security implementation with the email article reply form.

app/frontend/apps/mobile/pages/ticket/composable/useTicketEditForm.ts

@@ -82,7 +82,6 @@ export const useTicketEditForm = (ticket: Ref<TicketById | undefined>) => {
         props: {
           options: ticketArticleTypes,
         },
-        triggerFormUpdater: false,
       },
       {
         name: 'internal',
@@ -115,7 +114,6 @@ export const useTicketEditForm = (ticket: Ref<TicketById | undefined>) => {
           contact: recipientContact,
           multiple: true,
         },
-        triggerFormUpdater: false,
       },
       {
         if: '$fns.includes($currentArticleType.attributes, "cc")',
@@ -126,7 +124,6 @@ export const useTicketEditForm = (ticket: Ref<TicketById | undefined>) => {
           contact: recipientContact,
           multiple: true,
         },
-        triggerFormUpdater: false,
       },
       {
         if: '$fns.includes($currentArticleType.attributes, "subject")',
@@ -143,9 +140,6 @@ export const useTicketEditForm = (ticket: Ref<TicketById | undefined>) => {
         name: 'security',
         label: __('Security'),
         type: 'security',
-        props: {
-          // TODO ...
-        },
         triggerFormUpdater: false,
       },
       {

app/models/form_updater/concerns/has_security_options.rb

@@ -21,7 +21,7 @@ module FormUpdater::Concerns::HasSecurityOptions
   end
 
   def email_channel?
-    data['articleSenderType'] == 'email-out'
+    data['articleSenderType'] == 'email-out' || data.dig('article', 'articleType') == 'email'
   end
 
   def agent?
@@ -61,7 +61,7 @@ module FormUpdater::Concerns::HasSecurityOptions
   end
 
   def smime_encryption?
-    return false if !data['customer_id'] && !data['cc']
+    return false if !data['customer_id'] && !data['cc'] && !data.dig('article', 'to') && !data.dig('article', 'cc')
 
     recipients = verified_recipient_addresses
     return false if recipients.blank?
@@ -70,43 +70,46 @@ module FormUpdater::Concerns::HasSecurityOptions
   end
 
   def recipient_addresses
-    result = []
+    customer_recipient + target_recipients + additional_recipients
+  end
 
-    if data['customer_id'].present?
-      customer = ::User.find_by(id: data['customer_id'])
+  def customer_recipient
+    return [] if data['customer_id'].nil?
 
-      if customer && customer.email.present?
-        result.push(customer.email)
-      end
-    end
+    customer = ::User.find_by(id: data['customer_id'])
 
-    if data['cc'].present?
-      result.push(data['cc'])
-    end
+    return [] if !customer || customer.email.empty?
 
-    result
+    [customer.email]
   end
 
-  def verified_recipient_addresses
+  def target_recipients
+    return [] if data.dig('article', 'to').nil?
+
+    data['article']['to']
+  end
+
+  def additional_recipients
     result = []
 
-    list = Mail::AddressList.new(recipient_addresses.compact.join(',').to_s)
-    list.addresses.each do |address|
-      result.push address.address
-    end
+    result.concat data['cc'] if data['cc'].present?
+    result.concat data['article']['cc'] if data.dig('article', 'cc').present?
 
     result
   end
 
+  def verified_recipient_addresses
+    list = Mail::AddressList.new(recipient_addresses.compact.join(',').to_s)
+    list.addresses.map(&:address)
+  end
+
   def recipients_have_valid_certificate?(recipients)
     result = false
 
     begin
       certs = SMIMECertificate.for_recipipent_email_addresses!(recipients)
 
-      if certs
-        result = certs.none?(&:expired?)
-      end
+      result = certs.none?(&:expired?) if certs
     rescue
       result = false
     end
@@ -131,9 +134,7 @@ module FormUpdater::Concerns::HasSecurityOptions
       from = list.addresses.first.to_s
       cert = SMIMECertificate.for_sender_email_address(from)
 
-      if cert
-        result = !cert.expired?
-      end
+      result = !cert.expired? if cert
     rescue
       result = false
     end

app/models/form_updater/updater/ticket/edit.rb

@@ -2,6 +2,7 @@
 
 class FormUpdater::Updater::Ticket::Edit < FormUpdater::Updater
   include FormUpdater::Concerns::ChecksCoreWorkflow
+  include FormUpdater::Concerns::HasSecurityOptions
 
   core_workflow_screen 'edit'
 

spec/models/form_updater/concerns/has_security_options_examples.rb

@@ -1,11 +1,20 @@
 # Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
-RSpec.shared_examples 'HasSecurityOptions' do
+RSpec.shared_examples 'HasSecurityOptions' do |type:|
   context 'with security options' do
     let(:base_data) do
-      {
-        'articleSenderType' => 'email-out',
-      }
+      case type
+      when 'create'
+        {
+          'articleSenderType' => 'email-out',
+        }
+      when 'edit'
+        {
+          'article' => {
+            'articleType' => 'email',
+          },
+        }
+      end
     end
     let(:data) { base_data }
 
@@ -40,14 +49,32 @@ RSpec.shared_examples 'HasSecurityOptions' do
       it_behaves_like 'not resolving security field'
     end
 
-    context 'without articleSenderType present' do
-      let(:data) { base_data.tap { |data| data.delete('articleSenderType') } }
+    context 'without article type present' do
+      let(:data) do
+        base_data.tap do |data|
+          case type
+          when 'create'
+            data.delete('articleSenderType')
+          when 'edit'
+            data['article'].delete('articleType')
+          end
+        end
+      end
 
       it_behaves_like 'not resolving security field'
     end
 
-    context 'with phone articleSenderType present' do
-      let(:data) { base_data.tap { |data| data['articleSenderType'] = 'phone-out' } }
+    context 'with phone article type present' do
+      let(:data) do
+        base_data.tap do |data|
+          case type
+          when 'create'
+            data['articleSenderType'] = 'phone-out'
+          when 'edit'
+            data['article']['articleType'] = 'phone'
+          end
+        end
+      end
 
       it_behaves_like 'not resolving security field'
     end
@@ -58,10 +85,19 @@ RSpec.shared_examples 'HasSecurityOptions' do
       it_behaves_like 'not resolving security field'
     end
 
-    context 'with customer_id present' do
+    context 'with recipient present' do
       let(:recipient_email_address) { 'smime2@example.com' }
       let(:customer)                { create(:customer, email: recipient_email_address) }
-      let(:data)                    { base_data.tap { |data| data['customer_id'] = customer.id.to_s } }
+      let(:data)                    do
+        base_data.tap do |data|
+          case type
+          when 'create'
+            data['customer_id'] = customer.id.to_s
+          when 'edit'
+            data['article']['to'] = [customer.email]
+          end
+        end
+      end
 
       it_behaves_like 'resolving security field', expected_result: { allowed: [], value: [] }
 
@@ -74,9 +110,18 @@ RSpec.shared_examples 'HasSecurityOptions' do
       end
     end
 
-    context 'with cc present' do
+    context 'with additional recipient present' do
       let(:recipient_email_address) { 'smime3@example.com' }
-      let(:data) { base_data.tap { |data| data['cc'] = recipient_email_address } }
+      let(:data) do
+        base_data.tap do |data|
+          case type
+          when 'create'
+            data['cc'] = [recipient_email_address]
+          when 'edit'
+            data['article']['cc'] = [recipient_email_address]
+          end
+        end
+      end
 
       it_behaves_like 'resolving security field', expected_result: { allowed: [], value: [] }
 
@@ -95,14 +140,20 @@ RSpec.shared_examples 'HasSecurityOptions' do
       end
     end
 
-    context 'with both customer_id and cc present' do
+    context 'with both recipient and additional recipient present' do
       let(:recipient_email_address1) { 'smime2@example.com' }
       let(:recipient_email_address2) { 'smime3@example.com' }
       let(:customer)                 { create(:customer, email: recipient_email_address1) }
       let(:data) do
         base_data.tap do |data|
-          data['customer_id'] = customer.id.to_s
-          data['cc'] = recipient_email_address2
+          case type
+          when 'create'
+            data['customer_id'] = customer.id.to_s
+            data['cc'] = [recipient_email_address2]
+          when 'edit'
+            data['article']['to'] = [customer.email]
+            data['article']['cc'] = [recipient_email_address2]
+          end
         end
       end
 
@@ -126,7 +177,7 @@ RSpec.shared_examples 'HasSecurityOptions' do
       end
     end
 
-    context 'with group_id present' do
+    context 'with group present' do
       let(:data) { base_data.tap { |data| data['group_id'] = group.id } }
 
       it_behaves_like 'resolving security field', expected_result: { allowed: [], value: [] }
@@ -148,7 +199,7 @@ RSpec.shared_examples 'HasSecurityOptions' do
       end
     end
 
-    context 'with customer_id and group_id present' do
+    context 'with recipient and group present' do
       let(:recipient_email_address) { 'smime2@example.com' }
       let(:system_email_address)    { 'smime1@example.com' }
       let(:customer)                { create(:customer, email: recipient_email_address) }
@@ -156,8 +207,14 @@ RSpec.shared_examples 'HasSecurityOptions' do
       let(:group)                   { create(:group, email_address: email_address) }
       let(:data) do
         base_data.tap do |data|
-          data['customer_id'] = customer.id.to_s
-          data['group_id'] = group.id
+          case type
+          when 'create'
+            data['customer_id'] = customer.id.to_s
+            data['group_id'] = group.id
+          when 'edit'
+            data['article']['to'] = [customer.email]
+            data['group_id'] = group.id
+          end
         end
       end
 

spec/models/form_updater/updater/ticket/create_spec.rb

@@ -64,5 +64,5 @@ RSpec.describe(FormUpdater::Updater::Ticket::Create) do
   end
 
   include_examples 'ChecksCoreWorkflow', object_name: 'Ticket'
-  include_examples 'HasSecurityOptions'
+  include_examples 'HasSecurityOptions', type: 'create'
 end

spec/models/form_updater/updater/ticket/edit_spec.rb

@@ -0,0 +1,68 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+require 'models/form_updater/concerns/checks_core_workflow_examples'
+require 'models/form_updater/concerns/has_security_options_examples'
+
+RSpec.describe(FormUpdater::Updater::Ticket::Edit) do
+  subject(:resolved_result) do
+    described_class.new(
+      context:         context,
+      relation_fields: relation_fields,
+      meta:            meta,
+      data:            data,
+      id:              nil
+    )
+  end
+
+  let(:group)   { create(:group) }
+  let(:user)    { create(:agent, groups: [group]) }
+  let(:context) { { current_user: user } }
+  let(:meta)    { { initial: true, form_id: 12_345 } }
+  let(:data)    { {} }
+
+  let(:relation_fields) do
+    [
+      {
+        name:     'group_id',
+        relation: 'group',
+      },
+      {
+        name:     'state_id',
+        relation: 'TicketState',
+      },
+      {
+        name:     'priority_id',
+        relation: 'TicketPriority',
+      },
+    ]
+  end
+
+  let(:expected_result) do
+    {
+      'group_id'    => {
+        options: [ { value: group.id, label: group.name } ],
+      },
+      'state_id'    => {
+        options: Ticket::State.by_category(:viewable_agent_edit).order(name: :asc).map { |state| { value: state.id, label: state.name } },
+      },
+      'priority_id' => {
+        options: Ticket::Priority.where(active: true).order(id: :asc).map { |priority| { value: priority.id, label: priority.name } },
+      },
+    }
+  end
+
+  context 'when resolving' do
+    it 'returns all resolved relation fields with correct value + label' do
+      expect(resolved_result.resolve).to include(
+        'group_id'    => include(expected_result['group_id']),
+        'state_id'    => include(expected_result['state_id']),
+        'priority_id' => include(expected_result['priority_id']),
+      )
+    end
+  end
+
+  include_examples 'ChecksCoreWorkflow', object_name: 'Ticket'
+  include_examples 'HasSecurityOptions', type: 'edit'
+end

spec/system/apps/mobile/tickets/ticket_create_security_spec.rb

@@ -1,7 +1,6 @@
 # Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
 
 require 'rails_helper'
-require 'system/apps/mobile/examples/core_workflow_examples'
 
 RSpec.describe 'Mobile > Ticket > Create with security options', app: :mobile, authenticated_as: :authenticate, type: :system do
   let(:group)     { Group.find_by(name: 'Users') }

spec/system/apps/mobile/tickets/ticket_email_reply_security_spec.rb

@@ -0,0 +1,172 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Mobile > Ticket > Email reply with security options', app: :mobile, authenticated_as: :authenticate, type: :system do
+  let(:group)    { Group.find_by(name: 'Users') }
+  let(:agent)    { create(:agent, groups: [group]) }
+  let(:customer) { create(:customer) }
+  let(:ticket)   { create(:ticket, group: group, customer: customer) }
+  let(:article)  { create(:ticket_article, ticket: ticket, from: customer.email, to: group.email_address.email) }
+
+  def authenticate
+    Setting.set('smime_integration', true)
+    Setting.set('smime_config', smime_config) if defined?(smime_config)
+
+    article
+    agent
+  end
+
+  def prepare_phone_reply
+    find_button('Article actions').click
+    find_button('Reply').click
+
+    within_form(form_updater_gql_number: 1) do
+      find_select('Article Type', visible: :all).select_option('Phone')
+    end
+  end
+
+  def prepare_email_reply(with_body: false)
+    find_button('Article actions').click
+    find_button('Reply').click
+
+    return if !with_body
+
+    find_editor('Text').type(Faker::Hacker.say_something_smart)
+  end
+
+  def save_ticket
+    find_button('Done').click
+    find_button('Save ticket').click
+
+    wait_for_gql('apps/mobile/pages/ticket/graphql/mutations/update.graphql')
+  end
+
+  before do
+    visit "/tickets/#{ticket.id}"
+
+    wait_for_gql('apps/mobile/pages/ticket/graphql/queries/ticket/articles.graphql')
+    wait_for_form_to_settle('form-ticket-edit')
+  end
+
+  shared_examples 'having available security options' do |encrypt:, sign:|
+    it "available security options - encrypt: #{encrypt}, sign: #{sign}" do
+      prepare_email_reply
+
+      expect { find_outer('Security') }.not_to raise_error
+      expect(find_button('Encrypt', disabled: !encrypt).disabled?).to be(!encrypt)
+      expect(find_button('Sign', disabled: !sign).disabled?).to be(!sign)
+    end
+  end
+
+  shared_examples 'replying via email' do |encrypt:, sign:|
+    it "can reply via email - encrypt: #{encrypt}, sign: #{sign}" do
+      prepare_email_reply with_body: true
+      save_ticket
+
+      find('[role=alert]', text: 'Ticket updated successfully.')
+
+      expect(Ticket.last.articles.last.preferences['security']['encryption']['success']).to be(encrypt)
+      expect(Ticket.last.articles.last.preferences['security']['sign']['success']).to be(sign)
+    end
+  end
+
+  context 'without certificates present' do
+    it_behaves_like 'having available security options', encrypt: false, sign: false
+    it_behaves_like 'replying via email', encrypt: false, sign: false
+  end
+
+  context 'with sender certificate present' do
+    let(:system_email_address) { 'smime1@example.com' }
+    let(:email_address)        { create(:email_address, email: system_email_address) }
+    let(:group)                { create(:group, email_address: email_address) }
+
+    before do
+      create(:smime_certificate, :with_private, fixture: system_email_address)
+    end
+
+    it_behaves_like 'having available security options', encrypt: false, sign: true
+    it_behaves_like 'replying via email', encrypt: false, sign: true
+
+    context 'with recipient certificate present' do
+      let(:recipient_email_address) { 'smime2@example.com' }
+      let(:customer)                { create(:customer, email: recipient_email_address) }
+
+      before do
+        create(:smime_certificate, fixture: recipient_email_address)
+      end
+
+      it_behaves_like 'having available security options', encrypt: true, sign: true
+      it_behaves_like 'replying via email', encrypt: true, sign: true
+
+      it 'hides the security field for phone articles' do
+        prepare_phone_reply
+
+        expect(page).to have_no_css('label', text: 'Security')
+      end
+
+      context 'with default group configuration' do
+        let(:smime_config) do
+          {
+            'group_id' => group_defaults
+          }
+        end
+
+        let(:group_defaults) do
+          {
+            'default_encryption' => {
+              group.id.to_s => default_encryption,
+            },
+            'default_sign'       => {
+              group.id.to_s => default_sign,
+            }
+          }
+        end
+
+        let(:default_sign)       { true }
+        let(:default_encryption) { true }
+
+        shared_examples 'having default security options' do |encrypt:, sign:|
+          it "default security options - encrypt: #{encrypt}, sign: #{sign}" do
+            prepare_email_reply
+
+            expect(find_button('Encrypt')['aria-selected']).to eq(encrypt.to_s)
+            expect(find_button('Sign')['aria-selected']).to eq(sign.to_s)
+          end
+        end
+
+        it_behaves_like 'having default security options', encrypt: true, sign: true
+
+        context 'when it has no value' do
+          let(:group_defaults) { {} }
+
+          it_behaves_like 'having default security options', encrypt: true, sign: true
+        end
+
+        context 'when signing is disabled' do
+          let(:default_sign) { false }
+
+          it_behaves_like 'having default security options', encrypt: true, sign: false
+        end
+
+        context 'when encryption is disabled' do
+          let(:default_encryption) { false }
+
+          it_behaves_like 'having default security options', encrypt: false, sign: true
+        end
+      end
+    end
+  end
+
+  context 'with recipient certificate present' do
+    let(:recipient_email_address) { 'smime2@example.com' }
+    let(:customer)                { create(:customer, email: recipient_email_address) }
+
+    before do
+      create(:smime_certificate, fixture: recipient_email_address)
+    end
+
+    it_behaves_like 'having available security options', encrypt: true, sign: false
+    it_behaves_like 'replying via email', encrypt: true, sign: false
+  end
+end