Merge branch 'master' into pf4

# Conflicts:
#	passbook/core/static/img/logos/discord.svg
#	passbook/core/static/js/passbook.js
#	passbook/core/templates/login/with_sources.html
#	passbook/core/templates/overview/index.html
#	passbook/core/views/authentication.py

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 0.8.1-beta
+current_version = 0.8.5-beta
 tag = True
 commit = True
 parse = (?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)\-(?P<release>.*)

.github/workflows/release.yml

@@ -16,11 +16,11 @@ jobs:
       - name: Building Docker Image
         run: docker build
           --no-cache
-          -t beryju/passbook:0.8.1-beta
+          -t beryju/passbook:0.8.5-beta
           -t beryju/passbook:latest
           -f Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook:0.8.1-beta
+        run: docker push beryju/passbook:0.8.5-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook:latest
   build-gatekeeper:
@@ -37,11 +37,11 @@ jobs:
           cd gatekeeper
           docker build \
           --no-cache \
-          -t beryju/passbook-gatekeeper:0.8.1-beta \
+          -t beryju/passbook-gatekeeper:0.8.5-beta \
           -t beryju/passbook-gatekeeper:latest \
           -f Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook-gatekeeper:0.8.1-beta
+        run: docker push beryju/passbook-gatekeeper:0.8.5-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook-gatekeeper:latest
   build-static:
@@ -66,11 +66,11 @@ jobs:
         run: docker build
           --no-cache
           --network=$(docker network ls | grep github | awk '{print $1}')
-          -t beryju/passbook-static:0.8.1-beta
+          -t beryju/passbook-static:0.8.5-beta
           -t beryju/passbook-static:latest
           -f static.Dockerfile .
       - name: Push Docker Container to Registry (versioned)
-        run: docker push beryju/passbook-static:0.8.1-beta
+        run: docker push beryju/passbook-static:0.8.5-beta
       - name: Push Docker Container to Registry (latest)
         run: docker push beryju/passbook-static:latest
   test-release:

helm/Chart.yaml

@@ -1,6 +1,6 @@
 apiVersion: v1
-appVersion: "0.8.1-beta"
+appVersion: "0.8.5-beta"
 description: A Helm chart for passbook.
 name: passbook
-version: "0.8.1-beta"
+version: "0.8.5-beta"
 icon: https://git.beryju.org/uploads/-/system/project/avatar/108/logo.png

helm/values.yaml

@@ -2,7 +2,7 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 image:
-  tag: 0.8.1-beta
+  tag: 0.8.5-beta
 
 nameOverride: ""
 

manage.py

@@ -2,6 +2,9 @@
 """Django manage.py"""
 import os
 import sys
+from defusedxml import defuse_stdlib
+
+defuse_stdlib()
 
 if __name__ == '__main__':
     os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'passbook.root.settings')

passbook/__init__.py

@@ -1,2 +1,2 @@
 """passbook"""
-__version__ = "0.8.1-beta"
+__version__ = "0.8.5-beta"

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/api/applications.py

@@ -15,11 +15,13 @@ class ApplicationSerializer(ModelSerializer):
             "pk",
             "name",
             "slug",
-            "launch_url",
-            "icon_url",
-            "provider",
-            "policies",
             "skip_authorization",
+            "provider",
+            "meta_launch_url",
+            "meta_icon_url",
+            "meta_description",
+            "meta_publisher",
+            "policies",
         ]
 
 

passbook/core/exceptions.py

@@ -1,5 +1,6 @@
 """passbook core exceptions"""
+from passbook.lib.sentry import SentryIgnoredException
 
 
-class PropertyMappingExpressionException(Exception):
+class PropertyMappingExpressionException(SentryIgnoredException):
     """Error when a PropertyMapping Exception expression could not be parsed or evaluated."""

passbook/core/forms/applications.py

@@ -19,19 +19,25 @@ class ApplicationForm(forms.ModelForm):
         fields = [
             "name",
             "slug",
-            "launch_url",
-            "icon_url",
-            "provider",
-            "policies",
             "skip_authorization",
+            "provider",
+            "meta_launch_url",
+            "meta_icon_url",
+            "meta_description",
+            "meta_publisher",
+            "policies",
         ]
         widgets = {
             "name": forms.TextInput(),
-            "launch_url": forms.TextInput(),
+            "meta_launch_url": forms.TextInput(),
-            "icon_url": forms.TextInput(),
+            "meta_icon_url": forms.TextInput(),
+            "meta_publisher": forms.TextInput(),
             "policies": FilteredSelectMultiple(_("policies"), False),
         }
         labels = {
-            "launch_url": _("Launch URL"),
+            "meta_launch_url": _("Launch URL"),
-            "icon_url": _("Icon URL"),
+            "meta_icon_url": _("Icon URL"),
+            "meta_description": _("Description"),
+            "meta_publisher": _("Publisher"),
         }
+        help_texts = {"policies": _("Policies required to access this Application.")}

passbook/core/migrations/0008_auto_20200220_1242.py

