Working on code coverage

.gitignore

@@ -0,0 +1,2 @@
+coverage/
+spec/support/example_private_key.pem

.rspec

@@ -0,0 +1 @@
+--color

Gemfile

@@ -0,0 +1,7 @@
+source :rubygems
+
+gemspec
+
+group :development do
+  gem 'pry'
+end

Gemfile.lock

@@ -0,0 +1,64 @@
+PATH
+  remote: .
+  specs:
+    omniauth-saml (0.9.1)
+      omniauth (~> 1.0)
+      uuid (~> 2.3)
+      xmlcanonicalizer (= 0.1.1)
+
+GEM
+  remote: http://rubygems.org/
+  specs:
+    coderay (1.0.5)
+    diff-lcs (1.1.3)
+    ffi (1.0.11)
+    guard (1.0.1)
+      ffi (>= 0.5.0)
+      thor (~> 0.14.6)
+    guard-rspec (0.6.0)
+      guard (>= 0.10.0)
+    hashie (1.2.0)
+    macaddr (1.5.0)
+      systemu (>= 2.4.0)
+    method_source (0.7.1)
+    multi_json (1.1.0)
+    omniauth (1.0.3)
+      hashie (~> 1.2)
+      rack
+    pry (0.9.8.4)
+      coderay (~> 1.0.5)
+      method_source (~> 0.7.1)
+      slop (>= 2.4.4, < 3)
+    rack (1.4.1)
+    rack-test (0.6.1)
+      rack (>= 1.0)
+    rspec (2.8.0)
+      rspec-core (~> 2.8.0)
+      rspec-expectations (~> 2.8.0)
+      rspec-mocks (~> 2.8.0)
+    rspec-core (2.8.0)
+    rspec-expectations (2.8.0)
+      diff-lcs (~> 1.1.2)
+    rspec-mocks (2.8.0)
+    simplecov (0.6.1)
+      multi_json (~> 1.0)
+      simplecov-html (~> 0.5.3)
+    simplecov-html (0.5.3)
+    slop (2.4.4)
+    systemu (2.4.2)
+    thor (0.14.6)
+    uuid (2.3.5)
+      macaddr (~> 1.0)
+    xmlcanonicalizer (0.1.1)
+
+PLATFORMS
+  ruby
+
+DEPENDENCIES
+  guard (= 1.0.1)
+  guard-rspec (= 0.6.0)
+  omniauth-saml!
+  pry
+  rack-test (= 0.6.1)
+  rspec (= 2.8)
+  simplecov (= 0.6.1)

Guardfile

@@ -0,0 +1,9 @@
+# A sample Guardfile
+# More info at https://github.com/guard/guard#readme
+
+guard 'rspec', :version => 2 do
+  watch(%r{^spec/.+_spec\.rb$})
+  watch(%r{^lib/(.+)\.rb$})     { |m| "spec/lib/#{m[1]}_spec.rb" }
+  watch('spec/spec_helper.rb')  { "spec" }
+end
+

lib/omniauth/strategies/saml.rb

@@ -20,12 +20,11 @@ def callback_phase
         begin
           response = OmniAuth::Strategies::SAML::AuthResponse.new(request.params['SAMLResponse'])
           response.settings = options
-          response.validate!
 
           @name_id  = response.name_id
           @attributes = response.attributes
 
-          return fail!(:invalid_ticket, 'Invalid SAML Ticket') if @name_id.nil? || @name_id.empty?
+          return fail!(:invalid_ticket, 'Invalid SAML Ticket') if @name_id.nil? || @name_id.empty? || !response.valid?
           super
         rescue ArgumentError => e
           fail!(:invalid_ticket, 'Invalid SAML Response')

lib/omniauth/strategies/saml/auth_request.rb

@@ -35,4 +35,4 @@ def create(settings, params = {})
       end
     end
   end
-end
+end

lib/omniauth/strategies/saml/auth_response.rb

@@ -18,7 +18,7 @@ def initialize(response, options = {})
           self.document = OmniAuth::Strategies::SAML::XMLSecurity::SignedDocument.new(Base64.decode64(response))
         end
 
-        def is_valid?
+        def valid?
           validate(soft = true)
         end
 
@@ -29,47 +29,41 @@ def validate!
         # The value of the user identifier as designated by the initialization request response
         def name_id
           @name_id ||= begin
