Add error for wrong SignatureAlgorithm

.rubocop.yml

@@ -23,6 +23,12 @@ Rails:
 RSpec/Capybara/FeatureMethods:
   Enabled: false
 
+RSpec/RSpec/MultipleMemoizedHelpers:
+  Enabled: false
+
+RSpec/NamedSubject:
+  Enabled: false
+
 Metrics/AbcSize:
   Description: A calculated magnitude based on number of assignments, branches, and
     conditions.

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    saml_idp (0.23.0.pre.18f)
+    saml_idp (0.23.1.pre.18f)
       activesupport
       builder
       faraday

lib/saml_idp/configurator.rb

@@ -10,7 +10,7 @@ class Configurator
     def initialize
       self.x509_certificate = Default::X509_CERTIFICATE
       self.secret_key = Default::SECRET_KEY
-      self.algorithm = :sha1
+      self.algorithm = OpenSSL::Digest::SHA256
       self.reference_id_generator = -> { SecureRandom.uuid }
       self.service_provider = OpenStruct.new
       service_provider.finder = ->(_) { Default::SERVICE_PROVIDER }

lib/saml_idp/request.rb

@@ -71,10 +71,6 @@ def force_authn?
       request['ForceAuthn'] == 'true'
     end
 
-    def requested_authn_context
-      authn_context_node.content if authn_request? && authn_context_node
-    end
-
     def requested_authn_context_comparison
       requested_authn_context_node['Comparison'] if authn_request? && requested_authn_context_node
     end
@@ -163,6 +159,7 @@ def matching_cert
 
     def cert_errors
       return nil unless signed?
+
       begin
         return nil if matching_cert.present?
       rescue SamlIdp::XMLSecurity::SignedDocument::ValidationError => e

lib/saml_idp/version.rb

@@ -1,3 +1,3 @@
 module SamlIdp
-  VERSION = '0.23.0-18f'.freeze
+  VERSION = '0.23.1-18f'.freeze
 end

lib/saml_idp/xml_security.rb

@@ -56,7 +56,7 @@ def validate(idp_cert_fingerprint, soft = true, options = {})
         cert =
           begin
             OpenSSL::X509::Certificate.new(cert_text)
