Skip to content

Commit 267d373

Browse files
committed
Merge pull request #201 from Umofomia/fix-default-security-settings
Freeze and duplicate default security settings hash so that it doesn't get modified.
2 parents d57a8e4 + 3029930 commit 267d373

File tree

4 files changed

+30
-11
lines changed

4 files changed

+30
-11
lines changed

lib/onelogin/ruby-saml/settings.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ def initialize(overrides = {})
55
config = DEFAULTS.merge(overrides)
66
config.each do |k,v|
77
acc = "#{k.to_s}=".to_sym
8-
self.send(acc, v) if self.respond_to? acc
8+
if self.respond_to? acc
9+
value = v.is_a?(Hash) ? v.dup : v
10+
self.send(acc, value)
11+
end
912
end
1013
@attribute_consuming_service = AttributeService.new
1114
end
@@ -97,8 +100,8 @@ def get_sp_key
97100
private
98101

99102
DEFAULTS = {
100-
:assertion_consumer_service_binding => "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST",
101-
:single_logout_service_binding => "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect",
103+
:assertion_consumer_service_binding => "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST".freeze,
104+
:single_logout_service_binding => "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect".freeze,
102105
:compress_request => true,
103106
:compress_response => true,
104107
:security => {
@@ -108,9 +111,9 @@ def get_sp_key
108111
:embed_sign => false,
109112
:digest_method => XMLSecurity::Document::SHA1,
110113
:signature_method => XMLSecurity::Document::SHA1
111-
},
114+
}.freeze,
112115
:double_quote_xml_attribute_values => false,
113-
}
116+
}.freeze
114117
end
115118
end
116119
end

test/logoutrequest_test.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ class RequestTest < Minitest::Test
104104

105105
inflated = decode_saml_request_payload(unauth_url)
106106
assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], inflated
107+
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], inflated
108+
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], inflated
107109
end
108110

109111
it "create a signed logout request with 256 digest and signature methods" do
@@ -123,8 +125,8 @@ class RequestTest < Minitest::Test
123125
request_xml = Base64.decode64(params["SAMLRequest"])
124126

125127
assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
126-
request_xml =~ /<ds:SignatureMethod Algorithm='http:\/\/www.w3.org\/2001\/04\/xmldsig-more#rsa-sha256'\/>/
127-
request_xml =~ /<ds:DigestMethod Algorithm='http:\/\/www.w3.org\/2001\/04\/xmldsig-more#rsa-sha512'\/>/
128+
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], request_xml
129+
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha512'/>], request_xml
128130
end
129131
end
130132

test/request_test.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ class RequestTest < Minitest::Test
156156
params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
157157
request_xml = Base64.decode64(params["SAMLRequest"])
158158
assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
159-
request_xml =~ /<ds:SignatureMethod Algorithm='http:\/\/www.w3.org\/2000\/09\/xmldsig#rsa-sha1'\/>/
160-
request_xml =~ /<ds:DigestMethod Algorithm='http:\/\/www.w3.org\/2000\/09\/xmldsig#rsa-sha1'\/>/
159+
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
160+
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
161161
end
162162

163163
it "create a signed request with 256 digest and signature methods" do
@@ -174,8 +174,8 @@ class RequestTest < Minitest::Test
174174
params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
175175
request_xml = Base64.decode64(params["SAMLRequest"])
176176
assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
177-
request_xml =~ /<ds:SignatureMethod Algorithm='http:\/\/www.w3.org\/2001\/04\/xmldsig-more#rsa-sha256'\/>/
178-
request_xml =~ /<ds:DigestMethod Algorithm='http:\/\/www.w3.org\/2001\/04\/xmldsig-more#rsa-sha512'\/>/
177+
assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], request_xml
178+
assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha512'/>], request_xml
179179
end
180180
end
181181

test/settings_test.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,5 +61,19 @@ class SettingsTest < Minitest::Test
6161
assert_equal @settings.attribute_consuming_service.name, "Test Service"
6262
assert_equal @settings.attribute_consuming_service.attributes, [{:name => "Name", :name_format => "Name Format", :friendly_name => "Friendly Name" }]
6363
end
64+
65+
it "does not modify default security settings" do
66+
settings = OneLogin::RubySaml::Settings.new
67+
settings.security[:authn_requests_signed] = true
68+
settings.security[:embed_sign] = true
69+
settings.security[:digest_method] = XMLSecurity::Document::SHA256
70+
settings.security[:signature_method] = XMLSecurity::Document::SHA256
71+
72+
new_settings = OneLogin::RubySaml::Settings.new
73+
assert_equal new_settings.security[:authn_requests_signed], false
74+
assert_equal new_settings.security[:embed_sign], false
75+
assert_equal new_settings.security[:digest_method], XMLSecurity::Document::SHA1
76+
assert_equal new_settings.security[:signature_method], XMLSecurity::Document::SHA1
77+
end
6478
end
6579
end

0 commit comments

Comments
 (0)