Skip to content

Fix Azure AD tenant identifier canonicalization in FAB auth manager - #71920

Open
aaron-y-chen wants to merge 2 commits into
apache:mainfrom
aaron-y-chen:fix-fab-azure-tenant-canonicalization
Open

Fix Azure AD tenant identifier canonicalization in FAB auth manager#71920
aaron-y-chen wants to merge 2 commits into
apache:mainfrom
aaron-y-chen:fix-fab-azure-tenant-canonicalization

Conversation

@aaron-y-chen

@aaron-y-chen aaron-y-chen commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

related: #71735

Why

In apache-airflow-providers-fab==3.8.1rc1 (#71735), Azure AD token validation uses the configured tenant identifier directly as the expected iss claim. But Azure mints the issuer published in the tenant's OpenID Connect metadata, which always carries the canonical lowercase tenant GUID-- see Microsoft Entra ID guidance on validating the issuer. Deployments configured with a domain name (e.g. *.onmicrosoft.com or a custom domain) or an uppercase GUID therefore fail login with InvalidClaimError.

How to Reproduce

In an environment with apache-airflow-providers-fab==3.8.1rc1 installed, running token validation against domain or uppercase GUID authorities fails:

from types import SimpleNamespace
from airflow.providers.fab.auth_manager.security_manager.override import (
    FabAirflowSecurityManagerOverride as SecurityManager,
)
from authlib.jose import JsonWebKey, jwt as authlib_jwt

TENANT_GUID = "72f988bf-86f1-41af-91ab-2d7cd011db47"
CLIENT_ID = "app-xyz"

key = JsonWebKey.generate_key("RSA", 2048, options={"kid": "test-kid"}, is_private=True)
public_key = key.as_dict(is_private=False, kid="test-kid")
id_token = authlib_jwt.encode(
    {"alg": "RS256", "kid": "test-kid"},
    {
        "iss": f"https://login.microsoftonline.com/{TENANT_GUID}/v2.0",
        "aud": CLIENT_ID,
        "tid": TENANT_GUID,
        "oid": "user-oid",
    },
    key,
).decode()

azure_remote = SimpleNamespace(
    client_kwargs={},
    client_id=CLIENT_ID,
    api_base_url="https://login.microsoftonline.com/microsoft.onmicrosoft.com/oauth2/v2.0/",
)
sm = SimpleNamespace(
    oauth_remotes={"azure": azure_remote},
    _get_azure_tenant_id=lambda: SecurityManager._get_azure_tenant_id(sm),
    _get_microsoft_jwks=lambda: {"keys": [public_key]},
)

# Fails on 3.8.1rc1 with InvalidClaimError: invalid_claim: Invalid claim 'iss'
SecurityManager._decode_and_validate_azure_jwt(sm, id_token)

Summary of Changes

  • Canonicalize & Resolve Tenant GUIDs: Added _resolve_azure_tenant_guid() to normalize GUIDs locally and resolve domain names to canonical tenant GUIDs via Microsoft OpenID Connect discovery (with in-memory caching).
  • Strict Endpoint Parsing & Fail-Closed Security: Replaced regex with urllib.parse.urlsplit to safely extract configured tenant identifiers from HTTPS login.microsoftonline.com endpoints, and added AzureTenantResolutionError(FabException) for discovery/validation failures.
Configured tenant_identifier Resolution Strategy _resolve_azure_tenant_guid Output
Lowercase GUID (72f988bf-86f1-41af-91ab-2d7cd011db47) Validated & normalized locally (no HTTP call) 72f988bf-86f1-41af-91ab-2d7cd011db47
Uppercase GUID (72F988BF-86F1-41AF-91AB-2D7CD011DB47) Normalized to canonical lowercase (no HTTP call) 72f988bf-86f1-41af-91ab-2d7cd011db47
Default Domain (microsoft.onmicrosoft.com) Resolved via Microsoft OpenID discovery metadata 72f988bf-86f1-41af-91ab-2d7cd011db47
Custom Verified Domain (contoso.com) Resolved via Microsoft OpenID discovery metadata <canonical-tenant-guid>


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: [GPT 5.6-sol] following the guidelines

…clarify validation of issuer and audience claims, and support for tenant domain and GUID authorities.
@aaron-y-chen
aaron-y-chen marked this pull request as ready for review August 21, 2026 04:32
@aaron-y-chen
aaron-y-chen requested a review from vincbeck as a code owner August 21, 2026 04:32
3.8.1
.....

.. note::

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove. Changelog are supposed to be updated by release managers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants