o-nikolas commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1339330087
##########
airflow/api_connexion/security.py:
##########
@@ -55,3 +68,166 @@ def decorated(*args, **kwargs):
return cast(T, decorated)
return requires_access_decorator
+
+
+def _requires_access(*, is_authorized_callback: Callable[[], bool], func:
Callable, args, kwargs):
+ """
+ Define the behavior whether the user is authorized to access the resource.
+
+ :param is_authorized_callback: callback to execute to figure whether the
user is authorized to access
+ the resource?
Review Comment:
minor nit, but why the question mark at the end here?
##########
airflow/auth/managers/base_auth_manager.py:
##########
@@ -145,12 +151,30 @@ def is_authorized_dataset(
self,
*,
method: ResourceMethod,
+ details: DatasetDetails | None = None,
user: BaseUser | None = None,
) -> bool:
"""
Return whether the user is authorized to perform a given action on a
dataset.
:param method: the method to perform
+ :param details: optional details about the variable
Review Comment:
```suggestion
:param details: optional details about the dataset
```
##########
airflow/auth/managers/base_auth_manager.py:
##########
@@ -82,12 +86,14 @@ def is_authorized_configuration(
self,
*,
method: ResourceMethod,
+ details: ConfigurationDetails | None = None,
user: BaseUser | None = None,
) -> bool:
"""
Return whether the user is authorized to perform a given action on
configuration.
:param method: the method to perform
+ :param details: optional details about the connection
Review Comment:
```suggestion
:param details: optional details about the configuration
```
##########
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:
Just for my own understanding, why are we checking these things when we're
interested if the user is authorized for the website?
##########
airflow/auth/managers/base_auth_manager.py:
##########
@@ -145,12 +151,30 @@ def is_authorized_dataset(
self,
*,
method: ResourceMethod,
+ details: DatasetDetails | None = None,
user: BaseUser | None = None,
) -> bool:
"""
Return whether the user is authorized to perform a given action on a
dataset.
:param method: the method to perform
+ :param details: optional details about the variable
+ :param user: the user to perform the action on. If not provided (or
None), it uses the current user
+ """
+
+ @abstractmethod
+ def is_authorized_pool(
+ self,
+ *,
+ method: ResourceMethod,
+ details: PoolDetails | None = None,
+ user: BaseUser | None = None,
+ ) -> bool:
+ """
+ Return whether the user is authorized to perform a given action on a
pool.
+
+ :param method: the method to perform
+ :param details: optional details about the variable
Review Comment:
```suggestion
:param details: optional details about the pool
```
##########
airflow/api_connexion/endpoints/dag_endpoint.py:
##########
@@ -69,7 +69,7 @@ def get_dag_details(*, dag_id: str) -> APIResponse:
return dag_detail_schema.dump(dag)
[email protected]_access([(permissions.ACTION_CAN_READ,
permissions.RESOURCE_DAG)])
+@requires_authentication
Review Comment:
Just curious, why no method on this one? I expected:
```python
@security.requires_access_dag("GET")
```
--
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]
