QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-04-19 08:04:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

[dashboard] Fix, prevent delete and update on dashes not owned (#8911)

Browse Source
This commit is contained in:
Daniel Vaz Gaspar
2020-01-06 15:29:39 +00:00
committed by GitHub
parent 478e445a5a
commit 2726f21cbc
5 changed files with 120 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
superset/views/base.py
View File

@@ -155,6 +155,26 @@ def handle_api_exception(f):
return functools.update_wrapper(wraps, f)
def check_ownership_and_item_exists(f):
"""
A Decorator that checks if an object exists and is owned by the current user
"""
def wraps(self, pk): # pylint: disable=invalid-name
item = self.datamodel.get(
pk, self._base_filters # pylint: disable=protected-access
)
if not item:
return self.response_404()
try:
check_ownership(item)
except SupersetSecurityException as e:
return self.response(403, message=str(e))
return f(self, item)
return functools.update_wrapper(wraps, f)
def get_datasource_exist_error_msg(full_name):
return __("Datasource %(name)s already exists", name=full_name)

111
superset/views/dashboard/api.py
View File

@@ -24,11 +24,15 @@ from marshmallow import fields, post_load, pre_load, Schema, ValidationError
from marshmallow.validate import Length
from sqlalchemy.exc import SQLAlchemyError
import superset.models.core as models
from superset import appbuilder
from superset.exceptions import SupersetException
from superset.models.dashboard import Dashboard
from superset.utils import core as utils
from superset.views.base import BaseSupersetModelRestApi, BaseSupersetSchema
from superset.views.base import (
BaseSupersetModelRestApi,
BaseSupersetSchema,
check_ownership_and_item_exists,
)
from .mixin import DashboardMixin
@@ -67,7 +71,7 @@ def validate_slug_uniqueness(value):
# slug is not required but must be unique
if value:
item = (
current_app.appbuilder.get_session.query(models.Dashboard.id)
current_app.appbuilder.get_session.query(Dashboard.id)
.filter_by(slug=value)
.one_or_none()
)
@@ -123,7 +127,7 @@ class DashboardPostSchema(BaseDashboardSchema):
@post_load
def make_object(self, data): # pylint: disable=no-self-use
instance = models.Dashboard()
instance = Dashboard()
self.set_owners(instance, data["owners"])
for field in data:
if field == "owners":
@@ -157,7 +161,7 @@ class DashboardPutSchema(BaseDashboardSchema):
class DashboardRestApi(DashboardMixin, BaseSupersetModelRestApi):
datamodel = SQLAInterface(models.Dashboard)
datamodel = SQLAInterface(Dashboard)
resource_name = "dashboard"
allow_browser_login = True
@@ -207,6 +211,62 @@ class DashboardRestApi(DashboardMixin, BaseSupersetModelRestApi):
}
filter_rel_fields_field = {"owners": "first_name", "slices": "slice_name"}
@expose("/<pk>", methods=["PUT"])
@protect()
@check_ownership_and_item_exists
@safe
def put(self, item): # pylint: disable=arguments-differ
"""Changes a dashboard
---
put:
parameters:
- in: path
schema:
type: integer
name: pk
requestBody:
description: Model schema
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/{{self.__class__.__name__}}.put'
responses:
200:
description: Item changed
content:
application/json:
schema:
type: object
properties:
result:
$ref: '#/components/schemas/{{self.__class__.__name__}}.put'
400:
$ref: '#/components/responses/400'
401:
$ref: '#/components/responses/401'
403:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
if not request.is_json:
self.response_400(message="Request is not JSON")
item = self.edit_model_schema.load(request.json, instance=item)
if item.errors:
return self.response_422(message=item.errors)
try:
self.datamodel.edit(item.data, raise_exception=True)
return self.response(
200, result=self.edit_model_schema.dump(item.data, many=False).data
)
except SQLAlchemyError as e:
return self.response_422(message=str(e))
@expose("/", methods=["POST"])
@protect()
@safe
@@ -258,39 +318,33 @@ class DashboardRestApi(DashboardMixin, BaseSupersetModelRestApi):
except SQLAlchemyError as e:
return self.response_422(message=str(e))
@expose("/<pk>", methods=["PUT"])
@expose("/<pk>", methods=["DELETE"])
@protect()
@check_ownership_and_item_exists
@safe
def put(self, pk):
"""Changes a dashboard
def delete(self, item): # pylint: disable=arguments-differ
"""Delete Dashboard
---
put:
delete:
parameters:
- in: path
schema:
type: integer
name: pk
requestBody:
description: Model schema
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/{{self.__class__.__name__}}.put'
responses:
200:
description: Item changed
description: Dashboard delete
content:
application/json:
schema:
type: object
properties:
result:
$ref: '#/components/schemas/{{self.__class__.__name__}}.put'
400:
$ref: '#/components/responses/400'
message:
type: string
401:
$ref: '#/components/responses/401'
403:
$ref: '#/components/responses/401'
404:
$ref: '#/components/responses/404'
422:
@@ -298,20 +352,9 @@ class DashboardRestApi(DashboardMixin, BaseSupersetModelRestApi):
500:
$ref: '#/components/responses/500'
"""
if not request.is_json:
self.response_400(message="Request is not JSON")
item = self.datamodel.get(pk, self._base_filters)
if not item:
return self.response_404()
item = self.edit_model_schema.load(request.json, instance=item)
if item.errors:
return self.response_422(message=item.errors)
try:
self.datamodel.edit(item.data, raise_exception=True)
return self.response(
200, result=self.edit_model_schema.dump(item.data, many=False).data
)
self.datamodel.delete(item, raise_exception=True)
return self.response(200, message="OK")
except SQLAlchemyError as e:
return self.response_422(message=str(e))

