iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(rbac): show objects accessible by database access perm (#23118)

This commit is contained in:
Ville Brofeldt 2023-02-24 10:45:16 +02:00 committed by GitHub
parent 9a4839f45c
commit 89576f8a87
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 116 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
UPDATING.md
View File

@ -44,6 +44,8 @@ assists people when migrating to a new version.
### Other
- [23118](https://github.com/apache/superset/pull/23118): Previously the "database access on <database>" permission granted access to all datasets on the underlying database, but they didn't show up on the list views. Now all dashboards, charts and datasets that are accessible via this permission will also show up on their respective list views.
## 2.0.1
- [21895](https://github.com/apache/superset/pull/21895): Markdown components had their security increased by adhering to the same sanitization process enforced by Github. This means that some HTML elements found in markdowns are not allowed anymore due to the security risks they impose. If you're deploying Superset in a trusted environment and wish to use some of the blocked elements, then you can use the HTML_SANITIZATION_SCHEMA_EXTENSIONS configuration to extend the default sanitization schema. There's also the option to disable HTML sanitization using the HTML_SANITIZATION configuration but we do not recommend this approach because of the security risks. Given the provided configurations, we don't view the improved sanitization as a breaking change but as a security patch.

26
superset/charts/filters.py
View File

@ -18,13 +18,15 @@ from typing import Any
from flask_babel import lazy_gettext as _
from sqlalchemy import and_, or_
from sqlalchemy.orm import aliased
from sqlalchemy.orm.query import Query
from superset import db, security_manager
from superset import security_manager
from superset.connectors.sqla import models
from superset.connectors.sqla.models import SqlaTable
from superset.models.slice import Slice
from superset.utils.core import get_user_id
from superset.utils.filters import get_dataset_access_filters
from superset.views.base import BaseFilter
from superset.views.base_api import BaseFavoriteFilter, BaseTagFilter
@ -87,23 +89,13 @@ class ChartFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():
return query
perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
owner_ids_query = (
db.session.query(models.SqlaTable.id)
.join(models.SqlaTable.owners)
.filter(
security_manager.user_model.id
== security_manager.user_model.get_user_id()
)
)
return query.filter(
or_(
self.model.perm.in_(perms),
self.model.schema_perm.in_(schema_perms),
models.SqlaTable.id.in_(owner_ids_query),
)
table_alias = aliased(SqlaTable)
query = query.join(table_alias, self.model.datasource_id == table_alias.id)
query = query.join(
models.Database, table_alias.database_id == models.Database.id
)
return query.filter(get_dataset_access_filters(self.model))
class ChartHasCreatedByFilter(BaseFilter): # pylint: disable=too-few-public-methods

14
superset/dashboards/filters.py
View File

@ -24,12 +24,14 @@ from sqlalchemy import and_, or_
from sqlalchemy.orm.query import Query
from superset import db, is_feature_enabled, security_manager
from superset.models.core import FavStar
from superset.connectors.sqla.models import SqlaTable
from superset.models.core import Database, FavStar
from superset.models.dashboard import Dashboard
from superset.models.embedded_dashboard import EmbeddedDashboard
from superset.models.slice import Slice
from superset.security.guest_token import GuestTokenResourceType, GuestUser
from superset.utils.core import get_user_id
from superset.utils.filters import get_dataset_access_filters
from superset.views.base import BaseFilter
from superset.views.base_api import BaseFavoriteFilter, BaseTagFilter
@ -111,9 +113,6 @@ class DashboardAccessFilter(BaseFilter): # pylint: disable=too-few-public-metho
if security_manager.is_admin():
return query
datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
is_rbac_disabled_filter = []
dashboard_has_roles = Dashboard.roles.any()
if is_feature_enabled("DASHBOARD_RBAC"):
@ -122,13 +121,14 @@ class DashboardAccessFilter(BaseFilter): # pylint: disable=too-few-public-metho
datasource_perm_query = (
db.session.query(Dashboard.id)
.join(Dashboard.slices, isouter=True)
.join(SqlaTable, Slice.datasource_id == SqlaTable.id)
.join(Database, SqlaTable.database_id == Database.id)
.filter(
and_(
Dashboard.published.is_(True),
*is_rbac_disabled_filter,
or_(
Slice.perm.in_(datasource_perms),
Slice.schema_perm.in_(schema_perms),
get_dataset_access_filters(
Slice,
security_manager.can_access_all_datasources(),
),
)

25
superset/security/manager.py
View File

@ -84,6 +84,7 @@ from superset.utils.core import (
get_user_id,
RowLevelSecurityFilterType,
)
from superset.utils.filters import get_dataset_access_filters
from superset.utils.urls import get_url_host
if TYPE_CHECKING:
@ -98,6 +99,8 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
DATABASE_PERM_REGEX = re.compile(r"^\[.+\]\.\(id\:(?P<id>\d+)\)$")
class DatabaseAndSchema(NamedTuple):
database: str
@ -525,8 +528,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
:returns: The list of datasources
"""
user_perms = self.user_view_menu_names("datasource_access")
schema_perms = self.user_view_menu_names("schema_access")
user_datasources = set()
# pylint: disable=import-outside-toplevel
@ -534,12 +535,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
user_datasources.update(
self.get_session.query(SqlaTable)
.filter(
or_(
SqlaTable.perm.in_(user_perms),
SqlaTable.schema_perm.in_(schema_perms),
)
)
.filter(get_dataset_access_filters(SqlaTable))
.all()
)
@ -604,6 +600,19 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
return {s.name for s in view_menu_names}
return set()
def get_accessible_databases(self) -> List[int]:
"""
Return the list of databases accessible by the user.
:return: The list of accessible Databases
"""
perms = self.user_view_menu_names("database_access")
return [
int(match.group("id"))
for perm in perms
if (match := DATABASE_PERM_REGEX.match(perm))
]
def get_schemas_accessible_by_user(
self, database: "Database", schemas: List[str], hierarchical: bool = True
) -> List[str]:

41
superset/utils/filters.py Normal file
View File

@ -0,0 +1,41 @@
# 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 Any, Type
from flask_appbuilder import Model
from sqlalchemy import or_
from sqlalchemy.sql.elements import BooleanClauseList
def get_dataset_access_filters(
base_model: Type[Model],
*args: Any,
) -> BooleanClauseList:
# pylint: disable=import-outside-toplevel
from superset import security_manager
from superset.connectors.sqla.models import Database
database_ids = security_manager.get_accessible_databases()
perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
return or_(
Database.id.in_(database_ids),
base_model.perm.in_(perms),
base_model.schema_perm.in_(schema_perms),
*args,
)

21
superset/views/base.py
View File

@ -46,7 +46,7 @@ from flask_jwt_extended.exceptions import NoAuthorizationError
from flask_wtf.csrf import CSRFError
from flask_wtf.form import FlaskForm
from pkg_resources import resource_filename
from sqlalchemy import exc, or_
from sqlalchemy import exc
from sqlalchemy.orm import Query
from werkzeug.exceptions import HTTPException
from wtforms import Form
@ -78,7 +78,7 @@ from superset.reports.models import ReportRecipientType
from superset.superset_typing import FlaskResponse
from superset.translations.utils import get_language_pack
from superset.utils import core as utils
from superset.utils.core import get_user_id
from superset.utils.filters import get_dataset_access_filters
from .utils import bootstrap_user_data
@ -670,20 +670,11 @@ class DatasourceFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():
return query
datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
owner_ids_query = (
db.session.query(models.SqlaTable.id)
.join(models.SqlaTable.owners)
.filter(security_manager.user_model.id == get_user_id())
)
return query.filter(
or_(
self.model.perm.in_(datasource_perms),
self.model.schema_perm.in_(schema_perms),
models.SqlaTable.id.in_(owner_ids_query),
)
query = query.join(
models.Database,
models.Database.id == self.model.database_id,
)
return query.filter(get_dataset_access_filters(self.model))
class CsvResponse(Response):

9
superset/views/chart/filters.py
View File

@ -16,10 +16,10 @@
# under the License.
from typing import Any
from sqlalchemy import or_
from sqlalchemy.orm.query import Query
from superset import security_manager
from superset.utils.filters import get_dataset_access_filters
from superset.views.base import BaseFilter
@ -27,8 +27,5 @@ class SliceFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():
return query
perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
return query.filter(
or_(self.model.perm.in_(perms), self.model.schema_perm.in_(schema_perms))
)
return query.filter(get_dataset_access_filters(self.model))

41
tests/integration_tests/datasets/api_tests.py
View File

@ -239,28 +239,47 @@ class TestDatasetApi(SupersetTestCase):
response = json.loads(rv.data.decode("utf-8"))
assert response["result"] == []
def test_get_dataset_list_gamma_owned(self):
def test_get_dataset_list_gamma_has_database_access(self):
"""
Dataset API: Test get dataset list owned by gamma
Dataset API: Test get dataset list with database access
"""
if backend() == "sqlite":
return
main_db = get_main_database()
owned_dataset = self.insert_dataset(
"ab_user", [self.get_user("gamma").id], main_db
)
self.login(username="gamma")
# create new dataset
main_db = get_main_database()
dataset = self.insert_dataset("ab_user", [], main_db)
# make sure dataset is not visible due to missing perms
uri = "api/v1/dataset/"
rv = self.get_assert_metric(uri, "get_list")
assert rv.status_code == 200
response = json.loads(rv.data.decode("utf-8"))
assert response["count"] == 1
assert response["result"][0]["table_name"] == "ab_user"
assert response["count"] == 0
db.session.delete(owned_dataset)
# give database access to main db
main_db_pvm = security_manager.find_permission_view_menu(
"database_access", main_db.perm
)
gamma_role = security_manager.find_role("Gamma")
gamma_role.permissions.append(main_db_pvm)
db.session.commit()
# make sure dataset is now visible
uri = "api/v1/dataset/"
rv = self.get_assert_metric(uri, "get_list")
assert rv.status_code == 200
response = json.loads(rv.data.decode("utf-8"))
tables = {tbl["table_name"] for tbl in response["result"]}
assert tables == {"ab_user"}
# revert gamma permission
gamma_role.permissions.remove(main_db_pvm)
db.session.delete(dataset)
db.session.commit()
def test_get_dataset_related_database_gamma(self):
@ -2255,6 +2274,8 @@ class TestDatasetApi(SupersetTestCase):
assert len(new_dataset.columns) == 2
assert new_dataset.columns[0].column_name == "id"
assert new_dataset.columns[1].column_name == "name"
db.session.delete(new_dataset)
db.session.commit()
@pytest.mark.usefixtures("create_datasets")
def test_duplicate_physical_dataset(self):