From 3c2b8e5ee1584d9dc9fdd3fd6dfdc66fceb05d7f Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 20 Feb 2020 13:51:41 +0100 Subject: [PATCH] all: prefix all UI related methods with ui_, switch to property and return dataclass --- .../templates/administration/source/list.html | 2 +- passbook/core/models.py | 32 ++++++----------- .../templatetags/passbook_user_settings.py | 31 ++++++++++------- passbook/core/types.py | 29 ++++++++++++++++ passbook/core/views/authentication.py | 6 ++-- passbook/factors/otp/models.py | 13 ++++--- passbook/factors/password/models.py | 12 ++++--- passbook/policies/expression/evaluator.py | 2 +- passbook/sources/oauth/models.py | 34 +++++++++++-------- passbook/sources/saml/models.py | 16 +++++---- 10 files changed, 110 insertions(+), 67 deletions(-) create mode 100644 passbook/core/types.py diff --git a/passbook/admin/templates/administration/source/list.html b/passbook/admin/templates/administration/source/list.html index b2de358b2..e0e5c9e36 100644 --- a/passbook/admin/templates/administration/source/list.html +++ b/passbook/admin/templates/administration/source/list.html @@ -36,7 +36,7 @@ {{ source.name }} {{ source|fieldtype }} - {{ source.additional_info|safe }} + {{ source.ui_additional_info|safe|default:"" }} {% trans 'Edit' %} diff --git a/passbook/core/models.py b/passbook/core/models.py index 1d718a6f7..32bda701b 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -21,6 +21,7 @@ from jinja2.nativetypes import NativeEnvironment from model_utils.managers import InheritanceManager from structlog import get_logger +from passbook.core.types import UIUserSettings, UILoginButton from passbook.core.exceptions import PropertyMappingExpressionException from passbook.core.signals import password_changed from passbook.lib.models import CreatedUpdatedModel, UUIDModel @@ -102,19 +103,6 @@ class PolicyModel(UUIDModel, CreatedUpdatedModel): policies = models.ManyToManyField("Policy", blank=True) -class UserSettings: - """Dataclass for Factor and Source's user_settings""" - - name: str - icon: str - view_name: str - - def __init__(self, name: str, icon: str, view_name: str): - self.name = name - self.icon = icon - self.view_name = view_name - - class Factor(ExportModelOperationsMixin("factor"), PolicyModel): """Authentication factor, multiple instances of the same Factor can be used""" @@ -127,9 +115,10 @@ class Factor(ExportModelOperationsMixin("factor"), PolicyModel): type = "" form = "" - def user_settings(self) -> Optional[UserSettings]: + @property + def ui_user_settings(self) -> Optional[UIUserSettings]: """Entrypoint to integrate with User settings. Can either return None if no - user settings are available, or an instanace of UserSettings.""" + user settings are available, or an instanace of UIUserSettings.""" return None def __str__(self): @@ -181,19 +170,20 @@ class Source(ExportModelOperationsMixin("source"), PolicyModel): objects = InheritanceManager() @property - def login_button(self): - """Return a tuple of URL, Icon name and Name - if Source should get a link on the login page""" + def ui_login_button(self) -> Optional[UILoginButton]: + """If source uses a http-based flow, return UI Information about the login + button. If source doesn't use http-based flow, return None.""" return None @property - def additional_info(self): + def ui_additional_info(self) -> Optional[str]: """Return additional Info, such as a callback URL. Show in the administration interface.""" return None - def user_settings(self) -> Optional[UserSettings]: + @property + def ui_user_settings(self) -> Optional[UIUserSettings]: """Entrypoint to integrate with User settings. Can either return None if no - user settings are available, or an instanace of UserSettings.""" + user settings are available, or an instanace of UIUserSettings.""" return None def __str__(self): diff --git a/passbook/core/templatetags/passbook_user_settings.py b/passbook/core/templatetags/passbook_user_settings.py index 82c7ec60c..39773799e 100644 --- a/passbook/core/templatetags/passbook_user_settings.py +++ b/passbook/core/templatetags/passbook_user_settings.py @@ -1,46 +1,53 @@ """passbook user settings template tags""" -from typing import List +from typing import List, Iterable from django import template from django.template.context import RequestContext -from passbook.core.models import Factor, Source, UserSettings +from passbook.core.types import UIUserSettings +from passbook.core.models import Factor, Source from passbook.policies.engine import PolicyEngine register = template.Library() @register.simple_tag(takes_context=True) -def user_factors(context: RequestContext) -> List[UserSettings]: +def user_factors(context: RequestContext) -> List[UIUserSettings]: """Return list of all factors which apply to user""" user = context.get("request").user - _all_factors = ( + _all_factors: Iterable[Factor] = ( Factor.objects.filter(enabled=True).order_by("order").select_subclasses() ) - matching_factors: List[UserSettings] = [] + matching_factors: List[UIUserSettings] = [] for factor in _all_factors: - user_settings = factor.user_settings() + user_settings = factor.ui_user_settings + if not user_settings: + continue policy_engine = PolicyEngine( factor.policies.all(), user, context.get("request") ) policy_engine.build() - if policy_engine.passing and user_settings: + if policy_engine.passing: matching_factors.append(user_settings) return matching_factors @register.simple_tag(takes_context=True) -def user_sources(context: RequestContext) -> List[UserSettings]: +def user_sources(context: RequestContext) -> List[UIUserSettings]: """Return a list of all sources which are enabled for the user""" user = context.get("request").user - _all_sources = Source.objects.filter(enabled=True).select_subclasses() - matching_sources: List[UserSettings] = [] + _all_sources: Iterable[Source] = ( + Source.objects.filter(enabled=True).select_subclasses() + ) + matching_sources: List[UIUserSettings] = [] for factor in _all_sources: - user_settings = factor.user_settings() + user_settings = factor.ui_user_settings + if not user_settings: + continue policy_engine = PolicyEngine( factor.policies.all(), user, context.get("request") ) policy_engine.build() - if policy_engine.passing and user_settings: + if policy_engine.passing: matching_sources.append(user_settings) return matching_sources diff --git a/passbook/core/types.py b/passbook/core/types.py new file mode 100644 index 000000000..17937b527 --- /dev/null +++ b/passbook/core/types.py @@ -0,0 +1,29 @@ +"""passbook core dataclasses""" +from typing import Optional +from dataclasses import dataclass + + +@dataclass +class UIUserSettings: + """Dataclass for Factor and Source's user_settings""" + + name: str + icon: str + view_name: str + + +@dataclass +class UILoginButton: + """Dataclass for Source's ui_ui_login_button""" + + # Name, ran through i18n + name: str + + # URL Which Button points to + url: str + + # Icon name, ran through django's static + icon_path: Optional[str] = None + + # Icon URL, used as-is + icon_url: Optional[str] = None diff --git a/passbook/core/views/authentication.py b/passbook/core/views/authentication.py index f85aaa019..05bee8ba1 100644 --- a/passbook/core/views/authentication.py +++ b/passbook/core/views/authentication.py @@ -47,9 +47,9 @@ class LoginView(UserPassesTestMixin, FormView): kwargs["sources"] = [] sources = Source.objects.filter(enabled=True).select_subclasses() for source in sources: - login_button = source.login_button - if login_button: - kwargs["sources"].append(login_button) + ui_login_button = source.ui_login_button + if ui_login_button: + kwargs["sources"].append(ui_login_button) if kwargs["sources"]: self.template_name = "login/with_sources.html" return super().get_context_data(**kwargs) diff --git a/passbook/factors/otp/models.py b/passbook/factors/otp/models.py index daf9fb4a2..8e751e625 100644 --- a/passbook/factors/otp/models.py +++ b/passbook/factors/otp/models.py @@ -1,9 +1,9 @@ """OTP Factor""" - from django.db import models from django.utils.translation import gettext as _ -from passbook.core.models import Factor, UserSettings +from passbook.core.types import UIUserSettings +from passbook.core.models import Factor class OTPFactor(Factor): @@ -17,9 +17,12 @@ class OTPFactor(Factor): type = "passbook.factors.otp.factors.OTPFactor" form = "passbook.factors.otp.forms.OTPFactorForm" - def user_settings(self) -> UserSettings: - return UserSettings( - _("OTP"), "pficon-locked", "passbook_factors_otp:otp-user-settings" + @property + def ui_user_settings(self) -> UIUserSettings: + return UIUserSettings( + name="OTP", + icon="pficon-locked", + view_name="passbook_factors_otp:otp-user-settings", ) def __str__(self): diff --git a/passbook/factors/password/models.py b/passbook/factors/password/models.py index b5ed6bdf5..e9af9722a 100644 --- a/passbook/factors/password/models.py +++ b/passbook/factors/password/models.py @@ -3,7 +3,8 @@ from django.contrib.postgres.fields import ArrayField from django.db import models from django.utils.translation import gettext_lazy as _ -from passbook.core.models import Factor, Policy, User, UserSettings +from passbook.core.types import UIUserSettings +from passbook.core.models import Factor, Policy, User class PasswordFactor(Factor): @@ -18,9 +19,12 @@ class PasswordFactor(Factor): type = "passbook.factors.password.factor.PasswordFactor" form = "passbook.factors.password.forms.PasswordFactorForm" - def user_settings(self): - return UserSettings( - _("Change Password"), "pficon-key", "passbook_core:user-change-password" + @property + def ui_user_settings(self) -> UIUserSettings: + return UIUserSettings( + name="Change Password", + icon="pficon-key", + view_name="passbook_core:user-change-password", ) def password_passes(self, user: User) -> bool: diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index 042a4d9db..db14c7eba 100644 --- a/passbook/policies/expression/evaluator.py +++ b/passbook/policies/expression/evaluator.py @@ -76,7 +76,7 @@ class Evaluator: src=expression_source, req=request, ) - return PolicyRequest(False) + return PolicyResult(False) if isinstance(result, list) and len(result) == 2: return PolicyResult(*result) if result: diff --git a/passbook/sources/oauth/models.py b/passbook/sources/oauth/models.py index 40d070df6..1c1cabfd9 100644 --- a/passbook/sources/oauth/models.py +++ b/passbook/sources/oauth/models.py @@ -4,7 +4,8 @@ from django.db import models from django.urls import reverse, reverse_lazy from django.utils.translation import gettext_lazy as _ -from passbook.core.models import Source, UserSettings, UserSourceConnection +from passbook.core.types import UILoginButton, UIUserSettings +from passbook.core.models import Source, UserSourceConnection from passbook.sources.oauth.clients import get_client @@ -28,30 +29,35 @@ class OAuthSource(Source): form = "passbook.sources.oauth.forms.OAuthSourceForm" @property - def login_button(self): - url = reverse_lazy( - "passbook_sources_oauth:oauth-client-login", - kwargs={"source_slug": self.slug}, + def ui_login_button(self) -> UILoginButton: + return UILoginButton( + url=reverse_lazy( + "passbook_sources_oauth:oauth-client-login", + kwargs={"source_slug": self.slug}, + ), + icon_path=f"{self.provider_type}.svg", + name=self.name, ) - return url, self.provider_type, self.name @property - def additional_info(self): - return "Callback URL: 
%s
" % reverse_lazy( + def ui_additional_info(self) -> str: + url = reverse_lazy( "passbook_sources_oauth:oauth-client-callback", kwargs={"source_slug": self.slug}, ) + return f"Callback URL: 
{url}
" - def user_settings(self) -> UserSettings: + @property + def ui_user_settings(self) -> UIUserSettings: icon_type = self.provider_type if icon_type == "azure ad": icon_type = "windows" - icon_class = "fa fa-%s" % icon_type + icon_class = f"fa fa-{icon_type}" view_name = "passbook_sources_oauth:oauth-client-user" - return UserSettings( - self.name, - icon_class, - reverse((view_name), kwargs={"source_slug": self.slug}), + return UIUserSettings( + name=self.name, + icon=icon_class, + view_name=reverse((view_name), kwargs={"source_slug": self.slug}), ) class Meta: diff --git a/passbook/sources/saml/models.py b/passbook/sources/saml/models.py index 273380382..36356fdfa 100644 --- a/passbook/sources/saml/models.py +++ b/passbook/sources/saml/models.py @@ -3,11 +3,12 @@ from django.db import models from django.urls import reverse_lazy from django.utils.translation import gettext_lazy as _ +from passbook.core.types import UILoginButton from passbook.core.models import Source class SAMLSource(Source): - """SAML2 Source""" + """SAML Source""" entity_id = models.TextField(blank=True, default=None, verbose_name=_("Entity ID")) idp_url = models.URLField(verbose_name=_("IDP URL")) @@ -20,14 +21,17 @@ class SAMLSource(Source): form = "passbook.sources.saml.forms.SAMLSourceForm" @property - def login_button(self): - url = reverse_lazy( - "passbook_sources_saml:login", kwargs={"source_slug": self.slug} + def ui_login_button(self) -> UILoginButton: + return UILoginButton( + name=self.name, + url=reverse_lazy( + "passbook_sources_saml:login", kwargs={"source_slug": self.slug} + ), + icon_path="", ) - return url, "", self.name @property - def additional_info(self): + def ui_additional_info(self) -> str: metadata_url = reverse_lazy( "passbook_sources_saml:metadata", kwargs={"source_slug": self} )