aminghadersohi commented on PR #43786:
URL: https://github.com/apache/superset/pull/43786#issuecomment-5733519437

   Thanks for the thorough pass — the socket-level probing of crafted targets 
and the hook-ordering check are exactly the parts I could not prove to myself 
from reading alone.
   
   Addressed in `7a17771`: the exemption is now keyed on `request.endpoint == 
"health.health"` with your `url_map`-driven test, and the description is 
corrected to the narrower claim (3 metadata queries versus 0 on a probe 
carrying a session cookie, plus the fail-open warning disappearing when the 
metadata database is down) rather than the overbroad "independent of 
authentication metadata". Details in the inline thread.
   
   On `_EXEMPT_ENDPOINTS` in `superset/security/password_change.py:65`: that is 
a real bug and I would rather fix it separately. It lists `"health"` and 
`"healthcheck"`, and since every rule resolves to `health.health`, 
`_is_exempt_endpoint` never matches — so with `ENABLE_FORCE_PASSWORD_CHANGE` a 
flagged user gets the 302 you saw. But it is a different hook with a different 
failure mode and its own test surface, and this PR is one thing. I will open a 
separate PR for it and link it here.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to