You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/08 21:42:02 UTC
[GitHub] [airflow] jhtimmins opened a new pull request #11362: Use resource permissions for Airflow view access
jhtimmins opened a new pull request #11362:
URL: https://github.com/apache/airflow/pull/11362
This PR is part of the "unofficial" epic to replace the current Flask Appbuilder permissions with updated, resource-based permissions. Rather than assigning access to individual view methods, access is doled out based on the data objects and models accessed within the method.
*This PR shouldn't be merged until after #11189*
This epic has three steps, *this is step 2*.
1. Update the dag-level permissions. Prefixes the DAG resource name with `DAG:`, and replaces `can_dag_edit` and `can_dag_read` with `can_read` and `can_edit`. https://github.com/apache/airflow/pull/11189
2. Migrate the Airflow view to use resource-backed permissions. This includes creation of a custom `has_access` decorator, deprecation of the `has_dag_access`decorator, a migration script to update existing roles to use the new permissions, and adding the new `has_access` decorator to the Airflow view class's methods.
3. Migration of the other view classes (beyond the class named Airflow) to use resource based permissions.
Related Issues:
https://github.com/apache/airflow/issues/10469
https://github.com/apache/airflow/issues/8112
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r509297000
##########
File path: airflow/security/permissions.py
##########
@@ -16,25 +16,34 @@
# under the License.
# Resource Constants
-RESOURCE_DAGS = 'Dags'
+RESOURCE_ADMIN = 'Admin'
+RESOURCE_AIRFLOW = 'Airflow'
+RESOURCE_BROWSE_MENU = 'Browse'
+RESOURCE_DAGS = 'DAGs'
RESOURCE_DAG_PREFIX = 'DAG:'
-RESOURCE_CONFIG = 'Config'
-RESOURCE_CONNECTION = 'Connection'
-RESOURCE_DAG_CODE = 'DagCode'
-RESOURCE_DAG_RUN = 'DagRun'
+RESOURCE_DOCS_MENU = 'Docs'
+RESOURCE_DOCS_LINK = 'Documentation'
+RESOURCE_CONFIG = 'Configurations'
+RESOURCE_CONNECTION = 'Connections'
+RESOURCE_DAG_CODE = 'DAG Code'
+RESOURCE_DAG_RUN = "DAG Runs"
RESOURCE_IMPORT_ERROR = 'ImportError'
-RESOURCE_LOG = 'Log'
-RESOURCE_POOL = 'Pool'
-RESOURCE_TASK = 'Task'
-RESOURCE_TASK_INSTANCE = 'TaskInstance'
-RESOURCE_VARIABLE = "Variable"
+RESOURCE_JOB = "Jobs"
+RESOURCE_LOG = 'Logs'
Review comment:
Sure, I'm fine with that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r510678868
##########
File path: docs/security/access-control.rst
##########
@@ -134,51 +123,106 @@ Permissions (each consistent of a resource + action pair) are then added to role
**To access an endpoint, the user needs all permissions assigned to that endpoint**
-================================================================================== ====== ====================================================================================
+There are five default roles: Public, Viewer, User, Op, and Admin. Each one has the permissions of the preceeding role, as well as additional permissions.
+
+================================================================================== ====== ================================================================= ============
Stable API Permissions
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-Endpoint Method Permissions
-================================================================================== ====== ====================================================================================
-/config GET Config.can_read
-/connections GET Connection.can_read
-/connections POST Connection.can_create
-/connections/{connection_id} DELETE Connection.can_delete
-/connections/{connection_id} GET Connection.can_read
-/connections/{connection_id} PATCH Connection.can_edit
-/dagSources/{file_token} GET DagCode.can_read
-/dags GET Dag.can_read
-/dags/{dag_id} GET Dag.can_read
-/dags/{dag_id} PATCH Dag.can_edit
-/dags/{dag_id}/clearTaskInstances POST Dag.can_read, DagRun.can_read, Task.can_edit
-/dags/{dag_id}/details GET Dag.can_read
-/dags/{dag_id}/tasks GET Dag.can_read, Task.can_read
-/dags/{dag_id}/tasks/{task_id} GET Dag.can_read, Task.can_read
-/dags/{dag_id}/dagRuns GET Dag.can_read, DagRun.can_read
-/dags/{dag_id}/dagRuns POST Dag.can_read, DagRun.can_create
-/dags/{dag_id}/dagRuns/{dag_run_id} DELETE Dag.can_read, DagRun.can_delete
-/dags/{dag_id}/dagRuns/{dag_run_id} GET Dag.can_read, DagRun.can_read
-/dags/~/dagRuns/list POST Dag.can_read, DagRun.can_read
-/eventLogs GET Log.can_read
-/eventLogs/{event_log_id} GET Log.can_read
-/importErrors GET ImportError.can_read
-/importErrors/{import_error_id} GET ImportError.can_read
-/health GET None
-/version GET None
-/pools GET Pool.can_read
-/pools POST Pool.can_create
-/pools/{pool_name} DELETE Pool.can_delete
-/pools/{pool_name} GET Pool.can_read
-/pools/{pool_name} PATCH Pool.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/~/dagRuns/~/taskInstances/list POST Dag.can_read, DagRun.can_read, Task.can_read
-/variables GET Variable.can_read
-/variables POST Variable.can_create
-/variables/{variable_key} DELETE Variable.can_delete
-/variables/{variable_key} GET Variable.can_read
-/variables/{variable_key} PATCH Variable.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-================================================================================== ====== ====================================================================================
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+Endpoint Method Permissions Minimum Role
+================================================================================== ====== ================================================================= ============
+/config GET Configurations.can_read Viewer
+/connections GET Connections.can_read Op
+/connections POST Connections.can_create Op
+/connections/{connection_id} DELETE Connections.can_delete Op
+/connections/{connection_id} PATCH Connections.can_edit Op
+/connections/{connection_id} GET Connections.can_read Op
+/dagSources/{file_token} GET DAG Code.can_read Viewer
+/dags GET DAGs.can_read Viewer
+/dags/{dag_id} GET DAGs.can_read Viewer
+/dags/{dag_id} PATCH DAGs.can_edit User
+/dags/{dag_id}/clearTaskInstances POST DAGs.can_read, DAG Runs.can_read, Tasks.can_edit User
+/dags/{dag_id}/details GET DAGs.can_read Viewer
+/dags/{dag_id}/tasks GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/tasks/{task_id} GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/{dag_id}/dagRuns POST DAGs.can_read, DAG Runs.can_create User
+/dags/{dag_id}/dagRuns/{dag_run_id} DELETE DAGs.can_read, DAG Runs.can_delete User
+/dags/{dag_id}/dagRuns/{dag_run_id} GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/~/dagRuns/list POST DAGs.can_read, DAG Runs.can_read Viewer
+/eventLogs GET Logs.can_read Viewer
+/eventLogs/{event_log_id} GET Logs.can_read Viewer
+/importErrors GET ImportError.can_read Viewer
+/importErrors/{import_error_id} GET ImportError.can_read Viewer
+/health GET None Public
+/version GET None Public
+/pools GET Pool.can_read Op
+/pools POST Pool.can_create Op
+/pools/{pool_name} DELETE Pool.can_delete Op
+/pools/{pool_name} GET Pool.can_read Op
+/pools/{pool_name} PATCH Pool.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/~/dagRuns/~/taskInstances/list POST DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/variables GET Variables.can_read Op
+/variables POST Variables.can_create Op
+/variables/{variable_key} DELETE Variables.can_delete Op
+/variables/{variable_key} GET Variables.can_read Op
+/variables/{variable_key} PATCH Variables.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+================================================================================== ====== ================================================================= ============
+
+
+====================================== ======================================================================= ============
+Website Permissions
+-------------------------------------- ------------------------------------------------------------------------------------
+Action Permissions Minimum Role
+====================================== ======================================================================= ============
+Access homepage Website.can_read Viewer
+Get DAG stats Dags.can_read, DAG Runs.can_read Viewer
Review comment:
Added here https://github.com/apache/airflow/pull/11362/files#diff-5821e749f4956a14f3d0aa939985a5422ab9724bc11184244b47f620dce435f6R131
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #11362:
URL: https://github.com/apache/airflow/pull/11362
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r507633734
##########
File path: airflow/www/views.py
##########
@@ -859,8 +895,12 @@ def rendered(self):
title=title)
@expose('/get_logs_with_metadata')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_LOG),
Review comment:
```suggestion
```
##########
File path: airflow/www/views.py
##########
@@ -1847,10 +1964,14 @@ class GraphForm(DateTimeWithNumRunsWithDagRunsForm):
dag_run_state=dt_nr_dr_data['dr_state'])
@expose('/duration')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ ])
@action_logging
- @provide_session
+ @provide_session # pylint: disable=too-many-locals
Review comment:
Errant pylint comment on decorator?
##########
File path: airflow/www/views.py
##########
@@ -2252,8 +2392,11 @@ def gantt(self, session=None):
)
@expose('/extra_links')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
Review comment:
```suggestion
(permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
(permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
```
##########
File path: airflow/www/views.py
##########
@@ -978,8 +1024,14 @@ def log(self, session=None):
root=root, wrapped=conf.getboolean('webserver', 'default_wrap'))
@expose('/redirect_to_external_log')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_LOG),
Review comment:
```suggestion
```
##########
File path: airflow/www/views.py
##########
@@ -1761,8 +1873,13 @@ def recurse_nodes(task, visited):
external_log_name=external_log_name)
@expose('/graph')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_LOG),
Review comment:
```suggestion
```
##########
File path: airflow/www/views.py
##########
@@ -944,8 +984,14 @@ def get_logs_with_metadata(self, session=None):
return jsonify(message=error_message, error=True, metadata=metadata)
@expose('/log')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_LOG),
Review comment:
```suggestion
```
##########
File path: airflow/www/views.py
##########
@@ -1365,8 +1450,12 @@ def clear(self):
recursive=recursive, confirmed=confirmed, only_failed=only_failed)
@expose('/dagrun_clear', methods=['POST'])
- @has_dag_access(can_dag_edit=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
+ (permissions.ACTION_CAN_DELETE, permissions.RESOURCE_TASK_INSTANCE),
Review comment:
```suggestion
(permissions.ACTION_CAN_DELETE, permissions.RESOURCE_DAG_RUN),
```
Possibly _also_ can_delete TI
##########
File path: airflow/www/views.py
##########
@@ -394,8 +394,10 @@ def health(self):
return wwwutils.json_response(payload)
@expose('/home')
- @has_access
- def index(self): # pylint: disable=too-many-locals,too-many-statements
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
Review comment:
So other than this one case, the website permission feels entirely needless. I wonder if this one could use `CAN_READ, RESOURCE_DAGS` here, and entirely the website permissions.
(can read dags here, because the home page is effectively a list of the DAGs)
##########
File path: airflow/www/views.py
##########
@@ -2108,8 +2237,10 @@ def landing_times(self, session=None):
)
@expose('/paused', methods=['POST'])
- @has_dag_access(can_dag_edit=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAGS),
Review comment:
Hmmmmm - I do wonder if long term we'd want more granular permissions on DAGs.
For example, being able to pause/unpause is a lot less "dangerous" than some things we could do on a DAG. (I guess right now there aren't any other edit operations on a DAG)
##########
File path: tests/www/test_views.py
##########
@@ -2058,17 +2315,42 @@ def test_run_success_for_all_dag_user(self):
self.check_content_in_response('', resp, resp_code=302)
def test_blocked_success(self):
- url = 'blocked'
self.logout()
- self.login()
+ username = 'blocked_success_user'
+ fab_utils.create_user(
+ self.app,
+ username=username,
+ role_name='blocked_success_role',
+ permissions=[
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ ]
+ )
+ self.login(username=username,
+ password=username)
Review comment:
This block appears a lot. I wonder if it's worth making a new helper methond on self:
```suggestion
username = 'blocked_success_user'
self.make_user_and_login
username=username,
permissions=[
(permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
(permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
(permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
]
)
```
##########
File path: airflow/www/views.py
##########
@@ -1595,11 +1702,16 @@ def success(self):
future, past, State.SUCCESS)
@expose('/tree')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_LOG),
Review comment:
```suggestion
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r506982825
##########
File path: airflow/www/views.py
##########
@@ -858,8 +856,11 @@ def rendered(self):
title=title)
@expose('/get_logs_with_metadata')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ ("can_read", "Dag"),
+ ("can_read", "TaskInstance"),
+ ("can_read", "Log"),
Review comment:
Wrong Log I think -- this is the LOG_RESOURCE which is audit logs
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-714273877
@ashb ok I added the docs for the website. Lemme know what you think of this format. https://github.com/apache/airflow/pull/11362/files#diff-5821e749f4956a14f3d0aa939985a5422ab9724bc11184244b47f620dce435f6R190
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r509601362
##########
File path: airflow/www/views.py
##########
@@ -859,8 +895,12 @@ def rendered(self):
title=title)
@expose('/get_logs_with_metadata')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_LOG),
Review comment:
Are you saying that this endpoint shouldn't check for can_read on Logs? Because this is the `get_logs_with_metadata` endpoint
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-717150609
[The Workflow run](https://github.com/apache/airflow/actions/runs/331118309) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-713840385
[The Workflow run](https://github.com/apache/airflow/actions/runs/320623917) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-715443673
> @ashb Updated the docs and Log resource names.
>
> Regarding the updated Task vs Task Instance resource names. It occurred to me when executing a task, the API handles this by creating a Task Instance object. Since there isn't an explicit "Task.execute" permission, I don't think it makes sense to get rid of the Task Instance permission altogether. It theoretically makes sense to use `Task Instances.can_create` to control execution access.
>
> Possible ways to handle this.
>
> 1. Use both `Task` and `Task Instances`. Don't worry about having multiple endpoints with both `Task.can_read` and `Task Instances.can_read` permission.
> 2. Use `Task.can_read` for almost everything. Only use `Task Instance.can_create` or `Task Instance.can_delete` for endpoints that explicitly run or delete task instances. If an endpoint currently has `Task.can_read` and `Task Instances.can_read`, consolidate this into `Task.can_read`. Consolidate reads, leave create/edit/delete in place.
> 3. Consolidate everything. Swap out all occurrences of `Task Instances.can_X` for `Task.can_X`.
>
> I'm fine with any of the three that you think make sense, but I think #1 makes the most sense, followed by #2.
>
> Thoughts?
For me, 3 makes it easy and removes the confusion from a user / admin perspective, as TaskInstance is required for a Task to run. Followed by 2
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-715055912
@ashb Updated the docs and Log resource names.
Regarding the updated Task vs Task Instance resource names. It occurred to me when executing a task, the API handles this by creating a Task Instance object. Since there isn't an explicit "Task.execute" permission, I don't think it makes sense to get rid of the Task Instance permission altogether. It theoretically makes sense to use `Task Instances.can_create` to control execution access.
Possible ways to handle this.
1. Use both `Task` and `Task Instances`. Don't worry about having multiple endpoints with both `Task.can_read` and `Task Instances.can_read` permission.
2. Use `Task.can_read` for almost everything. Only use `Task Instance.can_create` or `Task Instance.can_delete` for endpoints that explicitly run or delete task instances. If an endpoint currently has `Task.can_read` and `Task Instances.can_read`, consolidate this into `Task.can_read`. Consolidate reads, leave create/edit/delete in place.
3. Consolidate everything. Swap out all occurrences of `Task Instances.can_X` for `Task.can_X`.
I'm fine with any of the three that you think make sense, but I think #1 makes the most sense, followed by #2.
Thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r507633020
##########
File path: airflow/www/views.py
##########
@@ -394,8 +394,10 @@ def health(self):
return wwwutils.json_response(payload)
@expose('/home')
- @has_access
- def index(self): # pylint: disable=too-many-locals,too-many-statements
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ ]) # pylint: disable=too-many-locals,too-many-statements
Review comment:
Doesn't the pylint comment want to be on the next line?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-714278689
[The Workflow run](https://github.com/apache/airflow/actions/runs/321487712) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-716465144
[The Workflow run](https://github.com/apache/airflow/actions/runs/328871914) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r510291009
##########
File path: docs/security/access-control.rst
##########
@@ -134,51 +123,106 @@ Permissions (each consistent of a resource + action pair) are then added to role
**To access an endpoint, the user needs all permissions assigned to that endpoint**
-================================================================================== ====== ====================================================================================
+There are five default roles: Public, Viewer, User, Op, and Admin. Each one has the permissions of the preceeding role, as well as additional permissions.
+
+================================================================================== ====== ================================================================= ============
Stable API Permissions
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-Endpoint Method Permissions
-================================================================================== ====== ====================================================================================
-/config GET Config.can_read
-/connections GET Connection.can_read
-/connections POST Connection.can_create
-/connections/{connection_id} DELETE Connection.can_delete
-/connections/{connection_id} GET Connection.can_read
-/connections/{connection_id} PATCH Connection.can_edit
-/dagSources/{file_token} GET DagCode.can_read
-/dags GET Dag.can_read
-/dags/{dag_id} GET Dag.can_read
-/dags/{dag_id} PATCH Dag.can_edit
-/dags/{dag_id}/clearTaskInstances POST Dag.can_read, DagRun.can_read, Task.can_edit
-/dags/{dag_id}/details GET Dag.can_read
-/dags/{dag_id}/tasks GET Dag.can_read, Task.can_read
-/dags/{dag_id}/tasks/{task_id} GET Dag.can_read, Task.can_read
-/dags/{dag_id}/dagRuns GET Dag.can_read, DagRun.can_read
-/dags/{dag_id}/dagRuns POST Dag.can_read, DagRun.can_create
-/dags/{dag_id}/dagRuns/{dag_run_id} DELETE Dag.can_read, DagRun.can_delete
-/dags/{dag_id}/dagRuns/{dag_run_id} GET Dag.can_read, DagRun.can_read
-/dags/~/dagRuns/list POST Dag.can_read, DagRun.can_read
-/eventLogs GET Log.can_read
-/eventLogs/{event_log_id} GET Log.can_read
-/importErrors GET ImportError.can_read
-/importErrors/{import_error_id} GET ImportError.can_read
-/health GET None
-/version GET None
-/pools GET Pool.can_read
-/pools POST Pool.can_create
-/pools/{pool_name} DELETE Pool.can_delete
-/pools/{pool_name} GET Pool.can_read
-/pools/{pool_name} PATCH Pool.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/~/dagRuns/~/taskInstances/list POST Dag.can_read, DagRun.can_read, Task.can_read
-/variables GET Variable.can_read
-/variables POST Variable.can_create
-/variables/{variable_key} DELETE Variable.can_delete
-/variables/{variable_key} GET Variable.can_read
-/variables/{variable_key} PATCH Variable.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-================================================================================== ====== ====================================================================================
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+Endpoint Method Permissions Minimum Role
+================================================================================== ====== ================================================================= ============
+/config GET Configurations.can_read Viewer
+/connections GET Connections.can_read Op
+/connections POST Connections.can_create Op
+/connections/{connection_id} DELETE Connections.can_delete Op
+/connections/{connection_id} PATCH Connections.can_edit Op
+/connections/{connection_id} GET Connections.can_read Op
+/dagSources/{file_token} GET DAG Code.can_read Viewer
+/dags GET DAGs.can_read Viewer
+/dags/{dag_id} GET DAGs.can_read Viewer
+/dags/{dag_id} PATCH DAGs.can_edit User
+/dags/{dag_id}/clearTaskInstances POST DAGs.can_read, DAG Runs.can_read, Tasks.can_edit User
+/dags/{dag_id}/details GET DAGs.can_read Viewer
+/dags/{dag_id}/tasks GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/tasks/{task_id} GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/{dag_id}/dagRuns POST DAGs.can_read, DAG Runs.can_create User
+/dags/{dag_id}/dagRuns/{dag_run_id} DELETE DAGs.can_read, DAG Runs.can_delete User
+/dags/{dag_id}/dagRuns/{dag_run_id} GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/~/dagRuns/list POST DAGs.can_read, DAG Runs.can_read Viewer
+/eventLogs GET Logs.can_read Viewer
+/eventLogs/{event_log_id} GET Logs.can_read Viewer
+/importErrors GET ImportError.can_read Viewer
+/importErrors/{import_error_id} GET ImportError.can_read Viewer
+/health GET None Public
+/version GET None Public
+/pools GET Pool.can_read Op
+/pools POST Pool.can_create Op
+/pools/{pool_name} DELETE Pool.can_delete Op
+/pools/{pool_name} GET Pool.can_read Op
+/pools/{pool_name} PATCH Pool.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/~/dagRuns/~/taskInstances/list POST DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/variables GET Variables.can_read Op
+/variables POST Variables.can_create Op
+/variables/{variable_key} DELETE Variables.can_delete Op
+/variables/{variable_key} GET Variables.can_read Op
+/variables/{variable_key} PATCH Variables.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+================================================================================== ====== ================================================================= ============
+
+
+====================================== ======================================================================= ============
+Website Permissions
+-------------------------------------- ------------------------------------------------------------------------------------
+Action Permissions Minimum Role
+====================================== ======================================================================= ============
+Access homepage Website.can_read Viewer
+Get DAG stats Dags.can_read, DAG Runs.can_read Viewer
Review comment:
And _somewhere_ you need to mention that either you need "Dags.can_read or the can_read permission to a specific dag"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-715484267
What do we use more of in the API and website copy: Task or Task Instance?
I have a feeling it's TI, so I would say 4) - like 3, but use Task Instance everywhere
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r510290041
##########
File path: docs/security/access-control.rst
##########
@@ -134,51 +123,106 @@ Permissions (each consistent of a resource + action pair) are then added to role
**To access an endpoint, the user needs all permissions assigned to that endpoint**
-================================================================================== ====== ====================================================================================
+There are five default roles: Public, Viewer, User, Op, and Admin. Each one has the permissions of the preceeding role, as well as additional permissions.
+
+================================================================================== ====== ================================================================= ============
Stable API Permissions
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-Endpoint Method Permissions
-================================================================================== ====== ====================================================================================
-/config GET Config.can_read
-/connections GET Connection.can_read
-/connections POST Connection.can_create
-/connections/{connection_id} DELETE Connection.can_delete
-/connections/{connection_id} GET Connection.can_read
-/connections/{connection_id} PATCH Connection.can_edit
-/dagSources/{file_token} GET DagCode.can_read
-/dags GET Dag.can_read
-/dags/{dag_id} GET Dag.can_read
-/dags/{dag_id} PATCH Dag.can_edit
-/dags/{dag_id}/clearTaskInstances POST Dag.can_read, DagRun.can_read, Task.can_edit
-/dags/{dag_id}/details GET Dag.can_read
-/dags/{dag_id}/tasks GET Dag.can_read, Task.can_read
-/dags/{dag_id}/tasks/{task_id} GET Dag.can_read, Task.can_read
-/dags/{dag_id}/dagRuns GET Dag.can_read, DagRun.can_read
-/dags/{dag_id}/dagRuns POST Dag.can_read, DagRun.can_create
-/dags/{dag_id}/dagRuns/{dag_run_id} DELETE Dag.can_read, DagRun.can_delete
-/dags/{dag_id}/dagRuns/{dag_run_id} GET Dag.can_read, DagRun.can_read
-/dags/~/dagRuns/list POST Dag.can_read, DagRun.can_read
-/eventLogs GET Log.can_read
-/eventLogs/{event_log_id} GET Log.can_read
-/importErrors GET ImportError.can_read
-/importErrors/{import_error_id} GET ImportError.can_read
-/health GET None
-/version GET None
-/pools GET Pool.can_read
-/pools POST Pool.can_create
-/pools/{pool_name} DELETE Pool.can_delete
-/pools/{pool_name} GET Pool.can_read
-/pools/{pool_name} PATCH Pool.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/~/dagRuns/~/taskInstances/list POST Dag.can_read, DagRun.can_read, Task.can_read
-/variables GET Variable.can_read
-/variables POST Variable.can_create
-/variables/{variable_key} DELETE Variable.can_delete
-/variables/{variable_key} GET Variable.can_read
-/variables/{variable_key} PATCH Variable.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-================================================================================== ====== ====================================================================================
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+Endpoint Method Permissions Minimum Role
+================================================================================== ====== ================================================================= ============
+/config GET Configurations.can_read Viewer
+/connections GET Connections.can_read Op
+/connections POST Connections.can_create Op
+/connections/{connection_id} DELETE Connections.can_delete Op
+/connections/{connection_id} PATCH Connections.can_edit Op
+/connections/{connection_id} GET Connections.can_read Op
+/dagSources/{file_token} GET DAG Code.can_read Viewer
+/dags GET DAGs.can_read Viewer
+/dags/{dag_id} GET DAGs.can_read Viewer
+/dags/{dag_id} PATCH DAGs.can_edit User
+/dags/{dag_id}/clearTaskInstances POST DAGs.can_read, DAG Runs.can_read, Tasks.can_edit User
+/dags/{dag_id}/details GET DAGs.can_read Viewer
+/dags/{dag_id}/tasks GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/tasks/{task_id} GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/{dag_id}/dagRuns POST DAGs.can_read, DAG Runs.can_create User
+/dags/{dag_id}/dagRuns/{dag_run_id} DELETE DAGs.can_read, DAG Runs.can_delete User
+/dags/{dag_id}/dagRuns/{dag_run_id} GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/~/dagRuns/list POST DAGs.can_read, DAG Runs.can_read Viewer
+/eventLogs GET Logs.can_read Viewer
+/eventLogs/{event_log_id} GET Logs.can_read Viewer
+/importErrors GET ImportError.can_read Viewer
+/importErrors/{import_error_id} GET ImportError.can_read Viewer
+/health GET None Public
+/version GET None Public
+/pools GET Pool.can_read Op
+/pools POST Pool.can_create Op
+/pools/{pool_name} DELETE Pool.can_delete Op
+/pools/{pool_name} GET Pool.can_read Op
+/pools/{pool_name} PATCH Pool.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/~/dagRuns/~/taskInstances/list POST DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/variables GET Variables.can_read Op
+/variables POST Variables.can_create Op
+/variables/{variable_key} DELETE Variables.can_delete Op
+/variables/{variable_key} GET Variables.can_read Op
+/variables/{variable_key} PATCH Variables.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+================================================================================== ====== ================================================================= ============
+
+
+====================================== ======================================================================= ============
+Website Permissions
+-------------------------------------- ------------------------------------------------------------------------------------
+Action Permissions Minimum Role
+====================================== ======================================================================= ============
+Access homepage Website.can_read Viewer
Review comment:
Probably want to mention that Website.can_read is needed for all (but without listing it in every row?)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r509604020
##########
File path: airflow/www/views.py
##########
@@ -394,8 +394,10 @@ def health(self):
return wwwutils.json_response(payload)
@expose('/home')
- @has_access
- def index(self): # pylint: disable=too-many-locals,too-many-statements
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ ]) # pylint: disable=too-many-locals,too-many-statements
Review comment:
Nope. See the other comment. I'm not sure why’d , but it throws it at this level for some reason. I presume it has to do with the reassigning the view function data to the output of the wrapper function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r509295323
##########
File path: airflow/www/views.py
##########
@@ -2108,8 +2237,10 @@ def landing_times(self, session=None):
)
@expose('/paused', methods=['POST'])
- @has_dag_access(can_dag_edit=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAGS),
Review comment:
I think that's easy-ish to add later on, or at the point that granularity it needed. If we need it now, can add it though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-717150258
[The Workflow run](https://github.com/apache/airflow/actions/runs/331110609) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r510290629
##########
File path: docs/security/access-control.rst
##########
@@ -134,51 +123,106 @@ Permissions (each consistent of a resource + action pair) are then added to role
**To access an endpoint, the user needs all permissions assigned to that endpoint**
-================================================================================== ====== ====================================================================================
+There are five default roles: Public, Viewer, User, Op, and Admin. Each one has the permissions of the preceeding role, as well as additional permissions.
+
+================================================================================== ====== ================================================================= ============
Stable API Permissions
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
-Endpoint Method Permissions
-================================================================================== ====== ====================================================================================
-/config GET Config.can_read
-/connections GET Connection.can_read
-/connections POST Connection.can_create
-/connections/{connection_id} DELETE Connection.can_delete
-/connections/{connection_id} GET Connection.can_read
-/connections/{connection_id} PATCH Connection.can_edit
-/dagSources/{file_token} GET DagCode.can_read
-/dags GET Dag.can_read
-/dags/{dag_id} GET Dag.can_read
-/dags/{dag_id} PATCH Dag.can_edit
-/dags/{dag_id}/clearTaskInstances POST Dag.can_read, DagRun.can_read, Task.can_edit
-/dags/{dag_id}/details GET Dag.can_read
-/dags/{dag_id}/tasks GET Dag.can_read, Task.can_read
-/dags/{dag_id}/tasks/{task_id} GET Dag.can_read, Task.can_read
-/dags/{dag_id}/dagRuns GET Dag.can_read, DagRun.can_read
-/dags/{dag_id}/dagRuns POST Dag.can_read, DagRun.can_create
-/dags/{dag_id}/dagRuns/{dag_run_id} DELETE Dag.can_read, DagRun.can_delete
-/dags/{dag_id}/dagRuns/{dag_run_id} GET Dag.can_read, DagRun.can_read
-/dags/~/dagRuns/list POST Dag.can_read, DagRun.can_read
-/eventLogs GET Log.can_read
-/eventLogs/{event_log_id} GET Log.can_read
-/importErrors GET ImportError.can_read
-/importErrors/{import_error_id} GET ImportError.can_read
-/health GET None
-/version GET None
-/pools GET Pool.can_read
-/pools POST Pool.can_create
-/pools/{pool_name} DELETE Pool.can_delete
-/pools/{pool_name} GET Pool.can_read
-/pools/{pool_name} PATCH Pool.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET Dag.can_read, DagRun.can_read, Task.can_read
-/dags/~/dagRuns/~/taskInstances/list POST Dag.can_read, DagRun.can_read, Task.can_read
-/variables GET Variable.can_read
-/variables POST Variable.can_create
-/variables/{variable_key} DELETE Variable.can_delete
-/variables/{variable_key} GET Variable.can_read
-/variables/{variable_key} PATCH Variable.can_edit
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET Dag.can_read, DagRun.can_read, Task.can_read, XCom.can_read
-================================================================================== ====== ====================================================================================
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+Endpoint Method Permissions Minimum Role
+================================================================================== ====== ================================================================= ============
+/config GET Configurations.can_read Viewer
+/connections GET Connections.can_read Op
+/connections POST Connections.can_create Op
+/connections/{connection_id} DELETE Connections.can_delete Op
+/connections/{connection_id} PATCH Connections.can_edit Op
+/connections/{connection_id} GET Connections.can_read Op
+/dagSources/{file_token} GET DAG Code.can_read Viewer
+/dags GET DAGs.can_read Viewer
+/dags/{dag_id} GET DAGs.can_read Viewer
+/dags/{dag_id} PATCH DAGs.can_edit User
+/dags/{dag_id}/clearTaskInstances POST DAGs.can_read, DAG Runs.can_read, Tasks.can_edit User
+/dags/{dag_id}/details GET DAGs.can_read Viewer
+/dags/{dag_id}/tasks GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/tasks/{task_id} GET DAGs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/{dag_id}/dagRuns POST DAGs.can_read, DAG Runs.can_create User
+/dags/{dag_id}/dagRuns/{dag_run_id} DELETE DAGs.can_read, DAG Runs.can_delete User
+/dags/{dag_id}/dagRuns/{dag_run_id} GET DAGs.can_read, DAG Runs.can_read Viewer
+/dags/~/dagRuns/list POST DAGs.can_read, DAG Runs.can_read Viewer
+/eventLogs GET Logs.can_read Viewer
+/eventLogs/{event_log_id} GET Logs.can_read Viewer
+/importErrors GET ImportError.can_read Viewer
+/importErrors/{import_error_id} GET ImportError.can_read Viewer
+/health GET None Public
+/version GET None Public
+/pools GET Pool.can_read Op
+/pools POST Pool.can_create Op
+/pools/{pool_name} DELETE Pool.can_delete Op
+/pools/{pool_name} GET Pool.can_read Op
+/pools/{pool_name} PATCH Pool.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/links GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/logs/{task_try_number} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/dags/~/dagRuns/~/taskInstances/list POST DAGs.can_read, DAG Runs.can_read, Tasks.can_read Viewer
+/variables GET Variables.can_read Op
+/variables POST Variables.can_create Op
+/variables/{variable_key} DELETE Variables.can_delete Op
+/variables/{variable_key} GET Variables.can_read Op
+/variables/{variable_key} PATCH Variables.can_edit Op
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+/dags/{dag_id}/dagRuns/{dag_run_id}/taskInstances/{task_id}/xcomEntries/{xcom_key} GET DAGs.can_read, DAG Runs.can_read, Tasks.can_read, XComs.can_read Viewer
+================================================================================== ====== ================================================================= ============
+
+
+====================================== ======================================================================= ============
+Website Permissions
+-------------------------------------- ------------------------------------------------------------------------------------
+Action Permissions Minimum Role
+====================================== ======================================================================= ============
+Access homepage Website.can_read Viewer
+Get DAG stats Dags.can_read, DAG Runs.can_read Viewer
Review comment:
```suggestion
Get DAG stats Dags.can_read, DagRuns.can_read Viewer
```
etc.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r511853354
##########
File path: airflow/www/views.py
##########
@@ -1773,8 +1856,11 @@ def recurse_nodes(task, visited):
external_log_name=external_log_name)
@expose('/graph')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_LOG),
Review comment:
We should create a follow up PR to not make read task logs a requirement -- and if the permission is missing just don't do the `if task_log_reader.supports_external_link` bit.
##########
File path: airflow/www/views.py
##########
@@ -1607,11 +1687,14 @@ def success(self):
future, past, State.SUCCESS)
@expose('/tree')
- @has_dag_access(can_dag_read=True)
- @has_access
- @gzipped
- @action_logging
- def tree(self): # pylint: disable=too-many-locals
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_LOG),
Review comment:
We should create a follow up PR to not make read task logs a requirement -- and if the permission is missing just don't do the `if task_log_reader.supports_external_link` bit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r509294146
##########
File path: airflow/www/views.py
##########
@@ -1847,10 +1964,14 @@ class GraphForm(DateTimeWithNumRunsWithDagRunsForm):
dag_run_state=dt_nr_dr_data['dr_state'])
@expose('/duration')
- @has_dag_access(can_dag_read=True)
- @has_access
+ @auth.has_access([
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAGS),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE),
+ ])
@action_logging
- @provide_session
+ @provide_session # pylint: disable=too-many-locals
Review comment:
Surprisingly, no. Pylint gets mad if this isn't applied to the decorator, rather than the function itself. Not entirely sure why.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#discussion_r508481441
##########
File path: airflow/security/permissions.py
##########
@@ -16,25 +16,34 @@
# under the License.
# Resource Constants
-RESOURCE_DAGS = 'Dags'
+RESOURCE_ADMIN = 'Admin'
+RESOURCE_AIRFLOW = 'Airflow'
+RESOURCE_BROWSE_MENU = 'Browse'
+RESOURCE_DAGS = 'DAGs'
RESOURCE_DAG_PREFIX = 'DAG:'
-RESOURCE_CONFIG = 'Config'
-RESOURCE_CONNECTION = 'Connection'
-RESOURCE_DAG_CODE = 'DagCode'
-RESOURCE_DAG_RUN = 'DagRun'
+RESOURCE_DOCS_MENU = 'Docs'
+RESOURCE_DOCS_LINK = 'Documentation'
+RESOURCE_CONFIG = 'Configurations'
+RESOURCE_CONNECTION = 'Connections'
+RESOURCE_DAG_CODE = 'DAG Code'
+RESOURCE_DAG_RUN = "DAG Runs"
RESOURCE_IMPORT_ERROR = 'ImportError'
-RESOURCE_LOG = 'Log'
-RESOURCE_POOL = 'Pool'
-RESOURCE_TASK = 'Task'
-RESOURCE_TASK_INSTANCE = 'TaskInstance'
-RESOURCE_VARIABLE = "Variable"
+RESOURCE_JOB = "Jobs"
+RESOURCE_LOG = 'Logs'
Review comment:
I wonder if for clarity we should
```suggestion
RESOURCE_AUDIT_LOG = 'Logs'
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #11362: Use resource permissions for Airflow view access
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11362:
URL: https://github.com/apache/airflow/pull/11362#issuecomment-714642750
[The Workflow run](https://github.com/apache/airflow/actions/runs/322532627) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org