rusackas commented on code in PR #40584:
URL: https://github.com/apache/superset/pull/40584#discussion_r3337055988
##########
superset/commands/dataset/importers/v0.py:
##########
@@ -211,7 +211,13 @@ def import_from_dict(data: dict[str, Any], sync:
Optional[list[str]] = None) ->
if isinstance(data, dict):
logger.info("Importing %d %s", len(data.get(DATABASES_KEY, [])),
DATABASES_KEY)
for database in data.get(DATABASES_KEY, []):
- Database.import_from_dict(database, sync=sync)
+ db_obj = Database.import_from_dict(database, sync=sync)
+ # ``import_from_dict`` sets fields via setattr, bypassing
+ # ``set_sqlalchemy_uri``. Call it explicitly so that any plaintext
+ # password in the URI is extracted into the encrypted ``password``
+ # column and replaced with the password mask in ``sqlalchemy_uri``.
+ if db_obj is not None:
+ db_obj.set_sqlalchemy_uri(db_obj.sqlalchemy_uri)
Review Comment:
Good catch — addressed in the latest commit.
The guard added to in :
```python
parsed = make_url_safe(db_obj.sqlalchemy_uri)
if parsed.password and parsed.password != PASSWORD_MASK:
db_obj.set_sqlalchemy_uri(db_obj.sqlalchemy_uri)
```
and are imported from and respectively — the same sources used inside
itself — so the check is consistent.
A regression test was added to . It:
1. Imports a password-less URI into a fresh database record and confirms
stays .
2. Simulates a prior import by setting a stored password on the record, then
re-imports the same password-less URI and asserts the stored value is not
overwritten.
--
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]
