dabla commented on code in PR #69742:
URL: https://github.com/apache/airflow/pull/69742#discussion_r3566121436
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/msgraph.py:
##########
Review Comment:
```suggestion
# The pagination link (e.g. ``@odata.nextLink``) is echoed from the
API response and is
# re-fetched with the connection's bearer token attached. Kiota only
scopes that token to
# ``allowed_hosts``, which defaults to empty (any host) unless
configured, so a tampered
# response could redirect the token off-host. Pin follow-up requests
to the configured
# endpoint's host (CWE-918).
self.allowed_netloc = urlparse(request_adapter.base_url).netloc
return request_adapter
```
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/msgraph.py:
##########
@@ -660,6 +667,16 @@ async def run(
data=data,
responses=lambda: responses,
)
+ if (
+ next_url
+ and next_url.startswith("http")
+ and urlparse(next_url).netloc != allowed_netloc
Review Comment:
```suggestion
and urlparse(next_url).netloc !=
self.allowed_netloc
```
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/msgraph.py:
##########
Review Comment:
```suggestion
self.api_version = self.resolve_api_version_from_value(api_version)
self.allowed_netloc: str | None = None
```
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/msgraph.py:
##########
@@ -633,6 +633,13 @@ async def run(
url: str = "",
query_parameters: dict[str, Any] | None = None,
):
+ # The pagination link (e.g. ``@odata.nextLink``) is echoed from
the API response and is
+ # re-fetched with the connection's bearer token attached. Kiota
only scopes that token to
+ # ``allowed_hosts``, which defaults to empty (any host) unless
configured, so a tampered
+ # response could redirect the token off-host. Pin follow-up
requests to the configured
+ # endpoint's host (CWE-918).
+ allowed_netloc = urlparse((await
self.get_async_conn()).base_url).netloc
+
Review Comment:
Remove those lines, we only want to resolve `allowed_netloc` once and don't
want to do unnecessary lookups at each run once it's resolved
##########
providers/microsoft/azure/src/airflow/providers/microsoft/azure/hooks/msgraph.py:
##########
@@ -660,6 +667,16 @@ async def run(
data=data,
responses=lambda: responses,
)
+ if (
+ next_url
+ and next_url.startswith("http")
+ and urlparse(next_url).netloc != allowed_netloc
+ ):
+ raise ValueError(
+ f"Refusing to follow pagination link
{next_url!r}: its host differs "
+ f"from the configured Microsoft Graph endpoint
{allowed_netloc!r}."
Review Comment:
```suggestion
f"from the configured Microsoft Graph
endpoint {self.allowed_netloc!r}."
```
--
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]
