codeant-ai-for-open-source[bot] commented on code in PR #36856:
URL: https://github.com/apache/superset/pull/36856#discussion_r3478666691
##########
superset/db_engine_specs/snowflake.py:
##########
@@ -191,6 +224,99 @@ 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"]
+ )
Review Comment:
**Suggestion:** The OAuth2 enablement check only looks at
`DATABASE_OAUTH2_CLIENTS`, but Snowflake OAuth2 can also be configured
per-database in encrypted extra. This makes OAuth2 appear configured (token
retrieval path runs) while `impersonate_user` still treats it as disabled, so
OAuth authentication is never applied for database-level client configs. Use a
database-aware check (or pass `database.is_oauth2_enabled()` into this flow)
instead of a class-level global-config-only gate. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Snowflake OAuth per-database config fails to apply.
- ⚠️ User impersonation tokens retrieved but never used.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Snowflake Database with per-database OAuth2 client info in
`encrypted_extra["oauth2_client_info"]` (parsed in
`Database.get_oauth2_config()` at
`superset/models/core.py:33-41`) and do NOT add `"Snowflake"` to
`app.config["DATABASE_OAUTH2_CLIENTS"]`.
2. Observe `Database.is_oauth2_enabled()` at `superset/models/core.py:7-22`
returns True
because `get_oauth2_config()` finds the per-database config, while
`SnowflakeEngineSpec.is_oauth2_enabled()` at
`superset/db_engine_specs/snowflake.py:231-246` returns False (it only checks
`DATABASE_OAUTH2_CLIENTS`).
3. Run an interactive query (SQL Lab/dashboard) so
`Database.get_sqla_engine()` at
`superset/models/core.py:10-88` calls `_get_sqla_engine()`;
`_get_sqla_engine()` at
`superset/models/core.py:89-132` retrieves an OAuth2 access token via
`get_oauth2_access_token()` when a token exists and then calls
`self.db_engine_spec.impersonate_user(...)` at
`superset/models/core.py:136-140`.
4. In `SnowflakeEngineSpec.impersonate_user()` at
`superset/db_engine_specs/snowflake.py:259-293`, both the authenticator
switch and token
injection are gated on `cls.is_oauth2_enabled()`, so for databases
configured only via
`encrypted_extra` the OAuth authenticator and `token` query parameter are
never applied
even though tokens are present, resulting in connections that continue using
non-OAuth
authentication.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=945734d7fb9846ee99e4af2976d63b3c&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=945734d7fb9846ee99e4af2976d63b3c&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:** 243:246
**Comment:**
*Api Mismatch: The OAuth2 enablement check only looks at
`DATABASE_OAUTH2_CLIENTS`, but Snowflake OAuth2 can also be configured
per-database in encrypted extra. This makes OAuth2 appear configured (token
retrieval path runs) while `impersonate_user` still treats it as disabled, so
OAuth authentication is never applied for database-level client configs. Use a
database-aware check (or pass `database.is_oauth2_enabled()` into this flow)
instead of a class-level global-config-only gate.
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=4cb0ef6f25361dff6b802619b1e58ee6105fb04ec310a9e86de86d04158ecb6f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36856&comment_hash=4cb0ef6f25361dff6b802619b1e58ee6105fb04ec310a9e86de86d04158ecb6f&reaction=dislike'>👎</a>
##########
superset/db_engine_specs/snowflake.py:
##########
@@ -191,6 +224,99 @@ 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 = 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 cls.is_oauth2_enabled()
+ ):
+ url = url.update_query_dict({"authenticator": "oauth"})
+ connect_args["authenticator"] = "oauth"
+
+ if user_token and cls.is_oauth2_enabled():
+ if username is not None:
+ user = security_manager.find_user(username=username)
+ if user and user.email:
+ if is_feature_enabled("IMPERSONATE_WITH_EMAIL_PREFIX"):
+ url = url.set(username=user.email.split("@")[0])
+ else:
+ url = url.set(username=user.email)
+
+ url = url.update_query_dict({"token": user_token})
+
+ return url, engine_kwargs
+
+ @classmethod
+ def get_oauth2_authorization_uri(
+ cls,
+ config: OAuth2ClientConfig,
+ state: OAuth2State,
+ code_verifier: str | None = None, # pylint: disable=unused-argument
+ ) -> str:
+ """
+ Return URI for initial OAuth2 request.
+ """
+ uri = config["authorization_request_uri"]
+ # When calling the Snowflake OAuth authorization endpoint for a custom
client,
+ # specify only the query parameters documented in the URL below.
+ # Adding unsupported parameters
+ # (e.g., `prompt` as used in
BaseEngineSpec.get_oauth2_authorization_uri)
+ # will cause an error.
+ # https://docs.snowflake.com/user-guide/oauth-custom#query-parameters
+ params = {
+ "scope": config["scope"],
+ "response_type": "code",
+ "state": encode_oauth2_state(state),
+ "redirect_uri": config["redirect_uri"],
+ "client_id": config["id"],
+ }
+ return parse.urljoin(uri, "?" + parse.urlencode(params))
Review Comment:
**Suggestion:** The authorization URL override ignores `code_verifier` and
omits PKCE `code_challenge`, but the shared callback flow still forwards
`code_verifier` to token exchange. This creates a PKCE contract mismatch where
token exchange may fail because the authorization request did not include
matching PKCE parameters. Either include PKCE challenge parameters here or
override token exchange to avoid sending `code_verifier` for Snowflake. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Snowflake OAuth flow may fail due to PKCE mismatch.
- ⚠️ Users may be unable to authorize Snowflake access.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start an OAuth2 flow for a Snowflake database (e.g., from SQL Lab) so
`Database.get_sqla_engine()` at `superset/models/core.py:52-61` runs inside
`check_for_oauth2()` (`superset/utils/oauth2.py:11-20`); when a
Snowflake-specific OAuth
error is raised, `database.db_engine_spec.start_oauth2_dance(database)` is
invoked via
`BaseEngineSpec.needs_oauth2()` at
`superset/db_engine_specs/base.py:2195-2202`.
2. `BaseEngineSpec.start_oauth2_dance()` at
`superset/db_engine_specs/base.py:11-83`
generates a PKCE `code_verifier`, stores it in the KV store, builds an
`OAuth2State`, and
then calls `cls.get_oauth2_authorization_uri(oauth2_config, state,
code_verifier=code_verifier)`.
3. For Snowflake, the override
`SnowflakeEngineSpec.get_oauth2_authorization_uri()` at
`superset/db_engine_specs/snowflake.py:295-318` ignores the `code_verifier`
argument
(marked `# pylint: disable=unused-argument`) and builds the authorization
URL without
`code_challenge` or `code_challenge_method`, even though PKCE was initiated
upstream.
4. After the user authorizes, the callback in
`commands/database/oauth2.py:17-39` loads
`code_verifier` from the KV store and passes it to
`BaseEngineSpec.get_oauth2_token()` at
`superset/db_engine_specs/base.py:31-57`, which unconditionally includes
`code_verifier`
in the token request body; this creates a mismatch where the token request
uses PKCE
parameters but the authorization request did not supply a matching
challenge, risking
token exchange failures with Snowflake's OAuth provider.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d1d20cfdee42406c99816f3550194d98&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=d1d20cfdee42406c99816f3550194d98&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:** 295:318
**Comment:**
*Api Mismatch: The authorization URL override ignores `code_verifier`
and omits PKCE `code_challenge`, but the shared callback flow still forwards
`code_verifier` to token exchange. This creates a PKCE contract mismatch where
token exchange may fail because the authorization request did not include
matching PKCE parameters. Either include PKCE challenge parameters here or
override token exchange to avoid sending `code_verifier` for Snowflake.
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=48a24dbfd6c37548ded1940d945e240abd13166417cd325c3f22651da3a296f2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F36856&comment_hash=48a24dbfd6c37548ded1940d945e240abd13166417cd325c3f22651da3a296f2&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]
