From 2e15b24f0a76b5000e38510ffceb14b732d862b0 Mon Sep 17 00:00:00 2001 From: "Langhammer, Jens" Date: Wed, 9 Oct 2019 12:47:14 +0200 Subject: [PATCH] *(minor): switch has_user_settings to return Optional dataclass instead of tuple --- passbook/core/models.py | 33 +++++++++++++------ passbook/core/templates/user/base.html | 16 ++++----- .../templatetags/passbook_user_settings.py | 24 +++++++------- passbook/factors/captcha/admin.py | 5 +++ passbook/factors/captcha/forms.py | 3 ++ passbook/factors/otp/models.py | 6 ++-- passbook/factors/password/models.py | 7 ++-- passbook/sources/oauth/models.py | 11 +++---- 8 files changed, 62 insertions(+), 43 deletions(-) create mode 100644 passbook/factors/captcha/admin.py diff --git a/passbook/core/models.py b/passbook/core/models.py index 82bdbf9e3..ee433b7f5 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -2,6 +2,7 @@ from datetime import timedelta from random import SystemRandom from time import sleep +from typing import Optional from uuid import uuid4 from django.contrib.auth.models import AbstractUser @@ -74,6 +75,20 @@ 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(PolicyModel): """Authentication factor, multiple instances of the same Factor can be used""" @@ -86,11 +101,10 @@ class Factor(PolicyModel): type = '' form = '' - def has_user_settings(self): - """Entrypoint to integrate with User settings. Can either return False if no - user settings are available, or a tuple or string, string, string where the first string - is the name the item has, the second string is the icon and the third is the view-name.""" - return False + def user_settings(self) -> Optional[UserSettings]: + """Entrypoint to integrate with User settings. Can either return None if no + user settings are available, or an instanace of UserSettings.""" + return None def __str__(self): return f"Factor {self.slug}" @@ -147,11 +161,10 @@ class Source(PolicyModel): """Return additional Info, such as a callback URL. Show in the administration interface.""" return None - def has_user_settings(self): - """Entrypoint to integrate with User settings. Can either return False if no - user settings are available, or a tuple or string, string, string where the first string - is the name the item has, the second string is the icon and the third is the view-name.""" - return False + def user_settings(self) -> Optional[UserSettings]: + """Entrypoint to integrate with User settings. Can either return None if no + user settings are available, or an instanace of UserSettings.""" + return None def __str__(self): return self.name diff --git a/passbook/core/templates/user/base.html b/passbook/core/templates/user/base.html index f4c69937e..58b0bac53 100644 --- a/passbook/core/templates/user/base.html +++ b/passbook/core/templates/user/base.html @@ -18,19 +18,19 @@
  • {% user_factors as uf %} - {% for name, icon, link in uf %} -
  • - - {{ name }} + {% for user_settings in uf %} +
  • + + {{ user_settings.name }}
    • {% endfor %}
  • {% user_sources as us %} - {% for name, icon, link in us %} -
  • - - {{ name }} + {% for user_settings in us %} +
  • + + {{ user_settings.name }}
    • {% endfor %} diff --git a/passbook/core/templatetags/passbook_user_settings.py b/passbook/core/templatetags/passbook_user_settings.py index f560d03d3..d858351a6 100644 --- a/passbook/core/templatetags/passbook_user_settings.py +++ b/passbook/core/templatetags/passbook_user_settings.py @@ -1,37 +1,37 @@ """passbook user settings template tags""" - +from typing import List from django import template from django.template.context import RequestContext -from passbook.core.models import Factor, Source +from passbook.core.models import Factor, Source, UserSettings from passbook.policies.engine import PolicyEngine register = template.Library() @register.simple_tag(takes_context=True) -def user_factors(context: RequestContext): +def user_factors(context: RequestContext) -> List[UserSettings]: """Return list of all factors which apply to user""" user = context.get('request').user _all_factors = Factor.objects.filter(enabled=True).order_by('order').select_subclasses() - matching_factors = [] + matching_factors: List[UserSettings] = [] for factor in _all_factors: - _link = factor.has_user_settings() + user_settings = factor.user_settings() policy_engine = PolicyEngine(factor.policies.all()) policy_engine.for_user(user).with_request(context.get('request')).build() - if policy_engine.passing and _link: - matching_factors.append(_link) + if policy_engine.passing and user_settings: + matching_factors.append(user_settings) return matching_factors @register.simple_tag(takes_context=True) -def user_sources(context: RequestContext): +def user_sources(context: RequestContext) -> List[UserSettings]: """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 = [] + matching_sources: List[UserSettings] = [] for factor in _all_sources: - _link = factor.has_user_settings() + user_settings = factor.user_settings() policy_engine = PolicyEngine(factor.policies.all()) policy_engine.for_user(user).with_request(context.get('request')).build() - if policy_engine.passing and _link: - matching_sources.append(_link) + if policy_engine.passing and user_settings: + matching_sources.append(user_settings) return matching_sources diff --git a/passbook/factors/captcha/admin.py b/passbook/factors/captcha/admin.py new file mode 100644 index 000000000..50049a8d0 --- /dev/null +++ b/passbook/factors/captcha/admin.py @@ -0,0 +1,5 @@ +"""captcha factor admin""" + +from passbook.lib.admin import admin_autoregister + +admin_autoregister('passbook_factors_captcha') diff --git a/passbook/factors/captcha/forms.py b/passbook/factors/captcha/forms.py index 687b44a46..fdbdeca7f 100644 --- a/passbook/factors/captcha/forms.py +++ b/passbook/factors/captcha/forms.py @@ -1,6 +1,8 @@ """passbook captcha factor forms""" from captcha.fields import ReCaptchaField from django import forms +from django.contrib.admin.widgets import FilteredSelectMultiple +from django.utils.translation import gettext as _ from passbook.factors.captcha.models import CaptchaFactor from passbook.factors.forms import GENERAL_FIELDS @@ -21,6 +23,7 @@ class CaptchaFactorForm(forms.ModelForm): widgets = { 'name': forms.TextInput(), 'order': forms.NumberInput(), + 'policies': FilteredSelectMultiple(_('policies'), False), 'public_key': forms.TextInput(), 'private_key': forms.TextInput(), } diff --git a/passbook/factors/otp/models.py b/passbook/factors/otp/models.py index b86233a20..31d64df97 100644 --- a/passbook/factors/otp/models.py +++ b/passbook/factors/otp/models.py @@ -3,7 +3,7 @@ from django.db import models from django.utils.translation import gettext as _ -from passbook.core.models import Factor +from passbook.core.models import Factor, UserSettings class OTPFactor(Factor): @@ -15,8 +15,8 @@ class OTPFactor(Factor): type = 'passbook.factors.otp.factors.OTPFactor' form = 'passbook.factors.otp.forms.OTPFactorForm' - def has_user_settings(self): - return _('OTP'), 'pficon-locked', 'passbook_factors_otp:otp-user-settings' + def user_settings(self) -> UserSettings: + return UserSettings(_('OTP'), 'pficon-locked', 'passbook_factors_otp:otp-user-settings') def __str__(self): return f"OTP Factor {self.slug}" diff --git a/passbook/factors/password/models.py b/passbook/factors/password/models.py index 31b7ada9d..513139c49 100644 --- a/passbook/factors/password/models.py +++ b/passbook/factors/password/models.py @@ -3,7 +3,7 @@ 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 +from passbook.core.models import Factor, Policy, User, UserSettings class PasswordFactor(Factor): @@ -16,8 +16,9 @@ class PasswordFactor(Factor): type = 'passbook.factors.password.factor.PasswordFactor' form = 'passbook.factors.password.forms.PasswordFactorForm' - def has_user_settings(self): - return _('Change Password'), 'pficon-key', 'passbook_core:user-change-password' + def user_settings(self): + return UserSettings(_('Change Password'), 'pficon-key', + 'passbook_core:user-change-password') def password_passes(self, user: User) -> bool: """Return true if user's password passes, otherwise False or raise Exception""" diff --git a/passbook/sources/oauth/models.py b/passbook/sources/oauth/models.py index 053889194..99d6e15c7 100644 --- a/passbook/sources/oauth/models.py +++ b/passbook/sources/oauth/models.py @@ -4,7 +4,7 @@ from django.db import models from django.urls import reverse, reverse_lazy from django.utils.translation import gettext as _ -from passbook.core.models import Source, UserSourceConnection +from passbook.core.models import Source, UserSourceConnection, UserSettings from passbook.sources.oauth.clients import get_client @@ -37,18 +37,15 @@ class OAuthSource(Source): reverse_lazy('passbook_sources_oauth:oauth-client-callback', kwargs={'source_slug': self.slug}) - def has_user_settings(self): - """Entrypoint to integrate with User settings. Can either return False if no - user settings are available, or a tuple or string, string, string where the first string - is the name the item has, the second string is the icon and the third is the view-name.""" + def user_settings(self) -> UserSettings: icon_type = self.provider_type if icon_type == 'azure ad': icon_type = 'windows' icon_class = 'fa fa-%s' % icon_type view_name = 'passbook_sources_oauth:oauth-client-user' - return self.name, icon_class, reverse((view_name), kwargs={ + return UserSettings(self.name, icon_class, reverse((view_name), kwargs={ 'source_slug': self.slug - }) + })) class Meta: