You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2022/10/03 02:49:17 UTC
[superset] branch master updated: fix: catch error when masking encrypted extra is none (#21570)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new ef78ec6b30 fix: catch error when masking encrypted extra is none (#21570)
ef78ec6b30 is described below
commit ef78ec6b30ece829e6fcf0a73d35dac343dcd70c
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Sun Oct 2 19:49:01 2022 -0700
fix: catch error when masking encrypted extra is none (#21570)
---
superset/db_engine_specs/base.py | 6 ++-
superset/db_engine_specs/bigquery.py | 16 +++++--
superset/db_engine_specs/gsheets.py | 16 +++++--
superset/models/core.py | 12 ++---
tests/unit_tests/db_engine_specs/test_bigquery.py | 54 ++++++++++++++++++++++-
tests/unit_tests/db_engine_specs/test_gsheets.py | 45 ++++++++++++++++++-
6 files changed, 132 insertions(+), 17 deletions(-)
diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index 5a8354fd78..d80625dc0c 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1618,7 +1618,7 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
return user.username if user else None
@classmethod
- def mask_encrypted_extra(cls, encrypted_extra: str) -> str:
+ def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
"""
Mask ``encrypted_extra``.
@@ -1631,7 +1631,9 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
# pylint: disable=unused-argument
@classmethod
- def unmask_encrypted_extra(cls, old: str, new: str) -> str:
+ def unmask_encrypted_extra(
+ cls, old: Optional[str], new: Optional[str]
+ ) -> Optional[str]:
"""
Remove masks from ``encrypted_extra``.
diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py
index 4016c8f08f..38b1f92023 100644
--- a/superset/db_engine_specs/bigquery.py
+++ b/superset/db_engine_specs/bigquery.py
@@ -396,10 +396,13 @@ class BigQueryEngineSpec(BaseEngineSpec):
raise ValidationError("Invalid service credentials")
@classmethod
- def mask_encrypted_extra(cls, encrypted_extra: str) -> str:
+ def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
+ if encrypted_extra is None:
+ return encrypted_extra
+
try:
config = json.loads(encrypted_extra)
- except json.JSONDecodeError:
+ except (json.JSONDecodeError, TypeError):
return encrypted_extra
try:
@@ -410,14 +413,19 @@ class BigQueryEngineSpec(BaseEngineSpec):
return json.dumps(config)
@classmethod
- def unmask_encrypted_extra(cls, old: str, new: str) -> str:
+ def unmask_encrypted_extra(
+ cls, old: Optional[str], new: Optional[str]
+ ) -> Optional[str]:
"""
Reuse ``private_key`` if available and unchanged.
"""
+ if old is None or new is None:
+ return new
+
try:
old_config = json.loads(old)
new_config = json.loads(new)
- except json.JSONDecodeError:
+ except (TypeError, json.JSONDecodeError):
return new
if "credentials_info" not in new_config:
diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py
index 3ee284788c..3562e0d0b1 100644
--- a/superset/db_engine_specs/gsheets.py
+++ b/superset/db_engine_specs/gsheets.py
@@ -140,10 +140,13 @@ class GSheetsEngineSpec(SqliteEngineSpec):
raise ValidationError("Invalid service credentials")
@classmethod
- def mask_encrypted_extra(cls, encrypted_extra: str) -> str:
+ def mask_encrypted_extra(cls, encrypted_extra: Optional[str]) -> Optional[str]:
+ if encrypted_extra is None:
+ return encrypted_extra
+
try:
config = json.loads(encrypted_extra)
- except json.JSONDecodeError:
+ except (TypeError, json.JSONDecodeError):
return encrypted_extra
try:
@@ -154,14 +157,19 @@ class GSheetsEngineSpec(SqliteEngineSpec):
return json.dumps(config)
@classmethod
- def unmask_encrypted_extra(cls, old: str, new: str) -> str:
+ def unmask_encrypted_extra(
+ cls, old: Optional[str], new: Optional[str]
+ ) -> Optional[str]:
"""
Reuse ``private_key`` if available and unchanged.
"""
+ if old is None or new is None:
+ return new
+
try:
old_config = json.loads(old)
new_config = json.loads(new)
- except json.JSONDecodeError:
+ except (TypeError, json.JSONDecodeError):
return new
if "service_account_info" not in new_config:
diff --git a/superset/models/core.py b/superset/models/core.py
index 8c1b58171d..ca15137935 100755
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -248,7 +248,7 @@ class Database(
return sqlalchemy_url.get_backend_name()
@property
- def masked_encrypted_extra(self) -> str:
+ def masked_encrypted_extra(self) -> Optional[str]:
return self.db_engine_spec.mask_encrypted_extra(self.encrypted_extra)
@property
@@ -261,10 +261,12 @@ class Database(
masked_encrypted_extra = db_engine_spec.mask_encrypted_extra(
self.encrypted_extra
)
- try:
- encrypted_config = json.loads(masked_encrypted_extra)
- except (TypeError, json.JSONDecodeError):
- encrypted_config = {}
+ encrypted_config = {}
+ if masked_encrypted_extra is not None:
+ try:
+ encrypted_config = json.loads(masked_encrypted_extra)
+ except (TypeError, json.JSONDecodeError):
+ pass
try:
# pylint: disable=useless-suppression
diff --git a/tests/unit_tests/db_engine_specs/test_bigquery.py b/tests/unit_tests/db_engine_specs/test_bigquery.py
index 13f7fd3150..8c33489faf 100644
--- a/tests/unit_tests/db_engine_specs/test_bigquery.py
+++ b/tests/unit_tests/db_engine_specs/test_bigquery.py
@@ -186,9 +186,61 @@ def test_unmask_encrypted_extra() -> None:
}
)
- assert json.loads(BigQueryEngineSpec.unmask_encrypted_extra(old, new)) == {
+ assert json.loads(str(BigQueryEngineSpec.unmask_encrypted_extra(old, new))) == {
"credentials_info": {
"project_id": "yellow-unicorn-314419",
"private_key": "SECRET",
},
}
+
+
+def test_unmask_encrypted_extra_when_empty() -> None:
+ """
+ Test that a None value works for ``encrypted_extra``.
+ """
+ from superset.db_engine_specs.bigquery import BigQueryEngineSpec
+
+ old = None
+ new = json.dumps(
+ {
+ "credentials_info": {
+ "project_id": "yellow-unicorn-314419",
+ "private_key": "XXXXXXXXXX",
+ },
+ }
+ )
+
+ assert json.loads(str(BigQueryEngineSpec.unmask_encrypted_extra(old, new))) == {
+ "credentials_info": {
+ "project_id": "yellow-unicorn-314419",
+ "private_key": "XXXXXXXXXX",
+ },
+ }
+
+
+def test_unmask_encrypted_extra_when_new_is_empty() -> None:
+ """
+ Test that a None value works for ``encrypted_extra``.
+ """
+ from superset.db_engine_specs.bigquery import BigQueryEngineSpec
+
+ old = json.dumps(
+ {
+ "credentials_info": {
+ "project_id": "black-sanctum-314419",
+ "private_key": "SECRET",
+ },
+ }
+ )
+ new = None
+
+ assert BigQueryEngineSpec.unmask_encrypted_extra(old, new) is None
+
+
+def test_mask_encrypted_extra_when_empty() -> None:
+ """
+ Test that the encrypted extra will return a none value if the field is empty.
+ """
+ from superset.db_engine_specs.bigquery import BigQueryEngineSpec
+
+ assert BigQueryEngineSpec.mask_encrypted_extra(None) is None
diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py
index 219f259c1f..a226653ed5 100644
--- a/tests/unit_tests/db_engine_specs/test_gsheets.py
+++ b/tests/unit_tests/db_engine_specs/test_gsheets.py
@@ -228,9 +228,52 @@ def test_unmask_encrypted_extra() -> None:
}
)
- assert json.loads(GSheetsEngineSpec.unmask_encrypted_extra(old, new)) == {
+ assert json.loads(str(GSheetsEngineSpec.unmask_encrypted_extra(old, new))) == {
"service_account_info": {
"project_id": "yellow-unicorn-314419",
"private_key": "SECRET",
},
}
+
+
+def test_unmask_encrypted_extra_when_old_is_none() -> None:
+ """
+ Test that a None value works for ``encrypted_extra``.
+ """
+ from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+
+ old = None
+ new = json.dumps(
+ {
+ "service_account_info": {
+ "project_id": "yellow-unicorn-314419",
+ "private_key": "XXXXXXXXXX",
+ },
+ }
+ )
+
+ assert json.loads(str(GSheetsEngineSpec.unmask_encrypted_extra(old, new))) == {
+ "service_account_info": {
+ "project_id": "yellow-unicorn-314419",
+ "private_key": "XXXXXXXXXX",
+ },
+ }
+
+
+def test_unmask_encrypted_extra_when_new_is_none() -> None:
+ """
+ Test that a None value works for ``encrypted_extra``.
+ """
+ from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+
+ old = json.dumps(
+ {
+ "service_account_info": {
+ "project_id": "yellow-unicorn-314419",
+ "private_key": "XXXXXXXXXX",
+ },
+ }
+ )
+ new = None
+
+ assert GSheetsEngineSpec.unmask_encrypted_extra(old, new) is None