Skip to content

Commit c7e959e

Browse files
author
Kyle Rose
committed
Adds ability to parse out NotBefore and NotOnOrAfter for responses with signed Response element rather than signed Assertion element. Also abstracts out the retrieval of xpaths from within the signed Response/Assertion.
Also adds tests with Timecop to make sure NotBefore and NotOnOrAfter work, because existing tests failed once Time.now got past the short window of the test response.
1 parent bae45bc commit c7e959e

File tree

4 files changed

+37
-16
lines changed

4 files changed

+37
-16
lines changed

Gemfile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ group :test do
99
gem "rake"
1010
gem "mocha"
1111
gem "nokogiri"
12+
gem "timecop"
1213
end

lib/onelogin/ruby-saml/response.rb

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,14 @@ def validate!
3636
# The value of the user identifier as designated by the initialization request response
3737
def name_id
3838
@name_id ||= begin
39-
node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
40-
node ||= REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id}']/a:Assertion/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
39+
node = xpath_first_from_signed_assertion('/a:Subject/a:NameID')
4140
node.nil? ? nil : node.text
4241
end
4342
end
4443

4544
def sessionindex
4645
@sessionindex ||= begin
47-
node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
48-
node ||= REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id}']/a:Assertion/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
46+
node = xpath_first_from_signed_assertion('/a:AuthnStatement')
4947
node.nil? ? nil : node.attributes['SessionIndex']
5048
end
5149
end
@@ -55,7 +53,7 @@ def attributes
5553
@attr_statements ||= begin
5654
result = {}
5755

58-
stmt_element = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AttributeStatement", { "p" => PROTOCOL, "a" => ASSERTION })
56+
stmt_element = xpath_first_from_signed_assertion('/a:AttributeStatement')
5957
return {} if stmt_element.nil?
6058

6159
stmt_element.elements.each do |attr_element|
@@ -76,7 +74,7 @@ def attributes
7674
# When this user session should expire at latest
7775
def session_expires_at
7876
@expires_at ||= begin
79-
node = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
77+
node = xpath_first_from_signed_assertion('/a:AuthnStatement')
8078
parse_time(node, "SessionNotOnOrAfter")
8179
end
8280
end
@@ -91,15 +89,13 @@ def success?
9189

9290
# Conditions (if any) for the assertion to run
9391
def conditions
94-
@conditions ||= begin
95-
REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']/a:Conditions", { "p" => PROTOCOL, "a" => ASSERTION })
96-
end
92+
@conditions ||= xpath_first_from_signed_assertion('/a:Conditions')
9793
end
9894

9995
def issuer
10096
@issuer ||= begin
10197
node = REXML::XPath.first(document, "/p:Response/a:Issuer", { "p" => PROTOCOL, "a" => ASSERTION })
102-
node ||= REXML::XPath.first(document, "/p:Response/a:Assertion/a:Issuer", { "p" => PROTOCOL, "a" => ASSERTION })
98+
node ||= xpath_first_from_signed_assertion('/a:Issuer')
10399
node.nil? ? nil : node.text
104100
end
105101
end
@@ -146,6 +142,12 @@ def validate_response_state(soft = true)
146142
true
147143
end
148144

145+
def xpath_first_from_signed_assertion(subelt=nil)
146+
node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']#{subelt}", { "p" => PROTOCOL, "a" => ASSERTION })
147+
node ||= REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id}']/a:Assertion#{subelt}", { "p" => PROTOCOL, "a" => ASSERTION })
148+
node
149+
end
150+
149151
def get_fingerprint
150152
if settings.idp_cert
151153
cert = OpenSSL::X509::Certificate.new(settings.idp_cert)

test/test_helper.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
require 'shoulda'
44
require 'mocha'
55
require 'ruby-debug'
6+
require 'timecop'
67

78
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
89
$LOAD_PATH.unshift(File.dirname(__FILE__))

test/xml_security_test.rb

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,29 @@ class XmlSecurityTest < Test::Unit::TestCase
119119
end
120120

121121
context "StarfieldTMS" do
122-
should "be able to validate a response" do
123-
response = Onelogin::Saml::Response.new(fixture(:starfield_response))
124-
response.settings = Onelogin::Saml::Settings.new(
125-
:idp_cert_fingerprint => "8D:BA:53:8E:A3:B6:F9:F1:69:6C:BB:D9:D8:BD:41:B3:AC:4F:9D:4D"
126-
)
127-
assert response.validate!
122+
setup do
123+
@response = Onelogin::Saml::Response.new(fixture(:starfield_response))
124+
@response.settings = Onelogin::Saml::Settings.new(
125+
:idp_cert_fingerprint => "8D:BA:53:8E:A3:B6:F9:F1:69:6C:BB:D9:D8:BD:41:B3:AC:4F:9D:4D"
126+
)
127+
end
128+
129+
should "be able to validate a good response" do
130+
Timecop.freeze Time.parse('2012-11-28 17:55:00 UTC') do
131+
assert @response.validate!
132+
end
133+
end
134+
135+
should "fail before response is valid" do
136+
Timecop.freeze Time.parse('2012-11-20 17:55:00 UTC') do
137+
assert ! @response.is_valid?
138+
end
139+
end
140+
141+
should "fail after response expires" do
142+
Timecop.freeze Time.parse('2012-11-30 17:55:00 UTC') do
143+
assert ! @response.is_valid?
144+
end
128145
end
129146
end
130147

0 commit comments

Comments
 (0)