From 3909232fafd09ac72b49010ecdfd6ea48f06d5cf Mon Sep 17 00:00:00 2001 From: Kaxil Naik Date: Thu, 4 Feb 2021 18:37:26 +0000 Subject: [PATCH] Remove permissions to read Configurations for User and Viewer roles (#14067) Only `Admin` or `Op` roles should have permissions to view Configurations. Previously, Users with `User` or `Viewer` role were able to get/view configurations using the REST API or in the Webserver. From Airflow 2.0.1, only users with `Admin` or `Op` role would be able to get/view Configurations. --- UPDATING.md | 11 ++++ ...f_remove_can_read_permission_on_config_.py | 64 +++++++++++++++++++ airflow/www/security.py | 2 +- .../security/access-control.rst | 2 +- tests/www/test_security.py | 17 ++++- 5 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py diff --git a/UPDATING.md b/UPDATING.md index e3df0a19b9a5..6e4f9cdd25ea 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -68,6 +68,17 @@ https://developers.google.com/style/inclusive-documentation --> +### Permission to view Airflow Configurations has been removed from `User` and `Viewer` role + +Previously, Users with `User` or `Viewer` role were able to get/view configurations using +the REST API or in the Webserver. From Airflow 2.0.1, only users with `Admin` or `Op` role would be able +to get/view Configurations. + +To allow users with other roles to view configuration, add `can read on Configurations` permissions to that role. + +Note that if `[webserver] expose_config` is set to `False`, the API will throw a `403` response even if +the user has role with `can read on Configurations` permission. + ### Default `[celery] worker_concurrency` is changed to `16` The default value for `[celery] worker_concurrency` was `16` for Airflow <2.0.0. diff --git a/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py b/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py new file mode 100644 index 000000000000..6798d9312338 --- /dev/null +++ b/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py @@ -0,0 +1,64 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""Remove can_read permission on config resource for User and Viewer role + +Revision ID: 82b7c48c147f +Revises: 449b4072c2da +Create Date: 2021-02-04 12:45:58.138224 + +""" + +from airflow.security import permissions +from airflow.www.app import create_app + +# revision identifiers, used by Alembic. +revision = '82b7c48c147f' +down_revision = '449b4072c2da' +branch_labels = None +depends_on = None + + +def upgrade(): + """Remove can_read permission on config resource for User and Viewer role""" + appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder + roles_to_modify = [role for role in appbuilder.sm.get_all_roles() if role.name in ["User", "Viewer"]] + can_read_on_config_perm = appbuilder.sm.find_permission_view_menu( + permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG + ) + + for role in roles_to_modify: + if appbuilder.sm.exist_permission_on_roles( + permissions.RESOURCE_CONFIG, permissions.ACTION_CAN_READ, [role.id] + ): + appbuilder.sm.del_permission_role(role, can_read_on_config_perm) + + +def downgrade(): + """Add can_read permission on config resource for User and Viewer role""" + appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder + roles_to_modify = [role for role in appbuilder.sm.get_all_roles() if role.name in ["User", "Viewer"]] + can_read_on_config_perm = appbuilder.sm.find_permission_view_menu( + permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG + ) + + for role in roles_to_modify: + if not appbuilder.sm.exist_permission_on_roles( + permissions.RESOURCE_CONFIG, permissions.ACTION_CAN_READ, [role.id] + ): + appbuilder.sm.add_permission_role(role, can_read_on_config_perm) diff --git a/airflow/www/security.py b/airflow/www/security.py index d23cb73a1439..09af167acba0 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -52,7 +52,6 @@ class AirflowSecurityManager(SecurityManager, LoggingMixin): # pylint: disable= # [START security_viewer_perms] VIEWER_PERMISSIONS = [ - (permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), @@ -103,6 +102,7 @@ class AirflowSecurityManager(SecurityManager, LoggingMixin): # pylint: disable= # [START security_op_perms] OP_PERMISSIONS = [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_ADMIN_MENU), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_CONNECTION), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_POOL), diff --git a/docs/apache-airflow/security/access-control.rst b/docs/apache-airflow/security/access-control.rst index 5e4130867c1e..12b0d0fa1737 100644 --- a/docs/apache-airflow/security/access-control.rst +++ b/docs/apache-airflow/security/access-control.rst @@ -138,7 +138,7 @@ Stable API Permissions ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ Endpoint Method Permissions Minimum Role ================================================================================== ====== ================================================================= ============ -/config GET Configurations.can_read Viewer +/config GET Configurations.can_read Op /connections GET Connections.can_read Op /connections POST Connections.can_create Op /connections/{connection_id} DELETE Connections.can_delete Op diff --git a/tests/www/test_security.py b/tests/www/test_security.py index 745717eb3192..9179be29a124 100644 --- a/tests/www/test_security.py +++ b/tests/www/test_security.py @@ -254,7 +254,6 @@ def test_get_user_roles(self): def test_get_user_roles_for_anonymous_user(self): viewer_role_perms = { - (permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), @@ -527,3 +526,19 @@ def test_override_role_vm(self): test_security_manager = MockSecurityManager(appbuilder=self.appbuilder) assert len(test_security_manager.VIEWER_VMS) == 1 assert test_security_manager.VIEWER_VMS == {'Airflow'} + + def test_correct_roles_have_perms_to_read_config(self): + roles_to_check = self.security_manager.get_all_roles() + assert len(roles_to_check) >= 5 + for role in roles_to_check: + if role.name in ["Admin", "Op"]: + assert self.security_manager.exist_permission_on_roles( + permissions.RESOURCE_CONFIG, permissions.ACTION_CAN_READ, [role.id] + ) + else: + assert not self.security_manager.exist_permission_on_roles( + permissions.RESOURCE_CONFIG, permissions.ACTION_CAN_READ, [role.id] + ), ( + f"{role.name} should not have {permissions.ACTION_CAN_READ} " + f"on {permissions.RESOURCE_CONFIG}" + )