Skip to content

Commit 3d4ea13

Browse files
ai: apply changes for #912 (2 review threads)
Addresses: - #3798853448 at src/databricks/sql/auth/auth.py:141 - #3798853453 at src/databricks/sql/auth/auth.py:128 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent afb6dae commit 3d4ea13

2 files changed

Lines changed: 59 additions & 4 deletions

File tree

src/databricks/sql/auth/auth.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,10 @@ def get_python_sql_connector_auth_provider(hostname: str, http_client, **kwargs)
125125
oauth_redirect_port = kwargs.get("oauth_redirect_port")
126126
oauth_scopes = kwargs.get("oauth_scopes")
127127

128-
scopes = oauth_scopes or PYSQL_OAUTH_SCOPES
129128
if caller_client_id:
130129
client_id = caller_client_id
130+
# Caller owns the bundle: forward their scopes verbatim.
131+
scopes = oauth_scopes or PYSQL_OAUTH_SCOPES
131132
# Foreign client_id with no explicit port falls through to the base
132133
# (app-neutral) default, NOT the driver's app-specific range.
133134
redirect_port_range = (
@@ -137,9 +138,15 @@ def get_python_sql_connector_auth_provider(hostname: str, http_client, **kwargs)
137138
)
138139
else:
139140
client_id = default_client_id
140-
redirect_port_range = (
141-
[oauth_redirect_port] if oauth_redirect_port else default_redirect_port_range
142-
)
141+
# The caller relies on the driver's default client_id, whose OAuth app
142+
# only has the app-specific default scopes/range registered. A caller-
143+
# supplied oauth_scopes must NOT override the driver's app-specific
144+
# default scopes here, just as a caller-supplied oauth_redirect_port must
145+
# NOT override the default range (doing so risks a redirect_uri_mismatch
146+
# or scope error, PECOBLR-4039); both only take effect alongside a
147+
# caller-supplied oauth_client_id.
148+
scopes = PYSQL_OAUTH_SCOPES
149+
redirect_port_range = default_redirect_port_range
143150

144151
cfg = ClientContext(
145152
hostname=normalize_host_name(hostname),

tests/unit/test_auth.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
from databricks.sql.auth.auth import (
1313
get_python_sql_connector_auth_provider,
1414
PYSQL_OAUTH_CLIENT_ID,
15+
PYSQL_OAUTH_REDIRECT_PORT_RANGE,
16+
PYSQL_OAUTH_SCOPES,
1517
)
1618
from databricks.sql.auth.oauth import OAuthManager, Token, ClientCredentialsTokenSource
1719
from databricks.sql.auth.authenticators import (
@@ -251,6 +253,52 @@ def test_get_python_sql_connector_u2m_foreign_client_id_no_port(
251253
self.assertEqual(provider._client_id, "test-custom-u2m-app")
252254
self.assertEqual(provider.oauth_manager.port_range, [8030])
253255

256+
@patch.object(DatabricksOAuthProvider, "_initial_get_token")
257+
def test_get_python_sql_connector_u2m_default_client_id_ignores_redirect_port(
258+
self, mock__initial_get_token
259+
):
260+
# AUTH-013 / PECOBLR-4039: a caller relying on the driver's DEFAULT
261+
# client_id must stay pinned to the driver's registered redirect port
262+
# range even if oauth_redirect_port is supplied - the driver's OAuth app
263+
# only registers 8020-8024, so honoring an arbitrary port would produce a
264+
# redirect_uri_mismatch. oauth_redirect_port only applies with a
265+
# caller-supplied oauth_client_id.
266+
hostname = "foo.cloud.databricks.com"
267+
kwargs = {"oauth_redirect_port": 9000}
268+
mock_http_client = MagicMock()
269+
auth_provider = get_python_sql_connector_auth_provider(
270+
hostname, mock_http_client, **kwargs
271+
)
272+
273+
provider = auth_provider.external_provider
274+
self.assertEqual(type(provider).__name__, "DatabricksOAuthProvider")
275+
self.assertEqual(provider._client_id, PYSQL_OAUTH_CLIENT_ID)
276+
self.assertEqual(
277+
provider.oauth_manager.port_range, PYSQL_OAUTH_REDIRECT_PORT_RANGE
278+
)
279+
280+
@patch.object(DatabricksOAuthProvider, "_initial_get_token")
281+
def test_get_python_sql_connector_u2m_default_client_id_ignores_scopes(
282+
self, mock__initial_get_token
283+
):
284+
# AUTH-013 / PECOBLR-4039: a caller relying on the driver's DEFAULT
285+
# client_id must stay pinned to the driver's app-specific default scopes
286+
# even if oauth_scopes is supplied - the driver's OAuth app only has its
287+
# own default scopes registered, so honoring arbitrary scopes risks a
288+
# scope error. oauth_scopes only applies with a caller-supplied
289+
# oauth_client_id.
290+
hostname = "foo.cloud.databricks.com"
291+
kwargs = {"oauth_scopes": ["all-apis"]}
292+
mock_http_client = MagicMock()
293+
auth_provider = get_python_sql_connector_auth_provider(
294+
hostname, mock_http_client, **kwargs
295+
)
296+
297+
provider = auth_provider.external_provider
298+
self.assertEqual(type(provider).__name__, "DatabricksOAuthProvider")
299+
self.assertEqual(provider._client_id, PYSQL_OAUTH_CLIENT_ID)
300+
self.assertEqual(provider._scopes_as_str, " ".join(PYSQL_OAUTH_SCOPES))
301+
254302

255303
class TestClientCredentialsTokenSource:
256304
@pytest.fixture

0 commit comments

Comments
 (0)