Skip to content

Commit 78ab3ca

Browse files
committed
Destination URL Comparison is now case-insensitive for netloc
1 parent 54404b0 commit 78ab3ca

File tree

7 files changed

+215
-16
lines changed

7 files changed

+215
-16
lines changed

src/onelogin/saml2/logout_request.py

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -357,19 +357,18 @@ def is_valid(self, request_data, raise_exceptions=False):
357357
)
358358

359359
# Check destination
360-
if dom.get('Destination', None):
361-
destination = dom.get('Destination')
362-
if destination != '':
363-
if current_url not in destination:
364-
raise Exception(
365-
'The LogoutRequest was received at '
366-
'%(currentURL)s instead of %(destination)s' %
367-
{
368-
'currentURL': current_url,
369-
'destination': destination,
370-
},
371-
OneLogin_Saml2_ValidationError.WRONG_DESTINATION
372-
)
360+
destination = dom.get('Destination')
361+
if destination:
362+
if not OneLogin_Saml2_Utils.normalize_url(url=destination).startswith(OneLogin_Saml2_Utils.normalize_url(url=current_url)):
363+
raise Exception(
364+
'The LogoutRequest was received at '
365+
'%(currentURL)s instead of %(destination)s' %
366+
{
367+
'currentURL': current_url,
368+
'destination': destination,
369+
},
370+
OneLogin_Saml2_ValidationError.WRONG_DESTINATION
371+
)
373372

374373
# Check issuer
375374
issuer = OneLogin_Saml2_Logout_Request.get_issuer(dom)

