fix(Security): Apply permissions to the AllEntities list/get_objects API endpoint (#33577)

This commit is contained in:
Vitor Avila
2025-05-30 13:39:18 -03:00
committed by GitHub
parent 235c9d2ebf
commit ed20d2a917
7 changed files with 236 additions and 174 deletions

View File

@@ -127,7 +127,7 @@ describe('AllEntitiesTable', () => {
expect(screen.queryByText('Add tag to entities')).not.toBeInTheDocument();
});
it('renders the correct tags for each object type, excluding the current tag', () => {
it('renders the correct tags for each object type', () => {
render(
<AllEntitiesTable
search=""
@@ -149,8 +149,6 @@ describe('AllEntitiesTable', () => {
expect(screen.getByText('Queries')).toBeInTheDocument();
expect(screen.getByText('User Engagement')).toBeInTheDocument();
expect(screen.getByText('Engagement')).toBeInTheDocument();
expect(screen.queryByText('Current Tag')).not.toBeInTheDocument();
});
it('Only list asset types that have entities', () => {

View File

@@ -23,7 +23,6 @@ import { TagsList } from 'src/components/Tags';
import FacePile from 'src/components/FacePile';
import Tag from 'src/types/TagType';
import { EmptyState } from 'src/components/EmptyState';
import { NumberParam, useQueryParam } from 'use-query-params';
import { TaggedObject, TaggedObjects } from 'src/types/TaggedObject';
const MAX_TAGS_TO_SHOW = 3;
@@ -64,7 +63,6 @@ export default function AllEntitiesTable({
}: AllEntitiesTableProps) {
type objectType = 'dashboard' | 'chart' | 'query';
const [tagId] = useQueryParam('id', NumberParam);
const showDashboardList = objects.dashboard.length > 0;
const showChartList = objects.chart.length > 0;
const showQueryList = objects.query.length > 0;
@@ -106,8 +104,7 @@ export default function AllEntitiesTable({
tags={tags.filter(
(tag: Tag) =>
tag.type !== undefined &&
['TagType.custom', 1].includes(tag.type) &&
tag.id !== tagId,
['TagType.custom', 1].includes(tag.type),
)}
maxTags={MAX_TAGS_TO_SHOW}
/>

View File

@@ -179,20 +179,6 @@ export function addTag(
.catch(response => error(response));
}
export function fetchObjects(
{ tags = '', types }: { tags: string; types: string | null },
callback: (json: JsonObject) => void,
error: (response: Response) => void,
) {
let url = `/api/v1/tag/get_objects/?tags=${tags}`;
if (types) {
url += `&types=${types}`;
}
SupersetClient.get({ endpoint: url })
.then(({ json }) => callback(json.result))
.catch(response => error(response));
}
export function fetchObjectsByTagIds(
{ tagIds = [], types }: { tagIds: number[] | string; types: string | null },
callback: (json: JsonObject) => void,

View File

@@ -15,7 +15,6 @@
# specific language governing permissions and limitations
# under the License.
import logging
from operator import and_
from typing import Any, Optional
from flask import g
@@ -24,11 +23,11 @@ from sqlalchemy.exc import NoResultFound
from superset.commands.tag.exceptions import TagNotFoundError
from superset.commands.tag.utils import to_object_type
from superset.daos.base import BaseDAO
from superset.daos.chart import ChartDAO
from superset.daos.dashboard import DashboardDAO
from superset.daos.query import SavedQueryDAO
from superset.exceptions import MissingUserContextException
from superset.extensions import db
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import SavedQuery
from superset.tags.models import (
get_tag,
ObjectType,
@@ -43,8 +42,6 @@ logger = logging.getLogger(__name__)
class TagDAO(BaseDAO[Tag]):
# base_filter = TagAccessFilter
@staticmethod
def create_custom_tagged_objects(
object_type: ObjectType, object_id: int, tag_names: list[str]
@@ -139,6 +136,13 @@ class TagDAO(BaseDAO[Tag]):
"""
return db.session.query(Tag).filter(Tag.name == name).first()
@staticmethod
def find_by_names(names: list[str]) -> list[Tag]:
"""
returns tags by their names.
"""
return db.session.query(Tag).filter(Tag.name.in_(names)).all()
@staticmethod
def find_tagged_object(
object_type: ObjectType, object_id: int, tag_id: int
@@ -157,111 +161,105 @@ class TagDAO(BaseDAO[Tag]):
)
@staticmethod
def get_tagged_objects_by_tag_id(
def get_tagged_objects_by_tag_ids(
tag_ids: Optional[list[int]], obj_types: Optional[list[str]] = None
) -> list[dict[str, Any]]:
tags = db.session.query(Tag).filter(Tag.id.in_(tag_ids)).all()
tag_names = [tag.name for tag in tags]
return TagDAO.get_tagged_objects_for_tags(tag_names, obj_types)
results: list[dict[str, Any]] = []
query = db.session.query(TaggedObject).filter(TaggedObject.tag_id.in_(tag_ids))
if obj_types:
query = query.filter(
TaggedObject.object_type.in_(
[ObjectType[obj_type] for obj_type in obj_types]
)
)
tagged_objects = query.all()
# dashboards
if not obj_types or "dashboard" in obj_types:
tagged_dashboards = [
tagged_object.object_id
for tagged_object in tagged_objects
if tagged_object.object_type == ObjectType.dashboard
]
if tagged_dashboards:
results.extend(
{
"id": obj.id,
"type": ObjectType.dashboard.name,
"name": obj.dashboard_title,
"url": obj.url,
"changed_on": obj.changed_on,
"created_by": obj.created_by_fk,
"creator": obj.creator(),
"tags": obj.tags,
"owners": obj.owners,
}
for obj in DashboardDAO.find_by_ids(tagged_dashboards)
)
# charts
if not obj_types or "chart" in obj_types:
tagged_charts = [
tagged_object.object_id
for tagged_object in tagged_objects
if tagged_object.object_type == ObjectType.chart
]
if tagged_charts:
results.extend(
{
"id": obj.id,
"type": ObjectType.chart.name,
"name": obj.slice_name,
"url": obj.url,
"changed_on": obj.changed_on,
"created_by": obj.created_by_fk,
"creator": obj.creator(),
"tags": obj.tags,
"owners": obj.owners,
}
for obj in ChartDAO.find_by_ids(tagged_charts)
)
# saved queries
if not obj_types or "query" in obj_types:
tagged_queries = [
tagged_object.object_id
for tagged_object in tagged_objects
if tagged_object.object_type == ObjectType.query
]
if tagged_queries:
results.extend(
{
"id": obj.id,
"type": ObjectType.query.name,
"name": obj.label,
"url": obj.url(),
"changed_on": obj.changed_on,
"created_by": obj.created_by_fk,
"creator": obj.creator(),
"tags": obj.tags,
"owners": [obj.creator()],
}
for obj in SavedQueryDAO.find_by_ids(tagged_queries)
)
return results
@staticmethod
def get_tagged_objects_for_tags(
tags: Optional[list[str]] = None, obj_types: Optional[list[str]] = None
def get_tagged_objects_by_tag_names(
tag_names: Optional[list[str]] = None, obj_types: Optional[list[str]] = None
) -> list[dict[str, Any]]:
"""
returns a list of tagged objects filtered by tag names and object types
if no filters applied returns all tagged objects
"""
results: list[dict[str, Any]] = []
tags = TagDAO.find_by_names(tag_names) if tag_names else TagDAO.find_all()
if not tags:
return []
# dashboards
if (not obj_types) or ("dashboard" in obj_types):
dashboards = (
db.session.query(Dashboard)
.join(
TaggedObject,
and_(
TaggedObject.object_id == Dashboard.id,
TaggedObject.object_type == ObjectType.dashboard,
),
)
.join(Tag, TaggedObject.tag_id == Tag.id)
.filter(not tags or Tag.name.in_(tags))
)
results.extend(
{
"id": obj.id,
"type": ObjectType.dashboard.name,
"name": obj.dashboard_title,
"url": obj.url,
"changed_on": obj.changed_on,
"created_by": obj.created_by_fk,
"creator": obj.creator(),
"tags": obj.tags,
"owners": obj.owners,
}
for obj in dashboards
)
# charts
if (not obj_types) or ("chart" in obj_types):
charts = (
db.session.query(Slice)
.join(
TaggedObject,
and_(
TaggedObject.object_id == Slice.id,
TaggedObject.object_type == ObjectType.chart,
),
)
.join(Tag, TaggedObject.tag_id == Tag.id)
.filter(not tags or Tag.name.in_(tags))
)
results.extend(
{
"id": obj.id,
"type": ObjectType.chart.name,
"name": obj.slice_name,
"url": obj.url,
"changed_on": obj.changed_on,
"created_by": obj.created_by_fk,
"creator": obj.creator(),
"tags": obj.tags,
"owners": obj.owners,
}
for obj in charts
)
# saved queries
if (not obj_types) or ("query" in obj_types):
saved_queries = (
db.session.query(SavedQuery)
.join(
TaggedObject,
and_(
TaggedObject.object_id == SavedQuery.id,
TaggedObject.object_type == ObjectType.query,
),
)
.join(Tag, TaggedObject.tag_id == Tag.id)
.filter(not tags or Tag.name.in_(tags))
)
results.extend(
{
"id": obj.id,
"type": ObjectType.query.name,
"name": obj.label,
"url": obj.url(),
"changed_on": obj.changed_on,
"created_by": obj.created_by_fk,
"creator": obj.creator(),
"tags": obj.tags,
"owners": [obj.creator()],
}
for obj in saved_queries
)
return results
tag_ids = [tag.id for tag in tags]
return TagDAO.get_tagged_objects_by_tag_ids(tag_ids, obj_types)
@staticmethod
def favorite_tag_by_id_for_current_user( # pylint: disable=invalid-name

View File

@@ -583,9 +583,9 @@ class TagRestApi(BaseSupersetModelRestApi):
if tag_ids:
# priotize using ids for lookups vs. names mainly using this
# for backward compatibility
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(tag_ids, types)
tagged_objects = TagDAO.get_tagged_objects_by_tag_ids(tag_ids, types)
else:
tagged_objects = TagDAO.get_tagged_objects_for_tags(tags, types)
tagged_objects = TagDAO.get_tagged_objects_by_tag_names(tags, types)
result = [
self.object_entity_response_schema.dump(tagged_object)

View File

@@ -14,33 +14,32 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# isort:skip_file
"""Unit tests for Superset"""
import prison
from datetime import datetime
from flask import g # noqa: F401
import pytest
import prison # noqa: F811
from freezegun import freeze_time
from sqlalchemy.sql import func
from sqlalchemy import and_ # noqa: F401
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import SavedQuery # noqa: F401
from superset.tags.models import user_favorite_tag_table # noqa: F401
from unittest.mock import patch
from urllib import parse
import prison
import pytest
from freezegun import freeze_time
from sqlalchemy import and_
from sqlalchemy.sql import func
import tests.integration_tests.test_app # noqa: F401
from superset import db, security_manager # noqa: F401
from superset.common.db_query_status import QueryStatus # noqa: F401
from superset.models.core import Database # noqa: F401
from superset.utils.database import get_example_database, get_main_database # noqa: F401
from superset import db
from superset.connectors.sqla.models import SqlaTable
from superset.daos.tag import TagDAO
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.tags.models import (
ObjectType,
Tag,
TaggedObject,
TagType,
user_favorite_tag_table,
)
from superset.utils import json
from superset.tags.models import ObjectType, Tag, TagType, TaggedObject
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.constants import ADMIN_USERNAME, ALPHA_USERNAME
from tests.integration_tests.fixtures.birth_names_dashboard import (
load_birth_names_dashboard_with_slices, # noqa: F401
@@ -50,10 +49,7 @@ from tests.integration_tests.fixtures.world_bank_dashboard import (
load_world_bank_dashboard_with_slices, # noqa: F401
load_world_bank_data, # noqa: F401
)
from tests.integration_tests.fixtures.tags import with_tagging_system_feature # noqa: F401
from tests.integration_tests.base_tests import SupersetTestCase
from superset.daos.tag import TagDAO
from superset.tags.models import ObjectType # noqa: F811
from tests.integration_tests.insert_chart_mixin import InsertChartMixin
TAGS_FIXTURE_COUNT = 10
@@ -71,7 +67,7 @@ TAGS_LIST_COLUMNS = [
]
class TestTagApi(SupersetTestCase):
class TestTagApi(InsertChartMixin, SupersetTestCase):
def insert_tag(
self,
name: str,
@@ -406,6 +402,96 @@ class TestTagApi(SupersetTestCase):
# clean up tagged object
tagged_objects.delete()
def test_get_tagged_objects_restricted(self):
"""
Test that the get_objects endpoint returns only assets
the user has access to.
"""
owner = self.get_user(ADMIN_USERNAME)
# Create a tag
tag = self.insert_tag(
name="test_tagged_objects_visibility",
tag_type="custom",
)
# Create a chart
chart_first_dataset = self.insert_chart("first_chart", [owner.id], 1)
first_tag_relation = self.insert_tagged_object(
tag_id=tag.id,
object_id=chart_first_dataset.id,
object_type=ObjectType.chart,
)
# Create another chart and add it to a dashboard
chart_second_dataset = self.insert_chart("second_chart", [owner.id], 2)
second_tag_relation = self.insert_tagged_object(
tag_id=tag.id,
object_id=chart_second_dataset.id,
object_type=ObjectType.chart,
)
dashboard = self.insert_dashboard(
"test_dashboard",
"test_dashboard",
[owner.id],
slices=[chart_second_dataset],
published=True,
)
dashboard_tag_relation = self.insert_tagged_object(
tag_id=tag.id,
object_id=dashboard.id,
object_type=ObjectType.dashboard,
)
# Create a user without access to these items
user = self.create_user_with_roles(
"test_restricted_user",
["testing_new_role"],
should_create_roles=True,
)
self.login("test_restricted_user")
uri = f"api/v1/tag/get_objects/?tagIds={tag.id}"
rv = self.client.get(uri)
assert rv.status_code == 200
assert rv.json["result"] == []
# grant access to dataset ID 1
first_dataset = db.session.query(SqlaTable).filter(SqlaTable.id == 1).first()
self.grant_role_access_to_table(first_dataset, "testing_new_role")
rv = self.client.get(uri)
assert rv.status_code == 200
result = rv.json["result"]
assert len(result) == 1
assert result[0]["id"] == chart_first_dataset.id
# grant access to dataset ID 2
second_dataset = db.session.query(SqlaTable).filter(SqlaTable.id == 2).first()
self.grant_role_access_to_table(second_dataset, "testing_new_role")
rv = self.client.get(uri)
assert rv.status_code == 200
result = rv.json["result"]
assert len(result) == 3
assert sorted([res["id"] for res in result]) == sorted(
[chart_first_dataset.id, chart_second_dataset.id, dashboard.id]
)
# Clean up
db.session.delete(dashboard_tag_relation)
db.session.delete(dashboard)
db.session.delete(second_tag_relation)
db.session.delete(chart_second_dataset)
db.session.delete(first_tag_relation)
db.session.delete(chart_first_dataset)
db.session.delete(tag)
self.revoke_role_access_to_table("testing_new_role", first_dataset)
self.revoke_role_access_to_table("testing_new_role", second_dataset)
db.session.delete(user.roles[0])
db.session.delete(user)
db.session.commit()
# test delete tags
@pytest.mark.usefixtures("create_tags")
def test_delete_tags(self):
@@ -443,9 +529,6 @@ class TestTagApi(SupersetTestCase):
rv = self.client.post(uri, follow_redirects=True)
assert rv.status_code == 200
from sqlalchemy import and_ # noqa: F811
from superset.tags.models import user_favorite_tag_table # noqa: F811
from flask import g # noqa: F401, F811
association_row = (
db.session.query(user_favorite_tag_table)
@@ -630,10 +713,10 @@ class TestTagApi(SupersetTestCase):
assert rv.status_code == 200
result = TagDAO.get_tagged_objects_for_tags(tags, ["dashboard"])
result = TagDAO.get_tagged_objects_by_tag_names(tags, ["dashboard"])
assert len(result) == 1
result = TagDAO.get_tagged_objects_for_tags(tags, ["chart"])
result = TagDAO.get_tagged_objects_by_tag_names(tags, ["chart"])
assert len(result) == 1
tagged_objects = (

View File

@@ -14,27 +14,25 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# isort:skip_file
from operator import and_
from unittest.mock import patch # noqa: F401
import pytest
from superset.models.slice import Slice
from superset.models.sql_lab import SavedQuery # noqa: F401
from superset.daos.tag import TagDAO
from superset.tags.exceptions import InvalidTagNameError # noqa: F401
from superset.tags.models import ObjectType, Tag, TaggedObject
from tests.integration_tests.tags.api_tests import TAGS_FIXTURE_COUNT
import tests.integration_tests.test_app # pylint: disable=unused-import # noqa: F401
from superset import db, security_manager # noqa: F401
from superset.daos.dashboard import DashboardDAO # noqa: F401
import pytest
from superset import db
from superset.daos.tag import TagDAO
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.tags.models import ObjectType, Tag, TaggedObject
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.constants import ADMIN_USERNAME
from tests.integration_tests.fixtures.tags import (
with_tagging_system_feature, # noqa: F401
)
from tests.integration_tests.fixtures.world_bank_dashboard import (
load_world_bank_dashboard_with_slices, # noqa: F401
load_world_bank_data, # noqa: F401
)
from tests.integration_tests.fixtures.tags import with_tagging_system_feature # noqa: F401
from tests.integration_tests.tags.api_tests import TAGS_FIXTURE_COUNT
class TestTagsDAO(SupersetTestCase):
@@ -151,6 +149,7 @@ class TestTagsDAO(SupersetTestCase):
@pytest.mark.usefixtures("create_tags")
# test get objects from tag
def test_get_objects_from_tag(self):
self.login(ADMIN_USERNAME)
# create tagged objects
dashboard = (
db.session.query(Dashboard)
@@ -163,17 +162,17 @@ class TestTagsDAO(SupersetTestCase):
object_id=dashboard_id, object_type=ObjectType.dashboard, tag_id=tag.id
)
# get objects
tagged_objects = TagDAO.get_tagged_objects_for_tags(
tagged_objects = TagDAO.get_tagged_objects_by_tag_names(
["example_tag_1", "example_tag_2"]
)
assert len(tagged_objects) == 1
# test get objects from tag with type
tagged_objects = TagDAO.get_tagged_objects_for_tags(
tagged_objects = TagDAO.get_tagged_objects_by_tag_names(
["example_tag_1", "example_tag_2"], obj_types=["dashboard", "chart"]
)
assert len(tagged_objects) == 1
tagged_objects = TagDAO.get_tagged_objects_for_tags(
tagged_objects = TagDAO.get_tagged_objects_by_tag_names(
["example_tag_1", "example_tag_2"], obj_types=["chart"]
)
assert len(tagged_objects) == 0
@@ -206,12 +205,12 @@ class TestTagsDAO(SupersetTestCase):
+ num_charts
)
# gets all tagged objects of type dashboard and chart
tagged_objects = TagDAO.get_tagged_objects_for_tags(
tagged_objects = TagDAO.get_tagged_objects_by_tag_names(
obj_types=["dashboard", "chart"]
)
assert len(tagged_objects) == num_charts_and_dashboards
# test objects are retrieved by type
tagged_objects = TagDAO.get_tagged_objects_for_tags(obj_types=["chart"])
tagged_objects = TagDAO.get_tagged_objects_by_tag_names(obj_types=["chart"])
assert len(tagged_objects) == num_charts
@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
@@ -219,6 +218,7 @@ class TestTagsDAO(SupersetTestCase):
@pytest.mark.usefixtures("create_tags")
# test get objects from tag
def test_get_objects_from_tag_with_id(self):
self.login(ADMIN_USERNAME)
# create tagged objects
dashboard = (
db.session.query(Dashboard)
@@ -233,16 +233,16 @@ class TestTagsDAO(SupersetTestCase):
object_id=dashboard_id, object_type=ObjectType.dashboard, tag_id=tag_1.id
)
# get objects
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(tag_ids)
tagged_objects = TagDAO.get_tagged_objects_by_tag_ids(tag_ids)
assert len(tagged_objects) == 1
# test get objects from tag with type
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(
tagged_objects = TagDAO.get_tagged_objects_by_tag_ids(
tag_ids, obj_types=["dashboard", "chart"]
)
assert len(tagged_objects) == 1
tagged_objects = TagDAO.get_tagged_objects_by_tag_id(
tagged_objects = TagDAO.get_tagged_objects_by_tag_ids(
tag_ids, obj_types=["chart"]
)
assert len(tagged_objects) == 0