Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions msal/oauth2cli/authcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
43 changes: 42 additions & 1 deletion tests/test_authcode.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import unittest
import socket
import sys
import threading
import time

try: # Python 3
from unittest.mock import patch, mock_open
Expand All @@ -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):
Expand Down Expand Up @@ -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.
Expand Down