Skip to content

Commit 467f723

Browse files
dominic-rdominic-r
committed
core: Add input validation for service account creation
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
1 parent 44c01f8 commit 467f723

File tree

3 files changed

+320
-21
lines changed

3 files changed

+320
-21
lines changed

‎authentik/core/api/users.py‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,21 @@ class UserPasswordSetSerializer(PassiveSerializer):
334334
password = CharField(required=True)
335335

336336

337+
class UserServiceAccountSerializer(PassiveSerializer):
338+
"""Payload to create a service account"""
339+
340+
name = CharField(
341+
required=True,
342+
validators=[UniqueValidator(queryset=User.objects.all().order_by("username"))],
343+
)
344+
create_group = BooleanField(default=False)
345+
expiring = BooleanField(default=True)
346+
expires = DateTimeField(
347+
required=False,
348+
help_text="If not provided, valid for 360 days",
349+
)
350+
351+
337352
class UsersFilter(FilterSet):
338353
"""Filter for users"""
339354

@@ -494,18 +509,7 @@ def _create_recovery_link(self, for_email=False) -> tuple[str, Token]:
494509

495510
@permission_required(None, ["authentik_core.add_user", "authentik_core.add_token"])
496511
@extend_schema(
497-
request=inline_serializer(
498-
"UserServiceAccountSerializer",
499-
{
500-
"name": CharField(required=True),
501-
"create_group": BooleanField(default=False),
502-
"expiring": BooleanField(default=True),
503-
"expires": DateTimeField(
504-
required=False,
505-
help_text="If not provided, valid for 360 days",
506-
),
507-
},
508-
),
512+
request=UserServiceAccountSerializer,
509513
responses={
510514
200: inline_serializer(
511515
"UserServiceAccountResponse",
@@ -527,11 +531,12 @@ def _create_recovery_link(self, for_email=False) -> tuple[str, Token]:
527531
)
528532
def service_account(self, request: Request) -> Response:
529533
"""Create a new user account that is marked as a service account"""
530-
username = request.data.get("name")
531-
create_group = request.data.get("create_group", False)
532-
expiring = request.data.get("expiring", True)
533-
expires = request.data.get("expires", now() + timedelta(days=360))
534+
data = UserServiceAccountSerializer(data=request.data)
535+
data.is_valid(raise_exception=True)
536+
expires = data.validated_data.get("expires", now() + timedelta(days=360))
534537

538+
username = data.validated_data["name"]
539+
expiring = data.validated_data["expiring"]
535540
with atomic():
536541
try:
537542
user: User = User.objects.create(
@@ -549,10 +554,10 @@ def service_account(self, request: Request) -> Response:
549554
"user_uid": user.uid,
550555
"user_pk": user.pk,
551556
}
552-
if create_group and self.request.user.has_perm("authentik_core.add_group"):
553-
group = Group.objects.create(
554-
name=username,
555-
)
557+
if data.validated_data["create_group"] and self.request.user.has_perm(
558+
"authentik_core.add_group"
559+
):
560+
group = Group.objects.create(name=username)
556561
group.users.add(user)
557562
response["group_pk"] = str(group.pk)
558563
token = Token.objects.create(
@@ -565,7 +570,29 @@ def service_account(self, request: Request) -> Response:
565570
response["token"] = token.key
566571
return Response(response)
567572
except IntegrityError as exc:
568-
return Response(data={"non_field_errors": [str(exc)]}, status=400)
573+
error_msg = str(exc).lower()
574+
575+
if "unique" in error_msg:
576+
return Response(
577+
data={
578+
"non_field_errors": [
579+
_("A user/group with these details already exists")
580+
]
581+
},
582+
status=400,
583+
)
584+
else:
585+
LOGGER.warning("Service account creation failed", exc=exc)
586+
return Response(
587+
data={"non_field_errors": [_("Unable to create user")]},
588+
status=400,
589+
)
590+
except (ValueError, TypeError) as exc:
591+
LOGGER.error("Unexpected error during service account creation", exc=exc)
592+
return Response(
593+
data={"non_field_errors": [_("Unknown error occurred")]},
594+
status=500,
595+
)
569596

570597
@extend_schema(responses={200: SessionUserSerializer(many=False)})
571598
@action(

‎authentik/core/tests/test_users_api.py‎

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,3 +469,274 @@ def test_sort_by_date_joined(self):
469469
body = loads(response.content)
470470
self.assertEqual(len(body["results"]), 2)
471471
self.assertEqual(body["results"][0]["pk"], user.pk)
472+
473+
def test_service_account_validation_empty_username(self):
474+
"""Test service account creation with empty/blank username validation"""
475+
self.client.force_login(self.admin)
476+
477+
# Test with empty string
478+
response = self.client.post(
479+
reverse("authentik_api:user-service-account"),
480+
data={
481+
"name": "",
482+
"create_group": True,
483+
},
484+
)
485+
self.assertEqual(response.status_code, 400)
486+
self.assertJSONEqual(
487+
response.content,
488+
{"name": ["This field may not be blank."]},
489+
)
490+
491+
# Test with only whitespace
492+
response = self.client.post(
493+
reverse("authentik_api:user-service-account"),
494+
data={
495+
"name": " ",
496+
"create_group": True,
497+
},
498+
)
499+
self.assertEqual(response.status_code, 400)
500+
self.assertJSONEqual(
501+
response.content,
502+
{"name": ["This field may not be blank."]},
503+
)
504+
505+
# Test with tab and newline characters
506+
response = self.client.post(
507+
reverse("authentik_api:user-service-account"),
508+
data={
509+
"name": "\t\n",
510+
"create_group": True,
511+
},
512+
)
513+
self.assertEqual(response.status_code, 400)
514+
self.assertJSONEqual(
515+
response.content,
516+
{"name": ["This field may not be blank."]},
517+
)
518+
519+
def test_service_account_validation_valid_username(self):
520+
"""Test service account creation with valid username"""
521+
self.client.force_login(self.admin)
522+
523+
# Test with valid username
524+
response = self.client.post(
525+
reverse("authentik_api:user-service-account"),
526+
data={
527+
"name": "valid-service-account",
528+
"create_group": True,
529+
},
530+
)
531+
self.assertEqual(response.status_code, 200)
532+
533+
# Verify response structure
534+
body = loads(response.content)
535+
self.assertIn("username", body)
536+
self.assertIn("user_uid", body)
537+
self.assertIn("user_pk", body)
538+
self.assertIn("group_pk", body) # Should exist since create_group=True
539+
self.assertIn("token", body)
540+
541+
# Verify field types
542+
self.assertEqual(body["username"], "valid-service-account")
543+
self.assertIsInstance(body["user_pk"], int)
544+
self.assertIsInstance(body["user_uid"], str)
545+
self.assertIsInstance(body["token"], str)
546+
self.assertIsInstance(body["group_pk"], str)
547+
548+
def test_service_account_validation_without_group(self):
549+
"""Test service account creation without creating a group"""
550+
self.client.force_login(self.admin)
551+
552+
response = self.client.post(
553+
reverse("authentik_api:user-service-account"),
554+
data={
555+
"name": "no-group-service-account",
556+
"create_group": False,
557+
},
558+
)
559+
self.assertEqual(response.status_code, 200)
560+
561+
body = loads(response.content)
562+
self.assertIn("username", body)
563+
self.assertIn("user_uid", body)
564+
self.assertIn("user_pk", body)
565+
self.assertIn("token", body)
566+
# Should NOT have group_pk when create_group=False
567+
self.assertNotIn("group_pk", body)
568+
569+
def test_service_account_validation_duplicate_username(self):
570+
"""Test service account creation with duplicate username"""
571+
self.client.force_login(self.admin)
572+
573+
# Create first service account
574+
response = self.client.post(
575+
reverse("authentik_api:user-service-account"),
576+
data={
577+
"name": "duplicate-test",
578+
"create_group": True,
579+
},
580+
)
581+
self.assertEqual(response.status_code, 200)
582+
583+
# Attempt to create second with same username
584+
response = self.client.post(
585+
reverse("authentik_api:user-service-account"),
586+
data={
587+
"name": "duplicate-test",
588+
"create_group": True,
589+
},
590+
)
591+
self.assertEqual(response.status_code, 400)
592+
self.assertJSONEqual(
593+
response.content,
594+
{"name": ["This field must be unique."]},
595+
)
596+
597+
def test_service_account_validation_invalid_create_group(self):
598+
"""Test service account creation with invalid create_group field"""
599+
self.client.force_login(self.admin)
600+
601+
# Test with string instead of boolean
602+
response = self.client.post(
603+
reverse("authentik_api:user-service-account"),
604+
data={
605+
"name": "test-sa",
606+
"create_group": "invalid",
607+
},
608+
)
609+
self.assertEqual(response.status_code, 400)
610+
self.assertJSONEqual(
611+
response.content,
612+
{"create_group": ["Must be a valid boolean."]},
613+
)
614+
615+
# Test with number instead of boolean
616+
response = self.client.post(
617+
reverse("authentik_api:user-service-account"),
618+
data={
619+
"name": "test-sa",
620+
"create_group": 123,
621+
},
622+
)
623+
self.assertEqual(response.status_code, 400)
624+
self.assertJSONEqual(
625+
response.content,
626+
{"create_group": ["Must be a valid boolean."]},
627+
)
628+
629+
def test_service_account_validation_invalid_expiring(self):
630+
"""Test service account creation with invalid expiring field"""
631+
self.client.force_login(self.admin)
632+
633+
# Test with string instead of boolean
634+
response = self.client.post(
635+
reverse("authentik_api:user-service-account"),
636+
data={
637+
"name": "test-sa",
638+
"expiring": "invalid",
639+
},
640+
)
641+
self.assertEqual(response.status_code, 400)
642+
self.assertJSONEqual(
643+
response.content,
644+
{"expiring": ["Must be a valid boolean."]},
645+
)
646+
647+
def test_service_account_validation_invalid_expires(self):
648+
"""Test service account creation with invalid expires field"""
649+
self.client.force_login(self.admin)
650+
651+
# Test with invalid datetime string
652+
response = self.client.post(
653+
reverse("authentik_api:user-service-account"),
654+
data={
655+
"name": "test-sa",
656+
"expires": "invalid-datetime",
657+
},
658+
)
659+
self.assertEqual(response.status_code, 400)
660+
self.assertJSONEqual(
661+
response.content,
662+
{
663+
"expires": [
664+
"Datetime has wrong format. Use one of these formats instead: "
665+
"YYYY-MM-DDThh:mm[:ss[.uuuuuu]][+HH:MM|-HH:MM|Z]."
666+
]
667+
},
668+
)
669+
670+
# Test with invalid format
671+
response = self.client.post(
672+
reverse("authentik_api:user-service-account"),
673+
data={
674+
"name": "test-sa",
675+
"expires": "2024-13-45", # Invalid month/day
676+
},
677+
)
678+
self.assertEqual(response.status_code, 400)
679+
self.assertJSONEqual(
680+
response.content,
681+
{
682+
"expires": [
683+
"Datetime has wrong format. Use one of these formats instead: "
684+
"YYYY-MM-DDThh:mm[:ss[.uuuuuu]][+HH:MM|-HH:MM|Z]."
685+
]
686+
},
687+
)
688+
689+
def test_service_account_validation_multiple_errors(self):
690+
"""Test service account creation with multiple validation errors"""
691+
self.client.force_login(self.admin)
692+
693+
response = self.client.post(
694+
reverse("authentik_api:user-service-account"),
695+
data={
696+
"name": "", # Empty username
697+
"create_group": "invalid", # Invalid boolean
698+
"expiring": 123, # Invalid boolean
699+
"expires": "not-a-date", # Invalid datetime
700+
},
701+
)
702+
self.assertEqual(response.status_code, 400)
703+
self.assertJSONEqual(
704+
response.content,
705+
{
706+
"name": ["This field may not be blank."],
707+
"create_group": ["Must be a valid boolean."],
708+
"expiring": ["Must be a valid boolean."],
709+
"expires": [
710+
"Datetime has wrong format. Use one of these formats instead: "
711+
"YYYY-MM-DDThh:mm[:ss[.uuuuuu]][+HH:MM|-HH:MM|Z]."
712+
],
713+
},
714+
)
715+
716+
def test_service_account_validation_user_friendly_duplicate_error(self):
717+
"""Test that duplicate username returns user-friendly error, not database error"""
718+
self.client.force_login(self.admin)
719+
720+
# Create first service account
721+
response = self.client.post(
722+
reverse("authentik_api:user-service-account"),
723+
data={
724+
"name": "duplicate-username-test",
725+
"create_group": True,
726+
},
727+
)
728+
self.assertEqual(response.status_code, 200)
729+
730+
# Attempt to create second with same username
731+
response = self.client.post(
732+
reverse("authentik_api:user-service-account"),
733+
data={
734+
"name": "duplicate-username-test",
735+
"create_group": True,
736+
},
737+
)
738+
self.assertEqual(response.status_code, 400)
739+
self.assertJSONEqual(
740+
response.content,
741+
{"name": ["This field must be unique."]},
742+
)

‎schema.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59209,6 +59209,7 @@ components:
5920959209
- pk
5921059210
UserServiceAccountRequest:
5921159211
type: object
59212+
description: Payload to create a service account
5921259213
properties:
5921359214
name:
5921459215
type: string

0 commit comments

Comments
 (0)