This is an automated email from the ASF dual-hosted git repository.
eladkal pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 7b94de7d4d3 Validate issuer and audience of Azure AD id_tokens in FAB
auth manager (#71735)
7b94de7d4d3 is described below
commit 7b94de7d4d3bd768bd9b56f7800e6304e9110207
Author: Jarek Potiuk <[email protected]>
AuthorDate: Tue Aug 18 19:43:19 2026 +0200
Validate issuer and audience of Azure AD id_tokens in FAB auth manager
(#71735)
The Azure id_token signature is verified against Microsoft's key set, but
the decode call passed no claims_options, so authlib's claims.validate()
enforced neither the issuer nor the audience.
The key set in use is the multi-tenant one
(login.microsoftonline.com/common/discovery/keys), which serves signing
keys for every Azure tenant. A correctly-signed token from any tenant
therefore satisfied the signature check, and get_oauth_user_info() then
read the login identity (oid, email, roles) straight out of it.
Pin both claims:
* iss must be the configured tenant, accepting the v1.0
(sts.windows.net/<tenant>/) and v2.0
(login.microsoftonline.com/<tenant>/v2.0) issuer forms.
* aud must be this application's client_id.
The tenant is taken from an explicit tenant_id in client_kwargs when set,
and otherwise from the tenant segment of the configured endpoints, which
is where the documented configuration already puts it. Deployments that
follow the documented setup therefore need no configuration change.
A configuration that identifies no single tenant - the common,
organizations or consumers endpoints - now raises rather than accepting
tokens it cannot attribute to an issuer. That is a behaviour change for
those deployments: they need to set tenant_id explicitly.
The existing test that asserted the verification branch is reached by
default now supplies a tenant-bearing endpoint, since tenant resolution
happens before the key set is fetched.
Generated-by: Claude Opus 5 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
---
.../fab/auth_manager/security_manager/override.py | 60 +++++++++++++-
.../auth_manager/security_manager/test_override.py | 93 +++++++++++++++++++++-
2 files changed, 150 insertions(+), 3 deletions(-)
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
index 2871399cd1c..ec08332f735 100644
---
a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
+++
b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py
@@ -24,6 +24,7 @@ import importlib
import itertools
import json
import logging
+import re
import uuid
from collections.abc import Collection, Iterable, Mapping
from typing import TYPE_CHECKING, Any
@@ -69,6 +70,7 @@ from sqlalchemy.exc import IntegrityError,
MultipleResultsFound
from sqlalchemy.orm import joinedload
from werkzeug.security import check_password_hash, generate_password_hash
+from airflow.exceptions import AirflowConfigException
from airflow.providers.common.compat.sdk import conf
from airflow.providers.fab.auth_manager.models import (
Action,
@@ -2425,13 +2427,69 @@ class
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
return requests.get(MICROSOFT_KEY_SET_URL, timeout=30).json()
+ def _get_azure_tenant_id(self) -> str | None:
+ """
+ Resolve the Azure AD tenant the deployment is configured against.
+
+ Prefers an explicit ``tenant_id`` in ``client_kwargs``; otherwise
derives it from
+ the tenant segment of the configured Azure endpoints, which is where
the documented
+ configuration puts it
(``https://login.microsoftonline.com/<tenant-id>/...``).
+
+ Returns ``None`` when the configuration is tenant-agnostic (the
``common`` or
+ ``organizations`` endpoints), because there is then no single issuer
to pin to.
+ """
+ azure = self.oauth_remotes["azure"]
+
+ tenant_id = azure.client_kwargs.get("tenant_id")
+ if tenant_id:
+ return tenant_id
+
+ for url in (
+ getattr(azure, "api_base_url", None),
+ getattr(azure, "access_token_url", None),
+ getattr(azure, "authorize_url", None),
+ ):
+ if not isinstance(url, str):
+ continue
+ match = re.search(r"login\.microsoftonline\.com/([^/]+)/", url)
+ if match and match.group(1) not in ("common", "organizations",
"consumers"):
+ return match.group(1)
+
+ return None
+
def _decode_and_validate_azure_jwt(self, id_token: str) -> dict[str, str]:
verify_signature =
self.oauth_remotes["azure"].client_kwargs.get("verify_signature", True)
if verify_signature:
from authlib.jose import JsonWebKey, jwt as authlib_jwt
+ tenant_id = self._get_azure_tenant_id()
+ if not tenant_id:
+ raise AirflowConfigException(
+ "Azure AD tenant could not be determined from the OAuth
configuration. "
+ "The Microsoft key set used to verify id_token signatures
serves keys for "
+ "every tenant, so without a tenant the issuer of the token
cannot be "
+ "checked. Configure the tenant-specific endpoints "
+ "(https://login.microsoftonline.com/<tenant-id>/...) or
set 'tenant_id' "
+ "in the azure provider's client_kwargs."
+ )
+
+ claims_options = {
+ # The token must have been issued by the configured tenant.
Both the v1.0 and
+ # v2.0 issuer forms are accepted because either may be
returned depending on
+ # which endpoints the deployment is configured against.
+ "iss": {
+ "essential": True,
+ "values": [
+ f"https://login.microsoftonline.com/{tenant_id}/v2.0";,
+ f"https://sts.windows.net/{tenant_id}/";,
+ ],
+ },
+ # The token must have been minted for this application.
+ "aud": {"essential": True, "value":
self.oauth_remotes["azure"].client_id},
+ }
+
keyset = JsonWebKey.import_key_set(self._get_microsoft_jwks()) #
type: ignore
- claims = authlib_jwt.decode(id_token, keyset)
+ claims = authlib_jwt.decode(id_token, keyset,
claims_options=claims_options)
claims.validate()
return claims
diff --git
a/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
b/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
index 924ec22f6ab..7fd859555ba 100644
---
a/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
+++
b/providers/fab/tests/unit/fab/auth_manager/security_manager/test_override.py
@@ -475,8 +475,15 @@ class TestFabAirflowSecurityManagerOverride:
def test_decode_and_validate_azure_jwt_verifies_signature_by_default(self):
"""Azure AD id_token signatures are verified by default
(verify_signature defaults to True)."""
sm = EmptySecurityManager()
- # client_kwargs does not set verify_signature -> it must default to
verifying
- sm.oauth_remotes = {"azure": Mock(client_kwargs={})}
+ # client_kwargs does not set verify_signature -> it must default to
verifying.
+ # A resolvable tenant is required before the key set is fetched, so
the mock
+ # carries the tenant-specific endpoint the documented configuration
uses.
+ sm.oauth_remotes = {
+ "azure": Mock(
+ client_kwargs={},
+
api_base_url="https://login.microsoftonline.com/tenant-abc/oauth2/v2.0/";,
+ )
+ }
with mock.patch.object(
EmptySecurityManager, "_get_microsoft_jwks",
side_effect=RuntimeError("verify-branch-reached")
@@ -504,6 +511,88 @@ class TestFabAirflowSecurityManagerOverride:
mock_jwks.assert_not_called()
assert result == {"oid": "user-1"}
+ @pytest.mark.parametrize(
+ ("remote_kwargs", "expected"),
+ [
+ pytest.param(
+ {"client_kwargs": {"tenant_id": "explicit-tenant"}},
"explicit-tenant", id="explicit"
+ ),
+ pytest.param(
+ {
+ "client_kwargs": {},
+ "api_base_url":
"https://login.microsoftonline.com/tenant-from-url/oauth2/v2.0/";,
+ },
+ "tenant-from-url",
+ id="from-api-base-url",
+ ),
+ pytest.param(
+ {
+ "client_kwargs": {},
+ "api_base_url": None,
+ "access_token_url":
"https://login.microsoftonline.com/tenant-from-token-url/oauth2/v2.0/token";,
+ },
+ "tenant-from-token-url",
+ id="from-access-token-url",
+ ),
+ ],
+ )
+ def test_get_azure_tenant_id_resolves_configured_tenant(self,
remote_kwargs, expected):
+ """The tenant is taken from client_kwargs when set, otherwise from the
configured endpoints."""
+ sm = EmptySecurityManager()
+ sm.oauth_remotes = {"azure": Mock(**remote_kwargs)}
+
+ assert sm._get_azure_tenant_id() == expected
+
+ @pytest.mark.parametrize("multi_tenant_segment", ["common",
"organizations", "consumers"])
+ def
test_get_azure_tenant_id_returns_none_for_tenant_agnostic_endpoints(self,
multi_tenant_segment):
+ """The shared endpoints identify no single tenant, so no issuer can be
pinned."""
+ sm = EmptySecurityManager()
+ sm.oauth_remotes = {
+ "azure": Mock(
+ client_kwargs={},
+
api_base_url=f"https://login.microsoftonline.com/{multi_tenant_segment}/oauth2/v2.0/";,
+ access_token_url=None,
+ authorize_url=None,
+ )
+ }
+
+ assert sm._get_azure_tenant_id() is None
+
+ def test_decode_and_validate_azure_jwt_requires_a_resolvable_tenant(self):
+ """Without a tenant there is no issuer to check, so the token is not
accepted."""
+ from airflow.exceptions import AirflowConfigException
+
+ sm = EmptySecurityManager()
+ sm.oauth_remotes = {
+ "azure": Mock(
+ client_kwargs={},
+
api_base_url="https://login.microsoftonline.com/common/oauth2/v2.0/";,
+ access_token_url=None,
+ authorize_url=None,
+ )
+ }
+
+ with pytest.raises(AirflowConfigException, match="tenant could not be
determined"):
+ sm._decode_and_validate_azure_jwt("header.payload.signature")
+
+ def test_decode_and_validate_azure_jwt_pins_issuer_and_audience(self):
+ """The decode call constrains both the issuer and the audience of the
token."""
+ sm = EmptySecurityManager()
+ sm.oauth_remotes = {"azure": Mock(client_kwargs={"tenant_id":
"tenant-abc"}, client_id="app-xyz")}
+
+ with mock.patch.object(EmptySecurityManager, "_get_microsoft_jwks",
return_value={"keys": []}):
+ with mock.patch("authlib.jose.JsonWebKey.import_key_set"):
+ with mock.patch("authlib.jose.jwt.decode") as mock_decode:
+
sm._decode_and_validate_azure_jwt("header.payload.signature")
+
+ claims_options = mock_decode.call_args.kwargs["claims_options"]
+ assert claims_options["aud"] == {"essential": True, "value": "app-xyz"}
+ assert claims_options["iss"]["essential"] is True
+ assert claims_options["iss"]["values"] == [
+ "https://login.microsoftonline.com/tenant-abc/v2.0";,
+ "https://sts.windows.net/tenant-abc/";,
+ ]
+
def test_ldap_search_escapes_username_and_validates_filter():
"""Test that LDAP search properly escapes username and validates search
filter."""
