Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable broker support on Linux #766

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Enable broker support on Linux #766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d2fc35f
Enable broker support on Linux
DharshanBJ Nov 7, 2024
edf77b4
update version number
DharshanBJ Nov 7, 2024
3bf9111
Update sample/interactive_sample.py
DharshanBJ Nov 9, 2024
caab447
Update msal/application.py
DharshanBJ Nov 9, 2024
aa5c74c
Update tests/broker-test.py
DharshanBJ Nov 9, 2024
6f52d28
revert back release version bump
DharshanBJ Nov 9, 2024
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
1 change: 1 addition & 0 deletions msal/__main__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ def _main():
instance_discovery=instance_discovery,
enable_broker_on_windows=enable_broker,
enable_broker_on_mac=enable_broker,
enable_broker_on_linux=enable_broker,
enable_pii_log=enable_pii_log,
token_cache=global_cache,
) if not is_cca else msal.ConfidentialClientApplication(
Expand Down
14 changes: 11 additions & 3 deletions msal/application.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ 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=...)",
"...)",
DeprecationWarning)
opted_in_for_broker = (
self._enable_broker # True means Opted-in from PCA
Expand Down Expand Up @@ -1921,7 +1921,7 @@ def __init__(self, client_id, client_credential=None, **kwargs):

.. note::

You may set enable_broker_on_windows and/or enable_broker_on_mac to True.
You may set enable_broker_on_windows and/or enable_broker_on_mac and/or enable_broker_on_linux to True.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Does the Linux broker require yet another new redirect_uri being registered? If so, please also update the documentation here.


**What is a broker, and why use it?**

Expand Down Expand Up @@ -1989,15 +1989,23 @@ 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.31.0.

:param boolean enable_broker_on_linux:
This setting is only effective if your app is running on Linux.
This parameter defaults to None, which means MSAL will not utilize a broker.

New in MSAL Python 1.32.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)
enable_broker_on_mac = kwargs.pop("enable_broker_on_mac", False)
enable_broker_on_linux = kwargs.pop("enable_broker_on_linux", False)
self._enable_broker = bool(
enable_broker_on_windows and sys.platform == "win32"
or enable_broker_on_mac and sys.platform == "darwin")
or enable_broker_on_mac and sys.platform == "darwin"
or enable_broker_on_linux and sys.platform == "linux")
super(PublicClientApplication, self).__init__(
client_id, client_credential=None, **kwargs)

Expand Down
2 changes: 1 addition & 1 deletion sample/interactive_sample.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
oidc_authority=os.getenv('OIDC_AUTHORITY'), # For External ID with custom domain
#enable_broker_on_windows=True, # Opted in. You will be guided to meet the prerequisites, if your app hasn't already
#enable_broker_on_mac=True, # Opted in. You will be guided to meet the prerequisites, if your app hasn't already

#enable_broker_on_linux=True, # Opted in. You will be guided to meet the prerequisites, if your app hasn't already
token_cache=global_token_cache, # Let this app (re)use an existing token cache.
# If absent, ClientApplication will create its own empty token cache
)
Expand Down
6 changes: 4 additions & 2 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ broker =
# most existing MSAL Python apps do not have the redirect_uri needed by broker.
#
# We need pymsalruntime.CallbackData introduced in PyMsalRuntime 0.14
pymsalruntime>=0.14,<0.18; python_version>='3.6' and platform_system=='Windows'
pymsalruntime>=0.14,<0.19; python_version>='3.6' and platform_system=='Windows'
# On Mac, PyMsalRuntime 0.17+ is expected to support SSH cert and ROPC
pymsalruntime>=0.17,<0.18; python_version>='3.8' and platform_system=='Darwin'
pymsalruntime>=0.17,<0.19; python_version>='3.8' and platform_system=='Darwin'
# PyMsalRuntime 0.18+ is expected to support broker on Linux
pymsalruntime>=0.18,<0.19; python_version>='3.8' and platform_system=='Linux'
Copy link
Contributor

@fengga fengga Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you released or do you plan to release 0.18.0 for Linux? I saw right now it's still 0.17.1
https://pypi.org/project/pymsalruntime/#history

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not yet released 0.18.0, but i'll be making that release soon, and this will be the first package with linux broker support

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: This PR shall be changed to a draft. We won't merge it in until pymsalruntime 0.18 being released.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we ensure the next release of pymsalruntime have whls built for Python 3.13 as well? The latest version of pymsalruntime doesn't seem to have that.


[options.packages.find]
exclude =
Expand Down
4 changes: 3 additions & 1 deletion tests/broker-test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
_AZURE_CLI,
authority="https://login.microsoftonline.com/organizations",
enable_broker_on_mac=True,
enable_broker_on_windows=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you find this test file, please make sure it is able to invoke broker and you need to manually finish the signin process in the broker prompt.

enable_broker_on_windows=True,
enable_broker_on_linux=True,
)

def interactive_and_silent(scopes, auth_scheme, data, expected_token_type):
print("An account picker shall be pop up, possibly behind this console. Continue from there.")
Expand Down
1 change: 1 addition & 0 deletions tests/test_e2e.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ def _build_app(cls,
http_client=http_client or MinimalHttpClient(),
enable_broker_on_windows=broker_available,
enable_broker_on_mac=broker_available,
enable_broker_on_linux=broker_available,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Thanks for finding this test file!

suggestion: If you haven't already, please also do python -m unittest tests.test_e2e and make sure we won't run into any issue. Note that you will need to setup an .env first, whose content is described at the beginning of this test_e2e.py file.

)

def _test_username_password(self,
Expand Down