Skip to content

Commit 2410fb0

Browse files
ai: apply changes for #914 (1 review thread)
Addresses: - #3799256419 at src/databricks/sql/backend/kernel/auth_bridge.py:266 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent b2eed93 commit 2410fb0

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

src/databricks/sql/backend/kernel/auth_bridge.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,11 @@ def kernel_auth_kwargs(
247247
# http://localhost:{port}, with scheme/host/path fixed. The
248248
# connector registers a port *range* for its app but the kernel
249249
# accepts a single port, so we forward the first (canonical)
250-
# registered port.
250+
# registered port. A caller-supplied port only overrides that
251+
# default when an explicit client_id is ALSO supplied — matching
252+
# the Thrift path's coupling (a bare oauth_redirect_port paired
253+
# with the default databricks-sql-python app would resolve to an
254+
# unregistered redirect URI and fail the flow).
251255
if auth_type in ("databricks-oauth", "azure-oauth"):
252256
is_azure = auth_type == "azure-oauth"
253257
default_client_id = (
@@ -265,7 +269,7 @@ def kernel_auth_kwargs(
265269
"client_id": client_id or default_client_id,
266270
"redirect_port": (
267271
int(redirect_port)
268-
if redirect_port is not None
272+
if client_id and redirect_port is not None
269273
else default_port_range[0]
270274
),
271275
"oauth_scopes": (

tests/unit/test_kernel_auth_bridge.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,14 +324,34 @@ def test_u2m_custom_client_id_only_falls_back_to_connector_defaults(self):
324324

325325
def test_u2m_redirect_port_coerced_to_int(self):
326326
# oauth_redirect_port may arrive as a string (e.g. from a DSN);
327-
# the kernel binding wants an int.
327+
# the kernel binding wants an int. The port override is coupled to
328+
# an explicit client_id (see the coupling test below), so supply
329+
# one here to exercise the coercion path.
328330
kwargs = kernel_auth_kwargs(
329331
_FakeOAuthProvider(),
330-
{"auth_type": "databricks-oauth", "oauth_redirect_port": "8021"},
332+
{
333+
"auth_type": "databricks-oauth",
334+
"oauth_client_id": "custom-client",
335+
"oauth_redirect_port": "8021",
336+
},
331337
)
332338
assert kwargs["redirect_port"] == 8021
333339
assert isinstance(kwargs["redirect_port"], int)
334340

341+
def test_u2m_redirect_port_ignored_without_client_id(self):
342+
# A bare oauth_redirect_port (no explicit client_id) must NOT be
343+
# forwarded: it would be paired with the default databricks-sql-python
344+
# app, whose registered redirect URIs only cover the default port
345+
# range, so an arbitrary port would resolve to an unregistered URI
346+
# and fail the U2M flow. This mirrors the Thrift path's coupling,
347+
# where oauth_redirect_port_range is only overridden when both
348+
# oauth_client_id and oauth_redirect_port are supplied.
349+
kwargs = kernel_auth_kwargs(
350+
_FakeOAuthProvider(),
351+
{"auth_type": "databricks-oauth", "oauth_redirect_port": 9999},
352+
)
353+
assert kwargs["redirect_port"] == PYSQL_OAUTH_REDIRECT_PORT_RANGE[0]
354+
335355
@pytest.mark.parametrize("auth_type", ["databricks-oauth", "azure-oauth"])
336356
def test_u2m_forwards_custom_scopes(self, auth_type):
337357
kwargs = kernel_auth_kwargs(

0 commit comments

Comments
 (0)