Skip to content

Commit 0dcba92

Browse files
Shira SassoonCopilot
andcommitted
fix: environment drift check compares issuer host only, not full URL
The iss claim includes the tenant ID (e.g., https://sts.windows.net/{tid}/), so switching tenants falsely triggered the environment mismatch error instead of the tenant mismatch error. Now stores and compares only the issuer hostname (e.g., sts.windows.net vs sts.chinacloudapi.cn). Also reordered drift checks: tenant first (most common), then environment, then principal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 165aa7d commit 0dcba92

2 files changed

Lines changed: 19 additions & 12 deletions

File tree

src/fabric_cli/core/fab_auth.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -471,9 +471,11 @@ def set_azure_cli(self, tenant_id=None):
471471

472472
# Set identity_type after tenant to survive any logout triggered by tenant change
473473
auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"}
474-
# Store OID and issuer for drift detection (immutable, no PII)
474+
# Store OID and issuer host for drift detection (immutable, no PII)
475475
auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"]
476-
auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"]
476+
from urllib.parse import urlparse
477+
478+
auth_props[con.FAB_AZURE_CLI_ISSUER] = urlparse(claims["iss"]).hostname
477479
self._set_auth_properties(auth_props)
478480

479481
@staticmethod
@@ -527,15 +529,7 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict:
527529
status_code=con.ERROR_AUTHENTICATION_FAILED,
528530
)
529531

530-
# Environment drift check (issuer encodes cloud: public vs sovereign)
531-
stored_issuer = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER)
532-
if stored_issuer and claims["iss"] != stored_issuer:
533-
raise FabricCLIError(
534-
ErrorMessages.Auth.azure_cli_environment_mismatch(),
535-
status_code=con.ERROR_AUTHENTICATION_FAILED,
536-
)
537-
538-
# Tenant drift check
532+
# Tenant drift check (most common drift scenario)
539533
if stored_tenant and claims["tid"] != stored_tenant:
540534
raise FabricCLIError(
541535
ErrorMessages.Auth.azure_cli_tenant_mismatch(
@@ -544,6 +538,19 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict:
544538
status_code=con.ERROR_AUTHENTICATION_FAILED,
545539
)
546540

541+
# Environment drift check (issuer host encodes cloud: public vs sovereign)
542+
# Stored value is already a hostname; extract host from current token's iss
543+
stored_issuer_host = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER)
544+
if stored_issuer_host:
545+
from urllib.parse import urlparse
546+
547+
current_host = urlparse(claims["iss"]).hostname
548+
if stored_issuer_host != current_host:
549+
raise FabricCLIError(
550+
ErrorMessages.Auth.azure_cli_environment_mismatch(),
551+
status_code=con.ERROR_AUTHENTICATION_FAILED,
552+
)
553+
547554
# Principal drift check (OID-based)
548555
stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID)
549556
if stored_principal and claims["oid"] != stored_principal:

tests/test_core/test_fab_auth_azure_cli.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ def test_login_stores_oid_and_issuer_for_drift_detection(self, mock_credential_c
542542
auth.set_access_mode("azure_cli")
543543
auth.set_azure_cli()
544544
assert auth._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID) == "user-oid-123"
545-
assert auth._auth_info.get(con.FAB_AZURE_CLI_ISSUER) == "https://sts.windows.net/t1/"
545+
assert auth._auth_info.get(con.FAB_AZURE_CLI_ISSUER) == "sts.windows.net"
546546

547547
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
548548
def test_re_login_updates_tenant_and_oid(self, mock_credential_class, temp_dir_fixture):

0 commit comments

Comments
 (0)