Skip to content

Commit 2d38b97

Browse files
Shira SassoonCopilot
andcommitted
security: fail-closed on missing JWT identity claims
Reject tokens that lack iss, tid, or oid instead of silently skipping drift checks. Applied at both login (set_azure_cli) and token acquisition (_acquire_token_from_azure_cli). Also fixed redundant nested if in environment drift check and incorrect indentation in principal drift check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 8b82831 commit 2d38b97

3 files changed

Lines changed: 107 additions & 19 deletions

File tree

‎src/fabric_cli/core/fab_auth.py‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -448,18 +448,27 @@ def set_azure_cli(self, tenant_id=None):
448448
status_code=con.ERROR_AUTHENTICATION_FAILED,
449449
)
450450

451+
# Fail-closed: refuse to persist if identity claims are missing
452+
if (
453+
not claims.get("iss")
454+
or not claims.get("tid")
455+
or not claims.get("oid")
456+
):
457+
raise FabricCLIError(
458+
ErrorMessages.Auth.azure_cli_token_missing_claims(),
459+
status_code=con.ERROR_AUTHENTICATION_FAILED,
460+
)
461+
451462
# Set tenant from explicit param or JWT tid claim
452-
resolved_tenant = tenant_id or claims.get("tid")
463+
resolved_tenant = tenant_id or claims["tid"]
453464
if resolved_tenant:
454465
self.set_tenant(resolved_tenant)
455466

456467
# Set identity_type after tenant to survive any logout triggered by tenant change
457468
auth_props: dict = {con.IDENTITY_TYPE: "azure_cli"}
458469
# Store OID and issuer for drift detection (immutable, no PII)
459-
if claims.get("oid"):
460-
auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"]
461-
if claims.get("iss"):
462-
auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"]
470+
auth_props[con.FAB_AZURE_CLI_PRINCIPAL_ID] = claims["oid"]
471+
auth_props[con.FAB_AZURE_CLI_ISSUER] = claims["iss"]
463472
self._set_auth_properties(auth_props)
464473

465474
@staticmethod
@@ -506,30 +515,38 @@ def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict:
506515
# Post-acquisition drift detection from actual token claims
507516
claims = self._decode_jwt_claims(azure_token.token)
508517

