Create HITL specific permission for core-API

[sjyangkevin]

Close: #53874

1. HITL is an operator, and it provides an interface on which users can action. This interface shows the details about the operator, and can be considered as a sub-entity of the DAG (similar to DAG Runs, Code, XCom, etc.). Therefore, the new permission can be considered as an access to the DAG's sub-entity, HITL_DETAIL in this case, and can be added to the DagAccessEntity.
2. Then, the HITL endpoints can be configured with, e.g., dependencies=[Depends(requires_access_dag(method="PUT", access_entity=DagAccessEntity.HITL_DETAIL))], to check if the user had the access to HITL_DETAIL sub-entity.
3. The DagAccessEntity.HITL_DETAIL is mapped to a resource type RESOURCE_HITL_DETAIL. RESOURCE_HITL_DETAIL need to be defined in providers/fab/src/airflow/providers/fab/www/security/permissions.py and be configured in providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py (with actions "can read", "can edit"). After the configuration, the resource type will show up in ab_view_menu.
4. Then, admin user can configure the access to HITL endpoints by assigning permission such as "can read HITL_DETAIL" to a role and then assigning the role to user.

---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

[sjyangkevin]

Hi @Lee-W , I've created an initial draft, and want to see if I am on the right direction. Would you mind have a look on it when you have time and let me know if any of the comments here are not clear. I will try my best to explain my understanding and approach. Thanks!

[sjyangkevin]

In the provider check, I keep seeing the following error. It looks like there is a compatibility issue with 3.0.3 after adding the HITL_DETAIL to DagAccessEntity. I attempted to use the approach here #54043 (comment), but it doesn't look like the correct way to resolve this compatibility issue.

__________________________________________________________ ERROR collecting providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py ___________________________________________________________
providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py:48: in <module>
    from airflow.providers.fab.auth_manager.fab_auth_manager import FabAuthManager
/usr/local/lib/python3.10/site-packages/airflow/providers/fab/auth_manager/fab_auth_manager.py:134: in <module>
    DagAccessEntity.HITL_DETAIL: (RESOURCE_HITL_DETAIL,),
/usr/local/lib/python3.10/enum.py:437: in __getattr__
    raise AttributeError(name) from None
E   AttributeError: HITL_DETAIL

I would like to provide a summary regarding my approach of adding a new permission, and I would appreciate if I could get some guidance on how to properly make it available for only after 3.1.0.

1. HITL is an operator, and it provides an interface on which users can action. This interface shows the details about the operator, and can be considered as a sub-entity of the DAG (similar to DAG Runs, Code, XCom, etc.). Therefore, the new permission can be considered as an access to the DAG's sub-entity, HITL_DETAIL in this case, and can be added to the DagAccessEntity.
2. Then, the HITL endpoints can be configured with, e.g., dependencies=[Depends(requires_access_dag(method="PUT", access_entity=DagAccessEntity.HITL_DETAIL))], to check if the user had the access to HITL_DETAIL sub-entity.
3. The DagAccessEntity.HITL_DETAIL is mapped to a resource type RESOURCE_HITL_DETAIL. RESOURCE_HITL_DETAIL need to be defined in providers/fab/src/airflow/providers/fab/www/security/permissions.py and be configured in providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py (with actions "can read", "can edit"). After the configuration, the resource type will show up in ab_view_menu.
4. Then, admin user can configure the access to HITL endpoints by assigning permission such as "can read HITL_DETAIL" to a role and then assigning the role to user.

I think the proper approach is to have a check for Airflow version and then define/register this new permission only the version is 3.1.0. However, it looks like we need to introduce a lot of version check across core, providers, and tests.. Not sure if there is a better way to implement it. Thanks!

[Lee-W]

https://github.com/apache/airflow/pull/53035/files#diff-49012e8ddf75495e09f575dd46aa20b7639df73ff3aabe87000ca1474eefc9f4R174-R177

This is how I did the 3.1 check in another PR. and yes, we'll need to add this check in multiple places but I think we've already have most of them 🤔 is there anything missed?

[Lee-W]

The main branch fixes a CI issue. Thus I'll rebase the branch from main and start my first round review to provide early feedback 🙂

[Lee-W]

Mostly looks good. Except for a few minor adjustment

[sjyangkevin]

https://github.com/apache/airflow/pull/53035/files#diff-49012e8ddf75495e09f575dd46aa20b7639df73ff3aabe87000ca1474eefc9f4R174-R177

This is how I did the 3.1 check in another PR. and yes, we'll need to add this check in multiple places but I think we've already have most of them 🤔 is there anything missed?

Thanks, I just have a look into the #53035, and I think most of the changes related to permission are consistent with this PR. I think we also need to register that resource for dag access entity. Will try to add one and see if I can resolve the CI failure

[sjyangkevin]

Mostly looks good. Except for a few minor adjustment

Thanks! I will make those adjustments, and attach more test evidence, as well as checking if the test cases needed to be adjusted.

[pierrejeambrun]

Nice looking good beside compat check mentionned by Lee.

cc: @vincbeck

[vincbeck]

LGTM

[jason810496]

Nice! Thanks for the PR and LGTM as well.

[sjyangkevin]

Thanks everyone's time on reviewing this PR. I've made the following updates to resolve the compatibility test failures.

1. I've done some clean up since most of the changes have been added in Add UI for human in the loop operators #53035.
2. The compat check is resolved by the following two steps 1.) register DagAccessEntity.HITL_DETAIL only when Airflow version is greater than or equal to 3.1.0 in fab auth manager 2.) create a set of tests and only import/run those when Airflow version is greater than or equal to 3.1.0.

@Lee-W , @pierrejeambrun , @vincbeck , and @jason810496 feel free to let me know if you have further feedback.

I also did some manual functional tests by running Airflow with HITL example DAG.

First, I defined the following roles and assign the roles to the test user.

With HITL_Viewer role, the user can only access those GET endpoints but cannot access PATCH endpoints, as shown below.

Test access to an GET endpoint

Test access to a PATCH endpoint

Test access to a PATCH endpoint

With HITL_Editor role, the user's access to PATCH endpoints will not be forbidden.

[Lee-W]

Looks great! Let's merge it!