potiuk commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1355925044
##########
airflow/api_connexion/endpoints/xcom_endpoint.py:
##########
@@ -39,14 +39,7 @@
from airflow.api_connexion.types import APIResponse
-@security.requires_access(
- [
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_XCOM),
- ],
-)
+@security.requires_access_dag("GET", DagAccessEntity.XCOM)
Review Comment:
Yes. I think those "Resource" specifications were really too fine
grained/unnecessary (see comment above
https://github.com/apache/airflow/pull/34317/files#r1355917769). Also see the
"philosophy" change we want to have
https://github.com/apache/airflow/pull/34317/files#r1355923325.
The Auth Manager should not be asked if the user needs to access all the
multiple resources (XCom, TI, DagRun, dag) - we just want to check if the user
can read this particular Xcom value. For FAB it might be mapped to those 4
resources, but other auth managers might make different decisions - andthe user
might be able to only read XCOM data (but not DAG_RUN or DAG - this is entirely
plausible.).
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
