Skip to content

Commit

Permalink
fix: add config to disable dataset ownership on the old api (apache#1…
Browse files Browse the repository at this point in the history
…3051)

* fix: add config to disable dataset ownership on the old api

* fix CI docker build

* fix logic

* add deprecation comment on the config
  • Loading branch information
dpgaspar authored and amitmiran137 committed Feb 14, 2021
1 parent 230b1fb commit f1622fa
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 17 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/docker_build_push.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ SHA=$(git rev-parse HEAD)
REPO_NAME="apache/superset"

if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then
REFSPEC=$(echo "${GITHUB_HEAD_REF}" | sed 's/[^a-zA-Z0-9]/-/' | head -c 40)
REFSPEC=$(echo "${GITHUB_HEAD_REF}" | sed 's/[^a-zA-Z0-9]/-/g' | head -c 40)
PR_NUM=$(echo "${GITHUB_REF}" | sed 's:refs/pull/::' | sed 's:/merge::')
LATEST_TAG="pr-${PR_NUM}"
elif [[ "${GITHUB_EVENT_NAME}" == "release" ]]; then
REFSPEC=$(echo "${GITHUB_REF}" | sed 's:refs/tags/::' | head -c 40)
LATEST_TAG="${REFSPEC}"
else
REFSPEC=$(echo "${GITHUB_REF}" | sed 's:refs/heads/::' | sed 's/[^a-zA-Z0-9]/-/' | head -c 40)
REFSPEC=$(echo "${GITHUB_REF}" | sed 's:refs/heads/::' | sed 's/[^a-zA-Z0-9]/-/g' | head -c 40)
LATEST_TAG="${REFSPEC}"
fi

Expand Down
6 changes: 6 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,12 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
'class="alert-link">here</a>.'
)

# Turn this key to False to disable ownership check on the old dataset MVC and
# datasource API /datasource/save.
#
# Warning: This config key is deprecated and will be removed in version 2.0.0"
OLD_API_CHECK_DATASET_OWNERSHIP = True

# SQLA table mutator, every time we fetch the metadata for a certain table
# (superset.connectors.sqla.models.SqlaTable), we call this hook
# to allow mutating the object with this callback.
Expand Down
48 changes: 41 additions & 7 deletions superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,25 @@ 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)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item.table)

def pre_update(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item.table)

def pre_delete(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item.table)


class SqlMetricInlineView( # pylint: disable=too-many-ancestors
Expand Down Expand Up @@ -256,13 +268,25 @@ 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)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item.table)

def pre_update(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item.table)

def pre_delete(self, item: "models.SqlMetric") -> None:
check_ownership(item.table)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item.table)


class RowLevelSecurityListWidget(
Expand Down Expand Up @@ -476,10 +500,17 @@ class TableModelView( # pylint: disable=too-many-ancestors
}

def pre_add(self, item: "TableModelView") -> None:
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
validate_sqlatable(item)

def pre_update(self, item: "TableModelView") -> None:
check_ownership(item)
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
check_ownership(item)

def post_add( # pylint: disable=arguments-differ
self,
Expand Down Expand Up @@ -522,6 +553,9 @@ def edit(self, pk: str) -> FlaskResponse:
def refresh( # pylint: disable=no-self-use, too-many-branches
self, tables: Union["TableModelView", List["TableModelView"]]
) -> FlaskResponse:
logger.warning(
"This endpoint is deprecated and will be removed in version 2.0.0"
)
if not isinstance(tables, list):
tables = [tables]

Expand Down
6 changes: 3 additions & 3 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ def schemas( # pylint: disable=no-self-use
self, db_id: int, force_refresh: str = "false"
) -> FlaskResponse:
logger.warning(
"This API endpoint is deprecated and will be removed in version 1.0.0"
"This API endpoint is deprecated and will be removed in version 2.0.0"
)
db_id = int(db_id)
database = db.session.query(Database).get(db_id)
Expand Down Expand Up @@ -1753,7 +1753,7 @@ def publish( # pylint: disable=no-self-use
) -> FlaskResponse:
"""Gets and toggles published status on dashboards"""
logger.warning(
"This API endpoint is deprecated and will be removed in version 1.0.0"
"This API endpoint is deprecated and will be removed in version 2.0.0"
)
session = db.session()
Role = ab_models.Role
Expand Down Expand Up @@ -2067,7 +2067,7 @@ def select_star(
) -> FlaskResponse:
logging.warning(
"%s.select_star "
"This API endpoint is deprecated and will be removed in version 1.0.0",
"This API endpoint is deprecated and will be removed in version 2.0.0",
self.__class__.__name__,
)
stats_logger.incr(f"{self.__class__.__name__}.select_star.init")
Expand Down
11 changes: 6 additions & 5 deletions superset/views/datasource.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from flask_appbuilder.security.decorators import has_access_api
from flask_babel import _

from superset import db
from superset import app, db
from superset.connectors.connector_registry import ConnectorRegistry
from superset.datasets.commands.exceptions import DatasetForbiddenError
from superset.exceptions import SupersetException, SupersetSecurityException
Expand Down Expand Up @@ -55,10 +55,11 @@ def save(self) -> FlaskResponse:

if "owners" in datasource_dict and orm_datasource.owner_class is not None:
# Check ownership
try:
check_ownership(orm_datasource)
except SupersetSecurityException:
raise DatasetForbiddenError()
if app.config["OLD_API_CHECK_DATASET_OWNERSHIP"]:
try:
check_ownership(orm_datasource)
except SupersetSecurityException:
raise DatasetForbiddenError()

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

0 comments on commit f1622fa

Please sign in to comment.