Return error when SAML year is invalid

app/controllers/concerns/saml_idp_auth_concern.rb

@@ -7,6 +7,7 @@ module SamlIdpAuthConcern
 
   included do
     # rubocop:disable Rails/LexicallyScopedActionFilter
+    before_action :validate_year
     before_action :validate_and_create_saml_request_object, only: :auth
     before_action :validate_service_provider_and_authn_context, only: :auth
     before_action :check_sp_active, only: :auth
@@ -21,6 +22,12 @@ module SamlIdpAuthConcern
 
   private
 
+  def validate_year
+    if !SamlEndpoint.valid_year?(params[:path_year])
+      render plain: 'Invalid Year', status: :bad_request
+    end
+  end
+
   def sign_out_if_forceauthn_is_true_and_user_is_signed_in
     if !saml_request.force_authn?
       set_issuer_forced_reauthentication(

app/services/saml_endpoint.rb

@@ -1,12 +1,22 @@
 # frozen_string_literal: true
 
 class SamlEndpoint
+  SAML_YEARS = AppArtifacts.store.members.map(&:to_s).map do |key|
+    regex = /saml_(?<year>\d{4})_(?<key_cert>key|cert)/
+    matches = regex.match(key)
+    matches && matches[:year]
+  end.compact.uniq.freeze
+
   attr_reader :year
 
   def initialize(year)
     @year = year
   end
 
+  def self.valid_year?(year)
+    SAML_YEARS.include?(year)
+  end
+
   def self.suffixes
     endpoint_configs.pluck(:suffix)
   end

spec/controllers/saml_idp_controller_spec.rb

@@ -8,6 +8,36 @@
   let(:path_year) { SamlAuthHelper::PATH_YEAR }
 
   describe '/api/saml/logout' do
+    let(:service_provider) do
+      create(
+        :service_provider,
+        certs: ['sp_sinatra_demo', 'saml_test_sp'],
+        active: true,
+        assertion_consumer_logout_service_url: 'https://example.com',
+      )
+    end
+
+    let(:right_cert_settings) do
+      saml_settings(
+        overrides: {
+          issuer: service_provider.issuer,
+          assertion_consumer_logout_service_url: 'https://example.com',
+        },
+      )
+    end
+
+    let(:wrong_cert_settings) do
+      saml_settings(
+        overrides: {
+          issuer: service_provider.issuer,
+          certificate: File.read(Rails.root.join('certs', 'sp', 'saml_test_sp2.crt')),
+          private_key: OpenSSL::PKey::RSA.new(
+            File.read(Rails.root + 'keys/saml_test_sp2.key'),
+          ).to_pem,
+        },
+      )
+    end
+
     it 'assigns devise session limited failure redirect url' do
       delete :logout, params: { path_year: path_year }
 
@@ -25,6 +55,17 @@
       )
     end
 
+    context 'with invalid year' do
+      let(:path_year) { 1999 }
+
+      it 'returns bad request error' do
+        delete :logout, params: { SAMLRequest: 'foo', path_year: path_year }
+
+        expect(response).to be_bad_request
+        expect(response.body).to eq('Invalid Year')
+      end
+    end
+
     it 'tracks the event when sp initiated' do
       allow(controller).to receive(:saml_request).and_return(FakeSamlLogoutRequest.new)
       stub_analytics
@@ -55,36 +96,6 @@
       )
     end
 
-    let(:service_provider) do
-      create(
-        :service_provider,
-        certs: ['sp_sinatra_demo', 'saml_test_sp'],
-        active: true,
-        assertion_consumer_logout_service_url: 'https://example.com',
-      )
-    end
-
-    let(:right_cert_settings) do
-      saml_settings(
-        overrides: {
-          issuer: service_provider.issuer,
-          assertion_consumer_logout_service_url: 'https://example.com',
-        },
-      )
-    end
-
-    let(:wrong_cert_settings) do
-      saml_settings(
-        overrides: {
-          issuer: service_provider.issuer,
-          certificate: File.read(Rails.root.join('certs', 'sp', 'saml_test_sp2.crt')),
-          private_key: OpenSSL::PKey::RSA.new(
-            File.read(Rails.root + 'keys/saml_test_sp2.key'),
-          ).to_pem,
-        },
-      )
-    end
-
     it 'accepts requests from a correct cert' do
       saml_request = UriService.params(
         OneLogin::RubySaml::Logoutrequest.new.create(right_cert_settings),
@@ -207,21 +218,6 @@
   end
 
   describe '/api/saml/remotelogout' do
-    it 'tracks the event when the saml request is invalid' do
-      stub_analytics
-
-      post :remotelogout, params: { SAMLRequest: 'foo', path_year: path_year }
-
-      expect(@analytics).to have_logged_event('Remote Logout initiated', saml_request_valid: false)
-      expect(@analytics).to have_logged_event(
-        :sp_integration_errors_present,
-        error_details: [:issuer_missing_or_invald, :no_auth_or_logout_request, :invalid_signature],
-        error_types: { saml_request_errors: true },
-        event: :saml_remote_logout_request,
-        integration_exists: false,
-      )
-    end
-
     let(:agency) { create(:agency) }
     let(:service_provider) do
       create(
@@ -318,6 +314,21 @@
       )
     end
 
+    it 'tracks the event when the saml request is invalid' do
+      stub_analytics
+
+      post :remotelogout, params: { SAMLRequest: 'foo', path_year: path_year }
+
+      expect(@analytics).to have_logged_event('Remote Logout initiated', saml_request_valid: false)
+      expect(@analytics).to have_logged_event(
+        :sp_integration_errors_present,
+        error_details: [:issuer_missing_or_invald, :no_auth_or_logout_request, :invalid_signature],
+        error_types: { saml_request_errors: true },
+        event: :saml_remote_logout_request,
+        integration_exists: false,
+      )
+    end
+
     it 'accepts requests with correct cert and correct session index and renders logout response' do
       REDIS_POOL.with { |client| client.flushdb }
       session_accessor = OutOfBandSessionAccessor.new(session_id)
@@ -521,6 +532,15 @@
       expect(response.media_type).to eq 'text/xml'
     end
 
+    context 'with invalid year' do
+      let(:path_year) { 1999 }
+
+      it 'returns bad request error' do
+        expect(response).to be_bad_request
+        expect(response.body).to eq('Invalid Year')
+      end
+    end
+
     it 'contains an EntityDescriptor nodeset' do
       expect(xmldoc.metadata_nodeset.length).to eq(1)
     end
@@ -2139,15 +2159,15 @@ def name_id_version(format_urn)
 
     context 'with missing SAMLRequest params' do
       it 'responds with "403 Forbidden"' do
-        get :auth
+        get :auth, params: { path_year: path_year }
 
         expect(response.status).to eq(403)
       end
     end
 
     context 'with invalid SAMLRequest param' do
       it 'responds with "403 Forbidden"' do
-        get :auth
+        get :auth, params: { path_year: path_year }
 
         expect(response.status).to eq(403)
       end