policies: add PolicyAccessView, which does complete access checking

passbook/policies/views.py

@@ -1,11 +1,12 @@
 """passbook access helper classes"""
-from typing import Optional
+from typing import Any, Optional
 
 from django.contrib import messages
 from django.contrib.auth.mixins import AccessMixin
 from django.contrib.auth.views import redirect_to_login
 from django.http import HttpRequest, HttpResponse
 from django.utils.translation import gettext as _
+from django.views.generic.base import View
 from structlog import get_logger
 
 from passbook.core.models import Application, Provider, User
@@ -23,16 +24,40 @@ class BaseMixin:
     request: HttpRequest
 
 
-class PolicyAccessMixin(BaseMixin, AccessMixin):
+class PolicyAccessView(AccessMixin, View):
     """Mixin class for usage in Authorization views.
     Provider functions to check application access, etc"""
 
-    def handle_no_permission(self, application: Optional[Application] = None):
+    provider: Provider
+    application: Application
+
+    def resolve_provider_application(self):
+        """Resolve self.provider and self.application. *.DoesNotExist Exceptions cause a normal
+        AccessDenied view to be shown. An Http404 exception
+        is not caught, and will return directly"""
+        raise NotImplementedError
+
+    def dispatch(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
+        try:
+            self.resolve_provider_application()
+        except (Application.DoesNotExist, Provider.DoesNotExist):
+            return self.handle_no_permission_authenticated()
+        # Check if user is unauthenticated, so we pass the application
+        # for the identification stage
+        if not request.user.is_authenticated:
+            return self.handle_no_permission()
+        # Check permissions
+        result = self.user_has_access()
+        if not result.passing:
+            return self.handle_no_permission_authenticated(result)
+        return super().dispatch(request, *args, **kwargs)
+
+    def handle_no_permission(self) -> HttpResponse:
         """User has no access and is not authenticated, so we remember the application
         they try to access and redirect to the login URL. The application is saved to show
         a hint on the Identification Stage what the user should login for."""
-        if application:
+        if self.application:
-            self.request.session[SESSION_KEY_APPLICATION_PRE] = application
+            self.request.session[SESSION_KEY_APPLICATION_PRE] = self.application
         return redirect_to_login(
             self.request.get_full_path(),
             self.get_login_url(),
@@ -48,34 +73,18 @@ class PolicyAccessMixin(BaseMixin, AccessMixin):
             response.policy_result = result
         return response
 
-    def provider_to_application(self, provider: Provider) -> Application:
+    def user_has_access(self, user: Optional[User] = None) -> PolicyResult:
-        """Lookup application assigned to provider, throw error if no application assigned"""
-        try:
-            return provider.application
-        except Application.DoesNotExist as exc:
-            messages.error(
-                self.request,
-                _(
-                    'Provider "%(name)s" has no application assigned'
-                    % {"name": provider}
-                ),
-            )
-            raise exc
-
-    def user_has_access(
-        self, application: Application, user: Optional[User] = None
-    ) -> PolicyResult:
         """Check if user has access to application."""
         user = user or self.request.user
         policy_engine = PolicyEngine(
-            application, user or self.request.user, self.request
+            self.application, user or self.request.user, self.request
         )
         policy_engine.build()
         result = policy_engine.result
         LOGGER.debug(
             "AccessMixin user_has_access",
             user=user,
-            app=application,
+            app=self.application,
             result=result,
         )
         if not result.passing:

passbook/providers/oauth2/views/authorize.py

@@ -7,7 +7,6 @@ from uuid import uuid4
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.utils import timezone
-from django.views import View
 from structlog import get_logger
 
 from passbook.audit.models import Event, EventAction
@@ -24,7 +23,7 @@ from passbook.flows.views import SESSION_KEY_PLAN
 from passbook.lib.utils.time import timedelta_from_string
 from passbook.lib.utils.urls import redirect_with_qs
 from passbook.lib.views import bad_request_message
-from passbook.policies.mixins import PolicyAccessMixin
+from passbook.policies.views import PolicyAccessView
 from passbook.providers.oauth2.constants import (
     PROMPT_CONSNET,
     PROMPT_NONE,
@@ -329,28 +328,17 @@ class OAuthFulfillmentStage(StageView):
         return urlunsplit(uri)
 
 
-class AuthorizationFlowInitView(PolicyAccessMixin, View):
+class AuthorizationFlowInitView(PolicyAccessView):
     """OAuth2 Flow initializer, checks access to application and starts flow"""
 
+    def resolve_provider_application(self):
+        client_id = self.request.GET.get("client_id")
+        self.provider = get_object_or_404(OAuth2Provider, client_id=client_id)
+        self.application = self.provider.application
+
     # pylint: disable=unused-argument
     def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         """Check access to application, start FlowPLanner, return to flow executor shell"""
-        client_id = request.GET.get("client_id")
-        # TODO: This whole block should be moved to a base class
-        provider = get_object_or_404(OAuth2Provider, client_id=client_id)
-        try:
-            application = self.provider_to_application(provider)
-        except Application.DoesNotExist:
-            return self.handle_no_permission_authenticated()
-        # Check if user is unauthenticated, so we pass the application
-        # for the identification stage
-        if not request.user.is_authenticated:
-            return self.handle_no_permission(application)
-        # Check permissions
-        result = self.user_has_access(application)
-        if not result.passing:
-            return self.handle_no_permission_authenticated(result)
-        # TODO: End block
         # Extract params so we can save them in the plan context
         try:
             params = OAuthAuthorizationParams.from_request(request)
@@ -358,14 +346,14 @@ class AuthorizationFlowInitView(PolicyAccessMixin, View):
             # pylint: disable=no-member
             return bad_request_message(request, error.description, title=error.error)
         # Regardless, we start the planner and return to it
-        planner = FlowPlanner(provider.authorization_flow)
+        planner = FlowPlanner(self.provider.authorization_flow)
         # planner.use_cache = False
         planner.allow_empty_flows = True
         plan: FlowPlan = planner.plan(
             self.request,
             {
                 PLAN_CONTEXT_SSO: True,
-                PLAN_CONTEXT_APPLICATION: application,
+                PLAN_CONTEXT_APPLICATION: self.application,
                 # OAuth2 related params
                 PLAN_CONTEXT_PARAMS: params,
                 PLAN_CONTEXT_SCOPE_DESCRIPTIONS: UserInfoView().get_scope_descriptions(
@@ -390,5 +378,5 @@ class AuthorizationFlowInitView(PolicyAccessMixin, View):
         return redirect_with_qs(
             "passbook_flows:flow-executor-shell",
             self.request.GET,
-            flow_slug=provider.authorization_flow.slug,
+            flow_slug=self.provider.authorization_flow.slug,
         )

passbook/providers/saml/views.py

@@ -23,7 +23,7 @@ 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.lib.views import bad_request_message
-from passbook.policies.mixins import PolicyAccessMixin
+from passbook.policies.views import PolicyAccessView
 from passbook.providers.saml.exceptions import CannotHandleAssertion
 from passbook.providers.saml.models import SAMLBindings, SAMLProvider
 from passbook.providers.saml.processors.assertion import AssertionProcessor
@@ -46,34 +46,35 @@ REQUEST_KEY_RELAY_STATE = "RelayState"
 SESSION_KEY_AUTH_N_REQUEST = "authn_request"
 
 
-class SAMLSSOView(PolicyAccessMixin, View):
+class SAMLSSOView(PolicyAccessView):
     """ "SAML SSO Base View, which plans a flow and injects our final stage.
     Calls get/post handler."""
 
-    application: Application
+    def resolve_provider_application(self):
-    provider: SAMLProvider
+        self.application = get_object_or_404(
-
+            Application, slug=self.kwargs["application_slug"]
-    def dispatch(
+        )
-        self, request: HttpRequest, *args, application_slug: str, **kwargs
-    ) -> HttpResponse:
-        self.application = get_object_or_404(Application, slug=application_slug)
         self.provider: SAMLProvider = get_object_or_404(
             SAMLProvider, pk=self.application.provider_id
         )
-        if not request.user.is_authenticated:
+
-            return self.handle_no_permission(self.application)
+    def check_saml_request(self) -> Optional[HttpRequest]:
-        has_access = self.user_has_access(self.application)
+        """Handler to verify the SAML Request. Must be implemented by a subclass"""
-        if not has_access.passing:
+        raise NotImplementedError
-            return self.handle_no_permission_authenticated(has_access)
+
-        # Call the method handler, which checks the SAML Request
+    # pylint: disable=unused-argument
-        method_response = super().dispatch(request, *args, application_slug, **kwargs)
+    def get(self, request: HttpRequest, application_slug: str) -> HttpResponse:
+        """Verify the SAML Request, and if valid initiate the FlowPlanner for the application"""
+        # Call the method handler, which checks the SAML
+        # Request and returns a HTTP Response on error
+        method_response = self.check_saml_request()
         if method_response:
             return method_response
         # Regardless, we start the planner and return to it
         planner = FlowPlanner(self.provider.authorization_flow)
         planner.allow_empty_flows = True
         plan = planner.plan(
-            self.request,
+            request,
             {
                 PLAN_CONTEXT_SSO: True,
                 PLAN_CONTEXT_APPLICATION: self.application,
@@ -81,23 +82,25 @@ class SAMLSSOView(PolicyAccessMixin, View):
             },
         )
         plan.append(in_memory_stage(SAMLFlowFinalView))
-        self.request.session[SESSION_KEY_PLAN] = plan
+        request.session[SESSION_KEY_PLAN] = plan
         return redirect_with_qs(
             "passbook_flows:flow-executor-shell",
-            self.request.GET,
+            request.GET,
             flow_slug=self.provider.authorization_flow.slug,
         )
 
+    def post(self, request: HttpRequest, application_slug: str) -> HttpResponse:
+        """GET and POST use the same handler, but we can't
+        override .dispatch easily because PolicyAccessView's dispatch"""
+        return self.get(request, application_slug)
+
 
 class SAMLSSOBindingRedirectView(SAMLSSOView):
     """SAML Handler for SSO/Redirect bindings, which are sent via GET"""
 
-    # pylint: disable=unused-argument
+    def check_saml_request(self) -> Optional[HttpRequest]:
-    def get(  # lgtm [py/similar-function]
-        self, request: HttpRequest, application_slug: str
-    ) -> Optional[HttpResponse]:
         """Handle REDIRECT bindings"""
-        if REQUEST_KEY_SAML_REQUEST not in request.GET:
+        if REQUEST_KEY_SAML_REQUEST not in self.request.GET:
             LOGGER.info("handle_saml_request: SAML payload missing")
             return bad_request_message(
                 self.request, "The SAML request payload is missing."
@@ -105,10 +108,10 @@ class SAMLSSOBindingRedirectView(SAMLSSOView):
 
         try:
             auth_n_request = AuthNRequestParser(self.provider).parse_detached(
-                request.GET[REQUEST_KEY_SAML_REQUEST],
+                self.request.GET[REQUEST_KEY_SAML_REQUEST],
-                request.GET.get(REQUEST_KEY_RELAY_STATE),
+                self.request.GET.get(REQUEST_KEY_RELAY_STATE),
-                request.GET.get(REQUEST_KEY_SAML_SIGNATURE),
+                self.request.GET.get(REQUEST_KEY_SAML_SIGNATURE),
-                request.GET.get(REQUEST_KEY_SAML_SIG_ALG),
+                self.request.GET.get(REQUEST_KEY_SAML_SIG_ALG),
             )
             self.request.session[SESSION_KEY_AUTH_N_REQUEST] = auth_n_request
         except CannotHandleAssertion as exc:
@@ -121,12 +124,9 @@ class SAMLSSOBindingRedirectView(SAMLSSOView):
 class SAMLSSOBindingPOSTView(SAMLSSOView):
     """SAML Handler for SSO/POST bindings"""
 
-    # pylint: disable=unused-argument
+    def check_saml_request(self) -> Optional[HttpRequest]:
-    def post(
-        self, request: HttpRequest, application_slug: str
-    ) -> Optional[HttpResponse]:
         """Handle POST bindings"""
-        if REQUEST_KEY_SAML_REQUEST not in request.POST:
+        if REQUEST_KEY_SAML_REQUEST not in self.request.POST:
             LOGGER.info("handle_saml_request: SAML payload missing")
             return bad_request_message(
                 self.request, "The SAML request payload is missing."
@@ -134,8 +134,8 @@ class SAMLSSOBindingPOSTView(SAMLSSOView):
 
         try:
             auth_n_request = AuthNRequestParser(self.provider).parse(
-                request.POST[REQUEST_KEY_SAML_REQUEST],
+                self.request.POST[REQUEST_KEY_SAML_REQUEST],
-                request.POST.get(REQUEST_KEY_RELAY_STATE),
+                self.request.POST.get(REQUEST_KEY_RELAY_STATE),
             )
             self.request.session[SESSION_KEY_AUTH_N_REQUEST] = auth_n_request
         except CannotHandleAssertion as exc:
@@ -147,10 +147,7 @@ class SAMLSSOBindingPOSTView(SAMLSSOView):
 class SAMLSSOBindingInitView(SAMLSSOView):
     """SAML Handler for for IdP Initiated login flows"""
 
-    # pylint: disable=unused-argument
+    def check_saml_request(self) -> Optional[HttpRequest]:
-    def get(
-        self, request: HttpRequest, application_slug: str
-    ) -> Optional[HttpResponse]:
         """Create SAML Response from scratch"""
         LOGGER.debug(
             "handle_saml_no_request: No SAML Request, using IdP-initiated flow."

swagger.yaml

@@ -122,7 +122,7 @@ paths:
   /core/applications/:
     get:
       operationId: core_applications_list
-      description: Application Viewset
+      description: Custom list method that checks Policy based access instead of guardian
       parameters:
         - name: ordering
           in: query
@@ -186,7 +186,7 @@ paths:
       tags:
         - core
     parameters: []
-  /core/applications/{pbm_uuid}/:
+  /core/applications/{slug}/:
     get:
       operationId: core_applications_read
       description: Application Viewset
@@ -240,12 +240,13 @@ paths:
       tags:
         - core
     parameters:
-      - name: pbm_uuid
+      - name: slug
         in: path
-        description: A UUID string identifying this Application.
+        description: Internal application name, used in URLs.
         required: true
         type: string
-        format: uuid
+        format: slug
+        pattern: ^[-a-zA-Z0-9_]+$
   /core/groups/:
     get:
       operationId: core_groups_list