aminghadersohi commented on code in PR #44705:
URL: https://github.com/apache/superset/pull/44705#discussion_r4141831064


##########
superset/db_engine_specs/databricks.py:
##########
@@ -447,14 +480,40 @@ def impersonate_user(
         # back to an empty string to force re-authentication when none is set.
         url = url.set(password=user_token or "")
 
+        # Drop shared credentials from ``extra`` (a PAT, OAuth M2M client, 
Azure
+        # service principal) so none of them reaches the connector next to the
+        # user's token. The secure extra is filtered the same way in
+        # ``update_params_from_encrypted_extra``.
+        connect_args = engine_kwargs.get("connect_args") or {}
+        filtered_connect_args = {
+            key: value
+            for key, value in connect_args.items()
+            if key not in DATABRICKS_SHARED_CREDENTIAL_CONNECT_ARGS
+        }
         # The Python connector passes the token via ``connect_args`` instead 
of the
         # URL password, so keep it in sync (clearing it likewise forces 
re-auth).
-        connect_args = engine_kwargs.setdefault("connect_args", {})
         if "access_token" in connect_args:
-            connect_args["access_token"] = user_token or ""
+            filtered_connect_args["access_token"] = user_token or ""
+        engine_kwargs["connect_args"] = filtered_connect_args
 
         return url, engine_kwargs
 
+    @classmethod
+    def start_oauth2_dance(cls, database: Database) -> None:
+        """
+        Start the OAuth2 dance only when the database impersonates the user.
+
+        The user's OAuth2 token only reaches the connection through
+        ``impersonate_user``. Without impersonation the connection uses the
+        shared credential, so an authorization prompt cannot fix an auth 
failure
+        (e.g. a revoked shared token returning HTTP 401). Return instead, so 
the
+        caller raises the original error for an admin to act on.
+        """
+        if not database.impersonate_user:

Review Comment:
   Confirmed: `UpdateDatabaseCommand` catches `MissingOAuth2TokenError` at 
`update.py:128` and skips the sync. I added a "Known limitations (follow-up)" 
section to the PR description covering this path and the 
`ValidateDatabaseParametersCommand` one, so both are tracked for the same 
engine-agnostic follow-up.



-- 
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]

Reply via email to