john-bodley commented on PR #27858: URL: https://github.com/apache/superset/pull/27858#issuecomment-2033268240
@mistercrunch that was the other option I considered and had discussed internally, i.e., the error being somewhat context aware. Rather than `SQLGlot`'s error being wrapped in a `SupersetSecurityException` it would simply bubble up and be handled in a context aware case-by-case basis. Thus in the context of SQL Lab—which doesn't leverage a cache and where institutions _may_ have a third party security manager, e.g. Apache Ranger—then allowing the `SqlglotError` error to pass through to the engine (irrespective of if the user has access to the underlying tables/views) is likely rather safe—especially given that the engine's parser will most likely detect the error. The challenge arrises when: 1. There is no underlying authorization checks at the engine level. 2. `SQLGlot` returns a false positive ([example](https://github.com/tobymao/sqlglot/issues/3247)) which can be exploited by an actor if (1) is true. Furthermore the current implement of `raise_for_access()` isn't context aware. Given the enumerated challenges it might be best for individual institutions to augment their security manager as they see fit. For example at Airbnb if we feel that either, 1. `SQLGlot` has a non-acceptable false positive rate ,or 2. `SQLGlot`'s error messaging is too terse (when compared with the underlying engine) then we may augment our security manager to simply not raise when the `raise_for_access()` method results in a `SqlglotError` when invoked from SQL Lab (leveraging either the referrer or call stack) as we have the peace of mind that Apache Ranger will prevent unauthorized access. -- 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: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
