vincbeck commented on code in PR #34317:
URL: https://github.com/apache/airflow/pull/34317#discussion_r1355177606
##########
airflow/api_connexion/security.py:
##########
@@ -48,10 +61,194 @@ def requires_access_decorator(func: T):
@wraps(func)
def decorated(*args, **kwargs):
check_authentication()
- if appbuilder.sm.check_authorization(permissions,
kwargs.get("dag_id")):
+ if appbuilder.sm.check_authorization(permissions):
return func(*args, **kwargs)
raise PermissionDenied()
return cast(T, decorated)
return requires_access_decorator
+
+
+def _requires_access(*, is_authorized_callback: Callable[[], bool], func:
Callable, args, kwargs) -> bool:
+ """
+ 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
+ :param func: the function to call if the user is authorized
+ :param args: the arguments of ``func``
+ :param kwargs: the keyword arguments ``func``
+
+ :meta private:
+ """
+ check_authentication()
+ if is_authorized_callback():
+ return func(*args, **kwargs)
+ raise PermissionDenied()
+
+
+def requires_authentication(func: T):
+ """Decorator for functions that require authentication."""
+
+ @wraps(func)
+ def decorated(*args, **kwargs):
+ check_authentication()
+ return func(*args, **kwargs)
+
+ return cast(T, decorated)
+
+
+def requires_access_configuration(method: ResourceMethod) -> Callable[[T], T]:
+ def requires_access_decorator(func: T):
+ @wraps(func)
+ def decorated(*args, **kwargs):
+ section: str | None = kwargs.get("section")
+ return _requires_access(
+ is_authorized_callback=lambda:
get_auth_manager().is_authorized_configuration(
+ method=method,
details=ConfigurationDetails(section=section)
+ ),
+ func=func,
+ args=args,
+ kwargs=kwargs,
+ )
+
+ return cast(T, decorated)
+
+ return requires_access_decorator
+
+
+def requires_access_connection(method: ResourceMethod) -> Callable[[T], T]:
+ def requires_access_decorator(func: T):
+ @wraps(func)
+ def decorated(*args, **kwargs):
+ connection_id: str | None = kwargs.get("connection_id")
+ return _requires_access(
+ is_authorized_callback=lambda:
get_auth_manager().is_authorized_connection(
+ method=method,
details=ConnectionDetails(conn_id=connection_id)
+ ),
+ func=func,
+ args=args,
+ kwargs=kwargs,
+ )
+
+ return cast(T, decorated)
+
+ return requires_access_decorator
+
+
+def requires_access_dag(
+ method: ResourceMethod, access_entity: DagAccessEntity | None = None
+) -> Callable[[T], T]:
+ appbuilder = get_airflow_app().appbuilder
+
+ def _is_authorized_callback(dag_id: str):
+ def callback():
+ access = get_auth_manager().is_authorized_dag(
+ method=method,
+ access_entity=access_entity,
+ details=DagDetails(id=dag_id),
+ )
+
+ # ``access`` means here:
+ # - if a DAG id is provided (``dag_id`` not None): is the user
authorized to access this DAG
+ # - if no DAG id is provided: is the user authorized to access all
DAGs
+ if dag_id or access:
+ return access
+
+ # No DAG id is provided and the user is not authorized to access
all DAGs
+ # If method is "GET", return whether the user has read access to
any DAGs
+ # If method is "PUT", return whether the user has edit access to
any DAGs
+ return (method == "GET" and
any(appbuilder.sm.get_readable_dag_ids())) or (
+ method == "PUT" and any(appbuilder.sm.get_editable_dag_ids())
+ )
+
+ return callback
+
+ def requires_access_decorator(func: T):
+ @wraps(func)
+ def decorated(*args, **kwargs):
Review Comment:
Sure! Happy to do that in another PR. Let me put it in my list :)
##########
airflow/www/extensions/init_jinja_globals.py:
##########
@@ -69,10 +70,13 @@ def prepare_jinja_globals():
"git_version": git_version,
"k8s_or_k8scelery_executor": IS_K8S_OR_K8SCELERY_EXECUTOR,
"rest_api_enabled": False,
- "auth_manager": get_auth_manager(),
"config_test_connection": conf.get("core", "test_connection",
fallback="Disabled"),
}
+ # Extra global specific to auth manager
+ extra_globals["auth_manager"] = get_auth_manager()
+ extra_globals["DagDetails"] = DagDetails
Review Comment:
I could do that. The only reason I did that was in case we needed it in the
future for other things. But I guess I can do as you said for now
##########
airflow/www/extensions/init_jinja_globals.py:
##########
@@ -69,10 +70,13 @@ def prepare_jinja_globals():
"git_version": git_version,
"k8s_or_k8scelery_executor": IS_K8S_OR_K8SCELERY_EXECUTOR,
"rest_api_enabled": False,
- "auth_manager": get_auth_manager(),
"config_test_connection": conf.get("core", "test_connection",
fallback="Disabled"),
}
+ # Extra global specific to auth manager
+ extra_globals["auth_manager"] = get_auth_manager()
+ extra_globals["DagDetails"] = DagDetails
Review Comment:
But looking at the code, `dag.html` is used a lot in multiple views. So I
would have to compute and pass the value `can_edit` in multiple views. I am not
sure this is better either. WDYT?
##########
airflow/api_connexion/endpoints/task_endpoint.py:
##########
@@ -22,21 +22,16 @@
from airflow.api_connexion import security
from airflow.api_connexion.exceptions import BadRequest, NotFound
from airflow.api_connexion.schemas.task_schema import TaskCollection,
task_collection_schema, task_schema
+from airflow.auth.managers.models.resource_details import DagAccessEntity
from airflow.exceptions import TaskNotFound
-from airflow.security import permissions
from airflow.utils.airflow_flask_app import get_airflow_app
if TYPE_CHECKING:
from airflow import DAG
from airflow.api_connexion.types import APIResponse
[email protected]_access(
- [
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
- (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
- ],
-)
[email protected]_access_dag("GET", DagAccessEntity.TASK_INSTANCE)
Review Comment:
Can you elaborate? This is the exact same permissions we were checking
before. Before we were checking permissions on task instance and dag level.
`@security.requires_access_dag("GET", DagAccessEntity.TASK_INSTANCE)` does
exactly the same
--
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]
