Skip to content

Commit 72a5916

Browse files
Shira SassoonCopilot
andcommitted
feat: add principal drift detection to prevent silent identity swaps
- Unified _get_azure_cli_account() queries tenant + principal in single subprocess call - Store principal name at login for drift comparison - Block token acquisition if Azure CLI identity changes within same tenant - Error message is PII-free (no emails/OIDs exposed) - Graceful degradation: skip check if no principal stored (old auth files) - 3 new tests for drift block, match pass, and no-stored-principal skip Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 56878e7 commit 72a5916

4 files changed

Lines changed: 179 additions & 50 deletions

File tree

src/fabric_cli/core/fab_auth.py

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ def __init__(self):
5959
self._auth_info = {}
6060
# In-memory token cache for Azure CLI tokens
6161
self._azure_cli_token_cache: dict[str, dict] = {}
62-
# Cached tenant ID from az account show
63-
self._cached_az_tenant: Optional[str] = None
64-
self._cached_az_tenant_time: float = 0.0
62+
# Cached az account show result (tenant + principal in one call)
63+
self._cached_az_account: Optional[dict] = None
64+
self._cached_az_account_time: float = 0.0
6565

6666
# Load the auth info and environment variables
6767
self._load_auth()
@@ -440,57 +440,71 @@ def set_azure_cli(self, tenant_id=None):
440440
# Clear token cache on every login to prevent stale tokens from a
441441
# previous tenant (or no-tenant) session from being reused.
442442
self._azure_cli_token_cache.clear()
443+
# Query Azure CLI account once for both tenant and principal
444+
account = self._get_azure_cli_account(force_refresh=True)
443445
# Set tenant first — set_tenant() may call logout() which clears auth info
444446
if tenant_id:
445447
self.set_tenant(tenant_id)
446-
else:
447-
# Force refresh at login to avoid stale cached tenant
448-
captured_tenant = self._get_azure_cli_tenant(force_refresh=True)
449-
if captured_tenant:
450-
self.set_tenant(captured_tenant)
448+
elif account and account.get("tenant_id"):
449+
self.set_tenant(account["tenant_id"])
451450
# Set identity_type after tenant to survive any logout triggered by tenant change
452-
self._set_auth_properties(
453-
{
454-
con.IDENTITY_TYPE: "azure_cli",
455-
}
456-
)
451+
auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"}
452+
# Store principal for drift detection (no PII exposed in errors)
453+
if account and account.get("principal_name"):
454+
auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = account["principal_name"]
455+
self._set_auth_properties(auth_props)
457456

