Skip to content
Prev Previous commit
Next Next commit
Intelligently pick SSO binding based on IDP metadata instead of only …
…based on signing setting
  • Loading branch information
Reece authored and Reece committed Apr 12, 2017
commit 675dc3853d236c858d42ec4780e16812370b4eea
17 changes: 17 additions & 0 deletions djangosaml2/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from defusedxml import ElementTree
from django.conf import settings
from saml2.s_utils import UnknownSystemEntity


def get_custom_setting(name, default=None):
Expand All @@ -37,6 +38,22 @@ def available_idps(config, langpref=None):
return dict([(idp, config.metadata.name(idp, langpref)) for idp in idps])


def get_idp_sso_supported_bindings(idp_entity_id=None):
"""Returns the list of bindings supported by an IDP
This is not clear in the pysaml2 code, so wrapping it in a util"""
# avoid circular import
from djangosaml2.conf import get_config
# load metadata store from config
config = get_config()
meta = getattr(config, 'metadata', {})
# if idp is None, assume only one exists so just use that
idp_entity_id = available_idps(config).keys().pop()
try:
return meta.service(idp_entity_id, 'idpsso_descriptor', 'single_sign_on_service').keys()
except UnknownSystemEntity:
return []


def get_location(http_info):
"""Extract the redirect URL from a pysaml2 http_info object"""
assert 'headers' in http_info
Expand Down
32 changes: 25 additions & 7 deletions djangosaml2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def csrf_exempt(view_func):
from djangosaml2.conf import get_config
from djangosaml2.signals import post_authenticated
from djangosaml2.utils import get_custom_setting, available_idps, get_location, \
get_hidden_form_inputs
get_hidden_form_inputs, get_idp_sso_supported_bindings


logger = logging.getLogger('djangosaml2')
Expand Down Expand Up @@ -157,21 +157,39 @@ def login(request,
#
# Read more in the official SAML2 specs (3.4.4.1):
# http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf
binding = BINDING_HTTP_POST if getattr(conf, '_sp_authn_requests_signed', False) else BINDING_HTTP_REDIRECT
sign_requests = getattr(conf, '_sp_authn_requests_signed', False)
binding = BINDING_HTTP_POST if sign_requests else BINDING_HTTP_REDIRECT

# ensure our selected binding is supported by the IDP
supported_bindings = get_idp_sso_supported_bindings(selected_idp)
if binding not in supported_bindings:
logger.debug('Binding %s not in IDP %s supported bindings: %s' % (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common antipattern - please pass the arguments to the logger directly instead of pre-merging into the message. It's easier to process the logs then...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix. I'm having merge conflicts between my two PRs though, so I'm going to make one combined one in just a sec.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd actually prefer if you kept them separate. It's easier to discuss and review then. If that import line bothers you, just use a separate import line in each...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll redo the other one then and I'll reopen this one.

binding, selected_idp, supported_bindings))
if binding == BINDING_HTTP_POST:
logger.warning('IDP %s does not support %s, trying %s' % (
selected_idp, binding, BINDING_HTTP_REDIRECT))
binding = BINDING_HTTP_REDIRECT
if sign_requests:
sign_requests = False
logger.warning('sp_authn_requests_signed is True, but ignoring because pysaml2 does not support it for %s' % BINDING_HTTP_REDIRECT)
else:
binding = BINDING_HTTP_POST
# if switched binding still not supported, give up
if binding not in supported_bindings:
raise UnsupportedBinding('IDP does not support %s or %s' % (
BINDING_HTTP_POST, BINDING_HTTP_REDIRECT))

client = Saml2Client(conf)
try:
# we use sign kwarg to override in case of redirect binding
# otherwise pysaml2 may sign the xml for redirect which is incorrect
(session_id, result) = client.prepare_for_authenticate(
entityid=selected_idp, relay_state=came_from,
binding=binding,
binding=binding, sign=sign_requests,
)
except TypeError as e:
logger.error('Unable to know which IdP to use')
return HttpResponse(text_type(e))
except UnsupportedBinding as e:
logger.error('%s: sp_authn_requests_signed=%s and dictates the binding chosen, ensure it matches what the IDP metadata allows' % (
e, getattr(conf, '_sp_authn_requests_signed', False)))
raise

logger.debug('Saving the session_id in the OutstandingQueries cache')
oq_cache = OutstandingQueriesCache(request.session)
Expand Down