sadpandajoe commented on code in PR #45890:
URL: https://github.com/apache/superset/pull/45890#discussion_r4423994945
##########
superset/models/core.py:
##########
@@ -1607,6 +1759,151 @@ def make_sqla_column_compatible(
sqla_col.key = label_expected
return sqla_col
+ def embedded_fallback_applies(self) -> bool:
+ """
+ Is this request one the embedded fallback credential governs?
+
+ True when every condition *except* the presence of a stored credential
holds:
+
+ 1. the deployment opted in via ``EMBEDDED_CREDENTIAL_FALLBACK``;
+ 2. the engine opted in via ``supports_embedded_credential_fallback``;
+ 3. the current principal is an embedded guest, and embedding is
enabled;
+ 4. the connection uses per-user OAuth2 with impersonation enabled,
rather
+ than an ordinary service-account connection.
+
+ Split out from :meth:`get_embedded_fallback_credentials` so that the
two
+ callers cannot drift: the credential lookup adds the stored-credential
+ condition on top, while ``_get_sqla_engine`` uses this to tell "no
fallback
+ here, carry on" apart from "a guest this connection has no credential
for",
+ which it refuses rather than serving on the base credential.
+ """
+ return self._is_embedded_guest_on_impersonating_connection() and (
+ self._has_embedded_oauth2_client()
+ )
+
+ def _refuse_embedded_guest(self, reason: str) -> NoReturn:
+ """
+ Refuse an embedded guest's query on this connection.
+
+ Only embedded guests reach this, and their error responses are
redacted to
+ a generic message unless the error type is one Superset authors itself
+ (`superset.utils.error_sanitization`). So the viewer gets a fixed
message
+ under an allowlisted type that names nothing about the connection, and
the
+ administrator gets ``reason``, which says how to fix the connection, in
+ the server log.
+ """
+ logger.warning(
+ "Refused an embedded viewer's query on database connection %r (id
%s): %s",
+ self.database_name,
+ self.id,
+ reason,
+ )
+ raise SupersetErrorException(
+ SupersetError(
+ message=__(
+ "This chart can't be shown here because its database
connection "
+ "isn't set up for embedded dashboards. Please contact the "
+ "dashboard's owner."
+ ),
+ error_type=SupersetErrorType.DATABASE_SECURITY_ACCESS_ERROR,
Review Comment:
With `GLOBAL_ASYNC_QUERIES` on, a guest never sees this message: the async
worker stores only `str(exception)` as the task's `error_message`, and the task
status endpoint passes that untyped string through `sanitize_error_message`,
which always replaces it with the generic "An error occurred while fetching the
data." for guests. The allowlisted error type only survives the synchronous
response, and `test_async_embedded_guest_without_credentials_is_refused`
asserts just the raised type, so this path isn't covered. Yet the new docs say
the viewer is told the connection isn't set up for embedded dashboards. Should
the async path carry the type through, or should the docs say async guests
still get the generic message?
--
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]
