aaron-y-chen commented on code in PR #71920:
URL: https://github.com/apache/airflow/pull/71920#discussion_r3837509266
##########
providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py:
##########
@@ -2427,44 +2432,126 @@ def _get_microsoft_jwks(self) -> list[dict[str, Any]]:
return requests.get(MICROSOFT_KEY_SET_URL, timeout=30).json()
- def _get_azure_tenant_id(self) -> str | None:
+ def _get_azure_tenant_identifier(self) -> str | None:
"""
- Resolve the Azure AD tenant the deployment is configured against.
+ Extract the configured Azure AD tenant identifier.
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>/...``).
+ the tenant path segment of configured Azure HTTPS endpoints on
``login.microsoftonline.com``.
- Returns ``None`` when the configuration is tenant-agnostic (the
``common`` or
- ``organizations`` endpoints), because there is then no single issuer
to pin to.
+ Returns ``None`` when no tenant can be determined or when the
configuration uses
+ tenant-agnostic endpoints (``common``, ``organizations``,
``consumers``).
"""
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)
+ if tenant_id and isinstance(tenant_id, str) and tenant_id.strip():
+ tenant_identifier = tenant_id.strip()
+ else:
+ tenant_identifier = None
+ 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
+ parsed = urllib.parse.urlsplit(url)
+ if parsed.scheme.lower() != "https":
+ continue
+ if (parsed.hostname or "").lower() !=
"login.microsoftonline.com":
+ continue
+ path_parts = [segment for segment in parsed.path.split("/") if
segment]
+ if path_parts:
+ tenant_identifier = path_parts[0]
+ break
- return None
+ if not tenant_identifier or tenant_identifier.lower() in ("common",
"organizations", "consumers"):
+ return None
+ return tenant_identifier
+
+ def _resolve_azure_tenant_guid(self, tenant_identifier: str) -> str:
+ """
+ Resolve an Azure tenant identifier (GUID or domain) to a canonical
tenant GUID.
+
+ If the identifier is a valid UUID, it is normalized to lowercase
hyphenated format
+ without calling any network endpoint. If the identifier is a domain,
its OpenID
+ discovery metadata is queried from login.microsoftonline.com to
extract the canonical
+ tenant GUID from the issuer claim. Successful resolutions are cached.
+ """
+ try:
+ return str(uuid.UUID(tenant_identifier))
+ except (ValueError, AttributeError):
+ pass
+
+ if tenant_identifier in self._azure_tenant_guid_cache:
+ return self._azure_tenant_guid_cache[tenant_identifier]
+
+ import requests
Review Comment:
okay it meets coding standards in agents.md, let me modify it.
https://github.com/apache/airflow/blob/28110443797d37eb8cb9153cbf2e14502b616956/AGENTS.md?plain=1#L140
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
