LG-12085 Returns 400 when blank certificate element is passed through assertion

app/controllers/concerns/saml_idp_auth_concern.rb

@@ -5,7 +5,7 @@ module SamlIdpAuthConcern
 
   included do
     # rubocop:disable Rails/LexicallyScopedActionFilter
-    before_action :validate_saml_request, only: :auth
+    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
     before_action :log_external_saml_auth_request, only: [:auth]
@@ -45,21 +45,29 @@ def check_sp_active
   end
 
   def validate_service_provider_and_authn_context
-    @saml_request_validator = SamlRequestValidator.new
+    return if result.success?
+
+    analytics.saml_auth(
+      **result.to_h.merge(request_signed: saml_request.signed?),
+    )
+    render 'saml_idp/auth/error', status: :bad_request
+  end
 
-    @result = @saml_request_validator.call(
+  def result
+    @result ||= @saml_request_validator.call(
       service_provider: saml_request_service_provider,
       authn_context: requested_authn_contexts,
       authn_context_comparison: saml_request.requested_authn_context_comparison,
       nameid_format: name_id_format,
     )
+  end
 
-    return if @result.success?
-
-    analytics.saml_auth(
-      **@result.to_h.merge(request_signed: saml_request.signed?),
-    )
-    render 'saml_idp/auth/error', status: :bad_request
+  def validate_and_create_saml_request_object
+    # this saml_idp method creates the saml_request object used for validations
+    validate_saml_request
+    @saml_request_validator = SamlRequestValidator.new
+  rescue SamlIdp::XMLSecurity::SignedDocument::ValidationError
+    @saml_request_validator = SamlRequestValidator.new(blank_cert: true)
   end
 
   def name_id_format

app/controllers/saml_idp_controller.rb

@@ -118,7 +118,7 @@ def pii_requested_but_locked?
   end
 
   def capture_analytics
-    analytics_payload = @result.to_h.merge(
+    analytics_payload = result.to_h.merge(
       endpoint: api_saml_auth_path(path_year: params[:path_year]),
       idv: identity_needs_verification?,
       finish_profile: user_has_pending_profile?,

app/services/saml_request_validator.rb

@@ -1,10 +1,15 @@
 class SamlRequestValidator
   include ActiveModel::Model
 
+  validate :cert_exists
   validate :authorized_service_provider
   validate :authorized_authn_context
   validate :authorized_email_nameid_format
 
+  def initialize(blank_cert: false)
+    @blank_cert = blank_cert
+  end
+
   def call(service_provider:, authn_context:, nameid_format:, authn_context_comparison: nil)
     self.service_provider = service_provider
     self.authn_context = Array(authn_context)
@@ -46,6 +51,12 @@ def authorized_authn_context
     end
   end
 
+  def cert_exists
+    if @blank_cert
+      errors.add(:service_provider, :blank_cert_element_req, type: :blank_cert_element_req)
+    end
+  end
+
   def valid_authn_context?
     valid_contexts = Saml::Idp::Constants::VALID_AUTHN_CONTEXTS.dup
     valid_contexts += Saml::Idp::Constants::PASSWORD_AUTHN_CONTEXT_CLASSREFS if step_up_comparison?

config/locales/errors/en.yml

@@ -50,6 +50,7 @@ en:
     messages:
       already_confirmed: was already confirmed, please try signing in
       blank: Please fill in this field.
+      blank_cert_element_req: We cannot detect a certificate in your request.
       confirmation_code_incorrect: Incorrect verification code. Did you type it in correctly?
       confirmation_invalid_token: Invalid confirmation link. Either the link expired
         or you already confirmed your account.

config/locales/errors/es.yml

@@ -54,6 +54,7 @@ es:
     messages:
       already_confirmed: ya estaba confirmado, por favor intente iniciar una sesión
       blank: Por favor, rellenar este campo.
+      blank_cert_element_req: No podemos detectar un certificado en su solicitud.
       confirmation_code_incorrect: Código de verificación incorrecto. ¿Lo escribió correctamente?
       confirmation_invalid_token: El enlace de confirmación no es válido. El enlace
         expiró o usted ya ha confirmado su cuenta.

config/locales/errors/fr.yml

@@ -59,6 +59,7 @@ fr:
     messages:
       already_confirmed: a déjà été confirmé, veuillez essayer de vous connecter
       blank: Veuillez remplir ce champ.
+      blank_cert_element_req: Nous ne pouvons pas détecter un certificat sur votre demande.
       confirmation_code_incorrect: Code de vérification incorrect. L’avez-vous saisi correctement ?
       confirmation_invalid_token: Lien de confirmation non valide. Le lien est expiré
         ou vous avez déjà confirmé votre compte.

spec/controllers/saml_idp_controller_spec.rb

@@ -1362,6 +1362,23 @@ def name_id_version(format_urn)
     context 'cert element in SAML request is blank' do
       let(:user) { create(:user, :fully_registered) }
       let(:service_provider) { build(:service_provider, issuer: 'http://localhost:3000') }
+      let(:analytics_hash) do
+        {
+          success: false,
+          errors: { service_provider: ['We cannot detect a certificate in your request.'] },
+          error_details: { service_provider: { blank_cert_element_req: true } },
+          nameid_format: Saml::Idp::Constants::NAME_ID_FORMAT_PERSISTENT,
+          authn_context: [Saml::Idp::Constants::DEFAULT_AAL_AUTHN_CONTEXT_CLASSREF],
+          authn_context_comparison: 'exact',
+          service_provider: 'http://localhost:3000',
+          request_signed: true,
+        }
+      end
+
+      before do
+        stub_analytics
+        allow(@analytics).to receive(:track_event)
+      end
 
       # the RubySAML library won't let us pass an empty string in as the certificate
       # element, so this test substitutes a SAMLRequest that has that element blank
@@ -1395,7 +1412,6 @@ def name_id_version(format_urn)
             <samlp:NameIDPolicy AllowCreate="true" Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"/>
             <samlp:RequestedAuthnContext Comparison="exact">
           <saml:AuthnContextClassRef>urn:gov:gsa:ac:classes:sp:PasswordProtectedTransport:duo</saml:AuthnContextClassRef>
-              <saml:AuthnContextClassRef>http://idmanagement.gov/ns/assurance/ial/1</saml:AuthnContextClassRef>
             </samlp:RequestedAuthnContext>
           </samlp:AuthnRequest>
         XML
@@ -1410,11 +1426,17 @@ def name_id_version(format_urn)
         expect(CGI).to receive(:unescape).and_return deflated_encoded_req
       end
 
-      it 'a ValidationError is raised' do
-        expect { generate_saml_response(user, saml_settings) }.to raise_error(
-          SamlIdp::XMLSecurity::SignedDocument::ValidationError,
-          'Certificate element present in response (ds:X509Certificate) but evaluating to nil',
-        )
+      it 'notes it in the analytics event' do
+        generate_saml_response(user, saml_settings)
+        expect(@analytics).to have_received(:track_event).
+          with('SAML Auth', analytics_hash)
+      end
+
+      it 'returns a 400' do
+        generate_saml_response(user, saml_settings)
+        expect(controller).to render_template('saml_idp/auth/error')
+        expect(response.status).to eq(400)
+        expect(response.body).to include(t('errors.messages.blank_cert_element_req'))
       end
     end
 
@@ -2260,9 +2282,9 @@ def stub_requested_attributes
       expect(subject).to have_actions(
         :before,
         :disable_caching,
-        :validate_saml_request,
-        :validate_service_provider_and_authn_context,
         :store_saml_request,
+        :validate_and_create_saml_request_object,
+        :validate_service_provider_and_authn_context,
       )
     end
   end