hughhhh opened a new pull request, #45350: URL: https://github.com/apache/superset/pull/45350
### SUMMARY #44003 refuses an update that changes a connection's effective destination (URI host/port, engine params, SSH tunnel, OAuth2 endpoints) while a stored secret is left masked. For `encrypted_extra`, it decided "reused" by comparing whole blobs: the submission, unmasked against the stored value, had to be byte-identical to the stored one. That comparison goes wrong in both directions: - **Misses a masked secret.** If any other field in the blob changes in the same save (e.g. the OAuth2 `scope`), or the stored JSON is formatted differently (the create dialog stores compact JSON), the strings differ. A still-masked secret then follows the move to the new destination. For a key-pair connection with no URI password, this check is the only guard. - **Refuses a valid move.** Re-entering every secret reproduces the stored blob exactly, so the move is refused even though every secret was supplied. That's the move the error message asks the admin to make. This PR judges reuse per secret instead (`encrypted_extra_secrets_reused` in `superset/commands/database/utils.py`, mirroring how `oauth2_endpoint_rebind_unsafe` decides it). A stored secret (a non-empty value at one of the engine's sensitive paths) counts as reused when: - the submission leaves it masked, or - the submission leaves `masked_encrypted_extra` out entirely, so the stored blob stays attached. A changed value is fresh, even if it equals the stored one. A dropped path drops the secret. Found while validating Snowflake OAuth2 connections; reported against the embedded fallback credential work in #45068, though the behaviour predates it. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (backend). ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/commands/databases/update_test.py tests/unit_tests/commands/databases/utils_test.py ``` New tests: - **Refused:** a destination move with a masked secret plus another field changed, with a compact-JSON stored blob, and on a key-pair connection with no URI password. - **Accepted:** a move with every secret re-entered unchanged. - **Helper unit tests:** a dropped path, an omitted payload, and malformed or non-object JSON. With master's whole-blob comparison put back, the 4 guard tests fail. The two existing tests that mock the engine spec now give it real sensitive paths. `tests/unit_tests/commands/databases` and `tests/unit_tests/databases`: 547 passed. ### 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
