From 60b7d9de1fe10fff9d8e81512b5c64de3d3f20a9 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 8 Apr 2024 19:44:46 +0200 Subject: [PATCH 1/6] stages/authenticator_validate: add ability to limit webauthn device types Signed-off-by: Jens Langhammer --- .../stages/authenticator_validate/api.py | 7 ++ .../authenticator_validate/challenge.py | 28 +++-- ...datestage_webauthn_allowed_device_types.py | 27 +++++ .../stages/authenticator_validate/models.py | 3 + .../stages/authenticator_validate/stage.py | 20 +++- blueprints/schema.json | 8 ++ schema.yml | 21 ++++ .../AuthenticatorValidateStageForm.ts | 107 +++++++++++++----- .../AuthenticatorWebAuthnStageForm.ts | 27 +---- .../stages/authenticator_webauthn/utils.ts | 15 +++ .../stages/authenticator_validate/index.md | 12 ++ 11 files changed, 214 insertions(+), 61 deletions(-) create mode 100644 authentik/stages/authenticator_validate/migrations/0013_authenticatorvalidatestage_webauthn_allowed_device_types.py create mode 100644 web/src/admin/stages/authenticator_webauthn/utils.ts diff --git a/authentik/stages/authenticator_validate/api.py b/authentik/stages/authenticator_validate/api.py index 7808c9c48176..2ed053306f2e 100644 --- a/authentik/stages/authenticator_validate/api.py +++ b/authentik/stages/authenticator_validate/api.py @@ -7,11 +7,16 @@ from authentik.flows.api.stages import StageSerializer from authentik.flows.models import NotConfiguredAction from authentik.stages.authenticator_validate.models import AuthenticatorValidateStage +from authentik.stages.authenticator_webauthn.api.device_types import WebAuthnDeviceTypeSerializer class AuthenticatorValidateStageSerializer(StageSerializer): """AuthenticatorValidateStage Serializer""" + webauthn_allowed_device_types_obj = WebAuthnDeviceTypeSerializer( + source="webauthn_allowed_device_types", many=True, read_only=True + ) + def validate_not_configured_action(self, value): """Ensure that a configuration stage is set when not_configured_action is configure""" configuration_stages = self.initial_data.get("configuration_stages", None) @@ -31,6 +36,8 @@ class Meta: "configuration_stages", "last_auth_threshold", "webauthn_user_verification", + "webauthn_allowed_device_types", + "webauthn_allowed_device_types_obj", ] diff --git a/authentik/stages/authenticator_validate/challenge.py b/authentik/stages/authenticator_validate/challenge.py index 60a6786c5204..0c6fb5d604f6 100644 --- a/authentik/stages/authenticator_validate/challenge.py +++ b/authentik/stages/authenticator_validate/challenge.py @@ -14,8 +14,9 @@ from webauthn import options_to_json from webauthn.authentication.generate_authentication_options import generate_authentication_options from webauthn.authentication.verify_authentication_response import verify_authentication_response +from webauthn.helpers import parse_authentication_credential_json from webauthn.helpers.base64url_to_bytes import base64url_to_bytes -from webauthn.helpers.exceptions import InvalidAuthenticationResponse +from webauthn.helpers.exceptions import InvalidAuthenticationResponse, InvalidJSONStructure from webauthn.helpers.structs import UserVerificationRequirement from authentik.core.api.utils import JSONDictField, PassiveSerializer @@ -131,23 +132,32 @@ def validate_challenge_webauthn(data: dict, stage_view: StageView, user: User) - """Validate WebAuthn Challenge""" request = stage_view.request challenge = request.session.get(SESSION_KEY_WEBAUTHN_CHALLENGE) - credential_id = data.get("id") + stage: AuthenticatorValidateStage = stage_view.executor.current_stage + try: + credential = parse_authentication_credential_json(data) + except InvalidJSONStructure: + raise ValidationError("Invalid device", "invalid") from None - device = WebAuthnDevice.objects.filter(credential_id=credential_id).first() + device = WebAuthnDevice.objects.filter(credential_id=credential.id).first() if not device: - raise ValidationError("Invalid device") + raise ValidationError("Invalid device", "invalid") + # When a device_type was set when creating the device (2024.4+), and we have a limitation, + # make sure the device type is allowed. + if ( + device.device_type + and stage.webauthn_allowed_device_types.exists() + and not stage.webauthn_allowed_device_types.filter(pk=device.device_type.pk).exists() + ): + raise ValidationError("Invalid device", "invalid") # We can only check the device's user if the user we're given isn't anonymous # as this validation is also used for password-less login where webauthn is the very first # step done by a user. Only if this validation happens at a later stage we can check # that the device belongs to the user if not user.is_anonymous and device.user != user: - raise ValidationError("Invalid device") - - stage: AuthenticatorValidateStage = stage_view.executor.current_stage - + raise ValidationError("Invalid device", "invalid") try: authentication_verification = verify_authentication_response( - credential=data, + credential=credential, expected_challenge=challenge, expected_rp_id=get_rp_id(request), expected_origin=get_origin(request), diff --git a/authentik/stages/authenticator_validate/migrations/0013_authenticatorvalidatestage_webauthn_allowed_device_types.py b/authentik/stages/authenticator_validate/migrations/0013_authenticatorvalidatestage_webauthn_allowed_device_types.py new file mode 100644 index 000000000000..52c4be818291 --- /dev/null +++ b/authentik/stages/authenticator_validate/migrations/0013_authenticatorvalidatestage_webauthn_allowed_device_types.py @@ -0,0 +1,27 @@ +# Generated by Django 5.0.3 on 2024-04-08 18:33 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "authentik_stages_authenticator_validate", + "0012_authenticatorvalidatestage_webauthn_user_verification", + ), + ( + "authentik_stages_authenticator_webauthn", + "0010_webauthndevicetype_authenticatorwebauthnstage_and_more", + ), + ] + + operations = [ + migrations.AddField( + model_name="authenticatorvalidatestage", + name="webauthn_allowed_device_types", + field=models.ManyToManyField( + blank=True, to="authentik_stages_authenticator_webauthn.webauthndevicetype" + ), + ), + ] diff --git a/authentik/stages/authenticator_validate/models.py b/authentik/stages/authenticator_validate/models.py index 3b2c6b2cfdd0..b0c613368549 100644 --- a/authentik/stages/authenticator_validate/models.py +++ b/authentik/stages/authenticator_validate/models.py @@ -71,6 +71,9 @@ class AuthenticatorValidateStage(Stage): choices=UserVerification.choices, default=UserVerification.PREFERRED, ) + webauthn_allowed_device_types = models.ManyToManyField( + "authentik_stages_authenticator_webauthn.WebAuthnDeviceType", blank=True + ) @property def serializer(self) -> type[BaseSerializer]: diff --git a/authentik/stages/authenticator_validate/stage.py b/authentik/stages/authenticator_validate/stage.py index 1413ee671b18..2edb0525a65a 100644 --- a/authentik/stages/authenticator_validate/stage.py +++ b/authentik/stages/authenticator_validate/stage.py @@ -5,6 +5,7 @@ from django.conf import settings from django.http import HttpRequest, HttpResponse +from django.utils.translation import gettext_lazy as _ from jwt import PyJWTError, decode, encode from rest_framework.fields import CharField, IntegerField, ListField, UUIDField from rest_framework.serializers import ValidationError @@ -176,15 +177,30 @@ def get_device_challenges(self) -> list[dict]: threshold = timedelta_from_string(stage.last_auth_threshold) allowed_devices = [] + has_webauthn_filters_set = stage.webauthn_allowed_device_types.exists() + for device in user_devices: device_class = device.__class__.__name__.lower().replace("device", "") if device_class not in stage.device_classes: self.logger.debug("device class not allowed", device_class=device_class) continue if isinstance(device, SMSDevice) and device.is_hashed: - self.logger.debug("Hashed SMS device, skipping") + self.logger.debug("Hashed SMS device, skipping", device=device) continue allowed_devices.append(device) + # Ignore WebAuthn devices which are not in the allowed types + if ( + isinstance(device, WebAuthnDevice) + and device.device_type + and has_webauthn_filters_set + ): + if not stage.webauthn_allowed_device_types.filter( + pk=device.device_type.pk + ).exists(): + self.logger.debug( + "WebAuthn device type not allowed", device=device, type=device.device_type + ) + continue # Ensure only one challenge per device class # WebAuthn does another device loop to find all WebAuthn devices if device_class in seen_classes: @@ -251,7 +267,7 @@ def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: # noqa: P return self.executor.stage_ok() if stage.not_configured_action == NotConfiguredAction.DENY: self.logger.debug("Authenticator not configured, denying") - return self.executor.stage_invalid() + return self.executor.stage_invalid(_("No MFA authenticator configured.")) if stage.not_configured_action == NotConfiguredAction.CONFIGURE: self.logger.debug("Authenticator not configured, forcing configure") return self.prepare_stages(user) diff --git a/blueprints/schema.json b/blueprints/schema.json index 757f71cd64b1..c6fc3024d4d9 100644 --- a/blueprints/schema.json +++ b/blueprints/schema.json @@ -5637,6 +5637,14 @@ ], "title": "Webauthn user verification", "description": "Enforce user verification for WebAuthn devices." + }, + "webauthn_allowed_device_types": { + "type": "array", + "items": { + "type": "string", + "format": "uuid" + }, + "title": "Webauthn allowed device types" } }, "required": [] diff --git a/schema.yml b/schema.yml index 27e3a56977db..fb5240cbdb22 100644 --- a/schema.yml +++ b/schema.yml @@ -30619,6 +30619,16 @@ components: allOf: - $ref: '#/components/schemas/UserVerificationEnum' description: Enforce user verification for WebAuthn devices. + webauthn_allowed_device_types: + type: array + items: + type: string + format: uuid + webauthn_allowed_device_types_obj: + type: array + items: + $ref: '#/components/schemas/WebAuthnDeviceType' + readOnly: true required: - component - meta_model_name @@ -30626,6 +30636,7 @@ components: - pk - verbose_name - verbose_name_plural + - webauthn_allowed_device_types_obj AuthenticatorValidateStageRequest: type: object description: AuthenticatorValidateStage Serializer @@ -30661,6 +30672,11 @@ components: allOf: - $ref: '#/components/schemas/UserVerificationEnum' description: Enforce user verification for WebAuthn devices. + webauthn_allowed_device_types: + type: array + items: + type: string + format: uuid required: - name AuthenticatorValidationChallenge: @@ -37532,6 +37548,11 @@ components: allOf: - $ref: '#/components/schemas/UserVerificationEnum' description: Enforce user verification for WebAuthn devices. + webauthn_allowed_device_types: + type: array + items: + type: string + format: uuid PatchedAuthenticatorWebAuthnStageRequest: type: object description: AuthenticatorWebAuthnStage Serializer diff --git a/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts b/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts index 1642ffd785a5..ff8345e566a3 100644 --- a/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts +++ b/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts @@ -1,5 +1,9 @@ import { BaseStageForm } from "@goauthentik/admin/stages/BaseStageForm"; +import { deviceTypeRestrictionPair } from "@goauthentik/admin/stages/authenticator_webauthn/utils"; import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; +import "@goauthentik/elements/Alert"; +import "@goauthentik/elements/ak-dual-select/ak-dual-select-provider"; +import { DataProvision } from "@goauthentik/elements/ak-dual-select/types"; import "@goauthentik/elements/forms/FormGroup"; import "@goauthentik/elements/forms/HorizontalFormElement"; import "@goauthentik/elements/forms/Radio"; @@ -71,7 +75,8 @@ export class AuthenticatorValidateStageForm extends BaseStageForm + return html` + ${msg( "Stage used to validate any authenticator. This stage should be used during authentication or authorization flows.", )} @@ -166,33 +171,6 @@ export class AuthenticatorValidateStageForm extends BaseStageForm - - - - ${this.showConfigurationStages ? html` - `; + + + ${msg("WebAuthn-specific settings")} +
+ + + + + + => { + return new StagesApi(DEFAULT_CONFIG) + .stagesAuthenticatorWebauthnDeviceTypesList({ + page: page, + search: search, + }) + .then((results) => { + return { + pagination: results.pagination, + options: results.results.map(deviceTypeRestrictionPair), + }; + }); + }} + .selected=${(this.instance?.webauthnAllowedDeviceTypesObj ?? []).map( + deviceTypeRestrictionPair, + )} + available-label="${msg("Available Device types")}" + selected-label="${msg("Selected Device types")}" + > +

