pierrejeambrun commented on code in PR #47885:
URL: https://github.com/apache/airflow/pull/47885#discussion_r2001373841
##########
tests/api_fastapi/execution_api/conftest.py:
##########
@@ -16,12 +16,23 @@
# under the License.
from __future__ import annotations
+from unittest.mock import AsyncMock
+
import pytest
from fastapi.testclient import TestClient
from airflow.api_fastapi.app import cached_app
+from airflow.api_fastapi.auth.tokens import JWTValidator
+from airflow.api_fastapi.execution_api.app import lifespan
@pytest.fixture
-def client():
- return TestClient(cached_app(apps="execution"))
+def client(request: pytest.FixtureRequest):
+ app = cached_app(apps="execution")
+ with TestClient(app, headers={"Authorization": "Bearer fake"}) as client:
+ auth = AsyncMock(spec=JWTValidator)
+ auth.avalidated_claims.return_value = {"sub":
"edb09971-4e0e-4221-ad3f-800852d38085"}
+
+ # Inject our fake JWTValidaotr object. Can be over-ridden by tests if
they want
Review Comment:
```suggestion
# Inject our fake JWTValidator object. Can be over-ridden by tests
if they want
```
##########
airflow/api_fastapi/execution_api/app.py:
##########
@@ -34,10 +37,33 @@
logger = structlog.get_logger(logger_name=__name__)
-@asynccontextmanager
-async def lifespan(app: FastAPI):
- """Context manager for the lifespan of the FastAPI app. For now does
nothing."""
+def _jwt_validator():
+ from airflow.configuration import conf
+
+ required_claims = frozenset(["aud", "exp", "iat"])
+
+ if issuer := conf.get("api_auth", "jwt_issuer", fallback=None):
+ required_claims = required_claims | {"iss"}
+ validator = JWTValidator(
+ required_claims=required_claims,
+ issuer=issuer,
+ leeway=conf.getint("api_auth", "jwt_leeway"),
+ audience=conf.get_mandatory_list_value("execution_api",
"jwt_audience"),
+ **get_sig_validation_args(make_secret_key_if_needed=False),
+ )
+ return validator
+
+
[email protected]
+async def lifespan(app: FastAPI, registry: svcs.Registry):
app.state.lifespan_called = True
+
+ # According to svcs's docs this shouldn't be needed, but something about
SubApps is odd, and we need to
+ # record this here
+ app.state.svcs_registry = registry
+
+ # Create an app scoped validator, so that we don't have to fetch it every
time
+ registry.register_value(JWTValidator, _jwt_validator(),
ping=JWTValidator.status)
Review Comment:
I feel like introducing `svcs` just for caching `JWTValidator` might be a
little bit overkill. (especially when we can achieve the same with fastapi and
dependecy_override for tests) but I might be missing something.
(also not strongly against but I feel like this should be used if the usage
was a little bit more intensive and spread outside of fastapi only)
--
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]
