From cd2ab42abcd2ea5f93dd285321c448c97abf4580 Mon Sep 17 00:00:00 2001 From: Dennis O'Brien Date: Tue, 11 Oct 2016 16:49:40 -0700 Subject: [PATCH] do not overwrite the stored password with the masked password (#1209) * do not over-write the stored password with a masked password. his addresses issue #1199 * added a test to validate that sending a password-masked sqlalchemy_uri does not over-write the stored sqlalchemy_uri * requery the database object after the update. use self.assertEqual for more informative error messages. --- caravel/models.py | 7 +++++-- tests/core_tests.py | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/caravel/models.py b/caravel/models.py index 1c6ab9aac91ad..f160e038d9605 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -439,9 +439,12 @@ def backend(self): return url.get_backend_name() def set_sqlalchemy_uri(self, uri): + password_mask = "X" * 10 conn = sqla.engine.url.make_url(uri) - self.password = conn.password - conn.password = "X" * 10 if conn.password else None + if conn.password != password_mask: + # do not over-write the password with the password mask + self.password = conn.password + conn.password = password_mask if conn.password else None self.sqlalchemy_uri = str(conn) # hides the password def get_sqla_engine(self, schema=None): diff --git a/tests/core_tests.py b/tests/core_tests.py index 0271b257585b7..bfd9700aaed09 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -15,7 +15,11 @@ from flask import escape from flask_appbuilder.security.sqla import models as ab_models -from caravel import db, models, utils, appbuilder, sm +import caravel +from caravel import app, db, models, utils, appbuilder, sm +from caravel.source_registry import SourceRegistry +from caravel.models import DruidDatasource +from caravel.views import DatabaseView from .base_tests import CaravelTestCase @@ -176,6 +180,17 @@ def test_testconn(self): response = self.client.post('/caravel/testconn', data=data, content_type='application/json') assert response.status_code == 200 + def test_databaseview_edit(self, username='admin'): + # validate that sending a password-masked uri does not over-write the decrypted uri + self.login(username=username) + database = db.session.query(models.Database).filter_by(database_name='main').first() + sqlalchemy_uri_decrypted = database.sqlalchemy_uri_decrypted + url = 'databaseview/edit/{}'.format(database.id) + data = {k: database.__getattribute__(k) for k in DatabaseView.add_columns} + data['sqlalchemy_uri'] = database.safe_sqlalchemy_uri() + response = self.client.post(url, data=data) + database = db.session.query(models.Database).filter_by(database_name='main').first() + self.assertEqual(sqlalchemy_uri_decrypted, database.sqlalchemy_uri_decrypted) def test_warm_up_cache(self): slice = db.session.query(models.Slice).first()