@@ -0,0 +1,29 @@
+# Generated by Django 3.0.3 on 2020-02-20 12:42
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("passbook_core", "0007_auto_20200217_1934"),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name="application", old_name="icon_url", new_name="meta_icon_url",
+        ),
+        migrations.RenameField(
+            model_name="application", old_name="launch_url", new_name="meta_launch_url",
+        ),
+        migrations.AddField(
+            model_name="application",
+            name="meta_description",
+            field=models.TextField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name="application",
+            name="meta_publisher",
+            field=models.TextField(blank=True, null=True),
+        ),
+    ]

passbook/core/models.py

@@ -23,9 +23,10 @@ from structlog import get_logger
 
 from passbook.core.exceptions import PropertyMappingExpressionException
 from passbook.core.signals import password_changed
+from passbook.core.types import UILoginButton, UIUserSettings
 from passbook.lib.models import CreatedUpdatedModel, UUIDModel
 from passbook.policies.exceptions import PolicyException
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 LOGGER = get_logger()
 NATIVE_ENVIRONMENT = NativeEnvironment()
@@ -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):
@@ -143,16 +132,19 @@ class Application(ExportModelOperationsMixin("application"), PolicyModel):
 
     name = models.TextField()
     slug = models.SlugField()
-    launch_url = models.URLField(null=True, blank=True)
+    skip_authorization = models.BooleanField(default=False)
-    icon_url = models.TextField(null=True, blank=True)
     provider = models.OneToOneField(
         "Provider", null=True, blank=True, default=None, on_delete=models.SET_DEFAULT
     )
-    skip_authorization = models.BooleanField(default=False)
+
+    meta_launch_url = models.URLField(null=True, blank=True)
+    meta_icon_url = models.TextField(null=True, blank=True)
+    meta_description = models.TextField(null=True, blank=True)
+    meta_publisher = models.TextField(null=True, blank=True)
 
     objects = InheritanceManager()
 
-    def get_provider(self):
+    def get_provider(self) -> Optional[Provider]:
         """Get casted provider instance"""
         if not self.provider:
             return None
@@ -167,6 +159,7 @@ class Source(ExportModelOperationsMixin("source"), PolicyModel):
 
     name = models.TextField()
     slug = models.SlugField()
+
     enabled = models.BooleanField(default=True)
     property_mappings = models.ManyToManyField(
         "PropertyMapping", default=None, blank=True
@@ -177,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 Iterable, List
 
 from django import template
 from django.template.context import RequestContext
 
-from passbook.core.models import Factor, Source, UserSettings
+from passbook.core.models import Factor, Source
+from passbook.core.types import UIUserSettings
 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 dataclasses import dataclass
+from typing import Optional
+
+
+@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
+            if ui_login_button:
-            if login_button:
+                kwargs["sources"].append(ui_login_button)
-                kwargs["sources"].append(login_button)
+            ui_login_button = source.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.models import Factor
+from passbook.core.types import UIUserSettings
 
 
 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/exceptions.py

@@ -1,7 +1,8 @@
 """passbook password policy exceptions"""
+from passbook.lib.sentry import SentryIgnoredException
 
 
-class PasswordPolicyInvalid(Exception):
+class PasswordPolicyInvalid(SentryIgnoredException):
     """Exception raised when a Password Policy fails"""
 
     messages = []

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.models import Factor, Policy, User
+from passbook.core.types import UIUserSettings
 
 
 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/lib/sentry.py

@@ -4,6 +4,10 @@ from structlog import get_logger
 LOGGER = get_logger()
 
 
+class SentryIgnoredException(Exception):
+    """Base Class for all errors that are supressed, and not sent to sentry."""
+
+
 def before_send(event, hint):
     """Check if error is database error, and ignore if so"""
     from django_redis.exceptions import ConnectionInterrupted
@@ -29,6 +33,7 @@ def before_send(event, hint):
         ValidationError,
         OSError,
         RedisError,
+        SentryIgnoredException,
     )
     if "exc_info" in hint:
         _exc_type, exc_value, _ = hint["exc_info"]

passbook/policies/engine.py

@@ -9,7 +9,7 @@ from structlog import get_logger
 
 from passbook.core.models import Policy, User
 from passbook.policies.process import PolicyProcess, cache_key
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 LOGGER = get_logger()
 # This is only really needed for macOS, because Python 3.8 changed the default to spawn
@@ -63,13 +63,13 @@ class PolicyEngine:
         for policy in self._select_subclasses():
             cached_policy = cache.get(cache_key(policy, self.request.user), None)
             if cached_policy and self.use_cache:
-                LOGGER.debug("Taking result from cache", policy=policy)
+                LOGGER.debug("P_ENG: Taking result from cache", policy=policy)
                 self.__cached_policies.append(cached_policy)
                 continue
-            LOGGER.debug("Evaluating policy", policy=policy)
+            LOGGER.debug("P_ENG: Evaluating policy", policy=policy)
             our_end, task_end = Pipe(False)
             task = PolicyProcess(policy, self.request, task_end)
-            LOGGER.debug("Starting Process", policy=policy)
+            LOGGER.debug("P_ENG: Starting Process", policy=policy)
             task.start()
             self.__processes.append(
                 PolicyProcessInfo(process=task, connection=our_end, policy=policy)
@@ -90,7 +90,7 @@ class PolicyEngine:
             x.result for x in self.__processes if x.result
         ]
         for result in process_results + self.__cached_policies:
