fix(temporary-cache): when user is anonymous (#20181)

* fix(temporary-cache): fail on anonymous user

* make exceptions generic

* fix test

* remove redundant bool return

* fix unit tests

superset/dashboards/filter_state/commands/create.py

@@ -14,11 +14,13 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
+from typing import cast
+
 from flask import session
 
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
 from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
 from superset.temporary_cache.commands.create import CreateTemporaryCacheCommand
 from superset.temporary_cache.commands.entry import Entry
 from superset.temporary_cache.commands.parameters import CommandParameters
@@ -34,10 +36,9 @@ def create(self, cmd_params: CommandParameters) -> str:
         key = cache_manager.filter_state_cache.get(contextual_key)
         if not key or not tab_id:
             key = random_key()
-        value = cmd_params.value
-        dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
-        if dashboard and value:
-            entry: Entry = {"owner": actor.get_user_id(), "value": value}
-            cache_manager.filter_state_cache.set(cache_key(resource_id, key), entry)
-            cache_manager.filter_state_cache.set(contextual_key, key)
+        value = cast(str, cmd_params.value)  # schema ensures that value is not optional
+        check_access(resource_id)
+        entry: Entry = {"owner": get_owner(actor), "value": value}
+        cache_manager.filter_state_cache.set(cache_key(resource_id, key), entry)
+        cache_manager.filter_state_cache.set(contextual_key, key)
         return key

superset/dashboards/filter_state/commands/delete.py

@@ -16,8 +16,9 @@
 # under the License.
 from flask import session
 
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
 from superset.extensions import cache_manager
+from superset.key_value.utils import get_owner
 from superset.temporary_cache.commands.delete import DeleteTemporaryCacheCommand
 from superset.temporary_cache.commands.entry import Entry
 from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
@@ -30,14 +31,13 @@ def delete(self, cmd_params: CommandParameters) -> bool:
         resource_id = cmd_params.resource_id
         actor = cmd_params.actor
         key = cache_key(resource_id, cmd_params.key)
-        dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
-        if dashboard:
-            entry: Entry = cache_manager.filter_state_cache.get(key)
-            if entry:
-                if entry["owner"] != actor.get_user_id():
-                    raise TemporaryCacheAccessDeniedError()
-                tab_id = cmd_params.tab_id
-                contextual_key = cache_key(session.get("_id"), tab_id, resource_id)
-                cache_manager.filter_state_cache.delete(contextual_key)
-                return cache_manager.filter_state_cache.delete(key)
+        check_access(resource_id)
+        entry: Entry = cache_manager.filter_state_cache.get(key)
+        if entry:
+            if entry["owner"] != get_owner(actor):
+                raise TemporaryCacheAccessDeniedError()
+            tab_id = cmd_params.tab_id
+            contextual_key = cache_key(session.get("_id"), tab_id, resource_id)
+            cache_manager.filter_state_cache.delete(contextual_key)
+            return cache_manager.filter_state_cache.delete(key)
         return False

superset/dashboards/filter_state/commands/get.py

@@ -18,7 +18,7 @@
 
 from flask import current_app as app
 
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
 from superset.extensions import cache_manager
 from superset.temporary_cache.commands.get import GetTemporaryCacheCommand
 from superset.temporary_cache.commands.parameters import CommandParameters
@@ -34,7 +34,7 @@ def __init__(self, cmd_params: CommandParameters) -> None:
     def get(self, cmd_params: CommandParameters) -> Optional[str]:
         resource_id = cmd_params.resource_id
         key = cache_key(resource_id, cmd_params.key)
-        DashboardDAO.get_by_id_or_slug(str(resource_id))
+        check_access(resource_id)
         entry = cache_manager.filter_state_cache.get(key) or {}
         if entry and self._refresh_timeout:
             cache_manager.filter_state_cache.set(key, entry)

superset/dashboards/filter_state/commands/update.py

@@ -14,13 +14,13 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from typing import Optional
+from typing import cast, Optional
 
 from flask import session
 
-from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_state.commands.utils import check_access
 from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
 from superset.temporary_cache.commands.entry import Entry
 from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
 from superset.temporary_cache.commands.parameters import CommandParameters
@@ -33,28 +33,23 @@ def update(self, cmd_params: CommandParameters) -> Optional[str]:
         resource_id = cmd_params.resource_id
         actor = cmd_params.actor
         key = cmd_params.key
-        value = cmd_params.value
-        dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
-        if dashboard and value:
-            entry: Entry = cache_manager.filter_state_cache.get(
-                cache_key(resource_id, key)
-            )
-            if entry:
-                user_id = actor.get_user_id()
-                if entry["owner"] != user_id:
-                    raise TemporaryCacheAccessDeniedError()
+        value = cast(str, cmd_params.value)  # schema ensures that value is not optional
+        check_access(resource_id)
+        entry: Entry = cache_manager.filter_state_cache.get(cache_key(resource_id, key))
+        owner = get_owner(actor)
+        if entry:
+            if entry["owner"] != owner:
+                raise TemporaryCacheAccessDeniedError()
 
-                # Generate a new key if tab_id changes or equals 0
-                contextual_key = cache_key(
-                    session.get("_id"), cmd_params.tab_id, resource_id
-                )
-                key = cache_manager.filter_state_cache.get(contextual_key)
-                if not key or not cmd_params.tab_id:
-                    key = random_key()
-                    cache_manager.filter_state_cache.set(contextual_key, key)
+            # Generate a new key if tab_id changes or equals 0
+            contextual_key = cache_key(
+                session.get("_id"), cmd_params.tab_id, resource_id
+            )
+            key = cache_manager.filter_state_cache.get(contextual_key)
+            if not key or not cmd_params.tab_id:
+                key = random_key()
+                cache_manager.filter_state_cache.set(contextual_key, key)
 
-                new_entry: Entry = {"owner": actor.get_user_id(), "value": value}
-                cache_manager.filter_state_cache.set(
-                    cache_key(resource_id, key), new_entry
-                )
+            new_entry: Entry = {"owner": owner, "value": value}
+            cache_manager.filter_state_cache.set(cache_key(resource_id, key), new_entry)
         return key

superset/dashboards/filter_state/commands/utils.py

@@ -0,0 +1,35 @@
+# 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 superset.dashboards.commands.exceptions import (
+    DashboardAccessDeniedError,
+    DashboardNotFoundError,
+)
+from superset.dashboards.dao import DashboardDAO
+from superset.temporary_cache.commands.exceptions import (
+    TemporaryCacheAccessDeniedError,
+    TemporaryCacheResourceNotFoundError,
+)
+
+
+def check_access(resource_id: int) -> None:
+    try:
+        DashboardDAO.get_by_id_or_slug(str(resource_id))
+    except DashboardNotFoundError as ex:
+        raise TemporaryCacheResourceNotFoundError from ex
+    except DashboardAccessDeniedError as ex:
+        raise TemporaryCacheAccessDeniedError from ex

