fix(openapi): distinguish unset and explicitly-empty route handler security - #5027
fix(openapi): distinguish unset and explicitly-empty route handler security#5027shuanat wants to merge 1 commit into
Conversation
…curity HTTPRouteHandler collapsed "no security configured" and "security=[]" into the same empty tuple, so declaring security=[] on a handler to opt out of a security requirement configured on an ancestor layer (app, router, controller) silently did nothing - the operation still inherited the document-level security in the generated OpenAPI schema. Keep None (nothing set on this layer) distinct from an explicit empty sequence through the handler's own layer of the ownership-layer merge, and check "is not None" instead of truthiness when building the per-operation security field.
|
@provinzkraut I couldn't request you as a reviewer directly (no write access from a fork PR), so tagging you here instead. This is a standalone fix, not the #3013 auto-wiring itself - it just fixes |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5027 +/- ##
=======================================
Coverage 67.19% 67.20%
=======================================
Files 293 293
Lines 15363 15367 +4
Branches 1745 1748 +3
=======================================
+ Hits 10323 10327 +4
Misses 4890 4890
Partials 150 150 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
While looking into #3013 (excluded/public paths still showing up as requiring auth in the generated OpenAPI docs), I ran into a more basic problem underneath it:
security=[]on a route handler is supposed to let you explicitly opt a route out of whatever security is configured above it (app/router/controller), but it doesn't actually work.HTTPRouteHandlerstores security astuple(security) if security else (), so "nothing was passed" and "an empty list was passed" both end up as the same(). By the timePathItemFactorybuilds the operation,if route_handler.security else Nonecollapses that()down toNone, which per the OpenAPI spec means "inherit whateversecurityis set at the document root". So a handler that explicitly declaressecurity=[]still ends up documented as requiring the app-wide security scheme - the escape hatch silently does nothing.This PR keeps
None(nothing configured on this layer) distinct from an explicit empty tuple through the handler's own layer of_get_merge_opts, and checksis not Noneinstead of truthiness inPathItemFactory. I left the ancestor layers (Router/Controller/app) untouched - their.securitystill defaults to[]and merges the same way it always did. A Router explicitly passedsecurity=[]is indistinguishable from one that wasn't passed anything at all, given howRouter.__init__stores it today, so extending this to ancestor layers as well would have meant touchingrouter.pytoo and I wanted to keep this small.I want to be upfront about scope: this does not close #3013 on its own.
exclude/exclude_from_authonAbstractSecurityConfigare still purely a middleware-time thing and don't touchroute_handler.securityat all, so a route excluded that way is still documented as requiring auth. I saw the two previous attempts at fixing this directly (#4755, #4767) were closed for baking security-specific logic into the OpenAPI construction path, and #4757 looks like the right foundation for that follow-up. So I kept this PR to the standalone bug in the merge logic, which was silently broken independently of any of that and is worth fixing on its own.The thing I was most worried about while making this change: could a handler somehow use
security=[]to make a genuinely protected endpoint look unprotected in the docs? Verified it can't - if any ancestor layer declares a non-empty security requirement, that still wins regardless of what the handler does, since ancestor layers only ever contribute when they're non-empty. Added a test for that specifically (test_explicit_empty_route_security_does_not_cancel_ownership_layer_security), on top of the existing layered-security test which still passes unchanged.Test plan
pytest tests/unit/test_openapi/test_security_schemes.py tests/unit/test_handlers/test_http_handlers/test_resolution.py(new + existing tests)pytest tests/unit/test_security tests/unit/test_openapi tests/unit/test_handlers tests/unit/test_middleware/test_session tests/unit/test_static_files/test_create_static_router.py- 686 passed, no regressionsmypy/pyrightclean on the touched files