From 92d19a6221b10e7895bc6f2a47e13aacd4e56bc0 Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@zammad.com>
Date: Fri, 8 Apr 2022 14:54:05 +0200
Subject: [PATCH] Fixes #4029 - Cannot encrypt if multiple S/MIME certificates
 exist and one is expired.

---
 app/models/smime_certificate.rb       |  8 +++++---
 spec/models/smime_certificate_spec.rb | 23 ++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/app/models/smime_certificate.rb b/app/models/smime_certificate.rb
index 1d0e6ebee..dda513ac7 100644
--- a/app/models/smime_certificate.rb
+++ b/app/models/smime_certificate.rb
@@ -1,6 +1,8 @@
 # Copyright (C) 2012-2022 Zammad Foundation, https://zammad-foundation.org/
 
 class SMIMECertificate < ApplicationModel
+  default_scope { order('not_after_at DESC, not_before_at DESC, id DESC') }
+
   validates :fingerprint, uniqueness: { case_sensitive: true }
 
   def self.parts(raw)
@@ -38,8 +40,8 @@ class SMIMECertificate < ApplicationModel
   # @return [SMIMECertificate, nil] The found certificate record or nil
   def self.for_sender_email_address(address)
     downcased_address = address.downcase
-    where.not(private_key: nil).find_each.detect do |certificate|
-      certificate.email_addresses.include?(downcased_address)
+    where.not(private_key: nil).all.as_batches do |certificate|
+      return certificate if certificate.email_addresses.include?(downcased_address)
     end
   end
 
@@ -55,7 +57,7 @@ class SMIMECertificate < ApplicationModel
   def self.for_recipipent_email_addresses!(addresses)
     certificates        = []
     remaining_addresses = addresses.map(&:downcase)
-    find_each do |certificate|
+    all.as_batches do |certificate|
 
       # intersection of both lists
       cerfiticate_for = certificate.email_addresses & remaining_addresses
diff --git a/spec/models/smime_certificate_spec.rb b/spec/models/smime_certificate_spec.rb
index 1b4e75270..5382b7f85 100644
--- a/spec/models/smime_certificate_spec.rb
+++ b/spec/models/smime_certificate_spec.rb
@@ -107,7 +107,7 @@ RSpec.describe SMIMECertificate, type: :model do
       end
 
       it 'returns certificates' do
-        expect(described_class.for_recipipent_email_addresses!(lookup_addresses)).to eq(certificates)
+        expect(described_class.for_recipipent_email_addresses!(lookup_addresses)).to include(*certificates)
       end
     end
 
@@ -196,4 +196,25 @@ RSpec.describe SMIMECertificate, type: :model do
   it 'ensures uniqueness of records' do
     expect { create_list(:smime_certificate, 2, fixture: 'smime1@example.com') }.to raise_error(ActiveRecord::RecordInvalid, %r{Validation failed})
   end
+
+  describe 'Cannot encrypt if multiple S/MIME certificates exist and one is expired #4029' do
+    let(:lookup_address) { 'smime1@example.com' }
+
+    before do
+      create(:smime_certificate, :with_private, fixture: lookup_address, not_before_at: '2021-04-07', not_after_at: '2021-04-07', fingerprint: 'A')
+      create(:smime_certificate, :with_private, fixture: lookup_address, not_before_at: '2022-04-07', not_after_at: '2042-04-07', fingerprint: 'B')
+    end
+
+    describe '.for_sender_email_address' do
+      it 'does return the latest certificate when there is also an old expired certificate' do
+        expect(described_class.for_sender_email_address('smime1@example.com').fingerprint).to eq('B')
+      end
+    end
+
+    describe '.for_recipipent_email_addresses!' do
+      it 'does return the latest certificate when there is also an old expired certificate' do
+        expect(described_class.for_recipipent_email_addresses!(['smime1@example.com']).first.fingerprint).to eq('B')
+      end
+    end
+  end
 end