search

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

Errors in documentation, no migration path for pre v1.0 users #43

Open jrial opened 3 years ago

jrial commented 3 years ago

The documentation (the README in the project root) contains a few errors and omissions:

This second item is causing an issue for people migrating from pre 1.0 to post 1.0: the class-based views have changed the SAML URLs, which means the SSO provider must update their settings to match the new URLs. In some organisations this can be a bit problematic due to slow formal ITIL procedures.

Not sure if this is a bug, or simply not or badly documented, but I would have guessed that setting the SAML_ROUTE setting, and including the URLs for ACS and SLS under the SAML_PROVIDERS dictionary, would result in the system using the same URLs as previously under the function-based views. This doesn't seem to work. I already had them in my settings, and they remain unchanged. What's worse: the new URLs don't seem to work either. They either result in a 404 or a 500. Before I file a separate bug for that, can someone from the team enlighten me as to whether these settings should in fact override the URLs for the class-based views as well?

shepdelacreme commented 3 years ago

Apologies for the documentation errors, SAML_ROUTE is no longer used since it makes more sense for users of this package to just specify a route prefix in their own URL patterns. 

path("sso/saml/", include((saml_urls), namespace="saml")),

or more simply:

path("sso/saml/", include("saml2_pro_auth.urls")),

Unfortunately, the old function_urls didn't fit with the substantial changes we needed to make in v1.0 to make this package work for our use cases internally. The rewrite of the views and some of the other fundamental changes we needed to make are why this was cut as a new major release. If you need support for the old function_urls or you would prefer querystring parameters for specifying provider or acs those could likely be implemented by inheriting and overriding some of the class-based view set up with a different set of URLs in your app.

I would also accept and consider any PR that mimics the old URL scheme as long as it doesn't impact complexity much further.

Lastly, if you are receiving 404 or 500's for the new views, please submit a ticket with as much information as you can provide and we will take a look, but it does sound like it is likely a configuration mismatch in your app or the IdP.

jrial commented 3 years ago

Hey, thanks for the response. No need to assist me with anything; as long as you're aware of the errors in the documentation, I'm good.

On my end, I won't be debugging this further. For now I'm going to stick with the previous version in use, as that one worked. It just lacked one feature of 1.0 which I can achieve through different means.