codeant-ai-for-open-source[bot] commented on code in PR #44036:
URL: https://github.com/apache/superset/pull/44036#discussion_r3963582181
##########
superset/db_engine_specs/base.py:
##########
@@ -901,6 +901,38 @@ def get_oauth2_config(cls) -> OAuth2ClientConfig | None:
return config
+ @staticmethod
+ def _validate_oauth2_endpoint_host(uri: str) -> None:
+ """
+ Validate an OAuth2 authorization/token endpoint URI before it's used.
+
+ ``config["authorization_request_uri"]``/``config["token_request_uri"]``
+ can come from a database's own ``encrypted_extra.oauth2_client_info``
+ (editable by anyone with ``can_write`` on Database, not just the
+ deployment operator). The authorization URI is handed to the user's
+ browser as a redirect target; the token URI is POSTed to directly by
+ this server, carrying the connection's ``client_secret`` in the
+ request body. Neither is otherwise validated, so an attacker with
+ write access to one database's config could point either at an
+ internal host, exfiltrating the client secret (token URI) or using
+ Superset as an open redirect into the internal network (authorization
+ URI) -- and since the connection is typically shared, this is
+ exercised by every user who goes through that database's OAuth2 flow,
+ not just the one who configured it.
+
+ Operators with a legitimately internal IdP can opt out via
+ ``DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS``.
+ """
+ if app.config["DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS"]:
+ return
Review Comment:
**Suggestion:** When `DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS` is enabled,
non-HTTP schemes such as `file://` bypass validation and reach redirects or
token requests. [security]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=54e6157cb4fc4dffb0d37e7e2da17609&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=54e6157cb4fc4dffb0d37e7e2da17609&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/base.py
**Line:** 926:927
**Comment:**
*Security: When `DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS` is enabled,
non-HTTP schemes such as `file://` bypass validation and reach redirects or
token requests.
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%2F44036&comment_hash=74bd2b7767badde398a41557d4d7ca1040f3fda20866e139aa4b72aab60016ba&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44036&comment_hash=74bd2b7767badde398a41557d4d7ca1040f3fda20866e139aa4b72aab60016ba&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/base.py:
##########
@@ -901,6 +901,38 @@ def get_oauth2_config(cls) -> OAuth2ClientConfig | None:
return config
+ @staticmethod
+ def _validate_oauth2_endpoint_host(uri: str) -> None:
+ """
+ Validate an OAuth2 authorization/token endpoint URI before it's used.
+
+ ``config["authorization_request_uri"]``/``config["token_request_uri"]``
+ can come from a database's own ``encrypted_extra.oauth2_client_info``
+ (editable by anyone with ``can_write`` on Database, not just the
+ deployment operator). The authorization URI is handed to the user's
+ browser as a redirect target; the token URI is POSTed to directly by
+ this server, carrying the connection's ``client_secret`` in the
+ request body. Neither is otherwise validated, so an attacker with
+ write access to one database's config could point either at an
+ internal host, exfiltrating the client secret (token URI) or using
+ Superset as an open redirect into the internal network (authorization
+ URI) -- and since the connection is typically shared, this is
+ exercised by every user who goes through that database's OAuth2 flow,
+ not just the one who configured it.
+
+ Operators with a legitimately internal IdP can opt out via
+ ``DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS``.
+ """
+ if app.config["DATABASE_OAUTH2_ALLOW_INTERNAL_HOSTS"]:
+ return
+
+ parsed = urlparse(uri)
Review Comment:
**Suggestion:** Malformed endpoint syntax such as an unmatched IPv6 bracket
makes `urlparse` raise `ValueError`, producing an uncaught server error during
authorization. [possible bug]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=652a010bbc37439a860c735f29d9d966&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=652a010bbc37439a860c735f29d9d966&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/base.py
**Line:** 929:929
**Comment:**
*Possible Bug: Malformed endpoint syntax such as an unmatched IPv6
bracket makes `urlparse` raise `ValueError`, producing an uncaught server error
during authorization.
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%2F44036&comment_hash=44f15ae42c6a0a7d71074442d50c35e06f219b21ba6b1df3d6ccb5351e097268&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44036&comment_hash=44f15ae42c6a0a7d71074442d50c35e06f219b21ba6b1df3d6ccb5351e097268&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/base.py:
##########
@@ -947,6 +980,7 @@ def get_oauth2_token(
"""
timeout = app.config["DATABASE_OAUTH2_TIMEOUT"].total_seconds()
uri = config["token_request_uri"]
+ cls._validate_oauth2_endpoint_host(uri)
Review Comment:
**Suggestion:** Hostname validation does not prevent DNS rebinding or
307/308 redirects, allowing token requests containing `client_secret` or
`refresh_token` to reach internal targets. [ssrf]
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=844c64875ff24549bc8042a753afd4b6&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=844c64875ff24549bc8042a753afd4b6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/base.py
**Line:** 983:983
**Comment:**
*Ssrf: Hostname validation does not prevent DNS rebinding or 307/308
redirects, allowing token requests containing `client_secret` or
`refresh_token` to reach internal targets.
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%2F44036&comment_hash=08063af51e4ded590e0c3a8828b2f174f82bf75c1c33b6406be4e396d6c632c6&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44036&comment_hash=08063af51e4ded590e0c3a8828b2f174f82bf75c1c33b6406be4e396d6c632c6&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]
