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.

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

[Identity Broker] Propagate enable support logging #34780

Merged
merged 1 commit into from
Mar 16, 2024
Merged
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
2 changes: 2 additions & 0 deletions sdk/identity/azure-identity-broker/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Features Added

- `InteractiveBrowserBrokerCredential` now supports a `use_operating_system_account` property to enable the use of the currently logged in operating system account for authentication rather than prompting for a credential.
- Added `enable_support_logging` as a keyword argument to `InteractiveBrowserBrokerCredential`. This allows additional support logging which may contain PII.

### Breaking Changes

Expand All @@ -13,6 +14,7 @@
### Other Changes

- Python 3.7 is no longer supported. Please use Python version 3.8 or later.
- Bumped minimum dependency on `azure-identity` to `1.15.0`.

## 1.0.0 (2023-11-07)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class InteractiveBrowserBrokerCredential(_InteractiveBrowserCredential):
https://login.microsoft.com/ to validate the authority. By setting this to **True**, the validation of the
authority is disabled. As a result, it is crucial to ensure that the configured authority host is valid and
trustworthy.
:keyword bool enable_support_logging: Enables additional support logging in the underlying MSAL library.
This logging potentially contains personally identifiable information and is intended to be used only for
troubleshooting purposes.
:raises ValueError: invalid **redirect_uri**
"""

Expand Down Expand Up @@ -135,6 +138,7 @@ def _get_app(self, **kwargs: Any) -> msal.ClientApplication:
http_client=self._client,
instance_discovery=self._instance_discovery,
enable_broker_on_windows=True,
enable_pii_log=self._enable_support_logging,
)

return client_applications_map[tenant_id]
2 changes: 1 addition & 1 deletion sdk/identity/azure-identity-broker/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
},
python_requires=">=3.8",
install_requires=[
"azure-identity<2.0.0,>=1.14.0",
"azure-identity<2.0.0,>=1.15.0",
"msal[broker]>=1.25,<2",
],
)
30 changes: 30 additions & 0 deletions sdk/identity/azure-identity-broker/tests/test_broker.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import pytest
import sys
from unittest.mock import patch, Mock

from azure.core.exceptions import ClientAuthenticationError
from azure.identity.broker import InteractiveBrowserBrokerCredential


Expand All @@ -25,3 +27,31 @@ def test_interactive_browser_broker_cred_signed_in_account():
except Exception: # msal raises TypeError which is expected. We are not testing msal here.
pass
assert mock_signin_silently.called


def test_enable_support_logging_default():
"""The keyword argument for enabling PII in MSAL should be disabled by default."""

cred = InteractiveBrowserBrokerCredential(parent_window_handle="window_handle")
with patch("msal.PublicClientApplication") as mock_client_application:
with patch("msal.PublicClientApplication.acquire_token_interactive"):
with pytest.raises(ClientAuthenticationError):
cred.get_token("scope")

assert mock_client_application.call_count == 1, "credential didn't create an msal application"
_, kwargs = mock_client_application.call_args
assert not kwargs["enable_pii_log"]


def test_enable_support_logging_enabled():
"""The keyword argument for enabling PII in MSAL should be propagated correctly."""

cred = InteractiveBrowserBrokerCredential(parent_window_handle="window_handle", enable_support_logging=True)
with patch("msal.PublicClientApplication") as mock_client_application:
with patch("msal.PublicClientApplication.acquire_token_interactive"):
with pytest.raises(ClientAuthenticationError):
cred.get_token("scope")

assert mock_client_application.call_count == 1, "credential didn't create an msal application"
_, kwargs = mock_client_application.call_args
assert kwargs["enable_pii_log"]