rusackas commented on code in PR #40139:
URL: https://github.com/apache/superset/pull/40139#discussion_r3416917764
##########
tests/unit_tests/extensions/ssh_test.py:
##########
@@ -69,6 +95,154 @@ def test_ssh_tunnel_timeout_setting() -> None:
assert sshtunnel.SSH_TIMEOUT == 321.0
+def _make_ed25519_pem() -> str:
+ """Generate a fresh OpenSSH-format ed25519 private key PEM."""
+ key = Ed25519PrivateKey.generate()
+ return key.private_bytes(
+ encoding=Encoding.PEM,
+ format=PrivateFormat.OpenSSH,
+ encryption_algorithm=NoEncryption(),
+ ).decode()
+
+
+def test_create_tunnel_accepts_ed25519_private_key() -> None:
+ """
+ Regression for #24180: ed25519 SSH keys must be loadable for tunnel
+ setup. The bug surfaces as ``unpack requires 4 bytes`` because the
+ code unconditionally calls ``RSAKey.from_private_key`` regardless of
+ key type, which mis-parses the ed25519 byte stream.
+
+ The Superset UI accepts any key the user pastes, so the fix is to
+ detect the key type (or use ``paramiko.PKey.from_private_key`` once
+ paramiko exposes a polymorphic loader) rather than hard-coding RSA.
+
+ This test exercises ``create_tunnel`` end-to-end with a freshly
+ generated ed25519 key. It mocks ``sshtunnel.open_tunnel`` so the
+ test does not actually open a network connection — only the key
+ parsing path is exercised.
+ """
+ manager = _make_manager()
+ ssh_tunnel = _make_ssh_tunnel(_make_ed25519_pem())
+
+ with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+ manager.create_tunnel(
+ ssh_tunnel, "postgresql://user:[email protected]:5432/x"
+ )
+
+ # Key-type-agnostic loader must produce a paramiko PKey usable as ssh_pkey.
+ assert mock_open.called, "open_tunnel was never invoked — key parsing
aborted"
+ forwarded_pkey = mock_open.call_args.kwargs["ssh_pkey"]
+ assert isinstance(forwarded_pkey, Ed25519Key)
+
+
+def test_create_tunnel_accepts_rsa_private_key_unchanged() -> None:
+ """
+ Companion to test_create_tunnel_accepts_ed25519_private_key: pin the
+ backward-compatible RSA path so a fix for ed25519 doesn't regress the
+ historically-supported RSA case. Uses a freshly generated RSA key in
+ OpenSSH format.
+ """
+ rsa_pem = (
+ generate_rsa_key(public_exponent=65537, key_size=2048)
+ .private_bytes(
+ encoding=Encoding.PEM,
+ format=PrivateFormat.OpenSSH,
+ encryption_algorithm=NoEncryption(),
+ )
+ .decode()
+ )
+
+ manager = _make_manager()
+ ssh_tunnel = _make_ssh_tunnel(rsa_pem)
+
+ with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+ manager.create_tunnel(
+ ssh_tunnel, "postgresql://user:[email protected]:5432/x"
+ )
+
+ assert mock_open.called, "open_tunnel was never invoked — RSA key parsing
aborted"
+ assert isinstance(mock_open.call_args.kwargs["ssh_pkey"], RSAKey)
+
+
+def test_create_tunnel_accepts_ecdsa_private_key() -> None:
+ """
+ Companion to the ed25519 and RSA tests: verify ECDSAKey (the third type
+ in _SSH_KEY_TYPES) is reachable by _load_private_key. Uses NIST P-256,
+ the most common ECDSA curve in practice.
+ """
+ ecdsa_pem = (
+ ec.generate_private_key(ec.SECP256R1())
+ .private_bytes(
+ encoding=Encoding.PEM,
+ format=PrivateFormat.OpenSSH,
+ encryption_algorithm=NoEncryption(),
+ )
+ .decode()
+ )
+
+ manager = _make_manager()
+ ssh_tunnel = _make_ssh_tunnel(ecdsa_pem)
+
+ with patch("superset.extensions.ssh.sshtunnel.open_tunnel") as mock_open:
+ manager.create_tunnel(
+ ssh_tunnel, "postgresql://user:[email protected]:5432/x"
+ )
+
+ assert mock_open.called, "open_tunnel was never invoked — ECDSA key
parsing aborted"
+ assert mock_open.call_args.kwargs["ssh_pkey"] is not None
Review Comment:
Good catch — tightened it to `assert isinstance(..., ECDSAKey)` to match the
ed25519 and RSA tests, so a fall-through to the wrong key class would now fail.
--
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]
