From 90a7d436d5a1ac1a472eef4b522827d31b34609b Mon Sep 17 00:00:00 2001 From: Yashvardhan Thanvi Date: Thu, 15 Oct 2026 14:30:08 +0530 Subject: [PATCH] authcode: drop idle connections after 10 seconds AuthCodeReceiver serves one connection at a time with HTTPServer.handle_request(). _AuthCodeHandler had no read timeout, so a client that connected and never sent a request blocked handle_request() inside _get_auth_response(). The browser's form_post callback then waited in the listen backlog until the overall timeout, or forever when there was none. This was seen with az login behind VS Code port forwarding (Remote SSH and Dev Containers). Give _AuthCodeHandler a timeout of 10 seconds. StreamRequestHandler applies it to the accepted socket, and handle_one_request() closes the connection when a read times out, so handle_request() returns and the loop accepts the next connection. A client delivering the auth response sends it right after connecting, so only idle connections are affected. Each idle connection queued ahead of the callback now delays it by up to 10 seconds instead of blocking it. Fixes #950 --- msal/oauth2cli/authcode.py | 9 ++++++++ tests/test_authcode.py | 43 +++++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/msal/oauth2cli/authcode.py b/msal/oauth2cli/authcode.py index 0a5f0689..1ae2eaf5 100644 --- a/msal/oauth2cli/authcode.py +++ b/msal/oauth2cli/authcode.py @@ -136,6 +136,15 @@ def _printify(text): return repr(text) if isinstance(text, str) and not text.isprintable() else text class _AuthCodeHandler(BaseHTTPRequestHandler): + # Read timeout, in seconds, for each accepted connection. Our server handles + # one connection at a time, so a client that connects but sends nothing + # would otherwise block the auth response queued behind it, maybe forever. + # On timeout, handle_one_request() closes that connection, + # so that handle_request() returns and the next one can be accepted. + # A client delivering the auth response sends it right after connecting, + # so in practice only idle connections would hit this limit. + timeout = 10 + def do_GET(self): qs = parse_qs(urlparse(self.path).query) welcome_param = qs.get('welcome', [None])[0] diff --git a/tests/test_authcode.py b/tests/test_authcode.py index 178cf285..911b7414 100644 --- a/tests/test_authcode.py +++ b/tests/test_authcode.py @@ -1,6 +1,8 @@ import unittest import socket import sys +import threading +import time try: # Python 3 from unittest.mock import patch, mock_open @@ -9,7 +11,8 @@ import requests -from msal.oauth2cli.authcode import AuthCodeReceiver, _is_inside_docker +from msal.oauth2cli.authcode import ( + AuthCodeReceiver, _AuthCodeHandler, _is_inside_docker) class TestAuthCodeReceiver(unittest.TestCase): @@ -110,6 +113,44 @@ def test_post_request_state_mismatch(self): result = receiver.get_auth_response(timeout=3, state="expected_state") self.assertIsNone(result, "Should not receive auth response due to state mismatch") + def _post_auth_response_behind_idle_connections(self, idle_connections): + """Return (expected, actual, elapsed) for an auth response that a browser + posts after the given number of idle connections reached the receiver""" + expected = {"code": "test_auth_code_12345", "state": "test_state_67890"} + with AuthCodeReceiver() as receiver: + address = ("127.0.0.1", receiver.get_port()) + for _ in range(idle_connections): # They connect but never send a request + self.addCleanup(socket.create_connection(address).close) + + def browser_posts_auth_response(): + time.sleep(1) # By now, the receiver has accepted an idle connection + try: + requests.post( + "http://{}:{}".format(*address), data=expected, timeout=30) + except requests.RequestException: + pass # The receiver closed without reading it. Caller will notice. + browser = threading.Thread(target=browser_posts_auth_response) + browser.daemon = True + browser.start() + begin = time.time() + result = receiver.get_auth_response(timeout=30, state=expected["state"]) + elapsed = time.time() - begin + browser.join(5) + return expected, result, elapsed + + def test_idle_connection_should_not_block_auth_response(self): + # Seen behind VS Code port forwarding, see + # https://github.com/AzureAD/microsoft-authentication-library-for-python/issues/950 + expected, result, elapsed = self._post_auth_response_behind_idle_connections(1) + self.assertEqual(expected, result) + self.assertLess(elapsed, _AuthCodeHandler.timeout + 5) + + def test_several_idle_connections_should_not_block_auth_response(self): + with patch.object(_AuthCodeHandler, "timeout", 1): # Keeps this test short + expected, result, elapsed = self._post_auth_response_behind_idle_connections(3) + self.assertEqual(expected, result) + self.assertLess(elapsed, 3 * 1 + 5) + class TestIsInsideDocker(unittest.TestCase): """Guard the container-detection heuristic used to decide the bind address.