lawrencepit · x4d3 · Mar 21, 2017
diff --git a/lib/saml_idp/controller.rb b/lib/saml_idp/controller.rb
@@ -1,4 +1,5 @@
 # encoding: utf-8
+require 'logger'
 
 module SamlIdp
   module Controller
@@ -54,18 +55,25 @@ def decode_SAMLRequest(saml_request)
         @saml_request = zstream.inflate(Base64.decode64(saml_request))
         zstream.finish
         zstream.close
-        @saml_request_id = @saml_request[/ID=['"](.+?)['"]/, 1]
-        @saml_acs_url = @saml_request[/AssertionConsumerServiceURL=['"](.+?)['"]/, 1]
+        xml_doc  = Nokogiri::XML(@saml_request)do |config|
+          # Strict parsing; raise an error when parsing malformed documents
+          config.strict
+        end
+        auth_request = xml_doc.xpath('//samlp:AuthnRequest').first
+        @saml_request_id = auth_request['ID']
+        @saml_acs_url = auth_request['AssertionConsumerServiceURL']
       end
 
       def encode_SAMLResponse(nameID, opts = {})
         now = Time.now.utc
+        encoded_saml_acs_url = @saml_acs_url.encode(xml: :attr)
+
         response_id, reference_id = SecureRandom.uuid, SecureRandom.uuid
         audience_uri = opts[:audience_uri] || saml_acs_url[/^(.*?\/\/.*?\/)/, 1]
         issuer_uri = opts[:issuer_uri] || (defined?(request) && request.url) || "http://example.com"
         attributes_statement = attributes(opts[:attributes_provider], nameID)
 
-        assertion = %[<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_#{reference_id}" IssueInstant="#{now.iso8601}" Version="2.0"><saml:Issuer Format="urn:oasis:names:SAML:2.0:nameid-format:entity">#{issuer_uri}</saml:Issuer><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">#{nameID}</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData InResponseTo="#{@saml_request_id}" NotOnOrAfter="#{(now+3*60).iso8601}" Recipient="#{@saml_acs_url}"></saml:SubjectConfirmationData></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="#{(now-5).iso8601}" NotOnOrAfter="#{(now+60*60).iso8601}"><saml:AudienceRestriction><saml:Audience>#{audience_uri}</saml:Audience></saml:AudienceRestriction></saml:Conditions>#{attributes_statement}<saml:AuthnStatement AuthnInstant="#{now.iso8601}" SessionIndex="_#{reference_id}"><saml:AuthnContext><saml:AuthnContextClassRef>urn:federation:authentication:windows</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement></saml:Assertion>]
+        assertion = %[<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="_#{reference_id}" IssueInstant="#{now.iso8601}" Version="2.0"><saml:Issuer Format="urn:oasis:names:SAML:2.0:nameid-format:entity">#{issuer_uri}</saml:Issuer><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">#{nameID}</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData InResponseTo="#{@saml_request_id}" NotOnOrAfter="#{(now+3*60).iso8601}" Recipient=#{encoded_saml_acs_url}></saml:SubjectConfirmationData></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="#{(now-5).iso8601}" NotOnOrAfter="#{(now+60*60).iso8601}"><saml:AudienceRestriction><saml:Audience>#{audience_uri}</saml:Audience></saml:AudienceRestriction></saml:Conditions>#{attributes_statement}<saml:AuthnStatement AuthnInstant="#{now.iso8601}" SessionIndex="_#{reference_id}"><saml:AuthnContext><saml:AuthnContextClassRef>urn:federation:authentication:windows</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement></saml:Assertion>]
 
         digest_value = Base64.encode64(algorithm.digest(assertion)).gsub(/\n/, '')
 
@@ -77,7 +85,7 @@ def encode_SAMLResponse(nameID, opts = {})
 
         assertion_and_signature = assertion.sub(/Issuer\>\<saml:Subject/, "Issuer>#{signature}<saml:Subject")
 
-        xml = %[<samlp:Response ID="_#{response_id}" Version="2.0" IssueInstant="#{now.iso8601}" Destination="#{@saml_acs_url}" Consent="urn:oasis:names:tc:SAML:2.0:consent:unspecified" InResponseTo="#{@saml_request_id}" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"><saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">#{issuer_uri}</saml:Issuer><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" /></samlp:Status>#{assertion_and_signature}</samlp:Response>]
+        xml = %[<samlp:Response ID="_#{response_id}" Version="2.0" IssueInstant="#{now.iso8601}" Destination=#{encoded_saml_acs_url} Consent="urn:oasis:names:tc:SAML:2.0:consent:unspecified" InResponseTo="#{@saml_request_id}" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"><saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">#{issuer_uri}</saml:Issuer><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success" /></samlp:Status>#{assertion_and_signature}</samlp:Response>]
 
         Base64.encode64(xml)
       end

diff --git a/spec/saml_idp/controller_spec.rb b/spec/saml_idp/controller_spec.rb
@@ -7,12 +7,41 @@
   def params
     @params ||= {}
   end
+  SAML_ACS_URLS = %w(https://example.com/saml/consume https://example.com/saml/consume?toto=value&tata=value2)
 
-  it "should find the SAML ACS URL" do
-    requested_saml_acs_url = "https://example.com/saml/consume"
-    params[:SAMLRequest] = make_saml_request(requested_saml_acs_url)
+  SAML_ACS_URLS.each do |requested_saml_acs_url|
+    it "should find the SAML ACS URL: #{requested_saml_acs_url}" do
+      params[:SAMLRequest] = make_saml_request(requested_saml_acs_url)
+      validate_saml_request
+      expect(saml_acs_url).to eq(requested_saml_acs_url)
+    end
+  end
+
+  it 'should find the SAML ACS URL' do
+    xml = %q(
+      <samlp:ArtifactResponse xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
+         <samlp:AuthnRequest ID="_306f8ec5b618f361c70b6ffb1480eade" AssertionConsumerServiceURL="https://sp.example.com/SAML2/SSO/Artifact" />
+      </samlp:ArtifactResponse>
+    )
+    params[:SAMLRequest] =  prepare_saml_request(xml)
     validate_saml_request
-    expect(saml_acs_url).to eq(requested_saml_acs_url)
+    expect(saml_acs_url).to eq('https://sp.example.com/SAML2/SSO/Artifact')
+  end
+
+  it 'does not validate wrong requests' do
+    params[:SAMLRequest] = 'FAKE NEWS'
+    expect{validate_saml_request}.to raise_error
+  end
+
+  it 'does not validate wrong xmls' do
+    xml = %q(
+      <samlp:ArtifactResponse xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
+         <samlp:AuthnRequest ID="_306f8ec5b618f361c70b6ffb1480eade" AssertionConsumerServiceURL="https://sp.example.com/SAML2/SSO/Artifact?wrongparam=titi&wrongcaract=titi" />
+      </samlp:ArtifactResponse>
+    )
+
+    params[:SAMLRequest] = prepare_saml_request(xml)
+    expect{validate_saml_request}.to raise_error
   end
 
   context "SAML Responses" do
@@ -54,4 +83,20 @@ def params
       end
     end
   end
+  context "SAML Responses with special characters" do
+    before(:each) do
+      params[:SAMLRequest] = make_saml_request('https://example.com/saml/consume?toto=value&tata=value2')
+      validate_saml_request
+    end
+    it "should create a SAML Response" do
+      saml_response = encode_SAMLResponse("foo@example.com")
+      response = OneLogin::RubySaml::Response.new(saml_response)
+      expect(response.name_id).to eq("foo@example.com")
+      expect(response.issuer).to eq("http://example.com")
+      response.settings = saml_settings
+      expect(response.is_valid?).to be true
+    end
+  end
+
+
 end
diff --git a/spec/support/saml_request_macros.rb b/spec/support/saml_request_macros.rb
@@ -16,4 +16,17 @@ def saml_settings(options = {})
     settings
   end
 
+  def prepare_saml_request(xml)
+    deflated = deflate(xml)
+    encode(deflated)
+  end
+
+  def encode(encoded)
+    Base64.encode64(encoded).gsub(/\n/, "")
+  end
+
+  def deflate(inflated)
+    Zlib::Deflate.deflate(inflated, 9)[2..-5]
+  end
+
 end