Skip to content

Commit

Permalink
fix: impose dataset ownership check on old API (apache#12491)
Browse files Browse the repository at this point in the history
* fix: impose dataset ownership check on old API

* update UPDATING.md

* partially protect the old MVC also

* prevent metric and column add and update
  • Loading branch information
dpgaspar authored and amitmiran137 committed Jan 14, 2021
1 parent de08fad commit bf0ebfa
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 2 deletions.
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ This file documents any backwards-incompatible changes in Superset and
assists people when migrating to a new version.

## Next
- [11509](https://github.com/apache/superset/pull/12491): Dataset metadata updates check user ownership, only owners or an Admin are allowed.
- Security simplification (SIP-19), the following permission domains were simplified:
- [12072](https://github.com/apache/superset/pull/12072): `Query` with `can_read`, `can_write`
- [12036](https://github.com/apache/superset/pull/12036): `Database` with `can_read`, `can_write`.
Expand Down
2 changes: 1 addition & 1 deletion superset/commands/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class DeleteFailedError(CommandException):


class ForbiddenError(CommandException):
status = 500
status = 403
message = "Action is forbidden"


Expand Down
22 changes: 22 additions & 0 deletions superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from superset.typing import FlaskResponse
from superset.utils import core as utils
from superset.views.base import (
check_ownership,
create_table_permissions,
DatasourceFilter,
DeleteMixin,
Expand Down Expand Up @@ -171,6 +172,15 @@ class TableColumnInlineView( # pylint: disable=too-many-ancestors

edit_form_extra_fields = add_form_extra_fields

def pre_add(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)

def pre_update(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)

def pre_delete(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)


class SqlMetricInlineView( # pylint: disable=too-many-ancestors
CompactCRUDMixin, SupersetModelView
Expand Down Expand Up @@ -245,6 +255,15 @@ class SqlMetricInlineView( # pylint: disable=too-many-ancestors

edit_form_extra_fields = add_form_extra_fields

def pre_add(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)

def pre_update(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)

def pre_delete(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)


class RowLevelSecurityListWidget(
SupersetListWidget
Expand Down Expand Up @@ -459,6 +478,9 @@ class TableModelView( # pylint: disable=too-many-ancestors
def pre_add(self, item: "TableModelView") -> None:
validate_sqlatable(item)

def pre_update(self, item: "TableModelView") -> None:
check_ownership(item)

def post_add( # pylint: disable=arguments-differ
self,
item: "TableModelView",
Expand Down
12 changes: 11 additions & 1 deletion superset/views/datasource.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@

from superset import db
from superset.connectors.connector_registry import ConnectorRegistry
from superset.exceptions import SupersetException
from superset.datasets.commands.exceptions import DatasetForbiddenError
from superset.exceptions import SupersetException, SupersetSecurityException
from superset.typing import FlaskResponse
from superset.views.base import check_ownership

from .base import api, BaseSupersetView, handle_api_exception, json_error_response

Expand All @@ -52,6 +54,14 @@ def save(self) -> FlaskResponse:
orm_datasource.database_id = database_id

if "owners" in datasource_dict and orm_datasource.owner_class is not None:
# Check ownership
try:
check_ownership(orm_datasource)
except SupersetSecurityException:
return json_error_response(
f"{DatasetForbiddenError.message}", DatasetForbiddenError.status
)

datasource_dict["owners"] = (
db.session.query(orm_datasource.owner_class)
.filter(orm_datasource.owner_class.id.in_(datasource_dict["owners"]))
Expand Down

0 comments on commit bf0ebfa

Please sign in to comment.