zibasec / django-saml2-pro-auth

SAML2 authentication backend for Django wrapping OneLogin's python-saml package https://github.com/onelogin/python3-saml
MIT License
30 stars 28 forks source link

Attempt Patch for Issue #9 #11

Closed juliedavila closed 6 years ago

juliedavila commented 6 years ago

For https://github.com/MindPointGroup/django-saml2-pro-auth/issues/9

After looking at the python3-saml package it seems I missed a kwarg to support redirects see; https://github.com/onelogin/python3-saml/blob/07bb2ba245d3aebbd24c808a1823b12645f4d755/src/onelogin/saml2/auth.py#L321

This is an attempt to fix the issue.

rezende commented 6 years ago

It didn't work for my case because IDP is calling /saml/sso/acs?provider=myprovider

So the code never reaches line 50 (because it is a elif). I believe that:

if hasattr(settings, 'SAML_REDIRECT'):
    return HttpResponseRedirect(auth.login(return_to=settings.SAML_REDIRECT))

should be right under line 43, which would be consistent with what's written on the documentation:

Order of precedence is: SAML_REDIRECT value (if defined), RELAY_STATE provided in the SAML response, and the fallback is simply to go to the root path of your application.

So it would try to redirect to SAML_REDIRECT first, before RelayState

juliedavila commented 6 years ago

Ah, I see, yes, for SP initiated logins the logic is proper but for IDP initiated it doesnt hit that at all. Ok, I'll make an update.

juliedavila commented 6 years ago

@rezende I've pushed an update. Let me know if it works for you

rezende commented 6 years ago

@defionscode That's exactly what I needed. Thank you!

juliedavila commented 6 years ago

@rezende just to get explicit confirmation. you have tested this locally and it works?

rezende commented 6 years ago

Yes, I've just finished testing @defionscode and it worked perfectly

juliedavila commented 6 years ago

Thank you for your help! I'll push an update to pypi shortly