superset/explore/form_data/api.py

@@ -21,23 +21,18 @@
 from flask_appbuilder.api import BaseApi, expose, protect, safe
 from marshmallow import ValidationError
 
-from superset.charts.commands.exceptions import (
-    ChartAccessDeniedError,
-    ChartNotFoundError,
-)
 from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
-from superset.datasets.commands.exceptions import (
-    DatasetAccessDeniedError,
-    DatasetNotFoundError,
-)
 from superset.explore.form_data.commands.create import CreateFormDataCommand
 from superset.explore.form_data.commands.delete import DeleteFormDataCommand
 from superset.explore.form_data.commands.get import GetFormDataCommand
 from superset.explore.form_data.commands.parameters import CommandParameters
 from superset.explore.form_data.commands.update import UpdateFormDataCommand
 from superset.explore.form_data.schemas import FormDataPostSchema, FormDataPutSchema
 from superset.extensions import event_logger
-from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError
+from superset.temporary_cache.commands.exceptions import (
+    TemporaryCacheAccessDeniedError,
+    TemporaryCacheResourceNotFoundError,
+)
 from superset.views.base_api import requires_json
 
 logger = logging.getLogger(__name__)
@@ -118,13 +113,9 @@ def post(self) -> Response:
             return self.response(201, key=key)
         except ValidationError as ex:
             return self.response(400, message=ex.messages)
-        except (
-            ChartAccessDeniedError,
-            DatasetAccessDeniedError,
-            TemporaryCacheAccessDeniedError,
-        ) as ex:
+        except TemporaryCacheAccessDeniedError as ex:
             return self.response(403, message=str(ex))
-        except (ChartNotFoundError, DatasetNotFoundError) as ex:
+        except TemporaryCacheResourceNotFoundError as ex:
             return self.response(404, message=str(ex))
 
     @expose("/form_data/<string:key>", methods=["PUT"])
@@ -195,13 +186,9 @@ def put(self, key: str) -> Response:
             return self.response(200, key=result)
         except ValidationError as ex:
             return self.response(400, message=ex.messages)
-        except (
-            ChartAccessDeniedError,
-            DatasetAccessDeniedError,
-            TemporaryCacheAccessDeniedError,
-        ) as ex:
+        except TemporaryCacheAccessDeniedError as ex:
             return self.response(403, message=str(ex))
-        except (ChartNotFoundError, DatasetNotFoundError) as ex:
+        except TemporaryCacheResourceNotFoundError as ex:
             return self.response(404, message=str(ex))
 
     @expose("/form_data/<string:key>", methods=["GET"])
@@ -250,13 +237,9 @@ def get(self, key: str) -> Response:
             if not form_data:
                 return self.response_404()
             return self.response(200, form_data=form_data)
