Skip to content

Commit 3cb80da

Browse files
author
peco-engineer-bot[bot]
committed
If the token is null, the connection hangs (#458)
Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 9775996 commit 3cb80da

2 files changed

Lines changed: 67 additions & 0 deletions

File tree

src/databricks/sql/auth/oauth.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ def refresh(self) -> Token:
6262

6363

6464
class OAuthManager:
65+
# Maximum time (in seconds) to wait for the browser OAuth redirect callback
66+
# before giving up. Without this, the local callback server would block
67+
# forever in a headless environment (e.g. a notebook/job with no browser),
68+
# making the connection appear to hang indefinitely. See issue #458.
69+
REDIRECT_CALLBACK_TIMEOUT_SECONDS = 60 * 5
70+
6571
def __init__(
6672
self,
6773
port_range: List[int],
@@ -133,6 +139,10 @@ def __get_authorization_code(self, client, auth_url, scope, state, challenge):
133139
for port in self.port_range:
134140
try:
135141
with HTTPServer(("", port), handler) as httpd:
142+
# Bound how long we wait for the browser redirect callback so
143+
# that a headless environment (no browser to complete the
144+
# flow) fails with a clear error instead of hanging forever.
145+
httpd.timeout = self.REDIRECT_CALLBACK_TIMEOUT_SECONDS
136146
redirect_url = OAuthManager.__get_redirect_url(port)
137147
auth_req_uri, _, _ = client.prepare_authorization_request(
138148
authorization_url=auth_url,

tests/unit/test_auth.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,63 @@ def test_get_python_sql_connector_default_auth(self, mock__initial_get_token):
207207

208208
self.assertEqual(auth_provider.external_provider._client_id, PYSQL_OAUTH_CLIENT_ID)
209209

210+
@patch("databricks.sql.auth.oauth.webbrowser.open_new")
211+
@patch.object(OAuthManager, "_OAuthManager__fetch_well_known_config")
212+
def test_get_tokens_does_not_hang_when_no_callback_received(
213+
self, mock_fetch_config, mock_open_new
214+
):
215+
"""When the U2M browser OAuth callback never arrives (e.g. a headless
216+
notebook/job with a null token), the local redirect server must not
217+
block forever. It should time out and surface a clear error rather than
218+
hang. See issue #458."""
219+
mock_fetch_config.return_value = {
220+
"authorization_endpoint": "https://foo.cloud.databricks.com/oidc/oauth2/v2.0/authorize",
221+
"token_endpoint": "https://foo.cloud.databricks.com/oidc/oauth2/v2.0/token",
222+
}
223+
# Do not actually launch a browser during the test.
224+
mock_open_new.return_value = True
225+
226+
oauth_manager = OAuthManager(
227+
port_range=[8030],
228+
client_id="mock-id",
229+
idp_endpoint=InHouseOAuthEndpointCollection(),
230+
http_client=MagicMock(),
231+
)
232+
# Keep the test fast: shorten the callback wait. The production default
233+
# is minutes; the bug is that WITHOUT any bound the wait is infinite.
234+
oauth_manager.REDIRECT_CALLBACK_TIMEOUT_SECONDS = 2
235+
236+
# No callback is ever delivered to the redirect server. Run the flow in
237+
# a daemon thread and join with a wall-clock bound so a regression to
238+
# the infinite-block behaviour fails this test loudly instead of hanging
239+
# the whole suite.
240+
import threading
241+
242+
result = {}
243+
244+
def run():
245+
try:
246+
oauth_manager.get_tokens(
247+
hostname="foo.cloud.databricks.com", scope="offline_access sql"
248+
)
249+
result["outcome"] = "returned"
250+
except BaseException as e: # noqa: BLE001
251+
result["outcome"] = "raised"
252+
result["error"] = e
253+
254+
worker = threading.Thread(target=run, daemon=True)
255+
worker.start()
256+
worker.join(timeout=30)
257+
258+
self.assertFalse(
259+
worker.is_alive(),
260+
"OAuth callback server blocked indefinitely waiting for a callback "
261+
"that never arrives (issue #458)",
262+
)
263+
self.assertEqual(result.get("outcome"), "raised")
264+
self.assertIsInstance(result.get("error"), RuntimeError)
265+
self.assertIn("No path parameters were returned", str(result.get("error")))
266+
210267

211268
class TestClientCredentialsTokenSource:
212269
@pytest.fixture

0 commit comments

Comments
 (0)