Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): unable to delete virtual dataset, wrong permission name #11019

Merged
merged 9 commits into from
Sep 29, 2020
2 changes: 1 addition & 1 deletion superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ def datasource_type(self) -> str:

@property
def database_name(self) -> str:
return self.database.name
return self.database.database_name

@classmethod
def get_datasource_by_name(
Expand Down
30 changes: 16 additions & 14 deletions superset/datasets/commands/delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,29 @@ def __init__(self, user: User, model_id: int):
def run(self) -> Model:
self.validate()
try:
dataset = DatasetDAO.delete(self._model, commit=False)

view_menu = (
security_manager.find_view_menu(self._model.get_perm())
if self._model
else None
)
if not view_menu:
logger.error(
"Could not find the data access permission for the dataset"
if view_menu:
permission_views = (
db.session.query(security_manager.permissionview_model)
.filter_by(view_menu=view_menu)
.all()
)
raise DatasetDeleteFailedError()
permission_views = (
db.session.query(security_manager.permissionview_model)
.filter_by(view_menu=view_menu)
.all()
)
dataset = DatasetDAO.delete(self._model, commit=False)

for permission_view in permission_views:
db.session.delete(permission_view)
if view_menu:
db.session.delete(view_menu)
for permission_view in permission_views:
db.session.delete(permission_view)
if view_menu:
db.session.delete(view_menu)
else:
if not view_menu:
logger.error(
"Could not find the data access permission for the dataset"
)
Copy link
Member Author

@dpgaspar dpgaspar Sep 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a dataset permission does not exist, log the error and delete anyway

db.session.commit()
except (SQLAlchemyError, DAODeleteFailedError) as ex:
logger.exception(ex)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# 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.
"""fix data access permissions for virtual datasets

Revision ID: 3fbbc6e8d654
Revises: e5ef6828ac4e
Create Date: 2020-09-24 12:04:33.827436

"""

# revision identifiers, used by Alembic.
revision = "3fbbc6e8d654"
down_revision = "e5ef6828ac4e"

import re

from alembic import op
from sqlalchemy import orm
from sqlalchemy.exc import SQLAlchemyError


def upgrade():
"""
Previous sqla_viz behaviour when creating a virtual dataset was faulty
by creating an associated data access permission with [None] on the database name.

This migration revision, fixes all faulty permissions that may exist on the db
Only fixes permissions that still have an associated dataset (fetch by id)
and replaces them with the current (correct) permission name
"""
from flask_appbuilder.security.sqla.models import ViewMenu
from superset.connectors.sqla.models import SqlaTable

bind = op.get_bind()
session = orm.Session(bind=bind)

faulty_perms = (
session.query(ViewMenu).filter(ViewMenu.name.ilike("[None].%(id:%)")).all()
)
for faulty_perm in faulty_perms:
match_ds_id = re.match("\[None\]\.\[.*\]\(id:(.*)\)", faulty_perm.name)
if match_ds_id:
try:
dataset_id = int(match_ds_id.group(1))
except ValueError:
continue
dataset = session.query(SqlaTable).get(dataset_id)
if dataset:
faulty_perm.name = dataset.get_perm()
try:
session.commit()
except SQLAlchemyError:
session.rollback()


def downgrade():
pass
12 changes: 9 additions & 3 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1820,16 +1820,22 @@ def sqllab_table_viz(self) -> FlaskResponse: # pylint: disable=no-self-use
@event_logger.log_this
def sqllab_viz(self) -> FlaskResponse: # pylint: disable=no-self-use
data = json.loads(request.form["data"])
table_name = data["datasourceName"]
database_id = data["dbId"]
try:
table_name = data["datasourceName"]
database_id = data["dbId"]
except KeyError:
return json_error_response("Missing required fields", status=400)
database = db.session.query(Database).get(database_id)
if not database:
return json_error_response("Database not found", status=400)
table = (
db.session.query(SqlaTable)
.filter_by(database_id=database_id, table_name=table_name)
.one_or_none()
)
if not table:
table = SqlaTable(table_name=table_name, owners=[g.user])
table.database_id = database_id
table.database = database
table.schema = data.get("schema")
table.template_params = data.get("templateParams")
table.is_sqllab_view = True
Expand Down