fix(admin): restore the admin_auth import dropped in #332 - #338
Merged
Conversation
#332 removed the shadowed /identity/register-batch route. The deletion boundary searched for the next line starting with `class `, `@app.` or `def `, and the section banner plus this import sit before `class AdminLoginRequest`, so they went with it: from app.admin_auth import ( verify_password, create_session, verify_session, invalidate_session, ADMIN_USERS, ) `import app.main` still succeeded — a missing name only raises when the line using it runs. So the suite stayed green while every admin route failed at call time with NameError: name 'verify_session' is not defined. Affected: /admin/login (ADMIN_USERS, verify_password, create_session), /admin/logout (invalidate_session), and the 18 routes behind _get_admin_session, including the dashboard. The check after that deletion confirmed AdminLoginRequest and _get_admin_session had survived. It did not check what they call. tests/test_admin_auth_wiring.py closes that gap: it exercises the routes rather than the module. /admin/login with an unknown user must answer 401 rather than 500, a known user with a wrong password must reach the password check, /admin/me and /admin/dashboard/overview must answer 401, /admin/logout must not raise, and a direct assertion covers all five names. Verified as a negative control: with the import removed again, all six fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Production fix. Every admin route is currently raising
NameErrorat call time.What happened
#332 removed the shadowed
/identity/register-batchroute. My deletion boundary looked for the next line starting withclass,@app.ordef— and the section banner plus this import sit beforeclass AdminLoginRequest:import app.mainstill succeeded. A missing name only raises when the line using it runs — so 677 tests stayed green while /admin/login, /admin/logout and the 18 routes behind_get_admin_sessionall failed on every call.I checked after that deletion that
AdminLoginRequestand_get_admin_sessionsurvived. I did not check what they call. That was the gap.The test that closes it
tests/test_admin_auth_wiring.pyexercises the routes rather than the module: unknown user → 401 not 500, known user with wrong password reachesverify_password,/admin/meand/admin/dashboard/overview→ 401,/admin/logoutdoes not raise, plus a direct assertion on all five names.Negative control run: with the import removed again, all six fail. The tests catch this class of bug rather than merely passing alongside it.
Tests
6 new passed · full suite 693 passed.
🤖 Generated with Claude Code