+ ${msg( + "Optionally restrict which WebAuthn device types may be used. When no device types are selected, all devices are allowed.", + )} +

+ + ${ + /* TODO: Remove this after 2024.6..or maybe later? */ + msg( + "This restriction only applies to devices created in authentik 2024.4 or later.", + ) + } + + +
+ + `; } } diff --git a/web/src/admin/stages/authenticator_webauthn/AuthenticatorWebAuthnStageForm.ts b/web/src/admin/stages/authenticator_webauthn/AuthenticatorWebAuthnStageForm.ts index 6bcd69504349..cff867b12ac2 100644 --- a/web/src/admin/stages/authenticator_webauthn/AuthenticatorWebAuthnStageForm.ts +++ b/web/src/admin/stages/authenticator_webauthn/AuthenticatorWebAuthnStageForm.ts @@ -1,9 +1,7 @@ import { RenderFlowOption } from "@goauthentik/admin/flows/utils"; import { BaseStageForm } from "@goauthentik/admin/stages/BaseStageForm"; -import { - DataProvision, - DualSelectPair, -} from "@goauthentik/authentik/elements/ak-dual-select/types"; +import { deviceTypeRestrictionPair } from "@goauthentik/admin/stages/authenticator_webauthn/utils"; +import { DataProvision } from "@goauthentik/authentik/elements/ak-dual-select/types"; import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; import { first } from "@goauthentik/common/utils"; import "@goauthentik/elements/ak-dual-select/ak-dual-select-provider"; @@ -25,23 +23,12 @@ import { ResidentKeyRequirementEnum, StagesApi, UserVerificationEnum, - WebAuthnDeviceType, } from "@goauthentik/api"; @customElement("ak-stage-authenticator-webauthn-form") export class AuthenticatorWebAuthnStageForm extends BaseStageForm { - deviceTypeRestrictionPair(item: WebAuthnDeviceType): DualSelectPair { - const label = item.description ? item.description : item.aaguid; - return [ - item.aaguid, - html`
${label}
-
${item.aaguid}
`, - label, - ]; - } - - loadInstance(pk: string): Promise { - return new StagesApi(DEFAULT_CONFIG).stagesAuthenticatorWebauthnRetrieve({ + async loadInstance(pk: string): Promise { + return await new StagesApi(DEFAULT_CONFIG).stagesAuthenticatorWebauthnRetrieve({ stageUuid: pk, }); } @@ -194,14 +181,12 @@ export class AuthenticatorWebAuthnStageForm extends BaseStageForm { return { pagination: results.pagination, - options: results.results.map( - this.deviceTypeRestrictionPair, - ), + options: results.results.map(deviceTypeRestrictionPair), }; }); }} .selected=${(this.instance?.deviceTypeRestrictionsObj ?? []).map( - this.deviceTypeRestrictionPair, + deviceTypeRestrictionPair, )} available-label="${msg("Available Device types")}" selected-label="${msg("Selected Device types")}" diff --git a/web/src/admin/stages/authenticator_webauthn/utils.ts b/web/src/admin/stages/authenticator_webauthn/utils.ts new file mode 100644 index 000000000000..9743d48aa90c --- /dev/null +++ b/web/src/admin/stages/authenticator_webauthn/utils.ts @@ -0,0 +1,15 @@ +import { DualSelectPair } from "@goauthentik/elements/ak-dual-select/types"; + +import { html } from "lit"; + +import { WebAuthnDeviceType } from "@goauthentik/api"; + +export function deviceTypeRestrictionPair(item: WebAuthnDeviceType): DualSelectPair { + const label = item.description ? item.description : item.aaguid; + return [ + item.aaguid, + html`
${label}
+
${item.aaguid}
`, + label, + ]; +} diff --git a/website/docs/flow/stages/authenticator_validate/index.md b/website/docs/flow/stages/authenticator_validate/index.md index 9a7df4e0e3b9..6620db378b83 100644 --- a/website/docs/flow/stages/authenticator_validate/index.md +++ b/website/docs/flow/stages/authenticator_validate/index.md @@ -72,3 +72,15 @@ Logins which used Passwordless authentication have the _auth_method_ context var } } ``` + +### `WebAuthn Device type restrictions` + +:::info +Requires authentik 2024.4 +::: + +Optionally restrict which WebAuthn device types can be used to authenticate. + +When no restriction is set, all WebAuthn devices a user has registered are allowed. + +These restrictions only apply to WebAuthn devices created with authentik 2024.4 or later. From c8cf1d8ec4180b6b73c3ab2692d9bdb3ccab262c Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 10 Apr 2024 14:57:35 +0200 Subject: [PATCH 2/6] reword Signed-off-by: Jens Langhammer --- .../authenticator_validate/AuthenticatorValidateStageForm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts b/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts index ff8345e566a3..70a359930e93 100644 --- a/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts +++ b/web/src/admin/stages/authenticator_validate/AuthenticatorValidateStageForm.ts @@ -124,7 +124,7 @@ export class AuthenticatorValidateStageForm extends BaseStageForm

