E2e attempts broker albeit PyMsalRuntime init fails

rayluo · rayluo · commit a565ba20e5bc · 2025-02-20T01:15:47.000-08:00
diff --git a/msal/application.py b/msal/application.py
@@ -675,7 +675,8 @@ def _decide_broker(self, allow_broker, enable_pii_log):
                 "allow_broker is deprecated. "
                 "Please use PublicClientApplication(..., "
                 "enable_broker_on_windows=True, "
-                "enable_broker_on_mac=...)",
+                # No need to mention non-Windows platforms, because allow_broker is only for Windows
+                "...)",
                 DeprecationWarning)
         opted_in_for_broker = (
             self._enable_broker  # True means Opted-in from PCA
@@ -697,7 +698,7 @@ def _decide_broker(self, allow_broker, enable_pii_log):
                 _init_broker(enable_pii_log)
             except RuntimeError:
                 self._enable_broker = False
-                logger.exception(
+                logger.warning(  # It is common on Mac and Linux where broker is not built-in
                     "Broker is unavailable on this platform. "
                     "We will fallback to non-broker.")
         logger.debug("Broker enabled? %s", self._enable_broker)
diff --git a/tests/broker_util.py b/tests/broker_util.py
@@ -0,0 +1,21 @@
+import logging
+
+
+logger = logging.getLogger(__name__)
+
+
+def is_pymsalruntime_installed() -> bool:
+    try:
+        import pymsalruntime
+        logger.info("PyMsalRuntime installed and initialized")
+        return True
+    except ImportError:
+        logger.info("PyMsalRuntime not installed")
+        return False
+    except RuntimeError:
+        logger.warning(
+            "PyMsalRuntime installed but failed to initialize the real broker. "
+            "This may happen on Mac and Linux where broker is not built-in. "
+            "Test cases shall attempt broker and test its fallback behavior."
+        )
+        return True
diff --git a/tests/test_account_source.py b/tests/test_account_source.py
@@ -3,15 +3,11 @@
     from unittest.mock import patch
 except:
     from mock import patch
-try:
-    import pymsalruntime
-    broker_available = True
-except ImportError:
-    broker_available = False
 import msal
 from tests import unittest
 from tests.test_token_cache import build_response
 from tests.http_client import MinimalResponse
+from tests.broker_util import is_pymsalruntime_installed
 
 
 SCOPE = "scope_foo"
@@ -24,54 +20,62 @@
 def _mock_post(url, headers=None, *args, **kwargs):
     return MinimalResponse(status_code=200, text=json.dumps(TOKEN_RESPONSE))
 
-@unittest.skipUnless(broker_available, "These test cases need pip install msal[broker]")
+@unittest.skipUnless(is_pymsalruntime_installed(), "These test cases need pip install msal[broker]")
 @patch("msal.broker._acquire_token_silently", return_value=dict(
-    TOKEN_RESPONSE, _account_id="placeholder"))
+   TOKEN_RESPONSE, _account_id="placeholder"))
 @patch.object(msal.authority, "tenant_discovery", return_value={
     "authorization_endpoint": "https://contoso.com/placeholder",
     "token_endpoint": "https://contoso.com/placeholder",
 })  # Otherwise it would fail on OIDC discovery
 class TestAccountSourceBehavior(unittest.TestCase):
 
+    def setUp(self):
+        self.app = msal.PublicClientApplication(
+            "client_id",
+            enable_broker_on_windows=True,
+            )
+        if not self.app._enable_broker:
+            self.skipTest(
+                "These test cases require patching msal.broker which is only possible "
+                "when broker enabled successfully i.e. no RuntimeError")
+        return super().setUp()
+
     def test_device_flow_and_its_silent_call_should_bypass_broker(self, _, mocked_broker_ats):
-        app = msal.PublicClientApplication("client_id", enable_broker_on_windows=True)
-        result = app.acquire_token_by_device_flow({"device_code": "123"}, post=_mock_post)
+        result = self.app.acquire_token_by_device_flow({"device_code": "123"}, post=_mock_post)
         self.assertEqual(result["token_source"], "identity_provider")
 
-        account = app.get_accounts()[0]
+        account = self.app.get_accounts()[0]
         self.assertEqual(account["account_source"], "urn:ietf:params:oauth:grant-type:device_code")
 
