rusackas commented on PR #40024: URL: https://github.com/apache/superset/pull/40024#issuecomment-4910104883
Thanks @AlbertoSolaro. I went through the code and the gating looks right... `session_token` sits behind `@protect()` and only mints a JWT for the already-authenticated caller's own identity, and `auth_providers` only surfaces what the login page already shows. So I'm not seeing a privilege boundary getting crossed. That said, a net-new unauthenticated discovery endpoint plus a cookie→JWT exchange is exactly the kind of auth surface I don't think should land on a single maintainer's review. Could we get this in front of the dev@ list for lazy consensus, plus a heads-up to the private security@ list since it's authentication? Happy to help kick off that thread if you're not on the list. One correction on my earlier note: CI is actually green now, so ignore the red-CI part... the threat-model question for `session_token` still stands though. And a couple of the bot nits are worth folding in while it's open: returning a stable string `auth_type` rather than the raw FAB int before this becomes public API, and a test for the SAML branch. -- 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]
