orbisai0security commented on PR #40026: URL: https://github.com/apache/superset/pull/40026#issuecomment-4701756246
Thanks for the detailed explanation. I want to clarify one point, though: `validate_guest_token_resources` only checks that the referenced resource *exists* (raising `EmbeddedDashboardNotFoundError` if not), it doesn't enforce that the calling user has *access* to that resource. So the authorisation gap is real at the code level. That said, I take your point about the threat model: the endpoint is gated behind `grant_guest_token`, which is intended for a trusted backend service, not end users. In a well-configured deployment, that service would only ever request tokens for dashboards it is legitimately serving. Where this matters is in misconfiguration or multi-tenant scenarios: if `grant_guest_token` is granted to a role that multiple backend services share, one service could (accidentally or maliciously) mint tokens for dashboards belonging to another tenant. The fix I added would catch that. I'll leave it to you whether that's a risk worth guarding against in the codebase itself. If you'd like, I'm happy to reframe this as a hardening/defence-in-depth PR rather than a critical vulnerability fix; the code change is the same, the framing is just more accurate. Happy to reopen and update the description if that's useful. -- 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]
