Skip to content

Commit

Permalink
fix: shut off unneeded endpoints (apache#8960)
Browse files Browse the repository at this point in the history 
* fix: shut off all uneeded endpoints

We recently added a new feature to FAB allowing to whitelist the needed
endpoints in ModelView and ModelRestApi.

First, we set our base wrapper class to an empty set, forcing each
class inheriting from it to explicitely turn on the endpoints that
Superset intends to use.

Second, we go ModelView by ModelView to whitelist the actual endpoints
used in the app.

Notes:
* as a result a large set of [unneeded] permissions should be cleaned up
* outside of the "private" use of endpoints in the app, people that have
  been using endpoints in their environment for other purposes may
  experience loss of functionality

* Tweaking

* Reduce the amount of endpoints using white lists

* Fix, included needed endpoints for dashboard and druid

* Drying things up

* fixes

* limiting more endpoints

* Read only on some FAB model views

* fixing some tests

* fixes

* Fixing more tests

* Addressing comments

* Drying up route_methods

* further drying

Co-authored-by: Daniel Vaz Gaspar <danielvazgaspar@gmail.com>
  • Loading branch information
@mistercrunch @dpgaspar
mistercrunch and dpgaspar authored Jan 23, 2020
1 parent 22699a2 commit 315a11d
Show file tree
Hide file tree
Showing 25 changed files with 173 additions and 187 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*.pyc
*.sqllite
*.swp
.bento*
.cache-loader
.coverage
.DS_Store
Expand Down
2 changes: 2 additions & 0 deletions .rat-excludes
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ apache_superset.egg-info
.*json
.*csv
# Generated doc files
env/*
docs/_build/*
docs/_modules/*
_build/*
_static/*
.buildinfo
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ croniter==0.3.31
cryptography==2.8
decorator==4.4.1 # via retry
defusedxml==0.6.0 # via python3-openid
flask-appbuilder==2.2.1
flask-appbuilder==2.2.2rc3
flask-babel==0.12.2 # via flask-appbuilder
flask-caching==1.8.0
flask-compress==1.4.0
Expand Down
50 changes: 19 additions & 31 deletions superset/app.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,15 @@ def init_views(self) -> None:
CssTemplateAsyncModelView,
)
from superset.views.chart.api import ChartRestApi
from superset.views.chart.views import SliceModelView, SliceAsync, SliceAddView
from superset.views.chart.views import SliceModelView, SliceAsync
from superset.views.dashboard.api import DashboardRestApi
from superset.views.dashboard.views import (
DashboardModelView,
Dashboard,
DashboardAddView,
DashboardModelViewAsync,
)
from superset.views.database.api import DatabaseRestApi
from superset.views.database.views import (
DatabaseView,
DatabaseTablesAsync,
CsvToDatabaseView,
DatabaseAsync,
)
from superset.views.database.views import DatabaseView, CsvToDatabaseView
from superset.views.datasource import Datasource
from superset.views.log.api import LogRestApi
from superset.views.log.views import LogModelView
Expand Down Expand Up @@ -218,6 +212,16 @@ def init_views(self) -> None:
category_label=__("Sources"),
category_icon="fa-database",
)
appbuilder.add_link(
"Tables",
label=__("Tables"),
href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
icon="fa-table",
category="Sources",
category_label=__("Sources"),
category_icon="fa-table",
)
appbuilder.add_separator("Sources")
appbuilder.add_view(
SliceModelView,
"Charts",
Expand Down Expand Up @@ -259,16 +263,12 @@ def init_views(self) -> None:
appbuilder.add_view_no_menu(CssTemplateAsyncModelView)
appbuilder.add_view_no_menu(CsvToDatabaseView)
appbuilder.add_view_no_menu(Dashboard)
appbuilder.add_view_no_menu(DashboardAddView)
appbuilder.add_view_no_menu(DashboardModelViewAsync)
appbuilder.add_view_no_menu(DatabaseAsync)
appbuilder.add_view_no_menu(DatabaseTablesAsync)
appbuilder.add_view_no_menu(Datasource)
appbuilder.add_view_no_menu(KV)
appbuilder.add_view_no_menu(R)
appbuilder.add_view_no_menu(SavedQueryView)
appbuilder.add_view_no_menu(SavedQueryViewApi)
appbuilder.add_view_no_menu(SliceAddView)
appbuilder.add_view_no_menu(SliceAsync)
appbuilder.add_view_no_menu(SqlLab)
appbuilder.add_view_no_menu(SqlMetricInlineView)
Expand All @@ -282,12 +282,6 @@ def init_views(self) -> None:
#
# Add links
#
appbuilder.add_link(
__("Saved Queries"),
href="/sqllab/my_queries/",
icon="fa-save",
category="SQL Lab",
)
appbuilder.add_link(
"Import Dashboards",
label=__("Import Dashboards"),
Expand All @@ -306,6 +300,12 @@ def init_views(self) -> None:
category="SQL Lab",
category_label=__("SQL Lab"),
)
appbuilder.add_link(
__("Saved Queries"),
href="/sqllab/my_queries/",
icon="fa-save",
category="SQL Lab",
)
appbuilder.add_link(
"Query Search",
label=_("Query Search"),
Expand All @@ -324,23 +324,11 @@ def init_views(self) -> None:
category_label=__("Sources"),
category_icon="fa-wrench",
)
appbuilder.add_link(
"Tables",
label=__("Tables"),
href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
icon="fa-table",
category="Sources",
category_label=__("Sources"),
category_icon="fa-table",
)

#
# Conditionally setup log views
#
if (
not self.config["FAB_ADD_SECURITY_VIEWS"] is False
or self.config["SUPERSET_LOG_VIEW"] is False
):
if self.config["FAB_ADD_SECURITY_VIEWS"] and self.config["SUPERSET_LOG_VIEW"]:
appbuilder.add_api(LogRestApi)
appbuilder.add_view(
LogModelView,
Expand Down
2 changes: 1 addition & 1 deletion superset/assets/src/dashboard/actions/sliceEntities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export function fetchAllSlices(userId) {
dispatch(fetchAllSlicesStarted());

return SupersetClient.get({
endpoint: `/sliceaddview/api/read?_flt_0_created_by=${userId}`,
endpoint: `/sliceasync/api/read?_flt_0_created_by=${userId}`,
})
.then(({ json }) => {
const slices = {};
Expand Down
7 changes: 5 additions & 2 deletions superset/connectors/druid/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from superset import app, appbuilder, db, security_manager
from superset.connectors.base.views import DatasourceModelView
from superset.connectors.connector_registry import ConnectorRegistry
from superset.constants import RouteMethod
from superset.utils import core as utils
from superset.views.base import (
BaseSupersetView,
Expand All @@ -47,6 +48,7 @@

class DruidColumnInlineView(CompactCRUDMixin, SupersetModelView):
datamodel = SQLAInterface(models.DruidColumn)
include_route_methods = RouteMethod.RELATED_VIEW_SET

list_title = _("Columns")
show_title = _("Show Druid Column")
Expand Down Expand Up @@ -133,6 +135,7 @@ def post_add(self, col):

class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView):
datamodel = SQLAInterface(models.DruidMetric)
include_route_methods = RouteMethod.RELATED_VIEW_SET

list_title = _("Metrics")
show_title = _("Show Druid Metric")
Expand Down Expand Up @@ -185,7 +188,7 @@ class DruidMetricInlineView(CompactCRUDMixin, SupersetModelView):

class DruidClusterModelView(SupersetModelView, DeleteMixin, YamlExportMixin):
datamodel = SQLAInterface(models.DruidCluster)

include_route_methods = RouteMethod.CRUD_SET
list_title = _("Druid Clusters")
show_title = _("Show Druid Cluster")
add_title = _("Add Druid Cluster")
Expand Down Expand Up @@ -247,7 +250,7 @@ def _delete(self, pk):

class DruidDatasourceModelView(DatasourceModelView, DeleteMixin, YamlExportMixin):
datamodel = SQLAInterface(models.DruidDatasource)

include_route_methods = RouteMethod.CRUD_SET
list_title = _("Druid Datasources")
show_title = _("Show Druid Datasource")
add_title = _("Add Druid Datasource")
Expand Down
5 changes: 5 additions & 0 deletions superset/connectors/sqla/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

from superset import appbuilder, db, security_manager
from superset.connectors.base.views import DatasourceModelView
from superset.constants import RouteMethod
from superset.utils import core as utils
from superset.views.base import (
DatasourceFilter,
Expand All @@ -48,6 +49,8 @@

class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):
datamodel = SQLAInterface(models.TableColumn)
# TODO TODO, review need for this on related_views
include_route_methods = RouteMethod.RELATED_VIEW_SET

list_title = _("Columns")
show_title = _("Show Column")
Expand Down Expand Up @@ -164,6 +167,7 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):

class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView):
datamodel = SQLAInterface(models.SqlMetric)
include_route_methods = RouteMethod.RELATED_VIEW_SET

list_title = _("Metrics")
show_title = _("Show Metric")
Expand Down Expand Up @@ -223,6 +227,7 @@ class SqlMetricInlineView(CompactCRUDMixin, SupersetModelView):

class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin):
datamodel = SQLAInterface(models.SqlaTable)
include_route_methods = RouteMethod.CRUD_SET

list_title = _("Tables")
show_title = _("Show Table")
Expand Down
40 changes: 40 additions & 0 deletions superset/constants.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,43 @@

# string to use when None values *need* to be converted to/from strings
NULL_STRING = "<NULL>"


class RouteMethod: # pylint: disable=too-few-public-methods
"""
Route methods are a FAB concept around ModelView and RestModelView
classes in FAB. Derivatives can define `include_route_method` and
`exclude_route_methods` class attribute as a set of methods that
will or won't get exposed.
This class is a collection of static constants to reference common
route methods, namely the ones defined in the base classes in FAB
"""

# ModelView specific
ACTION = "action"
ACTION_POST = "action_post"
ADD = "add"
API_CREATE = "api_create"
API_DELETE = "api_delete"
API_GET = "api_get"
API_READ = "api_read"
API_UPDATE = "api_update"
DELETE = "delete"
DOWNLOAD = "download"
EDIT = "edit"
LIST = "list"
SHOW = "show"

# RestModelView specific
EXPORT = "export"
GET = "get"
GET_LIST = "get_list"
POST = "post"
PUT = "put"
RELATED = "related"

# Commonly used sets
CRUD_SET = {ADD, LIST, EDIT, DELETE, ACTION_POST}
RELATED_VIEW_SET = {ADD, LIST, EDIT, DELETE}
REST_MODEL_VIEW_CRUD_SET = {DELETE, GET, GET_LIST, POST, PUT}
10 changes: 10 additions & 0 deletions superset/security/manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
PermissionViewModelView,
RoleModelView,
UserModelView,
ViewMenuModelView,
)
from flask_appbuilder.widgets import ListWidget
from sqlalchemy import or_
Expand All @@ -40,6 +41,7 @@

from superset import sql_parse
from superset.connectors.connector_registry import ConnectorRegistry
from superset.constants import RouteMethod
from superset.exceptions import SupersetSecurityException
from superset.utils.core import DatasourceName

Expand Down Expand Up @@ -76,8 +78,16 @@ def __init__(self, **kwargs):
PermissionViewModelView.list_widget = SupersetSecurityListWidget
PermissionModelView.list_widget = SupersetSecurityListWidget

# Limiting routes on FAB model views
UserModelView.include_route_methods = RouteMethod.CRUD_SET | {"userinfo"}
RoleModelView.include_route_methods = RouteMethod.CRUD_SET
PermissionViewModelView.include_route_methods = {RouteMethod.LIST}
PermissionModelView.include_route_methods = {RouteMethod.LIST}
ViewMenuModelView.include_route_methods = {RouteMethod.LIST}


class SupersetSecurityManager(SecurityManager):
userstatschartview = None
READ_ONLY_MODEL_VIEWS = {"DatabaseAsync", "DatabaseView", "DruidClusterModelView"}

USER_MODEL_VIEWS = {
Expand Down
3 changes: 3 additions & 0 deletions superset/views/annotations.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from flask_babel import lazy_gettext as _
from wtforms.validators import StopValidation

from superset.constants import RouteMethod
from superset.models.annotations import Annotation, AnnotationLayer

from .base import DeleteMixin, SupersetModelView
Expand Down Expand Up @@ -45,6 +46,7 @@ class AnnotationModelView(
SupersetModelView, DeleteMixin
): # pylint: disable=too-many-ancestors
datamodel = SQLAInterface(Annotation)
include_route_methods = RouteMethod.CRUD_SET

list_title = _("List Annotation")
show_title = _("Show Annotation")
Expand Down Expand Up @@ -93,6 +95,7 @@ class AnnotationLayerModelView(
SupersetModelView, DeleteMixin
): # pylint: disable=too-many-ancestors
datamodel = SQLAInterface(AnnotationLayer)
include_route_methods = RouteMethod.CRUD_SET

list_title = _("List Annotation Layer")
show_title = _("Show Annotation Layer")
Expand Down
11 changes: 11 additions & 0 deletions superset/views/base_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ class BaseSupersetModelRestApi(ModelRestApi):
"""

logger = logging.getLogger(__name__)
method_permission_name = {
"get_list": "list",
"get": "show",
"export": "mulexport",
"post": "add",
"put": "edit",
"delete": "delete",
"bulk_delete": "delete",
"info": "list",
"related": "list",
}

order_rel_fields: Dict[str, Tuple[str, str]] = {}
"""
Expand Down
9 changes: 0 additions & 9 deletions superset/views/chart/api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,6 @@ class ChartRestApi(SliceMixin, BaseOwnedModelRestApi):
allow_browser_login = True

class_permission_name = "SliceModelView"
method_permission_name = {
"get_list": "list",
"get": "show",
"post": "add",
"put": "edit",
"delete": "delete",
"info": "list",
"related": "list",
}
show_columns = [
"slice_name",
"description",
Expand Down
Loading

0 comments on commit 315a11d

Please sign in to comment.