diff --git a/app/services/saml_endpoint.rb b/app/services/saml_endpoint.rb index bd235e52464..ebe3347f266 100644 --- a/app/services/saml_endpoint.rb +++ b/app/services/saml_endpoint.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true class SamlEndpoint - SAML_YEARS = AppArtifacts.store.members.map(&:to_s).map do |key| - regex = /saml_(?\d{4})_(?key|cert)/ - matches = regex.match(key) - matches && matches[:year] - end.compact.uniq.freeze + SAML_YEARS = IdentityConfig.store.saml_endpoint_configs.map do |config| + config.fetch(:suffix).to_s + end.uniq.sort.freeze attr_reader :year @@ -18,29 +16,59 @@ def self.valid_year?(year) end def self.suffixes - endpoint_configs.pluck(:suffix) + SAML_YEARS end - def self.endpoint_configs - IdentityConfig.store.saml_endpoint_configs + def self.build_saml_keys_by_year + SAML_YEARS.each_with_object({}) do |year, map| + config = IdentityConfig.store.saml_endpoint_configs.find do |config| + config[:suffix] == year + end + + key_contents = begin + AppArtifacts.store["saml_#{year}_key"] + rescue NameError + raise "No SAML private key for suffix #{year}" + end + + map[year] = + begin + OpenSSL::PKey::RSA.new( + key_contents, + config.fetch(:secret_key_passphrase), + ) + rescue OpenSSL::PKey::RSAError + raise "SAML key or passphrase for #{year} is invalid" + end + end end - def secret_key - key_contents = begin - AppArtifacts.store["saml_#{year}_key"] - rescue NameError - raise "No SAML private key for suffix #{year}" + def self.build_saml_certs_by_year + SAML_YEARS.each_with_object({}) do |year, map| + x509_cert = + begin + cert_string = AppArtifacts.store["saml_#{year}_cert"] + # Validate string value can be parsed to X509, but store string + OpenSSL::X509::Certificate.new(cert_string) + cert_string + rescue NameError + raise "No SAML certificate for suffix #{year}" + rescue OpenSSL::X509::CertificateError + raise "SAML certificate for #{year} is invalid" + end + map[year] = x509_cert end + end - OpenSSL::PKey::RSA.new( - key_contents, - endpoint_config[:secret_key_passphrase], - ) + def secret_key + SAML_YEAR_SECRET_KEYS.fetch(year) + rescue KeyError + raise "No SAML private key for suffix #{year}" end def x509_certificate - AppArtifacts.store["saml_#{year}_cert"] - rescue NameError + SAML_YEAR_CERTS.fetch(year) + rescue KeyError raise "No SAML certificate for suffix #{year}" end @@ -55,11 +83,7 @@ def saml_metadata ) end - private + SAML_YEAR_CERTS = self.build_saml_certs_by_year.freeze - def endpoint_config - @endpoint_config ||= self.class.endpoint_configs.find do |config| - config[:suffix] == year - end - end + SAML_YEAR_SECRET_KEYS = self.build_saml_keys_by_year.freeze end diff --git a/config/routes.rb b/config/routes.rb index c4362f8cb7b..8d902318366 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -44,7 +44,7 @@ # SAML secret rotation paths constraints(path_year: SamlEndpoint.suffixes) do - get '/api/saml/metadata(:path_year)' => 'saml_idp#metadata', format: false + get '/api/saml/metadata(:path_year)' => 'saml_idp#metadata', format: false, as: :api_saml_metadata match '/api/saml/logout(:path_year)' => 'saml_idp#logout', via: %i[get post delete], as: :api_saml_logout match '/api/saml/remotelogout(:path_year)' => 'saml_idp#remotelogout', via: %i[get post], diff --git a/spec/requests/saml_requests_spec.rb b/spec/requests/saml_requests_spec.rb index d7cffa1a8c1..98b5e463c71 100644 --- a/spec/requests/saml_requests_spec.rb +++ b/spec/requests/saml_requests_spec.rb @@ -95,4 +95,24 @@ end end end + + describe 'GET /api/saml/metadata' do + let(:path_year) { SamlAuthHelper::PATH_YEAR } + + it 'is successful' do + get api_saml_metadata_url(path_year: path_year) + + expect(response).to be_ok + end + + context 'for an unsupported year' do + let(:path_year) { (SamlEndpoint.suffixes.max.to_i + 1).to_s } + + it '404s' do + get api_saml_metadata_url(path_year: path_year) + + expect(response).to be_not_found + end + end + end end diff --git a/spec/services/saml_endpoint_spec.rb b/spec/services/saml_endpoint_spec.rb index 632f86a58e5..a7db6fd36bf 100644 --- a/spec/services/saml_endpoint_spec.rb +++ b/spec/services/saml_endpoint_spec.rb @@ -9,25 +9,64 @@ it 'should list the suffixes that are configured' do result = described_class.suffixes - expect(result).to eq(%w[2025 2024]) + expect(result).to eq(%w[2024 2025]) end end - describe 'endpoint_configs' do - it 'should return an array of parsed endpoint config data' do - result = described_class.endpoint_configs - - expect(result).to eq( - [ - { suffix: '2025', secret_key_passphrase: 'trust-but-verify' }, - { - # rubocop:disable Layout/LineLength - comment: 'this extra year is needed to demonstrate how handling multiple live years works in spec/requests/saml_requests_spec.rb', - # rubocop:enable Layout/LineLength - secret_key_passphrase: 'trust-but-verify', - suffix: '2024', - }, - ], + describe '.build_saml_certs_by_year' do + it 'returns a map with keys based on SAML_YEARS and String values' do + saml_certs_by_year = described_class.build_saml_certs_by_year + expect(saml_certs_by_year.keys.sort).to eq(described_class::SAML_YEARS.sort) + expect(saml_certs_by_year.values).to all be_a(String) + end + + it 'raises exception if the certificate for a year does not exist' do + stub_const('SamlEndpoint::SAML_YEARS', ['2000']) + expect { described_class.build_saml_certs_by_year }.to raise_error( + RuntimeError, + 'No SAML certificate for suffix 2000', + ) + end + + it 'raises exception if the certificate value is invalid' do + cert_year = SamlEndpoint::SAML_YEARS.first + stub_const('SamlEndpoint::SAML_YEARS', [cert_year]) + allow(AppArtifacts.store).to(receive(:[])).with("saml_#{cert_year}_cert").and_return( + 'bad cert', + ) + + expect { described_class.build_saml_certs_by_year }.to raise_error( + RuntimeError, + "SAML certificate for #{cert_year} is invalid", + ) + end + end + + describe '.build_saml_keys_by_year' do + it 'returns a map with keys based on SAML_YEARS and String values' do + saml_keys_by_year = described_class.build_saml_keys_by_year + expect(saml_keys_by_year.keys.sort).to eq(described_class::SAML_YEARS.sort) + expect(saml_keys_by_year.values).to all be_a(OpenSSL::PKey::RSA) + end + + it 'raises exception if the key for a year does not exist' do + stub_const('SamlEndpoint::SAML_YEARS', ['2000']) + expect { described_class.build_saml_keys_by_year }.to raise_error( + RuntimeError, + 'No SAML private key for suffix 2000', + ) + end + + it 'raises exception if the key value is invalid' do + key_year = SamlEndpoint::SAML_YEARS.first + stub_const('SamlEndpoint::SAML_YEARS', [key_year]) + allow(AppArtifacts.store).to(receive(:[])).with("saml_#{key_year}_key").and_return( + 'bad key', + ) + + expect { described_class.build_saml_keys_by_year }.to raise_error( + RuntimeError, + "SAML key or passphrase for #{key_year} is invalid", ) end end @@ -47,14 +86,6 @@ context 'when the key file does not exist' do let(:year) { '_dne' } - before do - allow(SamlEndpoint).to receive(:endpoint_configs).and_return( - [ - { suffix: '_dne', secret_key_passphrase: 'asdf1234' }, - ], - ) - end - it 'raises an error' do expect { subject.secret_key }.to raise_error( 'No SAML private key for suffix _dne',