-        except (
-            ChartAccessDeniedError,
-            DatasetAccessDeniedError,
-            TemporaryCacheAccessDeniedError,
-        ) as ex:
+        except TemporaryCacheAccessDeniedError as ex:
             return self.response(403, message=str(ex))
-        except (ChartNotFoundError, DatasetNotFoundError) as ex:
+        except TemporaryCacheResourceNotFoundError as ex:
             return self.response(404, message=str(ex))
 
     @expose("/form_data/<string:key>", methods=["DELETE"])
@@ -306,11 +289,7 @@ def delete(self, key: str) -> Response:
             if not result:
                 return self.response_404()
             return self.response(200, message="Deleted successfully")
-        except (
-            ChartAccessDeniedError,
-            DatasetAccessDeniedError,
-            TemporaryCacheAccessDeniedError,
-        ) as ex:
+        except TemporaryCacheAccessDeniedError as ex:
             return self.response(403, message=str(ex))
-        except (ChartNotFoundError, DatasetNotFoundError) as ex:
+        except TemporaryCacheResourceNotFoundError as ex:
             return self.response(404, message=str(ex))

superset/explore/form_data/commands/create.py

@@ -22,9 +22,9 @@
 from superset.commands.base import BaseCommand
 from superset.explore.form_data.commands.parameters import CommandParameters
 from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
 from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
 from superset.temporary_cache.commands.exceptions import TemporaryCacheCreateFailedError
 from superset.temporary_cache.utils import cache_key
 from superset.utils.schema import validate_json
@@ -51,7 +51,7 @@ def run(self) -> str:
                 key = random_key()
             if form_data:
                 state: TemporaryExploreState = {
-                    "owner": actor.get_user_id(),
+                    "owner": get_owner(actor),
                     "dataset_id": dataset_id,
                     "chart_id": chart_id,
                     "form_data": form_data,

superset/explore/form_data/commands/delete.py

@@ -23,8 +23,9 @@
 from superset.commands.base import BaseCommand
 from superset.explore.form_data.commands.parameters import CommandParameters
 from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
 from superset.extensions import cache_manager
+from superset.key_value.utils import get_owner
 from superset.temporary_cache.commands.exceptions import (
     TemporaryCacheAccessDeniedError,
     TemporaryCacheDeleteFailedError,
@@ -49,7 +50,7 @@ def run(self) -> bool:
                 dataset_id = state["dataset_id"]
                 chart_id = state["chart_id"]
                 check_access(dataset_id, chart_id, actor)
-                if state["owner"] != actor.get_user_id():
+                if state["owner"] != get_owner(actor):
                     raise TemporaryCacheAccessDeniedError()
                 tab_id = self._cmd_params.tab_id
                 contextual_key = cache_key(

superset/explore/form_data/commands/get.py

@@ -24,7 +24,7 @@
 from superset.commands.base import BaseCommand
 from superset.explore.form_data.commands.parameters import CommandParameters
 from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
 from superset.extensions import cache_manager
 from superset.temporary_cache.commands.exceptions import TemporaryCacheGetFailedError
 

superset/explore/form_data/commands/state.py

@@ -20,7 +20,7 @@
 
 
 class TemporaryExploreState(TypedDict):
-    owner: int
+    owner: Optional[int]
     dataset_id: int
     chart_id: Optional[int]
     form_data: str

superset/explore/form_data/commands/update.py

@@ -24,9 +24,9 @@
 from superset.commands.base import BaseCommand
 from superset.explore.form_data.commands.parameters import CommandParameters
 from superset.explore.form_data.commands.state import TemporaryExploreState
-from superset.explore.utils import check_access
+from superset.explore.form_data.commands.utils import check_access
 from superset.extensions import cache_manager
-from superset.key_value.utils import random_key
+from superset.key_value.utils import get_owner, random_key
 from superset.temporary_cache.commands.exceptions import (
     TemporaryCacheAccessDeniedError,
     TemporaryCacheUpdateFailedError,
@@ -56,9 +56,9 @@ def run(self) -> Optional[str]:
             state: TemporaryExploreState = cache_manager.explore_form_data_cache.get(
                 key
             )
+            owner = get_owner(actor)
             if state and form_data:
-                user_id = actor.get_user_id()
-                if state["owner"] != user_id:
+                if state["owner"] != owner:
                     raise TemporaryCacheAccessDeniedError()
 
                 # Generate a new key if tab_id changes or equals 0
@@ -72,7 +72,7 @@ def run(self) -> Optional[str]:
                     cache_manager.explore_form_data_cache.set(contextual_key, key)
 
                 new_state: TemporaryExploreState = {
-                    "owner": actor.get_user_id(),
+                    "owner": owner,
                     "dataset_id": dataset_id,
                     "chart_id": chart_id,
                     "form_data": form_data,