aminghadersohi commented on code in PR #44710:
URL: https://github.com/apache/superset/pull/44710#discussion_r4113535088
##########
superset/db_engine_specs/databricks.py:
##########
@@ -320,6 +320,37 @@ def _workspace_oauth2_endpoint(cls, database: Database,
path: str) -> str:
)
return f"https://{host}/oidc/v1/{path}";
+ @classmethod
+ def resolve_oauth2_client_info(
+ cls,
+ database: Database,
+ client_info: dict[str, Any],
+ ) -> dict[str, Any]:
+ """
+ Derive missing OAuth2 endpoints from the workspace host.
+
+ ``authorization_request_uri`` and ``token_request_uri`` are required by
+ ``OAuth2ClientConfigSchema``; without them the database's OAuth2 was
+ disabled (``is_oauth2_enabled`` returned False) and every connection
+ failed with a ValidationError. Each missing or empty endpoint becomes
+ ``https://<workspace-host>/oidc/v1/{authorize,token}``; explicit values
+ win. A connection without a host raises ``OAuth2Error``.
+ """
+ endpoints = {
+ "authorization_request_uri": "authorize",
+ "token_request_uri": "token",
+ }
+ missing = [key for key in endpoints if not client_info.get(key)]
+ if not missing:
+ return client_info
+ return {
+ **client_info,
+ **{
+ key: cls._workspace_oauth2_endpoint(database, endpoints[key])
+ for key in missing
+ },
+ }
Review Comment:
Fixed in d210e8e3de. `UpdateDatabaseCommand._handle_oauth2` now resolves the
incoming `oauth2_client_info` through `resolve_oauth2_client_info` before
comparing. It resolves against the URI being saved, so saving the same config
keeps tokens and changing the workspace host still purges them. Covered by
`test_update_oauth2_derived_endpoints`.
##########
superset/models/core.py:
##########
@@ -1515,6 +1515,11 @@ def get_oauth2_config(self) -> OAuth2ClientConfig | None:
logger.error(ex, exc_info=True)
raise SupersetGenericDBErrorException(message=str(ex)) from ex
if oauth2_client_info := encrypted_extra.get("oauth2_client_info"):
+ # Let the engine spec fill values it can derive (e.g. endpoints
from
+ # the connection host) before the schema requires them.
+ oauth2_client_info =
self.db_engine_spec.resolve_oauth2_client_info(
+ self, oauth2_client_info
+ )
Review Comment:
Fixed in d210e8e3de. The Databricks resolver returns non-dict values
untouched, so `OAuth2ClientConfigSchema` rejects them with a `ValidationError`.
Added `test_get_oauth2_config_databricks_malformed_client_info`.
--
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]
