From 0f979dea064fe165dded3cabc81fe15c6cc35758 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Tue, 15 Dec 2020 08:48:00 +0000 Subject: [PATCH] feat(reports): security perm simplification (#11853) * feat: security converge report * black * fix: comment * add frontend changes and rebase * fix multiple heads --- .../src/views/CRUD/alert/AlertList.tsx | 6 +- .../40f16acf1ba7_security_converge_reports.py | 78 +++++++++++++++++++ superset/reports/api.py | 3 +- tests/reports/api_tests.py | 14 ++++ 4 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 superset/migrations/versions/40f16acf1ba7_security_converge_reports.py diff --git a/superset-frontend/src/views/CRUD/alert/AlertList.tsx b/superset-frontend/src/views/CRUD/alert/AlertList.tsx index 3ecc034cac040..e8437b1b52fc6 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertList.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertList.tsx @@ -97,9 +97,9 @@ function AlertList({ setAlertModalOpen(true); } - const canEdit = hasPerm('can_edit'); - const canDelete = hasPerm('can_delete'); - const canCreate = hasPerm('can_add'); + const canEdit = hasPerm('can_write'); + const canDelete = hasPerm('can_write'); + const canCreate = hasPerm('can_write'); const initialSort = [{ id: 'name', desc: true }]; diff --git a/superset/migrations/versions/40f16acf1ba7_security_converge_reports.py b/superset/migrations/versions/40f16acf1ba7_security_converge_reports.py new file mode 100644 index 0000000000000..227c421944a53 --- /dev/null +++ b/superset/migrations/versions/40f16acf1ba7_security_converge_reports.py @@ -0,0 +1,78 @@ +# 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. +"""security converge reports + +Revision ID: 40f16acf1ba7 +Revises: e38177dbf641 +Create Date: 2020-11-30 15:25:47.489419 + +""" + +# revision identifiers, used by Alembic. +revision = "40f16acf1ba7" +down_revision = "5daced1f0e76" + + +from alembic import op +from sqlalchemy.exc import SQLAlchemyError +from sqlalchemy.orm import Session + +from superset.migrations.shared.security_converge import ( + add_pvms, + get_reversed_new_pvms, + get_reversed_pvm_map, + migrate_roles, + Pvm, +) + +NEW_PVMS = {"ReportSchedule": ("can_read", "can_write",)} +PVM_MAP = { + Pvm("ReportSchedule", "can_list"): (Pvm("ReportSchedule", "can_read"),), + Pvm("ReportSchedule", "can_show"): (Pvm("ReportSchedule", "can_read"),), + Pvm("ReportSchedule", "can_add",): (Pvm("ReportSchedule", "can_write"),), + Pvm("ReportSchedule", "can_edit",): (Pvm("ReportSchedule", "can_write"),), + Pvm("ReportSchedule", "can_delete",): (Pvm("ReportSchedule", "can_write"),), +} + + +def upgrade(): + bind = op.get_bind() + session = Session(bind=bind) + + # Add the new permissions on the migration itself + add_pvms(session, NEW_PVMS) + migrate_roles(session, PVM_MAP) + try: + session.commit() + except SQLAlchemyError as ex: + print(f"An error occurred while upgrading permissions: {ex}") + session.rollback() + + +def downgrade(): + bind = op.get_bind() + session = Session(bind=bind) + + # Add the old permissions on the migration itself + add_pvms(session, get_reversed_new_pvms(PVM_MAP)) + migrate_roles(session, get_reversed_pvm_map(PVM_MAP)) + try: + session.commit() + except SQLAlchemyError as ex: + print(f"An error occurred while downgrading permissions: {ex}") + session.rollback() + pass diff --git a/superset/reports/api.py b/superset/reports/api.py index 6c5b8385e48c3..6be4a12f35fba 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -24,7 +24,7 @@ from marshmallow import ValidationError from superset.charts.filters import ChartFilter -from superset.constants import RouteMethod +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.dashboards.filters import DashboardFilter from superset.models.reports import ReportSchedule from superset.reports.commands.bulk_delete import BulkDeleteReportScheduleCommand @@ -60,6 +60,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): "bulk_delete", # not using RouteMethod since locally defined } class_permission_name = "ReportSchedule" + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP resource_name = "report" allow_browser_login = True diff --git a/tests/reports/api_tests.py b/tests/reports/api_tests.py index 23736ab2a0521..97aecea4b8ba5 100644 --- a/tests/reports/api_tests.py +++ b/tests/reports/api_tests.py @@ -179,6 +179,20 @@ def test_info_report_schedule(self): rv = self.get_assert_metric(uri, "info") assert rv.status_code == 200 + def test_info_security_report(self): + """ + ReportSchedule API: Test info security + """ + self.login(username="admin") + params = {"keys": ["permissions"]} + uri = f"api/v1/report/_info?q={prison.dumps(params)}" + rv = self.get_assert_metric(uri, "info") + data = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 200 + assert "can_read" in data["permissions"] + assert "can_write" in data["permissions"] + assert len(data["permissions"]) == 2 + @pytest.mark.usefixtures("create_report_schedules") def test_get_report_schedule_not_found(self): """