${msg( - "If any of the devices user of the types selected above have been used within this duration, this stage will be skipped.", + "If the user has successfully authenticated with a device in the classes listed above within this configured duration, this stage will be skipped.", )}

From bb3238e2e9579df210568cfbcc7c7e84807e7838 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 10 Apr 2024 20:45:59 +0200 Subject: [PATCH 3/6] require enterprise attestation when a device restriction is configured as we need the aaguid Signed-off-by: Jens Langhammer --- authentik/stages/authenticator_webauthn/aaguid.py | 0 authentik/stages/authenticator_webauthn/stage.py | 6 +++++- 2 files changed, 5 insertions(+), 1 deletion(-) delete mode 100644 authentik/stages/authenticator_webauthn/aaguid.py diff --git a/authentik/stages/authenticator_webauthn/aaguid.py b/authentik/stages/authenticator_webauthn/aaguid.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/authentik/stages/authenticator_webauthn/stage.py b/authentik/stages/authenticator_webauthn/stage.py index cda6ba18dd74..6f277e65955c 100644 --- a/authentik/stages/authenticator_webauthn/stage.py +++ b/authentik/stages/authenticator_webauthn/stage.py @@ -126,6 +126,10 @@ def get_challenge(self, *args, **kwargs) -> Challenge: if authenticator_attachment: authenticator_attachment = AuthenticatorAttachment(str(authenticator_attachment)) + attestation = AttestationConveyancePreference.DIRECT + if stage.device_type_restrictions.exists(): + attestation = AttestationConveyancePreference.ENTERPRISE + registration_options: PublicKeyCredentialCreationOptions = generate_registration_options( rp_id=get_rp_id(self.request), rp_name=self.request.brand.branding_title, @@ -137,7 +141,7 @@ def get_challenge(self, *args, **kwargs) -> Challenge: user_verification=UserVerificationRequirement(str(stage.user_verification)), authenticator_attachment=authenticator_attachment, ), - attestation=AttestationConveyancePreference.DIRECT, + attestation=attestation, ) self.request.session[SESSION_KEY_WEBAUTHN_CHALLENGE] = registration_options.challenge From 6b5b9990e1a7b6a0159af384bcfaca3a17878224 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 10 Apr 2024 20:46:31 +0200 Subject: [PATCH 4/6] add tests Signed-off-by: Jens Langhammer --- .../tests/test_webauthn.py | 128 +++++++++++++++--- 1 file changed, 110 insertions(+), 18 deletions(-) diff --git a/authentik/stages/authenticator_validate/tests/test_webauthn.py b/authentik/stages/authenticator_validate/tests/test_webauthn.py index 86b1242f05b4..01cd84c2dfb6 100644 --- a/authentik/stages/authenticator_validate/tests/test_webauthn.py +++ b/authentik/stages/authenticator_validate/tests/test_webauthn.py @@ -26,8 +26,16 @@ PLAN_CONTEXT_DEVICE_CHALLENGES, AuthenticatorValidateStageView, ) -from authentik.stages.authenticator_webauthn.models import UserVerification, WebAuthnDevice +from authentik.stages.authenticator_webauthn.models import ( + UserVerification, + WebAuthnDevice, + WebAuthnDeviceType, +) from authentik.stages.authenticator_webauthn.stage import SESSION_KEY_WEBAUTHN_CHALLENGE +from authentik.stages.authenticator_webauthn.tasks import ( + webauthn_aaguid_import, + webauthn_mds_import, +) from authentik.stages.identification.models import IdentificationStage, UserFields from authentik.stages.user_login.models import UserLoginStage @@ -190,17 +198,21 @@ def test_get_challenge_userless(self): }, ) - def test_validate_challenge(self): + def test_validate_challenge_unrestricted(self): """Test webauthn""" + webauthn_mds_import(force=True) + webauthn_aaguid_import() device = WebAuthnDevice.objects.create( user=self.user, public_key=( - "pQECAyYgASFYIGsBLkklToCQkT7qJT_bJYN1sEc1oJdbnmoOc43i0J" - "H6IlggLTXytuhzFVYYAK4PQNj8_coGrbbzSfUxdiPAcZTQCyU" + "pQECAyYgASFYIF-N4GvQJdTJMAmTOxFX9_boL00zBiSrP0DY9xvJl_FFIlggnyZloVSVofdJNTLMeMdjQHgW2Rzmd5_Xt5AWtNztcdo" ), - credential_id="QKZ97ASJAOIDyipAs6mKUxDUZgDrWrbAsUb5leL7-oU", - sign_count=4, + credential_id="X43ga9Al1MkwCZM7EXD1r8Sxj7aXnNsuR013XM7he4kZ-GS9TaA-u3i36wsswjPm", + sign_count=2, rp_id=generate_id(), + device_type=WebAuthnDeviceType.objects.get( + aaguid="2fc0579f-8113-47ea-b116-bb5a8db9202a" + ), ) flow = create_test_flow() stage = AuthenticatorValidateStage.objects.create( @@ -222,7 +234,7 @@ def test_validate_challenge(self): ] session[SESSION_KEY_PLAN] = plan session[SESSION_KEY_WEBAUTHN_CHALLENGE] = base64url_to_bytes( - "g98I51mQvZXo5lxLfhrD2zfolhZbLRyCgqkkYap1jwSaJ13BguoJWCF9_Lg3AgO4Wh-Bqa556JE20oKsYbl6RA" + "aCC6ak_DP45xMH1qyxzUM5iC2xc4QthQb09v7m4qDBmY8FvWvhxFzSuFlDYQmclrh5fWS5q0TPxgJGF4vimcFQ" ) session.save() @@ -230,24 +242,23 @@ def test_validate_challenge(self): reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), data={ "webauthn": { - "id": "QKZ97ASJAOIDyipAs6mKUxDUZgDrWrbAsUb5leL7-oU", - "rawId": "QKZ97ASJAOIDyipAs6mKUxDUZgDrWrbAsUb5leL7-oU", + "id": "X43ga9Al1MkwCZM7EXD1r8Sxj7aXnNsuR013XM7he4kZ-GS9TaA-u3i36wsswjPm", + "rawId": "X43ga9Al1MkwCZM7EXD1r8Sxj7aXnNsuR013XM7he4kZ-GS9TaA-u3i36wsswjPm", "type": "public-key", "assertionClientExtensions": "{}", "response": { "clientDataJSON": ( - "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiZzk4STUxbVF2WlhvN" - "Wx4TGZockQyemZvbGhaYkxSeUNncWtrWWFwMWp3U2FKMTNCZ3VvSldDRjlfTGczQW" - "dPNFdoLUJxYTU1NkpFMjBvS3NZYmw2UkEiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWx" - "ob3N0OjkwMDAiLCJjcm9zc09yaWdpbiI6ZmFsc2UsIm90aGVyX2tleXNfY2FuX2Jl" - "X2FkZGVkX2hlcmUiOiJkbyBub3QgY29tcGFyZSBjbGllbnREYXRhSlNPTiBhZ2Fpb" - "nN0IGEgdGVtcGxhdGUuIFNlZSBodHRwczovL2dvby5nbC95YWJQZXgifQ==" + "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiYUNDN" + "mFrX0RQNDV4TUgxcXl4elVNNWlDMnhjNFF0aFFiMDl2N200cURCbV" + "k4RnZXdmh4RnpTdUZsRFlRbWNscmg1ZldTNXEwVFB4Z0pHRjR2aW1" + "jRlEiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjkwMDAiLCJj" + "cm9zc09yaWdpbiI6ZmFsc2V9" ), "signature": ( - "MEQCIFNlrHf9ablJAalXLWkrqvHB8oIu8kwvRpH3X3rbJVpI" - "AiAqtOK6mIZPk62kZN0OzFsHfuvu_RlOl7zlqSNzDdz_Ag==" + "MEQCIAHQCGfE_PX1z6mBDaXUNqK_NrllhXylNOmETUD3Khv9AiBTl" + "rX3GDRj5OaOfTToOwUwAhtd74tu0T6DZAVHPb_hlQ==" ), - "authenticatorData": "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAABQ==", + "authenticatorData": "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAABg==", "userHandle": None, }, }, @@ -261,6 +272,87 @@ def test_validate_challenge(self): ) self.assertStageRedirects(response, reverse("authentik_core:root-redirect")) + def test_validate_challenge_restricted(self): + """Test webauthn""" + webauthn_mds_import(force=True) + webauthn_aaguid_import() + device = WebAuthnDevice.objects.create( + user=self.user, + public_key=( + "pQECAyYgASFYIF-N4GvQJdTJMAmTOxFX9_boL00zBiSrP0DY9xvJl_FFIlggnyZloVSVofdJNTLMeMdjQHgW2Rzmd5_Xt5AWtNztcdo" + ), + credential_id="X43ga9Al1MkwCZM7EXD1r8Sxj7aXnNsuR013XM7he4kZ-GS9TaA-u3i36wsswjPm", + sign_count=2, + rp_id=generate_id(), + device_type=WebAuthnDeviceType.objects.get( + aaguid="2fc0579f-8113-47ea-b116-bb5a8db9202a" + ), + ) + flow = create_test_flow() + stage = AuthenticatorValidateStage.objects.create( + name=generate_id(), + not_configured_action=NotConfiguredAction.CONFIGURE, + device_classes=[DeviceClasses.WEBAUTHN], + ) + stage.webauthn_allowed_device_types.set( + WebAuthnDeviceType.objects.filter( + description="Android Authenticator with SafetyNet Attestation" + ) + ) + session = self.client.session + plan = FlowPlan(flow_pk=flow.pk.hex) + plan.append_stage(stage) + plan.append_stage(UserLoginStage(name=generate_id())) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + plan.context[PLAN_CONTEXT_DEVICE_CHALLENGES] = [ + { + "device_class": device.__class__.__name__.lower().replace("device", ""), + "device_uid": device.pk, + "challenge": {}, + } + ] + session[SESSION_KEY_PLAN] = plan + session[SESSION_KEY_WEBAUTHN_CHALLENGE] = base64url_to_bytes( + "aCC6ak_DP45xMH1qyxzUM5iC2xc4QthQb09v7m4qDBmY8FvWvhxFzSuFlDYQmclrh5fWS5q0TPxgJGF4vimcFQ" + ) + session.save() + + response = self.client.post( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + data={ + "webauthn": { + "id": "X43ga9Al1MkwCZM7EXD1r8Sxj7aXnNsuR013XM7he4kZ-GS9TaA-u3i36wsswjPm", + "rawId": "X43ga9Al1MkwCZM7EXD1r8Sxj7aXnNsuR013XM7he4kZ-GS9TaA-u3i36wsswjPm", + "type": "public-key", + "assertionClientExtensions": "{}", + "response": { + "clientDataJSON": ( + "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiYUNDN" + "mFrX0RQNDV4TUgxcXl4elVNNWlDMnhjNFF0aFFiMDl2N200cURCbV" + "k4RnZXdmh4RnpTdUZsRFlRbWNscmg1ZldTNXEwVFB4Z0pHRjR2aW1" + "jRlEiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjkwMDAiLCJj" + "cm9zc09yaWdpbiI6ZmFsc2V9", + ), + "signature": ( + "MEQCIAHQCGfE_PX1z6mBDaXUNqK_NrllhXylNOmETUD3Khv9AiBTl" + "rX3GDRj5OaOfTToOwUwAhtd74tu0T6DZAVHPb_hlQ==", + ), + "authenticatorData": "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAABg==", + "userHandle": None, + }, + } + }, + SERVER_NAME="localhost", + SERVER_PORT="9000", + ) + self.assertEqual(response.status_code, 200) + self.assertStageResponse( + response, + flow, + component="ak-stage-authenticator-validate", + response_errors={"webauthn": [{"string": "Invalid device", "code": "invalid"}]}, + ) + def test_validate_challenge_userless(self): """Test webauthn""" device = WebAuthnDevice.objects.create( From 58b6cc16f6f07c7f2bad98e761a6cd14743d0a41 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 10 Apr 2024 20:55:44 +0200 Subject: [PATCH 5/6] improve error message Signed-off-by: Jens Langhammer --- .../authenticator_validate/challenge.py | 24 ++++++++++++------- .../tests/test_webauthn.py | 15 +++++++++--- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/authentik/stages/authenticator_validate/challenge.py b/authentik/stages/authenticator_validate/challenge.py index 0c6fb5d604f6..c11439684fef 100644 --- a/authentik/stages/authenticator_validate/challenge.py +++ b/authentik/stages/authenticator_validate/challenge.py @@ -135,12 +135,19 @@ def validate_challenge_webauthn(data: dict, stage_view: StageView, user: User) - stage: AuthenticatorValidateStage = stage_view.executor.current_stage try: credential = parse_authentication_credential_json(data) - except InvalidJSONStructure: + except InvalidJSONStructure as exc: + LOGGER.warning("Invalid WebAuthn challenge response", exc=exc) raise ValidationError("Invalid device", "invalid") from None device = WebAuthnDevice.objects.filter(credential_id=credential.id).first() if not device: raise ValidationError("Invalid device", "invalid") + # We can only check the device's user if the user we're given isn't anonymous + # as this validation is also used for password-less login where webauthn is the very first + # step done by a user. Only if this validation happens at a later stage we can check + # that the device belongs to the user + if not user.is_anonymous and device.user != user: + raise ValidationError("Invalid device", "invalid") # When a device_type was set when creating the device (2024.4+), and we have a limitation, # make sure the device type is allowed. if ( @@ -148,13 +155,14 @@ def validate_challenge_webauthn(data: dict, stage_view: StageView, user: User) - and stage.webauthn_allowed_device_types.exists() and not stage.webauthn_allowed_device_types.filter(pk=device.device_type.pk).exists() ): - raise ValidationError("Invalid device", "invalid") - # We can only check the device's user if the user we're given isn't anonymous - # as this validation is also used for password-less login where webauthn is the very first - # step done by a user. Only if this validation happens at a later stage we can check - # that the device belongs to the user - if not user.is_anonymous and device.user != user: - raise ValidationError("Invalid device", "invalid") + raise ValidationError( + _( + "Invalid device type. Contact your {brand} administrator for help.".format( + brand=stage_view.request.brand.branding_title + ) + ), + "invalid", + ) try: authentication_verification = verify_authentication_response( credential=credential, diff --git a/authentik/stages/authenticator_validate/tests/test_webauthn.py b/authentik/stages/authenticator_validate/tests/test_webauthn.py index 01cd84c2dfb6..96a3f81af167 100644 --- a/authentik/stages/authenticator_validate/tests/test_webauthn.py +++ b/authentik/stages/authenticator_validate/tests/test_webauthn.py @@ -331,11 +331,11 @@ def test_validate_challenge_restricted(self): "mFrX0RQNDV4TUgxcXl4elVNNWlDMnhjNFF0aFFiMDl2N200cURCbV" "k4RnZXdmh4RnpTdUZsRFlRbWNscmg1ZldTNXEwVFB4Z0pHRjR2aW1" "jRlEiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjkwMDAiLCJj" - "cm9zc09yaWdpbiI6ZmFsc2V9", + "cm9zc09yaWdpbiI6ZmFsc2V9" ), "signature": ( "MEQCIAHQCGfE_PX1z6mBDaXUNqK_NrllhXylNOmETUD3Khv9AiBTl" - "rX3GDRj5OaOfTToOwUwAhtd74tu0T6DZAVHPb_hlQ==", + "rX3GDRj5OaOfTToOwUwAhtd74tu0T6DZAVHPb_hlQ==" ), "authenticatorData": "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAABg==", "userHandle": None, @@ -350,7 +350,16 @@ def test_validate_challenge_restricted(self): response, flow, component="ak-stage-authenticator-validate", - response_errors={"webauthn": [{"string": "Invalid device", "code": "invalid"}]}, + response_errors={ + "webauthn": [ + { + "string": ( + "Invalid device type. Contact your authentik administrator for help." + ), + "code": "invalid", + } + ] + }, ) def test_validate_challenge_userless(self): From 3e6467b8eb4136c8458406cd77245d3215397b85 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 11 Apr 2024 00:21:44 +0200 Subject: [PATCH 6/6] add more tests Signed-off-by: Jens Langhammer --- .../stages/authenticator_validate/stage.py | 2 +- .../tests/test_webauthn.py | 55 ++++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/authentik/stages/authenticator_validate/stage.py b/authentik/stages/authenticator_validate/stage.py index 2edb0525a65a..c03ab9525a34 100644 --- a/authentik/stages/authenticator_validate/stage.py +++ b/authentik/stages/authenticator_validate/stage.py @@ -267,7 +267,7 @@ def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: # noqa: P return self.executor.stage_ok() if stage.not_configured_action == NotConfiguredAction.DENY: self.logger.debug("Authenticator not configured, denying") - return self.executor.stage_invalid(_("No MFA authenticator configured.")) + return self.executor.stage_invalid(_("No (allowed) MFA authenticator configured.")) if stage.not_configured_action == NotConfiguredAction.CONFIGURE: self.logger.debug("Authenticator not configured, forcing configure") return self.prepare_stages(user) diff --git a/authentik/stages/authenticator_validate/tests/test_webauthn.py b/authentik/stages/authenticator_validate/tests/test_webauthn.py index 96a3f81af167..c66ac0c20d9e 100644 --- a/authentik/stages/authenticator_validate/tests/test_webauthn.py +++ b/authentik/stages/authenticator_validate/tests/test_webauthn.py @@ -128,7 +128,56 @@ def test_device_challenge_webauthn(self): {}, StageView(FlowExecutorView(current_stage=stage), request=request), self.user ) - def test_get_challenge(self): + def test_device_challenge_webauthn_restricted(self): + """Test webauthn (getting device challenges with a webauthn + device that is not allowed due to aaguid restrictions)""" + webauthn_mds_import(force=True) + webauthn_aaguid_import() + request = get_request("/") + request.user = self.user + + WebAuthnDevice.objects.create( + user=self.user, + public_key=bytes_to_base64url(b"qwerqwerqre"), + credential_id=bytes_to_base64url(b"foobarbaz"), + sign_count=0, + rp_id=generate_id(), + device_type=WebAuthnDeviceType.objects.get( + aaguid="2fc0579f-8113-47ea-b116-bb5a8db9202a" + ), + ) + flow = create_test_flow() + stage = AuthenticatorValidateStage.objects.create( + name=generate_id(), + last_auth_threshold="milliseconds=0", + not_configured_action=NotConfiguredAction.DENY, + device_classes=[DeviceClasses.WEBAUTHN], + webauthn_user_verification=UserVerification.PREFERRED, + ) + stage.webauthn_allowed_device_types.set( + WebAuthnDeviceType.objects.filter( + description="Android Authenticator with SafetyNet Attestation" + ) + ) + session = self.client.session + plan = FlowPlan(flow_pk=flow.pk.hex) + plan.append_stage(stage) + plan.append_stage(UserLoginStage(name=generate_id())) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + session[SESSION_KEY_PLAN] = plan + session.save() + + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + ) + self.assertStageResponse( + response, + flow, + component="ak-stage-access-denied", + error_message="No (allowed) MFA authenticator configured.", + ) + + def test_raw_get_challenge(self): """Test webauthn""" request = get_request("/") request.user = self.user @@ -199,7 +248,7 @@ def test_get_challenge_userless(self): ) def test_validate_challenge_unrestricted(self): - """Test webauthn""" + """Test webauthn authentication (unrestricted webauthn device)""" webauthn_mds_import(force=True) webauthn_aaguid_import() device = WebAuthnDevice.objects.create( @@ -273,7 +322,7 @@ def test_validate_challenge_unrestricted(self): self.assertStageRedirects(response, reverse("authentik_core:root-redirect")) def test_validate_challenge_restricted(self): - """Test webauthn""" + """Test webauthn authentication (restricted device type, failure)""" webauthn_mds_import(force=True) webauthn_aaguid_import() device = WebAuthnDevice.objects.create(