aminghadersohi opened a new pull request, #44710:
URL: https://github.com/apache/superset/pull/44710
### SUMMARY
This PR lets an engine spec fill in OAuth2 client values it can derive
(here, the Databricks OAuth2 endpoints) before `oauth2_client_info` is
validated.
`Database.get_oauth2_config` validates a database's `oauth2_client_info`
with `OAuth2ClientConfigSchema`, which requires `authorization_request_uri` and
`token_request_uri`. That happens before the engine spec is consulted.
The Databricks engine spec documents that these endpoints derive from the
workspace host (`https://<host>/oidc/v1/{authorize,token}`), and
`get_oauth2_authorization_uri` implements that derivation. It is never reached,
though. When either endpoint is omitted:
- `is_oauth2_enabled()` swallows the `ValidationError` and returns `False`,
so the OAuth2 prompt (`check_for_oauth2` / `start_oauth2_dance`) never happens.
- Every connection fails in `Database._get_sqla_engine` with a raw
marshmallow `ValidationError`.
The change:
- `BaseEngineSpec.resolve_oauth2_client_info(database, client_info)` is
added. It is a no-op by default, and `Database.get_oauth2_config` calls it
before validation.
- `DatabricksDynamicBaseEngineSpec` implements it. Each missing or empty
endpoint is derived from the connection host. Explicit values win. A connection
without a host raises `OAuth2Error` instead of disabling OAuth2 silently.
- Other engines are unchanged.
### TESTING INSTRUCTIONS
- `tests/unit_tests/models/core_test.py`:
- `test_get_oauth2_config_databricks_derives_missing_endpoints` covers
three cases: both endpoints omitted, both empty, and an explicit authorize
endpoint kept.
- `test_get_oauth2_config_databricks_without_host_raises`.
- Both fail on `master` (a `ValidationError`, or no error) and pass here.
The existing `get_oauth2_config` tests are unchanged.
- The behaviour was also checked live against a Databricks SQL warehouse,
with a confidential OAuth app and `oauth2_client_info` containing neither
endpoint:
- Before the change, OAuth2 was disabled and connections raised
`ValidationError`.
- After it, the refresh-token exchange used the derived token endpoint,
queries ran as the user, and the re-authorization prompt pointed at the derived
authorize endpoint.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
