codeant-ai-for-open-source[bot] commented on code in PR #36856:
URL: https://github.com/apache/superset/pull/36856#discussion_r3564855626
##########
superset/db_engine_specs/snowflake.py:
##########
@@ -191,6 +242,105 @@ class SnowflakeEngineSpec(PostgresBaseEngineSpec):
),
}
+ # OAuth 2.0 support
+ supports_oauth2 = True
+ oauth2_exception = CustomSnowflakeAuthError
+
+ @classmethod
+ def is_oauth2_enabled(cls) -> bool:
+ """
+ Return whether OAuth2 authentication is enabled.
+ """
+
+ # When alerts or reports connect to the database in the background,
+ # OAuth2 authentication fails; therefore, OAuth2 authentication is
disabled
+ # for background execution.
+ if not has_request_context():
+ return False
+
+ return (
+ cls.supports_oauth2
+ and cls.engine_name in app.config["DATABASE_OAUTH2_CLIENTS"]
+ )
+
+ @classmethod
+ def get_oauth2_config(cls) -> OAuth2ClientConfig | None:
+ """
+ Build the DB engine spec level OAuth2 client config.
+ """
+ if not cls.is_oauth2_enabled():
+ return None
+
+ return super().get_oauth2_config()
+
+ @classmethod
+ def impersonate_user(
+ cls,
+ database: Database,
+ username: str | None,
+ user_token: str | None,
+ url: URL,
+ engine_kwargs: dict[str, Any],
+ ) -> tuple[URL, dict[str, Any]]:
+ """
+ Modify URL and/or engine kwargs to impersonate a different user.
+ """
+ connect_args: dict[str, Any] =
engine_kwargs.setdefault("connect_args", {})
+
+ # When test_connection is executed (i.e., when
validate_default_parameters is
+ # set to True in connect_args), authentication via OAuth is not
performed.
+ if (
+ not connect_args.get("validate_default_parameters", False)
+ and database.is_oauth2_enabled()
+ ):
Review Comment:
**Suggestion:** The OAuth enablement gate here relies on
`database.is_oauth2_enabled()`, which still returns true for database-level
OAuth client config even without a request/user context. In background
executions (alerts/reports), this makes Snowflake switch to OAuth auth without
a user token, causing connection failures instead of the intended non-OAuth
fallback. Add an explicit request/user-context guard in this condition before
enabling OAuth impersonation. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Background Snowflake connections use OAuth without user token.
- ❌ Scheduled exports/commands can fail authenticating to Snowflake.
- ⚠️ Alerts/reports against Snowflake risk intermittent auth failures.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Snowflake database with OAuth2 client info stored in
`encrypted_extra`, so
`Database.get_oauth2_config()` in `superset/models/core.py:23-31` returns a
non-None
config and `Database.is_oauth2_enabled()` at `superset/models/core.py:6-21`
evaluates to
True.
2. Enable user impersonation on that database (`database.impersonate_user =
True`), so
`Database._get_sqla_engine()` in `superset/models/core.py:15-22` will call
`self.db_engine_spec.impersonate_user(...)` when building engines.
3. Execute code that uses `database.get_sqla_engine()` outside a Flask
request context (no
`g.user`), for example via the streaming export command at
`superset/commands/streaming_export/base.py:226` which calls
`merged_database.get_sqla_engine(...)` to run SQL against Snowflake.
4. In `_get_sqla_engine()` (`superset/models/core.py:42-58`), `oauth2_config
=
self.get_oauth2_config()` succeeds but `access_token` is computed only when
`oauth2_config` and `g.user.id` are present, so in this background context
`access_token`
(passed as `user_token`) is None. `impersonate_user()` at
`superset/db_engine_specs/snowflake.py:31-51` then evaluates `if (not
connect_args.get("validate_default_parameters", False) and
database.is_oauth2_enabled())`
true because `database.is_oauth2_enabled()` uses DB-level config without
checking
`has_request_context()`. The method sets `authenticator="oauth"` on the URL
and
`connect_args` but, since `user_token` is None, does not add a `token` query
parameter.
Subsequent engine creation in `_get_sqla_engine()`
(`create_engine(sqlalchemy_url,
**engine_kwargs)` at `superset/models/core.py:105-107`) attempts a Snowflake
OAuth
connection with no token, leading to authentication failures instead of the
intended
non-OAuth fallback for background execution.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f3c92f600fa8483da98764acae98c438&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f3c92f600fa8483da98764acae98c438&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/snowflake.py
**Line:** 292:295
**Comment:**
*Incorrect Condition Logic: The OAuth enablement gate here relies on
`database.is_oauth2_enabled()`, which still returns true for database-level
OAuth client config even without a request/user context. In background
executions (alerts/reports), this makes Snowflake switch to OAuth auth without
a user token, causing connection failures instead of the intended non-OAuth
fallback. Add an explicit request/user-context guard in this condition before
enabling OAuth impersonation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36856&comment_hash=ec8839461aae769d563192d08f20dc110e71249a2671dbfef07c96f5df837730&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36856&comment_hash=ec8839461aae769d563192d08f20dc110e71249a2671dbfef07c96f5df837730&reaction=dislike'>👎</a>
--
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]
