Fix TypeError crash when auth dependency returns True in local dev#743
Open
jeremyzilar wants to merge 4 commits into
Open
Fix TypeError crash when auth dependency returns True in local dev#743jeremyzilar wants to merge 4 commits into
jeremyzilar wants to merge 4 commits into
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
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
1 participant
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