-            LOGGER.debug("result", passing=result.passing)
+            LOGGER.debug("P_ENG: result", passing=result.passing)
             if result.messages:
                 messages += result.messages
             if not result.passing:

passbook/policies/exceptions.py

@@ -1,5 +1,6 @@
 """policy exceptions"""
+from passbook.lib.sentry import SentryIgnoredException
 
 
-class PolicyException(Exception):
+class PolicyException(SentryIgnoredException):
     """Exception that should be raised during Policy Evaluation, and can be recovered from."""

passbook/policies/expiry/models.py

@@ -7,7 +7,7 @@ from django.utils.translation import gettext as _
 from structlog import get_logger
 
 from passbook.core.models import Policy
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 LOGGER = get_logger()
 

passbook/policies/expression/evaluator.py

@@ -9,7 +9,7 @@ from jinja2.nativetypes import NativeEnvironment
 from structlog import get_logger
 
 from passbook.factors.view import AuthenticationView
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 if TYPE_CHECKING:
     from passbook.core.models import User
@@ -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/policies/expression/models.py

@@ -4,7 +4,7 @@ from django.utils.translation import gettext as _
 
 from passbook.core.models import Policy
 from passbook.policies.expression.evaluator import Evaluator
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 
 class ExpressionPolicy(Policy):

passbook/policies/password/models.py

@@ -6,7 +6,7 @@ from django.utils.translation import gettext as _
 from structlog import get_logger
 
 from passbook.core.models import Policy
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 LOGGER = get_logger()
 

passbook/policies/process.py

@@ -7,7 +7,7 @@ from structlog import get_logger
 
 from passbook.core.models import Policy
 from passbook.policies.exceptions import PolicyException
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 LOGGER = get_logger()
 

passbook/policies/reputation/models.py

@@ -4,7 +4,7 @@ from django.utils.translation import gettext as _
 
 from passbook.core.models import Policy, User
 from passbook.lib.utils.http import get_client_ip
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 
 class ReputationPolicy(Policy):

passbook/policies/webhook/models.py

@@ -3,7 +3,7 @@ from django.db import models
 from django.utils.translation import gettext as _
 
 from passbook.core.models import Policy
-from passbook.policies.struct import PolicyRequest, PolicyResult
+from passbook.policies.types import PolicyRequest, PolicyResult
 
 
 class WebhookPolicy(Policy):

passbook/providers/saml/exceptions.py

@@ -1,5 +1,6 @@
 """passbook SAML IDP Exceptions"""
+from passbook.lib.sentry import SentryIgnoredException
 
 
-class CannotHandleAssertion(Exception):
+class CannotHandleAssertion(SentryIgnoredException):
     """This processor does not handle this assertion."""

passbook/providers/saml/processors/base.py

@@ -40,7 +40,7 @@ class Processor:
     @property
     def subject_format(self) -> str:
         """Get subject Format"""
-        return "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"
+        return "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
 
     def __init__(self, remote: "SAMLProvider"):
         self.name = remote.name
@@ -144,7 +144,7 @@ class Processor:
 
     def _decode_and_parse_request(self):
         """Parses various parameters from _request_xml into _request_params."""
-        decoded_xml = decode_base64_and_inflate(self._saml_request).decode("utf-8")
+        decoded_xml = decode_base64_and_inflate(self._saml_request)
 
         root = ElementTree.fromstring(decoded_xml)
 
@@ -183,15 +183,13 @@ class Processor:
         # Read the request.
         try:
             self._extract_saml_request()
-        except KeyError as exc:
+        except KeyError:
-            raise CannotHandleAssertion(
+            raise CannotHandleAssertion(f"Couldn't find SAML request in user session:")
-                f"can't find SAML request in user session: {exc}"
-            ) from exc
 
         try:
             self._decode_and_parse_request()
         except Exception as exc:
-            raise CannotHandleAssertion(f"can't parse SAML request: {exc}") from exc
+            raise CannotHandleAssertion(f"Couldn't parse SAML request: {exc}") from exc
 
         self._validate_request()
         return True

passbook/providers/saml/templates/saml/xml/metadata.xml

@@ -15,26 +15,9 @@
         </ds:X509Data>
       </ds:KeyInfo>
     </md:KeyDescriptor>
+    <md:NameIDFormat>{{ subject_format }}</md:NameIDFormat>
     <md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ slo_url }}"/>
-    <md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:persistent</md:NameIDFormat>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="{{ sso_post_url }}"/>
-    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ sso_url }}"/>
+    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ sso_redirect_url }}"/>
   </md:IDPSSODescriptor>
