From fcb5d36e077fe65dc37f3d5bc29a7f7fcbf89a89 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 3 Mar 2019 00:07:40 +0100 Subject: [PATCH] cleanup SAML urls --- passbook/saml_idp/models.py | 2 +- .../saml_idp/templates/saml/idp/settings.html | 2 +- passbook/saml_idp/urls.py | 19 ++++++------- passbook/saml_idp/views.py | 27 ++++++++++--------- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/passbook/saml_idp/models.py b/passbook/saml_idp/models.py index 55313e081..c10932306 100644 --- a/passbook/saml_idp/models.py +++ b/passbook/saml_idp/models.py @@ -42,7 +42,7 @@ class SAMLProvider(Provider): """Get link to download XML metadata for admin interface""" try: # pylint: disable=no-member - return reverse('passbook_saml_idp:metadata_xml', + return reverse('passbook_saml_idp:saml-metadata', kwargs={'application': self.application.slug}) except Provider.application.RelatedObjectDoesNotExist: return None diff --git a/passbook/saml_idp/templates/saml/idp/settings.html b/passbook/saml_idp/templates/saml/idp/settings.html index 812510fff..3e09645f5 100644 --- a/passbook/saml_idp/templates/saml/idp/settings.html +++ b/passbook/saml_idp/templates/saml/idp/settings.html @@ -39,7 +39,7 @@ diff --git a/passbook/saml_idp/urls.py b/passbook/saml_idp/urls.py index 81f99ca3a..33a67a66c 100644 --- a/passbook/saml_idp/urls.py +++ b/passbook/saml_idp/urls.py @@ -4,13 +4,14 @@ from django.urls import path from passbook.saml_idp import views urlpatterns = [ - path('login//', - views.LoginBeginView.as_view(), name="saml_login_begin"), - path('login//initiate/', - views.InitiateLoginView.as_view(), name="saml_login_init"), - path('login//process/', - views.LoginProcessView.as_view(), name='saml_login_process'), - path('logout/', views.LogoutView.as_view(), name="saml_logout"), - path('metadata//', - views.DescriptorDownloadView.as_view(), name='metadata_xml'), + path('/login/', + views.LoginBeginView.as_view(), name="saml-login"), + path('/login/initiate/', + views.InitiateLoginView.as_view(), name="saml-login-initiate"), + path('/login/process/', + views.LoginProcessView.as_view(), name='saml-login-process'), + path('/logout/', views.LogoutView.as_view(), name="saml-logout"), + path('/logout/slo/', views.SLOLogout.as_view(), name="saml-logout-slo"), + path('/metadata/', + views.DescriptorDownloadView.as_view(), name='saml-metadata'), ] diff --git a/passbook/saml_idp/views.py b/passbook/saml_idp/views.py index dae9a412a..244755d28 100644 --- a/passbook/saml_idp/views.py +++ b/passbook/saml_idp/views.py @@ -14,6 +14,7 @@ from django.views.decorators.csrf import csrf_exempt from signxml.util import strip_pem_header from passbook.core.models import Application +from passbook.core.policies import PolicyEngine from passbook.lib.config import CONFIG from passbook.lib.mixins import CSRFExemptMixin from passbook.lib.utils.template import render_to_string @@ -75,7 +76,7 @@ class LoginBeginView(LoginRequiredMixin, View): return HttpResponseBadRequest('the SAML request payload is missing') request.session['RelayState'] = source.get('RelayState', '') - return redirect(reverse('passbook_saml_idp:saml_login_process', kwargs={ + return redirect(reverse('passbook_saml_idp:saml-login-process', kwargs={ 'application': application })) @@ -94,17 +95,22 @@ class RedirectToSPView(LoginRequiredMixin, View): }) + class LoginProcessView(ProviderMixin, LoginRequiredMixin, View): """Processor-based login continuation. Presents a SAML 2.0 Assertion for POSTing back to the Service Provider.""" + def _has_access(self): + """Check if user has access to application""" + policy_engine = PolicyEngine(self.provider.application.policies.all()) + policy_engine.for_user(self.request.user) + return policy_engine.result + def get(self, request, application): """Handle get request, i.e. render form""" LOGGER.debug("Request: %s", request) # Check if user has access - access = True - # TODO: Check access here - if self.provider.application.skip_authorization and access: + if self.provider.application.skip_authorization and self._has_access(): ctx = self.provider.processor.generate_response() # TODO: AuditLog Skipped Authz return RedirectToSPView.as_view()( @@ -122,9 +128,7 @@ class LoginProcessView(ProviderMixin, LoginRequiredMixin, View): """Handle post request, return back to ACS""" LOGGER.debug("Request: %s", request) # Check if user has access - access = True - # TODO: Check access here - if request.POST.get('ACSUrl', None) and access: + if request.POST.get('ACSUrl', None) and self._has_access(): # User accepted request # TODO: AuditLog accepted return RedirectToSPView.as_view()( @@ -144,7 +148,7 @@ class LogoutView(CSRFExemptMixin, LoginRequiredMixin, View): returns a standard logged-out page. (SalesForce and others use this method, though it's technically not SAML 2.0).""" - def get(self, request): + def get(self, request, application): """Perform logout""" logout(request) @@ -164,11 +168,10 @@ class SLOLogout(CSRFExemptMixin, LoginRequiredMixin, View): """Receives a SAML 2.0 LogoutRequest from a Service Provider, logs out the user and returns a standard logged-out page.""" - def post(self, request): + def post(self, request, application): """Perform logout""" request.session['SAMLRequest'] = request.POST['SAMLRequest'] # TODO: Parse SAML LogoutRequest from POST data, similar to login_process(). - # TODO: Add a URL dispatch for this view. # TODO: Modify the base processor to handle logouts? # TODO: Combine this with login_process(), since they are so very similar? # TODO: Format a LogoutResponse and return it to the browser. @@ -183,8 +186,8 @@ class DescriptorDownloadView(ProviderMixin, View): def get(self, request, application): """Replies with the XML Metadata IDSSODescriptor.""" entity_id = CONFIG.y('saml_idp.issuer') - slo_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml_logout')) - sso_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml_login_begin', kwargs={ + slo_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml-logout')) + sso_url = request.build_absolute_uri(reverse('passbook_saml_idp:saml-login', kwargs={ 'application': application })) pubkey = strip_pem_header(self.provider.signing_cert.replace('\r', '')).replace('\n', '')