Skip to content

Commit 1952da2

Browse files
skorandaskoranda
committed
Fix for 459 HTTP_POST form nonconforming and shows submit
Fix for issue 459 "Form used with HTTP_POST binding nonconforming and shows submit button". The fix introduces an HTML5 DOCTYPE declaration and uses noscript tags appropriately to hide the submit button when Javascript is enabled. Modification of tests were necessary because the tests unecessarily relied on the response being a list of strings with the <form> element being the fourth item in the list, in order to unpack the form and pull out the SAMLResponse and relay state for comparison. The new tests do not require the response to be arbitrarily broken up as a list of strings.
1 parent 9cbbd9b commit 1952da2

File tree

4 files changed

+43
-22
lines changed

4 files changed

+43
-22
lines changed

‎src/saml2/pack.py

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,35 @@
4040
import defusedxml.ElementTree
4141

4242
NAMESPACE = "http://schemas.xmlsoap.org/soap/envelope/"
43-
FORM_SPEC = """<form method="post" action="%s">
44-
<input type="hidden" name="%s" value="%s" />
45-
<input type="hidden" name="RelayState" value="%s" />
46-
<input type="submit" value="Submit" />
47-
</form>"""
4843

44+
FORM_SPEC = """\
45+
<!DOCTYPE html>
46+
<html>
47+
<head>
48+
<meta charset="utf-8" />
49+
</head>
50+
<body onload="document.forms[0].submit()">
51+
<noscript>
52+
<p>
53+
<strong>Note:</strong> Since your browser does not support JavaScript,
54+
you must press the Continue button once to proceed.
55+
</p>
56+
</noscript>
57+
58+
<form action="{action}" method="post">
59+
<div>
60+
<input type="hidden" name="RelayState" value="{relay_state}"/>
61+
62+
<input type="hidden" name="{saml_type}" value="{saml_response}"/>
63+
</div>
64+
<noscript>
65+
<div>
66+
<input type="submit" value="Continue"/>
67+
</div>
68+
</noscript>
69+
</form>
70+
</body>
71+
</html>"""
4972

5073
def http_form_post_message(message, location, relay_state="",
5174
typ="SAMLRequest", **kwargs):
@@ -58,8 +81,6 @@ def http_form_post_message(message, location, relay_state="",
5881
:param relay_state: for preserving and conveying state information
5982
:return: A tuple containing header information and a HTML message.
6083
"""
61-
response = ["<head>", """<title>SAML 2.0 POST</title>""", "</head><body>"]
62-
6384
if not isinstance(message, six.string_types):
6485
message = str(message)
6586
if not isinstance(message, six.binary_type):
@@ -71,17 +92,17 @@ def http_form_post_message(message, location, relay_state="",
7192
_msg = message
7293
_msg = _msg.decode('ascii')
7394

74-
response.append(FORM_SPEC % (location, typ, _msg, relay_state))
95+
args = {
96+
'action' : location,
97+
'saml_type' : typ,
98+
'relay_state' : relay_state,
99+
'saml_response' : _msg
100+
}
75101

76-
response.append("""<script type="text/javascript">""")
77-
response.append(" window.onload = function ()")
78-
response.append(" { document.forms[0].submit(); }")
79-
response.append("""</script>""")
80-
response.append("</body>")
102+
response = FORM_SPEC.format(**args)
81103

82104
return {"headers": [("Content-type", "text/html")], "data": response}
83105

84-
85106
def http_post_message(message, relay_state="", typ="SAMLRequest", **kwargs):
86107
"""
87108

‎tests/test_51_client.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,7 +1398,7 @@ def test_do_logout_post(self):
13981398
binding, info = resp[entity_ids[0]]
13991399
assert binding == BINDING_HTTP_POST
14001400

1401-
_dic = unpack_form(info["data"][3])
1401+
_dic = unpack_form(info["data"])
14021402
res = self.server.parse_logout_request(_dic["SAMLRequest"],
14031403
BINDING_HTTP_POST)
14041404
assert b'<ns0:SessionIndex>_foo</ns0:SessionIndex>' in res.xmlstr
@@ -1428,7 +1428,7 @@ def test_do_logout_session_expired(self):
14281428
binding, info = resp[entity_ids[0]]
14291429
assert binding == BINDING_HTTP_POST
14301430

1431-
_dic = unpack_form(info["data"][3])
1431+
_dic = unpack_form(info["data"])
14321432
res = self.server.parse_logout_request(_dic["SAMLRequest"],
14331433
BINDING_HTTP_POST)
14341434
assert b'<ns0:SessionIndex>_foo</ns0:SessionIndex>' in res.xmlstr
@@ -1525,7 +1525,7 @@ def test_post_sso(self):
15251525
sid, http_args = self.client.prepare_for_authenticate(
15261526
"urn:mace:example.com:saml:roland:idp", relay_state="really",
15271527
binding=binding, response_binding=response_binding)
1528-
_dic = unpack_form(http_args["data"][3])
1528+
_dic = unpack_form(http_args["data"])
15291529

15301530
req = self.server.parse_authn_request(_dic["SAMLRequest"], binding)
15311531
resp_args = self.server.response_args(req.message, [response_binding])
@@ -1543,7 +1543,7 @@ def test_post_sso(self):
15431543

15441544
response = self.client.send(**http_args)
15451545
print(response.text)
1546-
_dic = unpack_form(response.text[3], "SAMLResponse")
1546+
_dic = unpack_form(response.text, "SAMLResponse")
15471547
resp = self.client.parse_authn_request_response(_dic["SAMLResponse"],
15481548
BINDING_HTTP_POST,
15491549
{sid: "/"})
@@ -1558,7 +1558,7 @@ def test_negotiated_post_sso(self):
15581558
sid, auth_binding, http_args = self.client.prepare_for_negotiated_authenticate(
15591559
"urn:mace:example.com:saml:roland:idp", relay_state="really",
15601560
binding=binding, response_binding=response_binding)
1561-
_dic = unpack_form(http_args["data"][3])
1561+
_dic = unpack_form(http_args["data"])
15621562

15631563
assert binding == auth_binding
15641564

@@ -1578,7 +1578,7 @@ def test_negotiated_post_sso(self):
15781578

15791579
response = self.client.send(**http_args)
15801580
print(response.text)
1581-
_dic = unpack_form(response.text[3], "SAMLResponse")
1581+
_dic = unpack_form(response.text, "SAMLResponse")
15821582
resp = self.client.parse_authn_request_response(_dic["SAMLResponse"],
15831583
BINDING_HTTP_POST,
15841584
{sid: "/"})

‎tests/test_65_authn_query.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def get_msg(hinfo, binding):
2828
if binding == BINDING_SOAP:
2929
xmlstr = hinfo["data"]
3030
elif binding == BINDING_HTTP_POST:
31-
_inp = hinfo["data"][3]
31+
_inp = hinfo["data"]
3232
i = _inp.find(TAG1)
3333
i += len(TAG1) + 1
3434
j = _inp.find('"', i)

‎tests/test_68_assertion_id.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def get_msg(hinfo, binding, response=False):
2727
if binding == BINDING_SOAP:
2828
msg = hinfo["data"]
2929
elif binding == BINDING_HTTP_POST:
30-
_inp = hinfo["data"][3]
30+
_inp = hinfo["data"]
3131
i = _inp.find(TAG1)
3232
i += len(TAG1) + 1
3333
j = _inp.find('"', i)

0 commit comments

Comments
 (0)