Revert "fix: Allows PUT and DELETE only for owners of dashboard filte…

…r state (apache#17644)"

This reverts commit 2ae83fa.

superset/dashboards/filter_state/commands/create.py

@@ -16,23 +16,17 @@
 # under the License.
 from typing import Optional
 
-from flask_appbuilder.security.sqla.models import User
-
 from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
 from superset.extensions import cache_manager
 from superset.key_value.commands.create import CreateKeyValueCommand
 from superset.key_value.utils import cache_key
 
 
 class CreateFilterStateCommand(CreateKeyValueCommand):
-    def create(
-        self, actor: User, resource_id: int, key: str, value: str
-    ) -> Optional[bool]:
+    def create(self, resource_id: int, key: str, value: str) -> Optional[bool]:
         dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
         if dashboard:
-            entry: Entry = {"owner": actor.get_user_id(), "value": value}
             return cache_manager.filter_state_cache.set(
-                cache_key(resource_id, key), entry
+                cache_key(resource_id, key), value
             )
         return False

superset/dashboards/filter_state/commands/delete.py

@@ -16,27 +16,15 @@
 # under the License.
 from typing import Optional
 
-from flask_appbuilder.security.sqla.models import User
-
 from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
 from superset.extensions import cache_manager
 from superset.key_value.commands.delete import DeleteKeyValueCommand
-from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
 from superset.key_value.utils import cache_key
 
 
 class DeleteFilterStateCommand(DeleteKeyValueCommand):
-    def delete(self, actor: User, resource_id: int, key: str) -> Optional[bool]:
+    def delete(self, resource_id: int, key: str) -> Optional[bool]:
         dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
         if dashboard:
-            entry: Entry = cache_manager.filter_state_cache.get(
-                cache_key(resource_id, key)
-            )
-            if entry:
-                if entry["owner"] != actor.get_user_id():
-                    raise KeyValueAccessDeniedError()
-                return cache_manager.filter_state_cache.delete(
-                    cache_key(resource_id, key)
-                )
+            return cache_manager.filter_state_cache.delete(cache_key(resource_id, key))
         return False

superset/dashboards/filter_state/commands/get.py

@@ -17,7 +17,6 @@
 from typing import Optional
 
 from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
 from superset.extensions import cache_manager
 from superset.key_value.commands.get import GetKeyValueCommand
 from superset.key_value.utils import cache_key
@@ -27,10 +26,8 @@ class GetFilterStateCommand(GetKeyValueCommand):
     def get(self, resource_id: int, key: str, refreshTimeout: bool) -> Optional[str]:
         dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
         if dashboard:
-            entry: Entry = cache_manager.filter_state_cache.get(
-                cache_key(resource_id, key)
-            )
+            value = cache_manager.filter_state_cache.get(cache_key(resource_id, key))
             if refreshTimeout:
-                cache_manager.filter_state_cache.set(key, entry)
-            return entry["value"]
+                cache_manager.filter_state_cache.set(key, value)
+            return value
         return None

superset/dashboards/filter_state/commands/update.py

@@ -16,31 +16,17 @@
 # under the License.
 from typing import Optional
 
-from flask_appbuilder.security.sqla.models import User
-
 from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
 from superset.extensions import cache_manager
-from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
 from superset.key_value.commands.update import UpdateKeyValueCommand
 from superset.key_value.utils import cache_key
 
 
 class UpdateFilterStateCommand(UpdateKeyValueCommand):
-    def update(
-        self, actor: User, resource_id: int, key: str, value: str
-    ) -> Optional[bool]:
+    def update(self, resource_id: int, key: str, value: str) -> Optional[bool]:
         dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
         if dashboard:
-            entry: Entry = cache_manager.filter_state_cache.get(
-                cache_key(resource_id, key)
+            return cache_manager.filter_state_cache.set(
+                cache_key(resource_id, key), value
             )
-            if entry:
-                user_id = actor.get_user_id()
-                if entry["owner"] != user_id:
-                    raise KeyValueAccessDeniedError()
-                new_entry: Entry = {"owner": actor.get_user_id(), "value": value}
-                return cache_manager.filter_state_cache.set(
-                    cache_key(resource_id, key), new_entry
-                )
         return False

superset/key_value/api.py

@@ -29,7 +29,6 @@
     DashboardNotFoundError,
 )
 from superset.exceptions import InvalidPayloadFormatError
-from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
 from superset.key_value.schemas import KeyValuePostSchema, KeyValuePutSchema
 
 logger = logging.getLogger(__name__)
@@ -65,7 +64,7 @@ def post(self, pk: int) -> Response:
             return self.response(201, key=key)
         except ValidationError as error:
             return self.response_400(message=error.messages)
-        except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
+        except DashboardAccessDeniedError:
             return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()
@@ -81,7 +80,7 @@ def put(self, pk: int, key: str) -> Response:
             return self.response(200, message="Value updated successfully.")
         except ValidationError as error:
             return self.response_400(message=error.messages)
-        except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
+        except DashboardAccessDeniedError:
             return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()
@@ -92,7 +91,7 @@ def get(self, pk: int, key: str) -> Response:
             if not value:
                 return self.response_404()
             return self.response(200, value=value)
-        except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
+        except DashboardAccessDeniedError:
             return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()
@@ -103,7 +102,7 @@ def delete(self, pk: int, key: str) -> Response:
             if not result:
                 return self.response_404()
             return self.response(200, message="Deleted successfully")
-        except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
+        except DashboardAccessDeniedError:
             return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()

superset/key_value/commands/create.py

@@ -31,16 +31,16 @@
 
 class CreateKeyValueCommand(BaseCommand, ABC):
     def __init__(
-        self, actor: User, resource_id: int, value: str,
+        self, user: User, resource_id: int, value: str,
     ):
-        self._actor = actor
+        self._actor = user
         self._resource_id = resource_id
         self._value = value
 
     def run(self) -> Model:
         try:
             key = token_urlsafe(48)
-            self.create(self._actor, self._resource_id, key, self._value)
+            self.create(self._resource_id, key, self._value)
             return key
         except SQLAlchemyError as ex:
             logger.exception("Error running create command")
@@ -50,7 +50,5 @@ def validate(self) -> None:
         pass
 
     @abstractmethod
-    def create(
-        self, actor: User, resource_id: int, key: str, value: str
-    ) -> Optional[bool]:
+    def create(self, resource_id: int, key: str, value: str) -> Optional[bool]:
         ...

superset/key_value/commands/delete.py

@@ -29,14 +29,14 @@
 
 
 class DeleteKeyValueCommand(BaseCommand, ABC):
-    def __init__(self, actor: User, resource_id: int, key: str):
-        self._actor = actor
+    def __init__(self, user: User, resource_id: int, key: str):
+        self._actor = user
         self._resource_id = resource_id
         self._key = key
 
     def run(self) -> Model:
         try:
-            return self.delete(self._actor, self._resource_id, self._key)
+            return self.delete(self._resource_id, self._key)
         except SQLAlchemyError as ex:
             logger.exception("Error running delete command")
             raise KeyValueDeleteFailedError() from ex
@@ -45,5 +45,5 @@ def validate(self) -> None:
         pass
 
     @abstractmethod
-    def delete(self, actor: User, resource_id: int, key: str) -> Optional[bool]:
+    def delete(self, resource_id: int, key: str) -> Optional[bool]:
         ...

superset/key_value/commands/exceptions.py

@@ -20,7 +20,6 @@
     CommandException,
     CreateFailedError,
     DeleteFailedError,
-    ForbiddenError,
     UpdateFailedError,
 )
 
