all: prefix all UI related methods with ui_, switch to property and return dataclass

passbook/admin/templates/administration/source/list.html

@@ -36,7 +36,7 @@
             <tr>
                 <td>{{ source.name }}</td>
                 <td>{{ source|fieldtype }}</td>
-                <td>{{ source.additional_info|safe }}</td>
+                <td>{{ source.ui_additional_info|safe|default:"" }}</td>
                 <td>
                     <a class="btn btn-default btn-sm"
                         href="{% url 'passbook_admin:source-update' pk=source.uuid %}?back={{ request.get_full_path }}">{% trans 'Edit' %}</a>

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):
+    def ui_login_button(self) -> Optional[UILoginButton]:
-        """Return a tuple of URL, Icon name and Name
+        """If source uses a http-based flow, return UI Information about the login
-        if Source should get a link on the login page"""
+        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):

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()
+    _all_sources: Iterable[Source] = (
-    matching_sources: List[UserSettings] = []
+        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

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

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
+            ui_login_button = source.ui_login_button
-            if login_button:
+            if ui_login_button:
-                kwargs["sources"].append(login_button)
+                kwargs["sources"].append(ui_login_button)
         if kwargs["sources"]:
             self.template_name = "login/with_sources.html"
         return super().get_context_data(**kwargs)

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:
+    @property
-        return UserSettings(
+    def ui_user_settings(self) -> UIUserSettings:
-            _("OTP"), "pficon-locked", "passbook_factors_otp:otp-user-settings"
+        return UIUserSettings(
+            name="OTP",
+            icon="pficon-locked",
+            view_name="passbook_factors_otp:otp-user-settings",
         )
 
     def __str__(self):

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):
+    @property
-        return UserSettings(
+    def ui_user_settings(self) -> UIUserSettings:
-            _("Change Password"), "pficon-key", "passbook_core:user-change-password"
+        return UIUserSettings(
+            name="Change Password",
+            icon="pficon-key",
+            view_name="passbook_core:user-change-password",
         )
 
     def password_passes(self, user: User) -> bool:

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:

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):
+    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):
+    def ui_additional_info(self) -> str:
-        return "Callback URL: <pre>%s</pre>" % reverse_lazy(
+        url = reverse_lazy(
             "passbook_sources_oauth:oauth-client-callback",
             kwargs={"source_slug": self.slug},
         )
+        return f"Callback URL: <pre>{url}</pre>"
 
-    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(
+        return UIUserSettings(
-            self.name,
+            name=self.name,
-            icon_class,
+            icon=icon_class,
-            reverse((view_name), kwargs={"source_slug": self.slug}),
+            view_name=reverse((view_name), kwargs={"source_slug": self.slug}),
         )
 
     class Meta:

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):
+    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}
         )