From 0e3f1f638c06de11668c37329ca7141fd1159293 Mon Sep 17 00:00:00 2001
From: John Bodley <4567245+john-bodley@users.noreply.github.com>
Date: Fri, 9 Jun 2023 18:54:31 -0700
Subject: [PATCH] chore: Remove deprecated ENABLE_ACCESS_REQUEST workflow
(#24266)
---
RESOURCES/STANDARD_ROLES.md | 6 -
UPDATING.md | 1 +
.../PropertiesModal/PropertiesModal.test.tsx | 7 +-
.../RowLevelSecurityList.test.tsx | 4 +-
superset/config.py | 4 -
superset/explore/commands/get.py | 22 +-
superset/initialization/__init__.py | 11 -
..._13-13_83e1abbe777f_drop_access_request.py | 50 +++
superset/models/__init__.py | 2 +-
superset/models/datasource_access_request.py | 97 -----
superset/security/manager.py | 17 -
superset/templates/email/role_extended.txt | 32 --
superset/templates/email/role_granted.txt | 36 --
.../templates/superset/request_access.html | 38 --
superset/utils/core.py | 30 +-
superset/views/__init__.py | 1 -
superset/views/access_requests.py | 59 ---
superset/views/core.py | 228 ----------
tests/integration_tests/access_tests.py | 406 ------------------
tests/integration_tests/base_tests.py | 13 -
tests/integration_tests/core_tests.py | 12 -
tests/integration_tests/datasets/api_tests.py | 2 +-
tests/integration_tests/security_tests.py | 45 --
23 files changed, 62 insertions(+), 1061 deletions(-)
create mode 100644 superset/migrations/versions/2023-06-01_13-13_83e1abbe777f_drop_access_request.py
delete mode 100644 superset/models/datasource_access_request.py
delete mode 100644 superset/templates/email/role_extended.txt
delete mode 100644 superset/templates/email/role_granted.txt
delete mode 100644 superset/templates/superset/request_access.html
delete mode 100644 superset/views/access_requests.py
diff --git a/RESOURCES/STANDARD_ROLES.md b/RESOURCES/STANDARD_ROLES.md
index b247585396..6f70620c7d 100644
--- a/RESOURCES/STANDARD_ROLES.md
+++ b/RESOURCES/STANDARD_ROLES.md
@@ -191,12 +191,6 @@
|can show on AlertLogModelView|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|can list on AlertObservationModelView|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|can show on AlertObservationModelView|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
-|can edit on AccessRequestsModelView|:heavy_check_mark:|O|O|O|
-|can list on AccessRequestsModelView|:heavy_check_mark:|O|O|O|
-|can show on AccessRequestsModelView|:heavy_check_mark:|O|O|O|
-|can add on AccessRequestsModelView|:heavy_check_mark:|O|O|O|
-|can delete on AccessRequestsModelView|:heavy_check_mark:|O|O|O|
-|muldelete on AccessRequestsModelView|:heavy_check_mark:|O|O|O|
|menu access on Row Level Security|:heavy_check_mark:|O|O|O|
|menu access on Access requests|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|menu access on Home|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
diff --git a/UPDATING.md b/UPDATING.md
index e2f45f6937..fb4851081c 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -33,6 +33,7 @@ assists people when migrating to a new version.
### Breaking Changes
+- [24266](https://github.com/apache/superset/pull/24266) Remove the `ENABLE_ACCESS_REQUEST` config parameter and the associated request/approval workflows.
- [24330](https://github.com/apache/superset/pull/24330) Removes `getUiOverrideRegistry` from `ExtensionsRegistry`.
- [23933](https://github.com/apache/superset/pull/23933) Removes the deprecated Multiple Line Charts.
- [23741](https://github.com/apache/superset/pull/23741) Migrates the TreeMap chart and removes the legacy Treemap code.
diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
index 3605131575..a453d94390 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
@@ -58,11 +58,6 @@ fetchMock.get(
value: 4,
extra: {},
},
- {
- text: 'granter',
- value: 5,
- extra: {},
- },
{
text: 'Public',
value: 2,
@@ -393,7 +388,7 @@ test('should show all roles', async () => {
const options = await findAllSelectOptions();
- expect(options).toHaveLength(6);
+ expect(options).toHaveLength(5);
expect(options[0]).toHaveTextContent('Admin');
});
diff --git a/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx b/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx
index 44b9c86bf5..a4621ed10e 100644
--- a/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx
+++ b/superset-frontend/src/pages/RowLevelSecurityList/RowLevelSecurityList.test.tsx
@@ -50,7 +50,7 @@ const mockRules = [
},
{
id: 5,
- name: 'granter',
+ name: 'Gamma',
},
],
tables: [
@@ -79,7 +79,7 @@ const mockRules = [
},
{
id: 5,
- name: 'granter',
+ name: 'Gamma',
},
],
tables: [
diff --git a/superset/config.py b/superset/config.py
index 80e7132e68..85528598cd 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1071,10 +1071,6 @@ CONFIG_PATH_ENV_VAR = "SUPERSET_CONFIG_PATH"
# example: FLASK_APP_MUTATOR = lambda x: x.before_request = f
FLASK_APP_MUTATOR = None
-# Set this to false if you don't want users to be able to request/grant
-# datasource access requests from/to other users.
-ENABLE_ACCESS_REQUEST = False
-
# smtp server configuration
EMAIL_NOTIFICATIONS = False # all the emails are sent using dryrun
SMTP_HOST = "localhost"
diff --git a/superset/explore/commands/get.py b/superset/explore/commands/get.py
index 490d198360..4543136020 100644
--- a/superset/explore/commands/get.py
+++ b/superset/explore/commands/get.py
@@ -19,11 +19,11 @@ from abc import ABC
from typing import Any, cast, Optional
import simplejson as json
-from flask import current_app, request
-from flask_babel import gettext as __, lazy_gettext as _
+from flask import request
+from flask_babel import lazy_gettext as _
from sqlalchemy.exc import SQLAlchemyError
-from superset import db, security_manager
+from superset import db
from superset.commands.base import BaseCommand
from superset.connectors.base.models import BaseDatasource
from superset.connectors.sqla.models import SqlaTable
@@ -31,7 +31,7 @@ from superset.dao.exceptions import DatasourceNotFound
from superset.datasource.dao import DatasourceDAO
from superset.exceptions import SupersetException
from superset.explore.commands.parameters import CommandParameters
-from superset.explore.exceptions import DatasetAccessDeniedError, WrongEndpointError
+from superset.explore.exceptions import WrongEndpointError
from superset.explore.form_data.commands.get import GetFormDataCommand
from superset.explore.form_data.commands.parameters import (
CommandParameters as FormDataCommandParameters,
@@ -119,20 +119,6 @@ class GetExploreCommand(BaseCommand, ABC):
except DatasourceNotFound:
pass
datasource_name = datasource.name if datasource else _("[Missing Dataset]")
-
- if datasource:
- if current_app.config["ENABLE_ACCESS_REQUEST"] and (
- not security_manager.can_access_datasource(datasource)
- ):
- message = __(
- security_manager.get_datasource_access_error_msg(datasource)
- )
- raise DatasetAccessDeniedError(
- message=message,
- datasource_type=self._datasource_type,
- datasource_id=self._datasource_id,
- )
-
viz_type = form_data.get("viz_type")
if not viz_type and datasource and datasource.default_endpoint:
raise WrongEndpointError(redirect=datasource.default_endpoint)
diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py
index bbe25f498b..fdf5492201 100644
--- a/superset/initialization/__init__.py
+++ b/superset/initialization/__init__.py
@@ -154,7 +154,6 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods
from superset.security.api import SecurityRestApi
from superset.sqllab.api import SqlLabRestApi
from superset.tags.api import TagRestApi
- from superset.views.access_requests import AccessRequestsModelView
from superset.views.alerts import AlertView, ReportView
from superset.views.all_entities import TaggedObjectsModelView, TaggedObjectView
from superset.views.annotations import AnnotationLayerView
@@ -419,16 +418,6 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods
category_label=__("Manage"),
)
- appbuilder.add_view(
- AccessRequestsModelView,
- "Access requests",
- label=__("Access requests"),
- category="Security",
- category_label=__("Security"),
- icon="fa-table",
- menu_cond=lambda: bool(self.config["ENABLE_ACCESS_REQUEST"]),
- )
-
appbuilder.add_view(
RowLevelSecurityView,
"Row Level Security",
diff --git a/superset/migrations/versions/2023-06-01_13-13_83e1abbe777f_drop_access_request.py b/superset/migrations/versions/2023-06-01_13-13_83e1abbe777f_drop_access_request.py
new file mode 100644
index 0000000000..a95650ec5a
--- /dev/null
+++ b/superset/migrations/versions/2023-06-01_13-13_83e1abbe777f_drop_access_request.py
@@ -0,0 +1,50 @@
+# 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.
+"""drop access_request
+
+Revision ID: 83e1abbe777f
+Revises: ae58e1e58e5c
+Create Date: 2023-06-01 13:13:18.147362
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "83e1abbe777f"
+down_revision = "ae58e1e58e5c"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.drop_table("access_request")
+
+
+def downgrade():
+ op.create_table(
+ "access_request",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("datasource_type", sa.String(length=200), nullable=True),
+ sa.Column("datasource_id", sa.Integer(), nullable=True),
+ sa.Column("changed_by_fk", sa.Integer(), nullable=True),
+ sa.Column("created_by_fk", sa.Integer(), nullable=True),
+ sa.ForeignKeyConstraint(["changed_by_fk"], ["ab_user.id"]),
+ sa.ForeignKeyConstraint(["created_by_fk"], ["ab_user.id"]),
+ sa.PrimaryKeyConstraint("id"),
+ )
diff --git a/superset/models/__init__.py b/superset/models/__init__.py
index a102a0fff5..067d6ae831 100644
--- a/superset/models/__init__.py
+++ b/superset/models/__init__.py
@@ -14,4 +14,4 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from . import core, datasource_access_request, dynamic_plugins, sql_lab, user_attributes
+from . import core, dynamic_plugins, sql_lab, user_attributes
diff --git a/superset/models/datasource_access_request.py b/superset/models/datasource_access_request.py
deleted file mode 100644
index 23df4cffae..0000000000
--- a/superset/models/datasource_access_request.py
+++ /dev/null
@@ -1,97 +0,0 @@
-# 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.
-from typing import Optional, TYPE_CHECKING
-
-from flask import Markup
-from flask_appbuilder import Model
-from sqlalchemy import Column, Integer, String
-
-from superset import app, db, security_manager
-from superset.models.helpers import AuditMixinNullable
-
-if TYPE_CHECKING:
- from superset.connectors.base.models import BaseDatasource
-
-config = app.config
-
-
-class DatasourceAccessRequest(Model, AuditMixinNullable):
- """ORM model for the access requests for datasources and dbs."""
-
- __tablename__ = "access_request"
- id = Column(Integer, primary_key=True)
-
- datasource_id = Column(Integer)
- datasource_type = Column(String(200))
-
- ROLES_DENYLIST = set(config["ROBOT_PERMISSION_ROLES"])
-
- @property
- def cls_model(self) -> type["BaseDatasource"]:
- # pylint: disable=import-outside-toplevel
- from superset.datasource.dao import DatasourceDAO
-
- return DatasourceDAO.sources[self.datasource_type]
-
- @property
- def username(self) -> Markup:
- return self.creator()
-
- @property
- def datasource(self) -> "BaseDatasource":
- return self.get_datasource
-
- @datasource.getter # type: ignore
- def get_datasource(self) -> "BaseDatasource":
- ds = db.session.query(self.cls_model).filter_by(id=self.datasource_id).first()
- return ds
-
- @property
- def datasource_link(self) -> Optional[Markup]:
- return self.datasource.link # pylint: disable=no-member
-
- @property
- def roles_with_datasource(self) -> str:
- action_list = ""
- perm = self.datasource.perm # pylint: disable=no-member
- pv = security_manager.find_permission_view_menu("datasource_access", perm)
- for role in pv.role:
- if role.name in self.ROLES_DENYLIST:
- continue
- href = (
- f"/superset/approve?datasource_type={self.datasource_type}&"
- f"datasource_id={self.datasource_id}&"
- f"created_by={self.created_by.username}&role_to_grant={role.name}"
- )
- link = f'Grant {role.name} Role'
- action_list = action_list + "
" + link + "
"
- return "
" + action_list + "
"
-
- @property
- def user_roles(self) -> str:
- action_list = ""
- for role in self.created_by.roles:
- href = (
- f"/superset/approve?datasource_type={self.datasource_type}&"
- f"datasource_id={self.datasource_id}&"
- f"created_by={self.created_by.username}&role_to_extend={role.name}"
- )
- link = f'Extend {role.name} Role'
- if role.name in self.ROLES_DENYLIST:
- link = f"{role.name} Role"
- action_list = action_list + "
" + link + "
"
- return "
" + action_list + "
"
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 39283d9941..8ff7f2f23c 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -164,7 +164,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
ADMIN_ONLY_VIEW_MENUS = {
"Access Requests",
- "AccessRequestsModelView",
"Action Log",
"Log",
"List Users",
@@ -195,8 +194,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
}
ADMIN_ONLY_PERMISSIONS = {
- "can_override_role_permissions",
- "can_approve",
"can_update_role",
"all_query_access",
"can_grant_guest_token",
@@ -767,7 +764,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
self.set_role("Admin", self._is_admin_pvm)
self.set_role("Alpha", self._is_alpha_pvm)
self.set_role("Gamma", self._is_gamma_pvm)
- self.set_role("granter", self._is_granter_pvm)
self.set_role("sql_lab", self._is_sql_lab_pvm)
# Configure public role
@@ -981,19 +977,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
in self.SQLLAB_EXTRA_PERMISSION_VIEWS
)
- def _is_granter_pvm( # pylint: disable=no-self-use
- self, pvm: PermissionView
- ) -> bool:
- """
- Return True if the user can grant the FAB permission/view, False
- otherwise.
-
- :param pvm: The FAB permission/view
- :returns: Whether the user can grant the FAB permission/view
- """
-
- return pvm.permission.name in {"can_override_role_permissions", "can_approve"}
-
def database_after_insert(
self,
mapper: Mapper,
diff --git a/superset/templates/email/role_extended.txt b/superset/templates/email/role_extended.txt
deleted file mode 100644
index 463fb32c9c..0000000000
--- a/superset/templates/email/role_extended.txt
+++ /dev/null
@@ -1,32 +0,0 @@
-
-Dear {{ user.username }},
-
-
- {{ granter.username }} has extended the role {{ role.name }} to include
-
- {{datasource.full_name}} and granted you access to it.
-
-
-To see all your permissions please visit your
-
- profile page.
-
-
-Regards, Superset Admin.
diff --git a/superset/templates/email/role_granted.txt b/superset/templates/email/role_granted.txt
deleted file mode 100644
index 312a049473..0000000000
--- a/superset/templates/email/role_granted.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-
-Dear {{ user.username }},
-
-
- {{ granter.username }} has granted you the role {{ role.name }}
- that gives access to the
-
- {{datasource.full_name}}
-
-
-In addition to that role grants you access to the: {{ role.permissions }}.
-
-
-To see all your permissions please visit your
-
- profile page.
-
-
-Regards, Superset Admin.
diff --git a/superset/templates/superset/request_access.html b/superset/templates/superset/request_access.html
deleted file mode 100644
index e157cb8136..0000000000
--- a/superset/templates/superset/request_access.html
+++ /dev/null
@@ -1,38 +0,0 @@
-{#
- 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.
-#}
-{% extends "superset/basic.html" %}
-{% block title %}{{ _("No Access!") }}{% endblock %}
-{% block body %}
-
- {% include "superset/flash_wrapper.html" %}
-
- {{ _("You do not have permissions to access the datasource(s): %(name)s.",
- name=datasource_names)
- }}
-
-
-
-
-
-
-{% endblock %}
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 036414ef1d..3a97c73fde 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -59,9 +59,9 @@ import pandas as pd
import sqlalchemy as sa
from cryptography.hazmat.backends import default_backend
from cryptography.x509 import Certificate, load_pem_x509_certificate
-from flask import current_app, flash, g, Markup, render_template, request
+from flask import current_app, flash, g, Markup, request
from flask_appbuilder import SQLA
-from flask_appbuilder.security.sqla.models import Role, User
+from flask_appbuilder.security.sqla.models import User
from flask_babel import gettext as __
from flask_babel.speaklater import LazyString
from pandas.api.types import infer_dtype
@@ -852,32 +852,6 @@ def pessimistic_connection_handling(some_engine: Engine) -> None:
connection.should_close_with_result = save_should_close_with_result
-def notify_user_about_perm_udate( # pylint: disable=too-many-arguments
- granter: User,
- user: User,
- role: Role,
- datasource: BaseDatasource,
- tpl_name: str,
- config: dict[str, Any],
-) -> None:
- msg = render_template(
- tpl_name, granter=granter, user=user, role=role, datasource=datasource
- )
- logger.info(msg)
- subject = __(
- "[Superset] Access to the datasource %(name)s was granted",
- name=datasource.full_name,
- )
- send_email_smtp(
- user.email,
- subject,
- msg,
- config,
- bcc=granter.email,
- dryrun=not config["EMAIL_NOTIFICATIONS"],
- )
-
-
def send_email_smtp( # pylint: disable=invalid-name,too-many-arguments,too-many-locals
to: str,
subject: str,
diff --git a/superset/views/__init__.py b/superset/views/__init__.py
index b5a21c77f0..1b8d1b8f09 100644
--- a/superset/views/__init__.py
+++ b/superset/views/__init__.py
@@ -15,7 +15,6 @@
# specific language governing permissions and limitations
# under the License.
from . import (
- access_requests,
alerts,
api,
base,
diff --git a/superset/views/access_requests.py b/superset/views/access_requests.py
deleted file mode 100644
index 063ef5e0be..0000000000
--- a/superset/views/access_requests.py
+++ /dev/null
@@ -1,59 +0,0 @@
-# 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.
-from flask import current_app as app
-from flask_appbuilder.hooks import before_request
-from flask_appbuilder.models.sqla.interface import SQLAInterface
-from flask_babel import lazy_gettext as _
-from werkzeug.exceptions import NotFound
-
-from superset.constants import RouteMethod
-from superset.views.base import DeleteMixin, SupersetModelView
-from superset.views.core import DAR
-
-
-class AccessRequestsModelView( # pylint: disable=too-many-ancestors
- SupersetModelView,
- DeleteMixin,
-):
- datamodel = SQLAInterface(DAR)
- include_route_methods = RouteMethod.CRUD_SET
- list_columns = [
- "username",
- "user_roles",
- "datasource_link",
- "roles_with_datasource",
- "created_on",
- ]
- order_columns = ["created_on"]
- base_order = ("changed_on", "desc")
- label_columns = {
- "username": _("User"),
- "user_roles": _("User Roles"),
- "database": _("Database URL"),
- "datasource_link": _("Datasource"),
- "roles_with_datasource": _("Roles to grant"),
- "created_on": _("Created On"),
- }
-
- @staticmethod
- def is_enabled() -> bool:
- return bool(app.config["ENABLE_ACCESS_REQUEST"])
-
- @before_request
- def ensure_enabled(self) -> None:
- if not self.is_enabled():
- raise NotFound()
diff --git a/superset/views/core.py b/superset/views/core.py
index 9ef2e45b30..53e088ebe6 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -39,7 +39,6 @@ from flask_appbuilder.security.sqla import models as ab_models
from flask_babel import gettext as __, lazy_gettext as _
from sqlalchemy import and_, or_
from sqlalchemy.exc import DBAPIError, NoSuchModuleError, SQLAlchemyError
-from sqlalchemy.orm.session import Session
from superset import (
app,
@@ -99,7 +98,6 @@ from superset.extensions import async_query_manager, cache_manager
from superset.jinja_context import get_template_processor
from superset.models.core import Database, FavStar
from superset.models.dashboard import Dashboard
-from superset.models.datasource_access_request import DatasourceAccessRequest
from superset.models.slice import Slice
from superset.models.sql_lab import Query, TabState
from superset.models.user_attributes import UserAttribute
@@ -175,7 +173,6 @@ from superset.viz import BaseViz
config = app.config
SQLLAB_QUERY_COST_ESTIMATE_TIMEOUT = config["SQLLAB_QUERY_COST_ESTIMATE_TIMEOUT"]
stats_logger = config["STATS_LOGGER"]
-DAR = DatasourceAccessRequest
logger = logging.getLogger(__name__)
DATABASE_KEYS = [
@@ -226,207 +223,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
)
)
- @has_access_api
- @event_logger.log_this
- @expose("/override_role_permissions/", methods=("POST",))
- @deprecated()
- def override_role_permissions(self) -> FlaskResponse:
- """Updates the role with the give datasource permissions.
-
- Permissions not in the request will be revoked. This endpoint should
- be available to admins only. Expects JSON in the format:
- {
- 'role_name': '{role_name}',
- 'database': [{
- 'datasource_type': '{table}',
- 'name': '{database_name}',
- 'schema': [{
- 'name': '{schema_name}',
- 'datasources': ['{datasource name}, {datasource name}']
- }]
- }]
- }
- """
- data = request.get_json(force=True)
- role_name = data["role_name"]
- databases = data["database"]
-
- db_ds_names = set()
- for dbs in databases:
- for schema in dbs["schema"]:
- for ds_name in schema["datasources"]:
- fullname = utils.get_datasource_full_name(
- dbs["name"], ds_name, schema=schema["name"]
- )
- db_ds_names.add(fullname)
-
- existing_datasources = SqlaTable.get_all_datasources(db.session)
- datasources = [d for d in existing_datasources if d.full_name in db_ds_names]
- role = security_manager.find_role(role_name)
- # remove all permissions
- role.permissions = []
- # grant permissions to the list of datasources
- granted_perms = []
- for datasource in datasources:
- view_menu_perm = security_manager.find_permission_view_menu(
- view_menu_name=datasource.perm, permission_name="datasource_access"
- )
- # prevent creating empty permissions
- if view_menu_perm and view_menu_perm.view_menu:
- role.permissions.append(view_menu_perm)
- granted_perms.append(view_menu_perm.view_menu.name)
- db.session.commit()
- return self.json_response(
- {"granted": granted_perms, "requested": list(db_ds_names)}, status=201
- )
-
- @has_access
- @event_logger.log_this
- @expose("/request_access/", methods=("POST",))
- @deprecated()
- def request_access(self) -> FlaskResponse:
- datasources = set()
- dashboard_id = request.args.get("dashboard_id")
- if dashboard_id:
- dash = db.session.query(Dashboard).filter_by(id=int(dashboard_id)).one()
- datasources |= dash.datasources
- datasource_id = request.args.get("datasource_id")
- datasource_type = request.args.get("datasource_type")
- if datasource_id and datasource_type:
- ds_class = DatasourceDAO.sources.get(datasource_type)
- datasource = (
- db.session.query(ds_class).filter_by(id=int(datasource_id)).one()
- )
- datasources.add(datasource)
-
- has_access_ = all(
- datasource and security_manager.can_access_datasource(datasource)
- for datasource in datasources
- )
- if has_access_:
- return redirect(f"/superset/dashboard/{dashboard_id}")
-
- if request.args.get("action") == "go":
- for datasource in datasources:
- access_request = DAR(
- datasource_id=datasource.id, datasource_type=datasource.type
- )
- db.session.add(access_request)
- db.session.commit()
- flash(__("Access was requested"), "info")
- return redirect("/")
-
- return self.render_template(
- "superset/request_access.html",
- datasources=datasources,
- datasource_names=", ".join([o.name for o in datasources]),
- )
-
- @has_access
- @event_logger.log_this
- @expose("/approve", methods=("POST",))
- @deprecated()
- def approve(self) -> FlaskResponse: # pylint: disable=too-many-locals,no-self-use
- def clean_fulfilled_requests(session: Session) -> None:
- for dar in session.query(DAR).all():
- datasource = DatasourceDAO.get_datasource(
- session, DatasourceType(dar.datasource_type), dar.datasource_id
- )
- if not datasource or security_manager.can_access_datasource(datasource):
- # Dataset does not exist anymore
- session.delete(dar)
- session.commit()
-
- datasource_type = request.args["datasource_type"]
- datasource_id = request.args["datasource_id"]
- created_by_username = request.args.get("created_by")
- role_to_grant = request.args.get("role_to_grant")
- role_to_extend = request.args.get("role_to_extend")
-
- session = db.session
- datasource = DatasourceDAO.get_datasource(
- session, DatasourceType(datasource_type), int(datasource_id)
- )
-
- if not datasource:
- flash(DATASOURCE_MISSING_ERR, "alert")
- return json_error_response(DATASOURCE_MISSING_ERR)
-
- requested_by = security_manager.find_user(username=created_by_username)
- if not requested_by:
- flash(USER_MISSING_ERR, "alert")
- return json_error_response(USER_MISSING_ERR)
-
- requests = (
- session.query(DAR)
- .filter( # pylint: disable=comparison-with-callable
- DAR.datasource_id == datasource_id,
- DAR.datasource_type == datasource_type,
- DAR.created_by_fk == requested_by.id,
- )
- .all()
- )
-
- if not requests:
- err = __("The access requests seem to have been deleted")
- flash(err, "alert")
- return json_error_response(err)
-
- # check if you can approve
- if security_manager.can_access_all_datasources() or security_manager.is_owner(
- datasource
- ):
- # can by done by admin only
- if role_to_grant:
- role = security_manager.find_role(role_to_grant)
- requested_by.roles.append(role)
- msg = __(
- "%(user)s was granted the role %(role)s that gives access "
- "to the %(datasource)s",
- user=requested_by.username,
- role=role_to_grant,
- datasource=datasource.full_name,
- )
- utils.notify_user_about_perm_udate(
- g.user,
- requested_by,
- role,
- datasource,
- "email/role_granted.txt",
- app.config,
- )
- flash(msg, "info")
-
- if role_to_extend:
- perm_view = security_manager.find_permission_view_menu(
- "email/datasource_access", datasource.perm
- )
- role = security_manager.find_role(role_to_extend)
- security_manager.add_permission_role(role, perm_view)
- msg = __(
- "Role %(r)s was extended to provide the access to "
- "the datasource %(ds)s",
- r=role_to_extend,
- ds=datasource.full_name,
- )
- utils.notify_user_about_perm_udate(
- g.user,
- requested_by,
- role,
- datasource,
- "email/role_extended.txt",
- app.config,
- )
- flash(msg, "info")
- clean_fulfilled_requests(session)
- else:
- flash(__("You have no permission to approve this request"), "danger")
- return redirect("/accessrequestsmodelview/list/")
- for request_ in requests:
- session.delete(request_)
- session.commit()
- return redirect("/accessrequestsmodelview/list/")
-
@has_access
@event_logger.log_this
@expose("/slice//")
@@ -888,21 +684,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
except DatasetNotFoundError:
pass
datasource_name = datasource.name if datasource else _("[Missing Dataset]")
-
- if datasource:
- if config["ENABLE_ACCESS_REQUEST"] and (
- not security_manager.can_access_datasource(datasource)
- ):
- flash(
- __(security_manager.get_datasource_access_error_msg(datasource)),
- "danger",
- )
- return redirect(
- "superset/request_access/?"
- f"datasource_type={datasource_type}&"
- f"datasource_id={datasource_id}&"
- )
-
viz_type = form_data.get("viz_type")
if not viz_type and datasource and datasource.default_endpoint:
return redirect(datasource.default_endpoint)
@@ -1893,15 +1674,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
):
has_access_ = True
- if has_access_ is False and config["ENABLE_ACCESS_REQUEST"]:
- flash(
- __(security_manager.get_datasource_access_error_msg(datasource)),
- "danger",
- )
- return redirect(
- f"/superset/request_access/?dashboard_id={dashboard.id}"
- )
-
if has_access_:
break
diff --git a/tests/integration_tests/access_tests.py b/tests/integration_tests/access_tests.py
index ab0100ac24..86e898462c 100644
--- a/tests/integration_tests/access_tests.py
+++ b/tests/integration_tests/access_tests.py
@@ -16,10 +16,8 @@
# under the License.
# isort:skip_file
"""Unit tests for Superset"""
-import json
import unittest
from typing import Optional
-from unittest import mock
import pytest
from flask.ctx import AppContext
@@ -42,7 +40,6 @@ from tests.integration_tests.test_app import app # isort:skip
from superset import db, security_manager
from superset.connectors.sqla.models import SqlaTable
from superset.models import core as models
-from superset.models.datasource_access_request import DatasourceAccessRequest
from superset.utils.core import get_user_id, get_username, override_user
from superset.utils.database import get_example_database
@@ -84,29 +81,6 @@ DB_ACCESS_ROLE = "db_access_role"
SCHEMA_ACCESS_ROLE = "schema_access_role"
-def create_access_request(session, ds_type, ds_name, role_name, username):
- # TODO: generalize datasource names
- if ds_type == "table":
- ds = session.query(SqlaTable).filter(SqlaTable.table_name == ds_name).first()
- else:
- # This function will only work for ds_type == "table"
- raise NotImplementedError()
- ds_perm_view = security_manager.find_permission_view_menu(
- "datasource_access", ds.perm
- )
- security_manager.add_permission_role(
- security_manager.find_role(role_name), ds_perm_view
- )
- access_request = DatasourceAccessRequest(
- datasource_id=ds.id,
- datasource_type=ds_type,
- created_by_fk=security_manager.find_user(username=username).id,
- )
- session.add(access_request)
- session.commit()
- return access_request
-
-
class TestRequestAccess(SupersetTestCase):
@classmethod
def setUpClass(cls):
@@ -139,386 +113,6 @@ class TestRequestAccess(SupersetTestCase):
db.session.commit()
db.session.close()
- def test_override_role_permissions_is_admin_only(self):
- self.logout()
- self.login("alpha")
- response = self.client.post(
- "/superset/override_role_permissions/",
- data=json.dumps(ROLE_TABLES_PERM_DATA),
- content_type="application/json",
- follow_redirects=True,
- )
- self.assertNotEqual(405, response.status_code)
-
- @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
- def test_override_role_permissions_1_table(self):
- database = get_example_database()
- with database.get_sqla_engine_with_context() as engine:
- schema = inspect(engine).default_schema_name
-
- perm_data = ROLE_TABLES_PERM_DATA.copy()
- perm_data["database"][0]["schema"][0]["name"] = schema
-
- response = self.client.post(
- "/superset/override_role_permissions/",
- data=json.dumps(perm_data),
- content_type="application/json",
- )
- self.assertEqual(201, response.status_code)
-
- updated_override_me = security_manager.find_role("override_me")
- self.assertEqual(1, len(updated_override_me.permissions))
- birth_names = self.get_table(name="birth_names")
- self.assertEqual(
- birth_names.perm, updated_override_me.permissions[0].view_menu.name
- )
- self.assertEqual(
- "datasource_access", updated_override_me.permissions[0].permission.name
- )
-
- @pytest.mark.usefixtures(
- "load_energy_table_with_slice", "load_birth_names_dashboard_with_slices"
- )
- def test_override_role_permissions_drops_absent_perms(self):
- database = get_example_database()
- with database.get_sqla_engine_with_context() as engine:
- schema = inspect(engine).default_schema_name
-
- override_me = security_manager.find_role("override_me")
- override_me.permissions.append(
- security_manager.find_permission_view_menu(
- view_menu_name=self.get_table(name="energy_usage").perm,
- permission_name="datasource_access",
- )
- )
- db.session.flush()
-
- perm_data = ROLE_TABLES_PERM_DATA.copy()
- perm_data["database"][0]["schema"][0]["name"] = schema
-
- response = self.client.post(
- "/superset/override_role_permissions/",
- data=json.dumps(perm_data),
- content_type="application/json",
- )
- self.assertEqual(201, response.status_code)
- updated_override_me = security_manager.find_role("override_me")
- self.assertEqual(1, len(updated_override_me.permissions))
- birth_names = self.get_table(name="birth_names")
- self.assertEqual(
- birth_names.perm, updated_override_me.permissions[0].view_menu.name
- )
- self.assertEqual(
- "datasource_access", updated_override_me.permissions[0].permission.name
- )
-
- def test_clean_requests_after_role_extend(self):
- session = db.session
-
- # Case 1. Gamma and gamma2 requested test_role1 on energy_usage access
- # Gamma already has role test_role1
- # Extend test_role1 with energy_usage access for gamma2
- # Check if access request for gamma at energy_usage was deleted
-
- # gamma2 and gamma request table_role on energy usage
- if app.config["ENABLE_ACCESS_REQUEST"]:
- access_request1 = create_access_request(
- session, "table", "random_time_series", TEST_ROLE_1, "gamma2"
- )
- ds_1_id = access_request1.datasource_id
- create_access_request(
- session, "table", "random_time_series", TEST_ROLE_1, "gamma"
- )
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- self.assertTrue(access_requests)
- # gamma gets test_role1
- self.get_resp(
- GRANT_ROLE_REQUEST.format("table", ds_1_id, "gamma", TEST_ROLE_1)
- )
- # extend test_role1 with access on energy usage
- self.client.get(
- EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_1)
- )
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- self.assertFalse(access_requests)
-
- gamma_user = security_manager.find_user(username="gamma")
- gamma_user.roles.remove(security_manager.find_role("test_role1"))
-
- @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
- def test_clean_requests_after_alpha_grant(self):
- session = db.session
-
- # Case 2. Two access requests from gamma and gamma2
- # Gamma becomes alpha, gamma2 gets granted
- # Check if request by gamma has been deleted
-
- access_request1 = create_access_request(
- session, "table", "birth_names", TEST_ROLE_1, "gamma"
- )
- create_access_request(session, "table", "birth_names", TEST_ROLE_2, "gamma2")
- ds_1_id = access_request1.datasource_id
- # gamma becomes alpha
- alpha_role = security_manager.find_role("Alpha")
- gamma_user = security_manager.find_user(username="gamma")
- gamma_user.roles.append(alpha_role)
- session.commit()
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- self.assertTrue(access_requests)
- self.client.post(
- EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_2)
- )
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- self.assertFalse(access_requests)
-
- gamma_user = security_manager.find_user(username="gamma")
- gamma_user.roles.remove(security_manager.find_role("Alpha"))
- session.commit()
-
- @pytest.mark.usefixtures("load_energy_table_with_slice")
- def test_clean_requests_after_db_grant(self):
- session = db.session
-
- # Case 3. Two access requests from gamma and gamma2
- # Gamma gets database access, gamma2 access request granted
- # Check if request by gamma has been deleted
-
- gamma_user = security_manager.find_user(username="gamma")
- access_request1 = create_access_request(
- session, "table", "energy_usage", TEST_ROLE_1, "gamma"
- )
- create_access_request(session, "table", "energy_usage", TEST_ROLE_2, "gamma2")
- ds_1_id = access_request1.datasource_id
- # gamma gets granted database access
- database = session.query(models.Database).first()
-
- security_manager.add_permission_view_menu("database_access", database.perm)
- ds_perm_view = security_manager.find_permission_view_menu(
- "database_access", database.perm
- )
- security_manager.add_permission_role(
- security_manager.find_role(DB_ACCESS_ROLE), ds_perm_view
- )
- gamma_user.roles.append(security_manager.find_role(DB_ACCESS_ROLE))
- session.commit()
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- self.assertTrue(access_requests)
- # gamma2 request gets fulfilled
- self.client.post(
- EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_2)
- )
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
-
- self.assertFalse(access_requests)
- gamma_user = security_manager.find_user(username="gamma")
- gamma_user.roles.remove(security_manager.find_role(DB_ACCESS_ROLE))
- session.commit()
-
- @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
- def test_clean_requests_after_schema_grant(self):
- session = db.session
-
- # Case 4. Two access requests from gamma and gamma2
- # Gamma gets schema access, gamma2 access request granted
- # Check if request by gamma has been deleted
-
- gamma_user = security_manager.find_user(username="gamma")
- access_request1 = create_access_request(
- session, "table", "wb_health_population", TEST_ROLE_1, "gamma"
- )
- create_access_request(
- session, "table", "wb_health_population", TEST_ROLE_2, "gamma2"
- )
- ds_1_id = access_request1.datasource_id
- ds = (
- session.query(SqlaTable)
- .filter_by(table_name="wb_health_population")
- .first()
- )
- original_schema = ds.schema
-
- ds.schema = "temp_schema"
- security_manager.add_permission_view_menu("schema_access", ds.schema_perm)
- schema_perm_view = security_manager.find_permission_view_menu(
- "schema_access", ds.schema_perm
- )
- security_manager.add_permission_role(
- security_manager.find_role(SCHEMA_ACCESS_ROLE), schema_perm_view
- )
- gamma_user.roles.append(security_manager.find_role(SCHEMA_ACCESS_ROLE))
- session.commit()
- # gamma2 request gets fulfilled
- self.client.post(
- EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_2)
- )
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- self.assertFalse(access_requests)
- gamma_user = security_manager.find_user(username="gamma")
- gamma_user.roles.remove(security_manager.find_role(SCHEMA_ACCESS_ROLE))
-
- ds.schema = original_schema
- session.commit()
-
- @mock.patch("superset.utils.core.send_mime_email")
- def test_approve(self, mock_send_mime):
- if app.config["ENABLE_ACCESS_REQUEST"]:
- session = db.session
- TEST_ROLE_NAME = "table_role"
- security_manager.add_role(TEST_ROLE_NAME)
-
- # Case 1. Grant new role to the user.
-
- access_request1 = create_access_request(
- session, "table", "unicode_test", TEST_ROLE_NAME, "gamma"
- )
- ds_1_id = access_request1.datasource_id
- self.get_resp(
- GRANT_ROLE_REQUEST.format("table", ds_1_id, "gamma", TEST_ROLE_NAME)
- )
- # Test email content.
- self.assertTrue(mock_send_mime.called)
- call_args = mock_send_mime.call_args[0]
- self.assertEqual(
- [
- security_manager.find_user(username="gamma").email,
- security_manager.find_user(username="admin").email,
- ],
- call_args[1],
- )
- self.assertEqual(
- "[Superset] Access to the datasource {} was granted".format(
- self.get_table_by_id(ds_1_id).full_name
- ),
- call_args[2]["Subject"],
- )
- self.assertIn(TEST_ROLE_NAME, call_args[2].as_string())
- self.assertIn("unicode_test", call_args[2].as_string())
-
- access_requests = self.get_access_requests("gamma", "table", ds_1_id)
- # request was removed
- self.assertFalse(access_requests)
- # user was granted table_role
- user_roles = [r.name for r in security_manager.find_user("gamma").roles]
- self.assertIn(TEST_ROLE_NAME, user_roles)
-
- # Case 2. Extend the role to have access to the table
-
- access_request2 = create_access_request(
- session, "table", "energy_usage", TEST_ROLE_NAME, "gamma"
- )
- ds_2_id = access_request2.datasource_id
- energy_usage_perm = access_request2.datasource.perm
-
- self.client.get(
- EXTEND_ROLE_REQUEST.format(
- "table", access_request2.datasource_id, "gamma", TEST_ROLE_NAME
- )
- )
- access_requests = self.get_access_requests("gamma", "table", ds_2_id)
-
- # Test email content.
- self.assertTrue(mock_send_mime.called)
- call_args = mock_send_mime.call_args[0]
- self.assertEqual(
- [
- security_manager.find_user(username="gamma").email,
- security_manager.find_user(username="admin").email,
- ],
- call_args[1],
- )
- self.assertEqual(
- "[Superset] Access to the datasource {} was granted".format(
- self.get_table_by_id(ds_2_id).full_name
- ),
- call_args[2]["Subject"],
- )
- self.assertIn(TEST_ROLE_NAME, call_args[2].as_string())
- self.assertIn("energy_usage", call_args[2].as_string())
-
- # request was removed
- self.assertFalse(access_requests)
- # table_role was extended to grant access to the energy_usage table/
- perm_view = security_manager.find_permission_view_menu(
- "datasource_access", energy_usage_perm
- )
- TEST_ROLE = security_manager.find_role(TEST_ROLE_NAME)
- self.assertIn(perm_view, TEST_ROLE.permissions)
-
- def test_request_access(self):
- if app.config["ENABLE_ACCESS_REQUEST"]:
- session = db.session
- self.logout()
- self.login(username="gamma")
- gamma_user = security_manager.find_user(username="gamma")
- security_manager.add_role("dummy_role")
- gamma_user.roles.append(security_manager.find_role("dummy_role"))
- session.commit()
-
- ACCESS_REQUEST = (
- "/superset/request_access?"
- "datasource_type={}&"
- "datasource_id={}&"
- "action={}&"
- )
- ROLE_GRANT_LINK = (
- 'Grant {} Role'
- )
-
- # Request table access, there are no roles have this table.
-
- table1 = (
- session.query(SqlaTable)
- .filter_by(table_name="random_time_series")
- .first()
- )
- table_1_id = table1.id
-
- # request access to the table
- resp = self.get_resp(ACCESS_REQUEST.format("table", table_1_id, "go"))
- assert "Access was requested" in resp
- access_request1 = self.get_access_requests("gamma", "table", table_1_id)
- assert access_request1 is not None
-
- # Request access, roles exist that contains the table.
- # add table to the existing roles
- table3 = (
- session.query(SqlaTable).filter_by(table_name="energy_usage").first()
- )
- table_3_id = table3.id
- table3_perm = table3.perm
-
- security_manager.add_role("energy_usage_role")
- alpha_role = security_manager.find_role("Alpha")
- security_manager.add_permission_role(
- alpha_role,
- security_manager.find_permission_view_menu(
- "datasource_access", table3_perm
- ),
- )
- security_manager.add_permission_role(
- security_manager.find_role("energy_usage_role"),
- security_manager.find_permission_view_menu(
- "datasource_access", table3_perm
- ),
- )
- session.commit()
-
- self.get_resp(ACCESS_REQUEST.format("table", table_3_id, "go"))
- access_request3 = self.get_access_requests("gamma", "table", table_3_id)
- approve_link_3 = ROLE_GRANT_LINK.format(
- "table", table_3_id, "gamma", "energy_usage_role", "energy_usage_role"
- )
- self.assertEqual(
- access_request3.roles_with_datasource,
- f"