eschutho opened a new pull request, #44145:
URL: https://github.com/apache/superset/pull/44145
### SUMMARY
`encrypted_extra_validator` in `superset/databases/schemas.py` validates
that its input is valid JSON but does not check that the decoded value is a
`dict`. Non-dict JSON values (`123`, `null`, `[1, 2]`, `true`, `"abc"`) pass
schema validation and later cause an `AttributeError` in `_handle_oauth2()`
when `.get()` is called on a non-mapping — surfacing as an opaque 500 instead
of a 422.
This adds an `isinstance(…, dict)` guard to `encrypted_extra_validator`,
mirroring the identical fix applied to `extra_validator` in #44092 (which was
preceded by PRs #42366 and #42401 for the same bug class on other fields).
**Problem:** `PUT /api/v1/database/<pk>/` with `masked_encrypted_extra` set
to e.g. `"123"` passes schema validation, then hits:
```python
encrypted_extra = json.loads(self._properties["encrypted_extra"])
new_config = encrypted_extra.get("oauth2_client_info", {}) # AttributeError
on int
```
This is reachable on any database whose engine spec has OAuth2 configured
(Snowflake/BigQuery/Databricks with OAuth2). The `AttributeError` is not caught
by the route or the `@transaction` decorator, so it escapes as a raw 500.
**Fix:** Reject non-dict `encrypted_extra` at the schema validation layer
(same location, same pattern as `extra_validator`).
`masked_encrypted_extra_validator` delegates to `encrypted_extra_validator`, so
it's covered automatically — no other files need changes.
**Tradeoffs:** This changes a 500 (`AttributeError`, unhandled) into a 422
(`ValidationError`) for this one malformed-input shape — a strict improvement,
no other behavior change. The error message style matches the existing
`extra_validator` pattern.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — API-only change, no UI impact.
### TESTING INSTRUCTIONS
1. Run the new parametrized test:
```bash
pytest
tests/unit_tests/databases/schema_tests.py::test_encrypted_extra_validator_rejects_non_dict_top_level_value
-v
```
2. Verify the existing sibling test still passes:
```bash
pytest
tests/unit_tests/databases/schema_tests.py::test_extra_validator_rejects_non_dict_top_level_value
-v
```
3. Optionally, confirm the fix end-to-end:
```bash
curl -X PUT http://localhost:8088/api/v1/database/1/ \
-H "Content-Type: application/json" \
-d '{"masked_encrypted_extra": "123"}' \
-H "Authorization: Bearer <token>"
```
Should return 422 with "must be a mapping" instead of 500.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] 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]