@@ -39,7 +38,3 @@ class KeyValueDeleteFailedError(DeleteFailedError):
 
 class KeyValueUpdateFailedError(UpdateFailedError):
     message = _("An error occurred while updating the value.")
-
-
-class KeyValueAccessDeniedError(ForbiddenError):
-    message = _("You don't have permission to modify the value.")

superset/key_value/commands/get.py

@@ -30,8 +30,8 @@
 
 
 class GetKeyValueCommand(BaseCommand, ABC):
-    def __init__(self, actor: User, resource_id: int, key: str):
-        self._actor = actor
+    def __init__(self, user: User, resource_id: int, key: str):
+        self._actor = user
         self._resource_id = resource_id
         self._key = key
 

superset/key_value/commands/update.py

@@ -30,16 +30,16 @@
 
 class UpdateKeyValueCommand(BaseCommand, ABC):
     def __init__(
-        self, actor: User, resource_id: int, key: str, value: str,
+        self, user: User, resource_id: int, key: str, value: str,
     ):
-        self._actor = actor
+        self._actor = user
         self._resource_id = resource_id
         self._key = key
         self._value = value
 
     def run(self) -> Model:
         try:
-            return self.update(self._actor, self._resource_id, self._key, self._value)
+            return self.update(self._resource_id, self._key, self._value)
         except SQLAlchemyError as ex:
             logger.exception("Error running update command")
             raise KeyValueUpdateFailedError() from ex
@@ -48,7 +48,5 @@ def validate(self) -> None:
         pass
 
     @abstractmethod
-    def update(
-        self, actor: User, resource_id: int, key: str, value: str
-    ) -> Optional[bool]:
+    def update(self, resource_id: int, key: str, value: str) -> Optional[bool]:
         ...