From 4d894ab75ada54cf846b37d3178e9ec37056ea81 Mon Sep 17 00:00:00 2001 From: Jeremy Zilar Date: Fri, 26 Jun 2026 11:18:55 -0600 Subject: [PATCH 1/4] Guard user dict access in crud_helper with isinstance check When AUTHENTIK_DISABLE_AUTHENTICATION=1 the auth dependency returns True instead of a claims dict. The previous `if user:` check passed for True, then user["sub"] raised TypeError, which bypassed FastAPI error handling and produced a raw 500 with no CORS headers. Changed to isinstance(user, dict) in model_adder and model_patcher so the audit fields are only written when a real claims dict is present. --- services/crud_helper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/crud_helper.py b/services/crud_helper.py index 49d1c914..2c7f3d41 100644 --- a/services/crud_helper.py +++ b/services/crud_helper.py @@ -43,7 +43,7 @@ def model_adder(session, table, model, user=None, **kwargs): if kwargs: md.update(kwargs) - if user: + if isinstance(user, dict): # TODO: see note in "AuditMixin" md["created_by_id"] = user["sub"] md["created_by_name"] = user["name"] @@ -115,14 +115,14 @@ def model_patcher( else: setattr(item, key, value) - if user: + if isinstance(user, dict): item.updated_by_id = user["sub"] item.updated_by_name = user["name"] session.commit() session.refresh(item) - if user: + if isinstance(user, dict): resource_type = _resource_type_for_item(model, item) if resource_type: label = _resource_label(item) From bd791f9f87394ef40b1e3e5cf6b2cc3b3d4c7896 Mon Sep 17 00:00:00 2001 From: Jeremy Zilar Date: Fri, 26 Jun 2026 11:19:02 -0600 Subject: [PATCH 2/4] Guard user dict access in observation_helper with isinstance check Same isinstance(user, dict) fix applied to observation_model_patcher to prevent TypeError when the auth dependency yields True in local dev. --- services/observation_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/observation_helper.py b/services/observation_helper.py index f99241db..4e1cab5e 100644 --- a/services/observation_helper.py +++ b/services/observation_helper.py @@ -326,7 +326,7 @@ def observation_model_patcher( for key, value in payload.model_dump(exclude_unset=True).items(): setattr(observation, key, value) - if user: + if isinstance(user, dict): observation.updated_by_id = user["sub"] observation.updated_by_name = user["name"] From e167081a026c7b2d079f790cd039b03624729f0b Mon Sep 17 00:00:00 2001 From: Jeremy Zilar Date: Fri, 26 Jun 2026 11:19:09 -0600 Subject: [PATCH 3/4] Add regression test for PATCH with boolean auth dependency Confirms the isinstance guard holds: when the auth dependency returns True (as it does locally with AUTHENTIK_DISABLE_AUTHENTICATION=1), PATCH /contact still returns 200 instead of a raw 500. --- tests/test_contact.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_contact.py b/tests/test_contact.py index 2076168a..c4cb4392 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -1117,3 +1117,29 @@ def test_delete_address_404_not_found(second_address): # assert response.status_code == 404 # data = response.json() # assert data["detail"] == f"ThingContactAssociation with ID {bad_id} not found." + + +# REGRESSION: isinstance(user, dict) guard in model_patcher ==================== +# When AUTHENTIK_DISABLE_AUTHENTICATION=1 the auth dependency returns True +# instead of a claims dict. Before the fix, model_patcher did user["sub"] on +# True, which raised TypeError and produced a 500 with no CORS headers. +# This test overrides the editor dependency with the boolean True (same as the +# live dev environment does) to confirm the patch still returns 200. + + +def test_patch_contact_with_bool_user(contact): + """PATCH returns 200 when the auth dependency yields True (not a dict).""" + app.dependency_overrides[amp_editor_function] = override_authentication( + default=True + ) + try: + payload = {"name": "Bool User Patch"} + response = client.patch(f"/contact/{contact.id}", json=payload) + assert response.status_code == 200 + assert response.json()["name"] == payload["name"] + finally: + # Restore the dict-user override so subsequent tests are unaffected. + app.dependency_overrides[amp_editor_function] = override_authentication( + default={"name": "foobar", "sub": "1234567890"} + ) + cleanup_patch_test(Contact, payload, contact) From 5531e68e9037f77760a06ee4e09aba324fbbdc76 Mon Sep 17 00:00:00 2001 From: Jeremy Zilar Date: Fri, 26 Jun 2026 11:31:01 -0600 Subject: [PATCH 4/4] Pass skip_if_exists=True when running seed from __main__ The Cypress CI runner calls python -m transfers.seed on each run. Without skip_if_exists, seed_all tries to insert contacts even when they already exist from a prior run, hitting the unique constraint on (name, organization) and crashing. The guard was already implemented in seed_all but not used at the call site. --- transfers/seed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transfers/seed.py b/transfers/seed.py index f3cf741b..bbe2c188 100644 --- a/transfers/seed.py +++ b/transfers/seed.py @@ -460,4 +460,4 @@ def seed_all(n: int = 5, skip_if_exists: bool = False): if __name__ == "__main__": - seed_all(5) + seed_all(5, skip_if_exists=True)