rusackas opened a new pull request, #44003:
URL: https://github.com/apache/superset/pull/44003

   ### SUMMARY
   `test_connection`, `validate_parameters`, database update, and database 
import all decide whether to reattach a stored connection secret (password, 
`encrypted_extra`, or SSH tunnel credential) by comparing one visible field 
against its masked/stored form, without checking whether other 
request-controlled fields also feed into the actual connection destination.
   
   `extra.engine_params` (in particular 
`engine_params.connect_args.host`/`.port`) is merged into the DBAPI connect 
kwargs by SQLAlchemy and can override the host/port carried in the URI itself. 
The SSH tunnel's `server_address`/`server_port` can change independently of the 
URI. For database update, the URI's own host/port can change directly while its 
password segment stays masked, with nothing comparing it against the stored 
host first.
   
   This PR adds an identity check across all four paths (mirroring the check 
#43393 already added for database import's URI-host case, extended to also 
cover `extra.engine_params`, and applied fresh to database update, which had 
none before) and refuses to reattach a stored secret unless a fresh credential 
is supplied for a destination that changed. Shared comparison logic lives in 
`superset/commands/database/utils.py`. Also scopes 
`DatabaseDAO.get_database_by_name` to the same `DatabaseFilter` 
object-visibility boundary `find_by_id`/`get_connection` already use for this 
resource.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A — backend-only change, no UI impact.
   
   ### TESTING INSTRUCTIONS
   Regression tests added per site (`extra.engine_params` variant, 
SSH-tunnel-endpoint variant, and for database update also the plain URI-host 
variant). Each was verified to fail without its corresponding fix and pass with 
it. `pytest tests/unit_tests/commands/databases/ tests/unit_tests/databases/ 
tests/unit_tests/daos/` — 521 passed. Legitimate flows (unchanged destination, 
or a deliberate move with a freshly supplied credential) are covered and 
unaffected.
   
   ### 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))
   - [ ] Confirm DB Migration upgrade and downgrade tested
   - [ ] 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]

Reply via email to