diff --git a/superset-frontend/src/pages/Tags/index.tsx b/superset-frontend/src/pages/Tags/index.tsx index b0c998c3f32f8..d395ce7cde25a 100644 --- a/superset-frontend/src/pages/Tags/index.tsx +++ b/superset-frontend/src/pages/Tags/index.tsx @@ -353,7 +353,7 @@ function TagList(props: TagListProps) { className="tags-list-view" columns={columns} count={tagCount} - data={tags} + data={tags.filter(tag => !tag.name.includes(':'))} disableBulkSelect={toggleBulkSelect} refreshData={refreshData} emptyState={emptyState} diff --git a/superset/daos/tag.py b/superset/daos/tag.py index e4aa89181644d..60362bfbbd3bd 100644 --- a/superset/daos/tag.py +++ b/superset/daos/tag.py @@ -24,7 +24,7 @@ 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.exceptions import DAODeleteFailedError +from superset.daos.exceptions import DAOCreateFailedError, DAODeleteFailedError from superset.exceptions import MissingUserContextException from superset.extensions import db from superset.models.dashboard import Dashboard @@ -46,12 +46,24 @@ class TagDAO(BaseDAO[Tag]): # base_filter = TagAccessFilter + @staticmethod + def validate_tag_name(tag_name: str) -> bool: + invalid_characters = [":", ","] + for invalid_character in invalid_characters: + if invalid_character in tag_name: + return False + return True + @staticmethod def create_custom_tagged_objects( object_type: ObjectType, object_id: int, tag_names: list[str] ) -> None: tagged_objects = [] for name in tag_names: + if not TagDAO.validate_tag_name(name): + raise DAOCreateFailedError( + message="Invalid Tag Name (cannot contain ':' or ',')" + ) type_ = TagType.custom tag_name = name.strip() tag = TagDAO.get_by_name(tag_name, type_) diff --git a/superset/tags/models.py b/superset/tags/models.py index bae4417507bd4..7a77677a367ad 100644 --- a/superset/tags/models.py +++ b/superset/tags/models.py @@ -19,7 +19,6 @@ import enum from typing import TYPE_CHECKING -from flask import escape from flask_appbuilder import Model from sqlalchemy import Column, Enum, ForeignKey, Integer, orm, String, Table, Text from sqlalchemy.engine.base import Connection @@ -116,7 +115,7 @@ def get_tag(name: str, session: orm.Session, type_: TagType) -> Tag: tag_name = name.strip() tag = session.query(Tag).filter_by(name=tag_name, type=type_).one_or_none() if tag is None: - tag = Tag(name=escape(tag_name), type=type_) + tag = Tag(name=tag_name, type=type_) session.add(tag) session.commit() return tag diff --git a/tests/integration_tests/tags/dao_tests.py b/tests/integration_tests/tags/dao_tests.py index 38bb4d0904d74..272ba43ed324a 100644 --- a/tests/integration_tests/tags/dao_tests.py +++ b/tests/integration_tests/tags/dao_tests.py @@ -124,19 +124,13 @@ def create_tagged_objects(self): @pytest.mark.usefixtures("with_tagging_system_feature") # test create tag def test_create_tagged_objects(self): - # test that a tag can be added if it has ':' in it - TagDAO.create_custom_tagged_objects( - object_type=ObjectType.dashboard.name, - object_id=1, - tag_names=["valid:example tag 1"], - ) - - # test that a tag can be added if it has ',' in it - TagDAO.create_custom_tagged_objects( - object_type=ObjectType.dashboard.name, - object_id=1, - tag_names=["example,tag,1"], - ) + # test that a tag cannot be added if it has ':' in it + with pytest.raises(DAOCreateFailedError): + TagDAO.create_custom_tagged_objects( + object_type=ObjectType.dashboard.name, + object_id=1, + tag_names=["invalid:example tag 1"], + ) # test that a tag can be added if it has a valid name TagDAO.create_custom_tagged_objects( @@ -326,3 +320,11 @@ def test_delete_tagged_object(self): .first() ) assert tagged_object is None + + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + @pytest.mark.usefixtures("with_tagging_system_feature") + def test_validate_tag_name(self): + assert TagDAO.validate_tag_name("example_tag_name") is True + assert TagDAO.validate_tag_name("invalid:tag_name") is False + db.session.query(TaggedObject).delete() + db.session.query(Tag).delete()