Skip to content

Fix msgraph/Power BI auth failure from empty allowed_hosts list#69014

Open
davidnzhang wants to merge 2 commits into
apache:mainfrom
davidnzhang:fix/msgraph-allowed-hosts-validation
Open

Fix msgraph/Power BI auth failure from empty allowed_hosts list#69014
davidnzhang wants to merge 2 commits into
apache:mainfrom
davidnzhang:fix/msgraph-allowed-hosts-validation

Conversation

@davidnzhang

Copy link
Copy Markdown

What

KiotaRequestAdapterHook._build_request_adapter derives allowed_hosts from the connection's extra field and passes it to AzureIdentityAuthenticationProvider. When neither allowed_hosts nor authority is configured, allowed_hosts returned [""] rather than a true empty list.

When this value eventually flows through to kiota's AllowedHostsValidator.is_url_host_valid method, it fails to skip validation via the early return:

if not self.get_allowed_hosts():
    return True

This PR filters out blank strings so an unconfigured connection yields [] and correctly takes the early exit described above.

Why

A recent change in the microsoft-kiota-abstractions package (microsoft/kiota-python@8755f70) fixed the AllowedHostsValidator to actually enforce a list of allowed hosts. Previously, it only checked whether a URL was structurally valid and ignored the configured allowed_hosts set entirely. The is_url_host_valid method now correctly extracts the hostname from the URL and checks is against the allow list.

This seemed to break existing Power BI connections which did not configure allowed_hosts or authority (which is the default for Power BI connections), as the validator receives a list with an empty string instead of a true empty list, causing requests to fail validation.

Note that before the Microsoft change this was harmless, as the validator method never actually compared against the configured hosts (return all([scheme, netloc]) -> True). This PR restores kiota's "skip validation" pathway for the default case where no allowed_hosts are set.


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

Generated-by: Claude Sonnet 4.6, following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@davidnzhang davidnzhang force-pushed the fix/msgraph-allowed-hosts-validation branch from 2fbbc56 to a738dc2 Compare June 26, 2026 07:25
@davidnzhang davidnzhang marked this pull request as ready for review June 26, 2026 07:26
@davidnzhang davidnzhang requested a review from dabla as a code owner June 26, 2026 07:26

@SameerMesiah97 SameerMesiah97 left a comment

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.

Fix is fine but the logic could be clearer and the test is a bit too shallow.

CI needs to be triggerred as well.

hook = KiotaRequestAdapterHook(conn_id="msgraph_api")
await hook.get_async_conn()

assert mock_auth_provider.call_args.kwargs["allowed_hosts"] == []

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.

Could we make this test exercise the filtering logic? Right now it only verifies the unconfigured case. Since the implementation now filters out empty entries, it woud be good to include input like "host1,,host2," (or even just ",") and assert that only the non-empty hosts are passed to AzureIdentityAuthenticationProvider.

verify = config.get("verify", True)
trust_env = config.get("trust_env", False)
allowed_hosts = (config.get("allowed_hosts", authority) or "").split(",")
allowed_hosts = [host for host in (config.get("allowed_hosts", authority) or "").split(",") if host]

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.

This could be clearer. Please see the below:

allowed_hosts = config.get("allowed_hosts", authority)
if not allowed_hosts:
    allowed_hosts = ""

allowed_hosts = [host for host in allowed_hosts.split(",") if host]

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