From 750669dcab256da8c9ac0f65e8f5906e29cbf103 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Tue, 19 Dec 2023 18:30:56 +0100 Subject: [PATCH] stages/email: improve error handling for incorrect template syntax (cherry-pick #7758) (#7936) stages/email: improve error handling for incorrect template syntax (#7758) * stages/email: improve error handling for incorrect template syntax * add tests --------- Signed-off-by: Jens Langhammer Co-authored-by: Jens L --- authentik/core/api/sources.py | 2 +- authentik/flows/stage.py | 6 +- authentik/stages/email/stage.py | 43 +++++++++----- .../stages/email/tests/test_templates.py | 58 +++++++++++++++++-- 4 files changed, 90 insertions(+), 19 deletions(-) diff --git a/authentik/core/api/sources.py b/authentik/core/api/sources.py index 292f38cd3..eff2c9211 100644 --- a/authentik/core/api/sources.py +++ b/authentik/core/api/sources.py @@ -38,7 +38,7 @@ class SourceSerializer(ModelSerializer, MetaNameSerializer): managed = ReadOnlyField() component = SerializerMethodField() - icon = ReadOnlyField(source="get_icon") + icon = ReadOnlyField(source="icon_url") def get_component(self, obj: Source) -> str: """Get object component so that we know how to edit the object""" diff --git a/authentik/flows/stage.py b/authentik/flows/stage.py index d9fa75893..528a7bef5 100644 --- a/authentik/flows/stage.py +++ b/authentik/flows/stage.py @@ -167,7 +167,11 @@ class ChallengeStageView(StageView): stage_type=self.__class__.__name__, method="get_challenge" ).time(), ): - challenge = self.get_challenge(*args, **kwargs) + try: + challenge = self.get_challenge(*args, **kwargs) + except StageInvalidException as exc: + self.logger.debug("Got StageInvalidException", exc=exc) + return self.executor.stage_invalid() with Hub.current.start_span( op="authentik.flow.stage._get_challenge", description=self.__class__.__name__, diff --git a/authentik/stages/email/stage.py b/authentik/stages/email/stage.py index 0b92173d5..1aaaa7482 100644 --- a/authentik/stages/email/stage.py +++ b/authentik/stages/email/stage.py @@ -5,6 +5,7 @@ from uuid import uuid4 from django.contrib import messages from django.http import HttpRequest, HttpResponse from django.http.request import QueryDict +from django.template.exceptions import TemplateSyntaxError from django.urls import reverse from django.utils.text import slugify from django.utils.timezone import now @@ -12,11 +13,14 @@ from django.utils.translation import gettext as _ from rest_framework.fields import CharField from rest_framework.serializers import ValidationError +from authentik.events.models import Event, EventAction from authentik.flows.challenge import Challenge, ChallengeResponse, ChallengeTypes +from authentik.flows.exceptions import StageInvalidException from authentik.flows.models import FlowDesignation, FlowToken from authentik.flows.planner import PLAN_CONTEXT_IS_RESTORED, PLAN_CONTEXT_PENDING_USER from authentik.flows.stage import ChallengeStageView from authentik.flows.views.executor import QS_KEY_TOKEN, QS_QUERY +from authentik.lib.utils.errors import exception_to_string from authentik.stages.email.models import EmailStage from authentik.stages.email.tasks import send_mails from authentik.stages.email.utils import TemplateEmailMessage @@ -104,18 +108,27 @@ class EmailStageView(ChallengeStageView): current_stage: EmailStage = self.executor.current_stage token = self.get_token() # Send mail to user - message = TemplateEmailMessage( - subject=_(current_stage.subject), - to=[email], - language=pending_user.locale(self.request), - template_name=current_stage.template, - template_context={ - "url": self.get_full_url(**{QS_KEY_TOKEN: token.key}), - "user": pending_user, - "expires": token.expires, - }, - ) - send_mails(current_stage, message) + try: + message = TemplateEmailMessage( + subject=_(current_stage.subject), + to=[email], + language=pending_user.locale(self.request), + template_name=current_stage.template, + template_context={ + "url": self.get_full_url(**{QS_KEY_TOKEN: token.key}), + "user": pending_user, + "expires": token.expires, + }, + ) + send_mails(current_stage, message) + except TemplateSyntaxError as exc: + Event.new( + EventAction.CONFIGURATION_ERROR, + message=_("Exception occurred while rendering E-mail template"), + error=exception_to_string(exc), + template=current_stage.template, + ).from_http(self.request) + raise StageInvalidException from exc def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: # Check if the user came back from the email link to verify @@ -136,7 +149,11 @@ class EmailStageView(ChallengeStageView): return self.executor.stage_invalid() # Check if we've already sent the initial e-mail if PLAN_CONTEXT_EMAIL_SENT not in self.executor.plan.context: - self.send_email() + try: + self.send_email() + except StageInvalidException as exc: + self.logger.debug("Got StageInvalidException", exc=exc) + return self.executor.stage_invalid() self.executor.plan.context[PLAN_CONTEXT_EMAIL_SENT] = True return super().get(request, *args, **kwargs) diff --git a/authentik/stages/email/tests/test_templates.py b/authentik/stages/email/tests/test_templates.py index 2bd4d0c5e..f8531b078 100644 --- a/authentik/stages/email/tests/test_templates.py +++ b/authentik/stages/email/tests/test_templates.py @@ -4,11 +4,20 @@ from pathlib import Path from shutil import rmtree from tempfile import mkdtemp, mkstemp from typing import Any +from unittest.mock import PropertyMock, patch from django.conf import settings -from django.test import TestCase +from django.core.mail.backends.locmem import EmailBackend +from django.urls import reverse -from authentik.stages.email.models import get_template_choices +from authentik.core.tests.utils import create_test_admin_user, create_test_flow +from authentik.events.models import Event, EventAction +from authentik.flows.markers import StageMarker +from authentik.flows.models import FlowDesignation, FlowStageBinding +from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan +from authentik.flows.tests import FlowTestCase +from authentik.flows.views.executor import SESSION_KEY_PLAN +from authentik.stages.email.models import EmailStage, get_template_choices def get_templates_setting(temp_dir: str) -> dict[str, Any]: @@ -18,11 +27,18 @@ def get_templates_setting(temp_dir: str) -> dict[str, Any]: return templates_setting -class TestEmailStageTemplates(TestCase): +class TestEmailStageTemplates(FlowTestCase): """Email tests""" def setUp(self) -> None: - self.dir = mkdtemp() + self.dir = Path(mkdtemp()) + self.user = create_test_admin_user() + + self.flow = create_test_flow(FlowDesignation.AUTHENTICATION) + self.stage = EmailStage.objects.create( + name="email", + ) + self.binding = FlowStageBinding.objects.create(target=self.flow, stage=self.stage, order=2) def tearDown(self) -> None: rmtree(self.dir) @@ -38,3 +54,37 @@ class TestEmailStageTemplates(TestCase): self.assertEqual(len(choices), 3) unlink(file) unlink(file2) + + def test_custom_template_invalid_syntax(self): + """Test with custom template""" + with open(self.dir / Path("invalid.html"), "w+", encoding="utf-8") as _invalid: + _invalid.write("{% blocktranslate %}") + with self.settings(TEMPLATES=get_templates_setting(self.dir)): + self.stage.template = "invalid.html" + plan = FlowPlan( + flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()] + ) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + with patch( + "authentik.stages.email.models.EmailStage.backend_class", + PropertyMock(return_value=EmailBackend), + ): + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertStageResponse( + response, + self.flow, + error_message="Unknown error", + ) + events = Event.objects.filter(action=EventAction.CONFIGURATION_ERROR) + self.assertEqual(len(events), 1) + event = events.first() + self.assertEqual( + event.context["message"], "Exception occurred while rendering E-mail template" + ) + self.assertEqual(event.context["template"], "invalid.html")