-            node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id[1,document.signed_element_id.size]}']/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
-            node ||=  REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id[1,document.signed_element_id.size]}']/a:Assertion/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
-            node.nil? ? nil : node.text
+            node = xpath("/p:Response/a:Assertion[@ID='#{signed_element_id}']/a:Subject/a:NameID")
+            node ||=  xpath("/p:Response[@ID='#{signed_element_id}']/a:Assertion/a:Subject/a:NameID")
+            node.nil? ? nil : strip(node.text)
           end
         end
 
         # A hash of all the attributes with the response. Assuming there is only one value for each key
         def attributes
           @attr_statements ||= begin
-            result = {}
-
-            stmt_element = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AttributeStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+            stmt_element = xpath("/p:Response/a:Assertion/a:AttributeStatement")
             return {} if stmt_element.nil?
 
-            stmt_element.elements.each do |attr_element|
-              name  = attr_element.attributes["Name"]
-              value = attr_element.elements.first.text
-
-              result[name] = value
-            end
+            {}.tap do |result|
+              stmt_element.elements.each do |attr_element|
+                name  = attr_element.attributes["Name"]
+                value = strip(attr_element.elements.first.text)
 
-            result.keys.each do |key|
-              result[key.intern] = result[key]
+                result[name] = result[name.to_sym] =  value
+              end
             end
-
-            result
           end
         end
 
         # When this user session should expire at latest
         def session_expires_at
           @expires_at ||= begin
-            node = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+            node = xpath("/p:Response/a:Assertion/a:AuthnStatement")
             parse_time(node, "SessionNotOnOrAfter")
           end
         end
 
         # Conditions (if any) for the assertion to run
         def conditions
           @conditions ||= begin
-            REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id[1,document.signed_element_id.size]}']/a:Conditions", { "p" => PROTOCOL, "a" => ASSERTION })
+            xpath("/p:Response/a:Assertion[@ID='#{signed_element_id}']/a:Conditions")
           end
         end
 
@@ -135,7 +129,20 @@ def parse_time(node, attribute)
           end
         end
 
+        def strip(string)
+          return string unless string
+          string.gsub(/^\s+/, '').gsub(/\s+$/, '')
+        end
+
+        def xpath(path)
+          REXML::XPath.first(document, path, { "p" => PROTOCOL, "a" => ASSERTION })
+        end
+
+        def signed_element_id
+          doc_id = document.signed_element_id
+          doc_id[1, doc_id.size]
+        end
       end
     end
   end
-end
+end

lib/omniauth/strategies/saml/xml_security.rb

@@ -123,4 +123,4 @@ def extract_signed_element_id
 
     end
   end
-end
+end

omniauth-saml.gemspec

@@ -11,9 +11,15 @@ Gem::Specification.new do |gem|
   gem.homepage      = "https://github.com/PracticallyGreen/omniauth-saml"
 
   gem.add_runtime_dependency 'omniauth', '~> 1.0'
-  gem.add_runtime_dependency 'xmlcanonicalizer'
+  gem.add_runtime_dependency 'xmlcanonicalizer', '0.1.1'
   gem.add_runtime_dependency 'uuid', '~> 2.3'
 
+  gem.add_development_dependency 'guard', '1.0.1'
+  gem.add_development_dependency 'guard-rspec', '0.6.0'
+  gem.add_development_dependency 'rspec', '2.8'
+  gem.add_development_dependency 'simplecov', '0.6.1'
+  gem.add_development_dependency 'rack-test', '0.6.1'
+
   gem.files         = ['README.md'] + Dir['lib/**/*.rb']
   gem.test_files    = Dir['spec/**/*.rb']
   gem.require_paths = ["lib"]

spec/omniauth/strategies/saml/auth_request_spec.rb