-        result = app.acquire_token_silent_with_error(
+        result = self.app.acquire_token_silent_with_error(
             [SCOPE], account, force_refresh=True, post=_mock_post)
         mocked_broker_ats.assert_not_called()
         self.assertEqual(result["token_source"], "identity_provider")
 
     def test_ropc_flow_and_its_silent_call_should_invoke_broker(self, _, mocked_broker_ats):
-        app = msal.PublicClientApplication("client_id", enable_broker_on_windows=True)
         with patch("msal.broker._signin_silently", return_value=dict(TOKEN_RESPONSE, _account_id="placeholder")):
-            result = app.acquire_token_by_username_password(
+            result = self.app.acquire_token_by_username_password(
                 "username", "placeholder", [SCOPE], post=_mock_post)
         self.assertEqual(result["token_source"], "broker")
 
-        account = app.get_accounts()[0]
+        account = self.app.get_accounts()[0]
         self.assertEqual(account["account_source"], "broker")
 
-        result = app.acquire_token_silent_with_error(
+        result = self.app.acquire_token_silent_with_error(
             [SCOPE], account, force_refresh=True, post=_mock_post)
         self.assertEqual(result["token_source"], "broker")
 
     def test_interactive_flow_and_its_silent_call_should_invoke_broker(self, _, mocked_broker_ats):
-        app = msal.PublicClientApplication("client_id", enable_broker_on_windows=True)
-        with patch.object(app, "_acquire_token_interactive_via_broker", return_value=dict(
+        with patch.object(self.app, "_acquire_token_interactive_via_broker", return_value=dict(
                 TOKEN_RESPONSE, _account_id="placeholder")):
-            result = app.acquire_token_interactive(
-                [SCOPE], parent_window_handle=app.CONSOLE_WINDOW_HANDLE)
+            result = self.app.acquire_token_interactive(
+                [SCOPE], parent_window_handle=self.app.CONSOLE_WINDOW_HANDLE)
         self.assertEqual(result["token_source"], "broker")
 
-        account = app.get_accounts()[0]
+        account = self.app.get_accounts()[0]
         self.assertEqual(account["account_source"], "broker")
 
-        result = app.acquire_token_silent_with_error(
+        result = self.app.acquire_token_silent_with_error(
             [SCOPE], account, force_refresh=True, post=_mock_post)
         mocked_broker_ats.assert_called_once()
         self.assertEqual(result["token_source"], "broker")
diff --git a/tests/test_e2e.py b/tests/test_e2e.py
@@ -29,12 +29,9 @@
 from tests.http_client import MinimalHttpClient, MinimalResponse
 from msal.oauth2cli import AuthCodeReceiver
 from msal.oauth2cli.oidc import decode_part
+from tests.broker_util import is_pymsalruntime_installed
+
 
-try:
-    import pymsalruntime
-    broker_available = True
-except ImportError:
-    broker_available = False
 logger = logging.getLogger(__name__)
 logging.basicConfig(level=logging.DEBUG if "-v" in sys.argv else logging.INFO)
 
@@ -44,6 +41,7 @@
 except ImportError:
     logger.warn("Run pip install -r requirements.txt for optional dependency")
 
+_PYMSALRUNTIME_INSTALLED = is_pymsalruntime_installed()
 _AZURE_CLI = "04b07795-8ddb-461a-bbee-02f9e1bf7b46"
 
 def _get_app_and_auth_code(
@@ -187,19 +185,14 @@ def _build_app(cls,
                 http_client=http_client or MinimalHttpClient(),
             )
         else:
-            # Reuse same test cases, by run them with and without broker
-            try:
-                import pymsalruntime
-                broker_available = True
-            except ImportError:
-                broker_available = False
+            # Reuse same test cases, by running them with and without PyMsalRuntime installed
             return msal.PublicClientApplication(
                 client_id,
                 authority=authority,
                 oidc_authority=oidc_authority,
                 http_client=http_client or MinimalHttpClient(),
-                enable_broker_on_windows=broker_available,
-                enable_broker_on_mac=broker_available,
+                enable_broker_on_windows=_PYMSALRUNTIME_INSTALLED,
+                enable_broker_on_mac=_PYMSALRUNTIME_INSTALLED,
                 )
 
     def _test_username_password(self,
@@ -1307,7 +1300,7 @@ def test_acquire_token_silent_with_an_empty_cache_should_return_none(self):
         #       it means MSAL Python is not affected by that.
 
 
-@unittest.skipUnless(broker_available, "AT POP feature is only supported by using broker")
+@unittest.skipUnless(_PYMSALRUNTIME_INSTALLED, "AT POP feature is only supported by using broker")
 class PopTestCase(LabBasedTestCase):
     def test_at_pop_should_contain_pop_scheme_content(self):
         auth_scheme = msal.PopAuthScheme(
