This is an automated email from the ASF dual-hosted git repository.
dpgaspar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9c8b65d feat(annotations): security permissions simplification
(#12014)
9c8b65d is described below
commit 9c8b65d03fd7676f2600be89d19000b9756b4e2c
Author: Kasia Kucharczyk <[email protected]>
AuthorDate: Wed Dec 16 10:08:06 2020 +0100
feat(annotations): security permissions simplification (#12014)
* Changed security permissions for annotations and annotation layers
* Updated permissions in annotation layers list
* Created test for retrieving premissions info. Updated uris from f-strings
to strings
* Updated annotations in security_tests and added annotations to
NEW_SECURITY_CONVERGE_VIEWS
* Added migration for annotations security converge
* Updated current revision after rebase master
* Updated migration name to annotations security converge
* Updated annotations permissions names in AnnotationLayersList and updated
test since 'can_write' has wider permissions
* Updated annotations migration to current
---
.../annotationlayers/AnnotationLayersList_spec.jsx | 4 +-
.../CRUD/annotationlayers/AnnotationLayersList.tsx | 6 +-
superset/annotation_layers/annotations/api.py | 6 +-
superset/annotation_layers/api.py | 6 +-
.../c25cb2c78727_security_converge_annotations.py | 84 ++++++++++++++++++++++
superset/views/annotations.py | 9 ++-
tests/annotation_layers/api_tests.py | 20 +++++-
tests/security_tests.py | 4 +-
8 files changed, 124 insertions(+), 15 deletions(-)
diff --git
a/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
b/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
index 7a48470..fa6addd 100644
---
a/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
+++
b/superset-frontend/spec/javascripts/views/CRUD/annotationlayers/AnnotationLayersList_spec.jsx
@@ -61,7 +61,7 @@ const mockUser = {
};
fetchMock.get(layersInfoEndpoint, {
- permissions: ['can_delete'],
+ permissions: ['can_write'],
});
fetchMock.get(layersEndpoint, {
result: mocklayers,
@@ -156,7 +156,7 @@ describe('AnnotationLayersList', () => {
});
it('shows/hides bulk actions when bulk actions is clicked', async () => {
- const button = wrapper.find(Button).at(0);
+ const button = wrapper.find(Button).at(1);
act(() => {
button.props().onClick();
});
diff --git
a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx
b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx
index d066d4d..92bf9bd 100644
--- a/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx
+++ b/superset-frontend/src/views/CRUD/annotationlayers/AnnotationLayersList.tsx
@@ -114,9 +114,9 @@ function AnnotationLayersList({
);
};
- const canCreate = hasPerm('can_add');
- const canEdit = hasPerm('can_edit');
- const canDelete = hasPerm('can_delete');
+ const canCreate = hasPerm('can_write');
+ const canEdit = hasPerm('can_write');
+ const canDelete = hasPerm('can_write');
function handleAnnotationLayerEdit(layer: AnnotationLayerObject | null) {
setCurrentAnnotationLayer(layer);
diff --git a/superset/annotation_layers/annotations/api.py
b/superset/annotation_layers/annotations/api.py
index d7241b6..1c63f69 100644
--- a/superset/annotation_layers/annotations/api.py
+++ b/superset/annotation_layers/annotations/api.py
@@ -52,7 +52,7 @@ from superset.annotation_layers.annotations.schemas import (
openapi_spec_methods_override,
)
from superset.annotation_layers.commands.exceptions import
AnnotationLayerNotFoundError
-from superset.constants import RouteMethod
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.models.annotations import Annotation
from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
@@ -65,7 +65,9 @@ class AnnotationRestApi(BaseSupersetModelRestApi):
include_route_methods = RouteMethod.REST_MODEL_VIEW_CRUD_SET | {
"bulk_delete", # not using RouteMethod since locally defined
}
- class_permission_name = "AnnotationLayerModelView"
+ class_permission_name = "Annotation"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
resource_name = "annotation_layer"
allow_browser_login = True
diff --git a/superset/annotation_layers/api.py
b/superset/annotation_layers/api.py
index c608e30..b47bf87 100644
--- a/superset/annotation_layers/api.py
+++ b/superset/annotation_layers/api.py
@@ -46,7 +46,7 @@ from superset.annotation_layers.schemas import (
get_delete_ids_schema,
openapi_spec_methods_override,
)
-from superset.constants import RouteMethod
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.extensions import event_logger
from superset.models.annotations import AnnotationLayer
from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
@@ -61,7 +61,9 @@ class AnnotationLayerRestApi(BaseSupersetModelRestApi):
RouteMethod.RELATED,
"bulk_delete", # not using RouteMethod since locally defined
}
- class_permission_name = "AnnotationLayerModelView"
+ class_permission_name = "Annotation"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
resource_name = "annotation_layer"
allow_browser_login = True
diff --git
a/superset/migrations/versions/c25cb2c78727_security_converge_annotations.py
b/superset/migrations/versions/c25cb2c78727_security_converge_annotations.py
new file mode 100644
index 0000000..33099dd
--- /dev/null
+++ b/superset/migrations/versions/c25cb2c78727_security_converge_annotations.py
@@ -0,0 +1,84 @@
+# 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 annotations
+
+Revision ID: c25cb2c78727
+Revises: ccb74baaa89b
+Create Date: 2020-12-11 17:02:21.213046
+
+"""
+
+from alembic import op
+from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.orm import Session
+
+# revision identifiers, used by Alembic.
+from superset.migrations.shared.security_converge import (
+ add_pvms,
+ get_reversed_new_pvms,
+ get_reversed_pvm_map,
+ migrate_roles,
+ Pvm,
+)
+
+revision = "c25cb2c78727"
+down_revision = "ccb74baaa89b"
+
+
+NEW_PVMS = {"Annotation": ("can_read", "can_write",)}
+PVM_MAP = {
+ Pvm("AnnotationLayerModelView", "can_delete"): (Pvm("Annotation",
"can_write"),),
+ Pvm("AnnotationLayerModelView", "can_list"): (Pvm("Annotation",
"can_read"),),
+ Pvm("AnnotationLayerModelView", "can_show",): (Pvm("Annotation",
"can_read"),),
+ Pvm("AnnotationLayerModelView", "can_add",): (Pvm("Annotation",
"can_write"),),
+ Pvm("AnnotationLayerModelView", "can_edit",): (Pvm("Annotation",
"can_write"),),
+ Pvm("AnnotationModelView", "can_annotation",): (Pvm("Annotation",
"can_read"),),
+ Pvm("AnnotationModelView", "can_show",): (Pvm("Annotation", "can_read"),),
+ Pvm("AnnotationModelView", "can_add",): (Pvm("Annotation", "can_write"),),
+ Pvm("AnnotationModelView", "can_delete",): (Pvm("Annotation",
"can_write"),),
+ Pvm("AnnotationModelView", "can_edit",): (Pvm("Annotation", "can_write"),),
+ Pvm("AnnotationModelView", "can_list",): (Pvm("Annotation", "can_read"),),
+}
+
+
+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 annotation 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 annotation permissions:
{ex}")
+ session.rollback()
+ pass
diff --git a/superset/views/annotations.py b/superset/views/annotations.py
index 03cc260..345fd2c 100644
--- a/superset/views/annotations.py
+++ b/superset/views/annotations.py
@@ -24,7 +24,7 @@ from flask_babel import lazy_gettext as _
from wtforms.validators import StopValidation
from superset import is_feature_enabled
-from superset.constants import RouteMethod
+from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.models.annotations import Annotation, AnnotationLayer
from superset.typing import FlaskResponse
from superset.views.base import SupersetModelView
@@ -54,6 +54,9 @@ class AnnotationModelView(
datamodel = SQLAInterface(Annotation)
include_route_methods = RouteMethod.CRUD_SET | {"annotation"}
+ class_permission_name = "Annotation"
+ method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
list_title = _("Annotations")
show_title = _("Show Annotation")
add_title = _("Add Annotation")
@@ -109,6 +112,10 @@ class AnnotationLayerModelView(SupersetModelView): #
pylint: disable=too-many-a
datamodel = SQLAInterface(AnnotationLayer)
include_route_methods = RouteMethod.CRUD_SET | {RouteMethod.API_READ}
related_views = [AnnotationModelView]
+
+ class_permission_name = "Annotation"
+ method_permission_name = MODEL_VIEW_RW_METHOD_PERMISSION_MAP
+
list_title = _("Annotation Layers")
show_title = _("Show Annotation Layer")
add_title = _("Add Annotation Layer")
diff --git a/tests/annotation_layers/api_tests.py
b/tests/annotation_layers/api_tests.py
index 0ee361b..b536243 100644
--- a/tests/annotation_layers/api_tests.py
+++ b/tests/annotation_layers/api_tests.py
@@ -75,10 +75,24 @@ class TestAnnotationLayerApi(SupersetTestCase):
Annotation API: Test info
"""
self.login(username="admin")
- uri = f"api/v1/annotation_layer/_info"
+ uri = "api/v1/annotation_layer/_info"
rv = self.get_assert_metric(uri, "info")
assert rv.status_code == 200
+ def test_info_security_query(self):
+ """
+ Annotation API: Test info security
+ """
+ self.login(username="admin")
+ params = {"keys": ["permissions"]}
+ uri = f"api/v1/annotation_layer/_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_annotation_layers")
def test_get_annotation_layer_not_found(self):
"""
@@ -96,7 +110,7 @@ class TestAnnotationLayerApi(SupersetTestCase):
Annotation Api: Test get list annotation layers
"""
self.login(username="admin")
- uri = f"api/v1/annotation_layer/"
+ uri = "api/v1/annotation_layer/"
rv = self.get_assert_metric(uri, "get_list")
expected_fields = [
@@ -120,7 +134,7 @@ class TestAnnotationLayerApi(SupersetTestCase):
Annotation Api: Test sorting on get list annotation layers
"""
self.login(username="admin")
- uri = f"api/v1/annotation_layer/"
+ uri = "api/v1/annotation_layer/"
order_columns = [
"name",
diff --git a/tests/security_tests.py b/tests/security_tests.py
index 4ef6392..2836b48 100644
--- a/tests/security_tests.py
+++ b/tests/security_tests.py
@@ -48,7 +48,7 @@ from .dashboard_utils import (
from .fixtures.energy_dashboard import load_energy_table_with_slice
from .fixtures.unicode_dashboard import load_unicode_dashboard_with_slice
-NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery", "Chart")
+NEW_SECURITY_CONVERGE_VIEWS = ("CssTemplate", "SavedQuery", "Chart",
"Annotation")
def get_perm_tuples(role_name):
@@ -672,7 +672,7 @@ class TestRolePermission(SupersetTestCase):
self.assert_can_menu("Dashboards", perm_set)
def assert_can_alpha(self, perm_set):
- self.assert_can_all("AnnotationLayerModelView", perm_set)
+ self.assert_can_all("Annotation", perm_set)
self.assert_can_all("CssTemplate", perm_set)
self.assert_can_all("TableModelView", perm_set)
self.assert_can_read("QueryView", perm_set)