@@ -0,0 +1,75 @@
+require 'spec_helper'
+
+describe OmniAuth::Strategies::SAML::AuthRequest do
+  describe :create do
+    let(:url) do
+      described_class.new.create(
+        {
+          :idp_sso_target_url => 'example.com',
+          :assertion_consumer_service_url => 'http://example.com/auth/saml/callback',
+          :issuer => 'This is an issuer',
+          :name_identifier_format => 'Some Policy'
+        },
+        {
+          :some_param => 'foo',
+          :some_other => 'bar'
+        }
+      )
+    end
+    let(:saml_request) { url.match(/SAMLRequest=(.*)/)[1] }
+
+    describe "the url" do
+      subject { url }
+
+      it "should contain a SAMLRequest query string param" do
+        subject.should match /^example\.com\?SAMLRequest=/
+      end
+
+      it "should contain any other parameters passed through" do
+        subject.should match /^example\.com\?SAMLRequest=(.*)&some_param=foo&some_other=bar/
+      end
+    end
+
+    describe "the saml request" do
+      subject { saml_request }
+
+      let(:decoded) do
+        cgi_unescaped = CGI.unescape(subject)
+        base64_decoded = Base64.decode64(cgi_unescaped)
+        Zlib::Inflate.new(-Zlib::MAX_WBITS).inflate(base64_decoded)
+      end
+
+      let(:xml) { REXML::Document.new(decoded) }
+      let(:root_element) { REXML::XPath.first(xml, '//samlp:AuthnRequest') }
+
+      it "should contain base64 encoded and zlib deflated xml" do
+        decoded.should match /^<samlp:AuthnRequest/
+      end
+
+      it "should contain a uuid with an underscore in front" do
+        UUID.any_instance.stub(:generate).and_return('MY_UUID')
+
+        root_element.attributes['ID'].should == '_MY_UUID'
+      end
+
+      it "should contain the current time as the IssueInstant" do
+        t = Time.now
+        Time.stub(:now).and_return(t)
+
+        root_element.attributes['IssueInstant'].should == t.utc.iso8601
+      end
+
+      it "should contain the callback url in the settings" do
+        root_element.attributes['AssertionConsumerServiceURL'].should == 'http://example.com/auth/saml/callback'
+      end
+
+      it "should contain the issuer" do
+        REXML::XPath.first(xml, '//saml:Issuer').text.should == 'This is an issuer'
+      end
+
+      it "should contain the name identifier format" do
+        REXML::XPath.first(xml, '//samlp:NameIDPolicy').attributes['Format'].should == 'Some Policy'
+      end
+    end
+  end
+end

spec/omniauth/strategies/saml/auth_response_spec.rb

@@ -0,0 +1,90 @@
+require 'spec_helper'
+
+describe OmniAuth::Strategies::SAML::AuthResponse do
+  let(:xml) { :example_response }
+  subject { described_class.new(load_xml(xml)) }
+
+  describe :initialize do
+    context "when the response is nil" do
+      it "should raise an exception" do
+        expect { described_class.new(nil) }.to raise_error ArgumentError
+      end
+    end
+  end
+
+  describe :name_id do
+    it "should load the name id from the assertion" do
+      subject.name_id.should == 'THISISANAMEID'
+    end
+
+    context "when the response contains the signed_element_id" do
+      let(:xml) { :response_contains_signed_element }
+
+      it "should load the name id from the assertion" do
+        subject.name_id.should == 'THISISANAMEID'
+      end
+    end
+  end
+
+  describe :attributes do
+    it "should return all of the attributes as a hash" do
+      subject.attributes.should == {
+        :forename => 'Steven',
+        :surname => 'Anderson',
+        :address_1 => '24 Made Up Drive',
+        :address_2 => nil,
+        :companyName => 'Test Company Ltd',
+        :postcode => 'XX2 4XX',
+        :city => 'Newcastle',
+        :country => 'United Kingdom',
+        :userEmailID => 'steve@example.com',
+        :county => 'TYNESIDE',
+        :versionID => '1',
+        :bundleID => '1',
+
+        'forename' => 'Steven',
+        'surname' => 'Anderson',
+        'address_1' => '24 Made Up Drive',
+        'address_2' => nil,
+        'companyName' => 'Test Company Ltd',
+        'postcode' => 'XX2 4XX',
+        'city' => 'Newcastle',
+        'country' => 'United Kingdom',
+        'userEmailID' => 'steve@example.com',
+        'county' => 'TYNESIDE',
+        'versionID' => '1',
+        'bundleID' => '1'
+      }
+    end
+
+    context "when no attributes exist in the XML" do
+      let(:xml) { :no_attributes }
+
+      it "should return an empty hash" do
+        subject.attributes.should == {}
+      end
+    end
+  end
+
+  describe :session_expires_at do
+    it "should return the SessionNotOnOrAfter as a Ruby date" do
+      subject.session_expires_at.to_i.should == Time.new(2012, 04, 8, 12, 0, 24, '+00:00').to_i
+    end
+  end
+
+  describe :conditions do
+    it "should return the conditions element from the XML" do
+      subject.conditions.attributes['NotOnOrAfter'].should == '2012-03-08T16:30:01.336Z'
+      subject.conditions.attributes['NotBefore'].should    == '2012-03-08T16:20:01.336Z'
+      REXML::XPath.first(subject.conditions, '//saml:Audience').text.should include 'AUDIENCE'
+    end
+  end
+
+  describe :valid? do
+    it_should_behave_like 'a validating method', true
+  end
+
+  describe :validate! do
+    it_should_behave_like 'a validating method', false
+  end
+end

spec/omniauth/strategies/saml/validation_error_spec.rb

@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe OmniAuth::Strategies::SAML::ValidationError do
+  it { should be_a Exception }
+end