From 24582d3ffc8d4864ce3d82ef5aeac5543be95889 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 15 Jul 2026 13:44:12 +0300 Subject: [PATCH 01/50] feat: Add Azure CLI auth source (POC) Add azure_cli as a new identity type that delegates token acquisition to Azure CLI via azure-identity's AzureCliCredential. This allows tools calling fab to reuse an existing az login session instead of requiring a separate interactive fab auth login. Changes: - Add 'azure_cli' to AUTH_KEYS identity type allow-list - Add _acquire_token_from_azure_cli() using AzureCliCredential - Add --azure-cli flag to fab auth login - Add 'Azure CLI' option to interactive login menu - Show auth_source in fab auth status output - Add azure-identity>=1.15.0 dependency - Add 12 unit tests covering dispatch, scopes, errors, sanitization Security: error messages are sanitized to never leak token content. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pyproject.toml | 1 + src/fabric_cli/commands/auth/fab_auth.py | 19 +- src/fabric_cli/core/fab_auth.py | 48 ++++ src/fabric_cli/core/fab_constant.py | 2 +- src/fabric_cli/parsers/fab_auth_parser.py | 11 + tests/test_core/test_fab_auth_azure_cli.py | 262 +++++++++++++++++++++ 6 files changed, 341 insertions(+), 2 deletions(-) create mode 100644 tests/test_core/test_fab_auth_azure_cli.py diff --git a/pyproject.toml b/pyproject.toml index f2c52dc5d..08dce9b8e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,6 +20,7 @@ dependencies = [ "msal>=1.34,<2", "msal_extensions", "azure-core>=1.29.0", + "azure-identity>=1.15.0", "questionary", "prompt_toolkit>=3.0.41", "cachetools>=5.5.0", diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 4e1039d3b..ad3270679 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -16,6 +16,7 @@ def init(args: Namespace) -> Any: auth_options = [ "Interactive with a web browser", + "Azure CLI (reuse existing 'az login' session)", "Service principal authentication with secret", "Service principal authentication with certificate", "Service principal authentication with federated credential", @@ -27,7 +28,15 @@ def init(args: Namespace) -> Any: # Clean up stale context files when logging in Context().cleanup_context_files(cleanup_all_stale=True, cleanup_current=False) - if args.identity: + if getattr(args, "azure_cli", False): + FabAuth().set_access_mode("azure_cli", args.tenant) + FabAuth().set_azure_cli(args.tenant) + FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) + FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) + FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + Context().context = FabAuth().get_tenant() + + elif args.identity: FabAuth().set_access_mode("managed_identity") FabAuth().set_managed_identity(args.username) FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) @@ -73,6 +82,13 @@ def init(args: Namespace) -> Any: FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) Context().context = FabAuth().get_tenant() + elif selected_auth.startswith("Azure CLI"): + FabAuth().set_access_mode("azure_cli", args.tenant) + FabAuth().set_azure_cli(args.tenant) + FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) + FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) + FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + Context().context = FabAuth().get_tenant() elif selected_auth.startswith("Service principal authentication"): fab_logger.log_warning( "Ensure tenant setting is enabled for Service Principal auth" @@ -275,6 +291,7 @@ def __mask_token(scope): auth_data = { "logged_in": is_logged_in, + "auth_source": auth.get_identity_type() or "N/A", "account": upn, "principal_id": oid, "tenant_id": tid, diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 8d1979d2f..6a2e5c4ac 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -418,6 +418,52 @@ def set_managed_identity(self, client_id=None): } ) + def set_azure_cli(self, tenant_id=None): + """Configure Azure CLI as the authentication source.""" + self._set_auth_properties( + { + con.IDENTITY_TYPE: "azure_cli", + } + ) + if tenant_id: + self.set_tenant(tenant_id) + + def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: + """Acquire a token using Azure CLI's AzureCliCredential.""" + try: + from azure.identity import AzureCliCredential, CredentialUnavailableError + except ImportError: + raise FabricCLIError( + "Azure CLI auth requires the 'azure-identity' package. " + "Install it with: pip install azure-identity", + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + + tenant_id = self.get_tenant_id() + try: + credential = AzureCliCredential(tenant_id=tenant_id) if tenant_id else AzureCliCredential() + # AzureCliCredential.get_token expects scopes as positional args + azure_token = credential.get_token(scope[0]) + return { + "access_token": azure_token.token, + "expires_on": azure_token.expires_on, + } + except CredentialUnavailableError: + raise FabricCLIError( + "Azure CLI is not installed or not logged in. " + "Run 'az login' to authenticate, then retry.", + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + except Exception as e: + # Sanitize: never include token content in error messages + error_msg = str(e) + if "accessToken" in error_msg or "token" in error_msg.lower(): + error_msg = "Azure CLI token acquisition failed. Run 'az account get-access-token' manually to diagnose." + raise FabricCLIError( + f"Azure CLI authentication failed: {error_msg}", + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + def print_auth_info(self): utils_ui.print_grey(json.dumps(self._get_auth_info(), indent=2)) @@ -480,6 +526,8 @@ def acquire_token(self, scope: list[str], interactive_renew=True) -> dict: ErrorMessages.Auth.managed_identity_token_failed(), status_code=con.ERROR_AUTHENTICATION_FAILED, ) + elif identity_type == "azure_cli": + token = self._acquire_token_from_azure_cli(scope) elif env_var_token: token = { "access_token": env_var_token, diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index 4fd00e7b9..49a19d992 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -63,7 +63,7 @@ AUTH_KEYS = { FAB_TENANT_ID: [], - IDENTITY_TYPE: ["user", "service_principal", "managed_identity"], + IDENTITY_TYPE: ["user", "service_principal", "managed_identity", "azure_cli"], } FAB_HOST_APP_ENV_VAR = "FAB_HOST_APP" diff --git a/src/fabric_cli/parsers/fab_auth_parser.py b/src/fabric_cli/parsers/fab_auth_parser.py index a908e09e5..d4e2738eb 100644 --- a/src/fabric_cli/parsers/fab_auth_parser.py +++ b/src/fabric_cli/parsers/fab_auth_parser.py @@ -30,6 +30,10 @@ def register_parser(subparsers: _SubParsersAction) -> None: "$ auth login\n", "# command_line mode", "$ fab auth login\n", + "# command_line mode using Azure CLI auth", + "$ fab auth login --azure-cli\n", + "# command_line mode using Azure CLI auth with specific tenant", + "$ fab auth login --azure-cli --tenant \n", "# command_line mode using service principal auth", "$ fab auth login -u -p --tenant \n", "# command_line mode using system assigned managed identity auth", @@ -84,6 +88,13 @@ def register_parser(subparsers: _SubParsersAction) -> None: required=False, help="Federated token that can be used for OIDC token exchange. Optional, only for service principal auth", ) + login_parser.add_argument( + "--azure-cli", + required=False, + action="store_true", + dest="azure_cli", + help="Use Azure CLI authentication (reuse existing 'az login' session)", + ) login_parser.usage = f"{utils_error_parser.get_usage_prog(login_parser)}" login_parser.set_defaults(func=lazy_command(_auth_module_path, 'init')) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py new file mode 100644 index 000000000..75095efed --- /dev/null +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -0,0 +1,262 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import time +from unittest.mock import MagicMock, patch + +import pytest + +from fabric_cli.core import fab_constant as con +from fabric_cli.core.fab_auth import FabAuth +from fabric_cli.core.fab_exceptions import FabricCLIError + + +@pytest.fixture(autouse=True) +def temp_dir_fixture(monkeypatch, tmp_path): + """Create a temporary directory and configure FabAuth to use it.""" + monkeypatch.setattr( + "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path) + ) + # Clear env vars that would interfere + monkeypatch.delenv("FAB_TOKEN", raising=False) + monkeypatch.delenv("FAB_TOKEN_ONELAKE", raising=False) + monkeypatch.delenv("FAB_TOKEN_AZURE", raising=False) + return str(tmp_path) + + +@pytest.fixture +def auth_instance(temp_dir_fixture): + """Get a fresh FabAuth instance.""" + # Clear singleton for test isolation + FabAuth.__wrapped__ = None # type: ignore + from fabric_cli.core import fab_auth as fab_auth_module + + if FabAuth in fab_auth_module.singleton.__wrapped__: # type: ignore + del fab_auth_module.singleton.__wrapped__[FabAuth] # type: ignore + return FabAuth() + + +@pytest.fixture +def fresh_auth(temp_dir_fixture, monkeypatch): + """Get a fresh FabAuth instance with singleton cleared.""" + # Reset singleton instances dict + import fabric_cli.core.fab_auth as auth_module + + # Access the closure variable of the singleton decorator + singleton_instances = auth_module.singleton.__code__.co_consts # noqa + # Simpler approach: just patch the module-level reference + monkeypatch.setattr( + "fabric_cli.core.fab_auth.FabAuth.__init__.__globals__", + {}, + raising=False, + ) + # Re-instantiate + auth = FabAuth.__new__(FabAuth) + auth.__init__() + return auth + + +class TestAzureCliIdentityType: + """Test that azure_cli is a valid identity type.""" + + def test_azure_cli_in_auth_keys(self): + """azure_cli should be in the allowed identity types.""" + assert "azure_cli" in con.AUTH_KEYS[con.IDENTITY_TYPE] + + def test_set_access_mode_accepts_azure_cli(self, temp_dir_fixture): + """set_access_mode should accept azure_cli without raising.""" + auth = FabAuth() + auth.set_access_mode("azure_cli") + assert auth.get_identity_type() == "azure_cli" + + def test_set_azure_cli_sets_identity_type(self, temp_dir_fixture): + """set_azure_cli should configure identity_type to azure_cli.""" + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_identity_type() == "azure_cli" + + def test_set_azure_cli_with_tenant(self, temp_dir_fixture): + """set_azure_cli with tenant_id should store the tenant.""" + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="test-tenant-id") + assert auth.get_tenant_id() == "test-tenant-id" + + +class TestAzureCliTokenAcquisition: + """Test token acquisition via AzureCliCredential.""" + + @patch("azure.identity.AzureCliCredential") + def test_acquire_token_dispatches_to_azure_cli( + self, mock_credential_class, temp_dir_fixture + ): + """acquire_token should use AzureCliCredential for azure_cli identity.""" + mock_token = MagicMock() + mock_token.token = "fake-token-123" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) + + assert result["access_token"] == "fake-token-123" + mock_credential.get_token.assert_called_once_with( + "https://api.fabric.microsoft.com/.default" + ) + + @patch("azure.identity.AzureCliCredential") + def test_acquire_token_from_azure_cli_success( + self, mock_credential_class, temp_dir_fixture + ): + """_acquire_token_from_azure_cli should return token dict on success.""" + mock_token = MagicMock() + mock_token.token = "az-cli-token-abc" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert result["access_token"] == "az-cli-token-abc" + mock_credential.get_token.assert_called_once_with( + "https://api.fabric.microsoft.com/.default" + ) + + @patch("azure.identity.AzureCliCredential") + def test_acquire_token_from_azure_cli_with_tenant( + self, mock_credential_class, temp_dir_fixture + ): + """_acquire_token_from_azure_cli should pass tenant_id to credential.""" + mock_token = MagicMock() + mock_token.token = "tenant-specific-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="my-tenant-id") + + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + mock_credential_class.assert_called_once_with(tenant_id="my-tenant-id") + + @patch("azure.identity.AzureCliCredential") + def test_acquire_token_from_azure_cli_credential_unavailable( + self, mock_credential_class, temp_dir_fixture + ): + """Should raise FabricCLIError when Azure CLI is not logged in.""" + from azure.identity import CredentialUnavailableError + + mock_credential = MagicMock() + mock_credential.get_token.side_effect = CredentialUnavailableError( + "Azure CLI not logged in" + ) + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert "not installed or not logged in" in str(exc_info.value) + + @patch( + "azure.identity.AzureCliCredential", + side_effect=ImportError("No module named 'azure.identity'"), + ) + def test_acquire_token_from_azure_cli_missing_package( + self, mock_import, temp_dir_fixture + ): + """Should raise FabricCLIError when azure-identity is not installed.""" + auth = FabAuth() + auth.set_access_mode("azure_cli") + + # Need to actually test the import failure path + with patch.dict("sys.modules", {"azure.identity": None}): + with patch( + "builtins.__import__", side_effect=ImportError("no azure.identity") + ): + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert "azure-identity" in str(exc_info.value) + + @patch("azure.identity.AzureCliCredential") + def test_acquire_token_sanitizes_error_messages( + self, mock_credential_class, temp_dir_fixture + ): + """Error messages should never contain token content.""" + mock_credential = MagicMock() + mock_credential.get_token.side_effect = Exception( + "Failed with accessToken: eyJ0eXAi..." + ) + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + # Should not contain the raw token + assert "eyJ0eXAi" not in str(exc_info.value) + assert "manually to diagnose" in str(exc_info.value) + + +class TestAzureCliScopeHandling: + """Test that different scopes are correctly passed to Azure CLI.""" + + @patch("azure.identity.AzureCliCredential") + def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): + """OneLake scope should be passed correctly.""" + mock_token = MagicMock() + mock_token.token = "storage-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) + + mock_credential.get_token.assert_called_once_with( + "https://storage.azure.com/.default" + ) + + @patch("azure.identity.AzureCliCredential") + def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): + """Azure management scope should be passed correctly.""" + mock_token = MagicMock() + mock_token.token = "mgmt-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + auth._acquire_token_from_azure_cli(con.SCOPE_AZURE_DEFAULT) + + mock_credential.get_token.assert_called_once_with( + "https://management.azure.com/.default" + ) From c0ac9591dbb9b6c376ba596aeb6a99697c5a9dd0 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 11:48:02 +0300 Subject: [PATCH 02/50] feat: production-ready Azure CLI auth hardening - Expand sanitization patterns (eyJ, Bearer, refresh_token, Authorization) - Auto-capture tenant from az account show at login - Tenant drift detection on every token acquisition - In-memory token caching by audience with 60s expiry buffer - Display tenant and auth mode at login and in auth status - Add 11 new tests (23 total) for drift, caching, sanitization Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 11 +- src/fabric_cli/core/fab_auth.py | 80 ++++++++- tests/test_core/test_fab_auth_azure_cli.py | 196 +++++++++++++++++++++ 3 files changed, 281 insertions(+), 6 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index ad3270679..846ffd6b3 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -35,6 +35,10 @@ def init(args: Namespace) -> Any: FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) Context().context = FabAuth().get_tenant() + tenant_id = FabAuth().get_tenant_id() or "unknown" + fab_ui.print_grey( + f"✓ Authenticated via Azure CLI (tenant: {tenant_id})" + ) elif args.identity: FabAuth().set_access_mode("managed_identity") @@ -282,16 +286,21 @@ def __mask_token(scope): # Check login status is_logged_in = fabric_secret != "N/A" + identity_type = auth.get_identity_type() or "N/A" login_status = ( "✓ Logged in to app.fabric.microsoft.com" if is_logged_in else "✗ Not logged in to app.fabric.microsoft.com" ) fab_ui.print_grey(login_status) + if identity_type == "azure_cli" and is_logged_in: + fab_ui.print_grey( + f" Auth mode: Azure CLI (tenant: {tid})" + ) auth_data = { "logged_in": is_logged_in, - "auth_source": auth.get_identity_type() or "N/A", + "auth_source": identity_type, "account": upn, "principal_id": oid, "tenant_id": tid, diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 6a2e5c4ac..95b153ccf 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -50,6 +50,8 @@ def __init__(self): # Reset the auth info self.app: msal.ClientApplication = None self._auth_info = {} + # In-memory token cache for Azure CLI tokens (avoids repeated subprocess calls) + self._azure_cli_token_cache: dict[str, dict] = {} # Load the auth info and environment variables self._load_auth() @@ -419,7 +421,11 @@ def set_managed_identity(self, client_id=None): ) def set_azure_cli(self, tenant_id=None): - """Configure Azure CLI as the authentication source.""" + """Configure Azure CLI as the authentication source. + + If tenant_id is not provided, auto-captures the current tenant + from Azure CLI's active session via 'az account show'. + """ self._set_auth_properties( { con.IDENTITY_TYPE: "azure_cli", @@ -427,6 +433,37 @@ def set_azure_cli(self, tenant_id=None): ) if tenant_id: self.set_tenant(tenant_id) + else: + # Auto-capture tenant from active az session + captured_tenant = self._get_azure_cli_tenant() + if captured_tenant: + self.set_tenant(captured_tenant) + + def _get_azure_cli_tenant(self) -> Optional[str]: + """Query Azure CLI for the current tenant ID via 'az account show'.""" + import subprocess + + try: + result = subprocess.run( + ["az", "account", "show", "--query", "tenantId", "-o", "tsv"], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode == 0 and result.stdout.strip(): + return result.stdout.strip() + except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + pass + return None + + # Sensitive patterns for error message sanitization + _SENSITIVE_PATTERNS = [ + "accessToken", + "eyJ", + "Bearer", + "refresh_token", + "Authorization", + ] def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: """Acquire a token using Azure CLI's AzureCliCredential.""" @@ -439,15 +476,35 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) - tenant_id = self.get_tenant_id() + # Tenant drift check: compare stored tenant against current az session + stored_tenant = self.get_tenant_id() + if stored_tenant: + current_tenant = self._get_azure_cli_tenant() + if current_tenant and current_tenant != stored_tenant: + raise FabricCLIError( + f"Tenant mismatch: Fabric CLI is pinned to tenant '{stored_tenant}' " + f"but Azure CLI is now logged into tenant '{current_tenant}'. " + "Run 'fab auth login --azure-cli' to re-authenticate.", + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + + # Check in-memory cache first + cache_key = scope[0] if scope else "" + cached = self._get_cached_azure_cli_token(cache_key) + if cached: + return cached + try: - credential = AzureCliCredential(tenant_id=tenant_id) if tenant_id else AzureCliCredential() + credential = AzureCliCredential(tenant_id=stored_tenant) if stored_tenant else AzureCliCredential() # AzureCliCredential.get_token expects scopes as positional args azure_token = credential.get_token(scope[0]) - return { + token_result = { "access_token": azure_token.token, "expires_on": azure_token.expires_on, } + # Cache the token + self._cache_azure_cli_token(cache_key, token_result) + return token_result except CredentialUnavailableError: raise FabricCLIError( "Azure CLI is not installed or not logged in. " @@ -457,13 +514,26 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: except Exception as e: # Sanitize: never include token content in error messages error_msg = str(e) - if "accessToken" in error_msg or "token" in error_msg.lower(): + if any(p.lower() in error_msg.lower() for p in self._SENSITIVE_PATTERNS): error_msg = "Azure CLI token acquisition failed. Run 'az account get-access-token' manually to diagnose." raise FabricCLIError( f"Azure CLI authentication failed: {error_msg}", status_code=con.ERROR_AUTHENTICATION_FAILED, ) + def _get_cached_azure_cli_token(self, cache_key: str) -> Optional[dict]: + """Return cached token if it exists and is not near expiry (60s buffer).""" + import time + + cached = self._azure_cli_token_cache.get(cache_key) + if cached and cached.get("expires_on", 0) > time.time() + 60: + return cached + return None + + def _cache_azure_cli_token(self, cache_key: str, token: dict) -> None: + """Cache a token by audience key.""" + self._azure_cli_token_cache[cache_key] = token + def print_auth_info(self): utils_ui.print_grey(json.dumps(self._get_auth_info(), indent=2)) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 75095efed..d11d011c2 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -83,6 +83,32 @@ def test_set_azure_cli_with_tenant(self, temp_dir_fixture): auth.set_azure_cli(tenant_id="test-tenant-id") assert auth.get_tenant_id() == "test-tenant-id" + @patch("subprocess.run") + def test_set_azure_cli_auto_captures_tenant( + self, mock_run, temp_dir_fixture + ): + """set_azure_cli without tenant_id should auto-capture from az account show.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="auto-captured-tenant-id\n" + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_tenant_id() == "auto-captured-tenant-id" + + @patch("subprocess.run") + def test_set_azure_cli_explicit_tenant_overrides_auto( + self, mock_run, temp_dir_fixture + ): + """Explicit tenant_id should be used even if az has a different one.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="az-tenant\n" + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="explicit-tenant") + assert auth.get_tenant_id() == "explicit-tenant" + class TestAzureCliTokenAcquisition: """Test token acquisition via AzureCliCredential.""" @@ -102,6 +128,8 @@ def test_acquire_token_dispatches_to_azure_cli( auth = FabAuth() auth.set_access_mode("azure_cli") + # Clear cache for clean test + auth._azure_cli_token_cache.clear() result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) @@ -125,6 +153,7 @@ def test_acquire_token_from_azure_cli_success( auth = FabAuth() auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -149,6 +178,7 @@ def test_acquire_token_from_azure_cli_with_tenant( auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="my-tenant-id") + auth._azure_cli_token_cache.clear() auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -169,6 +199,7 @@ def test_acquire_token_from_azure_cli_credential_unavailable( auth = FabAuth() auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -209,6 +240,7 @@ def test_acquire_token_sanitizes_error_messages( auth = FabAuth() auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -217,6 +249,168 @@ def test_acquire_token_sanitizes_error_messages( assert "eyJ0eXAi" not in str(exc_info.value) assert "manually to diagnose" in str(exc_info.value) + @pytest.mark.parametrize( + "error_msg", + [ + "Error with Bearer token xyz", + "refresh_token expired", + "Authorization header invalid", + "eyJhbGciOiJSUzI1NiIsInR5cCI6", + ], + ) + @patch("azure.identity.AzureCliCredential") + def test_acquire_token_sanitizes_expanded_patterns( + self, mock_credential_class, error_msg, temp_dir_fixture + ): + """All sensitive patterns should be sanitized from error messages.""" + mock_credential = MagicMock() + mock_credential.get_token.side_effect = Exception(error_msg) + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert "manually to diagnose" in str(exc_info.value) + + +class TestAzureCliTenantDrift: + """Test tenant drift detection during token acquisition.""" + + @patch("azure.identity.AzureCliCredential") + @patch("subprocess.run") + def test_tenant_drift_blocks_token_acquisition( + self, mock_run, mock_credential_class, temp_dir_fixture + ): + """Should block when stored tenant differs from current az session.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="different-tenant\n" + ) + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="original-tenant") + auth._azure_cli_token_cache.clear() + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert "Tenant mismatch" in str(exc_info.value) + assert "original-tenant" in str(exc_info.value) + + @patch("azure.identity.AzureCliCredential") + @patch("subprocess.run") + def test_tenant_match_allows_token_acquisition( + self, mock_run, mock_credential_class, temp_dir_fixture + ): + """Should allow when stored tenant matches current az session.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="same-tenant\n" + ) + mock_token = MagicMock() + mock_token.token = "valid-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="same-tenant") + auth._azure_cli_token_cache.clear() + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "valid-token" + + +class TestAzureCliTokenCache: + """Test in-memory token caching for Azure CLI tokens.""" + + @patch("azure.identity.AzureCliCredential") + def test_cached_token_avoids_subprocess( + self, mock_credential_class, temp_dir_fixture + ): + """Second call with same scope should use cache, not subprocess.""" + mock_token = MagicMock() + mock_token.token = "cached-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() + + result1 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + result2 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert result1["access_token"] == "cached-token" + assert result2["access_token"] == "cached-token" + # get_token should only be called once (second call uses cache) + mock_credential.get_token.assert_called_once() + + @patch("azure.identity.AzureCliCredential") + def test_expired_cache_triggers_refresh( + self, mock_credential_class, temp_dir_fixture + ): + """Expired cached token should trigger a new subprocess call.""" + mock_token = MagicMock() + mock_token.token = "fresh-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + # Pre-populate cache with expired token + auth._azure_cli_token_cache.clear() + auth._azure_cli_token_cache[con.SCOPE_FABRIC_DEFAULT[0]] = { + "access_token": "old-token", + "expires_on": int(time.time()) - 10, # already expired + } + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "fresh-token" + mock_credential.get_token.assert_called_once() + + @patch("azure.identity.AzureCliCredential") + def test_different_scopes_cached_separately( + self, mock_credential_class, temp_dir_fixture + ): + """Different scopes should have separate cache entries.""" + call_count = 0 + + def make_token(*args): + nonlocal call_count + call_count += 1 + token = MagicMock() + token.token = f"token-{call_count}" + token.expires_on = int(time.time()) + 3600 + return token + + mock_credential = MagicMock() + mock_credential.get_token.side_effect = make_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() + + r1 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + r2 = auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) + + assert r1["access_token"] == "token-1" + assert r2["access_token"] == "token-2" + assert mock_credential.get_token.call_count == 2 + class TestAzureCliScopeHandling: """Test that different scopes are correctly passed to Azure CLI.""" @@ -234,6 +428,7 @@ def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): auth = FabAuth() auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) @@ -254,6 +449,7 @@ def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): auth = FabAuth() auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() auth._acquire_token_from_azure_cli(con.SCOPE_AZURE_DEFAULT) From 8d48a1a20df6a8b7aa4970b24e29d517b759e1cd Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 12:07:51 +0300 Subject: [PATCH 03/50] perf: validate only Fabric scope at Azure CLI login Defer OneLake and Azure management token acquisition to first use, matching the lazy approach. Tokens are cached in-memory after first call. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 846ffd6b3..3084c32d9 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -32,8 +32,6 @@ def init(args: Namespace) -> Any: FabAuth().set_access_mode("azure_cli", args.tenant) FabAuth().set_azure_cli(args.tenant) FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" fab_ui.print_grey( From aef875ad7536c643af3216cada987b46c6543f7c Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 14:39:20 +0300 Subject: [PATCH 04/50] fix: restore 3-scope validation for Azure CLI login consistency All auth modes validate Fabric, OneLake, and Azure scopes at login. In-memory caching ensures no redundant subprocess calls at runtime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 3084c32d9..846ffd6b3 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -32,6 +32,8 @@ def init(args: Namespace) -> Any: FabAuth().set_access_mode("azure_cli", args.tenant) FabAuth().set_azure_cli(args.tenant) FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) + FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) + FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" fab_ui.print_grey( From a2c5de66fbc4d6b5676fc136917b17ee1782fd39 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 15:37:50 +0300 Subject: [PATCH 05/50] fix: add tenant display to interactive Azure CLI login path Ensures both --azure-cli flag and interactive menu selection show the same confirmation message with tenant ID. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 846ffd6b3..5807f9ba8 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -93,6 +93,10 @@ def init(args: Namespace) -> Any: FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) Context().context = FabAuth().get_tenant() + tenant_id = FabAuth().get_tenant_id() or "unknown" + fab_ui.print_grey( + f"✓ Authenticated via Azure CLI (tenant: {tenant_id})" + ) elif selected_auth.startswith("Service principal authentication"): fab_logger.log_warning( "Ensure tenant setting is enabled for Service Principal auth" From 7c499a254e15c123e009bb1b0e640f9060bbca7b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 15:49:36 +0300 Subject: [PATCH 06/50] update --- src/fabric_cli/commands/auth/fab_auth.py | 44 +++++++++--------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 5807f9ba8..6e2882faa 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -16,7 +16,7 @@ def init(args: Namespace) -> Any: auth_options = [ "Interactive with a web browser", - "Azure CLI (reuse existing 'az login' session)", + "Azure CLI (existing 'az login' session)", "Service principal authentication with secret", "Service principal authentication with certificate", "Service principal authentication with federated credential", @@ -31,21 +31,15 @@ def init(args: Namespace) -> Any: if getattr(args, "azure_cli", False): FabAuth().set_access_mode("azure_cli", args.tenant) FabAuth().set_azure_cli(args.tenant) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" - fab_ui.print_grey( - f"✓ Authenticated via Azure CLI (tenant: {tenant_id})" - ) + fab_ui.print_grey(f"✓ Authenticated via Azure CLI (tenant: {tenant_id})") elif args.identity: FabAuth().set_access_mode("managed_identity") FabAuth().set_managed_identity(args.username) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() elif any([args.username, args.password]): @@ -67,9 +61,7 @@ def init(args: Namespace) -> Any: FabAuth().set_spn(args.username, password=args.password) elif args.federated_token: FabAuth().set_spn(args.username, client_assertion=args.federated_token) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() else: selected_auth = fab_ui.prompt_select_item( @@ -82,16 +74,12 @@ def init(args: Namespace) -> Any: try: if selected_auth == "Interactive with a web browser": FabAuth().set_access_mode("user", args.tenant) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() elif selected_auth.startswith("Azure CLI"): FabAuth().set_access_mode("azure_cli", args.tenant) FabAuth().set_azure_cli(args.tenant) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" fab_ui.print_grey( @@ -198,9 +186,7 @@ def init(args: Namespace) -> Any: FabAuth().set_spn(client_id, password=client_secret) elif federated_token: FabAuth().set_spn(client_id, client_assertion=federated_token) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() elif selected_auth == "Managed identity authentication": fab_logger.log_warning( @@ -215,9 +201,7 @@ def init(args: Namespace) -> Any: FabAuth().set_access_mode("managed_identity") FabAuth().set_managed_identity(client_id) - FabAuth().get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) - FabAuth().get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) + _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() except KeyboardInterrupt: @@ -298,9 +282,7 @@ def __mask_token(scope): ) fab_ui.print_grey(login_status) if identity_type == "azure_cli" and is_logged_in: - fab_ui.print_grey( - f" Auth mode: Azure CLI (tenant: {tid})" - ) + fab_ui.print_grey(f" Auth mode: Azure CLI (tenant: {tid})") auth_data = { "logged_in": is_logged_in, @@ -321,3 +303,9 @@ def _get_token_info_from_bearer_token(bearer_token: str) -> Optional[dict[str, s return FabAuth()._get_claims_from_token( bearer_token, ["upn", "oid", "tid", "appid"] ) + + +def _acquire_default_access_tokens(auth: FabAuth) -> None: + auth.get_access_token(scope=fab_constant.SCOPE_FABRIC_DEFAULT) + auth.get_access_token(scope=fab_constant.SCOPE_ONELAKE_DEFAULT) + auth.get_access_token(scope=fab_constant.SCOPE_AZURE_DEFAULT) From a0215d27cedba11d0f9f0c31072c7b5e375ecd2d Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 16:21:25 +0300 Subject: [PATCH 07/50] perf: cache az account show result for 30s to avoid repeated subprocess calls During login, _get_azure_cli_tenant() was called 4 times (auto-capture + 3 drift checks). Now caches for 30s, reducing to 1 subprocess call. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 20 ++++++++++++++++++-- tests/test_core/test_fab_auth_azure_cli.py | 5 +++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 95b153ccf..4c3b277ba 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -440,8 +440,22 @@ def set_azure_cli(self, tenant_id=None): self.set_tenant(captured_tenant) def _get_azure_cli_tenant(self) -> Optional[str]: - """Query Azure CLI for the current tenant ID via 'az account show'.""" + """Query Azure CLI for the current tenant ID via 'az account show'. + + Caches the result for 30 seconds to avoid repeated subprocess calls + during multi-scope login flows. + """ import subprocess + import time + + # Return cached result if fresh (within 30s) + if ( + hasattr(self, "_cached_az_tenant") + and self._cached_az_tenant is not None + and hasattr(self, "_cached_az_tenant_time") + and time.time() - self._cached_az_tenant_time < 30 + ): + return self._cached_az_tenant try: result = subprocess.run( @@ -451,7 +465,9 @@ def _get_azure_cli_tenant(self) -> Optional[str]: timeout=10, ) if result.returncode == 0 and result.stdout.strip(): - return result.stdout.strip() + self._cached_az_tenant = result.stdout.strip() + self._cached_az_tenant_time = time.time() + return self._cached_az_tenant except (subprocess.TimeoutExpired, FileNotFoundError, OSError): pass return None diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index d11d011c2..33a49c080 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -21,6 +21,11 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.delenv("FAB_TOKEN", raising=False) monkeypatch.delenv("FAB_TOKEN_ONELAKE", raising=False) monkeypatch.delenv("FAB_TOKEN_AZURE", raising=False) + # Clear singleton caches between tests + auth = FabAuth() + auth._azure_cli_token_cache.clear() + if hasattr(auth, "_cached_az_tenant"): + auth._cached_az_tenant = None return str(tmp_path) From 250e8fbbb3fe0a97786b95e2ca1aed783dcf86b9 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 16:30:52 +0300 Subject: [PATCH 08/50] fix: address tenant cache review feedback - Initialize cache fields in __init__ (predictable object shape) - Use time.monotonic() for TTL (immune to clock changes) - Reduce TTL from 30s to 10s (safer drift detection window) - Clear caches on logout() (singleton survives auth resets) - Force refresh at login (prevents stale cache rejecting re-login) - Add debug logging for subprocess failures - Add tests for logout invalidation and forced refresh at login Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 31 ++++++++++----- tests/test_core/test_fab_auth_azure_cli.py | 45 +++++++++++++++++++++- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 4c3b277ba..d2233ccc7 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -52,6 +52,9 @@ def __init__(self): self._auth_info = {} # In-memory token cache for Azure CLI tokens (avoids repeated subprocess calls) self._azure_cli_token_cache: dict[str, dict] = {} + # Cached tenant ID from az account show (avoids repeated subprocess calls) + self._cached_az_tenant: Optional[str] = None + self._cached_az_tenant_time: float = 0.0 # Load the auth info and environment variables self._load_auth() @@ -425,6 +428,7 @@ def set_azure_cli(self, tenant_id=None): If tenant_id is not provided, auto-captures the current tenant from Azure CLI's active session via 'az account show'. + Always forces a fresh query (bypasses cache) since this is a login action. """ self._set_auth_properties( { @@ -434,26 +438,28 @@ def set_azure_cli(self, tenant_id=None): if tenant_id: self.set_tenant(tenant_id) else: - # Auto-capture tenant from active az session - captured_tenant = self._get_azure_cli_tenant() + # Force refresh at login to avoid stale cached tenant + captured_tenant = self._get_azure_cli_tenant(force_refresh=True) if captured_tenant: self.set_tenant(captured_tenant) - def _get_azure_cli_tenant(self) -> Optional[str]: + def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: """Query Azure CLI for the current tenant ID via 'az account show'. - Caches the result for 30 seconds to avoid repeated subprocess calls - during multi-scope login flows. + Caches the result for 10 seconds to avoid repeated subprocess calls + during multi-scope token acquisition flows. + + Args: + force_refresh: If True, bypass the cache and query az directly. """ import subprocess import time - # Return cached result if fresh (within 30s) + # Return cached result if fresh (within 10s) and not forced if ( - hasattr(self, "_cached_az_tenant") + not force_refresh and self._cached_az_tenant is not None - and hasattr(self, "_cached_az_tenant_time") - and time.time() - self._cached_az_tenant_time < 30 + and time.monotonic() - self._cached_az_tenant_time < 10 ): return self._cached_az_tenant @@ -466,7 +472,7 @@ def _get_azure_cli_tenant(self) -> Optional[str]: ) if result.returncode == 0 and result.stdout.strip(): self._cached_az_tenant = result.stdout.strip() - self._cached_az_tenant_time = time.time() + self._cached_az_tenant_time = time.monotonic() return self._cached_az_tenant except (subprocess.TimeoutExpired, FileNotFoundError, OSError): pass @@ -680,6 +686,11 @@ def logout(self): self.app = None + # Clear Azure CLI caches + self._azure_cli_token_cache.clear() + self._cached_az_tenant = None + self._cached_az_tenant_time = 0.0 + if os.path.exists(self.cache_file): os.remove(self.cache_file) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 33a49c080..747d64293 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -24,8 +24,8 @@ def temp_dir_fixture(monkeypatch, tmp_path): # Clear singleton caches between tests auth = FabAuth() auth._azure_cli_token_cache.clear() - if hasattr(auth, "_cached_az_tenant"): - auth._cached_az_tenant = None + auth._cached_az_tenant = None + auth._cached_az_tenant_time = 0.0 return str(tmp_path) @@ -461,3 +461,44 @@ def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): mock_credential.get_token.assert_called_once_with( "https://management.azure.com/.default" ) + + +class TestAzureCliCacheInvalidation: + """Test cache invalidation on logout and forced refresh at login.""" + + @patch("subprocess.run") + def test_logout_clears_tenant_cache(self, mock_run, temp_dir_fixture): + """logout() should clear the cached tenant.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="cached-tenant\n" + ) + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth._cached_az_tenant == "cached-tenant" + + auth.logout() + assert auth._cached_az_tenant is None + assert auth._cached_az_tenant_time == 0.0 + assert auth._azure_cli_token_cache == {} + + @patch("subprocess.run") + def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): + """set_azure_cli should bypass cache and query az fresh.""" + # First call returns tenant-A + mock_run.return_value = MagicMock( + returncode=0, stdout="tenant-A\n" + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_tenant_id() == "tenant-A" + + # Simulate user switching az tenant, then re-logging in + mock_run.return_value = MagicMock( + returncode=0, stdout="tenant-B\n" + ) + auth.set_access_mode("azure_cli") + auth.set_azure_cli() # Should force refresh, get tenant-B + assert auth.get_tenant_id() == "tenant-B" From 28bf323f33a821fb9bcdb711df6ad3543411ee32 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Tue, 11 Aug 2026 16:46:16 +0300 Subject: [PATCH 09/50] refactor: address code review feedback on Azure CLI auth - Move subprocess and time imports to module level - Define TTL as named constant (_AZURE_CLI_TENANT_CACHE_TTL_SECONDS) - Format AzureCliCredential expression across multiple lines - Fix set_azure_cli ordering: set tenant before identity_type to survive logout triggered by tenant change - Add tests: single subprocess across 3 login scopes, identity_type preserved after tenant change - Clean singleton state in test fixture for isolation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 34 ++++++++------ tests/test_core/test_fab_auth_azure_cli.py | 53 ++++++++++++++++++++++ 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index d2233ccc7..2ad7bf1f5 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -3,6 +3,8 @@ import json import os +import subprocess +import time import uuid from binascii import hexlify from typing import Any, NamedTuple, Optional @@ -28,6 +30,9 @@ from fabric_cli.utils import fab_ui as utils_ui +_AZURE_CLI_TENANT_CACHE_TTL_SECONDS = 10 + + def singleton(class_): instances = {} @@ -430,11 +435,7 @@ def set_azure_cli(self, tenant_id=None): from Azure CLI's active session via 'az account show'. Always forces a fresh query (bypasses cache) since this is a login action. """ - self._set_auth_properties( - { - con.IDENTITY_TYPE: "azure_cli", - } - ) + # Set tenant first — set_tenant() may call logout() which clears auth info if tenant_id: self.set_tenant(tenant_id) else: @@ -442,24 +443,27 @@ def set_azure_cli(self, tenant_id=None): captured_tenant = self._get_azure_cli_tenant(force_refresh=True) if captured_tenant: self.set_tenant(captured_tenant) + # Set identity_type after tenant to survive any logout triggered by tenant change + self._set_auth_properties( + { + con.IDENTITY_TYPE: "azure_cli", + } + ) def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: """Query Azure CLI for the current tenant ID via 'az account show'. - Caches the result for 10 seconds to avoid repeated subprocess calls + Caches the result to avoid repeated subprocess calls during multi-scope token acquisition flows. Args: force_refresh: If True, bypass the cache and query az directly. """ - import subprocess - import time - - # Return cached result if fresh (within 10s) and not forced + # Return cached result if fresh and not forced if ( not force_refresh and self._cached_az_tenant is not None - and time.monotonic() - self._cached_az_tenant_time < 10 + and time.monotonic() - self._cached_az_tenant_time < _AZURE_CLI_TENANT_CACHE_TTL_SECONDS ): return self._cached_az_tenant @@ -517,7 +521,11 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: return cached try: - credential = AzureCliCredential(tenant_id=stored_tenant) if stored_tenant else AzureCliCredential() + credential = ( + AzureCliCredential(tenant_id=stored_tenant) + if stored_tenant + else AzureCliCredential() + ) # AzureCliCredential.get_token expects scopes as positional args azure_token = credential.get_token(scope[0]) token_result = { @@ -545,8 +553,6 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: def _get_cached_azure_cli_token(self, cache_key: str) -> Optional[dict]: """Return cached token if it exists and is not near expiry (60s buffer).""" - import time - cached = self._azure_cli_token_cache.get(cache_key) if cached and cached.get("expires_on", 0) > time.time() + 60: return cached diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 747d64293..e4358df5c 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -26,6 +26,7 @@ def temp_dir_fixture(monkeypatch, tmp_path): auth._azure_cli_token_cache.clear() auth._cached_az_tenant = None auth._cached_az_tenant_time = 0.0 + auth._auth_info = {} return str(tmp_path) @@ -502,3 +503,55 @@ def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): auth.set_access_mode("azure_cli") auth.set_azure_cli() # Should force refresh, get tenant-B assert auth.get_tenant_id() == "tenant-B" + + @patch("azure.identity.AzureCliCredential") + @patch("subprocess.run") + def test_single_subprocess_across_three_login_scopes( + self, mock_run, mock_credential_class, temp_dir_fixture + ): + """Login should call az account show only once across 3 scope validations.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="login-tenant\n" + ) + + mock_token = MagicMock() + mock_token.token = "login-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() # 1 subprocess call (force_refresh) + + # 3 scope validations — each calls drift check, but cache should hit + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) + auth._acquire_token_from_azure_cli(con.SCOPE_AZURE_DEFAULT) + + # az account show called once at login, cached for drift checks + assert mock_run.call_count == 1 + + @patch("subprocess.run") + def test_identity_type_preserved_after_tenant_change( + self, mock_run, temp_dir_fixture + ): + """identity_type should remain azure_cli after tenant changes.""" + mock_run.return_value = MagicMock( + returncode=0, stdout="tenant-A\n" + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_identity_type() == "azure_cli" + + # Re-login with different tenant + mock_run.return_value = MagicMock( + returncode=0, stdout="tenant-B\n" + ) + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_identity_type() == "azure_cli" + assert auth.get_tenant_id() == "tenant-B" From 277dd0dd7a5424020a7b200baeccffedc37c3811 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 11:18:20 +0300 Subject: [PATCH 10/50] refactor: move Azure CLI error messages to ErrorMessages.Auth Route hard-coded Azure CLI auth error strings through the centralized ErrorMessages.Auth class for consistency with the rest of FabAuth. Added: azure_cli_missing_azure_identity, azure_cli_tenant_mismatch, azure_cli_not_available, azure_cli_auth_failed, azure_cli_token_acquisition_failed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 14 +++++--------- src/fabric_cli/errors/auth.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 2ad7bf1f5..92852651c 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -497,8 +497,7 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: from azure.identity import AzureCliCredential, CredentialUnavailableError except ImportError: raise FabricCLIError( - "Azure CLI auth requires the 'azure-identity' package. " - "Install it with: pip install azure-identity", + ErrorMessages.Auth.azure_cli_missing_azure_identity(), status_code=con.ERROR_AUTHENTICATION_FAILED, ) @@ -508,9 +507,7 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: current_tenant = self._get_azure_cli_tenant() if current_tenant and current_tenant != stored_tenant: raise FabricCLIError( - f"Tenant mismatch: Fabric CLI is pinned to tenant '{stored_tenant}' " - f"but Azure CLI is now logged into tenant '{current_tenant}'. " - "Run 'fab auth login --azure-cli' to re-authenticate.", + ErrorMessages.Auth.azure_cli_tenant_mismatch(stored_tenant, current_tenant), status_code=con.ERROR_AUTHENTICATION_FAILED, ) @@ -537,17 +534,16 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: return token_result except CredentialUnavailableError: raise FabricCLIError( - "Azure CLI is not installed or not logged in. " - "Run 'az login' to authenticate, then retry.", + ErrorMessages.Auth.azure_cli_not_available(), status_code=con.ERROR_AUTHENTICATION_FAILED, ) except Exception as e: # Sanitize: never include token content in error messages error_msg = str(e) if any(p.lower() in error_msg.lower() for p in self._SENSITIVE_PATTERNS): - error_msg = "Azure CLI token acquisition failed. Run 'az account get-access-token' manually to diagnose." + error_msg = ErrorMessages.Auth.azure_cli_token_acquisition_failed() raise FabricCLIError( - f"Azure CLI authentication failed: {error_msg}", + ErrorMessages.Auth.azure_cli_auth_failed(error_msg), status_code=con.ERROR_AUTHENTICATION_FAILED, ) diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index e068b7d44..29f4a0873 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -119,3 +119,36 @@ def cert_read_failed(error: str) -> str: @staticmethod def only_supported_with_user_authentication() -> str: return "This operation is only supported with user authentication" + + @staticmethod + def azure_cli_missing_azure_identity() -> str: + return ( + "Azure CLI auth requires the 'azure-identity' package. " + "Install it with: pip install azure-identity" + ) + + @staticmethod + def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str: + return ( + f"Tenant mismatch: Fabric CLI is pinned to tenant '{stored_tenant}' " + f"but Azure CLI is now logged into tenant '{current_tenant}'. " + "Run 'fab auth login --azure-cli' to re-authenticate." + ) + + @staticmethod + def azure_cli_not_available() -> str: + return ( + "Azure CLI is not installed or not logged in. " + "Run 'az login' to authenticate, then retry." + ) + + @staticmethod + def azure_cli_auth_failed(error_msg: str) -> str: + return f"Azure CLI authentication failed: {error_msg}" + + @staticmethod + def azure_cli_token_acquisition_failed() -> str: + return ( + "Azure CLI token acquisition failed. " + "Run 'az account get-access-token' manually to diagnose." + ) From 8ed989268caeccb8d199763cbee374167778c96e Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 11:19:28 +0300 Subject: [PATCH 11/50] fix wording --- src/fabric_cli/core/fab_auth.py | 8 ++++---- src/fabric_cli/parsers/fab_auth_parser.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 2ad7bf1f5..20e827a56 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -29,7 +29,6 @@ from fabric_cli.errors import ErrorMessages from fabric_cli.utils import fab_ui as utils_ui - _AZURE_CLI_TENANT_CACHE_TTL_SECONDS = 10 @@ -55,9 +54,9 @@ def __init__(self): # Reset the auth info self.app: msal.ClientApplication = None self._auth_info = {} - # In-memory token cache for Azure CLI tokens (avoids repeated subprocess calls) + # In-memory token cache for Azure CLI tokens self._azure_cli_token_cache: dict[str, dict] = {} - # Cached tenant ID from az account show (avoids repeated subprocess calls) + # Cached tenant ID from az account show self._cached_az_tenant: Optional[str] = None self._cached_az_tenant_time: float = 0.0 @@ -463,7 +462,8 @@ def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: if ( not force_refresh and self._cached_az_tenant is not None - and time.monotonic() - self._cached_az_tenant_time < _AZURE_CLI_TENANT_CACHE_TTL_SECONDS + and time.monotonic() - self._cached_az_tenant_time + < _AZURE_CLI_TENANT_CACHE_TTL_SECONDS ): return self._cached_az_tenant diff --git a/src/fabric_cli/parsers/fab_auth_parser.py b/src/fabric_cli/parsers/fab_auth_parser.py index d4e2738eb..c1f033809 100644 --- a/src/fabric_cli/parsers/fab_auth_parser.py +++ b/src/fabric_cli/parsers/fab_auth_parser.py @@ -93,11 +93,11 @@ def register_parser(subparsers: _SubParsersAction) -> None: required=False, action="store_true", dest="azure_cli", - help="Use Azure CLI authentication (reuse existing 'az login' session)", + help="Use Azure CLI authentication (existing 'az login' session)", ) login_parser.usage = f"{utils_error_parser.get_usage_prog(login_parser)}" - login_parser.set_defaults(func=lazy_command(_auth_module_path, 'init')) + login_parser.set_defaults(func=lazy_command(_auth_module_path, "init")) # Subcommand for 'logout' logout_examples = [ @@ -115,7 +115,7 @@ def register_parser(subparsers: _SubParsersAction) -> None: ) logout_parser.usage = f"{utils_error_parser.get_usage_prog(logout_parser)}" - logout_parser.set_defaults(func=lazy_command(_auth_module_path, 'logout')) + logout_parser.set_defaults(func=lazy_command(_auth_module_path, "logout")) # Subcommand for 'status' status_examples = [ @@ -132,7 +132,7 @@ def register_parser(subparsers: _SubParsersAction) -> None: ) status_parser.usage = f"{utils_error_parser.get_usage_prog(status_parser)}" - status_parser.set_defaults(func=lazy_command(_auth_module_path, 'status')) + status_parser.set_defaults(func=lazy_command(_auth_module_path, "status")) def show_help(args: Namespace) -> None: From cb334ebf4ec13536b283406c971889523748f052 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 11:38:57 +0300 Subject: [PATCH 12/50] refactor: replace denylist sanitization with SDK exception allowlist Replace _SENSITIVE_PATTERNS denylist with an allowlist approach: - SDK exceptions (ClientAuthenticationError, HttpResponseError) surface their pre-sanitized messages for diagnostic value - All other exceptions return a safe generic message - Removes _SENSITIVE_PATTERNS constant (no longer needed) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 17 ++++------- tests/test_core/test_fab_auth_azure_cli.py | 33 ++++++++-------------- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 3181cca8d..06ce6cc31 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -482,15 +482,6 @@ def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: pass return None - # Sensitive patterns for error message sanitization - _SENSITIVE_PATTERNS = [ - "accessToken", - "eyJ", - "Bearer", - "refresh_token", - "Authorization", - ] - def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: """Acquire a token using Azure CLI's AzureCliCredential.""" try: @@ -538,9 +529,11 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) except Exception as e: - # Sanitize: never include token content in error messages - error_msg = str(e) - if any(p.lower() in error_msg.lower() for p in self._SENSITIVE_PATTERNS): + # Allowlist: SDK exceptions are pre-sanitized by azure-identity; + # unknown exceptions get a safe generic message. + if type(e).__name__ in ("ClientAuthenticationError", "HttpResponseError"): + error_msg = str(e) + else: error_msg = ErrorMessages.Auth.azure_cli_token_acquisition_failed() raise FabricCLIError( ErrorMessages.Auth.azure_cli_auth_failed(error_msg), diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index e4358df5c..5cd585cb8 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -234,14 +234,13 @@ def test_acquire_token_from_azure_cli_missing_package( assert "azure-identity" in str(exc_info.value) @patch("azure.identity.AzureCliCredential") - def test_acquire_token_sanitizes_error_messages( + def test_sdk_exception_surfaces_message( self, mock_credential_class, temp_dir_fixture ): - """Error messages should never contain token content.""" + """SDK exceptions (pre-sanitized by azure-identity) surface their message.""" mock_credential = MagicMock() - mock_credential.get_token.side_effect = Exception( - "Failed with accessToken: eyJ0eXAi..." - ) + error = type("ClientAuthenticationError", (Exception,), {})("Tenant not found") + mock_credential.get_token.side_effect = error mock_credential_class.return_value = mock_credential auth = FabAuth() @@ -251,26 +250,17 @@ def test_acquire_token_sanitizes_error_messages( with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - # Should not contain the raw token - assert "eyJ0eXAi" not in str(exc_info.value) - assert "manually to diagnose" in str(exc_info.value) + assert "Tenant not found" in str(exc_info.value) - @pytest.mark.parametrize( - "error_msg", - [ - "Error with Bearer token xyz", - "refresh_token expired", - "Authorization header invalid", - "eyJhbGciOiJSUzI1NiIsInR5cCI6", - ], - ) @patch("azure.identity.AzureCliCredential") - def test_acquire_token_sanitizes_expanded_patterns( - self, mock_credential_class, error_msg, temp_dir_fixture + def test_unknown_exception_returns_safe_message( + self, mock_credential_class, temp_dir_fixture ): - """All sensitive patterns should be sanitized from error messages.""" + """Non-SDK exceptions should always return a safe generic message.""" mock_credential = MagicMock() - mock_credential.get_token.side_effect = Exception(error_msg) + mock_credential.get_token.side_effect = RuntimeError( + "accessToken: eyJ0eXAi..." + ) mock_credential_class.return_value = mock_credential auth = FabAuth() @@ -280,6 +270,7 @@ def test_acquire_token_sanitizes_expanded_patterns( with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert "eyJ0eXAi" not in str(exc_info.value) assert "manually to diagnose" in str(exc_info.value) From da260d31697e56dd810e723ecf5c987cb0b1bc83 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 11:49:22 +0300 Subject: [PATCH 13/50] refactor: move azure-identity import to module level azure-identity is a required dependency (pyproject.toml), so the try/except ImportError guard is unnecessary. Move import to module level and remove the missing-package error message and test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 9 +--- src/fabric_cli/errors/auth.py | 7 --- tests/test_core/test_fab_auth_azure_cli.py | 51 +++++++--------------- 3 files changed, 16 insertions(+), 51 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 06ce6cc31..c1e395b4d 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -9,6 +9,7 @@ from binascii import hexlify from typing import Any, NamedTuple, Optional +from azure.identity import AzureCliCredential, CredentialUnavailableError import jwt import msal import requests @@ -484,14 +485,6 @@ def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: """Acquire a token using Azure CLI's AzureCliCredential.""" - try: - from azure.identity import AzureCliCredential, CredentialUnavailableError - except ImportError: - raise FabricCLIError( - ErrorMessages.Auth.azure_cli_missing_azure_identity(), - status_code=con.ERROR_AUTHENTICATION_FAILED, - ) - # Tenant drift check: compare stored tenant against current az session stored_tenant = self.get_tenant_id() if stored_tenant: diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index 29f4a0873..38dc44fe4 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -120,13 +120,6 @@ def cert_read_failed(error: str) -> str: def only_supported_with_user_authentication() -> str: return "This operation is only supported with user authentication" - @staticmethod - def azure_cli_missing_azure_identity() -> str: - return ( - "Azure CLI auth requires the 'azure-identity' package. " - "Install it with: pip install azure-identity" - ) - @staticmethod def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str: return ( diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 5cd585cb8..ece8709aa 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -119,7 +119,7 @@ def test_set_azure_cli_explicit_tenant_overrides_auto( class TestAzureCliTokenAcquisition: """Test token acquisition via AzureCliCredential.""" - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_acquire_token_dispatches_to_azure_cli( self, mock_credential_class, temp_dir_fixture ): @@ -144,7 +144,7 @@ def test_acquire_token_dispatches_to_azure_cli( "https://api.fabric.microsoft.com/.default" ) - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_acquire_token_from_azure_cli_success( self, mock_credential_class, temp_dir_fixture ): @@ -168,7 +168,7 @@ def test_acquire_token_from_azure_cli_success( "https://api.fabric.microsoft.com/.default" ) - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_acquire_token_from_azure_cli_with_tenant( self, mock_credential_class, temp_dir_fixture ): @@ -190,12 +190,12 @@ def test_acquire_token_from_azure_cli_with_tenant( mock_credential_class.assert_called_once_with(tenant_id="my-tenant-id") - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_acquire_token_from_azure_cli_credential_unavailable( self, mock_credential_class, temp_dir_fixture ): """Should raise FabricCLIError when Azure CLI is not logged in.""" - from azure.identity import CredentialUnavailableError + from fabric_cli.core.fab_auth import CredentialUnavailableError mock_credential = MagicMock() mock_credential.get_token.side_effect = CredentialUnavailableError( @@ -212,28 +212,7 @@ def test_acquire_token_from_azure_cli_credential_unavailable( assert "not installed or not logged in" in str(exc_info.value) - @patch( - "azure.identity.AzureCliCredential", - side_effect=ImportError("No module named 'azure.identity'"), - ) - def test_acquire_token_from_azure_cli_missing_package( - self, mock_import, temp_dir_fixture - ): - """Should raise FabricCLIError when azure-identity is not installed.""" - auth = FabAuth() - auth.set_access_mode("azure_cli") - - # Need to actually test the import failure path - with patch.dict("sys.modules", {"azure.identity": None}): - with patch( - "builtins.__import__", side_effect=ImportError("no azure.identity") - ): - with pytest.raises(FabricCLIError) as exc_info: - auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - - assert "azure-identity" in str(exc_info.value) - - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_sdk_exception_surfaces_message( self, mock_credential_class, temp_dir_fixture ): @@ -252,7 +231,7 @@ def test_sdk_exception_surfaces_message( assert "Tenant not found" in str(exc_info.value) - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_unknown_exception_returns_safe_message( self, mock_credential_class, temp_dir_fixture ): @@ -277,7 +256,7 @@ def test_unknown_exception_returns_safe_message( class TestAzureCliTenantDrift: """Test tenant drift detection during token acquisition.""" - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") @patch("subprocess.run") def test_tenant_drift_blocks_token_acquisition( self, mock_run, mock_credential_class, temp_dir_fixture @@ -298,7 +277,7 @@ def test_tenant_drift_blocks_token_acquisition( assert "Tenant mismatch" in str(exc_info.value) assert "original-tenant" in str(exc_info.value) - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") @patch("subprocess.run") def test_tenant_match_allows_token_acquisition( self, mock_run, mock_credential_class, temp_dir_fixture @@ -327,7 +306,7 @@ def test_tenant_match_allows_token_acquisition( class TestAzureCliTokenCache: """Test in-memory token caching for Azure CLI tokens.""" - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_cached_token_avoids_subprocess( self, mock_credential_class, temp_dir_fixture ): @@ -352,7 +331,7 @@ def test_cached_token_avoids_subprocess( # get_token should only be called once (second call uses cache) mock_credential.get_token.assert_called_once() - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_expired_cache_triggers_refresh( self, mock_credential_class, temp_dir_fixture ): @@ -378,7 +357,7 @@ def test_expired_cache_triggers_refresh( assert result["access_token"] == "fresh-token" mock_credential.get_token.assert_called_once() - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_different_scopes_cached_separately( self, mock_credential_class, temp_dir_fixture ): @@ -412,7 +391,7 @@ def make_token(*args): class TestAzureCliScopeHandling: """Test that different scopes are correctly passed to Azure CLI.""" - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): """OneLake scope should be passed correctly.""" mock_token = MagicMock() @@ -433,7 +412,7 @@ def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): "https://storage.azure.com/.default" ) - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): """Azure management scope should be passed correctly.""" mock_token = MagicMock() @@ -495,7 +474,7 @@ def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): auth.set_azure_cli() # Should force refresh, get tenant-B assert auth.get_tenant_id() == "tenant-B" - @patch("azure.identity.AzureCliCredential") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") @patch("subprocess.run") def test_single_subprocess_across_three_login_scopes( self, mock_run, mock_credential_class, temp_dir_fixture From 7705598ca10c58069342a04a7f512f65e44ae8d0 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 11:50:35 +0300 Subject: [PATCH 14/50] fix comment --- src/fabric_cli/core/fab_auth.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index c1e395b4d..02387a7be 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -9,10 +9,10 @@ from binascii import hexlify from typing import Any, NamedTuple, Optional -from azure.identity import AzureCliCredential, CredentialUnavailableError import jwt import msal import requests +from azure.identity import AzureCliCredential, CredentialUnavailableError from cryptography import x509 from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes, serialization @@ -491,7 +491,9 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: current_tenant = self._get_azure_cli_tenant() if current_tenant and current_tenant != stored_tenant: raise FabricCLIError( - ErrorMessages.Auth.azure_cli_tenant_mismatch(stored_tenant, current_tenant), + ErrorMessages.Auth.azure_cli_tenant_mismatch( + stored_tenant, current_tenant + ), status_code=con.ERROR_AUTHENTICATION_FAILED, ) @@ -522,8 +524,7 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) except Exception as e: - # Allowlist: SDK exceptions are pre-sanitized by azure-identity; - # unknown exceptions get a safe generic message. + # Allowlist: SDK exceptions are pre-sanitized by azure-identity; unknown exceptions get a safe generic message if type(e).__name__ in ("ClientAuthenticationError", "HttpResponseError"): error_msg = str(e) else: From 5be8a2a751ae4acf12d912c62a0cd53b0b1e1486 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 11:55:30 +0300 Subject: [PATCH 15/50] chore: remove unused fresh_auth fixture Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index ece8709aa..410081fab 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -42,26 +42,6 @@ def auth_instance(temp_dir_fixture): return FabAuth() -@pytest.fixture -def fresh_auth(temp_dir_fixture, monkeypatch): - """Get a fresh FabAuth instance with singleton cleared.""" - # Reset singleton instances dict - import fabric_cli.core.fab_auth as auth_module - - # Access the closure variable of the singleton decorator - singleton_instances = auth_module.singleton.__code__.co_consts # noqa - # Simpler approach: just patch the module-level reference - monkeypatch.setattr( - "fabric_cli.core.fab_auth.FabAuth.__init__.__globals__", - {}, - raising=False, - ) - # Re-instantiate - auth = FabAuth.__new__(FabAuth) - auth.__init__() - return auth - - class TestAzureCliIdentityType: """Test that azure_cli is a valid identity type.""" From 22ee7d967ed114bf952805d27999c651ba6f61c7 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 12:11:41 +0300 Subject: [PATCH 16/50] fix: add azure_cli to test args fixture to prevent MagicMock truthy leak MagicMock auto-creates truthy attributes for undefined keys, causing getattr(args, 'azure_cli', False) to always be truthy in test_auth.py. This made all 22 interactive/SPN/MI tests hit the Azure CLI branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_commands/test_auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_commands/test_auth.py b/tests/test_commands/test_auth.py index 0de502188..6d4eeaac3 100644 --- a/tests/test_commands/test_auth.py +++ b/tests/test_commands/test_auth.py @@ -971,6 +971,7 @@ def prepare_auth_args(args=None): "identity", "certificate", "federated_token", + "azure_cli", ] } ) From 40213f09257f1c94604a62be5f8bbbb7f893c003 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 12:28:12 +0300 Subject: [PATCH 17/50] add changelog --- .changes/unreleased/added-20260812-122446.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/added-20260812-122446.yaml diff --git a/.changes/unreleased/added-20260812-122446.yaml b/.changes/unreleased/added-20260812-122446.yaml new file mode 100644 index 000000000..a5c813ba9 --- /dev/null +++ b/.changes/unreleased/added-20260812-122446.yaml @@ -0,0 +1,6 @@ +kind: added +body: Add support for Azure CLI authentication source +time: 2026-08-12T12:24:46.8603823+03:00 +custom: + Author: shirasassoon + AuthorLink: https://github.com/shirasassoon From 3e09380ca9e705b49887a934a6876f1097792223 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 13:17:42 +0300 Subject: [PATCH 18/50] test: expand Azure CLI auth test coverage Core tests (30 total, +7 new): - Tenant discovery failures: nonzero return code, empty stdout, timeout, az not installed - TTL cache: hit before expiry, miss after expiry - Error status code assertion - Renamed test_cached_token_avoids_subprocess for clarity - Removed unused auth_instance fixture Command-level tests (+3 new in test_auth.py): - fab auth login --azure-cli - fab auth login --azure-cli --tenant - Interactive menu Azure CLI selection Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_commands/test_auth.py | 56 +++++++++++++ tests/test_core/test_fab_auth_azure_cli.py | 93 +++++++++++++++++++--- 2 files changed, 137 insertions(+), 12 deletions(-) diff --git a/tests/test_commands/test_auth.py b/tests/test_commands/test_auth.py index 6d4eeaac3..b0956cfbf 100644 --- a/tests/test_commands/test_auth.py +++ b/tests/test_commands/test_auth.py @@ -949,6 +949,62 @@ def test_init_when_user_cancels_the_prompt( assert_prompt_cancelled(capsys) +class TestAuthAzureCli: + """Command-level tests for Azure CLI auth paths.""" + + def test_init_with_azure_cli_flag(self, mock_fab_auth, mock_fab_context): + """fab auth login --azure-cli should set azure_cli mode.""" + args = prepare_auth_args({"azure_cli": True}) + + with patch.object( + mock_fab_auth["instance"], "set_azure_cli", MagicMock() + ): + result = fab_auth.init(args) + + mock_fab_auth_instance = mock_fab_auth.get("instance") + mock_fab_auth_instance.set_access_mode.assert_called_with( + "azure_cli", None + ) + assert result is True + + def test_init_with_azure_cli_flag_and_tenant( + self, mock_fab_auth, mock_fab_context + ): + """fab auth login --azure-cli --tenant should pass tenant.""" + args = prepare_auth_args({"azure_cli": True, "tenant": "my-tenant"}) + + with patch.object( + mock_fab_auth["instance"], "set_azure_cli", MagicMock() + ): + result = fab_auth.init(args) + + mock_fab_auth_instance = mock_fab_auth.get("instance") + mock_fab_auth_instance.set_access_mode.assert_called_with( + "azure_cli", "my-tenant" + ) + assert result is True + + def test_init_with_interactive_azure_cli_selection( + self, mock_fab_auth, mock_fab_context + ): + """Interactive menu Azure CLI selection should set azure_cli mode.""" + with patch( + "fabric_cli.utils.fab_ui.prompt_select_item", + return_value="Azure CLI (existing 'az login' session)", + ): + with patch.object( + mock_fab_auth["instance"], "set_azure_cli", MagicMock() + ): + args = prepare_auth_args() + result = fab_auth.init(args) + + mock_fab_auth_instance = mock_fab_auth.get("instance") + mock_fab_auth_instance.set_access_mode.assert_called_with( + "azure_cli", None + ) + assert result is True + + # Helpers diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 410081fab..1feb24d13 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import subprocess import time from unittest.mock import MagicMock, patch @@ -30,16 +31,6 @@ def temp_dir_fixture(monkeypatch, tmp_path): return str(tmp_path) -@pytest.fixture -def auth_instance(temp_dir_fixture): - """Get a fresh FabAuth instance.""" - # Clear singleton for test isolation - FabAuth.__wrapped__ = None # type: ignore - from fabric_cli.core import fab_auth as fab_auth_module - - if FabAuth in fab_auth_module.singleton.__wrapped__: # type: ignore - del fab_auth_module.singleton.__wrapped__[FabAuth] # type: ignore - return FabAuth() class TestAzureCliIdentityType: @@ -287,10 +278,10 @@ class TestAzureCliTokenCache: """Test in-memory token caching for Azure CLI tokens.""" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_cached_token_avoids_subprocess( + def test_cached_token_avoids_repeated_credential_calls( self, mock_credential_class, temp_dir_fixture ): - """Second call with same scope should use cache, not subprocess.""" + """Second call with same scope should use cache, not call get_token again.""" mock_token = MagicMock() mock_token.token = "cached-token" mock_token.expires_on = int(time.time()) + 3600 @@ -505,3 +496,81 @@ def test_identity_type_preserved_after_tenant_change( auth.set_azure_cli() assert auth.get_identity_type() == "azure_cli" assert auth.get_tenant_id() == "tenant-B" + + +class TestAzureCliTenantDiscoveryFailures: + """Test _get_azure_cli_tenant failure paths.""" + + @patch("subprocess.run") + def test_nonzero_return_code_returns_none(self, mock_run, temp_dir_fixture): + """Should return None when az account show fails.""" + mock_run.return_value = MagicMock(returncode=1, stdout="") + auth = FabAuth() + auth._cached_az_tenant = None + auth._cached_az_tenant_time = 0.0 + assert auth._get_azure_cli_tenant(force_refresh=True) is None + + @patch("subprocess.run") + def test_empty_stdout_returns_none(self, mock_run, temp_dir_fixture): + """Should return None when az returns empty stdout.""" + mock_run.return_value = MagicMock(returncode=0, stdout=" \n") + auth = FabAuth() + auth._cached_az_tenant = None + auth._cached_az_tenant_time = 0.0 + assert auth._get_azure_cli_tenant(force_refresh=True) is None + + @patch("subprocess.run", side_effect=subprocess.TimeoutExpired("az", 10)) + def test_timeout_returns_none(self, mock_run, temp_dir_fixture): + """Should return None on subprocess timeout.""" + auth = FabAuth() + auth._cached_az_tenant = None + auth._cached_az_tenant_time = 0.0 + assert auth._get_azure_cli_tenant(force_refresh=True) is None + + @patch("subprocess.run", side_effect=FileNotFoundError("az not found")) + def test_az_not_installed_returns_none(self, mock_run, temp_dir_fixture): + """Should return None when az CLI is not installed.""" + auth = FabAuth() + auth._cached_az_tenant = None + auth._cached_az_tenant_time = 0.0 + assert auth._get_azure_cli_tenant(force_refresh=True) is None + + @patch("subprocess.run") + def test_cache_hit_before_ttl_expiry(self, mock_run, temp_dir_fixture): + """Should return cached tenant without calling subprocess.""" + auth = FabAuth() + auth._cached_az_tenant = "cached-tenant" + auth._cached_az_tenant_time = time.monotonic() # Just cached now + result = auth._get_azure_cli_tenant() + assert result == "cached-tenant" + mock_run.assert_not_called() + + @patch("subprocess.run") + def test_cache_miss_after_ttl_expiry(self, mock_run, temp_dir_fixture): + """Should call subprocess after TTL expires.""" + mock_run.return_value = MagicMock(returncode=0, stdout="new-tenant\n") + auth = FabAuth() + auth._cached_az_tenant = "old-tenant" + auth._cached_az_tenant_time = time.monotonic() - 30 + result = auth._get_azure_cli_tenant() + assert result == "new-tenant" + mock_run.assert_called_once() + + @patch("subprocess.run") + def test_error_status_code_on_credential_unavailable( + self, mock_run, temp_dir_fixture + ): + """CredentialUnavailableError should produce correct status code.""" + from fabric_cli.core.fab_auth import CredentialUnavailableError + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() + + with patch("fabric_cli.core.fab_auth.AzureCliCredential") as mock_cred: + mock_instance = MagicMock() + mock_instance.get_token.side_effect = CredentialUnavailableError("nope") + mock_cred.return_value = mock_instance + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert exc_info.value.status_code == con.ERROR_AUTHENTICATION_FAILED From 3c7fd912b46cb70a3e82e121cc623965f1928a68 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 13:34:06 +0300 Subject: [PATCH 19/50] test: address review feedback on test quality - Fix: test_set_azure_cli_sets_identity_type no longer calls az CLI (passes explicit tenant_id instead) - Fix: fixture updates auth_file/cache_file to each test's tmp_path - Fix: command tests assert set_azure_cli was called with correct args, verify token acquisition and context assignment - Improve: error assertions use ErrorMessages.Auth catalog messages instead of substring fragments - Remove unused auth_instance fixture Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_commands/test_auth.py | 15 ++++++++++++--- tests/test_core/test_fab_auth_azure_cli.py | 12 ++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/tests/test_commands/test_auth.py b/tests/test_commands/test_auth.py index b0956cfbf..7cca5e5a9 100644 --- a/tests/test_commands/test_auth.py +++ b/tests/test_commands/test_auth.py @@ -955,9 +955,10 @@ class TestAuthAzureCli: def test_init_with_azure_cli_flag(self, mock_fab_auth, mock_fab_context): """fab auth login --azure-cli should set azure_cli mode.""" args = prepare_auth_args({"azure_cli": True}) + mock_set_azure_cli = MagicMock() with patch.object( - mock_fab_auth["instance"], "set_azure_cli", MagicMock() + mock_fab_auth["instance"], "set_azure_cli", mock_set_azure_cli ): result = fab_auth.init(args) @@ -965,6 +966,7 @@ def test_init_with_azure_cli_flag(self, mock_fab_auth, mock_fab_context): mock_fab_auth_instance.set_access_mode.assert_called_with( "azure_cli", None ) + mock_set_azure_cli.assert_called_once_with(None) assert result is True def test_init_with_azure_cli_flag_and_tenant( @@ -972,9 +974,10 @@ def test_init_with_azure_cli_flag_and_tenant( ): """fab auth login --azure-cli --tenant should pass tenant.""" args = prepare_auth_args({"azure_cli": True, "tenant": "my-tenant"}) + mock_set_azure_cli = MagicMock() with patch.object( - mock_fab_auth["instance"], "set_azure_cli", MagicMock() + mock_fab_auth["instance"], "set_azure_cli", mock_set_azure_cli ): result = fab_auth.init(args) @@ -982,18 +985,21 @@ def test_init_with_azure_cli_flag_and_tenant( mock_fab_auth_instance.set_access_mode.assert_called_with( "azure_cli", "my-tenant" ) + mock_set_azure_cli.assert_called_once_with("my-tenant") assert result is True def test_init_with_interactive_azure_cli_selection( self, mock_fab_auth, mock_fab_context ): """Interactive menu Azure CLI selection should set azure_cli mode.""" + mock_set_azure_cli = MagicMock() + with patch( "fabric_cli.utils.fab_ui.prompt_select_item", return_value="Azure CLI (existing 'az login' session)", ): with patch.object( - mock_fab_auth["instance"], "set_azure_cli", MagicMock() + mock_fab_auth["instance"], "set_azure_cli", mock_set_azure_cli ): args = prepare_auth_args() result = fab_auth.init(args) @@ -1002,6 +1008,9 @@ def test_init_with_interactive_azure_cli_selection( mock_fab_auth_instance.set_access_mode.assert_called_with( "azure_cli", None ) + mock_set_azure_cli.assert_called_once_with(None) + assert_get_access_token(mock_fab_auth_instance) + assert_fab_context(mock_fab_context) assert result is True diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 1feb24d13..e0466fdd6 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import os import subprocess import time from unittest.mock import MagicMock, patch @@ -10,6 +11,7 @@ from fabric_cli.core import fab_constant as con from fabric_cli.core.fab_auth import FabAuth from fabric_cli.core.fab_exceptions import FabricCLIError +from fabric_cli.errors import ErrorMessages @pytest.fixture(autouse=True) @@ -28,6 +30,9 @@ def temp_dir_fixture(monkeypatch, tmp_path): auth._cached_az_tenant = None auth._cached_az_tenant_time = 0.0 auth._auth_info = {} + # Update file paths to use the test's tmp_path + auth.auth_file = os.path.join(str(tmp_path), "auth.json") + auth.cache_file = os.path.join(str(tmp_path), "cache.bin") return str(tmp_path) @@ -50,7 +55,7 @@ def test_set_azure_cli_sets_identity_type(self, temp_dir_fixture): """set_azure_cli should configure identity_type to azure_cli.""" auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli() + auth.set_azure_cli(tenant_id="test-tenant") assert auth.get_identity_type() == "azure_cli" def test_set_azure_cli_with_tenant(self, temp_dir_fixture): @@ -181,7 +186,7 @@ def test_acquire_token_from_azure_cli_credential_unavailable( with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert "not installed or not logged in" in str(exc_info.value) + assert ErrorMessages.Auth.azure_cli_not_available() in str(exc_info.value) @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_sdk_exception_surfaces_message( @@ -245,8 +250,7 @@ def test_tenant_drift_blocks_token_acquisition( with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert "Tenant mismatch" in str(exc_info.value) - assert "original-tenant" in str(exc_info.value) + assert ErrorMessages.Auth.azure_cli_tenant_mismatch("original-tenant", "different-tenant") in str(exc_info.value) @patch("fabric_cli.core.fab_auth.AzureCliCredential") @patch("subprocess.run") From e91c94bc544ca43cbb3d28b66335cc2b997adf2b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 13:40:42 +0300 Subject: [PATCH 20/50] test: add parser and MSAL bridge coverage for Azure CLI auth Parser tests (4 new): - --azure-cli flag maps to args.azure_cli=True - --azure-cli --tenant maps both attributes - Absent flag defaults to False - --tenant alone works for other auth modes Bridge tests (2 new): - MsalTokenCredential.get_token returns AccessToken via Azure CLI dispatch - Invalid scope is rejected with ClientAuthenticationError Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test_fab_msal_bridge_azure_cli.py | 67 +++++++++++++++++++ tests/test_parsers/test_fab_auth_parser.py | 49 ++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 tests/test_core/test_fab_msal_bridge_azure_cli.py create mode 100644 tests/test_parsers/test_fab_auth_parser.py diff --git a/tests/test_core/test_fab_msal_bridge_azure_cli.py b/tests/test_core/test_fab_msal_bridge_azure_cli.py new file mode 100644 index 000000000..adc676619 --- /dev/null +++ b/tests/test_core/test_fab_msal_bridge_azure_cli.py @@ -0,0 +1,67 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for the MSAL bridge with Azure CLI identity type.""" + +import time +from unittest.mock import MagicMock, patch + +import pytest + +from fabric_cli.core import fab_constant as con +from fabric_cli.core.fab_auth import FabAuth +from fabric_cli.core.fab_msal_bridge import MsalTokenCredential + + +@pytest.fixture(autouse=True) +def temp_dir_fixture(monkeypatch, tmp_path): + """Isolate FabAuth singleton for bridge tests.""" + monkeypatch.setattr( + "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path) + ) + monkeypatch.delenv("FAB_TOKEN", raising=False) + monkeypatch.delenv("FAB_TOKEN_ONELAKE", raising=False) + monkeypatch.delenv("FAB_TOKEN_AZURE", raising=False) + auth = FabAuth() + auth._azure_cli_token_cache.clear() + auth._cached_az_tenant = None + auth._cached_az_tenant_time = 0.0 + auth._auth_info = {} + + +class TestMsalBridgeAzureCli: + """Verify MsalTokenCredential works when identity_type is azure_cli.""" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_bridge_returns_access_token_for_azure_cli( + self, mock_credential_class + ): + """MsalTokenCredential.get_token should return an AccessToken via Azure CLI.""" + mock_token = MagicMock() + mock_token.token = "bridge-azure-cli-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + credential = MsalTokenCredential(auth) + result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) + + assert result.token == "bridge-azure-cli-token" + assert result.expires_on == mock_token.expires_on + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_bridge_rejects_invalid_scope(self, mock_credential_class): + """MsalTokenCredential should reject scopes not in the allowlist.""" + from azure.core.exceptions import ClientAuthenticationError + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + credential = MsalTokenCredential(auth) + with pytest.raises(ClientAuthenticationError): + credential.get_token("https://evil.example.com/.default") diff --git a/tests/test_parsers/test_fab_auth_parser.py b/tests/test_parsers/test_fab_auth_parser.py new file mode 100644 index 000000000..3f6a4e9de --- /dev/null +++ b/tests/test_parsers/test_fab_auth_parser.py @@ -0,0 +1,49 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Tests for the auth parser module — verifies argparse flag mapping.""" + +import argparse + +from fabric_cli.core.fab_parser_setup import CustomArgumentParser +from fabric_cli.parsers import fab_auth_parser + + +def _build_auth_parser(): + """Build a parser with auth subcommands registered.""" + parser = CustomArgumentParser() + subparsers = parser.add_subparsers(dest="command") + fab_auth_parser.register_parser(subparsers) + return parser + + +class TestAuthParserAzureCli: + """Verify --azure-cli flag is parsed correctly.""" + + def test_azure_cli_flag_sets_attribute(self): + """--azure-cli should map to args.azure_cli=True.""" + parser = _build_auth_parser() + args = parser.parse_args(["auth", "login", "--azure-cli"]) + assert args.azure_cli is True + + def test_azure_cli_flag_with_tenant(self): + """--azure-cli --tenant should set both attributes.""" + parser = _build_auth_parser() + args = parser.parse_args( + ["auth", "login", "--azure-cli", "--tenant", "my-tenant-id"] + ) + assert args.azure_cli is True + assert args.tenant == "my-tenant-id" + + def test_azure_cli_flag_absent_defaults_false(self): + """Without --azure-cli, azure_cli should be falsy.""" + parser = _build_auth_parser() + args = parser.parse_args(["auth", "login"]) + assert not args.azure_cli + + def test_tenant_flag_without_azure_cli(self): + """--tenant alone should work (used by other auth modes).""" + parser = _build_auth_parser() + args = parser.parse_args(["auth", "login", "--tenant", "some-tenant"]) + assert args.tenant == "some-tenant" + assert not args.azure_cli From 76917c666033ec03337a1bb398afbe7ae853243b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 13:47:17 +0300 Subject: [PATCH 21/50] test: address review feedback on test quality (round 2) - Fixture: clear FAB_TENANT_ID, FAB_SPN_*, FAB_MANAGED_IDENTITY env vars; reset _msal_app - Explicit-tenant test: assert subprocess.run not called (proves discovery bypassed) - Tenant-drift test: assert AzureCliCredential not instantiated (blocked before credential) - Fix expired-cache docstring: 'credential token request' not 'subprocess call' - Move misplaced test_error_status_code_on_credential_unavailable to TestAzureCliTokenAcquisition - Remove unused mock_run from status code test - Fix excess blank lines after fixture Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 61 +++++++++++++--------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index e0466fdd6..f06c2ce38 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -20,24 +20,31 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.setattr( "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path) ) - # Clear env vars that would interfere - monkeypatch.delenv("FAB_TOKEN", raising=False) - monkeypatch.delenv("FAB_TOKEN_ONELAKE", raising=False) - monkeypatch.delenv("FAB_TOKEN_AZURE", raising=False) + # Clear env vars that would interfere with auth + for var in ( + "FAB_TOKEN", + "FAB_TOKEN_ONELAKE", + "FAB_TOKEN_AZURE", + "FAB_TENANT_ID", + "FAB_SPN_CLIENT_ID", + "FAB_SPN_CLIENT_SECRET", + "FAB_SPN_CERT_PATH", + "FAB_MANAGED_IDENTITY", + ): + monkeypatch.delenv(var, raising=False) # Clear singleton caches between tests auth = FabAuth() auth._azure_cli_token_cache.clear() auth._cached_az_tenant = None auth._cached_az_tenant_time = 0.0 auth._auth_info = {} + auth._msal_app = None # Update file paths to use the test's tmp_path auth.auth_file = os.path.join(str(tmp_path), "auth.json") auth.cache_file = os.path.join(str(tmp_path), "cache.bin") return str(tmp_path) - - class TestAzureCliIdentityType: """Test that azure_cli is a valid identity type.""" @@ -90,6 +97,7 @@ def test_set_azure_cli_explicit_tenant_overrides_auto( auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="explicit-tenant") assert auth.get_tenant_id() == "explicit-tenant" + mock_run.assert_not_called() class TestAzureCliTokenAcquisition: @@ -228,6 +236,25 @@ def test_unknown_exception_returns_safe_message( assert "eyJ0eXAi" not in str(exc_info.value) assert "manually to diagnose" in str(exc_info.value) + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_error_status_code_on_credential_unavailable( + self, mock_credential_class, temp_dir_fixture + ): + """CredentialUnavailableError should produce correct status code.""" + from fabric_cli.core.fab_auth import CredentialUnavailableError + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() + + mock_instance = MagicMock() + mock_instance.get_token.side_effect = CredentialUnavailableError("nope") + mock_credential_class.return_value = mock_instance + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert exc_info.value.status_code == con.ERROR_AUTHENTICATION_FAILED + class TestAzureCliTenantDrift: """Test tenant drift detection during token acquisition.""" @@ -251,6 +278,7 @@ def test_tenant_drift_blocks_token_acquisition( auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) assert ErrorMessages.Auth.azure_cli_tenant_mismatch("original-tenant", "different-tenant") in str(exc_info.value) + mock_credential_class.assert_not_called() @patch("fabric_cli.core.fab_auth.AzureCliCredential") @patch("subprocess.run") @@ -310,7 +338,7 @@ def test_cached_token_avoids_repeated_credential_calls( def test_expired_cache_triggers_refresh( self, mock_credential_class, temp_dir_fixture ): - """Expired cached token should trigger a new subprocess call.""" + """Expired cached token should trigger a new credential token request.""" mock_token = MagicMock() mock_token.token = "fresh-token" mock_token.expires_on = int(time.time()) + 3600 @@ -559,22 +587,3 @@ def test_cache_miss_after_ttl_expiry(self, mock_run, temp_dir_fixture): result = auth._get_azure_cli_tenant() assert result == "new-tenant" mock_run.assert_called_once() - - @patch("subprocess.run") - def test_error_status_code_on_credential_unavailable( - self, mock_run, temp_dir_fixture - ): - """CredentialUnavailableError should produce correct status code.""" - from fabric_cli.core.fab_auth import CredentialUnavailableError - - auth = FabAuth() - auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() - - with patch("fabric_cli.core.fab_auth.AzureCliCredential") as mock_cred: - mock_instance = MagicMock() - mock_instance.get_token.side_effect = CredentialUnavailableError("nope") - mock_cred.return_value = mock_instance - with pytest.raises(FabricCLIError) as exc_info: - auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert exc_info.value.status_code == con.ERROR_AUTHENTICATION_FAILED From af7b21a3063b0c3ab8ff11b5efd3a1a00080b3ac Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 13:55:38 +0300 Subject: [PATCH 22/50] test: address review feedback round 3 - Fix fixture: reset auth.app (not nonexistent _msal_app), clear FAB_SPN_*/FAB_MANAGED_IDENTITY - Add 60-second buffer test: token expiring within buffer triggers refresh - Merge duplicate credential-unavailable tests (message + status code in one) - Assert subprocess contract: exact command, timeout, no shell invocation - Remove unnecessary mock_run setup in explicit-tenant test - Prove tenant drift blocks before credential construction - Break long assertion line for Black compliance Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 62 +++++++++++++--------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index f06c2ce38..d7248159b 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -38,7 +38,7 @@ def temp_dir_fixture(monkeypatch, tmp_path): auth._cached_az_tenant = None auth._cached_az_tenant_time = 0.0 auth._auth_info = {} - auth._msal_app = None + auth.app = None # Update file paths to use the test's tmp_path auth.auth_file = os.path.join(str(tmp_path), "auth.json") auth.cache_file = os.path.join(str(tmp_path), "cache.bin") @@ -84,15 +84,18 @@ def test_set_azure_cli_auto_captures_tenant( auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth.get_tenant_id() == "auto-captured-tenant-id" + mock_run.assert_called_once_with( + ["az", "account", "show", "--query", "tenantId", "-o", "tsv"], + capture_output=True, + text=True, + timeout=10, + ) @patch("subprocess.run") def test_set_azure_cli_explicit_tenant_overrides_auto( self, mock_run, temp_dir_fixture ): """Explicit tenant_id should be used even if az has a different one.""" - mock_run.return_value = MagicMock( - returncode=0, stdout="az-tenant\n" - ) auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="explicit-tenant") @@ -195,6 +198,7 @@ def test_acquire_token_from_azure_cli_credential_unavailable( auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) assert ErrorMessages.Auth.azure_cli_not_available() in str(exc_info.value) + assert exc_info.value.status_code == con.ERROR_AUTHENTICATION_FAILED @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_sdk_exception_surfaces_message( @@ -236,25 +240,6 @@ def test_unknown_exception_returns_safe_message( assert "eyJ0eXAi" not in str(exc_info.value) assert "manually to diagnose" in str(exc_info.value) - @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_error_status_code_on_credential_unavailable( - self, mock_credential_class, temp_dir_fixture - ): - """CredentialUnavailableError should produce correct status code.""" - from fabric_cli.core.fab_auth import CredentialUnavailableError - - auth = FabAuth() - auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() - - mock_instance = MagicMock() - mock_instance.get_token.side_effect = CredentialUnavailableError("nope") - mock_credential_class.return_value = mock_instance - - with pytest.raises(FabricCLIError) as exc_info: - auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert exc_info.value.status_code == con.ERROR_AUTHENTICATION_FAILED - class TestAzureCliTenantDrift: """Test tenant drift detection during token acquisition.""" @@ -277,7 +262,10 @@ def test_tenant_drift_blocks_token_acquisition( with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert ErrorMessages.Auth.azure_cli_tenant_mismatch("original-tenant", "different-tenant") in str(exc_info.value) + expected_msg = ErrorMessages.Auth.azure_cli_tenant_mismatch( + "original-tenant", "different-tenant" + ) + assert expected_msg in str(exc_info.value) mock_credential_class.assert_not_called() @patch("fabric_cli.core.fab_auth.AzureCliCredential") @@ -360,6 +348,32 @@ def test_expired_cache_triggers_refresh( assert result["access_token"] == "fresh-token" mock_credential.get_token.assert_called_once() + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_near_expiry_within_buffer_triggers_refresh( + self, mock_credential_class, temp_dir_fixture + ): + """Token valid but expiring within 60s buffer should trigger refresh.""" + mock_token = MagicMock() + mock_token.token = "refreshed-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth._azure_cli_token_cache.clear() + # Token expires in 30s — still valid but within 60s buffer + auth._azure_cli_token_cache[con.SCOPE_FABRIC_DEFAULT[0]] = { + "access_token": "almost-expired-token", + "expires_on": int(time.time()) + 30, + } + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "refreshed-token" + mock_credential.get_token.assert_called_once() + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_different_scopes_cached_separately( self, mock_credential_class, temp_dir_fixture From cbeba014294643775f25ec56d09fd201ac0235b5 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 14:14:13 +0300 Subject: [PATCH 23/50] test: use monkeypatch.setattr for singleton file paths Ensures pytest restores auth_file/cache_file after each test, preventing leaked tmp_path references into later test modules. Also removes unused os import. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index d7248159b..fbbbfb50e 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -1,7 +1,6 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -import os import subprocess import time from unittest.mock import MagicMock, patch @@ -40,8 +39,8 @@ def temp_dir_fixture(monkeypatch, tmp_path): auth._auth_info = {} auth.app = None # Update file paths to use the test's tmp_path - auth.auth_file = os.path.join(str(tmp_path), "auth.json") - auth.cache_file = os.path.join(str(tmp_path), "cache.bin") + monkeypatch.setattr(auth, "auth_file", str(tmp_path / "auth.json")) + monkeypatch.setattr(auth, "cache_file", str(tmp_path / "cache.bin")) return str(tmp_path) From f560488758546662bd8a0126003dd81a470ef391 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 14:19:31 +0300 Subject: [PATCH 24/50] refactor: extract token refresh buffer to named constant Replace magic number 60 with _AZURE_CLI_TOKEN_REFRESH_BUFFER_SECONDS. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 02387a7be..aa6068302 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -31,6 +31,7 @@ from fabric_cli.utils import fab_ui as utils_ui _AZURE_CLI_TENANT_CACHE_TTL_SECONDS = 10 +_AZURE_CLI_TOKEN_REFRESH_BUFFER_SECONDS = 60 def singleton(class_): @@ -535,9 +536,13 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: ) def _get_cached_azure_cli_token(self, cache_key: str) -> Optional[dict]: - """Return cached token if it exists and is not near expiry (60s buffer).""" + """Return cached token if it exists and is not near expiry.""" cached = self._azure_cli_token_cache.get(cache_key) - if cached and cached.get("expires_on", 0) > time.time() + 60: + if ( + cached + and cached.get("expires_on", 0) + > time.time() + _AZURE_CLI_TOKEN_REFRESH_BUFFER_SECONDS + ): return cached return None From 02f76181833b9dad8eb65a91ec1547556e2a0c2e Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 14:27:53 +0300 Subject: [PATCH 25/50] fix: clear token cache on set_azure_cli to prevent stale cross-tenant tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When transitioning from no stored tenant to a stored tenant, set_tenant() did not call logout() (only triggers on tenant mismatch). This left stale cached tokens from the previous no-tenant session. Now set_azure_cli() always clears _azure_cli_token_cache at the start of every login. Adds regression test verifying cache is cleared on no-tenant → tenant-B. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 3 +++ tests/test_core/test_fab_auth_azure_cli.py | 23 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index aa6068302..5c007d736 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -436,6 +436,9 @@ def set_azure_cli(self, tenant_id=None): from Azure CLI's active session via 'az account show'. Always forces a fresh query (bypasses cache) since this is a login action. """ + # Clear token cache on every login to prevent stale tokens from a + # previous tenant (or no-tenant) session from being reused. + self._azure_cli_token_cache.clear() # Set tenant first — set_tenant() may call logout() which clears auth info if tenant_id: self.set_tenant(tenant_id) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index fbbbfb50e..9ad6d2b71 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -542,6 +542,29 @@ def test_identity_type_preserved_after_tenant_change( assert auth.get_identity_type() == "azure_cli" assert auth.get_tenant_id() == "tenant-B" + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_login_clears_token_cache_from_no_tenant_state( + self, mock_credential_class, temp_dir_fixture + ): + """set_azure_cli should clear token cache even when transitioning from no tenant.""" + mock_token = MagicMock() + mock_token.token = "stale-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + # Acquire a token with no tenant stored + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert auth._azure_cli_token_cache.get(con.SCOPE_FABRIC_DEFAULT[0]) is not None + + # Login with explicit tenant — cache must be cleared + auth.set_azure_cli(tenant_id="new-tenant") + assert auth._azure_cli_token_cache.get(con.SCOPE_FABRIC_DEFAULT[0]) is None + class TestAzureCliTenantDiscoveryFailures: """Test _get_azure_cli_tenant failure paths.""" From 8a3ae9153507fe11661408592d3b4106cd195d5d Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 14:58:57 +0300 Subject: [PATCH 26/50] fix: resolve az executable path for Windows compatibility On Windows, 'az' is installed as 'az.cmd' which subprocess.run cannot find via CreateProcess. Use shutil.which('az') to resolve the full path before invoking subprocess. This fixes tenant auto-capture showing 'unknown' on Windows. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 6 +++++- tests/test_core/test_fab_auth_azure_cli.py | 8 +++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 5c007d736..a502d9455 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -3,6 +3,7 @@ import json import os +import shutil import subprocess import time import uuid @@ -473,8 +474,11 @@ def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: return self._cached_az_tenant try: + az_path = shutil.which("az") + if not az_path: + return None result = subprocess.run( - ["az", "account", "show", "--query", "tenantId", "-o", "tsv"], + [az_path, "account", "show", "--query", "tenantId", "-o", "tsv"], capture_output=True, text=True, timeout=10, diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 9ad6d2b71..759a8b316 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -19,6 +19,8 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.setattr( "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path) ) + # Ensure shutil.which("az") resolves in tests (Windows uses az.cmd) + monkeypatch.setattr("shutil.which", lambda cmd: f"/usr/bin/{cmd}" if cmd == "az" else None) # Clear env vars that would interfere with auth for var in ( "FAB_TOKEN", @@ -84,7 +86,7 @@ def test_set_azure_cli_auto_captures_tenant( auth.set_azure_cli() assert auth.get_tenant_id() == "auto-captured-tenant-id" mock_run.assert_called_once_with( - ["az", "account", "show", "--query", "tenantId", "-o", "tsv"], + ["/usr/bin/az", "account", "show", "--query", "tenantId", "-o", "tsv"], capture_output=True, text=True, timeout=10, @@ -595,9 +597,9 @@ def test_timeout_returns_none(self, mock_run, temp_dir_fixture): auth._cached_az_tenant_time = 0.0 assert auth._get_azure_cli_tenant(force_refresh=True) is None - @patch("subprocess.run", side_effect=FileNotFoundError("az not found")) - def test_az_not_installed_returns_none(self, mock_run, temp_dir_fixture): + def test_az_not_installed_returns_none(self, monkeypatch, temp_dir_fixture): """Should return None when az CLI is not installed.""" + monkeypatch.setattr("shutil.which", lambda cmd: None) auth = FabAuth() auth._cached_az_tenant = None auth._cached_az_tenant_time = 0.0 From a4d4ec44ca37f5df7a3a706f51ccc68d370f3709 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 15:55:15 +0300 Subject: [PATCH 27/50] format fix --- tests/test_core/test_fab_auth_azure_cli.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 759a8b316..d270ff805 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -20,7 +20,10 @@ def temp_dir_fixture(monkeypatch, tmp_path): "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path) ) # Ensure shutil.which("az") resolves in tests (Windows uses az.cmd) - monkeypatch.setattr("shutil.which", lambda cmd: f"/usr/bin/{cmd}" if cmd == "az" else None) + monkeypatch.setattr( + "shutil.which", + lambda cmd: f"/usr/bin/{cmd}" if cmd == "az" else None, + ) # Clear env vars that would interfere with auth for var in ( "FAB_TOKEN", From 7ec4ee1a958d88b147d27a9b9f0f1c50692b2f1b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 16:30:04 +0300 Subject: [PATCH 28/50] docs: add Azure CLI authentication to command reference and examples Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/commands/auth/index.md | 5 +++-- docs/examples/auth_examples.md | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/docs/commands/auth/index.md b/docs/commands/auth/index.md index 288d36d66..1ec3d1703 100644 --- a/docs/commands/auth/index.md +++ b/docs/commands/auth/index.md @@ -23,7 +23,7 @@ Authenticate with Fabric CLI. **Usage:** ``` -fab auth login [-u ] [-p ] [--federated-token ] [--certificate ] [--tenant ] +fab auth login [-u ] [-p ] [--federated-token ] [--certificate ] [--azure-cli] [--tenant ] ``` **Parameters:** @@ -32,7 +32,8 @@ fab auth login [-u ] [-p ] [--federated-token ] - `-p, --password`: Client secret for service principal. Optional. - `--federated-token`: Federated token for workload identity. Optional. - `--certificate`: Path to certificate file. Optional. -- `--tenant`: Tenant ID. Optional. +- `--azure-cli`: Use an existing Azure CLI login session as the token provider. Requires Azure CLI to be installed and logged in (`az login`). Optional. +- `--tenant`: Tenant ID. Optional. When used with `--azure-cli`, pins Fabric CLI to the specified tenant. --- diff --git a/docs/examples/auth_examples.md b/docs/examples/auth_examples.md index bdab9d7af..5527605a9 100644 --- a/docs/examples/auth_examples.md +++ b/docs/examples/auth_examples.md @@ -81,6 +81,38 @@ Log in using service principal with federated credential directly fab auth login -u --federated-token --tenant ``` +### Azure CLI Authentication + +Reuse an existing Azure CLI session instead of requiring a separate Fabric CLI login. Useful when tools or scripts already have `az login` done (e.g., in development environments or CI/CD pipelines with Azure CLI pre-authenticated). + +!!! info "Requires Azure CLI to be installed and logged in (`az login`)" + +#### Log in using Azure CLI in interactive mode + +``` +fab auth login +? How would you like to authenticate Fabric CLI? Azure CLI authentication +``` + +#### Log in using Azure CLI directly from command line + +``` +fab auth login --azure-cli +``` + +#### Log in using Azure CLI with a specific tenant + +``` +fab auth login --azure-cli --tenant +``` + +!!! note "Tenant behavior" + - If `--tenant` is not specified, Fabric CLI records the tenant from your current Azure CLI session at login time. + - On each subsequent command, Fabric CLI checks that Azure CLI's active tenant still matches the recorded tenant. If you switch tenants in Azure CLI (e.g., `az login --tenant `), Fabric CLI will report a tenant mismatch error and ask you to re-run `fab auth login --azure-cli`. + - This prevents accidentally operating against the wrong tenant after an `az login` switch. + +--- + ### Managed Identity Authentication !!! info "Requires 'Allow service principals to use Fabric APIs' tenant switch must be enabled" From 2a6ffe1018938a8c539534c3164ad6cf9154cc22 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 16:30:04 +0300 Subject: [PATCH 29/50] docs: add Azure CLI authentication to command reference and examples Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/commands/auth/index.md | 33 ++++++++++++++++++++++++++------- docs/examples/auth_examples.md | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/docs/commands/auth/index.md b/docs/commands/auth/index.md index 288d36d66..87de3c237 100644 --- a/docs/commands/auth/index.md +++ b/docs/commands/auth/index.md @@ -8,11 +8,11 @@ Not resource-specific; applies to CLI authentication context. ## Available Commands -| Command | Description | Usage | -|----------------|---------------------------|-----------------------------------------------------------------------| -| `auth login` | Log in to Fabric CLI | `auth login [parameters]` | -| `auth logout` | Log out of current session| `auth logout` | -| `auth status` | Show authentication status| `auth status` | +| Command | Description | Usage | +| --- | --- | --- | +| `auth login` | Log in to Fabric CLI | `auth login [parameters]` | +| `auth logout` | Log out of current session | `auth logout` | +| `auth status` | Show authentication status | `auth status` | --- @@ -22,8 +22,26 @@ Authenticate with Fabric CLI. **Usage:** +#### Interactive login ``` -fab auth login [-u ] [-p ] [--federated-token ] [--certificate ] [--tenant ] +fab auth login +``` + +#### Azure CLI +``` +fab auth login --azure-cli [--tenant ] +``` + +#### Service principal +``` +fab auth login -u -p --tenant # Service principal with secret + +fab auth login -u --certificate --tenant # Service principal with certificate +``` + +#### Workload identity +``` +fab auth login -u --federated-token --tenant # Workload identity ``` **Parameters:** @@ -32,7 +50,8 @@ fab auth login [-u ] [-p ] [--federated-token ] - `-p, --password`: Client secret for service principal. Optional. - `--federated-token`: Federated token for workload identity. Optional. - `--certificate`: Path to certificate file. Optional. -- `--tenant`: Tenant ID. Optional. +- `--azure-cli`: Use an existing Azure CLI login session as the token provider. Requires Azure CLI to be installed and logged in (`az login`). Optional. +- `--tenant`: Tenant ID. Optional. When used with `--azure-cli`, pins Fabric CLI to the specified tenant. --- diff --git a/docs/examples/auth_examples.md b/docs/examples/auth_examples.md index bdab9d7af..5527605a9 100644 --- a/docs/examples/auth_examples.md +++ b/docs/examples/auth_examples.md @@ -81,6 +81,38 @@ Log in using service principal with federated credential directly fab auth login -u --federated-token --tenant ``` +### Azure CLI Authentication + +Reuse an existing Azure CLI session instead of requiring a separate Fabric CLI login. Useful when tools or scripts already have `az login` done (e.g., in development environments or CI/CD pipelines with Azure CLI pre-authenticated). + +!!! info "Requires Azure CLI to be installed and logged in (`az login`)" + +#### Log in using Azure CLI in interactive mode + +``` +fab auth login +? How would you like to authenticate Fabric CLI? Azure CLI authentication +``` + +#### Log in using Azure CLI directly from command line + +``` +fab auth login --azure-cli +``` + +#### Log in using Azure CLI with a specific tenant + +``` +fab auth login --azure-cli --tenant +``` + +!!! note "Tenant behavior" + - If `--tenant` is not specified, Fabric CLI records the tenant from your current Azure CLI session at login time. + - On each subsequent command, Fabric CLI checks that Azure CLI's active tenant still matches the recorded tenant. If you switch tenants in Azure CLI (e.g., `az login --tenant `), Fabric CLI will report a tenant mismatch error and ask you to re-run `fab auth login --azure-cli`. + - This prevents accidentally operating against the wrong tenant after an `az login` switch. + +--- + ### Managed Identity Authentication !!! info "Requires 'Allow service principals to use Fabric APIs' tenant switch must be enabled" From 8fae5dc7794f109849cf229eb2077d5f858d0890 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 17:23:24 +0300 Subject: [PATCH 30/50] update docs --- docs/commands/auth/index.md | 34 +++++++++++++++---- docs/examples/auth_examples.md | 61 +++++++++++++++++----------------- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/docs/commands/auth/index.md b/docs/commands/auth/index.md index 1ec3d1703..9c72bb4fa 100644 --- a/docs/commands/auth/index.md +++ b/docs/commands/auth/index.md @@ -8,11 +8,11 @@ Not resource-specific; applies to CLI authentication context. ## Available Commands -| Command | Description | Usage | -|----------------|---------------------------|-----------------------------------------------------------------------| -| `auth login` | Log in to Fabric CLI | `auth login [parameters]` | -| `auth logout` | Log out of current session| `auth logout` | -| `auth status` | Show authentication status| `auth status` | +| Command | Description | Usage | +| --- | --- | --- | +| `auth login` | Log in to Fabric CLI | `auth login [parameters]` | +| `auth logout` | Log out of current session | `auth logout` | +| `auth status` | Show authentication status | `auth status` | --- @@ -22,8 +22,28 @@ Authenticate with Fabric CLI. **Usage:** +#### Interactive login ``` -fab auth login [-u ] [-p ] [--federated-token ] [--certificate ] [--azure-cli] [--tenant ] +fab auth login +``` + +#### Azure CLI +``` +fab auth login --azure-cli [--tenant ] +``` + +#### Service principal +``` +# Service principal with secret +fab auth login -u -p --tenant + +# Service principal with certificate +fab auth login -u --certificate --tenant +``` + +#### Workload identity +``` +fab auth login -u --federated-token --tenant ``` **Parameters:** @@ -61,4 +81,4 @@ fab auth status --- -For more examples and detailed scenarios, see [Authentication Examples](../../examples/auth_examples.md). +For more examples and detailed scenarios, see [Authentication Examples](../../examples/auth_examples.md). \ No newline at end of file diff --git a/docs/examples/auth_examples.md b/docs/examples/auth_examples.md index 5527605a9..60922e617 100644 --- a/docs/examples/auth_examples.md +++ b/docs/examples/auth_examples.md @@ -25,6 +25,36 @@ fab auth login ``` +### Azure CLI Authentication + +Reuse an existing Azure CLI session instead of requiring a separate Fabric CLI login. Useful when tools or scripts already have `az login` done (e.g., in development environments or CI/CD pipelines with Azure CLI pre-authenticated). + +!!! info "Requires Azure CLI to be installed and logged in (`az login`)" + +#### Log in using Azure CLI in interactive mode + +``` +fab auth login +? How would you like to authenticate Fabric CLI? Azure CLI authentication +``` + +#### Log in using Azure CLI directly from command line + +``` +fab auth login --azure-cli +``` + +#### Log in using Azure CLI with a specific tenant + +``` +fab auth login --azure-cli --tenant +``` + +!!! note "Tenant behavior" + - If `--tenant` is not specified, Fabric CLI captures and records the tenant from the current Azure CLI session at login time. + - Throughout the `fab` session, the Azure CLI's active tenant is checked against the recorded tenant. If you switch tenants in Azure CLI (e.g., `az login --tenant `), Fabric CLI will raise a tenant mismatch error and require you to re-authenticate, e.g., `fab auth login --azure-cli`. + + ### Service Principal Authentication !!! info "Requires 'Allow service principals to use Fabric APIs' tenant switch to be enabled in the admin portal" @@ -81,37 +111,6 @@ Log in using service principal with federated credential directly fab auth login -u --federated-token --tenant ``` -### Azure CLI Authentication - -Reuse an existing Azure CLI session instead of requiring a separate Fabric CLI login. Useful when tools or scripts already have `az login` done (e.g., in development environments or CI/CD pipelines with Azure CLI pre-authenticated). - -!!! info "Requires Azure CLI to be installed and logged in (`az login`)" - -#### Log in using Azure CLI in interactive mode - -``` -fab auth login -? How would you like to authenticate Fabric CLI? Azure CLI authentication -``` - -#### Log in using Azure CLI directly from command line - -``` -fab auth login --azure-cli -``` - -#### Log in using Azure CLI with a specific tenant - -``` -fab auth login --azure-cli --tenant -``` - -!!! note "Tenant behavior" - - If `--tenant` is not specified, Fabric CLI records the tenant from your current Azure CLI session at login time. - - On each subsequent command, Fabric CLI checks that Azure CLI's active tenant still matches the recorded tenant. If you switch tenants in Azure CLI (e.g., `az login --tenant `), Fabric CLI will report a tenant mismatch error and ask you to re-run `fab auth login --azure-cli`. - - This prevents accidentally operating against the wrong tenant after an `az login` switch. - ---- ### Managed Identity Authentication From 4851ffa041994b294ace75d07ea362bfaa51e021 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 12 Aug 2026 17:35:19 +0300 Subject: [PATCH 31/50] update wording --- docs/examples/auth_examples.md | 2 +- src/fabric_cli/parsers/fab_auth_parser.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/examples/auth_examples.md b/docs/examples/auth_examples.md index 60922e617..3bc260cb5 100644 --- a/docs/examples/auth_examples.md +++ b/docs/examples/auth_examples.md @@ -35,7 +35,7 @@ Reuse an existing Azure CLI session instead of requiring a separate Fabric CLI l ``` fab auth login -? How would you like to authenticate Fabric CLI? Azure CLI authentication +? How would you like to authenticate Fabric CLI? Azure CLI (existing 'az login' session) ``` #### Log in using Azure CLI directly from command line diff --git a/src/fabric_cli/parsers/fab_auth_parser.py b/src/fabric_cli/parsers/fab_auth_parser.py index c1f033809..2fc0a75f0 100644 --- a/src/fabric_cli/parsers/fab_auth_parser.py +++ b/src/fabric_cli/parsers/fab_auth_parser.py @@ -93,7 +93,7 @@ def register_parser(subparsers: _SubParsersAction) -> None: required=False, action="store_true", dest="azure_cli", - help="Use Azure CLI authentication (existing 'az login' session)", + help="Azure CLI authentication, must have an existing 'az login' session. Optional, only for Azure CLI auth", ) login_parser.usage = f"{utils_error_parser.get_usage_prog(login_parser)}" From ed0b2ede7d1531615d0801e138725cb5c2b45c27 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 13 Aug 2026 10:08:02 +0300 Subject: [PATCH 32/50] fix: expand SDK exception allowlist with ServiceRequestError and ServiceResponseError Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index a502d9455..8d2263c7a 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -533,7 +533,12 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: ) except Exception as e: # Allowlist: SDK exceptions are pre-sanitized by azure-identity; unknown exceptions get a safe generic message - if type(e).__name__ in ("ClientAuthenticationError", "HttpResponseError"): + if type(e).__name__ in ( + "ClientAuthenticationError", + "HttpResponseError", + "ServiceRequestError", + "ServiceResponseError", + ): error_msg = str(e) else: error_msg = ErrorMessages.Auth.azure_cli_token_acquisition_failed() From edfe3853a2e6c33a6a072b7bcb65c91831d9d063 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 13 Aug 2026 10:20:34 +0300 Subject: [PATCH 33/50] test: verify non-azure-cli identity types do not invoke AzureCliCredential Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../test_fab_msal_bridge_azure_cli.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/test_core/test_fab_msal_bridge_azure_cli.py b/tests/test_core/test_fab_msal_bridge_azure_cli.py index adc676619..0f4e643cd 100644 --- a/tests/test_core/test_fab_msal_bridge_azure_cli.py +++ b/tests/test_core/test_fab_msal_bridge_azure_cli.py @@ -65,3 +65,49 @@ def test_bridge_rejects_invalid_scope(self, mock_credential_class): credential = MsalTokenCredential(auth) with pytest.raises(ClientAuthenticationError): credential.get_token("https://evil.example.com/.default") + + +class TestMsalBridgeNonAzureCli: + """Verify non-azure-cli identity types never invoke AzureCliCredential.""" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_user_identity_does_not_invoke_azure_cli(self, mock_credential_class): + """When identity_type is 'user', AzureCliCredential must not be instantiated.""" + auth = FabAuth() + auth.set_access_mode("user") + + # Simulate MSAL returning a cached token so no interactive prompt + mock_app = MagicMock() + mock_app.get_accounts.return_value = [{"username": "test@contoso.com"}] + mock_app.acquire_token_silent.return_value = { + "access_token": "msal-user-token", + "expires_on": str(int(time.time()) + 3600), + } + auth.app = mock_app + + credential = MsalTokenCredential(auth) + result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) + + assert result.token == "msal-user-token" + mock_credential_class.assert_not_called() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_service_principal_does_not_invoke_azure_cli( + self, mock_credential_class + ): + """When identity_type is 'service_principal', AzureCliCredential must not be instantiated.""" + auth = FabAuth() + auth.set_access_mode("service_principal") + + mock_app = MagicMock() + mock_app.acquire_token_for_client.return_value = { + "access_token": "spn-token", + "expires_on": str(int(time.time()) + 3600), + } + auth.app = mock_app + + credential = MsalTokenCredential(auth) + result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) + + assert result.token == "spn-token" + mock_credential_class.assert_not_called() From d0c48c2ffde12c8d5fe9637bfdb2f22e37c92b10 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 13 Aug 2026 10:23:27 +0300 Subject: [PATCH 34/50] refactor: move auth isolation tests from bridge to core auth test file Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 43 +++++++++++++++++ .../test_fab_msal_bridge_azure_cli.py | 46 ------------------- 2 files changed, 43 insertions(+), 46 deletions(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index d270ff805..c75617be5 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -628,3 +628,46 @@ def test_cache_miss_after_ttl_expiry(self, mock_run, temp_dir_fixture): result = auth._get_azure_cli_tenant() assert result == "new-tenant" mock_run.assert_called_once() + + +class TestNonAzureCliIsolation: + """Verify non-azure-cli identity types never invoke AzureCliCredential.""" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_user_identity_does_not_invoke_azure_cli( + self, mock_credential_class, temp_dir_fixture + ): + """When identity_type is 'user', AzureCliCredential must not be instantiated.""" + auth = FabAuth() + auth.set_access_mode("user") + + mock_app = MagicMock() + mock_app.get_accounts.return_value = [{"username": "test@contoso.com"}] + mock_app.acquire_token_silent.return_value = { + "access_token": "msal-user-token", + "expires_on": str(int(time.time()) + 3600), + } + auth.app = mock_app + + result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "msal-user-token" + mock_credential_class.assert_not_called() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_service_principal_does_not_invoke_azure_cli( + self, mock_credential_class, temp_dir_fixture + ): + """When identity_type is 'service_principal', AzureCliCredential must not be instantiated.""" + auth = FabAuth() + auth.set_access_mode("service_principal") + + mock_app = MagicMock() + mock_app.acquire_token_for_client.return_value = { + "access_token": "spn-token", + "expires_on": str(int(time.time()) + 3600), + } + auth.app = mock_app + + result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "spn-token" + mock_credential_class.assert_not_called() diff --git a/tests/test_core/test_fab_msal_bridge_azure_cli.py b/tests/test_core/test_fab_msal_bridge_azure_cli.py index 0f4e643cd..adc676619 100644 --- a/tests/test_core/test_fab_msal_bridge_azure_cli.py +++ b/tests/test_core/test_fab_msal_bridge_azure_cli.py @@ -65,49 +65,3 @@ def test_bridge_rejects_invalid_scope(self, mock_credential_class): credential = MsalTokenCredential(auth) with pytest.raises(ClientAuthenticationError): credential.get_token("https://evil.example.com/.default") - - -class TestMsalBridgeNonAzureCli: - """Verify non-azure-cli identity types never invoke AzureCliCredential.""" - - @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_user_identity_does_not_invoke_azure_cli(self, mock_credential_class): - """When identity_type is 'user', AzureCliCredential must not be instantiated.""" - auth = FabAuth() - auth.set_access_mode("user") - - # Simulate MSAL returning a cached token so no interactive prompt - mock_app = MagicMock() - mock_app.get_accounts.return_value = [{"username": "test@contoso.com"}] - mock_app.acquire_token_silent.return_value = { - "access_token": "msal-user-token", - "expires_on": str(int(time.time()) + 3600), - } - auth.app = mock_app - - credential = MsalTokenCredential(auth) - result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) - - assert result.token == "msal-user-token" - mock_credential_class.assert_not_called() - - @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_service_principal_does_not_invoke_azure_cli( - self, mock_credential_class - ): - """When identity_type is 'service_principal', AzureCliCredential must not be instantiated.""" - auth = FabAuth() - auth.set_access_mode("service_principal") - - mock_app = MagicMock() - mock_app.acquire_token_for_client.return_value = { - "access_token": "spn-token", - "expires_on": str(int(time.time()) + 3600), - } - auth.app = mock_app - - credential = MsalTokenCredential(auth) - result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) - - assert result.token == "spn-token" - mock_credential_class.assert_not_called() From f4379adde63af2826aba5476049740a63357d38b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 13 Aug 2026 10:30:24 +0300 Subject: [PATCH 35/50] =?UTF-8?q?test:=20add=20bidirectional=20auth=20meth?= =?UTF-8?q?od=20isolation=20=E2=80=94=20azure=5Fcli=20must=20not=20invoke?= =?UTF-8?q?=20MSAL?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index c75617be5..498a1aac0 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -631,7 +631,7 @@ def test_cache_miss_after_ttl_expiry(self, mock_run, temp_dir_fixture): class TestNonAzureCliIsolation: - """Verify non-azure-cli identity types never invoke AzureCliCredential.""" + """Verify each auth method uses only its own credential path — no overlap.""" @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_user_identity_does_not_invoke_azure_cli( @@ -671,3 +671,28 @@ def test_service_principal_does_not_invoke_azure_cli( result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) assert result["access_token"] == "spn-token" mock_credential_class.assert_not_called() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_azure_cli_does_not_invoke_msal_app( + self, mock_credential_class, temp_dir_fixture + ): + """When identity_type is 'azure_cli', MSAL app methods must not be called.""" + mock_token = MagicMock() + mock_token.token = "az-cli-token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + + mock_app = MagicMock() + auth.app = mock_app + + result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "az-cli-token" + mock_app.acquire_token_silent.assert_not_called() + mock_app.acquire_token_interactive.assert_not_called() + mock_app.acquire_token_for_client.assert_not_called() From 8565bda6b118b1613de685d452f794e21412370c Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 13 Aug 2026 11:08:54 +0300 Subject: [PATCH 36/50] chore: align azure-identity minimum to >=1.25.0 (consistent with fabric-cicd) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 7bd17b6a5..325df41cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ dependencies = [ "msal>=1.34,<2", "msal_extensions", "azure-core>=1.29.0", - "azure-identity>=1.15.0", + "azure-identity>=1.25.0", "questionary", "prompt_toolkit>=3.0.41", "cachetools>=5.5.0", From 72a591663b9e727a46838a44bcd7a0a32e7693c8 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Mon, 17 Aug 2026 11:31:09 +0300 Subject: [PATCH 37/50] feat: add principal drift detection to prevent silent identity swaps - Unified _get_azure_cli_account() queries tenant + principal in single subprocess call - Store principal name at login for drift comparison - Block token acquisition if Azure CLI identity changes within same tenant - Error message is PII-free (no emails/OIDs exposed) - Graceful degradation: skip check if no principal stored (old auth files) - 3 new tests for drift block, match pass, and no-stored-principal skip Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 80 ++++++++---- src/fabric_cli/core/fab_constant.py | 1 + src/fabric_cli/errors/auth.py | 7 + tests/test_core/test_fab_auth_azure_cli.py | 141 +++++++++++++++++---- 4 files changed, 179 insertions(+), 50 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 8d2263c7a..703ec87d8 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -59,9 +59,9 @@ def __init__(self): self._auth_info = {} # In-memory token cache for Azure CLI tokens self._azure_cli_token_cache: dict[str, dict] = {} - # Cached tenant ID from az account show - self._cached_az_tenant: Optional[str] = None - self._cached_az_tenant_time: float = 0.0 + # Cached az account show result (tenant + principal in one call) + self._cached_az_account: Optional[dict] = None + self._cached_az_account_time: float = 0.0 # Load the auth info and environment variables self._load_auth() @@ -440,57 +440,71 @@ def set_azure_cli(self, tenant_id=None): # Clear token cache on every login to prevent stale tokens from a # previous tenant (or no-tenant) session from being reused. self._azure_cli_token_cache.clear() + # Query Azure CLI account once for both tenant and principal + account = self._get_azure_cli_account(force_refresh=True) # Set tenant first — set_tenant() may call logout() which clears auth info if tenant_id: self.set_tenant(tenant_id) - else: - # Force refresh at login to avoid stale cached tenant - captured_tenant = self._get_azure_cli_tenant(force_refresh=True) - if captured_tenant: - self.set_tenant(captured_tenant) + elif account and account.get("tenant_id"): + self.set_tenant(account["tenant_id"]) # Set identity_type after tenant to survive any logout triggered by tenant change - self._set_auth_properties( - { - con.IDENTITY_TYPE: "azure_cli", - } - ) + auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"} + # Store principal for drift detection (no PII exposed in errors) + if account and account.get("principal_name"): + auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = account["principal_name"] + self._set_auth_properties(auth_props) - def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: - """Query Azure CLI for the current tenant ID via 'az account show'. + def _get_azure_cli_account(self, force_refresh: bool = False) -> Optional[dict]: + """Query Azure CLI account info (tenant + principal) in a single subprocess call. - Caches the result to avoid repeated subprocess calls - during multi-scope token acquisition flows. + Returns a dict with 'tenant_id' and 'principal_name' keys, or None + if Azure CLI is unavailable. Caches the result to avoid repeated + subprocess calls during multi-scope token acquisition flows. Args: force_refresh: If True, bypass the cache and query az directly. """ - # Return cached result if fresh and not forced if ( not force_refresh - and self._cached_az_tenant is not None - and time.monotonic() - self._cached_az_tenant_time + and self._cached_az_account is not None + and time.monotonic() - self._cached_az_account_time < _AZURE_CLI_TENANT_CACHE_TTL_SECONDS ): - return self._cached_az_tenant + return self._cached_az_account try: az_path = shutil.which("az") if not az_path: return None result = subprocess.run( - [az_path, "account", "show", "--query", "tenantId", "-o", "tsv"], + [az_path, "account", "show", "--query", "{tenantId:tenantId,userName:user.name}", "-o", "json"], capture_output=True, text=True, timeout=10, ) if result.returncode == 0 and result.stdout.strip(): - self._cached_az_tenant = result.stdout.strip() - self._cached_az_tenant_time = time.monotonic() - return self._cached_az_tenant - except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + data = json.loads(result.stdout.strip()) + account_info = { + "tenant_id": data.get("tenantId"), + "principal_name": data.get("userName"), + } + self._cached_az_account = account_info + self._cached_az_account_time = time.monotonic() + return self._cached_az_account + except (subprocess.TimeoutExpired, FileNotFoundError, OSError, ValueError): pass return None + def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: + """Get the current Azure CLI tenant ID (thin wrapper over _get_azure_cli_account).""" + account = self._get_azure_cli_account(force_refresh=force_refresh) + return account.get("tenant_id") if account else None + + def _get_azure_cli_principal(self, force_refresh: bool = False) -> Optional[str]: + """Get the current Azure CLI principal name (thin wrapper over _get_azure_cli_account).""" + account = self._get_azure_cli_account(force_refresh=force_refresh) + return account.get("principal_name") if account else None + def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: """Acquire a token using Azure CLI's AzureCliCredential.""" # Tenant drift check: compare stored tenant against current az session @@ -505,6 +519,16 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) + # Principal drift check: detect identity change within same tenant + stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) + if stored_principal: + current_principal = self._get_azure_cli_principal() + if current_principal and current_principal != stored_principal: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_principal_mismatch(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + # Check in-memory cache first cache_key = scope[0] if scope else "" cached = self._get_cached_azure_cli_token(cache_key) @@ -694,8 +718,8 @@ def logout(self): # Clear Azure CLI caches self._azure_cli_token_cache.clear() - self._cached_az_tenant = None - self._cached_az_tenant_time = 0.0 + self._cached_az_account = None + self._cached_az_account_time = 0.0 if os.path.exists(self.cache_file): os.remove(self.cache_file) diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index d19811500..a28e6e5c7 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -56,6 +56,7 @@ FAB_TENANT_ID = "fab_tenant_id" FAB_REFRESH_TOKEN = "fab_refresh_token" +FAB_AZURE_CLI_PRINCIPAL_ID = "fab_azure_cli_principal_id" IDENTITY_TYPE = "identity_type" FAB_AUTH_MODE = "fab_auth_mode" # Kept for backward compatibility FAB_AUTHORITY = "fab_authority" diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index 38dc44fe4..5e44c42a4 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -128,6 +128,13 @@ def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str: "Run 'fab auth login --azure-cli' to re-authenticate." ) + @staticmethod + def azure_cli_principal_mismatch() -> str: + return ( + "Azure CLI identity has changed since 'fab auth login --azure-cli' was run. " + "Run 'fab auth login --azure-cli' to re-authenticate with the current identity." + ) + @staticmethod def azure_cli_not_available() -> str: return ( diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 498a1aac0..31ef06767 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -13,6 +13,13 @@ from fabric_cli.errors import ErrorMessages +def _az_account_json(tenant_id: str, user_name: str = "testuser@contoso.com") -> str: + """Return mock JSON output for 'az account show'.""" + import json as _json + + return _json.dumps({"tenantId": tenant_id, "userName": user_name}) + + @pytest.fixture(autouse=True) def temp_dir_fixture(monkeypatch, tmp_path): """Create a temporary directory and configure FabAuth to use it.""" @@ -39,8 +46,8 @@ def temp_dir_fixture(monkeypatch, tmp_path): # Clear singleton caches between tests auth = FabAuth() auth._azure_cli_token_cache.clear() - auth._cached_az_tenant = None - auth._cached_az_tenant_time = 0.0 + auth._cached_az_account = None + auth._cached_az_account_time = 0.0 auth._auth_info = {} auth.app = None # Update file paths to use the test's tmp_path @@ -82,14 +89,14 @@ def test_set_azure_cli_auto_captures_tenant( ): """set_azure_cli without tenant_id should auto-capture from az account show.""" mock_run.return_value = MagicMock( - returncode=0, stdout="auto-captured-tenant-id\n" + returncode=0, stdout=_az_account_json("auto-captured-tenant-id") ) auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth.get_tenant_id() == "auto-captured-tenant-id" mock_run.assert_called_once_with( - ["/usr/bin/az", "account", "show", "--query", "tenantId", "-o", "tsv"], + ["/usr/bin/az", "account", "show", "--query", "{tenantId:tenantId,userName:user.name}", "-o", "json"], capture_output=True, text=True, timeout=10, @@ -100,11 +107,15 @@ def test_set_azure_cli_explicit_tenant_overrides_auto( self, mock_run, temp_dir_fixture ): """Explicit tenant_id should be used even if az has a different one.""" + mock_run.return_value = MagicMock( + returncode=0, stdout=_az_account_json("other-tenant") + ) auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="explicit-tenant") assert auth.get_tenant_id() == "explicit-tenant" - mock_run.assert_not_called() + # Still calls az account show once to capture principal for drift detection + mock_run.assert_called_once() class TestAzureCliTokenAcquisition: @@ -255,7 +266,7 @@ def test_tenant_drift_blocks_token_acquisition( ): """Should block when stored tenant differs from current az session.""" mock_run.return_value = MagicMock( - returncode=0, stdout="different-tenant\n" + returncode=0, stdout=_az_account_json("different-tenant") ) auth = FabAuth() @@ -279,7 +290,7 @@ def test_tenant_match_allows_token_acquisition( ): """Should allow when stored tenant matches current az session.""" mock_run.return_value = MagicMock( - returncode=0, stdout="same-tenant\n" + returncode=0, stdout=_az_account_json("same-tenant") ) mock_token = MagicMock() mock_token.token = "valid-token" @@ -298,6 +309,91 @@ def test_tenant_match_allows_token_acquisition( assert result["access_token"] == "valid-token" +class TestAzureCliPrincipalDrift: + """Test principal (identity) drift detection during token acquisition.""" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + @patch("subprocess.run") + def test_principal_drift_blocks_token_acquisition( + self, mock_run, mock_credential_class, temp_dir_fixture + ): + """Should block when stored principal differs from current az identity.""" + # Login as alice + mock_run.return_value = MagicMock( + returncode=0, stdout=_az_account_json("same-tenant", "alice@contoso.com") + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="same-tenant") + auth._azure_cli_token_cache.clear() + + # Now az is logged in as bob (same tenant) + mock_run.return_value = MagicMock( + returncode=0, stdout=_az_account_json("same-tenant", "bob@contoso.com") + ) + auth._cached_az_account = None # Force re-query + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + # Error message must NOT contain PII (no email addresses) + assert "alice" not in str(exc_info.value) + assert "bob" not in str(exc_info.value) + assert "identity has changed" in str(exc_info.value) + mock_credential_class.assert_not_called() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + @patch("subprocess.run") + def test_principal_match_allows_token_acquisition( + self, mock_run, mock_credential_class, temp_dir_fixture + ): + """Should allow when principal matches stored identity.""" + mock_run.return_value = MagicMock( + returncode=0, stdout=_az_account_json("same-tenant", "alice@contoso.com") + ) + mock_token = MagicMock() + mock_token.token = "valid-token" + mock_token.expires_on = int(time.time()) + 3600 + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="same-tenant") + auth._azure_cli_token_cache.clear() + auth._cached_az_account = None # Force re-query + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "valid-token" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + @patch("subprocess.run") + def test_no_stored_principal_skips_drift_check( + self, mock_run, mock_credential_class, temp_dir_fixture + ): + """If no principal was stored at login, drift check is skipped.""" + mock_run.return_value = MagicMock( + returncode=0, stdout=_az_account_json("same-tenant", "anyone@contoso.com") + ) + mock_token = MagicMock() + mock_token.token = "valid-token" + mock_token.expires_on = int(time.time()) + 3600 + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + # Manually set identity without principal (simulate old auth file) + auth._set_auth_properties({con.IDENTITY_TYPE: "azure_cli"}) + auth.set_tenant("same-tenant") + auth._azure_cli_token_cache.clear() + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert result["access_token"] == "valid-token" + + class TestAzureCliTokenCache: """Test in-memory token caching for Azure CLI tokens.""" @@ -460,19 +556,20 @@ class TestAzureCliCacheInvalidation: @patch("subprocess.run") def test_logout_clears_tenant_cache(self, mock_run, temp_dir_fixture): - """logout() should clear the cached tenant.""" + """logout() should clear the cached account info.""" mock_run.return_value = MagicMock( - returncode=0, stdout="cached-tenant\n" + returncode=0, stdout=_az_account_json("cached-tenant") ) auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() - assert auth._cached_az_tenant == "cached-tenant" + assert auth._cached_az_account is not None + assert auth._cached_az_account["tenant_id"] == "cached-tenant" auth.logout() - assert auth._cached_az_tenant is None - assert auth._cached_az_tenant_time == 0.0 + assert auth._cached_az_account is None + assert auth._cached_az_account_time == 0.0 assert auth._azure_cli_token_cache == {} @patch("subprocess.run") @@ -480,7 +577,7 @@ def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): """set_azure_cli should bypass cache and query az fresh.""" # First call returns tenant-A mock_run.return_value = MagicMock( - returncode=0, stdout="tenant-A\n" + returncode=0, stdout=_az_account_json("tenant-A") ) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -489,7 +586,7 @@ def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): # Simulate user switching az tenant, then re-logging in mock_run.return_value = MagicMock( - returncode=0, stdout="tenant-B\n" + returncode=0, stdout=_az_account_json("tenant-B") ) auth.set_access_mode("azure_cli") auth.set_azure_cli() # Should force refresh, get tenant-B @@ -502,7 +599,7 @@ def test_single_subprocess_across_three_login_scopes( ): """Login should call az account show only once across 3 scope validations.""" mock_run.return_value = MagicMock( - returncode=0, stdout="login-tenant\n" + returncode=0, stdout=_az_account_json("login-tenant") ) mock_token = MagicMock() @@ -531,7 +628,7 @@ def test_identity_type_preserved_after_tenant_change( ): """identity_type should remain azure_cli after tenant changes.""" mock_run.return_value = MagicMock( - returncode=0, stdout="tenant-A\n" + returncode=0, stdout=_az_account_json("tenant-A") ) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -540,7 +637,7 @@ def test_identity_type_preserved_after_tenant_change( # Re-login with different tenant mock_run.return_value = MagicMock( - returncode=0, stdout="tenant-B\n" + returncode=0, stdout=_az_account_json("tenant-B") ) auth.set_access_mode("azure_cli") auth.set_azure_cli() @@ -612,8 +709,8 @@ def test_az_not_installed_returns_none(self, monkeypatch, temp_dir_fixture): def test_cache_hit_before_ttl_expiry(self, mock_run, temp_dir_fixture): """Should return cached tenant without calling subprocess.""" auth = FabAuth() - auth._cached_az_tenant = "cached-tenant" - auth._cached_az_tenant_time = time.monotonic() # Just cached now + auth._cached_az_account = {"tenant_id": "cached-tenant", "principal_name": "user@test.com"} + auth._cached_az_account_time = time.monotonic() # Just cached now result = auth._get_azure_cli_tenant() assert result == "cached-tenant" mock_run.assert_not_called() @@ -621,10 +718,10 @@ def test_cache_hit_before_ttl_expiry(self, mock_run, temp_dir_fixture): @patch("subprocess.run") def test_cache_miss_after_ttl_expiry(self, mock_run, temp_dir_fixture): """Should call subprocess after TTL expires.""" - mock_run.return_value = MagicMock(returncode=0, stdout="new-tenant\n") + mock_run.return_value = MagicMock(returncode=0, stdout=_az_account_json("new-tenant")) auth = FabAuth() - auth._cached_az_tenant = "old-tenant" - auth._cached_az_tenant_time = time.monotonic() - 30 + auth._cached_az_account = {"tenant_id": "old-tenant", "principal_name": "user@test.com"} + auth._cached_az_account_time = time.monotonic() - 30 result = auth._get_azure_cli_tenant() assert result == "new-tenant" mock_run.assert_called_once() From 502224c47d022c9c970648f08c28f7a6d7279d1b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 19 Aug 2026 16:02:40 +0300 Subject: [PATCH 38/50] refactor: remove in-memory token cache and globalize AzureCliCredential - Remove _azure_cli_token_cache dict, _get_cached_azure_cli_token(), _cache_azure_cli_token(), and _AZURE_CLI_TOKEN_REFRESH_BUFFER_SECONDS - Add self._azure_cli_credential as singleton instance variable (consistent with self.app for MSAL), created lazily in _acquire_token_from_azure_cli(), cleared in set_azure_cli() and logout() - Ensures same credential instance across all 3 scopes, preventing cross-scope identity mismatch - Update tests: replace 4 cache tests with 3 singleton credential tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 51 ++---- tests/test_core/test_fab_auth_azure_cli.py | 149 +++++++----------- .../test_fab_msal_bridge_azure_cli.py | 2 +- 3 files changed, 76 insertions(+), 126 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 703ec87d8..9f8a6ace8 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -32,7 +32,6 @@ from fabric_cli.utils import fab_ui as utils_ui _AZURE_CLI_TENANT_CACHE_TTL_SECONDS = 10 -_AZURE_CLI_TOKEN_REFRESH_BUFFER_SECONDS = 60 def singleton(class_): @@ -57,8 +56,8 @@ def __init__(self): # Reset the auth info self.app: msal.ClientApplication = None self._auth_info = {} - # In-memory token cache for Azure CLI tokens - self._azure_cli_token_cache: dict[str, dict] = {} + # Singleton AzureCliCredential instance (like self.app for MSAL) + self._azure_cli_credential: Optional[AzureCliCredential] = None # Cached az account show result (tenant + principal in one call) self._cached_az_account: Optional[dict] = None self._cached_az_account_time: float = 0.0 @@ -437,9 +436,8 @@ def set_azure_cli(self, tenant_id=None): from Azure CLI's active session via 'az account show'. Always forces a fresh query (bypasses cache) since this is a login action. """ - # Clear token cache on every login to prevent stale tokens from a - # previous tenant (or no-tenant) session from being reused. - self._azure_cli_token_cache.clear() + # Clear credential to force recreation with the correct tenant + self._azure_cli_credential = None # Query Azure CLI account once for both tenant and principal account = self._get_azure_cli_account(force_refresh=True) # Set tenant first — set_tenant() may call logout() which clears auth info @@ -529,26 +527,20 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) - # Check in-memory cache first - cache_key = scope[0] if scope else "" - cached = self._get_cached_azure_cli_token(cache_key) - if cached: - return cached - try: - credential = ( - AzureCliCredential(tenant_id=stored_tenant) - if stored_tenant - else AzureCliCredential() - ) + # Create singleton credential if not yet initialized + if self._azure_cli_credential is None: + self._azure_cli_credential = ( + AzureCliCredential(tenant_id=stored_tenant) + if stored_tenant + else AzureCliCredential() + ) # AzureCliCredential.get_token expects scopes as positional args - azure_token = credential.get_token(scope[0]) + azure_token = self._azure_cli_credential.get_token(scope[0]) token_result = { "access_token": azure_token.token, "expires_on": azure_token.expires_on, } - # Cache the token - self._cache_azure_cli_token(cache_key, token_result) return token_result except CredentialUnavailableError: raise FabricCLIError( @@ -571,21 +563,6 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) - def _get_cached_azure_cli_token(self, cache_key: str) -> Optional[dict]: - """Return cached token if it exists and is not near expiry.""" - cached = self._azure_cli_token_cache.get(cache_key) - if ( - cached - and cached.get("expires_on", 0) - > time.time() + _AZURE_CLI_TOKEN_REFRESH_BUFFER_SECONDS - ): - return cached - return None - - def _cache_azure_cli_token(self, cache_key: str, token: dict) -> None: - """Cache a token by audience key.""" - self._azure_cli_token_cache[cache_key] = token - def print_auth_info(self): utils_ui.print_grey(json.dumps(self._get_auth_info(), indent=2)) @@ -716,8 +693,8 @@ def logout(self): self.app = None - # Clear Azure CLI caches - self._azure_cli_token_cache.clear() + # Clear Azure CLI state + self._azure_cli_credential = None self._cached_az_account = None self._cached_az_account_time = 0.0 diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 31ef06767..ddde4dfab 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -45,7 +45,7 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.delenv(var, raising=False) # Clear singleton caches between tests auth = FabAuth() - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None auth._cached_az_account = None auth._cached_az_account_time = 0.0 auth._auth_info = {} @@ -137,7 +137,7 @@ def test_acquire_token_dispatches_to_azure_cli( auth = FabAuth() auth.set_access_mode("azure_cli") # Clear cache for clean test - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) @@ -161,7 +161,7 @@ def test_acquire_token_from_azure_cli_success( auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -186,7 +186,7 @@ def test_acquire_token_from_azure_cli_with_tenant( auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="my-tenant-id") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -207,7 +207,7 @@ def test_acquire_token_from_azure_cli_credential_unavailable( auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -227,7 +227,7 @@ def test_sdk_exception_surfaces_message( auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -247,7 +247,7 @@ def test_unknown_exception_returns_safe_message( auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -272,7 +272,7 @@ def test_tenant_drift_blocks_token_acquisition( auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="original-tenant") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -303,7 +303,7 @@ def test_tenant_match_allows_token_acquisition( auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="same-tenant") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) assert result["access_token"] == "valid-token" @@ -325,7 +325,7 @@ def test_principal_drift_blocks_token_acquisition( auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="same-tenant") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None # Now az is logged in as bob (same tenant) mock_run.return_value = MagicMock( @@ -361,7 +361,7 @@ def test_principal_match_allows_token_acquisition( auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="same-tenant") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None auth._cached_az_account = None # Force re-query result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -388,22 +388,22 @@ def test_no_stored_principal_skips_drift_check( # Manually set identity without principal (simulate old auth file) auth._set_auth_properties({con.IDENTITY_TYPE: "azure_cli"}) auth.set_tenant("same-tenant") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) assert result["access_token"] == "valid-token" -class TestAzureCliTokenCache: - """Test in-memory token caching for Azure CLI tokens.""" +class TestAzureCliSingletonCredential: + """Test singleton AzureCliCredential lifecycle.""" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_cached_token_avoids_repeated_credential_calls( + def test_singleton_credential_reused_across_calls( self, mock_credential_class, temp_dir_fixture ): - """Second call with same scope should use cache, not call get_token again.""" + """Repeated calls should reuse the same AzureCliCredential instance.""" mock_token = MagicMock() - mock_token.token = "cached-token" + mock_token.token = "reused-token" mock_token.expires_on = int(time.time()) + 3600 mock_credential = MagicMock() @@ -412,73 +412,21 @@ def test_cached_token_avoids_repeated_credential_calls( auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None - result1 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - result2 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - - assert result1["access_token"] == "cached-token" - assert result2["access_token"] == "cached-token" - # get_token should only be called once (second call uses cache) - mock_credential.get_token.assert_called_once() - - @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_expired_cache_triggers_refresh( - self, mock_credential_class, temp_dir_fixture - ): - """Expired cached token should trigger a new credential token request.""" - mock_token = MagicMock() - mock_token.token = "fresh-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential - - auth = FabAuth() - auth.set_access_mode("azure_cli") - # Pre-populate cache with expired token - auth._azure_cli_token_cache.clear() - auth._azure_cli_token_cache[con.SCOPE_FABRIC_DEFAULT[0]] = { - "access_token": "old-token", - "expires_on": int(time.time()) - 10, # already expired - } - - result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "fresh-token" - mock_credential.get_token.assert_called_once() - - @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_near_expiry_within_buffer_triggers_refresh( - self, mock_credential_class, temp_dir_fixture - ): - """Token valid but expiring within 60s buffer should trigger refresh.""" - mock_token = MagicMock() - mock_token.token = "refreshed-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential - - auth = FabAuth() - auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() - # Token expires in 30s — still valid but within 60s buffer - auth._azure_cli_token_cache[con.SCOPE_FABRIC_DEFAULT[0]] = { - "access_token": "almost-expired-token", - "expires_on": int(time.time()) + 30, - } + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "refreshed-token" - mock_credential.get_token.assert_called_once() + # AzureCliCredential constructor called only once (singleton) + mock_credential_class.assert_called_once() + # get_token called twice (no in-memory cache) + assert mock_credential.get_token.call_count == 2 @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_different_scopes_cached_separately( + def test_different_scopes_use_same_credential( self, mock_credential_class, temp_dir_fixture ): - """Different scopes should have separate cache entries.""" + """Different scopes should use the same singleton credential instance.""" call_count = 0 def make_token(*args): @@ -495,15 +443,40 @@ def make_token(*args): auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None r1 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) r2 = auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) assert r1["access_token"] == "token-1" assert r2["access_token"] == "token-2" + # Same credential instance for both scopes + mock_credential_class.assert_called_once() assert mock_credential.get_token.call_count == 2 + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_login_clears_credential( + self, mock_credential_class, temp_dir_fixture + ): + """set_azure_cli should clear and recreate the credential instance.""" + mock_token = MagicMock() + mock_token.token = "token" + mock_token.expires_on = int(time.time()) + 3600 + + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_credential_class.return_value = mock_credential + + auth = FabAuth() + auth.set_access_mode("azure_cli") + # Acquire a token — creates singleton credential + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert auth._azure_cli_credential is not None + + # Login with explicit tenant — credential must be cleared + auth.set_azure_cli(tenant_id="new-tenant") + assert auth._azure_cli_credential is None + class TestAzureCliScopeHandling: """Test that different scopes are correctly passed to Azure CLI.""" @@ -521,7 +494,7 @@ def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) @@ -542,7 +515,7 @@ def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): auth = FabAuth() auth.set_access_mode("azure_cli") - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None auth._acquire_token_from_azure_cli(con.SCOPE_AZURE_DEFAULT) @@ -570,7 +543,7 @@ def test_logout_clears_tenant_cache(self, mock_run, temp_dir_fixture): auth.logout() assert auth._cached_az_account is None assert auth._cached_az_account_time == 0.0 - assert auth._azure_cli_token_cache == {} + assert auth._azure_cli_credential is None @patch("subprocess.run") def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): @@ -645,12 +618,12 @@ def test_identity_type_preserved_after_tenant_change( assert auth.get_tenant_id() == "tenant-B" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_login_clears_token_cache_from_no_tenant_state( + def test_login_clears_credential_on_tenant_change( self, mock_credential_class, temp_dir_fixture ): - """set_azure_cli should clear token cache even when transitioning from no tenant.""" + """set_azure_cli should clear credential when transitioning tenants.""" mock_token = MagicMock() - mock_token.token = "stale-token" + mock_token.token = "token" mock_token.expires_on = int(time.time()) + 3600 mock_credential = MagicMock() @@ -661,11 +634,11 @@ def test_login_clears_token_cache_from_no_tenant_state( auth.set_access_mode("azure_cli") # Acquire a token with no tenant stored auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert auth._azure_cli_token_cache.get(con.SCOPE_FABRIC_DEFAULT[0]) is not None + assert auth._azure_cli_credential is not None - # Login with explicit tenant — cache must be cleared + # Login with explicit tenant — credential must be cleared for recreation auth.set_azure_cli(tenant_id="new-tenant") - assert auth._azure_cli_token_cache.get(con.SCOPE_FABRIC_DEFAULT[0]) is None + assert auth._azure_cli_credential is None class TestAzureCliTenantDiscoveryFailures: diff --git a/tests/test_core/test_fab_msal_bridge_azure_cli.py b/tests/test_core/test_fab_msal_bridge_azure_cli.py index adc676619..9f66f0f1f 100644 --- a/tests/test_core/test_fab_msal_bridge_azure_cli.py +++ b/tests/test_core/test_fab_msal_bridge_azure_cli.py @@ -23,7 +23,7 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.delenv("FAB_TOKEN_ONELAKE", raising=False) monkeypatch.delenv("FAB_TOKEN_AZURE", raising=False) auth = FabAuth() - auth._azure_cli_token_cache.clear() + auth._azure_cli_credential = None auth._cached_az_tenant = None auth._cached_az_tenant_time = 0.0 auth._auth_info = {} From 178577502f304410299a9da0a0471bbd1eb09292 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 19 Aug 2026 16:38:39 +0300 Subject: [PATCH 39/50] refactor: single-channel Azure CLI via JWT-based drift detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove all subprocess calls to 'az account show' — AzureCliCredential is now the only interface to Azure CLI - Remove _get_azure_cli_account(), _get_azure_cli_tenant(), _get_azure_cli_principal(), cached account state, and TTL constant - Remove subprocess, shutil, time imports from fab_auth.py - Add _decode_jwt_claims() to extract tid/oid from token payload - set_azure_cli() now acquires a probe token and reads identity from JWT claims instead of a separate subprocess call - _acquire_token_from_azure_cli() verifies tid:oid from every returned token (post-acquisition drift detection) - Add 'except FabricCLIError: raise' to prevent drift errors from being swallowed by the generic exception handler - Update tests: JWT-based mocking, add decode/lifecycle tests, remove subprocess-based discovery failure tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 168 +++---- tests/test_core/test_fab_auth_azure_cli.py | 452 ++++++------------ .../test_fab_msal_bridge_azure_cli.py | 17 +- 3 files changed, 237 insertions(+), 400 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 9f8a6ace8..2b7f74bb0 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -3,9 +3,7 @@ import json import os -import shutil -import subprocess -import time +import base64 import uuid from binascii import hexlify from typing import Any, NamedTuple, Optional @@ -31,8 +29,6 @@ from fabric_cli.errors import ErrorMessages from fabric_cli.utils import fab_ui as utils_ui -_AZURE_CLI_TENANT_CACHE_TTL_SECONDS = 10 - def singleton(class_): instances = {} @@ -58,9 +54,6 @@ def __init__(self): self._auth_info = {} # Singleton AzureCliCredential instance (like self.app for MSAL) self._azure_cli_credential: Optional[AzureCliCredential] = None - # Cached az account show result (tenant + principal in one call) - self._cached_az_account: Optional[dict] = None - self._cached_az_account_time: float = 0.0 # Load the auth info and environment variables self._load_auth() @@ -432,100 +425,70 @@ def set_managed_identity(self, client_id=None): def set_azure_cli(self, tenant_id=None): """Configure Azure CLI as the authentication source. - If tenant_id is not provided, auto-captures the current tenant - from Azure CLI's active session via 'az account show'. - Always forces a fresh query (bypasses cache) since this is a login action. + Acquires a probe token from Azure CLI to discover and store + the tenant ID and principal OID from the actual JWT claims. + If tenant_id is provided, pins to that tenant; otherwise + auto-discovers from the token. """ # Clear credential to force recreation with the correct tenant self._azure_cli_credential = None - # Query Azure CLI account once for both tenant and principal - account = self._get_azure_cli_account(force_refresh=True) - # Set tenant first — set_tenant() may call logout() which clears auth info - if tenant_id: - self.set_tenant(tenant_id) - elif account and account.get("tenant_id"): - self.set_tenant(account["tenant_id"]) + + # Acquire a probe token to discover identity from JWT claims + try: + probe_credential = ( + AzureCliCredential(tenant_id=tenant_id) + if tenant_id + else AzureCliCredential() + ) + probe_token = probe_credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) + claims = self._decode_jwt_claims(probe_token.token) + except CredentialUnavailableError: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_not_available(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + + # Set tenant from explicit param or JWT tid claim + resolved_tenant = tenant_id or claims.get("tid") + if resolved_tenant: + self.set_tenant(resolved_tenant) + # Set identity_type after tenant to survive any logout triggered by tenant change auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"} - # Store principal for drift detection (no PII exposed in errors) - if account and account.get("principal_name"): - auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = account["principal_name"] + # Store OID for drift detection (immutable, no PII) + if claims.get("oid"): + auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"] self._set_auth_properties(auth_props) - def _get_azure_cli_account(self, force_refresh: bool = False) -> Optional[dict]: - """Query Azure CLI account info (tenant + principal) in a single subprocess call. - - Returns a dict with 'tenant_id' and 'principal_name' keys, or None - if Azure CLI is unavailable. Caches the result to avoid repeated - subprocess calls during multi-scope token acquisition flows. + @staticmethod + def _decode_jwt_claims(token: str) -> dict: + """Decode JWT payload claims without signature validation. - Args: - force_refresh: If True, bypass the cache and query az directly. + Used to extract identity claims (tid, oid) from tokens + returned by AzureCliCredential. Signature validation is + unnecessary here — the token was just returned by the + Azure CLI SDK over a local subprocess call. """ - if ( - not force_refresh - and self._cached_az_account is not None - and time.monotonic() - self._cached_az_account_time - < _AZURE_CLI_TENANT_CACHE_TTL_SECONDS - ): - return self._cached_az_account - try: - az_path = shutil.which("az") - if not az_path: - return None - result = subprocess.run( - [az_path, "account", "show", "--query", "{tenantId:tenantId,userName:user.name}", "-o", "json"], - capture_output=True, - text=True, - timeout=10, - ) - if result.returncode == 0 and result.stdout.strip(): - data = json.loads(result.stdout.strip()) - account_info = { - "tenant_id": data.get("tenantId"), - "principal_name": data.get("userName"), - } - self._cached_az_account = account_info - self._cached_az_account_time = time.monotonic() - return self._cached_az_account - except (subprocess.TimeoutExpired, FileNotFoundError, OSError, ValueError): - pass - return None - - def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]: - """Get the current Azure CLI tenant ID (thin wrapper over _get_azure_cli_account).""" - account = self._get_azure_cli_account(force_refresh=force_refresh) - return account.get("tenant_id") if account else None - - def _get_azure_cli_principal(self, force_refresh: bool = False) -> Optional[str]: - """Get the current Azure CLI principal name (thin wrapper over _get_azure_cli_account).""" - account = self._get_azure_cli_account(force_refresh=force_refresh) - return account.get("principal_name") if account else None + parts = token.split(".") + if len(parts) < 2: + return {} + # Add padding for base64url decoding + payload = parts[1] + payload += "=" * (4 - len(payload) % 4) + decoded = base64.urlsafe_b64decode(payload) + return json.loads(decoded) + except (ValueError, json.JSONDecodeError, UnicodeDecodeError): + return {} def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: - """Acquire a token using Azure CLI's AzureCliCredential.""" - # Tenant drift check: compare stored tenant against current az session - stored_tenant = self.get_tenant_id() - if stored_tenant: - current_tenant = self._get_azure_cli_tenant() - if current_tenant and current_tenant != stored_tenant: - raise FabricCLIError( - ErrorMessages.Auth.azure_cli_tenant_mismatch( - stored_tenant, current_tenant - ), - status_code=con.ERROR_AUTHENTICATION_FAILED, - ) + """Acquire a token using Azure CLI's AzureCliCredential. - # Principal drift check: detect identity change within same tenant - stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) - if stored_principal: - current_principal = self._get_azure_cli_principal() - if current_principal and current_principal != stored_principal: - raise FabricCLIError( - ErrorMessages.Auth.azure_cli_principal_mismatch(), - status_code=con.ERROR_AUTHENTICATION_FAILED, - ) + After acquiring the token, decodes JWT claims and verifies + that tid and oid match the stored values from login to detect + identity drift (e.g., user ran 'az login' as a different user). + """ + stored_tenant = self.get_tenant_id() try: # Create singleton credential if not yet initialized @@ -537,6 +500,29 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: ) # AzureCliCredential.get_token expects scopes as positional args azure_token = self._azure_cli_credential.get_token(scope[0]) + + # Post-acquisition drift detection from actual token claims + claims = self._decode_jwt_claims(azure_token.token) + + # Tenant drift check + if stored_tenant and claims.get("tid"): + if claims["tid"] != stored_tenant: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_tenant_mismatch( + stored_tenant, claims["tid"] + ), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + + # Principal drift check (OID-based) + stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) + if stored_principal and claims.get("oid"): + if claims["oid"] != stored_principal: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_principal_mismatch(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + token_result = { "access_token": azure_token.token, "expires_on": azure_token.expires_on, @@ -547,6 +533,8 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: ErrorMessages.Auth.azure_cli_not_available(), status_code=con.ERROR_AUTHENTICATION_FAILED, ) + except FabricCLIError: + raise except Exception as e: # Allowlist: SDK exceptions are pre-sanitized by azure-identity; unknown exceptions get a safe generic message if type(e).__name__ in ( @@ -695,8 +683,6 @@ def logout(self): # Clear Azure CLI state self._azure_cli_credential = None - self._cached_az_account = None - self._cached_az_account_time = 0.0 if os.path.exists(self.cache_file): os.remove(self.cache_file) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index ddde4dfab..407ffd27b 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -1,7 +1,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -import subprocess +import base64 +import json as _json import time from unittest.mock import MagicMock, patch @@ -13,11 +14,24 @@ from fabric_cli.errors import ErrorMessages -def _az_account_json(tenant_id: str, user_name: str = "testuser@contoso.com") -> str: - """Return mock JSON output for 'az account show'.""" - import json as _json +def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid", **extra_claims) -> str: + """Create a fake JWT with specified claims (no signature validation needed).""" + header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() + claims = {"tid": tid, "oid": oid, **extra_claims} + payload = base64.urlsafe_b64encode(_json.dumps(claims).encode()).rstrip(b"=").decode() + return f"{header}.{payload}.fakesig" - return _json.dumps({"tenantId": tenant_id, "userName": user_name}) + +def _mock_credential_with_jwt(mock_class, tid="test-tenant", oid="test-oid", **extra): + """Set up a mock AzureCliCredential that returns a JWT with given claims.""" + token_str = _make_jwt(tid=tid, oid=oid, **extra) + mock_token = MagicMock() + mock_token.token = token_str + mock_token.expires_on = int(time.time()) + 3600 + mock_credential = MagicMock() + mock_credential.get_token.return_value = mock_token + mock_class.return_value = mock_credential + return mock_credential, mock_token @pytest.fixture(autouse=True) @@ -26,11 +40,6 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.setattr( "fabric_cli.core.fab_state_config.config_location", lambda: str(tmp_path) ) - # Ensure shutil.which("az") resolves in tests (Windows uses az.cmd) - monkeypatch.setattr( - "shutil.which", - lambda cmd: f"/usr/bin/{cmd}" if cmd == "az" else None, - ) # Clear env vars that would interfere with auth for var in ( "FAB_TOKEN", @@ -43,11 +52,9 @@ def temp_dir_fixture(monkeypatch, tmp_path): "FAB_MANAGED_IDENTITY", ): monkeypatch.delenv(var, raising=False) - # Clear singleton caches between tests + # Clear singleton state between tests auth = FabAuth() auth._azure_cli_credential = None - auth._cached_az_account = None - auth._cached_az_account_time = 0.0 auth._auth_info = {} auth.app = None # Update file paths to use the test's tmp_path @@ -71,51 +78,43 @@ def test_set_access_mode_accepts_azure_cli(self, temp_dir_fixture): def test_set_azure_cli_sets_identity_type(self, temp_dir_fixture): """set_azure_cli should configure identity_type to azure_cli.""" - auth = FabAuth() - auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="test-tenant") - assert auth.get_identity_type() == "azure_cli" + with patch("fabric_cli.core.fab_auth.AzureCliCredential") as mock_class: + _mock_credential_with_jwt(mock_class, tid="test-tenant") + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="test-tenant") + assert auth.get_identity_type() == "azure_cli" def test_set_azure_cli_with_tenant(self, temp_dir_fixture): """set_azure_cli with tenant_id should store the tenant.""" - auth = FabAuth() - auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="test-tenant-id") - assert auth.get_tenant_id() == "test-tenant-id" + with patch("fabric_cli.core.fab_auth.AzureCliCredential") as mock_class: + _mock_credential_with_jwt(mock_class, tid="test-tenant-id") + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="test-tenant-id") + assert auth.get_tenant_id() == "test-tenant-id" - @patch("subprocess.run") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_set_azure_cli_auto_captures_tenant( - self, mock_run, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): - """set_azure_cli without tenant_id should auto-capture from az account show.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("auto-captured-tenant-id") - ) + """set_azure_cli without tenant_id should auto-capture from JWT claims.""" + _mock_credential_with_jwt(mock_credential_class, tid="auto-captured-tenant-id") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth.get_tenant_id() == "auto-captured-tenant-id" - mock_run.assert_called_once_with( - ["/usr/bin/az", "account", "show", "--query", "{tenantId:tenantId,userName:user.name}", "-o", "json"], - capture_output=True, - text=True, - timeout=10, - ) - @patch("subprocess.run") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_set_azure_cli_explicit_tenant_overrides_auto( - self, mock_run, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): - """Explicit tenant_id should be used even if az has a different one.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("other-tenant") - ) + """Explicit tenant_id should be used even if JWT has a different one.""" + _mock_credential_with_jwt(mock_credential_class, tid="other-tenant") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="explicit-tenant") assert auth.get_tenant_id() == "explicit-tenant" - # Still calls az account show once to capture principal for drift detection - mock_run.assert_called_once() class TestAzureCliTokenAcquisition: @@ -126,23 +125,16 @@ def test_acquire_token_dispatches_to_azure_cli( self, mock_credential_class, temp_dir_fixture ): """acquire_token should use AzureCliCredential for azure_cli identity.""" - mock_token = MagicMock() - mock_token.token = "fake-token-123" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + mock_credential, _ = _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") - # Clear cache for clean test auth._azure_cli_credential = None result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "fake-token-123" - mock_credential.get_token.assert_called_once_with( + assert "access_token" in result + mock_credential.get_token.assert_called_with( "https://api.fabric.microsoft.com/.default" ) @@ -151,13 +143,7 @@ def test_acquire_token_from_azure_cli_success( self, mock_credential_class, temp_dir_fixture ): """_acquire_token_from_azure_cli should return token dict on success.""" - mock_token = MagicMock() - mock_token.token = "az-cli-token-abc" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + mock_credential, mock_token = _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -165,7 +151,7 @@ def test_acquire_token_from_azure_cli_success( result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "az-cli-token-abc" + assert result["access_token"] == mock_token.token mock_credential.get_token.assert_called_once_with( "https://api.fabric.microsoft.com/.default" ) @@ -175,13 +161,7 @@ def test_acquire_token_from_azure_cli_with_tenant( self, mock_credential_class, temp_dir_fixture ): """_acquire_token_from_azure_cli should pass tenant_id to credential.""" - mock_token = MagicMock() - mock_token.token = "tenant-specific-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class, tid="my-tenant-id") auth = FabAuth() auth.set_access_mode("azure_cli") @@ -190,7 +170,9 @@ def test_acquire_token_from_azure_cli_with_tenant( auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - mock_credential_class.assert_called_once_with(tenant_id="my-tenant-id") + # Credential created with tenant_id (may be called twice: once at login, once here) + calls = mock_credential_class.call_args_list + assert any(c == ((), {"tenant_id": "my-tenant-id"}) for c in calls) @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_acquire_token_from_azure_cli_credential_unavailable( @@ -257,23 +239,23 @@ def test_unknown_exception_returns_safe_message( class TestAzureCliTenantDrift: - """Test tenant drift detection during token acquisition.""" + """Test tenant drift detection via JWT claims.""" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - @patch("subprocess.run") def test_tenant_drift_blocks_token_acquisition( - self, mock_run, mock_credential_class, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): - """Should block when stored tenant differs from current az session.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("different-tenant") - ) - + """Should block when token tid differs from stored tenant.""" + # Login with original-tenant + _mock_credential_with_jwt(mock_credential_class, tid="original-tenant", oid="user1") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="original-tenant") auth._azure_cli_credential = None + # Now credential returns token for different-tenant + _mock_credential_with_jwt(mock_credential_class, tid="different-tenant", oid="user1") + with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -281,24 +263,13 @@ def test_tenant_drift_blocks_token_acquisition( "original-tenant", "different-tenant" ) assert expected_msg in str(exc_info.value) - mock_credential_class.assert_not_called() @patch("fabric_cli.core.fab_auth.AzureCliCredential") - @patch("subprocess.run") def test_tenant_match_allows_token_acquisition( - self, mock_run, mock_credential_class, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): - """Should allow when stored tenant matches current az session.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("same-tenant") - ) - mock_token = MagicMock() - mock_token.token = "valid-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + """Should allow when token tid matches stored tenant.""" + _mock_credential_with_jwt(mock_credential_class, tid="same-tenant", oid="user1") auth = FabAuth() auth.set_access_mode("azure_cli") @@ -306,82 +277,56 @@ def test_tenant_match_allows_token_acquisition( auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "valid-token" + assert "access_token" in result class TestAzureCliPrincipalDrift: - """Test principal (identity) drift detection during token acquisition.""" + """Test principal (identity) drift detection via JWT OID claims.""" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - @patch("subprocess.run") def test_principal_drift_blocks_token_acquisition( - self, mock_run, mock_credential_class, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): - """Should block when stored principal differs from current az identity.""" - # Login as alice - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("same-tenant", "alice@contoso.com") - ) + """Should block when token oid differs from stored principal.""" + # Login as alice (oid=alice-oid) + _mock_credential_with_jwt(mock_credential_class, tid="same-tenant", oid="alice-oid") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="same-tenant") auth._azure_cli_credential = None - # Now az is logged in as bob (same tenant) - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("same-tenant", "bob@contoso.com") - ) - auth._cached_az_account = None # Force re-query + # Now credential returns token for bob (oid=bob-oid, same tenant) + _mock_credential_with_jwt(mock_credential_class, tid="same-tenant", oid="bob-oid") with pytest.raises(FabricCLIError) as exc_info: auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - # Error message must NOT contain PII (no email addresses) + # Error message must NOT contain PII (no OIDs exposed) assert "alice" not in str(exc_info.value) assert "bob" not in str(exc_info.value) assert "identity has changed" in str(exc_info.value) - mock_credential_class.assert_not_called() @patch("fabric_cli.core.fab_auth.AzureCliCredential") - @patch("subprocess.run") def test_principal_match_allows_token_acquisition( - self, mock_run, mock_credential_class, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): - """Should allow when principal matches stored identity.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("same-tenant", "alice@contoso.com") - ) - mock_token = MagicMock() - mock_token.token = "valid-token" - mock_token.expires_on = int(time.time()) + 3600 - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + """Should allow when token oid matches stored principal.""" + _mock_credential_with_jwt(mock_credential_class, tid="same-tenant", oid="alice-oid") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli(tenant_id="same-tenant") auth._azure_cli_credential = None - auth._cached_az_account = None # Force re-query result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "valid-token" + assert "access_token" in result @patch("fabric_cli.core.fab_auth.AzureCliCredential") - @patch("subprocess.run") def test_no_stored_principal_skips_drift_check( - self, mock_run, mock_credential_class, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): """If no principal was stored at login, drift check is skipped.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("same-tenant", "anyone@contoso.com") - ) - mock_token = MagicMock() - mock_token.token = "valid-token" - mock_token.expires_on = int(time.time()) + 3600 - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class, tid="same-tenant", oid="anyone-oid") auth = FabAuth() auth.set_access_mode("azure_cli") @@ -391,7 +336,7 @@ def test_no_stored_principal_skips_drift_check( auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "valid-token" + assert "access_token" in result class TestAzureCliSingletonCredential: @@ -402,13 +347,7 @@ def test_singleton_credential_reused_across_calls( self, mock_credential_class, temp_dir_fixture ): """Repeated calls should reuse the same AzureCliCredential instance.""" - mock_token = MagicMock() - mock_token.token = "reused-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -420,52 +359,32 @@ def test_singleton_credential_reused_across_calls( # AzureCliCredential constructor called only once (singleton) mock_credential_class.assert_called_once() # get_token called twice (no in-memory cache) - assert mock_credential.get_token.call_count == 2 + assert mock_credential_class.return_value.get_token.call_count == 2 @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_different_scopes_use_same_credential( self, mock_credential_class, temp_dir_fixture ): """Different scopes should use the same singleton credential instance.""" - call_count = 0 - - def make_token(*args): - nonlocal call_count - call_count += 1 - token = MagicMock() - token.token = f"token-{call_count}" - token.expires_on = int(time.time()) + 3600 - return token - - mock_credential = MagicMock() - mock_credential.get_token.side_effect = make_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") auth._azure_cli_credential = None - r1 = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - r2 = auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) - assert r1["access_token"] == "token-1" - assert r2["access_token"] == "token-2" # Same credential instance for both scopes mock_credential_class.assert_called_once() - assert mock_credential.get_token.call_count == 2 + assert mock_credential_class.return_value.get_token.call_count == 2 @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_login_clears_credential( self, mock_credential_class, temp_dir_fixture ): """set_azure_cli should clear and recreate the credential instance.""" - mock_token = MagicMock() - mock_token.token = "token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -484,13 +403,7 @@ class TestAzureCliScopeHandling: @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): """OneLake scope should be passed correctly.""" - mock_token = MagicMock() - mock_token.token = "storage-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + mock_credential, _ = _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -505,13 +418,7 @@ def test_onelake_scope(self, mock_credential_class, temp_dir_fixture): @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): """Azure management scope should be passed correctly.""" - mock_token = MagicMock() - mock_token.token = "mgmt-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + mock_credential, _ = _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -524,94 +431,74 @@ def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture): ) -class TestAzureCliCacheInvalidation: - """Test cache invalidation on logout and forced refresh at login.""" +class TestAzureCliLoginLogoutLifecycle: + """Test login/logout lifecycle and credential management.""" - @patch("subprocess.run") - def test_logout_clears_tenant_cache(self, mock_run, temp_dir_fixture): - """logout() should clear the cached account info.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("cached-tenant") - ) + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_logout_clears_credential(self, mock_credential_class, temp_dir_fixture): + """logout() should clear the credential instance.""" + _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() - assert auth._cached_az_account is not None - assert auth._cached_az_account["tenant_id"] == "cached-tenant" + assert auth._azure_cli_credential is None # cleared after login probe + + # Acquire token to set credential + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert auth._azure_cli_credential is not None auth.logout() - assert auth._cached_az_account is None - assert auth._cached_az_account_time == 0.0 assert auth._azure_cli_credential is None - @patch("subprocess.run") - def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture): - """set_azure_cli should bypass cache and query az fresh.""" - # First call returns tenant-A - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("tenant-A") - ) + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_login_discovers_tenant_from_jwt(self, mock_credential_class, temp_dir_fixture): + """set_azure_cli should discover tenant from probe token JWT claims.""" + _mock_credential_with_jwt(mock_credential_class, tid="discovered-tenant") + auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() - assert auth.get_tenant_id() == "tenant-A" - - # Simulate user switching az tenant, then re-logging in - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("tenant-B") - ) - auth.set_access_mode("azure_cli") - auth.set_azure_cli() # Should force refresh, get tenant-B - assert auth.get_tenant_id() == "tenant-B" + assert auth.get_tenant_id() == "discovered-tenant" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - @patch("subprocess.run") - def test_single_subprocess_across_three_login_scopes( - self, mock_run, mock_credential_class, temp_dir_fixture - ): - """Login should call az account show only once across 3 scope validations.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("login-tenant") - ) - - mock_token = MagicMock() - mock_token.token = "login-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + def test_login_stores_oid_for_drift_detection(self, mock_credential_class, temp_dir_fixture): + """set_azure_cli should store OID from JWT for drift detection.""" + _mock_credential_with_jwt(mock_credential_class, tid="t1", oid="user-oid-123") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli() # 1 subprocess call (force_refresh) + auth.set_azure_cli() + assert auth._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) == "user-oid-123" - # 3 scope validations — each calls drift check, but cache should hit - auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - auth._acquire_token_from_azure_cli(con.SCOPE_ONELAKE_DEFAULT) - auth._acquire_token_from_azure_cli(con.SCOPE_AZURE_DEFAULT) + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_re_login_updates_tenant_and_oid(self, mock_credential_class, temp_dir_fixture): + """Re-login should update tenant and OID from new probe token.""" + _mock_credential_with_jwt(mock_credential_class, tid="tenant-A", oid="oid-A") + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_tenant_id() == "tenant-A" - # az account show called once at login, cached for drift checks - assert mock_run.call_count == 1 + # Re-login with different identity + _mock_credential_with_jwt(mock_credential_class, tid="tenant-B", oid="oid-B") + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + assert auth.get_tenant_id() == "tenant-B" + assert auth._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) == "oid-B" - @patch("subprocess.run") + @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_identity_type_preserved_after_tenant_change( - self, mock_run, temp_dir_fixture + self, mock_credential_class, temp_dir_fixture ): """identity_type should remain azure_cli after tenant changes.""" - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("tenant-A") - ) + _mock_credential_with_jwt(mock_credential_class, tid="tenant-A") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth.get_identity_type() == "azure_cli" - # Re-login with different tenant - mock_run.return_value = MagicMock( - returncode=0, stdout=_az_account_json("tenant-B") - ) + _mock_credential_with_jwt(mock_credential_class, tid="tenant-B") auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth.get_identity_type() == "azure_cli" @@ -622,17 +509,11 @@ def test_login_clears_credential_on_tenant_change( self, mock_credential_class, temp_dir_fixture ): """set_azure_cli should clear credential when transitioning tenants.""" - mock_token = MagicMock() - mock_token.token = "token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") - # Acquire a token with no tenant stored + # Acquire a token to set credential auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) assert auth._azure_cli_credential is not None @@ -641,63 +522,30 @@ def test_login_clears_credential_on_tenant_change( assert auth._azure_cli_credential is None -class TestAzureCliTenantDiscoveryFailures: - """Test _get_azure_cli_tenant failure paths.""" +class TestJwtClaimsDecoding: + """Test the _decode_jwt_claims helper.""" - @patch("subprocess.run") - def test_nonzero_return_code_returns_none(self, mock_run, temp_dir_fixture): - """Should return None when az account show fails.""" - mock_run.return_value = MagicMock(returncode=1, stdout="") - auth = FabAuth() - auth._cached_az_tenant = None - auth._cached_az_tenant_time = 0.0 - assert auth._get_azure_cli_tenant(force_refresh=True) is None - - @patch("subprocess.run") - def test_empty_stdout_returns_none(self, mock_run, temp_dir_fixture): - """Should return None when az returns empty stdout.""" - mock_run.return_value = MagicMock(returncode=0, stdout=" \n") + def test_valid_jwt_extracts_claims(self, temp_dir_fixture): + """Should decode tid and oid from a valid JWT.""" + token = _make_jwt(tid="my-tenant", oid="my-oid") auth = FabAuth() - auth._cached_az_tenant = None - auth._cached_az_tenant_time = 0.0 - assert auth._get_azure_cli_tenant(force_refresh=True) is None + claims = auth._decode_jwt_claims(token) + assert claims["tid"] == "my-tenant" + assert claims["oid"] == "my-oid" - @patch("subprocess.run", side_effect=subprocess.TimeoutExpired("az", 10)) - def test_timeout_returns_none(self, mock_run, temp_dir_fixture): - """Should return None on subprocess timeout.""" + def test_invalid_jwt_returns_empty(self, temp_dir_fixture): + """Should return empty dict for malformed tokens.""" auth = FabAuth() - auth._cached_az_tenant = None - auth._cached_az_tenant_time = 0.0 - assert auth._get_azure_cli_tenant(force_refresh=True) is None + assert auth._decode_jwt_claims("not-a-jwt") == {} + assert auth._decode_jwt_claims("") == {} + assert auth._decode_jwt_claims("a.!!!.c") == {} - def test_az_not_installed_returns_none(self, monkeypatch, temp_dir_fixture): - """Should return None when az CLI is not installed.""" - monkeypatch.setattr("shutil.which", lambda cmd: None) + def test_jwt_with_extra_claims(self, temp_dir_fixture): + """Should extract additional claims.""" + token = _make_jwt(tid="t1", oid="o1", upn="user@contoso.com") auth = FabAuth() - auth._cached_az_tenant = None - auth._cached_az_tenant_time = 0.0 - assert auth._get_azure_cli_tenant(force_refresh=True) is None - - @patch("subprocess.run") - def test_cache_hit_before_ttl_expiry(self, mock_run, temp_dir_fixture): - """Should return cached tenant without calling subprocess.""" - auth = FabAuth() - auth._cached_az_account = {"tenant_id": "cached-tenant", "principal_name": "user@test.com"} - auth._cached_az_account_time = time.monotonic() # Just cached now - result = auth._get_azure_cli_tenant() - assert result == "cached-tenant" - mock_run.assert_not_called() - - @patch("subprocess.run") - def test_cache_miss_after_ttl_expiry(self, mock_run, temp_dir_fixture): - """Should call subprocess after TTL expires.""" - mock_run.return_value = MagicMock(returncode=0, stdout=_az_account_json("new-tenant")) - auth = FabAuth() - auth._cached_az_account = {"tenant_id": "old-tenant", "principal_name": "user@test.com"} - auth._cached_az_account_time = time.monotonic() - 30 - result = auth._get_azure_cli_tenant() - assert result == "new-tenant" - mock_run.assert_called_once() + claims = auth._decode_jwt_claims(token) + assert claims["upn"] == "user@contoso.com" class TestNonAzureCliIsolation: @@ -747,13 +595,7 @@ def test_azure_cli_does_not_invoke_msal_app( self, mock_credential_class, temp_dir_fixture ): """When identity_type is 'azure_cli', MSAL app methods must not be called.""" - mock_token = MagicMock() - mock_token.token = "az-cli-token" - mock_token.expires_on = int(time.time()) + 3600 - - mock_credential = MagicMock() - mock_credential.get_token.return_value = mock_token - mock_credential_class.return_value = mock_credential + _mock_credential_with_jwt(mock_credential_class) auth = FabAuth() auth.set_access_mode("azure_cli") @@ -762,7 +604,7 @@ def test_azure_cli_does_not_invoke_msal_app( auth.app = mock_app result = auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) - assert result["access_token"] == "az-cli-token" + assert "access_token" in result mock_app.acquire_token_silent.assert_not_called() mock_app.acquire_token_interactive.assert_not_called() mock_app.acquire_token_for_client.assert_not_called() diff --git a/tests/test_core/test_fab_msal_bridge_azure_cli.py b/tests/test_core/test_fab_msal_bridge_azure_cli.py index 9f66f0f1f..3e301b78a 100644 --- a/tests/test_core/test_fab_msal_bridge_azure_cli.py +++ b/tests/test_core/test_fab_msal_bridge_azure_cli.py @@ -3,6 +3,8 @@ """Tests for the MSAL bridge with Azure CLI identity type.""" +import base64 +import json as _json import time from unittest.mock import MagicMock, patch @@ -13,6 +15,14 @@ from fabric_cli.core.fab_msal_bridge import MsalTokenCredential +def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid") -> str: + """Create a fake JWT with specified claims.""" + header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() + claims = {"tid": tid, "oid": oid} + payload = base64.urlsafe_b64encode(_json.dumps(claims).encode()).rstrip(b"=").decode() + return f"{header}.{payload}.fakesig" + + @pytest.fixture(autouse=True) def temp_dir_fixture(monkeypatch, tmp_path): """Isolate FabAuth singleton for bridge tests.""" @@ -24,8 +34,6 @@ def temp_dir_fixture(monkeypatch, tmp_path): monkeypatch.delenv("FAB_TOKEN_AZURE", raising=False) auth = FabAuth() auth._azure_cli_credential = None - auth._cached_az_tenant = None - auth._cached_az_tenant_time = 0.0 auth._auth_info = {} @@ -37,8 +45,9 @@ def test_bridge_returns_access_token_for_azure_cli( self, mock_credential_class ): """MsalTokenCredential.get_token should return an AccessToken via Azure CLI.""" + token_str = _make_jwt() mock_token = MagicMock() - mock_token.token = "bridge-azure-cli-token" + mock_token.token = token_str mock_token.expires_on = int(time.time()) + 3600 mock_credential = MagicMock() @@ -51,7 +60,7 @@ def test_bridge_returns_access_token_for_azure_cli( credential = MsalTokenCredential(auth) result = credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) - assert result.token == "bridge-azure-cli-token" + assert result.token == token_str assert result.expires_on == mock_token.expires_on @patch("fabric_cli.core.fab_auth.AzureCliCredential") From 8b8283193b8fb64ef4968576aec3decf7b6000cc Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 19 Aug 2026 16:48:37 +0300 Subject: [PATCH 40/50] feat: add issuer (iss) to drift detection key for sovereign cloud safety - Add FAB_AZURE_CLI_ISSUER constant and store iss claim at login - Check iss before tid and oid on every token acquisition - Drift key is now iss + tid + oid (environment + tenant + identity) - Add azure_cli_environment_mismatch error message - Add TestAzureCliEnvironmentDrift tests (public vs sovereign) - Update JWT test helpers to include iss claim Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 19 ++++-- src/fabric_cli/core/fab_constant.py | 1 + src/fabric_cli/errors/auth.py | 7 ++ tests/test_core/test_fab_auth_azure_cli.py | 65 +++++++++++++++++-- .../test_fab_msal_bridge_azure_cli.py | 2 +- 5 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 2b7f74bb0..31e25b17c 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -455,16 +455,18 @@ def set_azure_cli(self, tenant_id=None): # Set identity_type after tenant to survive any logout triggered by tenant change auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"} - # Store OID for drift detection (immutable, no PII) + # Store OID and issuer for drift detection (immutable, no PII) if claims.get("oid"): auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"] + if claims.get("iss"): + auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"] self._set_auth_properties(auth_props) @staticmethod def _decode_jwt_claims(token: str) -> dict: """Decode JWT payload claims without signature validation. - Used to extract identity claims (tid, oid) from tokens + Used to extract identity claims (iss, tid, oid) from tokens returned by AzureCliCredential. Signature validation is unnecessary here — the token was just returned by the Azure CLI SDK over a local subprocess call. @@ -485,8 +487,8 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: """Acquire a token using Azure CLI's AzureCliCredential. After acquiring the token, decodes JWT claims and verifies - that tid and oid match the stored values from login to detect - identity drift (e.g., user ran 'az login' as a different user). + that iss, tid, and oid match the stored values from login to detect + identity or environment drift. """ stored_tenant = self.get_tenant_id() @@ -504,6 +506,15 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: # Post-acquisition drift detection from actual token claims claims = self._decode_jwt_claims(azure_token.token) + # Environment drift check (issuer encodes cloud: public vs sovereign) + stored_issuer = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER) + if stored_issuer and claims.get("iss"): + if claims["iss"] != stored_issuer: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_environment_mismatch(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + # Tenant drift check if stored_tenant and claims.get("tid"): if claims["tid"] != stored_tenant: diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index a28e6e5c7..466f571d0 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -57,6 +57,7 @@ FAB_REFRESH_TOKEN = "fab_refresh_token" FAB_AZURE_CLI_PRINCIPAL_ID = "fab_azure_cli_principal_id" +FAB_AZURE_CLI_ISSUER = "fab_azure_cli_issuer" IDENTITY_TYPE = "identity_type" FAB_AUTH_MODE = "fab_auth_mode" # Kept for backward compatibility FAB_AUTHORITY = "fab_authority" diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index 5e44c42a4..a460c9529 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -128,6 +128,13 @@ def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str: "Run 'fab auth login --azure-cli' to re-authenticate." ) + @staticmethod + def azure_cli_environment_mismatch() -> str: + return ( + "Azure CLI cloud environment has changed since 'fab auth login --azure-cli' was run. " + "Run 'fab auth login --azure-cli' to re-authenticate in the current environment." + ) + @staticmethod def azure_cli_principal_mismatch() -> str: return ( diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 407ffd27b..7fd870b8d 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -14,17 +14,19 @@ from fabric_cli.errors import ErrorMessages -def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid", **extra_claims) -> str: +def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid", + iss: str = "https://sts.windows.net/test-tenant/", **extra_claims) -> str: """Create a fake JWT with specified claims (no signature validation needed).""" header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() - claims = {"tid": tid, "oid": oid, **extra_claims} + claims = {"tid": tid, "oid": oid, "iss": iss, **extra_claims} payload = base64.urlsafe_b64encode(_json.dumps(claims).encode()).rstrip(b"=").decode() return f"{header}.{payload}.fakesig" -def _mock_credential_with_jwt(mock_class, tid="test-tenant", oid="test-oid", **extra): +def _mock_credential_with_jwt(mock_class, tid="test-tenant", oid="test-oid", + iss="https://sts.windows.net/test-tenant/", **extra): """Set up a mock AzureCliCredential that returns a JWT with given claims.""" - token_str = _make_jwt(tid=tid, oid=oid, **extra) + token_str = _make_jwt(tid=tid, oid=oid, iss=iss, **extra) mock_token = MagicMock() mock_token.token = token_str mock_token.expires_on = int(time.time()) + 3600 @@ -280,6 +282,53 @@ def test_tenant_match_allows_token_acquisition( assert "access_token" in result +class TestAzureCliEnvironmentDrift: + """Test cloud environment drift detection via JWT iss claim.""" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_environment_drift_blocks_token_acquisition( + self, mock_credential_class, temp_dir_fixture + ): + """Should block when token issuer differs from stored environment.""" + # Login in Azure Public + _mock_credential_with_jwt( + mock_credential_class, tid="t1", oid="u1", + iss="https://sts.windows.net/t1/" + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + auth._azure_cli_credential = None + + # Now credential returns token from Azure Government + _mock_credential_with_jwt( + mock_credential_class, tid="t1", oid="u1", + iss="https://sts.microsoftonline.us/t1/" + ) + + with pytest.raises(FabricCLIError) as exc_info: + auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + + assert "environment has changed" in str(exc_info.value) + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_same_environment_allows_token_acquisition( + self, mock_credential_class, temp_dir_fixture + ): + """Should allow when token issuer matches stored environment.""" + _mock_credential_with_jwt( + mock_credential_class, tid="t1", oid="u1", + iss="https://sts.windows.net/t1/" + ) + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli() + auth._azure_cli_credential = None + + result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) + assert "access_token" in result + + class TestAzureCliPrincipalDrift: """Test principal (identity) drift detection via JWT OID claims.""" @@ -462,14 +511,16 @@ def test_login_discovers_tenant_from_jwt(self, mock_credential_class, temp_dir_f assert auth.get_tenant_id() == "discovered-tenant" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_login_stores_oid_for_drift_detection(self, mock_credential_class, temp_dir_fixture): - """set_azure_cli should store OID from JWT for drift detection.""" - _mock_credential_with_jwt(mock_credential_class, tid="t1", oid="user-oid-123") + def test_login_stores_oid_and_issuer_for_drift_detection(self, mock_credential_class, temp_dir_fixture): + """set_azure_cli should store OID and issuer from JWT for drift detection.""" + _mock_credential_with_jwt(mock_credential_class, tid="t1", oid="user-oid-123", + iss="https://sts.windows.net/t1/") auth = FabAuth() auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) == "user-oid-123" + assert auth._auth_info.get(con.FAB_AZURE_CLI_ISSUER) == "https://sts.windows.net/t1/" @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_re_login_updates_tenant_and_oid(self, mock_credential_class, temp_dir_fixture): diff --git a/tests/test_core/test_fab_msal_bridge_azure_cli.py b/tests/test_core/test_fab_msal_bridge_azure_cli.py index 3e301b78a..7f815a5de 100644 --- a/tests/test_core/test_fab_msal_bridge_azure_cli.py +++ b/tests/test_core/test_fab_msal_bridge_azure_cli.py @@ -18,7 +18,7 @@ def _make_jwt(tid: str = "test-tenant", oid: str = "test-oid") -> str: """Create a fake JWT with specified claims.""" header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() - claims = {"tid": tid, "oid": oid} + claims = {"tid": tid, "oid": oid, "iss": f"https://sts.windows.net/{tid}/"} payload = base64.urlsafe_b64encode(_json.dumps(claims).encode()).rstrip(b"=").decode() return f"{header}.{payload}.fakesig" From 2d38b9721d531e662c2c5cdf35372fdacba38644 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 09:54:33 +0300 Subject: [PATCH 41/50] security: fail-closed on missing JWT identity claims Reject tokens that lack iss, tid, or oid instead of silently skipping drift checks. Applied at both login (set_azure_cli) and token acquisition (_acquire_token_from_azure_cli). Also fixed redundant nested if in environment drift check and incorrect indentation in principal drift check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 55 ++++++++++++------- src/fabric_cli/errors/auth.py | 8 +++ tests/test_core/test_fab_auth_azure_cli.py | 63 ++++++++++++++++++++++ 3 files changed, 107 insertions(+), 19 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 31e25b17c..d6a8f015f 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -448,18 +448,27 @@ def set_azure_cli(self, tenant_id=None): status_code=con.ERROR_AUTHENTICATION_FAILED, ) + # Fail-closed: refuse to persist if identity claims are missing + if ( + not claims.get("iss") + or not claims.get("tid") + or not claims.get("oid") + ): + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_token_missing_claims(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + # Set tenant from explicit param or JWT tid claim - resolved_tenant = tenant_id or claims.get("tid") + resolved_tenant = tenant_id or claims["tid"] if resolved_tenant: self.set_tenant(resolved_tenant) # Set identity_type after tenant to survive any logout triggered by tenant change auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"} # Store OID and issuer for drift detection (immutable, no PII) - if claims.get("oid"): - auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"] - if claims.get("iss"): - auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"] + auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"] + auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"] self._set_auth_properties(auth_props) @staticmethod @@ -506,30 +515,38 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: # Post-acquisition drift detection from actual token claims claims = self._decode_jwt_claims(azure_token.token) + # Fail-closed: reject tokens with missing identity claims + if ( + not claims.get("iss") + or not claims.get("tid") + or not claims.get("oid") + ): + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_token_missing_claims(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + # Environment drift check (issuer encodes cloud: public vs sovereign) stored_issuer = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER) - if stored_issuer and claims.get("iss"): - if claims["iss"] != stored_issuer: - raise FabricCLIError( + if stored_issuer and claims["iss"] != stored_issuer: + raise FabricCLIError( ErrorMessages.Auth.azure_cli_environment_mismatch(), status_code=con.ERROR_AUTHENTICATION_FAILED, ) # Tenant drift check - if stored_tenant and claims.get("tid"): - if claims["tid"] != stored_tenant: - raise FabricCLIError( - ErrorMessages.Auth.azure_cli_tenant_mismatch( - stored_tenant, claims["tid"] - ), - status_code=con.ERROR_AUTHENTICATION_FAILED, - ) + if stored_tenant and claims["tid"] != stored_tenant: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_tenant_mismatch( + stored_tenant, claims["tid"] + ), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) # Principal drift check (OID-based) stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) - if stored_principal and claims.get("oid"): - if claims["oid"] != stored_principal: - raise FabricCLIError( + if stored_principal and claims["oid"] != stored_principal: + raise FabricCLIError( ErrorMessages.Auth.azure_cli_principal_mismatch(), status_code=con.ERROR_AUTHENTICATION_FAILED, ) diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index a460c9529..c24e432ab 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -153,6 +153,14 @@ def azure_cli_not_available() -> str: def azure_cli_auth_failed(error_msg: str) -> str: return f"Azure CLI authentication failed: {error_msg}" + @staticmethod + def azure_cli_token_missing_claims() -> str: + return ( + "Azure CLI returned a token with missing identity claims (iss, tid, or oid). " + "Run 'az account get-access-token --resource https://api.fabric.microsoft.com' " + "manually to diagnose." + ) + @staticmethod def azure_cli_token_acquisition_failed() -> str: return ( diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 7fd870b8d..5346c83b1 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -599,6 +599,69 @@ def test_jwt_with_extra_claims(self, temp_dir_fixture): assert claims["upn"] == "user@contoso.com" +class TestFailClosedOnMissingClaims: + """Verify tokens with missing identity claims are rejected, not silently used.""" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_login_rejects_token_missing_oid(self, mock_class, temp_dir_fixture): + """set_azure_cli should fail if probe token lacks oid.""" + header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() + payload = base64.urlsafe_b64encode( + _json.dumps({"tid": "t1", "iss": "https://sts.windows.net/t1/"}).encode() + ).rstrip(b"=").decode() + token_str = f"{header}.{payload}.fakesig" + mock_token = MagicMock() + mock_token.token = token_str + mock_token.expires_on = int(time.time()) + 3600 + mock_class.return_value.get_token.return_value = mock_token + auth = FabAuth() + with pytest.raises(FabricCLIError, match="missing identity claims"): + auth.set_azure_cli() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_login_rejects_token_missing_tid(self, mock_class, temp_dir_fixture): + """set_azure_cli should fail if probe token lacks tid.""" + header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() + payload = base64.urlsafe_b64encode( + _json.dumps({"oid": "o1", "iss": "https://sts.windows.net/t1/"}).encode() + ).rstrip(b"=").decode() + token_str = f"{header}.{payload}.fakesig" + mock_token = MagicMock() + mock_token.token = token_str + mock_token.expires_on = int(time.time()) + 3600 + mock_class.return_value.get_token.return_value = mock_token + auth = FabAuth() + with pytest.raises(FabricCLIError, match="missing identity claims"): + auth.set_azure_cli() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_login_rejects_malformed_token(self, mock_class, temp_dir_fixture): + """set_azure_cli should fail if probe token is not a valid JWT.""" + mock_token = MagicMock() + mock_token.token = "not-a-jwt" + mock_token.expires_on = int(time.time()) + 3600 + mock_class.return_value.get_token.return_value = mock_token + auth = FabAuth() + with pytest.raises(FabricCLIError, match="missing identity claims"): + auth.set_azure_cli() + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_acquisition_rejects_token_missing_claims(self, mock_class, temp_dir_fixture): + """Token acquisition should fail if returned token lacks identity claims.""" + # Login with good token + _mock_credential_with_jwt(mock_class) + auth = FabAuth() + auth.set_azure_cli() + + # Now return a bad token on next call + bad_token = MagicMock() + bad_token.token = "not-a-jwt" + bad_token.expires_on = int(time.time()) + 3600 + mock_class.return_value.get_token.return_value = bad_token + with pytest.raises(FabricCLIError, match="missing identity claims"): + auth.acquire_token(con.SCOPE_FABRIC_DEFAULT) + + class TestNonAzureCliIsolation: """Verify each auth method uses only its own credential path — no overlap.""" From f819ded9de6bb2267e4e83badcf942d5c6ab6b3b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 10:11:20 +0300 Subject: [PATCH 42/50] refactor: strict Azure CLI context inheritance, no tenant override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove tenant_id parameter from set_azure_cli() — Fabric CLI always inherits the tenant from Azure CLI's active session via JWT tid claim. Users must switch tenants via 'az login --tenant' rather than overriding at the Fabric CLI level. Also removes tenant pinning from AzureCliCredential at acquisition time, ensuring drift detection fires if the user switches Azure CLI context between calls. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 4 +-- src/fabric_cli/core/fab_auth.py | 29 ++++++--------- tests/test_core/test_fab_auth_azure_cli.py | 42 +++++++++++----------- 3 files changed, 33 insertions(+), 42 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 6e2882faa..f15b3088a 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -30,7 +30,7 @@ def init(args: Namespace) -> Any: if getattr(args, "azure_cli", False): FabAuth().set_access_mode("azure_cli", args.tenant) - FabAuth().set_azure_cli(args.tenant) + FabAuth().set_azure_cli() _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" @@ -78,7 +78,7 @@ def init(args: Namespace) -> Any: Context().context = FabAuth().get_tenant() elif selected_auth.startswith("Azure CLI"): FabAuth().set_access_mode("azure_cli", args.tenant) - FabAuth().set_azure_cli(args.tenant) + FabAuth().set_azure_cli() _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index d6a8f015f..6d70b8d68 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -422,24 +422,21 @@ def set_managed_identity(self, client_id=None): } ) - def set_azure_cli(self, tenant_id=None): + def set_azure_cli(self): """Configure Azure CLI as the authentication source. Acquires a probe token from Azure CLI to discover and store the tenant ID and principal OID from the actual JWT claims. - If tenant_id is provided, pins to that tenant; otherwise - auto-discovers from the token. + Fabric CLI strictly inherits the Azure CLI auth context — + tenant override is not supported; use 'az login --tenant' + to switch tenants. """ - # Clear credential to force recreation with the correct tenant + # Clear credential to force recreation self._azure_cli_credential = None # Acquire a probe token to discover identity from JWT claims try: - probe_credential = ( - AzureCliCredential(tenant_id=tenant_id) - if tenant_id - else AzureCliCredential() - ) + probe_credential = AzureCliCredential() probe_token = probe_credential.get_token(con.SCOPE_FABRIC_DEFAULT[0]) claims = self._decode_jwt_claims(probe_token.token) except CredentialUnavailableError: @@ -459,10 +456,8 @@ def set_azure_cli(self, tenant_id=None): status_code=con.ERROR_AUTHENTICATION_FAILED, ) - # Set tenant from explicit param or JWT tid claim - resolved_tenant = tenant_id or claims["tid"] - if resolved_tenant: - self.set_tenant(resolved_tenant) + # Tenant is always inherited from Azure CLI — no override + self.set_tenant(claims["tid"]) # Set identity_type after tenant to survive any logout triggered by tenant change auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"} @@ -502,13 +497,9 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: stored_tenant = self.get_tenant_id() try: - # Create singleton credential if not yet initialized + # Create singleton credential — no tenant pinning, inherit Azure CLI context if self._azure_cli_credential is None: - self._azure_cli_credential = ( - AzureCliCredential(tenant_id=stored_tenant) - if stored_tenant - else AzureCliCredential() - ) + self._azure_cli_credential = AzureCliCredential() # AzureCliCredential.get_token expects scopes as positional args azure_token = self._azure_cli_credential.get_token(scope[0]) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index 5346c83b1..f723a27e7 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -84,16 +84,16 @@ def test_set_azure_cli_sets_identity_type(self, temp_dir_fixture): _mock_credential_with_jwt(mock_class, tid="test-tenant") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="test-tenant") + auth.set_azure_cli() assert auth.get_identity_type() == "azure_cli" - def test_set_azure_cli_with_tenant(self, temp_dir_fixture): - """set_azure_cli with tenant_id should store the tenant.""" + def test_set_azure_cli_stores_jwt_tenant(self, temp_dir_fixture): + """set_azure_cli should store the tenant from the JWT tid claim.""" with patch("fabric_cli.core.fab_auth.AzureCliCredential") as mock_class: _mock_credential_with_jwt(mock_class, tid="test-tenant-id") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="test-tenant-id") + auth.set_azure_cli() assert auth.get_tenant_id() == "test-tenant-id" @patch("fabric_cli.core.fab_auth.AzureCliCredential") @@ -108,15 +108,15 @@ def test_set_azure_cli_auto_captures_tenant( assert auth.get_tenant_id() == "auto-captured-tenant-id" @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_set_azure_cli_explicit_tenant_overrides_auto( + def test_set_azure_cli_tenant_always_from_jwt( self, mock_credential_class, temp_dir_fixture ): - """Explicit tenant_id should be used even if JWT has a different one.""" - _mock_credential_with_jwt(mock_credential_class, tid="other-tenant") + """Tenant is always inherited from the Azure CLI JWT, never overridden.""" + _mock_credential_with_jwt(mock_credential_class, tid="jwt-tenant") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="explicit-tenant") - assert auth.get_tenant_id() == "explicit-tenant" + auth.set_azure_cli() + assert auth.get_tenant_id() == "jwt-tenant" class TestAzureCliTokenAcquisition: @@ -159,22 +159,22 @@ def test_acquire_token_from_azure_cli_success( ) @patch("fabric_cli.core.fab_auth.AzureCliCredential") - def test_acquire_token_from_azure_cli_with_tenant( + def test_acquire_token_credential_inherits_azure_cli_context( self, mock_credential_class, temp_dir_fixture ): - """_acquire_token_from_azure_cli should pass tenant_id to credential.""" + """Credential should be created without tenant_id — inherits Azure CLI context.""" _mock_credential_with_jwt(mock_credential_class, tid="my-tenant-id") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="my-tenant-id") + auth.set_azure_cli() auth._azure_cli_credential = None auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) - # Credential created with tenant_id (may be called twice: once at login, once here) - calls = mock_credential_class.call_args_list - assert any(c == ((), {"tenant_id": "my-tenant-id"}) for c in calls) + # All credential creations should be without tenant_id + for call in mock_credential_class.call_args_list: + assert call == ((), {}), f"Expected no tenant_id, got {call}" @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_acquire_token_from_azure_cli_credential_unavailable( @@ -252,7 +252,7 @@ def test_tenant_drift_blocks_token_acquisition( _mock_credential_with_jwt(mock_credential_class, tid="original-tenant", oid="user1") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="original-tenant") + auth.set_azure_cli() auth._azure_cli_credential = None # Now credential returns token for different-tenant @@ -275,7 +275,7 @@ def test_tenant_match_allows_token_acquisition( auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="same-tenant") + auth.set_azure_cli() auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -341,7 +341,7 @@ def test_principal_drift_blocks_token_acquisition( _mock_credential_with_jwt(mock_credential_class, tid="same-tenant", oid="alice-oid") auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="same-tenant") + auth.set_azure_cli() auth._azure_cli_credential = None # Now credential returns token for bob (oid=bob-oid, same tenant) @@ -364,7 +364,7 @@ def test_principal_match_allows_token_acquisition( auth = FabAuth() auth.set_access_mode("azure_cli") - auth.set_azure_cli(tenant_id="same-tenant") + auth.set_azure_cli() auth._azure_cli_credential = None result = auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) @@ -442,7 +442,7 @@ def test_login_clears_credential( assert auth._azure_cli_credential is not None # Login with explicit tenant — credential must be cleared - auth.set_azure_cli(tenant_id="new-tenant") + auth.set_azure_cli() assert auth._azure_cli_credential is None @@ -569,7 +569,7 @@ def test_login_clears_credential_on_tenant_change( assert auth._azure_cli_credential is not None # Login with explicit tenant — credential must be cleared for recreation - auth.set_azure_cli(tenant_id="new-tenant") + auth.set_azure_cli() assert auth._azure_cli_credential is None From 165aa7df99b8661965e804f21487d1cef57c8c8b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 10:21:51 +0300 Subject: [PATCH 43/50] feat: validate --tenant flag against Azure CLI context In azure-cli auth mode, --tenant is now validated against the Azure CLI session's actual tenant (from the JWT tid claim). If they match, login proceeds. If they differ, an error guides the user to run 'az login --tenant ' instead. This preserves strict context inheritance while giving clear feedback when the user's intent doesn't match reality. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 4 ++-- src/fabric_cli/core/fab_auth.py | 18 ++++++++++++++---- src/fabric_cli/errors/auth.py | 11 +++++++++++ tests/test_core/test_fab_auth_azure_cli.py | 22 ++++++++++++++++++++++ 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index f15b3088a..6fc066ab1 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -30,7 +30,7 @@ def init(args: Namespace) -> Any: if getattr(args, "azure_cli", False): FabAuth().set_access_mode("azure_cli", args.tenant) - FabAuth().set_azure_cli() + FabAuth().set_azure_cli(tenant_id=args.tenant) _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" @@ -78,7 +78,7 @@ def init(args: Namespace) -> Any: Context().context = FabAuth().get_tenant() elif selected_auth.startswith("Azure CLI"): FabAuth().set_access_mode("azure_cli", args.tenant) - FabAuth().set_azure_cli() + FabAuth().set_azure_cli(tenant_id=args.tenant) _acquire_default_access_tokens(FabAuth()) Context().context = FabAuth().get_tenant() tenant_id = FabAuth().get_tenant_id() or "unknown" diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 6d70b8d68..6309a6517 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -422,14 +422,15 @@ def set_managed_identity(self, client_id=None): } ) - def set_azure_cli(self): + def set_azure_cli(self, tenant_id=None): """Configure Azure CLI as the authentication source. Acquires a probe token from Azure CLI to discover and store the tenant ID and principal OID from the actual JWT claims. - Fabric CLI strictly inherits the Azure CLI auth context — - tenant override is not supported; use 'az login --tenant' - to switch tenants. + Fabric CLI strictly inherits the Azure CLI auth context. + If tenant_id is provided, it is validated against the Azure CLI + context — a mismatch raises an error directing the user to + switch tenants via 'az login --tenant'. """ # Clear credential to force recreation self._azure_cli_credential = None @@ -456,6 +457,15 @@ def set_azure_cli(self): status_code=con.ERROR_AUTHENTICATION_FAILED, ) + # Validate explicit tenant against Azure CLI context + if tenant_id and tenant_id != claims["tid"]: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_tenant_override_mismatch( + tenant_id, claims["tid"] + ), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + # Tenant is always inherited from Azure CLI — no override self.set_tenant(claims["tid"]) diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index c24e432ab..13cb9ec31 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -128,6 +128,17 @@ def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str: "Run 'fab auth login --azure-cli' to re-authenticate." ) + @staticmethod + def azure_cli_tenant_override_mismatch( + requested_tenant: str, cli_tenant: str + ) -> str: + return ( + f"Requested tenant '{requested_tenant}' does not match the Azure CLI " + f"session tenant '{cli_tenant}'. In Azure CLI auth mode, Fabric CLI " + "inherits the Azure CLI context. To switch tenants, run " + f"'az login --tenant {requested_tenant}' first, then retry." + ) + @staticmethod def azure_cli_environment_mismatch() -> str: return ( diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index f723a27e7..b79e425b7 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -118,6 +118,28 @@ def test_set_azure_cli_tenant_always_from_jwt( auth.set_azure_cli() assert auth.get_tenant_id() == "jwt-tenant" + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_set_azure_cli_matching_tenant_param_accepted( + self, mock_credential_class, temp_dir_fixture + ): + """Explicit tenant that matches Azure CLI context should succeed.""" + _mock_credential_with_jwt(mock_credential_class, tid="my-tenant") + auth = FabAuth() + auth.set_access_mode("azure_cli") + auth.set_azure_cli(tenant_id="my-tenant") + assert auth.get_tenant_id() == "my-tenant" + + @patch("fabric_cli.core.fab_auth.AzureCliCredential") + def test_set_azure_cli_mismatched_tenant_param_rejected( + self, mock_credential_class, temp_dir_fixture + ): + """Explicit tenant that differs from Azure CLI context should error.""" + _mock_credential_with_jwt(mock_credential_class, tid="cli-tenant") + auth = FabAuth() + auth.set_access_mode("azure_cli") + with pytest.raises(FabricCLIError, match="does not match"): + auth.set_azure_cli(tenant_id="other-tenant") + class TestAzureCliTokenAcquisition: """Test token acquisition via AzureCliCredential.""" From 0dcba92c2d2fa51b8ea2f3cb0c91ef31f333d1ab Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 12:33:05 +0300 Subject: [PATCH 44/50] fix: environment drift check compares issuer host only, not full URL The iss claim includes the tenant ID (e.g., https://sts.windows.net/{tid}/), so switching tenants falsely triggered the environment mismatch error instead of the tenant mismatch error. Now stores and compares only the issuer hostname (e.g., sts.windows.net vs sts.chinacloudapi.cn). Also reordered drift checks: tenant first (most common), then environment, then principal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 29 ++++++++++++++-------- tests/test_core/test_fab_auth_azure_cli.py | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 6309a6517..863d8d149 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -471,9 +471,11 @@ def set_azure_cli(self, tenant_id=None): # Set identity_type after tenant to survive any logout triggered by tenant change auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"} - # Store OID and issuer for drift detection (immutable, no PII) + # Store OID and issuer host for drift detection (immutable, no PII) auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"] - auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"] + from urllib.parse import urlparse + + auth_props[con.FAB_AZURE_CLI_ISSUER] = urlparse(claims["iss"]).hostname self._set_auth_properties(auth_props) @staticmethod @@ -527,15 +529,7 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) - # Environment drift check (issuer encodes cloud: public vs sovereign) - stored_issuer = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER) - if stored_issuer and claims["iss"] != stored_issuer: - raise FabricCLIError( - ErrorMessages.Auth.azure_cli_environment_mismatch(), - status_code=con.ERROR_AUTHENTICATION_FAILED, - ) - - # Tenant drift check + # Tenant drift check (most common drift scenario) if stored_tenant and claims["tid"] != stored_tenant: raise FabricCLIError( ErrorMessages.Auth.azure_cli_tenant_mismatch( @@ -544,6 +538,19 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: status_code=con.ERROR_AUTHENTICATION_FAILED, ) + # Environment drift check (issuer host encodes cloud: public vs sovereign) + # Stored value is already a hostname; extract host from current token's iss + stored_issuer_host = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER) + if stored_issuer_host: + from urllib.parse import urlparse + + current_host = urlparse(claims["iss"]).hostname + if stored_issuer_host != current_host: + raise FabricCLIError( + ErrorMessages.Auth.azure_cli_environment_mismatch(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) + # Principal drift check (OID-based) stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) if stored_principal and claims["oid"] != stored_principal: diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index b79e425b7..e9295073c 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -542,7 +542,7 @@ def test_login_stores_oid_and_issuer_for_drift_detection(self, mock_credential_c auth.set_access_mode("azure_cli") auth.set_azure_cli() assert auth._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) == "user-oid-123" - assert auth._auth_info.get(con.FAB_AZURE_CLI_ISSUER) == "https://sts.windows.net/t1/" + assert auth._auth_info.get(con.FAB_AZURE_CLI_ISSUER) == "sts.windows.net" @patch("fabric_cli.core.fab_auth.AzureCliCredential") def test_re_login_updates_tenant_and_oid(self, mock_credential_class, temp_dir_fixture): From acb923b866efb52b74737a9d472ca9f7f2848d33 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 13:00:26 +0300 Subject: [PATCH 45/50] fix: suppress noisy Azure SDK logging in fab auth status AzureCliCredential logs 'Please run az login' to stderr on every failed get_token call. Since status acquires 4 tokens, this produced 4 identical error messages. Now suppresses Azure SDK logging during status and shows one clear Fabric CLI message instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/commands/auth/fab_auth.py | 106 +++++++++++++---------- 1 file changed, 62 insertions(+), 44 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 6fc066ab1..17aff5ce0 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -226,55 +226,68 @@ def logout(args: Namespace) -> None: def status(args: Namespace) -> None: auth = FabAuth() tenant_id = auth.get_tenant_id() + identity_type = auth.get_identity_type() or "N/A" - def __get_token_info(scope): - try: - token = auth.get_access_token(scope, interactive_renew=False) - except FabricCLIError as e: - if e.status_code in [ - fab_constant.ERROR_UNAUTHORIZED, - fab_constant.ERROR_AUTHENTICATION_FAILED, - ]: - return {} - else: - raise e - if isinstance(token, str): - token = token.encode() # Ensure bytes type - return _get_token_info_from_bearer_token(token) if token else {} - - token_info = __get_token_info(fab_constant.SCOPE_FABRIC_DEFAULT) - - upn = token_info.get("upn") or "N/A" - oid = token_info.get("oid") or "N/A" - tid = token_info.get("tid", tenant_id) or "N/A" - appid = token_info.get("appid") or "N/A" - - def __mask_token(scope): - try: - token = auth.get_access_token(scope, interactive_renew=False) - except FabricCLIError as e: - if e.status_code in [ - fab_constant.ERROR_UNAUTHORIZED, - fab_constant.ERROR_AUTHENTICATION_FAILED, - ]: - return "N/A" - else: - raise e - if isinstance(token, str): - token = token.encode() # Ensure bytes type - return ( - token[:4].decode() + "************************************" - if token - else "N/A" - ) + # Suppress noisy Azure SDK stderr logging during status checks + # (AzureCliCredential logs "Please run 'az login'" before raising) + import logging + + azure_logger = logging.getLogger("azure.identity") + original_level = azure_logger.level + azure_logger.setLevel(logging.CRITICAL) + + try: + + def __get_token_info(scope): + try: + token = auth.get_access_token(scope, interactive_renew=False) + except FabricCLIError as e: + if e.status_code in [ + fab_constant.ERROR_UNAUTHORIZED, + fab_constant.ERROR_AUTHENTICATION_FAILED, + ]: + return {} + else: + raise e + if isinstance(token, str): + token = token.encode() # Ensure bytes type + return _get_token_info_from_bearer_token(token) if token else {} + + token_info = __get_token_info(fab_constant.SCOPE_FABRIC_DEFAULT) + + upn = token_info.get("upn") or "N/A" + oid = token_info.get("oid") or "N/A" + tid = token_info.get("tid", tenant_id) or "N/A" + appid = token_info.get("appid") or "N/A" + + def __mask_token(scope): + try: + token = auth.get_access_token(scope, interactive_renew=False) + except FabricCLIError as e: + if e.status_code in [ + fab_constant.ERROR_UNAUTHORIZED, + fab_constant.ERROR_AUTHENTICATION_FAILED, + ]: + return "N/A" + else: + raise e + if isinstance(token, str): + token = token.encode() # Ensure bytes type + return ( + token[:4].decode() + "************************************" + if token + else "N/A" + ) + + fabric_secret = __mask_token(fab_constant.SCOPE_FABRIC_DEFAULT) + storage_secret = __mask_token(fab_constant.SCOPE_ONELAKE_DEFAULT) + azure_secret = __mask_token(fab_constant.SCOPE_AZURE_DEFAULT) - fabric_secret = __mask_token(fab_constant.SCOPE_FABRIC_DEFAULT) - storage_secret = __mask_token(fab_constant.SCOPE_ONELAKE_DEFAULT) - azure_secret = __mask_token(fab_constant.SCOPE_AZURE_DEFAULT) + finally: + azure_logger.setLevel(original_level) # Check login status is_logged_in = fabric_secret != "N/A" - identity_type = auth.get_identity_type() or "N/A" login_status = ( "✓ Logged in to app.fabric.microsoft.com" if is_logged_in @@ -283,6 +296,11 @@ def __mask_token(scope): fab_ui.print_grey(login_status) if identity_type == "azure_cli" and is_logged_in: fab_ui.print_grey(f" Auth mode: Azure CLI (tenant: {tid})") + elif identity_type == "azure_cli" and not is_logged_in: + fab_ui.print_grey( + " Azure CLI session expired or logged out. " + "Run 'az login' then 'fab auth login --azure-cli' to re-authenticate." + ) auth_data = { "logged_in": is_logged_in, From 3ff1e13fb3056c0864cc6a792c147bd8bc36ee1b Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 13:52:55 +0300 Subject: [PATCH 46/50] fix: correct mock assertions for keyword arg in set_azure_cli tests The caller uses set_azure_cli(tenant_id=args.tenant) (keyword arg), but the test assertions used assert_called_once_with(None) (positional). Also fix base64 padding bug and add binascii.Error catch in JWT decoder. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fabric_cli/core/fab_auth.py | 7 ++++--- tests/test_commands/test_auth.py | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 863d8d149..6d34a285f 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -4,6 +4,7 @@ import json import os import base64 +import binascii import uuid from binascii import hexlify from typing import Any, NamedTuple, Optional @@ -491,12 +492,12 @@ def _decode_jwt_claims(token: str) -> dict: parts = token.split(".") if len(parts) < 2: return {} - # Add padding for base64url decoding + # Add padding for base64url decoding (avoid adding 4 when already aligned) payload = parts[1] - payload += "=" * (4 - len(payload) % 4) + payload += "=" * ((-len(payload)) % 4) decoded = base64.urlsafe_b64decode(payload) return json.loads(decoded) - except (ValueError, json.JSONDecodeError, UnicodeDecodeError): + except (ValueError, json.JSONDecodeError, UnicodeDecodeError, binascii.Error): return {} def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: diff --git a/tests/test_commands/test_auth.py b/tests/test_commands/test_auth.py index 7cca5e5a9..30be51710 100644 --- a/tests/test_commands/test_auth.py +++ b/tests/test_commands/test_auth.py @@ -966,7 +966,7 @@ def test_init_with_azure_cli_flag(self, mock_fab_auth, mock_fab_context): mock_fab_auth_instance.set_access_mode.assert_called_with( "azure_cli", None ) - mock_set_azure_cli.assert_called_once_with(None) + mock_set_azure_cli.assert_called_once_with(tenant_id=None) assert result is True def test_init_with_azure_cli_flag_and_tenant( @@ -985,7 +985,7 @@ def test_init_with_azure_cli_flag_and_tenant( mock_fab_auth_instance.set_access_mode.assert_called_with( "azure_cli", "my-tenant" ) - mock_set_azure_cli.assert_called_once_with("my-tenant") + mock_set_azure_cli.assert_called_once_with(tenant_id="my-tenant") assert result is True def test_init_with_interactive_azure_cli_selection( @@ -1008,7 +1008,7 @@ def test_init_with_interactive_azure_cli_selection( mock_fab_auth_instance.set_access_mode.assert_called_with( "azure_cli", None ) - mock_set_azure_cli.assert_called_once_with(None) + mock_set_azure_cli.assert_called_once_with(tenant_id=None) assert_get_access_token(mock_fab_auth_instance) assert_fab_context(mock_fab_context) assert result is True From 769b0cf298f78f4c68d3fc91a45c6743e486a546 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 14:18:18 +0300 Subject: [PATCH 47/50] fix error messages --- src/fabric_cli/errors/auth.py | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/fabric_cli/errors/auth.py b/src/fabric_cli/errors/auth.py index 13cb9ec31..23c9beed2 100644 --- a/src/fabric_cli/errors/auth.py +++ b/src/fabric_cli/errors/auth.py @@ -123,9 +123,8 @@ def only_supported_with_user_authentication() -> str: @staticmethod def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str: return ( - f"Tenant mismatch: Fabric CLI is pinned to tenant '{stored_tenant}' " - f"but Azure CLI is now logged into tenant '{current_tenant}'. " - "Run 'fab auth login --azure-cli' to re-authenticate." + f"Azure CLI tenant changed from '{stored_tenant}' to '{current_tenant}'. " + "Run 'fab auth login --azure-cli' to re-authenticate with the current tenant" ) @staticmethod @@ -133,31 +132,29 @@ def azure_cli_tenant_override_mismatch( requested_tenant: str, cli_tenant: str ) -> str: return ( - f"Requested tenant '{requested_tenant}' does not match the Azure CLI " - f"session tenant '{cli_tenant}'. In Azure CLI auth mode, Fabric CLI " - "inherits the Azure CLI context. To switch tenants, run " - f"'az login --tenant {requested_tenant}' first, then retry." + f"Tenant '{requested_tenant}' does not match the Azure CLI tenant " + f"'{cli_tenant}'. Run 'az login --tenant {requested_tenant}' first" ) @staticmethod def azure_cli_environment_mismatch() -> str: return ( - "Azure CLI cloud environment has changed since 'fab auth login --azure-cli' was run. " - "Run 'fab auth login --azure-cli' to re-authenticate in the current environment." + "Azure CLI cloud environment has changed. " + "Run 'fab auth login --azure-cli' to re-authenticate in the current environment" ) @staticmethod def azure_cli_principal_mismatch() -> str: return ( - "Azure CLI identity has changed since 'fab auth login --azure-cli' was run. " - "Run 'fab auth login --azure-cli' to re-authenticate with the current identity." + "Azure CLI identity has changed. " + "Run 'fab auth login --azure-cli' to re-authenticate with the current identity" ) @staticmethod def azure_cli_not_available() -> str: return ( "Azure CLI is not installed or not logged in. " - "Run 'az login' to authenticate, then retry." + "Run 'az login' to authenticate, then retry" ) @staticmethod @@ -167,14 +164,13 @@ def azure_cli_auth_failed(error_msg: str) -> str: @staticmethod def azure_cli_token_missing_claims() -> str: return ( - "Azure CLI returned a token with missing identity claims (iss, tid, or oid). " - "Run 'az account get-access-token --resource https://api.fabric.microsoft.com' " - "manually to diagnose." + "Unable to validate the Azure CLI identity. " + "Run 'az login' to authenticate, then retry" ) @staticmethod def azure_cli_token_acquisition_failed() -> str: return ( - "Azure CLI token acquisition failed. " - "Run 'az account get-access-token' manually to diagnose." + "Unable to get a token from Azure CLI. " + "Run 'az login' to authenticate, then retry" ) From bda02b2d72ae17c618675a65ba6fad02ad9ddf3d Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 14:23:04 +0300 Subject: [PATCH 48/50] fux --- src/fabric_cli/core/fab_auth.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/fabric_cli/core/fab_auth.py b/src/fabric_cli/core/fab_auth.py index 6d34a285f..04a250c7e 100644 --- a/src/fabric_cli/core/fab_auth.py +++ b/src/fabric_cli/core/fab_auth.py @@ -1,10 +1,10 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -import json -import os import base64 import binascii +import json +import os import uuid from binascii import hexlify from typing import Any, NamedTuple, Optional @@ -351,7 +351,7 @@ def set_tenant(self, tenant_id): if current_tenant_id is not None and current_tenant_id != tenant_id: fab_logger.log_warning( f"Tenant ID already set to {current_tenant_id}." - + f" Logout done and Tenant ID set to {tenant_id}." + + f" Logout done and Tenant ID set to {tenant_id}" ) self.logout() @@ -448,11 +448,7 @@ def set_azure_cli(self, tenant_id=None): ) # Fail-closed: refuse to persist if identity claims are missing - if ( - not claims.get("iss") - or not claims.get("tid") - or not claims.get("oid") - ): + if not claims.get("iss") or not claims.get("tid") or not claims.get("oid"): raise FabricCLIError( ErrorMessages.Auth.azure_cli_token_missing_claims(), status_code=con.ERROR_AUTHENTICATION_FAILED, @@ -520,11 +516,7 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: claims = self._decode_jwt_claims(azure_token.token) # Fail-closed: reject tokens with missing identity claims - if ( - not claims.get("iss") - or not claims.get("tid") - or not claims.get("oid") - ): + if not claims.get("iss") or not claims.get("tid") or not claims.get("oid"): raise FabricCLIError( ErrorMessages.Auth.azure_cli_token_missing_claims(), status_code=con.ERROR_AUTHENTICATION_FAILED, @@ -556,9 +548,9 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict: stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) if stored_principal and claims["oid"] != stored_principal: raise FabricCLIError( - ErrorMessages.Auth.azure_cli_principal_mismatch(), - status_code=con.ERROR_AUTHENTICATION_FAILED, - ) + ErrorMessages.Auth.azure_cli_principal_mismatch(), + status_code=con.ERROR_AUTHENTICATION_FAILED, + ) token_result = { "access_token": azure_token.token, From 72aa017f075a3abe5f0b88151c34395c9f6fd213 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 14:49:32 +0300 Subject: [PATCH 49/50] fix --- src/fabric_cli/commands/auth/fab_auth.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/fabric_cli/commands/auth/fab_auth.py b/src/fabric_cli/commands/auth/fab_auth.py index 17aff5ce0..a144c289f 100644 --- a/src/fabric_cli/commands/auth/fab_auth.py +++ b/src/fabric_cli/commands/auth/fab_auth.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import logging from argparse import Namespace from typing import Any, Optional @@ -228,10 +229,7 @@ def status(args: Namespace) -> None: tenant_id = auth.get_tenant_id() identity_type = auth.get_identity_type() or "N/A" - # Suppress noisy Azure SDK stderr logging during status checks - # (AzureCliCredential logs "Please run 'az login'" before raising) - import logging - + # Suppress noisy Azure SDK logging during status checks azure_logger = logging.getLogger("azure.identity") original_level = azure_logger.level azure_logger.setLevel(logging.CRITICAL) @@ -299,7 +297,7 @@ def __mask_token(scope): elif identity_type == "azure_cli" and not is_logged_in: fab_ui.print_grey( " Azure CLI session expired or logged out. " - "Run 'az login' then 'fab auth login --azure-cli' to re-authenticate." + "Run 'az login' then 'fab auth login --azure-cli' to re-authenticate" ) auth_data = { From 8d7f06268a41e9e6e6a5f33bfd7c513d3ecec861 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Thu, 20 Aug 2026 15:05:03 +0300 Subject: [PATCH 50/50] fix: update test assertions to match simplified error messages Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_core/test_fab_auth_azure_cli.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_core/test_fab_auth_azure_cli.py b/tests/test_core/test_fab_auth_azure_cli.py index e9295073c..cddca259e 100644 --- a/tests/test_core/test_fab_auth_azure_cli.py +++ b/tests/test_core/test_fab_auth_azure_cli.py @@ -259,7 +259,7 @@ def test_unknown_exception_returns_safe_message( auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT) assert "eyJ0eXAi" not in str(exc_info.value) - assert "manually to diagnose" in str(exc_info.value) + assert "Unable to get a token from Azure CLI" in str(exc_info.value) class TestAzureCliTenantDrift: @@ -637,7 +637,7 @@ def test_login_rejects_token_missing_oid(self, mock_class, temp_dir_fixture): mock_token.expires_on = int(time.time()) + 3600 mock_class.return_value.get_token.return_value = mock_token auth = FabAuth() - with pytest.raises(FabricCLIError, match="missing identity claims"): + with pytest.raises(FabricCLIError, match="Unable to validate"): auth.set_azure_cli() @patch("fabric_cli.core.fab_auth.AzureCliCredential") @@ -653,7 +653,7 @@ def test_login_rejects_token_missing_tid(self, mock_class, temp_dir_fixture): mock_token.expires_on = int(time.time()) + 3600 mock_class.return_value.get_token.return_value = mock_token auth = FabAuth() - with pytest.raises(FabricCLIError, match="missing identity claims"): + with pytest.raises(FabricCLIError, match="Unable to validate"): auth.set_azure_cli() @patch("fabric_cli.core.fab_auth.AzureCliCredential") @@ -664,7 +664,7 @@ def test_login_rejects_malformed_token(self, mock_class, temp_dir_fixture): mock_token.expires_on = int(time.time()) + 3600 mock_class.return_value.get_token.return_value = mock_token auth = FabAuth() - with pytest.raises(FabricCLIError, match="missing identity claims"): + with pytest.raises(FabricCLIError, match="Unable to validate"): auth.set_azure_cli() @patch("fabric_cli.core.fab_auth.AzureCliCredential") @@ -680,7 +680,7 @@ def test_acquisition_rejects_token_missing_claims(self, mock_class, temp_dir_fix bad_token.token = "not-a-jwt" bad_token.expires_on = int(time.time()) + 3600 mock_class.return_value.get_token.return_value = bad_token - with pytest.raises(FabricCLIError, match="missing identity claims"): + with pytest.raises(FabricCLIError, match="Unable to validate"): auth.acquire_token(con.SCOPE_FABRIC_DEFAULT)