Skip to content

Commit

Permalink
do not overwrite the stored password with the masked password (#1209)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
dennisobrien authored and mistercrunch committed Oct 11, 2016
1 parent bf1f5ea commit cd2ab42
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
7 changes: 5 additions & 2 deletions caravel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
17 changes: 16 additions & 1 deletion tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit cd2ab42

Please sign in to comment.