From cc0b8164b0a5a5ee76e5b5a7ae888e445cd188c1 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 1 Jul 2020 23:18:10 +0200 Subject: [PATCH] providers/*: use PolicyAccessMixin to simplify --- .../views/access.py => policies/mixins.py} | 32 ++++++++++++----- passbook/providers/oauth/views/oauth2.py | 9 ++--- passbook/providers/oidc/views.py | 9 ++--- passbook/providers/saml/views.py | 35 ++++--------------- passbook/providers/samlv2/views/base.py | 4 +-- 5 files changed, 38 insertions(+), 51 deletions(-) rename passbook/{core/views/access.py => policies/mixins.py} (59%) diff --git a/passbook/core/views/access.py b/passbook/policies/mixins.py similarity index 59% rename from passbook/core/views/access.py rename to passbook/policies/mixins.py index 287a12f80..e3a528833 100644 --- a/passbook/core/views/access.py +++ b/passbook/policies/mixins.py @@ -1,4 +1,6 @@ """passbook access helper classes""" +from typing import Optional + from django.contrib import messages from django.http import HttpRequest from django.utils.translation import gettext as _ @@ -11,13 +13,16 @@ from passbook.policies.types import PolicyResult LOGGER = get_logger() -class AccessMixin: +class BaseMixin: + """Base Mixin class, used to annotate View Member variables""" + + request: HttpRequest + + +class PolicyAccessMixin(BaseMixin): """Mixin class for usage in Authorization views. Provider functions to check application access, etc""" - # request is set by view but since this Mixin has no base class - request: HttpRequest = None - def provider_to_application(self, provider: Provider) -> Application: """Lookup application assigned to provider, throw error if no application assigned""" try: @@ -32,9 +37,20 @@ class AccessMixin: ) raise exc - def user_has_access(self, application: Application, user: User) -> PolicyResult: + def user_has_access( + self, application: Application, user: Optional[User] = None + ) -> PolicyResult: """Check if user has access to application.""" - LOGGER.debug("Checking permissions", user=user, application=application) - policy_engine = PolicyEngine(application, user, self.request) + user = user or self.request.user + policy_engine = PolicyEngine( + application, user or self.request.user, self.request + ) policy_engine.build() - return policy_engine.result + result = policy_engine.result + LOGGER.debug( + "AccessMixin user_has_access", user=user, app=application, result=result, + ) + if not result.passing: + for message in result.messages: + messages.error(self.request, _(message)) + return result diff --git a/passbook/providers/oauth/views/oauth2.py b/passbook/providers/oauth/views/oauth2.py index 95d5a93d8..058e08b8e 100644 --- a/passbook/providers/oauth/views/oauth2.py +++ b/passbook/providers/oauth/views/oauth2.py @@ -1,5 +1,4 @@ """passbook OAuth2 Views""" -from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect @@ -11,7 +10,6 @@ from structlog import get_logger from passbook.audit.models import Event, EventAction from passbook.core.models import Application -from passbook.core.views.access import AccessMixin from passbook.flows.models import in_memory_stage from passbook.flows.planner import ( PLAN_CONTEXT_APPLICATION, @@ -21,6 +19,7 @@ from passbook.flows.planner import ( from passbook.flows.stage import StageView from passbook.flows.views import SESSION_KEY_PLAN from passbook.lib.utils.urls import redirect_with_qs +from passbook.policies.mixins import PolicyAccessMixin from passbook.providers.oauth.models import OAuth2Provider from passbook.stages.consent.stage import PLAN_CONTEXT_CONSENT_TEMPLATE @@ -38,7 +37,7 @@ PLAN_CONTEXT_NONCE = "nonce" PLAN_CONTEXT_SCOPE_DESCRIPTION = "scope_descriptions" -class AuthorizationFlowInitView(AccessMixin, LoginRequiredMixin, View): +class AuthorizationFlowInitView(PolicyAccessMixin, LoginRequiredMixin, View): """OAuth2 Flow initializer, checks access to application and starts flow""" # pylint: disable=unused-argument @@ -51,10 +50,8 @@ class AuthorizationFlowInitView(AccessMixin, LoginRequiredMixin, View): except Application.DoesNotExist: return redirect("passbook_providers_oauth:oauth2-permission-denied") # Check permissions - result = self.user_has_access(application, request.user) + result = self.user_has_access(application) if not result.passing: - for policy_message in result.messages: - messages.error(request, policy_message) return redirect("passbook_providers_oauth:oauth2-permission-denied") # Regardless, we start the planner and return to it planner = FlowPlanner(provider.authorization_flow) diff --git a/passbook/providers/oidc/views.py b/passbook/providers/oidc/views.py index b537222ed..337490cfa 100644 --- a/passbook/providers/oidc/views.py +++ b/passbook/providers/oidc/views.py @@ -1,5 +1,4 @@ """passbook OIDC Views""" -from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.http import HttpRequest, HttpResponse, JsonResponse from django.shortcuts import get_object_or_404, redirect, reverse @@ -11,7 +10,6 @@ from oidc_provider.views import AuthorizeView from structlog import get_logger from passbook.core.models import Application -from passbook.core.views.access import AccessMixin from passbook.flows.models import in_memory_stage from passbook.flows.planner import ( PLAN_CONTEXT_APPLICATION, @@ -22,6 +20,7 @@ from passbook.flows.planner import ( from passbook.flows.stage import StageView from passbook.flows.views import SESSION_KEY_PLAN from passbook.lib.utils.urls import redirect_with_qs +from passbook.policies.mixins import PolicyAccessMixin from passbook.providers.oidc.models import OpenIDProvider from passbook.stages.consent.stage import PLAN_CONTEXT_CONSENT_TEMPLATE @@ -31,7 +30,7 @@ PLAN_CONTEXT_PARAMS = "params" PLAN_CONTEXT_SCOPES = "scopes" -class AuthorizationFlowInitView(AccessMixin, LoginRequiredMixin, View): +class AuthorizationFlowInitView(PolicyAccessMixin, LoginRequiredMixin, View): """OIDC Flow initializer, checks access to application and starts flow""" # pylint: disable=unused-argument @@ -44,10 +43,8 @@ class AuthorizationFlowInitView(AccessMixin, LoginRequiredMixin, View): except Application.DoesNotExist: return redirect("passbook_providers_oauth:oauth2-permission-denied") # Check permissions - result = self.user_has_access(application, request.user) + result = self.user_has_access(application) if not result.passing: - for policy_message in result.messages: - messages.error(request, policy_message) return redirect("passbook_providers_oauth:oauth2-permission-denied") # Extract params so we can save them in the plan context endpoint = AuthorizeEndpoint(request) diff --git a/passbook/providers/saml/views.py b/passbook/providers/saml/views.py index adc741475..fbb200460 100644 --- a/passbook/providers/saml/views.py +++ b/passbook/providers/saml/views.py @@ -1,7 +1,6 @@ """passbook SAML IDP Views""" from typing import Optional -from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.core.exceptions import PermissionDenied from django.core.validators import URLValidator @@ -9,7 +8,6 @@ from django.http import HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect, render, reverse from django.utils.decorators import method_decorator from django.utils.http import urlencode -from django.utils.translation import gettext as _ from django.views import View from django.views.decorators.csrf import csrf_exempt from signxml.util import strip_pem_header @@ -28,7 +26,7 @@ from passbook.flows.views import SESSION_KEY_PLAN from passbook.lib.utils.template import render_to_string from passbook.lib.utils.urls import redirect_with_qs from passbook.lib.views import bad_request_message -from passbook.policies.engine import PolicyEngine +from passbook.policies.mixins import PolicyAccessMixin from passbook.providers.saml.exceptions import CannotHandleAssertion from passbook.providers.saml.models import SAMLBindings, SAMLProvider from passbook.providers.saml.processors.types import SAMLResponseParams @@ -42,34 +40,13 @@ SESSION_KEY_RELAY_STATE = "RelayState" SESSION_KEY_PARAMS = "SAMLParams" -class SAMLAccessMixin: - """SAML base access mixin, checks access to an application based on its policies""" - - request: HttpRequest - application: Application - provider: SAMLProvider - - def _has_access(self) -> bool: - """Check if user has access to application, add an error if not""" - policy_engine = PolicyEngine(self.application, self.request.user, self.request) - policy_engine.build() - result = policy_engine.result - LOGGER.debug( - "SAMLFlowInit _has_access", - user=self.request.user, - app=self.application, - result=result, - ) - if not result.passing: - for message in result.messages: - messages.error(self.request, _(message)) - return result.passing - - -class SAMLSSOView(LoginRequiredMixin, SAMLAccessMixin, View): +class SAMLSSOView(LoginRequiredMixin, PolicyAccessMixin, View): """"SAML SSO Base View, which plans a flow and injects our final stage. Calls get/post handler.""" + application: Application + provider: SAMLProvider + def dispatch( self, request: HttpRequest, *args, application_slug: str, **kwargs ) -> HttpResponse: @@ -77,7 +54,7 @@ class SAMLSSOView(LoginRequiredMixin, SAMLAccessMixin, View): self.provider: SAMLProvider = get_object_or_404( SAMLProvider, pk=self.application.provider_id ) - if not self._has_access(): + if not self.user_has_access(self.application): raise PermissionDenied() # Call the method handler, which checks the SAML Request method_response = super().dispatch(request, *args, application_slug, **kwargs) diff --git a/passbook/providers/samlv2/views/base.py b/passbook/providers/samlv2/views/base.py index 6bea319bc..d25277bc9 100644 --- a/passbook/providers/samlv2/views/base.py +++ b/passbook/providers/samlv2/views/base.py @@ -6,12 +6,12 @@ from django.shortcuts import get_object_or_404 from django.views import View from passbook.core.models import Application -from passbook.core.views.access import AccessMixin +from passbook.policies.mixins import PolicyAccessMixin from passbook.providers.samlv2.saml.constants import SESSION_KEY from passbook.providers.samlv2.saml.parser import SAMLRequest -class BaseSAMLView(AccessMixin, View): +class BaseSAMLView(PolicyAccessMixin, View): """Base SAML View to resolve app_slug""" application: Application