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