From 9267d0c1dd172f34343111822d6d72bac34f431e Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 18 Feb 2020 22:12:51 +0100 Subject: [PATCH] all: general maintenance, prepare for pyright --- passbook/core/models.py | 4 +++- passbook/factors/otp/views.py | 2 +- passbook/lib/boilerplate.py | 12 ----------- passbook/lib/mixins.py | 10 ++++++++++ passbook/policies/struct.py | 6 +++--- passbook/providers/oidc/lib.py | 30 ++++++++++++++++++---------- passbook/providers/saml/views.py | 6 +++++- passbook/sources/oauth/views/core.py | 3 ++- passbook/sources/saml/models.py | 6 ++++-- passbook/sources/saml/urls.py | 8 ++++---- passbook/sources/saml/views.py | 16 +++++++-------- pyrightconfig.json | 3 +++ 12 files changed, 62 insertions(+), 44 deletions(-) delete mode 100644 passbook/lib/boilerplate.py create mode 100644 pyrightconfig.json diff --git a/passbook/core/models.py b/passbook/core/models.py index d895bc318..530c08b60 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -304,7 +304,9 @@ class PropertyMapping(UUIDModel): form = "" objects = InheritanceManager() - def evaluate(self, user: User, request: HttpRequest, **kwargs) -> Any: + def evaluate( + self, user: Optional[User], request: Optional[HttpRequest], **kwargs + ) -> Any: """Evaluate `self.expression` using `**kwargs` as Context.""" try: expression = NATIVE_ENVIRONMENT.from_string(self.expression) diff --git a/passbook/factors/otp/views.py b/passbook/factors/otp/views.py index 22a3c23ca..1ff3da602 100644 --- a/passbook/factors/otp/views.py +++ b/passbook/factors/otp/views.py @@ -19,7 +19,7 @@ from structlog import get_logger from passbook.audit.models import Event, EventAction from passbook.factors.otp.forms import OTPSetupForm from passbook.factors.otp.utils import otpauth_url -from passbook.lib.boilerplate import NeverCacheMixin +from passbook.lib.mixins import NeverCacheMixin from passbook.lib.config import CONFIG OTP_SESSION_KEY = "passbook_factors_otp_key" diff --git a/passbook/lib/boilerplate.py b/passbook/lib/boilerplate.py deleted file mode 100644 index 38c12800f..000000000 --- a/passbook/lib/boilerplate.py +++ /dev/null @@ -1,12 +0,0 @@ -"""passbook django boilerplate code""" -from django.utils.decorators import method_decorator -from django.views.decorators.cache import never_cache - - -class NeverCacheMixin: - """Use never_cache as mixin for CBV""" - - @method_decorator(never_cache) - def dispatch(self, *args, **kwargs): - """Use never_cache as mixin for CBV""" - return super().dispatch(*args, **kwargs) diff --git a/passbook/lib/mixins.py b/passbook/lib/mixins.py index c83b0676a..4c7a02837 100644 --- a/passbook/lib/mixins.py +++ b/passbook/lib/mixins.py @@ -1,4 +1,5 @@ """passbook util mixins""" +from django.views.decorators.cache import never_cache from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt @@ -10,3 +11,12 @@ class CSRFExemptMixin: def dispatch(self, *args, **kwargs): """wrapper to apply @csrf_exempt to CBV""" return super().dispatch(*args, **kwargs) + + +class NeverCacheMixin: + """Use never_cache as mixin for CBV""" + + @method_decorator(never_cache) + def dispatch(self, *args, **kwargs): + """Use never_cache as mixin for CBV""" + return super().dispatch(*args, **kwargs) diff --git a/passbook/policies/struct.py b/passbook/policies/struct.py index 126602690..99bb890cc 100644 --- a/passbook/policies/struct.py +++ b/passbook/policies/struct.py @@ -1,7 +1,7 @@ """policy structures""" from __future__ import annotations -from typing import TYPE_CHECKING, List +from typing import TYPE_CHECKING, Tuple from django.db.models import Model from django.http import HttpRequest @@ -27,8 +27,8 @@ class PolicyRequest: class PolicyResult: """Small data-class to hold policy results""" - passing: bool = False - messages: List[str] = [] + passing: bool + messages: Tuple[str] def __init__(self, passing: bool, *messages: str): self.passing = passing diff --git a/passbook/providers/oidc/lib.py b/passbook/providers/oidc/lib.py index 73bb953d4..334607afa 100644 --- a/passbook/providers/oidc/lib.py +++ b/passbook/providers/oidc/lib.py @@ -15,24 +15,32 @@ from passbook.policies.engine import PolicyEngine LOGGER = get_logger() +def client_related_provider(client: Client) -> Optional[Provider]: + """Lookup related Application from Client""" + # because oidc_provider is also used by app_gw, we can't be + # sure an OpenIDPRovider instance exists. hence we look through all related models + # and choose the one that inherits from Provider, which is guaranteed to + # have the application property + collector = Collector(using="default") + collector.collect([client]) + for _, related in collector.data.items(): + related_object = next(iter(related)) + if isinstance(related_object, Provider): + return related_object + return None + + def check_permissions( request: HttpRequest, user: User, client: Client ) -> Optional[HttpResponse]: """Check permissions, used for https://django-oidc-provider.readthedocs.io/en/latest/ sections/settings.html#oidc-after-userlogin-hook""" + provider = client_related_provider(client) + if not provider: + return redirect("passbook_providers_oauth:oauth2-permission-denied") try: - # because oidc_provider is also used by app_gw, we can't be - # sure an OpenIDPRovider instance exists. hence we look through all related models - # and choose the one that inherits from Provider, which is guaranteed to - # have the application property - collector = Collector(using="default") - collector.collect([client]) - for _, related in collector.data.items(): - related_object = next(iter(related)) - if isinstance(related_object, Provider): - application = related_object.application - break + application = provider.application except Application.DoesNotExist: return redirect("passbook_providers_oauth:oauth2-permission-denied") LOGGER.debug( diff --git a/passbook/providers/saml/views.py b/passbook/providers/saml/views.py index a065e1805..2675dfbeb 100644 --- a/passbook/providers/saml/views.py +++ b/passbook/providers/saml/views.py @@ -42,7 +42,11 @@ class AccessRequiredView(AccessMixin, View): application = get_object_or_404( Application, slug=self.kwargs["application"] ) - self._provider = get_object_or_404(SAMLProvider, pk=application.provider_id) + provider: SAMLProvider = get_object_or_404( + SAMLProvider, pk=application.provider_id + ) + self._provider = provider + return self._provider return self._provider def _has_access(self) -> bool: diff --git a/passbook/sources/oauth/views/core.py b/passbook/sources/oauth/views/core.py index fded716a8..0ac48a28b 100644 --- a/passbook/sources/oauth/views/core.py +++ b/passbook/sources/oauth/views/core.py @@ -1,4 +1,5 @@ """Core OAauth Views""" +from typing import Callable, Optional from django.conf import settings from django.contrib import messages @@ -23,7 +24,7 @@ LOGGER = get_logger() class OAuthClientMixin: "Mixin for getting OAuth client for a source." - client_class = None + client_class: Optional[Callable] = None def get_client(self, source): "Get instance of the OAuth client for this source." diff --git a/passbook/sources/saml/models.py b/passbook/sources/saml/models.py index ae4cb6c6b..273380382 100644 --- a/passbook/sources/saml/models.py +++ b/passbook/sources/saml/models.py @@ -21,13 +21,15 @@ class SAMLSource(Source): @property def login_button(self): - url = reverse_lazy("passbook_sources_saml:login", kwargs={"source": self.slug}) + url = reverse_lazy( + "passbook_sources_saml:login", kwargs={"source_slug": self.slug} + ) return url, "", self.name @property def additional_info(self): metadata_url = reverse_lazy( - "passbook_sources_saml:metadata", kwargs={"source": self} + "passbook_sources_saml:metadata", kwargs={"source_slug": self} ) return f'Metadata Download' diff --git a/passbook/sources/saml/urls.py b/passbook/sources/saml/urls.py index d5e5ad04d..22b0a8fd2 100644 --- a/passbook/sources/saml/urls.py +++ b/passbook/sources/saml/urls.py @@ -4,8 +4,8 @@ from django.urls import path from passbook.sources.saml.views import ACSView, InitiateView, MetadataView, SLOView urlpatterns = [ - path("/", InitiateView.as_view(), name="login"), - path("/acs/", ACSView.as_view(), name="acs"), - path("/slo/", SLOView.as_view(), name="slo"), - path("/metadata/", MetadataView.as_view(), name="metadata"), + path("/", InitiateView.as_view(), name="login"), + path("/acs/", ACSView.as_view(), name="acs"), + path("/slo/", SLOView.as_view(), name="slo"), + path("/metadata/", MetadataView.as_view(), name="metadata"), ] diff --git a/passbook/sources/saml/views.py b/passbook/sources/saml/views.py index 5610f060f..18a07f5ba 100644 --- a/passbook/sources/saml/views.py +++ b/passbook/sources/saml/views.py @@ -24,9 +24,9 @@ from passbook.sources.saml.xml_render import get_authnrequest_xml class InitiateView(View): """Get the Form with SAML Request, which sends us to the IDP""" - def get(self, request: HttpRequest, source: str) -> HttpResponse: + def get(self, request: HttpRequest, source_slug: str) -> HttpResponse: """Replies with an XHTML SSO Request.""" - source: SAMLSource = get_object_or_404(SAMLSource, slug=source) + source: SAMLSource = get_object_or_404(SAMLSource, slug=source_slug) if not source.enabled: raise Http404 sso_destination = request.GET.get("next", None) @@ -56,9 +56,9 @@ class InitiateView(View): class ACSView(View): """AssertionConsumerService, consume assertion and log user in""" - def post(self, request: HttpRequest, source: str) -> HttpResponse: + def post(self, request: HttpRequest, source_slug: str) -> HttpResponse: """Handles a POSTed SSO Assertion and logs the user in.""" - source: SAMLSource = get_object_or_404(SAMLSource, slug=source) + source: SAMLSource = get_object_or_404(SAMLSource, slug=source_slug) if not source.enabled: raise Http404 # sso_session = request.POST.get('RelayState', None) @@ -74,9 +74,9 @@ class ACSView(View): class SLOView(View): """Single-Logout-View""" - def dispatch(self, request: HttpRequest, source: str) -> HttpResponse: + def dispatch(self, request: HttpRequest, source_slug: str) -> HttpResponse: """Replies with an XHTML SSO Request.""" - source: SAMLSource = get_object_or_404(SAMLSource, slug=source) + source: SAMLSource = get_object_or_404(SAMLSource, slug=source_slug) if not source.enabled: raise Http404 logout(request) @@ -93,9 +93,9 @@ class SLOView(View): class MetadataView(View): """Return XML Metadata for IDP""" - def dispatch(self, request: HttpRequest, source: str) -> HttpResponse: + def dispatch(self, request: HttpRequest, source_slug: str) -> HttpResponse: """Replies with the XML Metadata SPSSODescriptor.""" - source: SAMLSource = get_object_or_404(SAMLSource, slug=source) + source: SAMLSource = get_object_or_404(SAMLSource, slug=source_slug) entity_id = get_entity_id(request, source) return render_xml( request, diff --git a/pyrightconfig.json b/pyrightconfig.json new file mode 100644 index 000000000..95021473a --- /dev/null +++ b/pyrightconfig.json @@ -0,0 +1,3 @@ +{ + "reportMissingTypeStubs": false +}