madhushreeag commented on PR #44365: URL: https://github.com/apache/superset/pull/44365#issuecomment-6010327735
@sha174n You are correct on all counts - `GET` isn't CSRF-protected (`WTF_CSRF_METHODS` is POST/PUT/PATCH/DELETE), `login_user` is unconditional, and there's no `current_user` check anywhere in the module. On the `SameSite` observation — the two halves of it turn out to be the same deployment case, which is the reason I'd scoped a binding to a follow-up rather than this PR. Where the parent shares a registrable domain with Superset, the required-configuration section keeps the defaults, so `Lax` still applies *and* a `state` cookie binding is feasible. Where the parent is on a genuinely different registrable domain, `SameSite="None"` is required — and a nonce can't help there either, because the parent has no cookie it can set that Superset will read. So the binding is a control with a conditional guarantee, and an optional security control that silently only holds under an unstated assumption seemed worse than documenting the risk. I think it would be better to land it as its own change with that caveat explicit. Filing the issue with the design, the rejected nonce variants and acceptance criteria. On your 1st suggestion, refusing when `current_user.is_authenticated` and the resolved identity differs closes the "existing session silently replaced" case you describe, but not the more likely one here — in this flow the victim usually has *no* Superset session, since that's the premise of the embed, so the attacker's session is planted rather than swapped. I'll add it to this PR unless you'd rather it went with the binding. -- 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]
