bito-code-review[bot] commented on code in PR #34619:
URL: https://github.com/apache/superset/pull/34619#discussion_r3463197659
##########
superset/db_engine_specs/databricks.py:
##########
@@ -563,6 +697,69 @@ class
DatabricksPythonConnectorEngineSpec(DatabricksDynamicBaseEngineSpec):
supports_dynamic_schema = supports_catalog = supports_dynamic_catalog =
True
+ # OAuth 2.0 support
+ supports_oauth2 = True
+ oauth2_exception = OAuth2RedirectError
+ oauth2_scope = "sql"
+
+ # OAuth2 endpoints are determined dynamically based on cloud provider
+ oauth2_authorization_request_uri = "" # Set dynamically
+ oauth2_token_request_uri = "" # Set dynamically
+
+ @classmethod
+ def get_oauth2_authorization_uri(
+ cls,
+ config: "OAuth2ClientConfig",
+ state: "OAuth2State",
+ ) -> str:
+ """
+ Return URI for initial OAuth2 request with dynamic endpoint detection.
+ """
+ from superset.models.core import Database
+
+ # Get the database to detect cloud provider
+ database_id = state["database_id"]
+ if database := Database.query.get(database_id):
+ provider = cls._detect_cloud_provider(database)
+ # Update config with the correct authorization URI for the cloud
provider
+ from typing import cast
+
+ config = cast(
+ "OAuth2ClientConfig",
+ dict(config)
+ | {
+ "authorization_request_uri":
cls._oauth2_endpoints[provider][
+ "authorization_request_uri"
+ ]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Unformatted Placeholders in OAuth2 URI</b></div>
<div id="fix">
OAuth2 endpoints contain `{}` placeholders for account/tenant IDs that are
never filled. Azure authorization will request
`https://login.microsoftonline.com//oauth2/v2.0/authorize` (double slash)
because `{}` is passed literally. AWS/GCP will similarly receive malformed
URIs. Extract the appropriate ID from `database.extra` JSON (e.g.,
`aws_account_id`, `azure_tenant_id`) and call `.format(account_id)` on the
endpoint string before use.
([CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
provider = cls._detect_cloud_provider(database)
+ import json
+ extra_config = json.loads(database.extra or "{}")
+
+ # Get the appropriate account/tenant ID for the provider
+ account_id = (
+ extra_config.get("azure_tenant_id") or
+ extra_config.get("aws_account_id") or
+ ""
+ )
+
from typing import cast
config = cast(
"OAuth2ClientConfig",
dict(config)
| {
"authorization_request_uri":
cls._oauth2_endpoints[provider][
- "authorization_request_uri"
- ]
+ "authorization_request_uri"
+ ].format(account_id)
},
)
return super().get_oauth2_authorization_uri(config, state)
```
</div>
</details>
</div>
<small><i>Code Review Run #6cc2a6</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