458-
def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]:
459-
"""Query Azure CLI for the current tenant ID via 'az account show'.
457+
def _get_azure_cli_account(self, force_refresh: bool = False) -> Optional[dict]:
458+
"""Query Azure CLI account info (tenant + principal) in a single subprocess call.
460459
461-
Caches the result to avoid repeated subprocess calls
462-
during multi-scope token acquisition flows.
460+
Returns a dict with 'tenant_id' and 'principal_name' keys, or None
461+
if Azure CLI is unavailable. Caches the result to avoid repeated
462+
subprocess calls during multi-scope token acquisition flows.
463463
464464
Args:
465465
force_refresh: If True, bypass the cache and query az directly.
466466
"""
467-
# Return cached result if fresh and not forced
468467
if (
469468
not force_refresh
470-
and self._cached_az_tenant is not None
471-
and time.monotonic() - self._cached_az_tenant_time
469+
and self._cached_az_account is not None
470+
and time.monotonic() - self._cached_az_account_time
472471
< _AZURE_CLI_TENANT_CACHE_TTL_SECONDS
473472
):
474-
return self._cached_az_tenant
473+
return self._cached_az_account
475474

476475
try:
477476
az_path = shutil.which("az")
478477
if not az_path:
479478
return None
480479
result = subprocess.run(
481-
[az_path, "account", "show", "--query", "tenantId", "-o", "tsv"],
480+
[az_path, "account", "show", "--query", "{tenantId:tenantId,userName:user.name}", "-o", "json"],
482481
capture_output=True,
483482
text=True,
484483
timeout=10,
485484
)
486485
if result.returncode == 0 and result.stdout.strip():
487-
self._cached_az_tenant = result.stdout.strip()
488-
self._cached_az_tenant_time = time.monotonic()
489-
return self._cached_az_tenant
490-
except (subprocess.TimeoutExpired, FileNotFoundError, OSError):
486+
data = json.loads(result.stdout.strip())
487+
account_info = {
488+
"tenant_id": data.get("tenantId"),
489+
"principal_name": data.get("userName"),
490+
}
491+
self._cached_az_account = account_info
492+
self._cached_az_account_time = time.monotonic()
493+
return self._cached_az_account
494+
except (subprocess.TimeoutExpired, FileNotFoundError, OSError, ValueError):
491495
pass
492496
return None
493497

498+
def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]:
499+
"""Get the current Azure CLI tenant ID (thin wrapper over _get_azure_cli_account)."""
500+
account = self._get_azure_cli_account(force_refresh=force_refresh)
501+
return account.get("tenant_id") if account else None
502+
503+
def _get_azure_cli_principal(self, force_refresh: bool = False) -> Optional[str]:
504+
"""Get the current Azure CLI principal name (thin wrapper over _get_azure_cli_account)."""
505+
account = self._get_azure_cli_account(force_refresh=force_refresh)
506+
return account.get("principal_name") if account else None
507+
494508
def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict:
495509
"""Acquire a token using Azure CLI's AzureCliCredential."""
496510
# Tenant drift check: compare stored tenant against current az session
@@ -505,6 +519,16 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict:
505519
status_code=con.ERROR_AUTHENTICATION_FAILED,
506520
)
507521

522+
# Principal drift check: detect identity change within same tenant
523+
stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID)
524+
if stored_principal:
525+
current_principal = self._get_azure_cli_principal()
526+
if current_principal and current_principal != stored_principal:
527+
raise FabricCLIError(
528+
ErrorMessages.Auth.azure_cli_principal_mismatch(),
529+
status_code=con.ERROR_AUTHENTICATION_FAILED,
530+
)
531+
508532
# Check in-memory cache first
509533
cache_key = scope[0] if scope else ""
510534
cached = self._get_cached_azure_cli_token(cache_key)
@@ -694,8 +718,8 @@ def logout(self):
694718

695719
# Clear Azure CLI caches
696720
self._azure_cli_token_cache.clear()
697-
self._cached_az_tenant = None
698-
self._cached_az_tenant_time = 0.0
721+
self._cached_az_account = None
722+
self._cached_az_account_time = 0.0
699723

700724
if os.path.exists(self.cache_file):
701725
os.remove(self.cache_file)

src/fabric_cli/core/fab_constant.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
FAB_TENANT_ID = "fab_tenant_id"
5757

5858
FAB_REFRESH_TOKEN = "fab_refresh_token"
59+
FAB_AZURE_CLI_PRINCIPAL_ID = "fab_azure_cli_principal_id"
5960
IDENTITY_TYPE = "identity_type"
6061
FAB_AUTH_MODE = "fab_auth_mode" # Kept for backward compatibility
6162
FAB_AUTHORITY = "fab_authority"

src/fabric_cli/errors/auth.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,13 @@ def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str:
128128
"Run 'fab auth login --azure-cli' to re-authenticate."
129129
)
130130

131+
@staticmethod
132+
def azure_cli_principal_mismatch() -> str:
133+
return (
134+
"Azure CLI identity has changed since 'fab auth login --azure-cli' was run. "
135+
"Run 'fab auth login --azure-cli' to re-authenticate with the current identity."
136+
)
137+
131138
@staticmethod
132139
def azure_cli_not_available() -> str:
133140
return (

0 commit comments

Comments
 (0)