rusackas commented on code in PR #44910:
URL: https://github.com/apache/superset/pull/44910#discussion_r4174025406
##########
tests/testcontainers/db_engine_specs/test_mysql.py:
##########
@@ -115,3 +147,98 @@ def test_get_columns_maps_native_types(engine: Engine) ->
None:
assert spec is not None
assert spec.generic_type == GenericDataType.NUMERIC
assert isinstance(spec.sqla_type, Integer)
+
+
[email protected](scope="module")
+def no_tls_container() -> Iterator[MySqlContainer]:
+ """A server with TLS fully disabled, to test the fail-closed guarantee.
+
+ `--ssl=0` is MySQL's deprecated-but-still-supported spelling for turning
+ off TLS support entirely (`have_ssl` reports `DISABLED`), verified
+ directly against this image before writing this fixture.
+ """
+ with MySqlContainer("mysql:8.0", command="--ssl=0") as container:
+ yield container
+
+
+def _require_tls_connect_args(uri_query: str = "ssl=1") -> tuple[URL,
dict[str, Any]]:
+ """Run a `ssl=1` request through the real `adjust_engine_params` path.
+
+ Goes through `MySQLEngineSpec.adjust_engine_params` rather than calling
+ `require_mysql_tls` directly, so the test exercises the exact call path
+ production code takes (including driver-name resolution from a bare
+ `mysql://` URL), not a shortcut around it.
+ """
+ uri = make_url(f"mysql://root:test@placeholder:3306/test?{uri_query}")
+ return MySQLEngineSpec.adjust_engine_params(uri, {})
+
+
+def test_require_mysql_tls_fails_closed_without_server_tls(
+ no_tls_container: MySqlContainer,
+) -> None:
+ """
+ The core guarantee apache/superset#44723 exists to provide: a server
+ offering no TLS at all must make the connection fail, never succeed
+ silently in cleartext. mysqlclient linked against MariaDB Connector/C
+ maps `ssl_mode=REQUIRED` to *opportunistic* TLS -- it degrades to
+ cleartext instead of refusing when the server can't negotiate TLS --
+ which is exactly the silent-downgrade this fix closes by requiring
+ `VERIFY_CA` instead for that client library. Only a real client library
+ actually attempting real TLS negotiation against a real non-TLS server
+ can show this; a mocked cursor never negotiates anything.
+ """
+ host = _tcp_host(no_tls_container)
+ port = no_tls_container.get_exposed_port(no_tls_container.port)
+ uri, connect_args = _require_tls_connect_args()
+ uri = uri.set(host=host, port=int(port), database=no_tls_container.dbname)
+ engine = create_engine(uri, connect_args=connect_args)
+ with pytest.raises(OperationalError):
Review Comment:
Good catch, @aminghadersohi. Applied your `match="SSL is required"`
suggestion so an unrelated auth error no longer passes. I also corrected the
docstring to say CI links Oracle libmysqlclient, so `VERIFY_CA` only runs on
MariaDB-linked builds, and dropped the leaked CA temp file.
--
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]
