fix: guard user dict access when auth dependency returns True#743
Merged
Conversation
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.
Same isinstance(user, dict) fix applied to observation_model_patcher to prevent TypeError when the auth dependency yields True in local dev.
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.
6 tasks
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.
jeremyzilar
requested review from
TylerAdamMartinez,
chasetmartin,
ksmuczynski and
likithabommasani21
jeremyzilar
changed the title
chasetmartin
approved these changes
Jun 30, 2026
chasetmartin
left a comment
chasetmartin
left a comment
Collaborator
There was a problem hiding this comment.
This looks ok to me, just two spots in the crud_helper.py file where the same fix could be useful.
Collaborator
There was a problem hiding this comment.
Probably worth updating the other if user instances in this file.
Same fix as model_patcher: the remaining `if user:` and `if user and resource_type:` checks in model_adder and model_deleter also pass for a boolean True, which would cause a TypeError if notify_edit_event were ever reached. Consistent with the fix already applied to model_patcher and observation_model_patcher.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
crud_helper.pyandobservation_helper.pywhereuser["sub"]was called on a booleanTruevalue, raising aTypeErrorBackground
When running locally with
AUTHENTIK_DISABLE_AUTHENTICATION=1, the auth dependency returnsTrueinstead of a claims dict like{"name": ..., "sub": ...}. The existingif user:guard passed forTrue, and the code then tried to subscript it (user["sub"]), which raised aTypeError.Because this error happened after FastAPI had already started building the response, it bypassed the exception handlers entirely and Uvicorn returned a raw 500 with no CORS headers. This meant the frontend saw a CORS error rather than a real API error, making it very hard to debug.
Changes
services/crud_helper.py: Changedif user:toif isinstance(user, dict):inmodel_adder,model_patcher, and thenotify_edit_eventblockservices/observation_helper.py: Same fix inobservation_model_patchertests/test_contact.py: Regression test that overrides the auth dependency withTrue(matching what local dev does) and confirmsPATCH /contact/:idreturns 200Test plan
pytest tests/test_contact.pyand confirm the new test passes