vincbeck commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1340301025
##########
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:
Good question. Long story short is we decided to "group" FAB resource types
into more broad categories. The number of resource types defined by FAB is too
big and not convenient to define permissions. So we decided to create a
category (here called website) which groups `RESOURCE_PLUGIN`,
`RESOURCE_PROVIDER `, `RESOURCE_TRIGGER` and `RESOURCE_WEBSITE`. You can see
the discussion about this topic in this [comment
thread](https://github.com/apache/airflow/pull/33213#discussion_r1308050422).
--
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]
