diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 0fa656a04..2fd31e932 100644 --- a/.bumpversion.cfg +++ b/.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\d+)\.(?P\d+)\.(?P\d+)\-(?P.*) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4242de3c6..58d72b47b 100644 --- a/.github/workflows/release.yml +++ b/.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: diff --git a/helm/Chart.yaml b/helm/Chart.yaml index 46ef49f0d..42f5d7af3 100644 --- a/helm/Chart.yaml +++ b/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 diff --git a/helm/values.yaml b/helm/values.yaml index cbd1ccc3d..b888fed40 100644 --- a/helm/values.yaml +++ b/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: "" diff --git a/manage.py b/manage.py index 46a562abb..a9cc30054 100755 --- a/manage.py +++ b/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') diff --git a/passbook/__init__.py b/passbook/__init__.py index 447ab674f..90df09769 100644 --- a/passbook/__init__.py +++ b/passbook/__init__.py @@ -1,2 +1,2 @@ """passbook""" -__version__ = "0.8.1-beta" +__version__ = "0.8.5-beta" 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/api/applications.py b/passbook/core/api/applications.py index 9e4930c46..f0b4c69e2 100644 --- a/passbook/core/api/applications.py +++ b/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", ] diff --git a/passbook/core/exceptions.py b/passbook/core/exceptions.py index 53961e631..01fc51bfe 100644 --- a/passbook/core/exceptions.py +++ b/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.""" diff --git a/passbook/core/forms/applications.py b/passbook/core/forms/applications.py index 0d6756585..8c1a91603 100644 --- a/passbook/core/forms/applications.py +++ b/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(), - "icon_url": forms.TextInput(), + "meta_launch_url": forms.TextInput(), + "meta_icon_url": forms.TextInput(), + "meta_publisher": forms.TextInput(), "policies": FilteredSelectMultiple(_("policies"), False), } labels = { - "launch_url": _("Launch URL"), - "icon_url": _("Icon URL"), + "meta_launch_url": _("Launch URL"), + "meta_icon_url": _("Icon URL"), + "meta_description": _("Description"), + "meta_publisher": _("Publisher"), } + help_texts = {"policies": _("Policies required to access this Application.")} diff --git a/passbook/core/migrations/0008_auto_20200220_1242.py b/passbook/core/migrations/0008_auto_20200220_1242.py new file mode 100644 index 000000000..fe35fffe1 --- /dev/null +++ b/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), + ), + ] diff --git a/passbook/core/models.py b/passbook/core/models.py index bcd00b43b..d11e2c906 100644 --- a/passbook/core/models.py +++ b/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) - icon_url = models.TextField(null=True, blank=True) + skip_authorization = models.BooleanField(default=False) 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): - """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..478903b1c 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 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() - 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..0e03a91af --- /dev/null +++ b/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 diff --git a/passbook/core/views/authentication.py b/passbook/core/views/authentication.py index 6a9e4428f..94053af81 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) + if ui_login_button: + kwargs["sources"].append(ui_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) diff --git a/passbook/factors/otp/models.py b/passbook/factors/otp/models.py index daf9fb4a2..1c87d6d69 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.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: - 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/exceptions.py b/passbook/factors/password/exceptions.py index f561ec5ac..ca56c03c0 100644 --- a/passbook/factors/password/exceptions.py +++ b/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 = [] diff --git a/passbook/factors/password/models.py b/passbook/factors/password/models.py index b5ed6bdf5..a97980641 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.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): - 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/lib/sentry.py b/passbook/lib/sentry.py index 886696e26..453df529e 100644 --- a/passbook/lib/sentry.py +++ b/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"] diff --git a/passbook/policies/engine.py b/passbook/policies/engine.py index 25709f166..62455b0f0 100644 --- a/passbook/policies/engine.py +++ b/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: diff --git a/passbook/policies/exceptions.py b/passbook/policies/exceptions.py index 5ff1f4d45..ec27684dd 100644 --- a/passbook/policies/exceptions.py +++ b/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.""" diff --git a/passbook/policies/expiry/models.py b/passbook/policies/expiry/models.py index 94c59a01e..102360dad 100644 --- a/passbook/policies/expiry/models.py +++ b/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() diff --git a/passbook/policies/expression/evaluator.py b/passbook/policies/expression/evaluator.py index 042a4d9db..baa22e278 100644 --- a/passbook/policies/expression/evaluator.py +++ b/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: diff --git a/passbook/policies/expression/models.py b/passbook/policies/expression/models.py index 8ec07211f..2ba3827a6 100644 --- a/passbook/policies/expression/models.py +++ b/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): diff --git a/passbook/policies/password/models.py b/passbook/policies/password/models.py index 6d81d2109..03b552a21 100644 --- a/passbook/policies/password/models.py +++ b/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() diff --git a/passbook/policies/process.py b/passbook/policies/process.py index 9615300cf..b8114380f 100644 --- a/passbook/policies/process.py +++ b/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() diff --git a/passbook/policies/reputation/models.py b/passbook/policies/reputation/models.py index c1f36e0b8..dfc9014c3 100644 --- a/passbook/policies/reputation/models.py +++ b/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): diff --git a/passbook/policies/struct.py b/passbook/policies/types.py similarity index 100% rename from passbook/policies/struct.py rename to passbook/policies/types.py diff --git a/passbook/policies/webhook/models.py b/passbook/policies/webhook/models.py index d5487ec3f..9e1f29296 100644 --- a/passbook/policies/webhook/models.py +++ b/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): diff --git a/passbook/providers/saml/exceptions.py b/passbook/providers/saml/exceptions.py index a78218ca6..81b0b1b39 100644 --- a/passbook/providers/saml/exceptions.py +++ b/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.""" diff --git a/passbook/providers/saml/processors/base.py b/passbook/providers/saml/processors/base.py index c407d44d0..cacef4c5b 100644 --- a/passbook/providers/saml/processors/base.py +++ b/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: - raise CannotHandleAssertion( - f"can't find SAML request in user session: {exc}" - ) from exc + except KeyError: + raise CannotHandleAssertion(f"Couldn't find SAML request in user session:") 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 diff --git a/passbook/providers/saml/templates/saml/xml/metadata.xml b/passbook/providers/saml/templates/saml/xml/metadata.xml index 2eff8fe23..b2ee996e1 100644 --- a/passbook/providers/saml/templates/saml/xml/metadata.xml +++ b/passbook/providers/saml/templates/saml/xml/metadata.xml @@ -15,26 +15,9 @@ + {{ subject_format }} - urn:oasis:names:tc:SAML:2.0:nameid-format:persistent - + + -{% comment %} - -{# if org #} - - {{ org.name }} - {{ org.display_name }} - {{ org.url }} - -{# endif #} - -{# for contact in contacts #} - - {{ contact.given_name }} - {{ contact.sur_name }} - {{ contact.email }} - -{# endfor #} -{% endcomment %} diff --git a/passbook/providers/saml/urls.py b/passbook/providers/saml/urls.py index c48edc1df..bddf07571 100644 --- a/passbook/providers/saml/urls.py +++ b/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( - "/login/", views.LoginProcessView.as_view(), name="saml-login" + "/login/", views.LoginBeginView.as_view(), name="saml-login" ), path( "/login/process/", diff --git a/passbook/providers/saml/utils/encoding.py b/passbook/providers/saml/utils/encoding.py index 9fe6b662a..9aac5246a 100644 --- a/passbook/providers/saml/utils/encoding.py +++ b/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): diff --git a/passbook/providers/saml/views.py b/passbook/providers/saml/views.py index d21dbd992..2c1931056 100644 --- a/passbook/providers/saml/views.py +++ b/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) - saml_params = self.provider.processor.generate_response() - return self.handle_redirect(saml_params, True) + return self.post(request, application) 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) - response = HttpResponse(metadata, content_type="application/xml") - response["Content-Disposition"] = ( - 'attachment; filename="' '%s_passbook_meta.xml"' % self.provider.name - ) - return response + try: + metadata = DescriptorDownloadView.get_metadata(request, self.provider) + except Provider.application.RelatedObjectDoesNotExist: # pylint: disable=no-member + return bad_request_message( + request, "Provider is not assigned to an application." + ) + 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): diff --git a/passbook/root/settings.py b/passbook/root/settings.py index 493d1291e..34735dcc2 100644 --- a/passbook/root/settings.py +++ b/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" -SESSION_COOKIE_NAME = "passbook_session" +if DEBUG: + 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(), diff --git a/passbook/root/wsgi.py b/passbook/root/wsgi.py index b90cca62a..ff4ff87e9 100644 --- a/passbook/root/wsgi.py +++ b/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: diff --git a/passbook/sources/ldap/connector.py b/passbook/sources/ldap/connector.py index bad3709f0..064a6a628 100644 --- a/passbook/sources/ldap/connector.py +++ b/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 diff --git a/passbook/sources/oauth/models.py b/passbook/sources/oauth/models.py index 40d070df6..17d4e8c2f 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.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): - 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"img/logos/{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/api.py b/passbook/sources/saml/api.py index b117e56df..d89014d4a 100644 --- a/passbook/sources/saml/api.py +++ b/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", diff --git a/passbook/sources/saml/exceptions.py b/passbook/sources/saml/exceptions.py new file mode 100644 index 000000000..2ea148df8 --- /dev/null +++ b/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.""" diff --git a/passbook/sources/saml/forms.py b/passbook/sources/saml/forms.py index 14e350e78..ff6ed84ee 100644 --- a/passbook/sources/saml/forms.py +++ b/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(), } diff --git a/passbook/sources/saml/migrations/0005_auto_20200220_1621.py b/passbook/sources/saml/migrations/0005_auto_20200220_1621.py new file mode 100644 index 000000000..ff15cdb60 --- /dev/null +++ b/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", + ), + ), + ] diff --git a/passbook/sources/saml/models.py b/passbook/sources/saml/models.py index 273380382..4144b2ac7 100644 --- a/passbook/sources/saml/models.py +++ b/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): - 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} + "passbook_sources_saml:metadata", kwargs={"source_slug": self.slug} ) return f'Metadata Download' diff --git a/passbook/sources/saml/processors/__init__.py b/passbook/sources/saml/processors/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/passbook/sources/saml/processors/base.py b/passbook/sources/saml/processors/base.py new file mode 100644 index 000000000..775791adf --- /dev/null +++ b/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.: + + + email@example.com + """ + 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 diff --git a/passbook/sources/saml/templates/saml/sp/xml/sp_sso_descriptor.xml b/passbook/sources/saml/templates/saml/sp/xml/sp_sso_descriptor.xml new file mode 100644 index 000000000..f702c2654 --- /dev/null +++ b/passbook/sources/saml/templates/saml/sp/xml/sp_sso_descriptor.xml @@ -0,0 +1,22 @@ + + + + + + {{ cert_public_key }} + + + + + + + {{ cert_public_key }} + + + + urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress + + + diff --git a/passbook/sources/saml/templates/saml/sp/xml/spssodescriptor.xml b/passbook/sources/saml/templates/saml/sp/xml/spssodescriptor.xml deleted file mode 100644 index e990f5a91..000000000 --- a/passbook/sources/saml/templates/saml/sp/xml/spssodescriptor.xml +++ /dev/null @@ -1,70 +0,0 @@ - - - - - - {{ cert_public_key }} - - - - - - - {{ cert_public_key }} - - - - - urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress - - -{% comment %} - - - - urn:oasis:names:tc:SAML:2.0:nameid-format:transient - - - - - - Service Provider Portal - - - - -{% endcomment %} - -{% comment %} - -{# if org #} - - {{ org.name }} - {{ org.display_name }} - {{ org.url }} - -{# endif #} - -{# for contact in contacts #} - - {{ contact.given_name }} - {{ contact.sur_name }} - {{ contact.email }} - -{# endfor #} -{% endcomment %} - diff --git a/passbook/sources/saml/utils.py b/passbook/sources/saml/utils.py index 4bd8b2b7f..139c556b4 100644 --- a/passbook/sources/saml/utils.py +++ b/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): - """Get Source's entity ID, falling back to our Metadata URL if none is set""" - entity_id = source.entity_id - if entity_id is None: +def get_issuer(request: HttpRequest, source: SAMLSource) -> str: + """Get Source's Issuer, falling back to our Metadata URL if none is set""" + issuer = source.issuer + 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.: - - - email@example.com - """ - 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 diff --git a/passbook/sources/saml/views.py b/passbook/sources/saml/views.py index 18a07f5ba..9f6516afa 100644 --- a/passbook/sources/saml/views.py +++ b/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.utils import ( - _get_user_from_response, - build_full_url, - get_entity_id, +from passbook.sources.saml.exceptions import ( + MissingSAMLResponse, + UnsupportedNameIDFormat, ) +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) - data = request.POST.get("SAMLResponse", None) - response = base64.b64decode(data) - root = ElementTree.fromstring(response) - user = _get_user_from_response(root) - # attributes = _get_attributes_from_response(root) - login(request, user, backend="django.contrib.auth.backends.ModelBackend") - return redirect(reverse("passbook_core:overview")) + processor = Processor(source) + try: + processor.parse(request) + except MissingSAMLResponse as exc: + return bad_request_message(request, str(exc)) + + try: + 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, - "cert_public_key": source.signing_cert, + "issuer": issuer, + "cert_public_key": cert_stripped, }, ) diff --git a/passbook/sources/saml/xml_render.py b/passbook/sources/saml/xml_render.py index 79fc9bd83..6d6726b14 100644 --- a/passbook/sources/saml/xml_render.py +++ b/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 diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh index 7e5a6e3b9..1658c5a96 100755 --- a/scripts/pre-commit.sh +++ b/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