4
superset/views/dashboard/mixin.py
View File

@@ -16,6 +16,7 @@
# under the License.
from flask_babel import lazy_gettext as _
from ..base import check_ownership
from .filters import DashboardFilter
@@ -80,3 +81,6 @@ class DashboardMixin: # pylint: disable=too-few-public-methods
"json_metadata": _("JSON Metadata"),
"table_names": _("Underlying Tables"),
}
def pre_delete(self, item): # pylint: disable=no-self-use
check_ownership(item)

3
superset/views/dashboard/views.py
View File

@@ -84,9 +84,6 @@ class DashboardModelView(
check_ownership(item)
self.pre_add(item)
def pre_delete(self, item): # pylint: disable=no-self-use
check_ownership(item)
class Dashboard(BaseSupersetView):
"""The base views for Superset!"""

24
tests/dashboard_api_tests.py
View File

@@ -23,6 +23,7 @@ from flask_appbuilder.security.sqla import models as ab_models
from superset import db, security_manager
from superset.models import core as models
from superset.models.slice import Slice
from .base_tests import SupersetTestCase
@@ -36,12 +37,14 @@ class DashboardApiTests(SupersetTestCase):
dashboard_title: str,
slug: str,
owners: List[int],
slices: List[Slice] = None,
position_json: str = "",
css: str = "",
json_metadata: str = "",
published: bool = False,
) -> models.Dashboard:
obj_owners = list()
slices = slices or []
for owner in owners:
user = db.session.query(security_manager.user_model).get(owner)
obj_owners.append(user)
@@ -52,6 +55,7 @@ class DashboardApiTests(SupersetTestCase):
position_json=position_json,
css=css,
json_metadata=json_metadata,
slices=slices,
published=published,
)
db.session.add(dashboard)
@@ -113,11 +117,16 @@ class DashboardApiTests(SupersetTestCase):
user_alpha2 = self.create_user(
"alpha2", "password", "Alpha", email="alpha2@superset.org"
)
dashboard = self.insert_dashboard("title", "slug1", [user_alpha1.id])
existing_slice = (
db.session.query(Slice).filter_by(slice_name="Girl Name Cloud").first()
)
dashboard = self.insert_dashboard(
"title", "slug1", [user_alpha1.id], slices=[existing_slice], published=True
)
self.login(username="alpha2", password="password")
uri = f"api/v1/dashboard/{dashboard.id}"
rv = self.client.delete(uri)
self.assertEqual(rv.status_code, 404)
self.assertEqual(rv.status_code, 403)
db.session.delete(dashboard)
db.session.delete(user_alpha1)
db.session.delete(user_alpha2)
@@ -333,7 +342,7 @@ class DashboardApiTests(SupersetTestCase):
def test_update_dashboard_not_owned(self):
"""
Dashboard API: Test update dashboard not owner
Dashboard API: Test update dashboard not owned
"""
user_alpha1 = self.create_user(
"alpha1", "password", "Alpha", email="alpha1@superset.org"
@@ -341,12 +350,17 @@ class DashboardApiTests(SupersetTestCase):
user_alpha2 = self.create_user(
"alpha2", "password", "Alpha", email="alpha2@superset.org"
)
dashboard = self.insert_dashboard("title", "slug1", [user_alpha1.id])
existing_slice = (
db.session.query(Slice).filter_by(slice_name="Girl Name Cloud").first()
)
dashboard = self.insert_dashboard(
"title", "slug1", [user_alpha1.id], slices=[existing_slice], published=True
)
self.login(username="alpha2", password="password")
dashboard_data = {"dashboard_title": "title1_changed", "slug": "slug1 changed"}
uri = f"api/v1/dashboard/{dashboard.id}"
rv = self.client.put(uri, json=dashboard_data)
self.assertEqual(rv.status_code, 404)
self.assertEqual(rv.status_code, 403)
db.session.delete(dashboard)
db.session.delete(user_alpha1)
db.session.delete(user_alpha2)