From 3d3d02f5a86f668a4662a9cbd7125d70e759a8da Mon Sep 17 00:00:00 2001 From: Ray Luo Date: Wed, 25 Oct 2023 14:46:43 -0700 Subject: [PATCH] Deprecate allow_broker, use enable_broker_on_windows --- msal/__main__.py | 6 +- msal/application.py | 132 ++++++++++++++++++++--------------- sample/interactive_sample.py | 4 +- tests/test_e2e.py | 34 +++++---- 4 files changed, 100 insertions(+), 76 deletions(-) diff --git a/msal/__main__.py b/msal/__main__.py index 48a50f6c..b0fb6b7a 100644 --- a/msal/__main__.py +++ b/msal/__main__.py @@ -190,9 +190,9 @@ def _main(): option_renderer=lambda a: a["name"], header="Impersonate this app (or you can type in the client_id of your own app)", accept_nonempty_string=True) - allow_broker = _input_boolean("Allow broker?") + enable_broker = _input_boolean("Enable broker? It will error out later if your app has not registered some redirect URI") enable_debug_log = _input_boolean("Enable MSAL Python's DEBUG log?") - enable_pii_log = _input_boolean("Enable PII in broker's log?") if allow_broker and enable_debug_log else False + enable_pii_log = _input_boolean("Enable PII in broker's log?") if enable_broker and enable_debug_log else False app = msal.PublicClientApplication( chosen_app["client_id"] if isinstance(chosen_app, dict) else chosen_app, authority=_select_options([ @@ -205,7 +205,7 @@ def _main(): header="Input authority (Note that MSA-PT apps would NOT use the /common authority)", accept_nonempty_string=True, ), - allow_broker=allow_broker, + enable_broker_on_windows=enable_broker, enable_pii_log=enable_pii_log, ) if enable_debug_log: diff --git a/msal/application.py b/msal/application.py index 95fa86c1..a5d97b47 100644 --- a/msal/application.py +++ b/msal/application.py @@ -181,6 +181,8 @@ class ClientApplication(object): _TOKEN_SOURCE_CACHE = "cache" _TOKEN_SOURCE_BROKER = "broker" + _enable_broker = False + def __init__( self, client_id, client_credential=None, authority=None, validate_authority=True, @@ -470,48 +472,7 @@ def __init__( New in version 1.19.0. :param boolean allow_broker: - This parameter is NOT applicable to :class:`ConfidentialClientApplication`. - - A broker is a component installed on your device. - Broker implicitly gives your device an identity. By using a broker, - your device becomes a factor that can satisfy MFA (Multi-factor authentication). - This factor would become mandatory - if a tenant's admin enables a corresponding Conditional Access (CA) policy. - The broker's presence allows Microsoft identity platform - to have higher confidence that the tokens are being issued to your device, - and that is more secure. - - An additional benefit of broker is, - it runs as a long-lived process with your device's OS, - and maintains its own cache, - so that your broker-enabled apps (even a CLI) - could automatically SSO from a previously established signed-in session. - - This parameter defaults to None, which means MSAL will not utilize a broker. - If this parameter is set to True, - MSAL will use the broker whenever possible, - and automatically fall back to non-broker behavior. - That also means your app does not need to enable broker conditionally, - you can always set allow_broker to True, - as long as your app meets the following prerequisite: - - * Installed optional dependency, e.g. ``pip install msal[broker]>=1.20,<2``. - (Note that broker is currently only available on Windows 10+) - - * Register a new redirect_uri for your desktop app as: - ``ms-appx-web://Microsoft.AAD.BrokerPlugin/your_client_id`` - - * Tested your app in following scenarios: - - * Windows 10+ - - * PublicClientApplication's following methods:: - acquire_token_interactive(), acquire_token_by_username_password(), - acquire_token_silent() (or acquire_token_silent_with_error()). - - * AAD and MSA accounts (i.e. Non-ADFS, non-B2C) - - New in version 1.20.0. + Deprecated. Please use ``enable_broker_on_windows`` instead. :param boolean enable_pii_log: When enabled, logs may include PII (Personal Identifiable Information). @@ -584,34 +545,47 @@ def __init__( ) else: raise - is_confidential_app = bool( - isinstance(self, ConfidentialClientApplication) or self.client_credential) + + self._decide_broker(allow_broker, enable_pii_log) + self.token_cache = token_cache or TokenCache() + self._region_configured = azure_region + self._region_detected = None + self.client, self._regional_client = self._build_client( + client_credential, self.authority) + self.authority_groups = None + self._telemetry_buffer = {} + self._telemetry_lock = Lock() + + def _decide_broker(self, allow_broker, enable_pii_log): + is_confidential_app = self.client_credential or isinstance( + self, ConfidentialClientApplication) if is_confidential_app and allow_broker: raise ValueError("allow_broker=True is only supported in PublicClientApplication") - self._enable_broker = False - if (allow_broker and not is_confidential_app - and sys.platform == "win32" + # Historically, we chose to support ClientApplication("client_id", allow_broker=True) + if allow_broker: + warnings.warn( + "allow_broker is deprecated. " + "Please use PublicClientApplication(..., enable_broker_on_windows=True)", + DeprecationWarning) + self._enable_broker = self._enable_broker or ( + # When we started the broker project on Windows platform, + # the allow_broker was meant to be cross-platform. Now we realize + # that other platforms have different redirect_uri requirements, + # so the old allow_broker is deprecated and will only for Windows. + allow_broker and sys.platform == "win32") + if (self._enable_broker and not is_confidential_app and not self.authority.is_adfs and not self.authority._is_b2c): try: from . import broker # Trigger Broker's initialization - self._enable_broker = True if enable_pii_log: broker._enable_pii_log() except RuntimeError: + self._enable_broker = False logger.exception( "Broker is unavailable on this platform. " "We will fallback to non-broker.") logger.debug("Broker enabled? %s", self._enable_broker) - self.token_cache = token_cache or TokenCache() - self._region_configured = azure_region - self._region_detected = None - self.client, self._regional_client = self._build_client( - client_credential, self.authority) - self.authority_groups = None - self._telemetry_buffer = {} - self._telemetry_lock = Lock() - def _decorate_scope( self, scopes, reserved_scope=frozenset(['openid', 'profile', 'offline_access'])): @@ -1746,9 +1720,53 @@ class PublicClientApplication(ClientApplication): # browser app or mobile app def __init__(self, client_id, client_credential=None, **kwargs): """Same as :func:`ClientApplication.__init__`, except that ``client_credential`` parameter shall remain ``None``. + + .. note:: + + You may set enable_broker_on_windows to True. + + What is a broker, and why use it? + + A broker is a component installed on your device. + Broker implicitly gives your device an identity. By using a broker, + your device becomes a factor that can satisfy MFA (Multi-factor authentication). + This factor would become mandatory + if a tenant's admin enables a corresponding Conditional Access (CA) policy. + The broker's presence allows Microsoft identity platform + to have higher confidence that the tokens are being issued to your device, + and that is more secure. + + An additional benefit of broker is, + it runs as a long-lived process with your device's OS, + and maintains its own cache, + so that your broker-enabled apps (even a CLI) + could automatically SSO from a previously established signed-in session. + + ADFS and B2C do not support broker. + MSAL will automatically fallback to use browser. + + You shall only enable broker when your app: + + 1. is running on supported platforms, + and already registered their corresponding redirect_uri + + * ``ms-appx-web://Microsoft.AAD.BrokerPlugin/your_client_id`` + if your app is expected to run on Windows 10+ + + 2. installed broker dependency, + e.g. ``pip install msal[broker]>=1.25,<2``. + + 3. tested with ``acquire_token_interactive()`` and ``acquire_token_silent()``. + + :param boolean enable_broker_on_windows: + This setting is only effective if your app is running on Windows 10+. + This parameter defaults to None, which means MSAL will not utilize a broker. """ if client_credential is not None: raise ValueError("Public Client should not possess credentials") + # Using kwargs notation for now. We will switch to keyword-only arguments. + enable_broker_on_windows = kwargs.pop("enable_broker_on_windows", False) + self._enable_broker = enable_broker_on_windows and sys.platform == "win32" super(PublicClientApplication, self).__init__( client_id, client_credential=None, **kwargs) diff --git a/sample/interactive_sample.py b/sample/interactive_sample.py index b063661c..f283ed29 100644 --- a/sample/interactive_sample.py +++ b/sample/interactive_sample.py @@ -37,8 +37,8 @@ # Create a preferably long-lived app instance, to avoid the overhead of app creation global_app = msal.PublicClientApplication( config["client_id"], authority=config["authority"], - #allow_broker=True, # If opted in, you will be guided to meet the prerequisites, when applicable - # See also: https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#wam-value-proposition + #enable_broker_on_windows=True, # Opted in. You will be guided to meet the prerequisites, if your app hasn't already + # See also: https://docs.microsoft.com/en-us/azure/active-directory/develop/scenario-desktop-acquire-token-wam#wam-value-proposition token_cache=global_token_cache, # Let this app (re)use an existing token cache. # If absent, ClientApplication will create its own empty token cache ) diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 9deec8f7..36e7a445 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -165,21 +165,27 @@ def _build_app(cls, http_client=None, azure_region=None, **kwargs): - try: - import pymsalruntime - broker_available = True - except ImportError: - broker_available = False - return (msal.ConfidentialClientApplication - if client_credential else msal.PublicClientApplication)( - client_id, - client_credential=client_credential, - authority=authority, - azure_region=azure_region, - http_client=http_client or MinimalHttpClient(), - allow_broker=broker_available # This way, we reuse same test cases, by run them with and without broker - and not client_credential, + if client_credential: + return msal.ConfidentialClientApplication( + client_id, + client_credential=client_credential, + authority=authority, + azure_region=azure_region, + 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 + return msal.PublicClientApplication( + client_id, + authority=authority, + http_client=http_client or MinimalHttpClient(), + enable_broker_on_windows=broker_available, + ) def _test_username_password(self, authority=None, client_id=None, username=None, password=None, scope=None,