o-nikolas commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1340500642
##########
airflow/auth/managers/fab/fab_auth_manager.py:
##########
@@ -171,34 +193,53 @@ def is_authorized_dag(
entity (e.g. DAG runs).
2. ``dag_access`` is provided which means the user wants to access a
sub entity of the DAG
(e.g. DAG runs).
- a. If ``method`` is GET, then check the user has READ permissions
on the DAG and the sub entity
- b. Else, check the user has EDIT permissions on the DAG and
``method`` on the sub entity
+ a. If ``method`` is GET, then check the user has READ permissions
on the DAG and the sub entity.
+ b. Else, check the user has EDIT permissions on the DAG and
``method`` on the sub entity.
+
+ However, if no specific DAG is targeted, just check the sub entity.
:param method: The method to authorize.
- :param dag_access_entity: The dag access entity.
- :param dag_details: The dag details.
+ :param access_entity: The dag access entity.
+ :param details: The dag details.
:param user: The user.
"""
- if not dag_access_entity:
+ if not access_entity:
# Scenario 1
- return self._is_authorized_dag(method=method,
dag_details=dag_details, user=user)
+ return self._is_authorized_dag(method=method, details=details,
user=user)
else:
# Scenario 2
- resource_type = self._get_fab_resource_type(dag_access_entity)
+ resource_type = self._get_fab_resource_type(access_entity)
dag_method: ResourceMethod = "GET" if method == "GET" else "PUT"
- return self._is_authorized_dag(
- method=dag_method, dag_details=dag_details, user=user
- ) and self._is_authorized(method=method,
resource_type=resource_type, user=user)
+ if (details and details.id) and not self._is_authorized_dag(
+ method=dag_method, details=details, user=user
+ ):
+ return False
+
+ return self._is_authorized(method=method,
resource_type=resource_type, user=user)
- def is_authorized_dataset(self, *, method: ResourceMethod, user: BaseUser
| None = None) -> bool:
+ def is_authorized_dataset(
+ self, *, method: ResourceMethod, details: DatasetDetails | None =
None, user: BaseUser | None = None
+ ) -> bool:
return self._is_authorized(method=method,
resource_type=RESOURCE_DATASET, user=user)
- def is_authorized_variable(self, *, method: ResourceMethod, user: BaseUser
| None = None) -> bool:
+ def is_authorized_pool(
+ self, *, method: ResourceMethod, details: PoolDetails | None = None,
user: BaseUser | None = None
+ ) -> bool:
+ return self._is_authorized(method=method, resource_type=RESOURCE_POOL,
user=user)
+
+ def is_authorized_variable(
+ self, *, method: ResourceMethod, details: VariableDetails | None =
None, user: BaseUser | None = None
+ ) -> bool:
return self._is_authorized(method=method,
resource_type=RESOURCE_VARIABLE, user=user)
def is_authorized_website(self, *, user: BaseUser | None = None) -> bool:
- return self._is_authorized(method="GET",
resource_type=RESOURCE_WEBSITE, user=user)
+ return (
+ self._is_authorized(method="GET", resource_type=RESOURCE_PLUGIN,
user=user)
+ or self._is_authorized(method="GET",
resource_type=RESOURCE_PROVIDER, user=user)
+ or self._is_authorized(method="GET",
resource_type=RESOURCE_TRIGGER, user=user)
Review Comment:
Yeah, grouping in general makes sense to me. But I suppose in this case I
don't see the logical grouping between plugin, provider, trigger and website.
But I have much less context so I'll trust Jarek and you on this one :)
--
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]
