Fixes #4029 - Cannot encrypt if multiple S/MIME certificates exist and one is expired.

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|
+    where.not(private_key: nil).all.as_batches do |certificate|
-      certificate.email_addresses.include?(downcased_address)
+      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

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