From ca66ba489399a3c37e39875f6d1f6c278cfcb9be Mon Sep 17 00:00:00 2001 From: Dennis O'Brien Date: Mon, 19 Sep 2016 15:14:00 -0700 Subject: [PATCH] Fix initialization of Database sqlalchemy_uri and password (#1137) * move initialization of Database sqlalchemy_uri and password from DatabaseView.pre_add to utils.get_or_create_main_db. Unit tests for mysql and postgres include username and password in the SQLALCHEMY_DATABASE_URI. * modified test_testconn to work with sqlalchemy uri with a username and password. --- .travis.yml | 3 +++ caravel/models.py | 6 ++++++ caravel/utils.py | 2 +- caravel/views.py | 16 +++++++--------- tests/core_tests.py | 21 +++++++++++++++++---- tox.ini | 10 +++++----- 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/.travis.yml b/.travis.yml index 5f72d3eb6..531e606ef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,10 @@ before_install: - npm install -g npm@'>=3.9.5' before_script: - mysql -e 'drop database if exists caravel; create database caravel DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci' -u root + - mysql -u root -e "CREATE USER 'mysqluser'@'localhost' IDENTIFIED BY 'mysqluserpassword';" + - mysql -u root -e "GRANT ALL ON caravel.* TO 'mysqluser'@'localhost';" - psql -c 'create database caravel;' -U postgres + - psql -c "CREATE USER postgresuser WITH PASSWORD 'pguserpassword';" -U postgres - export PATH=${PATH}:/tmp/hive/bin install: - pip install --upgrade pip diff --git a/caravel/models.py b/caravel/models.py index 785ebd8a4..956c36e26 100644 --- a/caravel/models.py +++ b/caravel/models.py @@ -433,6 +433,12 @@ class Database(Model, AuditMixinNullable): url = make_url(self.sqlalchemy_uri_decrypted) return url.get_backend_name() + def set_sqlalchemy_uri(self, uri): + conn = sqla.engine.url.make_url(uri) + self.password = conn.password + conn.password = "X" * 10 if conn.password else None + self.sqlalchemy_uri = str(conn) # hides the password + def get_sqla_engine(self, schema=None): extra = self.get_extra() url = make_url(self.sqlalchemy_uri_decrypted) diff --git a/caravel/utils.py b/caravel/utils.py index 2fb43ada9..04b0ee28a 100644 --- a/caravel/utils.py +++ b/caravel/utils.py @@ -100,7 +100,7 @@ def get_or_create_main_db(caravel): if not dbobj: dbobj = DB(database_name="main") logging.info(config.get("SQLALCHEMY_DATABASE_URI")) - dbobj.sqlalchemy_uri = config.get("SQLALCHEMY_DATABASE_URI") + dbobj.set_sqlalchemy_uri(config.get("SQLALCHEMY_DATABASE_URI")) dbobj.expose_in_sqllab = True db.session.add(dbobj) db.session.commit() diff --git a/caravel/views.py b/caravel/views.py index ceafcf723..db4a6e77c 100755 --- a/caravel/views.py +++ b/caravel/views.py @@ -497,10 +497,6 @@ class DatabaseView(CaravelModelView, DeleteMixin): # noqa } def pre_add(self, db): - conn = sqla.engine.url.make_url(db.sqlalchemy_uri) - db.password = conn.password - conn.password = "X" * 10 if conn.password else None - db.sqlalchemy_uri = str(conn) # hides the password utils.merge_perm(sm, 'database_access', db.perm) def pre_update(self, db): @@ -1252,15 +1248,17 @@ class Caravel(BaseCaravelView): try: uri = request.json.get('uri') db_name = request.json.get('name') - if db_name and ':XXXXXXXXXX@' in uri: + if db_name: database = ( - db.session() + db.session .query(models.Database) - .filter_by(database_name=db_name).first() + .filter_by(database_name=db_name) + .first() ) - if database is not None: + if uri == database.safe_sqlalchemy_uri(): + # the password-masked uri was passed + # use the URI associated with this database uri = database.sqlalchemy_uri_decrypted - connect_args = ( request.json .get('extras', {}) diff --git a/tests/core_tests.py b/tests/core_tests.py index 4e8e96199..01a7a8111 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -115,17 +115,30 @@ class CoreTests(CaravelTestCase): assert self.client.get('/ping').data.decode('utf-8') == "OK" def test_testconn(self): - data = json.dumps({'uri': 'sqlite:////tmp/caravel_unittests.db'}) - response = self.client.post('/caravel/testconn', data=data, content_type='application/json') - assert response.status_code == 200 + database = ( + db.session + .query(models.Database) + .filter_by(database_name='main') + .first() + ) + # validate that the endpoint works with the password-masked sqlalchemy uri data = json.dumps({ - 'uri': 'postgresql+psycopg2://foo:XXXXXXXXXX@127.0.0.1/bar', + 'uri': database.safe_sqlalchemy_uri(), 'name': 'main' }) response = self.client.post('/caravel/testconn', data=data, content_type='application/json') assert response.status_code == 200 + # validate that the endpoint works with the decrypted sqlalchemy uri + data = json.dumps({ + 'uri': database.sqlalchemy_uri_decrypted, + 'name': 'main' + }) + response = self.client.post('/caravel/testconn', data=data, content_type='application/json') + assert response.status_code == 200 + + def test_warm_up_cache(self): slice = db.session.query(models.Slice).first() resp = self.client.get( diff --git a/tox.ini b/tox.ini index 282cfa8c6..050a6c5c1 100644 --- a/tox.ini +++ b/tox.ini @@ -37,17 +37,17 @@ commands = [testenv:py27-mysql] basepython = python2.7 setenv = - CARAVEL__SQLALCHEMY_DATABASE_URI = mysql://root@localhost/caravel + CARAVEL__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/caravel [testenv:py34-mysql] basepython = python3.4 setenv = - CARAVEL__SQLALCHEMY_DATABASE_URI = mysql://root@localhost/caravel + CARAVEL__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/caravel [testenv:py35-mysql] basepython = python3.5 setenv = - CARAVEL__SQLALCHEMY_DATABASE_URI = mysql://root@localhost/caravel + CARAVEL__SQLALCHEMY_DATABASE_URI = mysql://mysqluser:mysqluserpassword@localhost/caravel [testenv:py27-sqlite] basepython = python2.7 @@ -62,12 +62,12 @@ setenv = [testenv:py27-postgres] basepython = python2.7 setenv = - CARAVEL__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://postgres@localhost/caravel + CARAVEL__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://postgresuser:pguserpassword@localhost/caravel [testenv:py34-postgres] basepython = python3.4 setenv = - CARAVEL__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://postgres@localhost/caravel + CARAVEL__SQLALCHEMY_DATABASE_URI = postgresql+psycopg2://postgresuser:pguserpassword@localhost/caravel [testenv:javascript] commands = {toxinidir}/caravel/assets/js_build.sh