From 813dd2894f1aef16ee08f74916065ef02884d437 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 7 May 2020 00:32:03 +0200 Subject: [PATCH] *: add pyright type checking --- .github/workflows/ci.yml | 7 +++++++ passbook/audit/models.py | 4 +++- passbook/policies/expression/evaluator.py | 4 ++-- passbook/providers/oauth/views/oauth2.py | 15 +++++++++------ passbook/providers/saml/models.py | 6 ++++-- scripts/pre-commit.sh | 1 + 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aeaa46de5..78828356c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,13 @@ jobs: run: sudo pip install -U wheel pipenv && pipenv install --dev - name: Lint with bandit run: pipenv run bandit -r passbook + pyright: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - uses: actions/setup-node@v1 + - name: Run pyright + run: pyright # Actual CI tests migrations: needs: diff --git a/passbook/audit/models.py b/passbook/audit/models.py index 85bf5695e..4e92dc6f8 100644 --- a/passbook/audit/models.py +++ b/passbook/audit/models.py @@ -66,7 +66,9 @@ class EventAction(Enum): @staticmethod def as_choices(): """Generate choices of actions used for database""" - return tuple((x, y.value) for x, y in EventAction.__members__.items()) + return tuple( + (x, y.value) for x, y in getattr(EventAction, "__members__").items() + ) class Event(UUIDModel): diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index 2a0e0193a..a2c4d1b3a 100644 --- a/passbook/policies/expression/evaluator.py +++ b/passbook/policies/expression/evaluator.py @@ -1,6 +1,6 @@ """passbook expression policy evaluator""" import re -from typing import TYPE_CHECKING, Any, Dict +from typing import TYPE_CHECKING, Any, Dict, Optional from django.core.exceptions import ValidationError from jinja2 import Undefined @@ -72,7 +72,7 @@ class Evaluator: except TemplateSyntaxError as exc: return PolicyResult(False, str(exc)) try: - result = expression.render( + result: Optional[Any] = expression.render( request=request, **self._get_expression_context(request) ) if isinstance(result, Undefined): diff --git a/passbook/providers/oauth/views/oauth2.py b/passbook/providers/oauth/views/oauth2.py index 3006f08e6..9d57ac179 100644 --- a/passbook/providers/oauth/views/oauth2.py +++ b/passbook/providers/oauth/views/oauth2.py @@ -1,8 +1,11 @@ """passbook OAuth2 Views""" +from typing import Optional from urllib.parse import urlencode from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin +from django.forms import Form +from django.http import HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect, reverse from django.utils.translation import ugettext as _ from oauth2_provider.views.base import AuthorizationView @@ -36,7 +39,7 @@ class OAuthPermissionDenied(PermissionDeniedView): class PassbookAuthorizationView(AccessMixin, AuthorizationView): """Custom OAuth2 Authorization View which checks policies, etc""" - _application = None + _application: Optional[Application] = None def _inject_response_type(self): """Inject response_type into querystring if not set""" @@ -47,7 +50,7 @@ class PassbookAuthorizationView(AccessMixin, AuthorizationView): reverse("passbook_providers_oauth:oauth2-ok-authorize") + "?" + querystring ) - def dispatch(self, request, *args, **kwargs): + def dispatch(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: """Update OAuth2Provider's skip_authorization state""" # Get client_id to get provider, so we can update skip_authorization field client_id = request.GET.get("client_id") @@ -69,12 +72,12 @@ class PassbookAuthorizationView(AccessMixin, AuthorizationView): # Some clients don't pass response_type, so we default to code if "response_type" not in request.GET: return self._inject_response_type() - actual_response = super().dispatch(request, *args, **kwargs) + actual_response = AuthorizationView.dispatch(self, request, *args, **kwargs) if actual_response.status_code == 400: - LOGGER.debug(request.GET.get("redirect_uri")) + LOGGER.debug("Bad request", redirect_uri=request.GET.get("redirect_uri")) return actual_response - def form_valid(self, form): + def form_valid(self, form: Form): # User has clicked on "Authorize" Event.new( EventAction.AUTHORIZE_APPLICATION, authorized_application=self._application, @@ -84,4 +87,4 @@ class PassbookAuthorizationView(AccessMixin, AuthorizationView): user=self.request.user, application=self._application, ) - return super().form_valid(form) + return AuthorizationView.form_valid(self, form) diff --git a/passbook/providers/saml/models.py b/passbook/providers/saml/models.py index f48eaba49..e55f3719c 100644 --- a/passbook/providers/saml/models.py +++ b/passbook/providers/saml/models.py @@ -100,7 +100,7 @@ class SAMLProvider(Provider): self._meta.get_field("processor_path").choices = get_provider_choices() @property - def processor(self) -> Processor: + def processor(self) -> Optional[Processor]: """Return selected processor as instance""" if not self._processor: try: @@ -163,4 +163,6 @@ class SAMLPropertyMapping(PropertyMapping): def get_provider_choices(): """Return tuple of class_path, class name of all providers.""" - return [(class_to_path(x), x.__name__) for x in Processor.__subclasses__()] + return [ + (class_to_path(x), x.__name__) for x in Processor.__dict__["__subclasses__"]() + ] diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh index 1658c5a96..e97bac37c 100755 --- a/scripts/pre-commit.sh +++ b/scripts/pre-commit.sh @@ -1,5 +1,6 @@ #!/bin/bash -xe isort -rc passbook +pyright black passbook scripts/coverage.sh bandit -r passbook