From 24582d3ffc8d4864ce3d82ef5aeac5543be95889 Mon Sep 17 00:00:00 2001 From: Shira Sassoon Date: Wed, 15 Jul 2026 13:44:12 +0300 Subject: [PATCH 01/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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/31] 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)}"