-{% comment %}
-<!-- #TODO: Add support for optional Organization section -->
-{# if org #}
-  <md:Organization>
-    <md:OrganizationName xml:lang="en">{{ org.name }}</md:OrganizationName>
-    <md:OrganizationDisplayName xml:lang="en">{{ org.display_name }}</md:OrganizationDisplayName>
-    <md:OrganizationURL xml:lang="en">{{ org.url }}</md:OrganizationURL>
-  </md:Organization>
-{# endif #}
-<!-- #TODO: Add support for optional ContactPerson section(s) -->
-{# for contact in contacts #}
-  <md:ContactPerson contactType="{{ contact.type }}">
-    <md:GivenName>{{ contact.given_name }}</md:GivenName>
-    <md:SurName>{{ contact.sur_name }}</md:SurName>
-    <md:EmailAddress>{{ contact.email }}</md:EmailAddress>
-  </md:ContactPerson>
-{# endfor #}
-{% endcomment %}
 </md:EntityDescriptor>

passbook/providers/saml/urls.py

@@ -13,7 +13,7 @@ urlpatterns = [
     # This view is the endpoint a SP would redirect to, and saves data into the session
     # this is required as the process view which it redirects to might have to login first.
     path(
-        "<slug:application>/login/", views.LoginProcessView.as_view(), name="saml-login"
+        "<slug:application>/login/", views.LoginBeginView.as_view(), name="saml-login"
     ),
     path(
         "<slug:application>/login/process/",

passbook/providers/saml/utils/encoding.py

@@ -3,20 +3,20 @@ import base64
 import zlib
 
 
-def decode_base64_and_inflate(b64string):
+def decode_base64_and_inflate(encoded: str, encoding="utf-8") -> str:
     """Base64 decode and ZLib decompress b64string"""
-    decoded_data = base64.b64decode(b64string)
+    decoded_data = base64.b64decode(encoded)
     try:
-        return zlib.decompress(decoded_data, -15)
+        return zlib.decompress(decoded_data, -15).decode(encoding)
     except zlib.error:
-        return decoded_data
+        return decoded_data.decode(encoding)
 
 
-def deflate_and_base64_encode(string_val):
+def deflate_and_base64_encode(inflated: bytes, encoding="utf-8"):
     """Base64 and ZLib Compress b64string"""
-    zlibbed_str = zlib.compress(string_val)
+    zlibbed_str = zlib.compress(inflated)
     compressed_string = zlibbed_str[2:-4]
-    return base64.b64encode(compressed_string)
+    return base64.b64encode(compressed_string).decode(encoding)
 
 
 def nice64(src):

passbook/providers/saml/views.py

@@ -17,7 +17,7 @@ from signxml.util import strip_pem_header
 from structlog import get_logger
 
 from passbook.audit.models import Event, EventAction
-from passbook.core.models import Application
+from passbook.core.models import Application, Provider
 from passbook.lib.utils.template import render_to_string
 from passbook.lib.views import bad_request_message
 from passbook.policies.engine import PolicyEngine
@@ -134,9 +134,7 @@ class LoginProcessView(AccessRequiredView):
         try:
             # application.skip_authorization is set so we directly redirect the user
             if self.provider.application.skip_authorization:
-                self.provider.processor.can_handle(request)
+                return self.post(request, application)
-                saml_params = self.provider.processor.generate_response()
-                return self.handle_redirect(saml_params, True)
 
             self.provider.processor.init_deep_link(request)
             params = self.provider.processor.generate_response()
@@ -233,7 +231,7 @@ class DescriptorDownloadView(AccessRequiredView):
                 kwargs={"application": provider.application.slug},
             )
         )
-        sso_url = request.build_absolute_uri(
+        sso_post_url = request.build_absolute_uri(
             reverse(
                 "passbook_providers_saml:saml-login",
                 kwargs={"application": provider.application.slug},
@@ -242,23 +240,33 @@ class DescriptorDownloadView(AccessRequiredView):
         pubkey = strip_pem_header(provider.signing_cert.replace("\r", "")).replace(
             "\n", ""
         )
+        subject_format = provider.processor.subject_format
         ctx = {
             "entity_id": entity_id,
             "cert_public_key": pubkey,
             "slo_url": slo_url,
-            "sso_url": sso_url,
+            # Currently, the same endpoint accepts POST and REDIRECT
+            "sso_post_url": sso_post_url,
+            "sso_redirect_url": sso_post_url,
+            "subject_format": subject_format,
         }
         return render_to_string("saml/xml/metadata.xml", ctx)
 
     # pylint: disable=unused-argument
     def get(self, request: HttpRequest, application: str) -> HttpResponse:
         """Replies with the XML Metadata IDSSODescriptor."""
-        metadata = DescriptorDownloadView.get_metadata(request, self.provider)
+        try:
-        response = HttpResponse(metadata, content_type="application/xml")
+            metadata = DescriptorDownloadView.get_metadata(request, self.provider)
-        response["Content-Disposition"] = (
+        except Provider.application.RelatedObjectDoesNotExist:  # pylint: disable=no-member
-            'attachment; filename="' '%s_passbook_meta.xml"' % self.provider.name
+            return bad_request_message(
-        )
+                request, "Provider is not assigned to an application."
-        return response
+            )
+        else:
+            response = HttpResponse(metadata, content_type="application/xml")
+            response["Content-Disposition"] = (
+                'attachment; filename="' '%s_passbook_meta.xml"' % self.provider.name
+            )
+            return response
 
 
 class InitiateLoginView(AccessRequiredView):

passbook/root/settings.py

@@ -49,10 +49,15 @@ LOGIN_URL = "passbook_core:auth-login"
 # Custom user model
 AUTH_USER_MODEL = "passbook_core.User"
 
-CSRF_COOKIE_NAME = "passbook_csrf"
+if DEBUG:
-SESSION_COOKIE_NAME = "passbook_session"
+    CSRF_COOKIE_NAME = "passbook_csrf_debug"
+    LANGUAGE_COOKIE_NAME = "passbook_language_debug"
+    SESSION_COOKIE_NAME = "passbook_session_debug"
+else:
+    CSRF_COOKIE_NAME = "passbook_csrf"
+    LANGUAGE_COOKIE_NAME = "passbook_language"
+    SESSION_COOKIE_NAME = "passbook_session"
 SESSION_COOKIE_DOMAIN = CONFIG.y("domain", None)
-LANGUAGE_COOKIE_NAME = "passbook_language"
 
 AUTHENTICATION_BACKENDS = [
     "django.contrib.auth.backends.ModelBackend",
@@ -271,6 +276,7 @@ STATIC_URL = "/static/"
 structlog.configure_once(
     processors=[
         structlog.stdlib.add_log_level,
+        structlog.stdlib.add_logger_name,
         structlog.stdlib.PositionalArgumentsFormatter(),
         structlog.processors.TimeStamper(),
         structlog.processors.StackInfoRenderer(),

passbook/root/wsgi.py

@@ -9,12 +9,14 @@ https://docs.djangoproject.com/en/2.1/howto/deployment/wsgi/
 import os
 from time import time
 
+from defusedxml import defuse_stdlib
 from django.core.wsgi import get_wsgi_application
 from structlog import get_logger
 
 from passbook.lib.utils.http import _get_client_ip_from_meta
 
 os.environ.setdefault("DJANGO_SETTINGS_MODULE", "passbook.root.settings")
+defuse_stdlib()
 
 
 class WSGILogger:

passbook/sources/ldap/connector.py

@@ -3,8 +3,8 @@ from typing import Any, Dict, Optional
 
 import ldap3
 import ldap3.core.exceptions
-from structlog import get_logger
 from django.db.utils import IntegrityError
+from structlog import get_logger
 
 from passbook.core.exceptions import PropertyMappingExpressionException
 from passbook.core.models import Group, User

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.models import Source, UserSourceConnection
+from passbook.core.types import UILoginButton, UIUserSettings
 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:
-        url = reverse_lazy(
+        return UILoginButton(
-            "passbook_sources_oauth:oauth-client-login",
+            url=reverse_lazy(
-            kwargs={"source_slug": self.slug},
+                "passbook_sources_oauth:oauth-client-login",
+                kwargs={"source_slug": self.slug},
+            ),
+            icon_path=f"img/logos/{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/api.py

@@ -13,7 +13,7 @@ class SAMLSourceSerializer(ModelSerializer):
         model = SAMLSource
         fields = [
             "pk",
-            "entity_id",
+            "issuer",
             "idp_url",
             "idp_logout_url",
             "auto_logout",

passbook/sources/saml/exceptions.py

@@ -0,0 +1,10 @@
+"""passbook saml source exceptions"""
+from passbook.lib.sentry import SentryIgnoredException
+
+
+class MissingSAMLResponse(SentryIgnoredException):
+    """Exception raised when request does not contain SAML Response."""
+
+
+class UnsupportedNameIDFormat(SentryIgnoredException):
+    """Exception raised when SAML Response contains NameID Format not supported."""

passbook/sources/saml/forms.py

@@ -22,7 +22,7 @@ class SAMLSourceForm(forms.ModelForm):
 
         model = SAMLSource
         fields = SOURCE_FORM_FIELDS + [
-            "entity_id",
+            "issuer",
             "idp_url",
             "idp_logout_url",
             "auto_logout",
@@ -31,7 +31,7 @@ class SAMLSourceForm(forms.ModelForm):
         widgets = {
             "name": forms.TextInput(),
             "policies": FilteredSelectMultiple(_("policies"), False),
-            "entity_id": forms.TextInput(),
+            "issuer": forms.TextInput(),
             "idp_url": forms.TextInput(),
             "idp_logout_url": forms.TextInput(),
         }

passbook/sources/saml/migrations/0005_auto_20200220_1621.py

@@ -0,0 +1,26 @@
+# Generated by Django 3.0.3 on 2020-02-20 16:21
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("passbook_sources_saml", "0004_auto_20200217_1526"),
+    ]
+
+    operations = [
+        migrations.RenameField(
+            model_name="samlsource", old_name="entity_id", new_name="issuer",
+        ),
+        migrations.AlterField(
+            model_name="samlsource",
+            name="issuer",
+            field=models.TextField(
+                blank=True,
+                default=None,
+                help_text="Also known as Entity ID. Defaults the Metadata URL.",
+                verbose_name="Issuer",
+            ),
+        ),
+    ]

passbook/sources/saml/models.py

@@ -4,12 +4,19 @@ from django.urls import reverse_lazy
 from django.utils.translation import gettext_lazy as _
 
 from passbook.core.models import Source
+from passbook.core.types import UILoginButton
 
 
 class SAMLSource(Source):
-    """SAML2 Source"""
+    """SAML Source"""
+
+    issuer = models.TextField(
+        blank=True,
+        default=None,
+        verbose_name=_("Issuer"),
+        help_text=_("Also known as Entity ID. Defaults the Metadata URL."),
+    )
 
-    entity_id = models.TextField(blank=True, default=None, verbose_name=_("Entity ID"))
     idp_url = models.URLField(verbose_name=_("IDP URL"))
     idp_logout_url = models.URLField(
         default=None, blank=True, null=True, verbose_name=_("IDP Logout URL")
@@ -20,16 +27,19 @@ class SAMLSource(Source):
     form = "passbook.sources.saml.forms.SAMLSourceForm"
 
     @property
-    def login_button(self):
+    def ui_login_button(self) -> UILoginButton:
-        url = reverse_lazy(
+        return UILoginButton(
-            "passbook_sources_saml:login", kwargs={"source_slug": self.slug}
+            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}
+            "passbook_sources_saml:metadata", kwargs={"source_slug": self.slug}
         )
         return f'<a href="{metadata_url}" class="btn btn-default btn-sm">Metadata Download</a>'
 

passbook/sources/saml/processors/base.py

@@ -0,0 +1,86 @@
+"""passbook saml source processor"""
+from typing import TYPE_CHECKING, Optional
+
+from defusedxml import ElementTree
+from django.http import HttpRequest
+from signxml import XMLVerifier
+from structlog import get_logger
+
+from passbook.core.models import User
+from passbook.providers.saml.utils.encoding import decode_base64_and_inflate
+from passbook.sources.saml.exceptions import (
+    MissingSAMLResponse,
+    UnsupportedNameIDFormat,
+)
+from passbook.sources.saml.models import SAMLSource
+
+LOGGER = get_logger()
+if TYPE_CHECKING:
+    from xml.etree.ElementTree import Element  # nosec
+
+
+class Processor:
+    """SAML Response Processor"""
+
+    _source: SAMLSource
+
+    _root: "Element"
+    _root_xml: str
+
+    def __init__(self, source: SAMLSource):
+        self._source = source
+
+    def parse(self, request: HttpRequest):
+        """Check if `request` contains SAML Response data, parse and validate it."""
+        # First off, check if we have any SAML Data at all.
+        raw_response = request.POST.get("SAMLResponse", None)
+        if not raw_response:
+            raise MissingSAMLResponse("Request does not contain 'SAMLResponse'")
+        # relay_state = request.POST.get('RelayState', None)
+        # Check if response is compressed, b64 decode it
+        self._root_xml = decode_base64_and_inflate(raw_response)
+        self._root = ElementTree.fromstring(self._root_xml)
+        # Verify signed XML
+        self._verify_signed()
+
+    def _verify_signed(self):
+        """Verify SAML Response's Signature"""
+        verifier = XMLVerifier()
+        verifier.verify(self._root_xml, x509_cert=self._source.signing_cert)
+
+    def _get_email(self) -> Optional[str]:
+        """
+        Returns the email out of the response.
+
+        At present, response must pass the email address as the Subject, eg.:
+
+        <saml:Subject>
+                <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
+                            SPNameQualifier=""
+                            >email@example.com</saml:NameID>
+        """
+        assertion = self._root.find("{urn:oasis:names:tc:SAML:2.0:assertion}Assertion")
+        subject = assertion.find("{urn:oasis:names:tc:SAML:2.0:assertion}Subject")
+        name_id = subject.find("{urn:oasis:names:tc:SAML:2.0:assertion}NameID")
+        name_id_format = name_id.attrib["Format"]
+        if name_id_format != "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress":
+            raise UnsupportedNameIDFormat(
+                f"Assertion contains NameID with unsupported format {name_id_format}."
+            )
+        return name_id.text
+
+    def get_user(self) -> User:
+        """
+        Gets info out of the response and locally logs in this user.
+        May create a local user account first.
+        Returns the user object that was created.
+        """
+        email = self._get_email()
+        try:
+            user = User.objects.get(email=email)
+        except User.DoesNotExist:
+            user = User.objects.create_user(username=email, email=email)
+            # TODO: Property Mappings
+            user.set_unusable_password()
+            user.save()
+        return user

passbook/sources/saml/templates/saml/sp/xml/sp_sso_descriptor.xml

@@ -0,0 +1,22 @@
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
+  xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
+  xmlns:ds="http://www.w3.org/2000/09/xmldsig#" entityID="{{ issuer }}">
+  <md:SPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <md:KeyDescriptor use="signing">
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>{{ cert_public_key }}</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:KeyDescriptor use="encryption">
+      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+        <ds:X509Data>
+          <ds:X509Certificate>{{ cert_public_key }}</ds:X509Certificate>
+        </ds:X509Data>
+      </ds:KeyInfo>
+    </md:KeyDescriptor>
+    <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>
+    <md:AssertionConsumerService isDefault="true" index="0" Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="{{ acs_url }}"/>
+  </md:SPSSODescriptor>
+</md:EntityDescriptor>

passbook/sources/saml/templates/saml/sp/xml/spssodescriptor.xml

@@ -1,70 +0,0 @@
-<md:EntityDescriptor
-  xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
-  xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
-  xmlns:ds="http://www.w3.org/2000/09/xmldsig#"
-  entityID="{{ entity_id }}">
-  <md:SPSSODescriptor
-    protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
-    <md:KeyDescriptor use="signing">
-      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
-        <ds:X509Data>
-          <ds:X509Certificate>{{ cert_public_key }}</ds:X509Certificate>
-        </ds:X509Data>
-      </ds:KeyInfo>
-    </md:KeyDescriptor>
-    <md:KeyDescriptor use="encryption">
-      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
-        <ds:X509Data>
-          <ds:X509Certificate>{{ cert_public_key }}</ds:X509Certificate>
-        </ds:X509Data>
-      </ds:KeyInfo>
-    </md:KeyDescriptor>
-    <md:NameIDFormat>
-      urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress
-    </md:NameIDFormat>
-    <md:AssertionConsumerService isDefault="true" index="0"
-      Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
-      Location="{{ acs_url }}"/>
-{% comment %}
-<!-- Other bits that we might need. -->
-<!-- Ref: saml-metadata-2.0-os.pdf, pg 10, section 2.3... -->
-    <md:NameIDFormat>
-      urn:oasis:names:tc:SAML:2.0:nameid-format:transient
-    </md:NameIDFormat>
-    <md:ArtifactResolutionService isDefault="true" index="0"
-      Binding="urn:oasis:names:tc:SAML:2.0:bindings:SOAP"
-      Location="https://sp.example.com/SAML2/ArtifactResolution"/>
-    <md:AssertionConsumerService index="1"
-      Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact"
-      Location="https://sp.example.com/SAML2/Artifact"/>
-    <md:AttributeConsumingService isDefault="true" index="1">
-      <md:ServiceName xml:lang="en">
-        Service Provider Portal
-      </md:ServiceName>
-      <md:RequestedAttribute
-        NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
-        Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.1"
-        FriendlyName="eduPersonAffiliation">
-      </md:RequestedAttribute>
-    </md:AttributeConsumingService>
-{% endcomment %}
-  </md:SPSSODescriptor>
-{% comment %}
-<!-- #TODO: Add support for optional Organization section -->
-{# if org #}
-  <md:Organization>
-    <md:OrganizationName xml:lang="en">{{ org.name }}</md:OrganizationName>
-    <md:OrganizationDisplayName xml:lang="en">{{ org.display_name }}</md:OrganizationDisplayName>
-    <md:OrganizationURL xml:lang="en">{{ org.url }}</md:OrganizationURL>
-  </md:Organization>
-{# endif #}
-<!-- #TODO: Add support for optional ContactPerson section(s) -->
-{# for contact in contacts #}
-  <md:ContactPerson contactType="{{ contact.type }}">
-    <md:GivenName>{{ contact.given_name }}</md:GivenName>
-    <md:SurName>{{ contact.sur_name }}</md:SurName>
-    <md:EmailAddress>{{ contact.email }}</md:EmailAddress>
-  </md:ContactPerson>
-{# endfor #}
-{% endcomment %}
-</md:EntityDescriptor>

passbook/sources/saml/utils.py

@@ -2,82 +2,19 @@
 from django.http import HttpRequest
 from django.shortcuts import reverse
 
-from passbook.core.models import User
 from passbook.sources.saml.models import SAMLSource
 
 
-def get_entity_id(request: HttpRequest, source: SAMLSource):
+def get_issuer(request: HttpRequest, source: SAMLSource) -> str:
-    """Get Source's entity ID, falling back to our Metadata URL if none is set"""
+    """Get Source's Issuer, falling back to our Metadata URL if none is set"""
-    entity_id = source.entity_id
+    issuer = source.issuer
-    if entity_id is None:
+    if issuer is None:
         return build_full_url("metadata", request, source)
-    return entity_id
+    return issuer
 
 
 def build_full_url(view: str, request: HttpRequest, source: SAMLSource) -> str:
     """Build Full ACS URL to be used in IDP"""
     return request.build_absolute_uri(
-        reverse(f"passbook_sources_saml:{view}", kwargs={"source": source.slug})
+        reverse(f"passbook_sources_saml:{view}", kwargs={"source_slug": source.slug})
     )
-
-
-def _get_email_from_response(root):
-    """
-    Returns the email out of the response.
-
-    At present, response must pass the email address as the Subject, eg.:
-
-    <saml:Subject>
-            <saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:email"
-                         SPNameQualifier=""
-                         >email@example.com</saml:NameID>
-    """
-    assertion = root.find("{urn:oasis:names:tc:SAML:2.0:assertion}Assertion")
-    subject = assertion.find("{urn:oasis:names:tc:SAML:2.0:assertion}Subject")
-    name_id = subject.find("{urn:oasis:names:tc:SAML:2.0:assertion}NameID")
-    return name_id.text
-
-
-def _get_attributes_from_response(root):
-    """
-    Returns the SAML Attributes (if any) that are present in the response.
-
-    NOTE: Technically, attribute values could be any XML structure.
-          But for now, just assume a single string value.
-    """
-    flat_attributes = {}
-    assertion = root.find("{urn:oasis:names:tc:SAML:2.0:assertion}Assertion")
-    attributes = assertion.find(
-        "{urn:oasis:names:tc:SAML:2.0:assertion}AttributeStatement"
-    )
-    for attribute in attributes.getchildren():
-        name = attribute.attrib.get("Name")
-        children = attribute.getchildren()
-        if not children:
-            # Ignore empty-valued attributes. (I think these are not allowed.)
-            continue
-        if len(children) == 1:
-            # See NOTE:
-            flat_attributes[name] = children[0].text
-        else:
-            # It has multiple values.
-            for child in children:
-                # See NOTE:
-                flat_attributes.setdefault(name, []).append(child.text)
-    return flat_attributes
-
-
-def _get_user_from_response(root):
-    """
-    Gets info out of the response and locally logs in this user.
-    May create a local user account first.
-    Returns the user object that was created.
-    """
-    email = _get_email_from_response(root)
-    try:
-        user = User.objects.get(email=email)
-    except User.DoesNotExist:
-        user = User.objects.create_user(username=email, email=email)
-        user.set_unusable_password()
-        user.save()
-    return user

passbook/sources/saml/views.py

@@ -1,23 +1,23 @@
 """saml sp views"""
-import base64
-
-from defusedxml import ElementTree
 from django.contrib.auth import login, logout
 from django.http import Http404, HttpRequest, HttpResponse
 from django.shortcuts import get_object_or_404, redirect, render, reverse
 from django.utils.decorators import method_decorator
 from django.views import View
 from django.views.decorators.csrf import csrf_exempt
+from signxml.util import strip_pem_header
 
+from passbook.lib.views import bad_request_message
 from passbook.providers.saml.utils import get_random_id, render_xml
 from passbook.providers.saml.utils.encoding import nice64
 from passbook.providers.saml.utils.time import get_time_string
-from passbook.sources.saml.models import SAMLSource
+from passbook.sources.saml.exceptions import (
-from passbook.sources.saml.utils import (
+    MissingSAMLResponse,
-    _get_user_from_response,
+    UnsupportedNameIDFormat,
-    build_full_url,
-    get_entity_id,
 )
+from passbook.sources.saml.models import SAMLSource
+from passbook.sources.saml.processors.base import Processor
+from passbook.sources.saml.utils import build_full_url, get_issuer
 from passbook.sources.saml.xml_render import get_authnrequest_xml
 
 
@@ -36,7 +36,7 @@ class InitiateView(View):
             "DESTINATION": source.idp_url,
             "AUTHN_REQUEST_ID": get_random_id(),
             "ISSUE_INSTANT": get_time_string(),
-            "ISSUER": get_entity_id(request, source),
+            "ISSUER": get_issuer(request, source),
         }
         authn_req = get_authnrequest_xml(parameters, signed=False)
         _request = nice64(str.encode(authn_req))
@@ -61,14 +61,18 @@ class ACSView(View):
         source: SAMLSource = get_object_or_404(SAMLSource, slug=source_slug)
         if not source.enabled:
             raise Http404
-        # sso_session = request.POST.get('RelayState', None)
+        processor = Processor(source)
-        data = request.POST.get("SAMLResponse", None)
+        try:
-        response = base64.b64decode(data)
+            processor.parse(request)
-        root = ElementTree.fromstring(response)
+        except MissingSAMLResponse as exc:
-        user = _get_user_from_response(root)
+            return bad_request_message(request, str(exc))
-        # attributes = _get_attributes_from_response(root)
+
-        login(request, user, backend="django.contrib.auth.backends.ModelBackend")
+        try:
-        return redirect(reverse("passbook_core:overview"))
+            user = processor.get_user()
+            login(request, user, backend="django.contrib.auth.backends.ModelBackend")
+            return redirect(reverse("passbook_core:overview"))
+        except UnsupportedNameIDFormat as exc:
+            return bad_request_message(request, str(exc))
 
 
 class SLOView(View):
@@ -96,13 +100,16 @@ class MetadataView(View):
     def dispatch(self, request: HttpRequest, source_slug: str) -> HttpResponse:
         """Replies with the XML Metadata SPSSODescriptor."""
         source: SAMLSource = get_object_or_404(SAMLSource, slug=source_slug)
-        entity_id = get_entity_id(request, source)
+        issuer = get_issuer(request, source)
+        cert_stripped = strip_pem_header(source.signing_cert.replace("\r", "")).replace(
+            "\n", ""
+        )
         return render_xml(
             request,
-            "saml/sp/xml/spssodescriptor.xml",
+            "saml/sp/xml/sp_sso_descriptor.xml",
             {
                 "acs_url": build_full_url("acs", request, source),
-                "entity_id": entity_id,
+                "issuer": issuer,
-                "cert_public_key": source.signing_cert,
+                "cert_public_key": cert_stripped,
             },
         )

passbook/sources/saml/xml_render.py

@@ -15,7 +15,6 @@ def get_authnrequest_xml(parameters, signed=False):
     params["AUTHN_REQUEST_SIGNATURE"] = ""
 
     unsigned = render_to_string("saml/sp/xml/authn_request.xml", params)
-    LOGGER.debug("AuthN Request", unsigned=unsigned)
     if not signed:
         return unsigned
 
@@ -24,5 +23,4 @@ def get_authnrequest_xml(parameters, signed=False):
     params["AUTHN_REQUEST_SIGNATURE"] = signature_xml
     signed = render_to_string("saml/sp/xml/authn_request.xml", params)
 
-    LOGGER.debug("AuthN Request", signed=signed)
     return signed

scripts/pre-commit.sh

@@ -1,5 +1,7 @@
 #!/bin/bash -xe
+isort -rc passbook
 black passbook
 scripts/coverage.sh
+bandit -r passbook
 pylint passbook
 prospector