-
Notifications
You must be signed in to change notification settings - Fork 208
Integration with Broker-on-Mac #596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f7b3f4e
77f64f4
21a625c
2613132
b9d9b05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -216,6 +216,8 @@ class ClientApplication(object): | |
| "You can enable broker by following these instructions. " | ||
| "https://msal-python.readthedocs.io/en/latest/#publicclientapplication") | ||
|
|
||
| _enable_broker = False | ||
|
|
||
| def __init__( | ||
| self, client_id, | ||
| client_credential=None, authority=None, validate_authority=True, | ||
|
|
@@ -640,7 +642,9 @@ def _decide_broker(self, allow_broker, enable_pii_log): | |
| if allow_broker: | ||
| warnings.warn( | ||
rayluo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "allow_broker is deprecated. " | ||
| "Please use PublicClientApplication(..., enable_broker_on_windows=True)", | ||
| "Please use PublicClientApplication(..., " | ||
| "enable_broker_on_windows=True, " | ||
| "enable_broker_on_mac=...)", | ||
| DeprecationWarning) | ||
| self._enable_broker = self._enable_broker or ( | ||
| # When we started the broker project on Windows platform, | ||
|
|
@@ -1881,7 +1885,7 @@ def __init__(self, client_id, client_credential=None, **kwargs): | |
|
|
||
| .. note:: | ||
|
|
||
| You may set enable_broker_on_windows to True. | ||
| You may set enable_broker_on_windows and/or enable_broker_on_mac to True. | ||
|
|
||
| **What is a broker, and why use it?** | ||
|
|
||
|
|
@@ -1907,9 +1911,11 @@ def __init__(self, client_id, client_credential=None, **kwargs): | |
|
|
||
| * ``ms-appx-web://Microsoft.AAD.BrokerPlugin/your_client_id`` | ||
| if your app is expected to run on Windows 10+ | ||
| * ``msauth.com.msauth.unsignedapp://auth`` | ||
| if your app is expected to run on Mac | ||
|
|
||
| 2. installed broker dependency, | ||
| e.g. ``pip install msal[broker]>=1.25,<2``. | ||
| e.g. ``pip install msal[broker]>=1.31,<2``. | ||
|
|
||
| 3. tested with ``acquire_token_interactive()`` and ``acquire_token_silent()``. | ||
|
|
||
|
|
@@ -1941,12 +1947,21 @@ def __init__(self, client_id, client_credential=None, **kwargs): | |
| This parameter defaults to None, which means MSAL will not utilize a broker. | ||
|
|
||
| New in MSAL Python 1.25.0. | ||
|
|
||
| :param boolean enable_broker_on_mac: | ||
| This setting is only effective if your app is running on Mac. | ||
| This parameter defaults to None, which means MSAL will not utilize a broker. | ||
|
|
||
| New in MSAL Python 1.31.0. | ||
| """ | ||
| 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" | ||
| enable_broker_on_mac = kwargs.pop("enable_broker_on_mac", False) | ||
| self._enable_broker = bool( | ||
| enable_broker_on_windows and sys.platform == "win32" | ||
| or enable_broker_on_mac and sys.platform == "darwin") | ||
rayluo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| super(PublicClientApplication, self).__init__( | ||
| client_id, client_credential=None, **kwargs) | ||
|
|
||
|
|
@@ -2024,11 +2039,23 @@ def acquire_token_interactive( | |
| New in version 1.15. | ||
|
|
||
| :param int parent_window_handle: | ||
| Required if your app is running on Windows and opted in to use broker. | ||
|
|
||
| If your app is a GUI app, | ||
| you are recommended to also provide its window handle, | ||
| so that the sign in UI window will properly pop up on top of your window. | ||
| OPTIONAL. | ||
|
|
||
| * If your app does not opt in to use broker, | ||
| you do not need to provide a ``parent_window_handle`` here. | ||
|
|
||
| * If your app opts in to use broker, | ||
| ``parent_window_handle`` is required. | ||
|
|
||
| - If your app is a GUI app running on modern Windows system, | ||
| you are required to also provide its window handle, | ||
| so that the sign-in window will pop up on top of your window. | ||
| - If your app is a console app runnong on Windows system, | ||
| you can use a placeholder | ||
| ``PublicClientApplication.CONSOLE_WINDOW_HANDLE``. | ||
| - If your app is running on Mac, | ||
| you can use a placeholder | ||
| ``PublicClientApplication.CONSOLE_WINDOW_HANDLE``. | ||
|
Comment on lines
+2042
to
+2058
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, those lines are the changes (or the lack thereof) of the The window handle is required on Windows. Such a decision was influenced by the MsalRuntime's desire to make window handle explicit (CC @MSamWils), and now on Mac this parameter remains required. (CC @kaisong1990 ) If the parameter remains required on Windows, do we want to keep it as required on Mac, or do we make it optional on Mac? Will the latter behavior - i.e. sometimes required, sometimes not - be confusing in itself? Note: Regardless of MSAL's choice, downstream library can make different decision, such as choose to make this parameter optional, by using MSAL's predefined CONSOLE_WINDOW_HANDLE placeholder as a default value. (CC @xiangyan99 ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, you mean it is required on Mac because it is required on Windows? I am a little confused.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Put it this way. We speculate that one consistent API across platform would be ideal. Most Python scripts are console apps and they shall ideally be cross-platform. How do we want those app developers to write their source code?
|
||
|
|
||
| If your app is a console app (most Python scripts are console apps), | ||
| you can use a placeholder value ``msal.PublicClientApplication.CONSOLE_WINDOW_HANDLE``. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # How to Smoke Test MSAL Python | ||
|
|
||
| The experimental `python -m msal` usage is designed to be an interactive tool, | ||
| which can impersonate arbitrary apps and test most of the MSAL Python APIs. | ||
| Note that MSAL Python API's behavior is modeled after OIDC behavior in browser, | ||
| which are not exactly the same as the broker API's behavior, | ||
| despite that the two sets of API happen to have similar names. | ||
|
|
||
| Tokens acquired during the tests will be cached by MSAL Python. | ||
| MSAL Python uses an in-memory token cache by default. | ||
| This test tool, however, saves a token cache snapshot on disk upon each exit, | ||
| and you may choose to reuse it or start afresh during start up. | ||
|
|
||
| Typical test cases are listed below. | ||
|
|
||
| 1. The tool starts with an empty token cache. | ||
| In this state, acquire_token_silent() shall always return empty result. | ||
|
|
||
| 2. When testing with broker, apps would need to register a certain redirect_uri | ||
| for the test cases below to work. | ||
| We will also test an app without the required redirect_uri registration, | ||
| MSAL Python shall return a meaningful error message on what URIs to register. | ||
|
|
||
| 3. Interactive acquire_token_interactive() shall get a token. In particular, | ||
|
|
||
| * The prompt=none option shall succeed when there is a default account, | ||
| or error out otherwise. | ||
| * The prompt=select_account option shall always prompt with an account picker. | ||
| * The prompt=absent option shall prompt an account picker UI | ||
| if there are multiple accounts available in browser | ||
| and none of them is considered a default account. | ||
| In such a case, an optional login_hint=`[email protected]` | ||
| shall bypass the account picker. | ||
|
|
||
| With a broker, the behavior shall largely match the browser behavior, | ||
| unless stated otherwise below. | ||
|
|
||
| * Broker (PyMsalRuntime) on Mac does not support silent signin, | ||
| so the prompt=absent will also always prompt. | ||
|
|
||
| 4. ROPC (Resource Owner Password Credential, a.k.a. the username password flow). | ||
| The acquire_token_by_username_password() is supported by broker on Windows. | ||
| As of Oct 2023, it is not yet supported by broker on Mac, | ||
| so it will fall back to non-broker behavior. | ||
|
|
||
| 5. After step 3 or 4, the acquire_token_silently() shall return a token fast, | ||
| because that is the same token returned by step 3 or 4, cached in MSAL Python. | ||
| We shall also retest this with the force_refresh=True, | ||
| a new token shall be obtained, | ||
| typically slower than a token served from MSAL Python's token cache. | ||
|
|
||
| 6. POP token. | ||
| POP token is supported via broker. | ||
| This tool test the POP token by using a hardcoded Signed Http Request (SHR). | ||
| A test is successful if the POP test function return a token with type as POP. | ||
|
|
||
| 7. SSH Cert. | ||
| The interactive test and silent test shall behave similarly to | ||
| their non ssh-cert counterparts, only the `token_type` would be different. | ||
|
|
||
| 8. Test the remove_account() API. It shall always be successful. | ||
| This effectively signs out an account from MSAL Python, | ||
| we can confirm that by running acquire_token_silent() | ||
| and see that account was gone. | ||
|
|
||
| The remove_account() shall also sign out from broker (if broker was enabled), | ||
| it does not sign out account from browser (even when browser was used). | ||
|
|
Uh oh!
There was an error while loading. Please reload this page.