src/onelogin/saml2/logout_response.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ def is_valid(self, request_data, request_id=None, raise_exceptions=False):
126126
# Check destination
127127
if self.document.documentElement.hasAttribute('Destination'):
128128
destination = self.document.documentElement.getAttribute('Destination')
129-
if destination != '':
130-
if current_url not in destination:
129+
if destination:
130+
if not OneLogin_Saml2_Utils.normalize_url(url=destination).startswith(OneLogin_Saml2_Utils.normalize_url(url=current_url)):
131131
raise OneLogin_Saml2_ValidationError(
132132
'The LogoutResponse was received at %s instead of %s' % (current_url, destination),
133133
OneLogin_Saml2_ValidationError.WRONG_DESTINATION

src/onelogin/saml2/response.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ def is_valid(self, request_data, request_id=None, raise_exceptions=False):
209209
# Checks destination
210210
destination = self.document.get('Destination', None)
211211
if destination:
212-
if not destination.startswith(current_url):
212+
if not OneLogin_Saml2_Utils.normalize_url(url=destination).startswith(OneLogin_Saml2_Utils.normalize_url(url=current_url)):
213213
# TODO: Review if following lines are required, since we can control the
214214
# request_data
215215
# current_url_routed = OneLogin_Saml2_Utils.get_self_routed_url_no_query(request_data)

src/onelogin/saml2/utils.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from tempfile import NamedTemporaryFile
2424
from textwrap import wrap
2525
from urllib import quote_plus
26+
from urlparse import urlsplit, urlunsplit
2627
from uuid import uuid4
2728
from xml.dom.minidom import Document, Element
2829
from defusedxml.minidom import parseString
@@ -1280,3 +1281,24 @@ def extract_raw_query_parameter(query_string, parameter, default=''):
12801281
def case_sensitive_urlencode(to_encode, lowercase=False):
12811282
encoded = quote_plus(to_encode)
12821283
return re.sub(r"%[A-F0-9]{2}", lambda m: m.group(0).lower(), encoded) if lowercase else encoded
1284+
1285+
@staticmethod
1286+
def normalize_url(url):
1287+
"""
1288+
Returns normalized URL for comparison.
1289+
This method converts the netloc to lowercase, as it should be case-insensitive (per RFC 4343, RFC 7617)
1290+
If standardization fails, the original URL is returned
1291+
Python documentation indicates that URL split also normalizes query strings if empty query fields are present
1292+
1293+
:param url: URL
1294+
:type url: String
1295+
1296+
:returns: A normalized URL, or the given URL string if parsing fails
1297+
:rtype: String
1298+
"""
1299+
try:
1300+
scheme, netloc, path, query, fragment = urlsplit(url)
1301+
normalized_url = urlunsplit((scheme.lower(), netloc.lower(), path, query, fragment))
1302+
return normalized_url
1303+
except Exception:
1304+
return url

tests/src/OneLogin/saml2_tests/logout_request_test.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,70 @@ def testIsInvalidDestination(self):
343343
logout_request4 = OneLogin_Saml2_Logout_Request(settings, b64encode(dom.toxml()))
344344
self.assertTrue(logout_request4.is_valid(request_data))
345345

346+
def testIsValidWithCapitalization(self):
347+
"""
348+
Tests the is_valid method of the OneLogin_Saml2_LogoutRequest
349+
"""
350+
request_data = {
351+
'http_host': 'exaMPLe.com',
352+
'script_name': 'index.html'
353+
}
354+
request = self.file_contents(join(self.data_path, 'logout_requests', 'logout_request.xml'))
355+
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
356+
357+
logout_request = OneLogin_Saml2_Logout_Request(settings, b64encode(request))
358+
self.assertTrue(logout_request.is_valid(request_data))
359+
360+
settings.set_strict(True)
361+
logout_request2 = OneLogin_Saml2_Logout_Request(settings, b64encode(request))
362+
self.assertFalse(logout_request2.is_valid(request_data))
363+
364+
settings.set_strict(False)
365+
dom = parseString(request)
366+
logout_request3 = OneLogin_Saml2_Logout_Request(settings, b64encode(dom.toxml()))
367+
self.assertTrue(logout_request3.is_valid(request_data))
368+
369+
settings.set_strict(True)
370+
logout_request4 = OneLogin_Saml2_Logout_Request(settings, b64encode(dom.toxml()))
371+
self.assertFalse(logout_request4.is_valid(request_data))
372+
373+
current_url = OneLogin_Saml2_Utils.get_self_url_no_query(request_data)
374+
request = request.replace('http://stuff.com/endpoints/endpoints/sls.php', current_url.lower())
375+
logout_request5 = OneLogin_Saml2_Logout_Request(settings, b64encode(request))
376+
self.assertTrue(logout_request5.is_valid(request_data))
377+
378+
def testIsInValidWithCapitalization(self):
379+
"""
380+
Tests the is_valid method of the OneLogin_Saml2_LogoutRequest
381+
"""
382+
request_data = {
383+
'http_host': 'example.com',
384+
'script_name': 'INdex.html'
385+
}
386+
request = self.file_contents(join(self.data_path, 'logout_requests', 'logout_request.xml'))
387+
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
388+
389+
logout_request = OneLogin_Saml2_Logout_Request(settings, b64encode(request))
390+
self.assertTrue(logout_request.is_valid(request_data))
391+
392+
settings.set_strict(True)
393+
logout_request2 = OneLogin_Saml2_Logout_Request(settings, b64encode(request))
394+
self.assertFalse(logout_request2.is_valid(request_data))
395+
396+
settings.set_strict(False)
397+
dom = parseString(request)
398+
logout_request3 = OneLogin_Saml2_Logout_Request(settings, b64encode(dom.toxml()))
399+
self.assertTrue(logout_request3.is_valid(request_data))
400+
401+
settings.set_strict(True)
402+
logout_request4 = OneLogin_Saml2_Logout_Request(settings, b64encode(dom.toxml()))
403+
self.assertFalse(logout_request4.is_valid(request_data))
404+
405+
current_url = OneLogin_Saml2_Utils.get_self_url_no_query(request_data)
406+
request = request.replace('http://stuff.com/endpoints/endpoints/sls.php', current_url.lower())
407+
logout_request5 = OneLogin_Saml2_Logout_Request(settings, b64encode(request))
408+
self.assertFalse(logout_request5.is_valid(request_data))
409+
346410
def testIsInvalidNotOnOrAfter(self):
347411
"""
348412
Tests the is_valid method of the OneLogin_Saml2_LogoutRequest

tests/src/OneLogin/saml2_tests/logout_response_test.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,62 @@ def testIsInValidDestination(self):
225225
response_4 = OneLogin_Saml2_Logout_Response(settings, message_4)
226226
self.assertTrue(response_4.is_valid(request_data))
227227

228+
def testIsValidWithCapitalization(self):
229+
"""
230+
Tests the is_valid method of the OneLogin_Saml2_LogoutResponse
231+
"""
232+
request_data = {
233+
'http_host': 'exaMPLe.com',
234+
'script_name': 'index.html',
235+
'get_data': {}
236+
}
237+
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
238+
message = self.file_contents(join(self.data_path, 'logout_responses', 'logout_response_deflated.xml.base64'))
239+
240+
response = OneLogin_Saml2_Logout_Response(settings, message)
241+
self.assertTrue(response.is_valid(request_data))
242+
243+
settings.set_strict(True)
244+
response_2 = OneLogin_Saml2_Logout_Response(settings, message)
245+
with self.assertRaisesRegexp(Exception, 'The LogoutResponse was received at'):
246+
response_2.is_valid(request_data, raise_exceptions=True)
247+
248+
plain_message = OneLogin_Saml2_Utils.decode_base64_and_inflate(message)
249+
current_url = OneLogin_Saml2_Utils.get_self_url_no_query(request_data).lower()
250+
plain_message = plain_message.replace('http://stuff.com/endpoints/endpoints/sls.php', current_url)
251+
message_3 = OneLogin_Saml2_Utils.deflate_and_base64_encode(plain_message)
252+
253+
response_3 = OneLogin_Saml2_Logout_Response(settings, message_3)
254+
self.assertTrue(response_3.is_valid(request_data))
255+
256+
def testIsInValidWithCapitalization(self):
257+
"""
258+
Tests the is_valid method of the OneLogin_Saml2_LogoutResponse
259+
"""
260+
request_data = {
261+
'http_host': 'example.com',
262+
'script_name': 'INdex.html',
263+
'get_data': {}
264+
}
265+
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
266+
message = self.file_contents(join(self.data_path, 'logout_responses', 'logout_response_deflated.xml.base64'))
267+
268+
response = OneLogin_Saml2_Logout_Response(settings, message)
269+
self.assertTrue(response.is_valid(request_data))
270+
271+
settings.set_strict(True)
272+
response_2 = OneLogin_Saml2_Logout_Response(settings, message)
273+
with self.assertRaisesRegexp(Exception, 'The LogoutResponse was received at'):
274+
response_2.is_valid(request_data, raise_exceptions=True)
275+
276+
plain_message = OneLogin_Saml2_Utils.decode_base64_and_inflate(message)
277+
current_url = OneLogin_Saml2_Utils.get_self_url_no_query(request_data).lower()
278+
plain_message = plain_message.replace('http://stuff.com/endpoints/endpoints/sls.php', current_url)
279+
message_3 = OneLogin_Saml2_Utils.deflate_and_base64_encode(plain_message)
280+
281+
response_3 = OneLogin_Saml2_Logout_Response(settings, message_3)
282+
self.assertFalse(response_3.is_valid(request_data))
283+
228284
def testIsValidRaisesExceptionWhenRaisesArgumentIsTrue(self):
229285
message = OneLogin_Saml2_Utils.deflate_and_base64_encode('<xml>invalid</xml>')
230286
request_data = {

tests/src/OneLogin/saml2_tests/response_test.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@ def get_request_data(self):
4242
'script_name': 'index.html'
4343
}
4444

45+
def get_request_data_domain_capitalized(self):
46+
return {
47+
'http_host': 'StuFF.Com',
48+
'script_name': 'endpoints/endpoints/acs.php'
49+
}
50+
51+
def get_request_data_path_capitalized(self):
52+
return {
53+
'http_host': 'stuff.com',
54+
'script_name': 'Endpoints/endPoints/acs.php'
55+
}
56+
57+
def get_request_data_both_capitalized(self):
58+
return {
59+
'http_host': 'StuFF.Com',
60+
'script_name': 'Endpoints/endPoints/aCs.php'
61+
}
62+
4563
def testConstruct(self):
4664
"""
4765
Tests the OneLogin_Saml2_Response Constructor.
@@ -1075,6 +1093,46 @@ def testIsInValidDestination(self):
10751093
self.assertFalse(response_5.is_valid(self.get_request_data()))
10761094
self.assertIn('A valid SubjectConfirmation was not found on this Response', response_5.get_error())
10771095

1096+
settings.set_strict(True)
1097+
response_2 = OneLogin_Saml2_Response(settings, message)
1098+
self.assertFalse(response_2.is_valid(self.get_request_data()))
1099+
self.assertIn('The response was received at', response_2.get_error())
1100+
1101+
def testIsInValidDestinationCapitalizationOfElements(self):
1102+
"""
1103+
Tests the is_valid method of the OneLogin_Saml2_Response class
1104+
Case Invalid Response due to differences in capitalization of path
1105+
"""
1106+
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
1107+
message = self.file_contents(join(self.data_path, 'responses', 'unsigned_response.xml.base64'))
1108+
1109+
# Test path capitalized
1110+
settings.set_strict(True)
1111+
response = OneLogin_Saml2_Response(settings, message)
1112+
self.assertFalse(response.is_valid(self.get_request_data_path_capitalized()))
1113+
self.assertIn('The response was received at', response.get_error())
1114+
1115+
# Test both domain and path capitalized
1116+
response_2 = OneLogin_Saml2_Response(settings, message)
1117+
self.assertFalse(response_2.is_valid(self.get_request_data_both_capitalized()))
1118+
self.assertIn('The response was received at', response_2.get_error())
1119+
1120+
def testIsValidDestinationCapitalizationOfHost(self):
1121+
"""
1122+
Tests the is_valid method of the OneLogin_Saml2_Response class
1123+
Case Valid Response, even if host is differently capitalized (per RFC)
1124+
"""
1125+
settings = OneLogin_Saml2_Settings(self.loadSettingsJSON())
1126+
message = self.file_contents(join(self.data_path, 'responses', 'unsigned_response.xml.base64'))
1127+
# Test domain capitalized
1128+
settings.set_strict(True)
1129+
response = OneLogin_Saml2_Response(settings, message)
1130+
self.assertFalse(response.is_valid(self.get_request_data_domain_capitalized()))
1131+
self.assertNotIn('The response was received at', response.get_error())
1132+
1133+
# Assert we got past the destination check, which appears later
1134+
self.assertIn('A valid SubjectConfirmation was not found', response.get_error())
1135+
10781136
def testIsInValidAudience(self):
10791137
"""
10801138
Tests the is_valid method of the OneLogin_Saml2_Response class

0 commit comments

Comments
 (0)