vincbeck commented on code in PR #53397:
URL: https://github.com/apache/airflow/pull/53397#discussion_r2210618694
##########
airflow-core/docs/core-concepts/auth-manager/index.rst:
##########
@@ -145,25 +146,27 @@ These authorization methods are:
* ``is_authorized_custom_view``: Return whether the user is authorized to
access a specific view not defined in Airflow. This view can be provided by the
auth manager itself or a plugin defined by the user.
* ``filter_authorized_menu_items``: Given the list of menu items in the UI,
return the list of menu items the user has access to.
+It should be noted that the API ``methods`` listed above may only have
relevance for a specific subset of the auth manager's authorization methods.
+For example, the ``configuration`` resource is by definition read-only, so
only the ``GET`` API method is relevant in the context of
``is_authorized_configuration``.
Review Comment:
```suggestion
It should be noted that the parameter ``methods`` listed above may only have
relevance for a specific subset of the auth manager's authorization methods.
For example, the ``configuration`` resource is by definition read-only, so
only the ``GET`` parameter method is relevant in the context of
``is_authorized_configuration``.
```
##########
airflow-core/docs/core-concepts/auth-manager/index.rst:
##########
@@ -123,11 +123,12 @@ Let's go over the different parameters used by most of
these methods.
* ``POST``: Can the user create a resource?
* ``PUT``: Can the user modify the resource?
* ``DELETE``: Can the user delete the resource?
- * ``MENU``: Can the user see the resource in the menu?
+ * ``MENU``: Can the user see the resource in the menu? Note: this method is
only applicable to a subset of auth manager Authorization methods.
Review Comment:
This has been removed recently actually. Can you remove `MENU` from the list?
##########
airflow-core/docs/core-concepts/auth-manager/index.rst:
##########
@@ -145,25 +146,27 @@ These authorization methods are:
* ``is_authorized_custom_view``: Return whether the user is authorized to
access a specific view not defined in Airflow. This view can be provided by the
auth manager itself or a plugin defined by the user.
* ``filter_authorized_menu_items``: Given the list of menu items in the UI,
return the list of menu items the user has access to.
+It should be noted that the API ``methods`` listed above may only have
relevance for a specific subset of the auth manager's authorization methods.
+For example, the ``configuration`` resource is by definition read-only, so
only the ``GET`` API method is relevant in the context of
``is_authorized_configuration``.
+
JWT token management by auth managers
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The auth manager is responsible for creating the JWT token needed to interact
with Airflow public API.
To achieve this, the auth manager **must** provide an endpoint to create this
JWT token. This endpoint is usually
available at ``POST /auth/token``.
Please double check the auth manager documentation to find the accurate token
generation endpoint.
-The auth manager is also responsible of passing the JWT token to Airflow UI.
The protocol to exchange the JWT
+The auth manager is also responsible for passing the JWT token to Airflow UI.
The protocol to exchange the JWT
token between the auth manager and Airflow UI is using cookies. The auth
manager needs to save the JWT token in a
-cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI
will then read the cookie, save it and
-delete the cookie.
+cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI
will then read the cookie, save it, and delete it.
.. code-block:: python
from airflow.api_fastapi.auth.managers.base_auth_manager import
COOKIE_NAME_JWT_TOKEN
response = RedirectResponse(url="/")
- secure = bool(conf.get("api", "ssl_cert", fallback=""))
+ secure = request.base_url.scheme == "https" or bool(conf.get("api",
"ssl_cert", fallback=""))
Review Comment:
That looks good to me
##########
airflow-core/docs/core-concepts/auth-manager/index.rst:
##########
@@ -178,6 +181,8 @@ The following methods aren't required to override to have a
functional Airflow a
* ``batch_is_authorized_dag``: Batch version of ``is_authorized_dag``. If not
overridden, it will call ``is_authorized_dag`` for every single item.
* ``get_authorized_dag_ids``: Return the list of DAG IDs the user has access
to. If not overridden, it will call ``is_authorized_dag`` for every single DAG
available in the environment.
+ * Note: The ``get_authorized_dag_ids`` may be of particular interest if you
rely on per-DAG access controls derived from one or more fields on a given DAG
(e.g. DAG tags).
Review Comment:
Actually, thanks for catching this but `get_authorized_dag_ids` is not
supposed to be overridden by users. On the other side,
`filter_authorized_dag_ids` is. `get_authorized_dag_ids` retrieve all the DAGs
from the DB and then call `filter_authorized_dag_ids` to filter DAGs based on
user permissions.
##########
airflow-core/docs/core-concepts/auth-manager/index.rst:
##########
@@ -229,6 +234,27 @@ Other optional methods
their class paths need to be returned as part of this method. By doing so,
they will be automatically added to the
config ``[database] external_db_managers``.
+
+Additional Caveats
+^^^^^^^^^^^^^^^^^^
+
+* Your auth manager should not reference anything from the
``airflow.security.permissions`` module, as that module is in the process of
being deprecated.
+ * Instead, your code should use the definitions in
``airflow.api_fastapi.auth.managers.models.resource_details``.
+* The ``access_control`` attribute of a DAG instance is only compatible with
the FAB auth manager. Custom auth manager implementations should leverage
``get_authorized_dag_ids`` for DAG instance attribute-based access controls in
more customizable ways (e.g. authorization based on DAG tags, DAG bundles,
etc.).
+
+DAG and DAG Sub-Component Authorization
Review Comment:
Nice section! It gives some nice details which can helped to understand
better the permission model 👍
--
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]
