From 24da24b5d58e2e60aebe71d0d275360f2f827771 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sat, 5 Jun 2021 14:02:15 +0200 Subject: [PATCH] stages/identification: allow setting of a password stage to check password and identity in a single step closes #970 Signed-off-by: Jens Langhammer --- authentik/flows/views.py | 5 ++ authentik/stages/identification/api.py | 1 + ...0010_identificationstage_password_stage.py | 26 +++++++ authentik/stages/identification/models.py | 11 +++ authentik/stages/identification/stage.py | 41 ++++++++-- authentik/stages/identification/tests.py | 76 ++++++++++++++++++- authentik/stages/password/tests.py | 8 +- schema.yml | 24 ++++++ web/src/api/Flows.ts | 10 --- web/src/elements/forms/FormElement.ts | 4 +- web/src/flows/stages/base.ts | 22 +++++- .../identification/IdentificationStage.ts | 24 +++++- web/src/flows/stages/prompt/PromptStage.ts | 19 ----- web/src/locales/en.po | 11 +++ web/src/locales/pseudo-LOCALE.po | 11 +++ .../identification/IdentificationStageForm.ts | 16 ++++ 16 files changed, 260 insertions(+), 49 deletions(-) create mode 100644 authentik/stages/identification/migrations/0010_identificationstage_password_stage.py delete mode 100644 web/src/api/Flows.ts diff --git a/authentik/flows/views.py b/authentik/flows/views.py index 149793161..e0dfc7328 100644 --- a/authentik/flows/views.py +++ b/authentik/flows/views.py @@ -2,6 +2,7 @@ from traceback import format_tb from typing import Any, Optional +from django.conf import settings from django.contrib.auth.mixins import LoginRequiredMixin from django.http import Http404, HttpRequest, HttpResponse, HttpResponseRedirect from django.http.request import QueryDict @@ -203,6 +204,8 @@ class FlowExecutorView(APIView): stage_response = self.current_stage_view.get(request, *args, **kwargs) return to_stage_response(request, stage_response) except Exception as exc: # pylint: disable=broad-except + if settings.DEBUG or settings.TEST: + raise exc capture_exception(exc) self._logger.warning(exc) return to_stage_response(request, FlowErrorResponse(request, exc)) @@ -242,6 +245,8 @@ class FlowExecutorView(APIView): stage_response = self.current_stage_view.post(request, *args, **kwargs) return to_stage_response(request, stage_response) except Exception as exc: # pylint: disable=broad-except + if settings.DEBUG or settings.TEST: + raise exc capture_exception(exc) self._logger.warning(exc) return to_stage_response(request, FlowErrorResponse(request, exc)) diff --git a/authentik/stages/identification/api.py b/authentik/stages/identification/api.py index 082241399..e2b44ce54 100644 --- a/authentik/stages/identification/api.py +++ b/authentik/stages/identification/api.py @@ -13,6 +13,7 @@ class IdentificationStageSerializer(StageSerializer): model = IdentificationStage fields = StageSerializer.Meta.fields + [ "user_fields", + "password_stage", "case_insensitive_matching", "show_matched_user", "enrollment_flow", diff --git a/authentik/stages/identification/migrations/0010_identificationstage_password_stage.py b/authentik/stages/identification/migrations/0010_identificationstage_password_stage.py new file mode 100644 index 000000000..3ebd17753 --- /dev/null +++ b/authentik/stages/identification/migrations/0010_identificationstage_password_stage.py @@ -0,0 +1,26 @@ +# Generated by Django 3.2.3 on 2021-06-05 12:30 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_stages_password", "0005_auto_20210402_2221"), + ("authentik_stages_identification", "0009_identificationstage_sources"), + ] + + operations = [ + migrations.AddField( + model_name="identificationstage", + name="password_stage", + field=models.ForeignKey( + default=None, + help_text="When set, shows a password field, instead of showing the password field as seaprate step.", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="authentik_stages_password.passwordstage", + ), + ), + ] diff --git a/authentik/stages/identification/models.py b/authentik/stages/identification/models.py index b2d71b358..a25d149c2 100644 --- a/authentik/stages/identification/models.py +++ b/authentik/stages/identification/models.py @@ -9,6 +9,7 @@ from rest_framework.serializers import BaseSerializer from authentik.core.models import Source from authentik.flows.models import Flow, Stage +from authentik.stages.password.models import PasswordStage class UserFields(models.TextChoices): @@ -32,6 +33,16 @@ class IdentificationStage(Stage): ), ) + password_stage = models.ForeignKey( + PasswordStage, + null=True, + default=None, + on_delete=models.SET_NULL, + help_text=_( + "When set, shows a password field, instead of showing the " + "password field as seaprate step.", + ), + ) case_insensitive_matching = models.BooleanField( default=True, help_text=_( diff --git a/authentik/stages/identification/stage.py b/authentik/stages/identification/stage.py index 187ffe8e5..ec86e2d9e 100644 --- a/authentik/stages/identification/stage.py +++ b/authentik/stages/identification/stage.py @@ -1,13 +1,14 @@ """Identification stage logic""" from dataclasses import asdict from time import sleep -from typing import Optional +from typing import Any, Optional +from django.core.exceptions import PermissionDenied from django.db.models import Q from django.http import HttpResponse from django.urls import reverse from django.utils.translation import gettext as _ -from rest_framework.fields import CharField, ListField +from rest_framework.fields import BooleanField, CharField, ListField from rest_framework.serializers import ValidationError from structlog.stdlib import get_logger @@ -22,6 +23,7 @@ from authentik.flows.stage import ( from authentik.flows.views import SESSION_KEY_APPLICATION_PRE from authentik.stages.identification.models import IdentificationStage from authentik.stages.identification.signals import identification_failed +from authentik.stages.password.stage import authenticate LOGGER = get_logger() @@ -30,6 +32,7 @@ class IdentificationChallenge(Challenge): """Identification challenges with all UI elements""" user_fields = ListField(child=CharField(), allow_empty=True, allow_null=True) + password_fields = BooleanField() application_pre = CharField(required=False) enroll_url = CharField(required=False) @@ -44,22 +47,43 @@ class IdentificationChallengeResponse(ChallengeResponse): """Identification challenge""" uid_field = CharField() + password = CharField(required=False, allow_blank=True, allow_null=True) component = CharField(default="ak-stage-identification") pre_user: Optional[User] = None - def validate_uid_field(self, value: str) -> str: - """Validate that user exists""" - pre_user = self.stage.get_user(value) + def validate(self, data: dict[str, Any]) -> dict[str, Any]: + """Validate that user exists, and optionally their password""" + uid_field = data["uid_field"] + current_stage: IdentificationStage = self.stage.executor.current_stage + + pre_user = self.stage.get_user(uid_field) if not pre_user: sleep(0.150) - LOGGER.debug("invalid_login", identifier=value) + LOGGER.debug("invalid_login", identifier=uid_field) identification_failed.send( - sender=self, request=self.stage.request, uid_field=value + sender=self, request=self.stage.request, uid_field=uid_field ) raise ValidationError("Failed to authenticate.") self.pre_user = pre_user - return value + if not current_stage.password_stage: + # No password stage select, don't validate the password + return data + + password = data["password"] + try: + user = authenticate( + self.stage.request, + current_stage.password_stage.backends, + username=self.pre_user.username, + password=password, + ) + if not user: + raise ValidationError("Failed to authenticate.") + self.pre_user = user + except PermissionDenied as exc: + raise ValidationError(str(exc)) from exc + return data class IdentificationStageView(ChallengeStageView): @@ -92,6 +116,7 @@ class IdentificationStageView(ChallengeStageView): "primary_action": _("Log in"), "component": "ak-stage-identification", "user_fields": current_stage.user_fields, + "password_fields": bool(current_stage.password_stage), } ) # If the user has been redirected to us whilst trying to access an diff --git a/authentik/stages/identification/tests.py b/authentik/stages/identification/tests.py index 812233914..315f7a32e 100644 --- a/authentik/stages/identification/tests.py +++ b/authentik/stages/identification/tests.py @@ -6,8 +6,10 @@ from django.utils.encoding import force_str from authentik.core.models import User from authentik.flows.challenge import ChallengeTypes from authentik.flows.models import Flow, FlowDesignation, FlowStageBinding +from authentik.providers.oauth2.generators import generate_client_secret from authentik.sources.oauth.models import OAuthSource from authentik.stages.identification.models import IdentificationStage, UserFields +from authentik.stages.password.models import PasswordStage class TestIdentificationStage(TestCase): @@ -15,7 +17,10 @@ class TestIdentificationStage(TestCase): def setUp(self): super().setUp() - self.user = User.objects.create(username="unittest", email="test@beryju.org") + self.password = generate_client_secret() + self.user = User.objects.create_user( + username="unittest", email="test@beryju.org", password=self.password + ) self.client = Client() # OAuthSource for the login view @@ -62,6 +67,73 @@ class TestIdentificationStage(TestCase): }, ) + def test_valid_with_password(self): + """Test with valid email and password in single step""" + pw_stage = PasswordStage.objects.create( + name="password", backends=["django.contrib.auth.backends.ModelBackend"] + ) + self.stage.password_stage = pw_stage + self.stage.save() + form_data = {"uid_field": self.user.email, "password": self.password} + url = reverse( + "authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug} + ) + response = self.client.post(url, form_data) + self.assertEqual(response.status_code, 200) + self.assertJSONEqual( + force_str(response.content), + { + "component": "xak-flow-redirect", + "to": reverse("authentik_core:root-redirect"), + "type": ChallengeTypes.REDIRECT.value, + }, + ) + + def test_invalid_with_password(self): + """Test with valid email and invalid password in single step""" + pw_stage = PasswordStage.objects.create( + name="password", backends=["django.contrib.auth.backends.ModelBackend"] + ) + self.stage.password_stage = pw_stage + self.stage.save() + form_data = { + "uid_field": self.user.email, + "password": self.password + "test", + } + url = reverse( + "authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug} + ) + response = self.client.post(url, form_data) + self.assertEqual(response.status_code, 200) + self.assertJSONEqual( + force_str(response.content), + { + "background": self.flow.background.url, + "type": ChallengeTypes.NATIVE.value, + "component": "ak-stage-identification", + "password_fields": True, + "primary_action": "Log in", + "response_errors": { + "non_field_errors": [ + {"code": "invalid", "string": "Failed to " "authenticate."} + ] + }, + "sources": [ + { + "challenge": { + "component": "xak-flow-redirect", + "to": "/source/oauth/login/test/", + "type": ChallengeTypes.REDIRECT.value, + }, + "icon_url": "/static/authentik/sources/.svg", + "name": "test", + } + ], + "title": "", + "user_fields": ["email"], + }, + ) + def test_invalid_with_username(self): """Test invalid with username (user exists but stage only allows email)""" form_data = {"uid_field": self.user.username} @@ -113,6 +185,7 @@ class TestIdentificationStage(TestCase): "type": ChallengeTypes.NATIVE.value, "component": "ak-stage-identification", "user_fields": ["email"], + "password_fields": False, "enroll_url": reverse( "authentik_core:if-flow", kwargs={"flow_slug": "unique-enrollment-string"}, @@ -160,6 +233,7 @@ class TestIdentificationStage(TestCase): "type": ChallengeTypes.NATIVE.value, "component": "ak-stage-identification", "user_fields": ["email"], + "password_fields": False, "recovery_url": reverse( "authentik_core:if-flow", kwargs={"flow_slug": "unique-recovery-string"}, diff --git a/authentik/stages/password/tests.py b/authentik/stages/password/tests.py index 21a9a8f99..af6026eda 100644 --- a/authentik/stages/password/tests.py +++ b/authentik/stages/password/tests.py @@ -1,6 +1,4 @@ """password tests""" -import string -from random import SystemRandom from unittest.mock import MagicMock, patch from django.core.exceptions import PermissionDenied @@ -15,6 +13,7 @@ from authentik.flows.models import Flow, FlowDesignation, FlowStageBinding from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan from authentik.flows.tests.test_views import TO_STAGE_RESPONSE_MOCK from authentik.flows.views import SESSION_KEY_PLAN +from authentik.providers.oauth2.generators import generate_client_secret from authentik.stages.password.models import PasswordStage MOCK_BACKEND_AUTHENTICATE = MagicMock(side_effect=PermissionDenied("test")) @@ -25,10 +24,7 @@ class TestPasswordStage(TestCase): def setUp(self): super().setUp() - self.password = "".join( - SystemRandom().choice(string.ascii_uppercase + string.digits) - for _ in range(8) - ) + self.password = generate_client_secret() self.user = User.objects.create_user( username="unittest", email="test@beryju.org", password=self.password ) diff --git a/schema.yml b/schema.yml index beee84f1b..ca6512e91 100644 --- a/schema.yml +++ b/schema.yml @@ -17679,6 +17679,8 @@ components: items: type: string nullable: true + password_fields: + type: boolean application_pre: type: string enroll_url: @@ -17692,6 +17694,7 @@ components: items: $ref: '#/components/schemas/UILoginButton' required: + - password_fields - primary_action - type - user_fields @@ -17704,6 +17707,9 @@ components: default: ak-stage-identification uid_field: type: string + password: + type: string + nullable: true required: - uid_field IdentificationStage: @@ -17736,6 +17742,12 @@ components: $ref: '#/components/schemas/UserFieldsEnum' description: Fields of the user object to match against. (Hold shift to select multiple options) + password_stage: + type: string + format: uuid + nullable: true + description: When set, shows a password field, instead of showing the password + field as seaprate step. case_insensitive_matching: type: boolean description: When enabled, user fields are matched regardless of their casing. @@ -17784,6 +17796,12 @@ components: $ref: '#/components/schemas/UserFieldsEnum' description: Fields of the user object to match against. (Hold shift to select multiple options) + password_stage: + type: string + format: uuid + nullable: true + description: When set, shows a password field, instead of showing the password + field as seaprate step. case_insensitive_matching: type: boolean description: When enabled, user fields are matched regardless of their casing. @@ -22307,6 +22325,12 @@ components: $ref: '#/components/schemas/UserFieldsEnum' description: Fields of the user object to match against. (Hold shift to select multiple options) + password_stage: + type: string + format: uuid + nullable: true + description: When set, shows a password field, instead of showing the password + field as seaprate step. case_insensitive_matching: type: boolean description: When enabled, user fields are matched regardless of their casing. diff --git a/web/src/api/Flows.ts b/web/src/api/Flows.ts deleted file mode 100644 index 2b147cc87..000000000 --- a/web/src/api/Flows.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { ChallengeChoices } from "authentik-api"; - -export interface Error { - code: string; - string: string; -} - -export interface ErrorDict { - [key: string]: Error[]; -} diff --git a/web/src/elements/forms/FormElement.ts b/web/src/elements/forms/FormElement.ts index b24c96e46..fbf805c81 100644 --- a/web/src/elements/forms/FormElement.ts +++ b/web/src/elements/forms/FormElement.ts @@ -1,8 +1,8 @@ import { customElement, LitElement, CSSResult, property, css } from "lit-element"; import { TemplateResult, html } from "lit-html"; -import { Error } from "../../api/Flows"; import PFForm from "@patternfly/patternfly/components/Form/form.css"; import PFFormControl from "@patternfly/patternfly/components/FormControl/form-control.css"; +import { ErrorDetail } from "authentik-api"; @customElement("ak-form-element") export class FormElement extends LitElement { @@ -25,7 +25,7 @@ export class FormElement extends LitElement { required = false; @property({ attribute: false }) - errors?: Error[]; + errors?: ErrorDetail[]; updated(): void { this.querySelectorAll("input[autofocus]").forEach(input => { diff --git a/web/src/flows/stages/base.ts b/web/src/flows/stages/base.ts index 187687de6..459d2038d 100644 --- a/web/src/flows/stages/base.ts +++ b/web/src/flows/stages/base.ts @@ -1,4 +1,5 @@ -import { LitElement, property } from "lit-element"; +import { ErrorDetail } from "authentik-api"; +import { html, LitElement, property, TemplateResult } from "lit-element"; export interface StageHost { challenge?: unknown; @@ -22,4 +23,23 @@ export class BaseStage extends LitElement { this.host?.submit(object as unknown as Tout); } + renderNonFieldErrors(errors: ErrorDetail[]): TemplateResult { + if (!errors) { + return html``; + } + return html`
+ ${errors.map(err => { + return html`
+
+ +
+

+ ${err.string} +

+
`; + })} +
`; + } + + } diff --git a/web/src/flows/stages/identification/IdentificationStage.ts b/web/src/flows/stages/identification/IdentificationStage.ts index 78d23b30f..9871effb2 100644 --- a/web/src/flows/stages/identification/IdentificationStage.ts +++ b/web/src/flows/stages/identification/IdentificationStage.ts @@ -7,6 +7,7 @@ import PFFormControl from "@patternfly/patternfly/components/FormControl/form-co import PFTitle from "@patternfly/patternfly/components/Title/title.css"; import PFButton from "@patternfly/patternfly/components/Button/button.css"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; +import PFAlert from "@patternfly/patternfly/components/Alert/alert.css"; import AKGlobal from "../../../authentik.css"; import "../../../elements/forms/FormElement"; import "../../../elements/EmptyState"; @@ -25,7 +26,7 @@ export const PasswordManagerPrefill: { export class IdentificationStage extends BaseStage { static get styles(): CSSResult[] { - return [PFBase, PFLogin, PFForm, PFFormControl, PFTitle, PFButton, AKGlobal].concat( + return [PFBase, PFAlert, PFLogin, PFForm, PFFormControl, PFTitle, PFButton, AKGlobal].concat( css` /* login page's icons */ .pf-c-login__main-footer-links-item button { @@ -160,7 +161,7 @@ export class IdentificationStage extends BaseStage + .errors=${(this.challenge.responseErrors || {})["uid_field"]}> + ${this.challenge.passwordFields ? html` + + + + `: html``} + ${"non_field_errors" in (this.challenge?.responseErrors || {}) ? + this.renderNonFieldErrors(this.challenge?.responseErrors?.non_field_errors || []) : + html``}
`; - } - render(): TemplateResult { if (!this.challenge) { return html`${t`Fields a user can identify themselves with. If no fields are selected, the user will only be able to use sources.`}

${t`Hold control/command to select multiple items.`}

+ + +

${t`When selected, a password field is shown on the same page instead of a separate page. This prevents username enumeration attacks.`}

+