stages/user_write: refuse to write id/pk claims onto the user model (#21667)

* stages/user_write: refuse to write id/pk claims onto the user model

When an enrollment or source flow maps IdP-supplied attributes onto the
User model, update_user walks each key and, if the user already has an
attribute by that name, calls setattr(user, key, value) unconditionally.
"id" is always present on the User model (it is the Django PK), so a
SAML assertion that ships an "id" claim, e.g. a hex string from
mocksaml, was written straight into the PK field. Django then rejected
the save:

  ValueError: Field 'id' expected a number but got '<hex>'.

The log surfaced as "Failed to save user" and the enrollment flow
silently failed for every incoming user.

Treat "id" and "pk" the same way the existing "groups" entry is
treated: add them to disallowed_user_attributes so the walker logs and
skips them. IdP attributes can still be stored on user.attributes via
the dotted/underscored forms (e.g. attributes.id), which go through
write_attribute and land in the JSONField safely.

Added a regression test covering both id and pk in the prompt context.

Fixes #21580

Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>

* fix lint

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

---------

Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Co-authored-by: SAY-5 <SAY-5@users.noreply.github.com>
Co-authored-by: Jens Langhammer <jens@goauthentik.io>

authentik/stages/user_write/stage.py

@@ -36,6 +36,14 @@ class UserWriteStageView(StageView):
         super().__init__(executor, **kwargs)
         self.disallowed_user_attributes = [
             "groups",
+            # Block attribute writes that would otherwise land on the model's
+            # primary key. An IdP that returns an `id` claim (mocksaml is one
+            # example) used to crash the enrollment flow with
+            # ValueError: Field 'id' expected a number but got '<hex>'
+            # because hasattr(user, "id") is true and setattr(user, "id", ...)
+            # was taken unchecked. See #21580.
+            "id",
+            "pk",
         ]
 
     @staticmethod

authentik/stages/user_write/tests.py

@@ -315,6 +315,34 @@ class TestUserWriteStage(FlowTestCase):
             component="ak-stage-access-denied",
         )
 
+    def test_user_update_ignores_id_from_idp(self):
+        """IdP-supplied `id`/`pk` attributes must not land on the model
+        primary key and crash user save (#21580)."""
+        existing = User.objects.create(username="unittest", email="test@goauthentik.io")
+        original_pk = existing.pk
+
+        plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()])
+        plan.context[PLAN_CONTEXT_PENDING_USER] = existing
+        plan.context[PLAN_CONTEXT_PROMPT] = {
+            "username": "idp-user",
+            # Hex string from a SAML IdP; would previously crash with
+            # ValueError: Field 'id' expected a number but got '<hex>'.
+            "id": "1dda9fb491dc01bd24d2423ba2f22ae561f56ddf2376b29a11c80281d21201f9",
+            "pk": "also-not-an-int",
+        }
+        session = self.client.session
+        session[SESSION_KEY_PLAN] = plan
+        session.save()
+
+        response = self.client.post(
+            reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug})
+        )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertStageRedirects(response, reverse("authentik_core:root-redirect"))
+        user = User.objects.get(username="idp-user")
+        self.assertEqual(user.pk, original_pk)
+
     def test_write_attribute(self):
         """Test write_attribute"""
         user = create_test_admin_user()

locale/en/dictionaries/idp.txt

@@ -4,3 +4,4 @@ Yubi
 Yubikey
 Yubikeys
 mycorp
+mocksaml