-          rescue OpenSSL::X509::CertificateError => e
+          rescue OpenSSL::X509::CertificateError
             return false if soft
             raise ValidationError.new(
               'Invalid certificate',
@@ -176,7 +176,6 @@ def validate_doc_embedded_signature(
         # check for inclusive namespaces
         inclusive_namespaces = extract_inclusive_namespaces
 
-
         sig_element = document.at_xpath('//ds:Signature | //Signature', DS_NS)
         signed_info_element = sig_element.at_xpath('./ds:SignedInfo | //SignedInfo', DS_NS)
 
@@ -237,6 +236,15 @@ def verify_signature(base64_cert, sig_alg, signature, canon_string, soft)
         cert = OpenSSL::X509::Certificate.new(cert_text)
         signature_algorithm = algorithm(sig_alg)
 
+        if signature_algorithm != SamlIdp.config.algorithm
+          return false if soft
+
+          raise ValidationError.new(
+            "Signature Algorithm needs to be #{SamlIdp.config.algorithm.new.name}",
+            :wrong_sig_algorithm
+          )
+        end
+
         unless cert.public_key.verify(signature_algorithm.new, signature, canon_string)
           return soft ? false : (raise ValidationError.new('Key validation error',
                                                            :key_validation_error))

spec/acceptance/idp_controller_spec.rb

@@ -1,13 +1,18 @@
-require File.expand_path(File.dirname(__FILE__) + '/acceptance_helper')
+require File.expand_path("#{File.dirname(__FILE__)}/acceptance_helper")
 
 feature 'IdpController' do
   scenario 'Login via default signup page' do
-    saml_request = make_saml_request('http://foo.example.com/saml/consume')
+    saml_request = custom_saml_request(
+      overrides: {
+        assertion_consumer_service_url: 'http://foo.example.com/saml/consume',
+      }
+    )
     visit "/saml/auth?SAMLRequest=#{CGI.escape(saml_request)}"
     fill_in 'Email', with: 'foo@example.com'
     fill_in 'Password', with: 'okidoki'
-    click_button 'Sign in'
-    click_button 'Submit' # simulating onload
+    click_on 'Sign in'
+    click_on 'Submit' # simulating onload
+
     expect(current_url).to eq('http://foo.example.com/saml/consume')
     expect(page).to have_content 'foo@example.com'
   end

spec/lib/saml_idp/configurator_spec.rb

@@ -25,7 +25,7 @@ module SamlIdp
     end
 
     it 'has a valid algorithm' do
-      expect(subject.algorithm).to eq(:sha1)
+      expect(subject.algorithm).to eq(OpenSSL::Digest::SHA256)
     end
 
     it 'has a valid reference_id_generator' do

spec/lib/saml_idp/controller_spec.rb

@@ -12,10 +12,9 @@ def params
   def head(status, options = {}); end
 
   it 'finds the SAML ACS URL' do
-    requested_saml_acs_url = 'https://example.com/saml/consume'
-    params[:SAMLRequest] = make_saml_request(requested_saml_acs_url)
+    params[:SAMLRequest] = custom_saml_request
     validate_saml_request
-    expect(saml_acs_url).to eq(requested_saml_acs_url)
+    expect(saml_acs_url).to eq(saml_settings.assertion_consumer_service_url)
   end
 
   context 'SP-initiated logout w/o embed' do
@@ -34,16 +33,14 @@ def head(status, options = {}); end
     end
 
     it 'respects Logout Request' do
-      request_url = URI.parse(make_sp_logout_request)
-      params.merge!(Rack::Utils.parse_nested_query(request_url.query)).symbolize_keys!
+      params.merge!(custom_logout_request).symbolize_keys!
       decode_request(params[:SAMLRequest])
       expect(saml_request.logout_request?).to eq true
       expect(valid_saml_request?).to eq true
     end
 
     it 'requires Signature be present in params' do
-      request_url = URI.parse(make_sp_logout_request)
-      params.merge!(Rack::Utils.parse_nested_query(request_url.query)).symbolize_keys!
+      params.merge!(custom_logout_request).symbolize_keys!
       params.delete(:Signature)
       decode_request(params[:SAMLRequest])
 
@@ -54,7 +51,7 @@ def head(status, options = {}); end
 
   context 'SAML Responses' do
     before do
-      params[:SAMLRequest] = make_saml_request
+      params[:SAMLRequest] = custom_saml_request
       validate_saml_request
     end
 
@@ -136,7 +133,7 @@ def head(status, options = {}); end
 
   context 'invalid SAML Request' do
     it 'returns headers only with a forbidden status' do
-      params[:SAMLRequest] = make_invalid_saml_request
+      params[:SAMLRequest] = custom_saml_request(overrides: {issuer: ''})
 
       expect(self).to receive(:head).with(:forbidden)
 
@@ -151,7 +148,7 @@ def head(status, options = {}); end
     end
 
     it 'returns headers only with a bad_request status' do
-      params[:SAMLRequest] = make_saml_request
+      params[:SAMLRequest] = custom_saml_request
 
       expect(self).to receive(:head).with(:bad_request)
 

spec/lib/saml_idp/logout_request_builder_spec.rb

@@ -31,7 +31,7 @@ module SamlIdp
       Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do
         slo_request = OneLogin::RubySaml::SloLogoutrequest.new(
           subject.encoded,
-          settings: saml_settings('localhost:3000')
+          settings: saml_settings
         )
         slo_request.soft = false
         expect(slo_request.is_valid?).to eq true

spec/lib/saml_idp/logout_response_builder_spec.rb

@@ -31,7 +31,7 @@ module SamlIdp
       Timecop.travel(Time.zone.local(2010, 6, 1, 13, 0, 0)) do
         logout_response = OneLogin::RubySaml::Logoutresponse.new(
           subject.encoded,
-          saml_settings('localhost:3000')
+          saml_settings
         )
         logout_response.soft = false
         expect(logout_response.validate).to eq true