Authorization and Permissions

[mik-laj]

Description

We need a simple mechanism to authorize operations performed by the API. It should be compatible with the Flask App Builder used by Airflow.

More information about REST API:
#8107

Use case / motivation

The purpose of the authorization is access control, which confirms whether a given entity is authorized to use the requested resource.

Related Issues

N/A

[jhtimmins]

Some thoughts on this:

Challenge

For the new API, we need to decide how to structure permissions for the API and how they should integrate with the existing user permissions schema.

Approaches

1. Tie permissions to specific resources and actions. “Read DAG”, “Update TI”, etc.
2. Tie permissions to specific endpoints.
3. Create ad hoc permissions that encapsulate specific actions in the web UI.

The existing permission structure seems to most-closely follow approach 3. This is problematic, since the web-focused permissions do not apply cleanly to API endpoints.

@ashb and I discussed this, and agree that approach 1 makes the most sense for our specific API system. Individuals can have permission to perform specific actions on specific resources. For individual endpoints, decorators can be used to define the set of permissions required to make use of the endpoint.

For example, to submit a PATCH request to /dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances{task_instance_id}, the user will need edit access for Dags, Dag runs, and Task instances.

So the view function might match the following.

@needs_permissions(['dag_edit', 'dag_run_edit', 'ti_edit'])
def update_ti_view(request):

Breaking Changes

This system will also work for the UI. While not allowing 1:1 permission overlap with the existing permission architecture, we will be able to update required permissions to work on a resource-action basis.

This will be a breaking change, so we need to decide whether or not to continue supporting the existing permission architecture, possibly by creating the new permissions as a standalone subsystem.

Dag-level permissions

Since dags are the only resource where permission can be given out on a per-object basis, the permission system can distinguish between collection access and object access behind the scenes.

User Roles

User roles can be updated to use resource-action permissions.

[jhtimmins]

@mik-laj Very curious about your thoughts

[ashb]

@needs_permissions(['dag_edit', 'dag_run_edit', 'ti_edit'])
def update_ti_view(request):

Should this maybe be ['dag_read', 'dag_run_read', 'ti_edit']?

[jhtimmins]

@ash I think that's a good question. I can see the argument going either way. Do you know if there's a standard here?

[mik-laj]

I think it's best if we reuse the existing operation ID defined in the API to create permission, if possible.

get_config
get_connections
post_connection
delete_connection
get_connection
patch_connection
get_dag_source
get_dags
get_dag
patch_dag
post_clear_task_instances
get_dag_runs
post_dag_run
delete_dag_run
get_dag_run
get_task_instances
get_task_instance
get_extra_links
get_log
get_xcom_entries
get_xcom_entry
get_dag_details
get_tasks
get_task
get_dag_runs_batch
get_task_instances_batch
get_event_logs
get_event_log
get_health
get_import_errors
get_import_error
get_pools
post_pool
delete_pool
get_pool
patch_pool
get_variables
post_variables
delete_variable
get_variable
patch_variable
get_version

If we want, we can combine some operations into one permission e.g. get_dag_runs_batch, get_dag_run, get_dag_run into get_dag_run.

To emphasize that this is related to the API, we can add a common prefix - api_ e.g. api_get_dag_run.

For example, to submit a PATCH request to /dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances{task_instance_id}, the user will need edit access for Dags, Dag runs, and Task instances.

I don't think that editing permission for Dag, DR, TI is required in this case. I think we can trust the administrator and only require DAG view permissions and edit permission for TI. This is done in a similar way in Web UI. In Web UI, we have two decorators: @has_dag_access, @has_access to handle permissions. We can apply a similar approach here.

[ashb]

Right now we have can {add,list,edit,delete} on PoolModelView:

Is there ever a valid use case where you want want a user to have permission only to edit (for example) Pools over the API but not the WWW UI (or via WWW UI but not the API?)

I can't think of any, and I would very strongly like us to not make the security screens any harder to use than they are, and having two permissions for the "same" end effect (i.e. editing Pools) would do that.

I think this would work to make the existing name clearer:

class PoolModelView(AirflowModelView):
    class_permission_name = "Pool"

(See https://github.com/dpgaspar/Flask-AppBuilder/blob/85567d5a15a48aa2ca1950515af379fe347b912a/docs/security.rst#permission-customization)

That would then look at "can_edit on Pool" which makes sense as a permission name for both WWW and API endpoints.

(There's a bit of migration work here to update old mappings to new, but that seems achievable.)

What do you think of this approach? Basically I am proposing changing the permissions from one view=one permission to the view is mapped to one of CRUD (plus a few special case) on the model it operates on.

So I think most of the *ModelView permissions and the API permissions (which map to almost the same thing) should be tied to the same permission

I don't think that editing permission for Dag, DR, TI is required in this case

If we first say "read on Dag, DR", then I can see arguments either way. But at the very least the equivalent of has_dag_access() would be needed on DR/TI, even if we don't need a direct grant of "can_read on DagRun" or similar.

[jhtimmins]

This seems to highlight the fact that there are two separate questions:

1. Should the UI and API use the same permissions?
2. What should the API permissions be named?

1 seems more important, and solving it should simplify question 2. I'm inclined to use the same permissions, assuming we can get close to replicating existing access via the new permission set.

[ashb]

(Oh hah, just discovered that FAB has previous_class_permission_name and so can handle the migration/rename for us if we choose that route)