[bug] Fix CSV upload feature for DB with password #4562
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4562 +/- ##
======================================
Coverage 71.2% 71.2%
======================================
Files 187 187
Lines 14786 14786
Branches 1083 1083
======================================
Hits 10528 10528
Misses 4255 4255
Partials 3 3
Continue to review full report at Codecov.
|
tox.ini
Outdated
| @@ -82,7 +82,7 @@ commands = | |||
| [testenv:py27-mysql] | |||
| basepython = python2.7 | |||
| setenv = | |||
| SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://root@localhost/superset?charset=utf8 | |||
| SUPERSET__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/superset | |||
There was a problem hiding this comment.
I believe you'll still need the ?charset=utf8 suffix as Python 2.7 uses ASCII by default.
|
@john-bodley done, thanks |
|
LGTM |
|
@mistercrunch would you mind taking a look at this? I just want to be sure there's no risk of leaking the decrypted URI. |
|
Afaik, that's the lowest level we can use the unencrypted password and it goes directly into the creation of the db_engine. I doubt there is any chance for leakage. Curious to hear what Max thinks |
|
LGTM |
* Use sqlalchemy_uri_decrypted in create_engine calls * Update tox mysql uri * Include mysql charset=utf8 for py2.7 in tox.ini
* Use sqlalchemy_uri_decrypted in create_engine calls * Update tox mysql uri * Include mysql charset=utf8 for py2.7 in tox.ini (cherry picked from commit 31a9957)
* Use sqlalchemy_uri_decrypted in create_engine calls * Update tox mysql uri * Include mysql charset=utf8 for py2.7 in tox.ini
* Use sqlalchemy_uri_decrypted in create_engine calls * Update tox mysql uri * Include mysql charset=utf8 for py2.7 in tox.ini
mistercrunch
added
🏷️ bot
As of #4298, the call to
create_enginewithincreate_table_from_csvlocated here is usingsqlalchemy_urito create a new SQLAlchemy engine. This works as expected if the URI does not contain a password - however if it does, this string contains the masked password (i.e. XXXXXXXX), and authentication is attempted using that instead.This change should fix #4285 and #4287.