Skip to content

Commit 68c0a89

Browse files
c00kiemon5terc00kiemon5ter
committed
Small refactor
Signed-off-by: Ivan Kanakarakis <[email protected]>
1 parent 5b07161 commit 68c0a89

File tree

3 files changed

+77
-65
lines changed

3 files changed

+77
-65
lines changed

‎src/saml2/entity.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,8 +1024,16 @@ def srv2typ(service):
10241024
else:
10251025
return typ
10261026

1027-
def _parse_request(self, enc_request, request_cls, service, binding,
1028-
relay_state=None, sigalg=None, signature=None):
1027+
def _parse_request(
1028+
self,
1029+
enc_request,
1030+
request_cls,
1031+
service,
1032+
binding,
1033+
relay_state=None,
1034+
sigalg=None,
1035+
signature=None,
1036+
):
10291037
"""Parse a Request
10301038
10311039
:param enc_request: The request in its transport format
@@ -1040,8 +1048,7 @@ def _parse_request(self, enc_request, request_cls, service, binding,
10401048
_log_debug = logger.debug
10411049

10421050
# The addresses I should receive messages like this on
1043-
receiver_addresses = self.config.endpoint(service, binding,
1044-
self.entity_type)
1051+
receiver_addresses = self.config.endpoint(service, binding, self.entity_type)
10451052
if not receiver_addresses and self.entity_type == "idp":
10461053
for typ in ["aa", "aq", "pdp"]:
10471054
receiver_addresses = self.config.endpoint(service, binding, typ)

‎src/saml2/request.py

Lines changed: 62 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import logging
22

3+
from saml2 import time_util
4+
from saml2 import BINDING_HTTP_REDIRECT
5+
from saml2 import BINDING_HTTP_POST
36
from saml2.attribute_converter import to_local
4-
from saml2 import time_util, BINDING_HTTP_REDIRECT
57
from saml2.s_utils import OtherError
68

79
from saml2.validate import valid_instance
@@ -37,81 +39,83 @@ def _clear(self):
3739
self.message = None
3840
self.not_on_or_after = 0
3941

40-
def _loads(self, xmldata, binding=None, origdoc=None, must=None,
41-
only_valid_cert=False, relayState=None, sigalg=None, signature=None):
42+
def _loads(
43+
self,
44+
xmldata,
45+
binding=None,
46+
origdoc=None,
47+
must=None,
48+
only_valid_cert=False,
49+
relay_state=None,
50+
sigalg=None,
51+
signature=None,
52+
):
4253
# own copy
4354
self.xmlstr = xmldata[:]
44-
logger.debug("xmlstr: %s, relayState: %s, sigalg: %s, signature: %s",
45-
self.xmlstr, relayState, sigalg, signature)
46-
# If redirect binding, and provided SigAlg, Signature use that to verify
47-
# and skip signatureCheck withing SAMLRequest/xmldata
48-
_need_redirect_sig_check, _saml_msg, must = self._should_do_redirect_sig_check(
49-
binding, must, origdoc, relayState, sigalg, signature)
55+
logger.debug("xmlstr: %s, relay_state: %s, sigalg: %s, signature: %s",
56+
self.xmlstr, relay_state, sigalg, signature)
57+
58+
signed_post = must and binding == BINDING_HTTP_POST
59+
signed_redirect = must and binding == BINDING_HTTP_REDIRECT
60+
incorrectly_signed = IncorrectlySigned("Request was not signed correctly")
5061

5162
try:
52-
self.message = self.signature_check(xmldata, origdoc=origdoc,
53-
must=must,
54-
only_valid_cert=only_valid_cert)
55-
except TypeError:
56-
raise
57-
except Exception as excp:
63+
self.message = self.signature_check(
64+
xmldata,
65+
origdoc=origdoc,
66+
must=signed_post,
67+
only_valid_cert=only_valid_cert,
68+
)
69+
except Exception as e:
5870
self.message = None
59-
logger.info("EXCEPTION: %s", excp)
71+
raise incorrectly_signed from e
6072

61-
if _need_redirect_sig_check and self.message is not None:
62-
_verified_ok = self._do_redirect_sig_check(_saml_msg)
63-
# Set self.message to None, it shall raise error further down.
64-
if not _verified_ok:
73+
if signed_redirect:
74+
if sigalg is None or signature is None:
75+
raise incorrectly_signed
76+
77+
_saml_msg = {
78+
"SAMLRequest": origdoc,
79+
"Signature": signature,
80+
"SigAlg": sigalg,
81+
}
82+
if relay_state is not None:
83+
_saml_msg["RelayState"] = relay_state
84+
try:
85+
sig_verified = self._do_redirect_sig_check(_saml_msg)
86+
except Exception as e:
6587
self.message = None
66-
logger.error('Failed to verify signature')
88+
raise incorrectly_signed from e
89+
else:
90+
if not sig_verified:
91+
self.message = None
92+
raise incorrectly_signed
6793

6894
if not self.message:
69-
logger.error("Request was not correctly signed")
70-
logger.info("Request: %s", xmldata)
71-
raise IncorrectlySigned()
95+
logger.error("Request was not signed correctly")
96+
logger.info("Request data: %s", xmldata)
97+
raise incorrectly_signed
7298

73-
logger.info("Request: %s", self.message)
99+
logger.info("Request message: %s", self.message)
74100

75101
try:
76102
valid_instance(self.message)
77103
except NotValid as exc:
78-
logger.error("Not valid request: %s", exc.args[0])
104+
logger.error("Request not valid: %s", exc.args[0])
79105
raise
80106

81107
return self
82108

83109
def _do_redirect_sig_check(self, _saml_msg):
84-
_issuer = self.message.issuer.text.strip()
85-
_certs = self.sec.metadata.certs(_issuer, "any", "signing")
86-
logger.debug("Certs: %s, _saml_msg: %s", _certs, _saml_msg)
87-
_verified_ok = False
88-
for cert in _certs:
89-
if verify_redirect_signature(_saml_msg, self.sec.sec_backend, cert):
90-
_verified_ok = True
91-
break
92-
logger.info("Redirect request signature check: %s", _verified_ok)
93-
return _verified_ok
94-
95-
def _should_do_redirect_sig_check(self, binding, must, origdoc, relayState, sigalg,
96-
signature):
97-
_do_redirect_sig_check = False
98-
_saml_msg = {}
99-
if binding == BINDING_HTTP_REDIRECT and must \
100-
and sigalg is not None and signature is not None:
101-
logger.debug("Request signature check will be done using query param,"
102-
" instead of SAMLRequest content")
103-
_do_redirect_sig_check = True
104-
must = False
105-
_saml_msg = {
106-
"SAMLRequest": origdoc,
107-
"SigAlg": sigalg,
108-
"Signature": signature
109-
}
110-
# RelayState is optional so only add when available,
111-
# signature validate fails if passed as None
112-
if relayState is not None:
113-
_saml_msg["RelayState"] = relayState
114-
return _do_redirect_sig_check, _saml_msg, must
110+
issuer = self.message.issuer.text.strip()
111+
certs = self.sec.metadata.certs(issuer, "any", "signing")
112+
logger.debug("Certs to verify request sig: %s, _saml_msg: %s", certs, _saml_msg)
113+
verified = any(
114+
verify_redirect_signature(_saml_msg, self.sec.sec_backend, cert)
115+
for cert_name, cert in certs
116+
)
117+
logger.info("Redirect request signature check: %s", verified)
118+
return verified
115119

116120
def issue_instant_ok(self):
117121
""" Check that the request was issued at a reasonable time """
@@ -144,7 +148,7 @@ def _verify(self):
144148
def loads(self, xmldata, binding, origdoc=None, must=None,
145149
only_valid_cert=False, relay_state=None, sigalg=None, signature=None):
146150
return self._loads(xmldata, binding, origdoc, must,
147-
only_valid_cert=only_valid_cert, relayState=relay_state,
151+
only_valid_cert=only_valid_cert, relay_state=relay_state,
148152
sigalg=sigalg, signature=signature)
149153

150154
def verify(self):

‎tests/test_51_client.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
from saml2.argtree import add_path
1212
from saml2.cert import OpenSSLWrapper
1313
from saml2.xmldsig import sig_default
14-
from saml2.xmldsig import SIG_RSA_SHA256, SIG_RSA_SHA1
14+
from saml2.xmldsig import SIG_RSA_SHA256
15+
from saml2.xmldsig import SIG_RSA_SHA1
1516
from saml2 import BINDING_HTTP_POST
1617
from saml2 import BINDING_HTTP_REDIRECT
1718
from saml2 import config
@@ -29,8 +30,8 @@
2930
from saml2.authn_context import INTERNETPROTOCOLPASSWORD
3031
from saml2.client import Saml2Client
3132
from saml2.pack import parse_soap_enveloped_saml
32-
from saml2.response import LogoutResponse, StatusInvalidNameidPolicy, StatusError, \
33-
IncorrectlySigned
33+
from saml2.response import LogoutResponse, StatusInvalidNameidPolicy, StatusError
34+
from saml2.response import IncorrectlySigned
3435
from saml2.saml import NAMEID_FORMAT_PERSISTENT, EncryptedAssertion, Advice
3536
from saml2.saml import NAMEID_FORMAT_TRANSIENT
3637
from saml2.saml import NameID

0 commit comments

Comments
 (0)