518+
# Fail-closed: reject tokens with missing identity claims
519+
if (
520+
not claims.get("iss")
521+
or not claims.get("tid")
522+
or not claims.get("oid")
523+
):
524+
raise FabricCLIError(
525+
ErrorMessages.Auth.azure_cli_token_missing_claims(),
526+
status_code=con.ERROR_AUTHENTICATION_FAILED,
527+
)
528+
509529
# Environment drift check (issuer encodes cloud: public vs sovereign)
510530
stored_issuer = self._auth_info.get(con.FAB_AZURE_CLI_ISSUER)
511-
if stored_issuer and claims.get("iss"):
512-
if claims["iss"] != stored_issuer:
513-
raise FabricCLIError(
531+
if stored_issuer and claims["iss"] != stored_issuer:
532+
raise FabricCLIError(
514533
ErrorMessages.Auth.azure_cli_environment_mismatch(),
515534
status_code=con.ERROR_AUTHENTICATION_FAILED,
516535
)
517536

518537
# Tenant drift check
519-
if stored_tenant and claims.get("tid"):
520-
if claims["tid"] != stored_tenant:
521-
raise FabricCLIError(
522-
ErrorMessages.Auth.azure_cli_tenant_mismatch(
523-
stored_tenant, claims["tid"]
524-
),
525-
status_code=con.ERROR_AUTHENTICATION_FAILED,
526-
)
538+
if stored_tenant and claims["tid"] != stored_tenant:
539+
raise FabricCLIError(
540+
ErrorMessages.Auth.azure_cli_tenant_mismatch(
541+
stored_tenant, claims["tid"]
542+
),
543+
status_code=con.ERROR_AUTHENTICATION_FAILED,
544+
)
527545

528546
# Principal drift check (OID-based)
529547
stored_principal = self._auth_info.get(con.FAB_AZURE_CLI_PRINCIPAL_ID)
530-
if stored_principal and claims.get("oid"):
531-
if claims["oid"] != stored_principal:
532-
raise FabricCLIError(
548+
if stored_principal and claims["oid"] != stored_principal:
549+
raise FabricCLIError(
533550
ErrorMessages.Auth.azure_cli_principal_mismatch(),
534551
status_code=con.ERROR_AUTHENTICATION_FAILED,
535552
)

‎src/fabric_cli/errors/auth.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,14 @@ def azure_cli_not_available() -> str:
153153
def azure_cli_auth_failed(error_msg: str) -> str:
154154
return f"Azure CLI authentication failed: {error_msg}"
155155

156+
@staticmethod
157+
def azure_cli_token_missing_claims() -> str:
158+
return (
159+
"Azure CLI returned a token with missing identity claims (iss, tid, or oid). "
160+
"Run 'az account get-access-token --resource https://api.fabric.microsoft.com' "
161+
"manually to diagnose."
162+
)
163+
156164
@staticmethod
157165
def azure_cli_token_acquisition_failed() -> str:
158166
return (

‎tests/test_core/test_fab_auth_azure_cli.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,69 @@ def test_jwt_with_extra_claims(self, temp_dir_fixture):
599599
assert claims["upn"] == "user@contoso.com"
600600

601601

602+
class TestFailClosedOnMissingClaims:
603+
"""Verify tokens with missing identity claims are rejected, not silently used."""
604+
605+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
606+
def test_login_rejects_token_missing_oid(self, mock_class, temp_dir_fixture):
607+
"""set_azure_cli should fail if probe token lacks oid."""
608+
header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode()
609+
payload = base64.urlsafe_b64encode(
610+
_json.dumps({"tid": "t1", "iss": "https://sts.windows.net/t1/"}).encode()
611+
).rstrip(b"=").decode()
612+
token_str = f"{header}.{payload}.fakesig"
613+
mock_token = MagicMock()
614+
mock_token.token = token_str
615+
mock_token.expires_on = int(time.time()) + 3600
616+
mock_class.return_value.get_token.return_value = mock_token
617+
auth = FabAuth()
618+
with pytest.raises(FabricCLIError, match="missing identity claims"):
619+
auth.set_azure_cli()
620+
621+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
622+
def test_login_rejects_token_missing_tid(self, mock_class, temp_dir_fixture):
623+
"""set_azure_cli should fail if probe token lacks tid."""
624+
header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode()
625+
payload = base64.urlsafe_b64encode(
626+
_json.dumps({"oid": "o1", "iss": "https://sts.windows.net/t1/"}).encode()
627+
).rstrip(b"=").decode()
628+
token_str = f"{header}.{payload}.fakesig"
629+
mock_token = MagicMock()
630+
mock_token.token = token_str
631+
mock_token.expires_on = int(time.time()) + 3600
632+
mock_class.return_value.get_token.return_value = mock_token
633+
auth = FabAuth()
634+
with pytest.raises(FabricCLIError, match="missing identity claims"):
635+
auth.set_azure_cli()
636+
637+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
638+
def test_login_rejects_malformed_token(self, mock_class, temp_dir_fixture):
639+
"""set_azure_cli should fail if probe token is not a valid JWT."""
640+
mock_token = MagicMock()
641+
mock_token.token = "not-a-jwt"
642+
mock_token.expires_on = int(time.time()) + 3600
643+
mock_class.return_value.get_token.return_value = mock_token
644+
auth = FabAuth()
645+
with pytest.raises(FabricCLIError, match="missing identity claims"):
646+
auth.set_azure_cli()
647+
648+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
649+
def test_acquisition_rejects_token_missing_claims(self, mock_class, temp_dir_fixture):
650+
"""Token acquisition should fail if returned token lacks identity claims."""
651+
# Login with good token
652+
_mock_credential_with_jwt(mock_class)
653+
auth = FabAuth()
654+
auth.set_azure_cli()
655+
656+
# Now return a bad token on next call
657+
bad_token = MagicMock()
658+
bad_token.token = "not-a-jwt"
659+
bad_token.expires_on = int(time.time()) + 3600
660+
mock_class.return_value.get_token.return_value = bad_token
661+
with pytest.raises(FabricCLIError, match="missing identity claims"):
662+
auth.acquire_token(con.SCOPE_FABRIC_DEFAULT)
663+
664+
602665
class TestNonAzureCliIsolation:
603666
"""Verify each auth method uses only its own credential path — no overlap."""
604667

0 commit comments

Comments
 (0)