Antonio-RiveroMartnez commented on code in PR #23099:
URL: https://github.com/apache/superset/pull/23099#discussion_r1112312284


##########
superset/databases/schemas.py:
##########
@@ -719,6 +723,65 @@ def validate_password(self, data: Dict[str, Any], 
**kwargs: Any) -> None:
         if password == PASSWORD_MASK and data.get("password") is None:
             raise ValidationError("Must provide a password for the database")
 
+    @validates_schema
+    def validate_ssh_tunnel_password(self, data: Dict[str, Any], **kwargs: 
Any) -> None:
+        """If ssh_tunnel has a masked password, password is required"""
+        uuid = data["uuid"]
+        existing = db.session.query(Database).filter_by(uuid=uuid).first()
+        if existing:
+            return
+
+        # Our DB has a ssh_tunnel in it
+        if ssh_tunnel := data.get("ssh_tunnel"):
+            if not is_feature_enabled("SSH_TUNNELING"):
+                raise SSHTunnelingNotEnabledError()
+            password = ssh_tunnel.get("password")
+            if password == PASSWORD_MASK:
+                raise ValidationError("Must provide a password for the ssh 
tunnel")
+        return
+
+    @validates_schema
+    def validate_ssh_tunnel_private_key(
+        self, data: Dict[str, Any], **kwargs: Any
+    ) -> None:
+        """If ssh_tunnel has a masked private key, private key is required"""
+        uuid = data["uuid"]
+        existing = db.session.query(Database).filter_by(uuid=uuid).first()
+        if existing:
+            return
+
+        # Our DB has a ssh_tunnel in it
+        if ssh_tunnel := data.get("ssh_tunnel"):
+            if not is_feature_enabled("SSH_TUNNELING"):
+                raise SSHTunnelingNotEnabledError()
+            private_key = ssh_tunnel.get("private_key")
+            if private_key == PASSWORD_MASK:
+                raise ValidationError("Must provide a private key for the ssh 
tunnel")

Review Comment:
   After re-reading this, I'm adding some more checks, for example as you 
mentioned, not only raising when `== PASSWORD_MASK` but also checking there's 
at least one login method (`password` OR `private_key + private_key_password`). 
That way, if users manipulate files manually, we still check the integrity of 
the data being passed. Thanks for the insight 👍 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to