From 64c4226817b04ff598be29b52d8e2c4a679ef70a Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Thu, 26 May 2022 14:45:20 +0300 Subject: [PATCH] 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 --- .../filter_state/commands/create.py | 17 ++++--- .../filter_state/commands/delete.py | 22 ++++---- .../dashboards/filter_state/commands/get.py | 4 +- .../filter_state/commands/update.py | 45 ++++++++-------- .../dashboards/filter_state/commands/utils.py | 35 +++++++++++++ superset/explore/form_data/api.py | 45 +++++----------- superset/explore/form_data/commands/create.py | 6 +-- superset/explore/form_data/commands/delete.py | 5 +- superset/explore/form_data/commands/get.py | 2 +- superset/explore/form_data/commands/state.py | 2 +- superset/explore/form_data/commands/update.py | 10 ++-- superset/explore/form_data/commands/utils.py | 42 +++++++++++++++ superset/explore/utils.py | 8 ++- superset/key_value/utils.py | 7 ++- superset/temporary_cache/api.py | 51 ++++--------------- superset/temporary_cache/commands/entry.py | 4 +- .../temporary_cache/commands/exceptions.py | 4 ++ tests/unit_tests/explore/utils_test.py | 8 +-- 18 files changed, 175 insertions(+), 142 deletions(-) create mode 100644 superset/dashboards/filter_state/commands/utils.py create mode 100644 superset/explore/form_data/commands/utils.py diff --git a/superset/dashboards/filter_state/commands/create.py b/superset/dashboards/filter_state/commands/create.py index 137623027a1fc..18dff8928fe83 100644 --- a/superset/dashboards/filter_state/commands/create.py +++ b/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 diff --git a/superset/dashboards/filter_state/commands/delete.py b/superset/dashboards/filter_state/commands/delete.py index 155c63f1084c6..3ddc08fc51900 100644 --- a/superset/dashboards/filter_state/commands/delete.py +++ b/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 diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py index 9cdd5bcddcb48..ca7ffa9879a9f 100644 --- a/superset/dashboards/filter_state/commands/get.py +++ b/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) diff --git a/superset/dashboards/filter_state/commands/update.py b/superset/dashboards/filter_state/commands/update.py index d27277f9afb97..7f150aae6bae3 100644 --- a/superset/dashboards/filter_state/commands/update.py +++ b/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 diff --git a/superset/dashboards/filter_state/commands/utils.py b/superset/dashboards/filter_state/commands/utils.py new file mode 100644 index 0000000000000..35f940f4343e1 --- /dev/null +++ b/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 diff --git a/superset/explore/form_data/api.py b/superset/explore/form_data/api.py index dc6ee7ea94cc3..ea2b38658bb67 100644 --- a/superset/explore/form_data/api.py +++ b/superset/explore/form_data/api.py @@ -21,15 +21,7 @@ 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 @@ -37,7 +29,10 @@ 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/", 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/", 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/", 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)) diff --git a/superset/explore/form_data/commands/create.py b/superset/explore/form_data/commands/create.py index 7b1f866c505df..84ad0f0078500 100644 --- a/superset/explore/form_data/commands/create.py +++ b/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, diff --git a/superset/explore/form_data/commands/delete.py b/superset/explore/form_data/commands/delete.py index ec537313d2ba0..69d5a79b8f01b 100644 --- a/superset/explore/form_data/commands/delete.py +++ b/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( diff --git a/superset/explore/form_data/commands/get.py b/superset/explore/form_data/commands/get.py index 5b582008218cc..809672f32f982 100644 --- a/superset/explore/form_data/commands/get.py +++ b/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 diff --git a/superset/explore/form_data/commands/state.py b/superset/explore/form_data/commands/state.py index 2aba14a8cb28f..c8061e81f5a7e 100644 --- a/superset/explore/form_data/commands/state.py +++ b/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 diff --git a/superset/explore/form_data/commands/update.py b/superset/explore/form_data/commands/update.py index 596c5f6e27ef2..76dfee1dadef8 100644 --- a/superset/explore/form_data/commands/update.py +++ b/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, diff --git a/superset/explore/form_data/commands/utils.py b/superset/explore/form_data/commands/utils.py new file mode 100644 index 0000000000000..5d09657fbec4f --- /dev/null +++ b/superset/explore/form_data/commands/utils.py @@ -0,0 +1,42 @@ +# 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 Optional + +from flask_appbuilder.security.sqla.models import User + +from superset.charts.commands.exceptions import ( + ChartAccessDeniedError, + ChartNotFoundError, +) +from superset.datasets.commands.exceptions import ( + DatasetAccessDeniedError, + DatasetNotFoundError, +) +from superset.explore.utils import check_access as explore_check_access +from superset.temporary_cache.commands.exceptions import ( + TemporaryCacheAccessDeniedError, + TemporaryCacheResourceNotFoundError, +) + + +def check_access(dataset_id: int, chart_id: Optional[int], actor: User) -> None: + try: + explore_check_access(dataset_id, chart_id, actor) + except (ChartNotFoundError, DatasetNotFoundError) as ex: + raise TemporaryCacheResourceNotFoundError from ex + except (ChartAccessDeniedError, DatasetAccessDeniedError) as ex: + raise TemporaryCacheAccessDeniedError from ex diff --git a/superset/explore/utils.py b/superset/explore/utils.py index 3eeb1bab9964a..7ab29de2f70ec 100644 --- a/superset/explore/utils.py +++ b/superset/explore/utils.py @@ -44,12 +44,10 @@ def check_dataset_access(dataset_id: int) -> Optional[bool]: raise DatasetNotFoundError() -def check_access( - dataset_id: int, chart_id: Optional[int], actor: User -) -> Optional[bool]: +def check_access(dataset_id: int, chart_id: Optional[int], actor: User) -> None: check_dataset_access(dataset_id) if not chart_id: - return True + return chart = ChartDAO.find_by_id(chart_id) if chart: can_access_chart = ( @@ -58,6 +56,6 @@ def check_access( or security_manager.can_access("can_read", "Chart") ) if can_access_chart: - return True + return raise ChartAccessDeniedError() raise ChartNotFoundError() diff --git a/superset/key_value/utils.py b/superset/key_value/utils.py index b2e8e729b0466..db27e505fbd6c 100644 --- a/superset/key_value/utils.py +++ b/superset/key_value/utils.py @@ -18,10 +18,11 @@ from hashlib import md5 from secrets import token_urlsafe -from typing import Union +from typing import Optional, Union from uuid import UUID import hashids +from flask_appbuilder.security.sqla.models import User from flask_babel import gettext as _ from superset.key_value.exceptions import KeyValueParseKeyError @@ -63,3 +64,7 @@ def get_uuid_namespace(seed: str) -> UUID: md5_obj = md5() md5_obj.update(seed.encode("utf-8")) return UUID(md5_obj.hexdigest()) + + +def get_owner(user: User) -> Optional[int]: + return user.get_user_id() if not user.is_anonymous else None diff --git a/superset/temporary_cache/api.py b/superset/temporary_cache/api.py index e91a2886691f4..bdbdda302e694 100644 --- a/superset/temporary_cache/api.py +++ b/superset/temporary_cache/api.py @@ -24,20 +24,11 @@ from flask_appbuilder.api import BaseApi 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.dashboards.commands.exceptions import ( - DashboardAccessDeniedError, - DashboardNotFoundError, -) -from superset.datasets.commands.exceptions import ( - DatasetAccessDeniedError, - DatasetNotFoundError, +from superset.temporary_cache.commands.exceptions import ( + TemporaryCacheAccessDeniedError, + TemporaryCacheResourceNotFoundError, ) -from superset.temporary_cache.commands.exceptions import TemporaryCacheAccessDeniedError from superset.temporary_cache.commands.parameters import CommandParameters from superset.temporary_cache.schemas import ( TemporaryCachePostSchema, @@ -86,14 +77,9 @@ def post(self, pk: int) -> Response: return self.response(201, key=key) except ValidationError as ex: return self.response(400, message=ex.messages) - except ( - ChartAccessDeniedError, - DashboardAccessDeniedError, - DatasetAccessDeniedError, - TemporaryCacheAccessDeniedError, - ) as ex: + except TemporaryCacheAccessDeniedError as ex: return self.response(403, message=str(ex)) - except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex: + except TemporaryCacheResourceNotFoundError as ex: return self.response(404, message=str(ex)) @requires_json @@ -112,14 +98,9 @@ def put(self, pk: int, key: str) -> Response: return self.response(200, key=key) except ValidationError as ex: return self.response(400, message=ex.messages) - except ( - ChartAccessDeniedError, - DashboardAccessDeniedError, - DatasetAccessDeniedError, - TemporaryCacheAccessDeniedError, - ) as ex: + except TemporaryCacheAccessDeniedError as ex: return self.response(403, message=str(ex)) - except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex: + except TemporaryCacheResourceNotFoundError as ex: return self.response(404, message=str(ex)) def get(self, pk: int, key: str) -> Response: @@ -129,14 +110,9 @@ def get(self, pk: int, key: str) -> Response: if not value: return self.response_404() return self.response(200, value=value) - except ( - ChartAccessDeniedError, - DashboardAccessDeniedError, - DatasetAccessDeniedError, - TemporaryCacheAccessDeniedError, - ) as ex: + except TemporaryCacheAccessDeniedError as ex: return self.response(403, message=str(ex)) - except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex: + except TemporaryCacheResourceNotFoundError as ex: return self.response(404, message=str(ex)) def delete(self, pk: int, key: str) -> Response: @@ -146,14 +122,9 @@ def delete(self, pk: int, key: str) -> Response: if not result: return self.response_404() return self.response(200, message="Deleted successfully") - except ( - ChartAccessDeniedError, - DashboardAccessDeniedError, - DatasetAccessDeniedError, - TemporaryCacheAccessDeniedError, - ) as ex: + except TemporaryCacheAccessDeniedError as ex: return self.response(403, message=str(ex)) - except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex: + except TemporaryCacheResourceNotFoundError as ex: return self.response(404, message=str(ex)) @abstractmethod diff --git a/superset/temporary_cache/commands/entry.py b/superset/temporary_cache/commands/entry.py index 0e9ad0a735069..90aa8e1bebf48 100644 --- a/superset/temporary_cache/commands/entry.py +++ b/superset/temporary_cache/commands/entry.py @@ -14,9 +14,11 @@ # 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_extensions import TypedDict class Entry(TypedDict): - owner: int + owner: Optional[int] value: str diff --git a/superset/temporary_cache/commands/exceptions.py b/superset/temporary_cache/commands/exceptions.py index 0f8c44cb18fd9..8652a732f7ea0 100644 --- a/superset/temporary_cache/commands/exceptions.py +++ b/superset/temporary_cache/commands/exceptions.py @@ -43,3 +43,7 @@ class TemporaryCacheUpdateFailedError(UpdateFailedError): class TemporaryCacheAccessDeniedError(ForbiddenError): message = _("You don't have permission to modify the value.") + + +class TemporaryCacheResourceNotFoundError(ForbiddenError): + message = _("Resource was not found.") diff --git a/tests/unit_tests/explore/utils_test.py b/tests/unit_tests/explore/utils_test.py index 3d12f5e911ee9..11e2906ed88db 100644 --- a/tests/unit_tests/explore/utils_test.py +++ b/tests/unit_tests/explore/utils_test.py @@ -75,7 +75,7 @@ def test_unsaved_chart_authorized_dataset( mocker.patch(dataset_find_by_id, return_value=SqlaTable()) mocker.patch(can_access_datasource, return_value=True) - assert check_access(dataset_id=1, chart_id=0, actor=User()) == True + check_access(dataset_id=1, chart_id=0, actor=User()) def test_saved_chart_unknown_chart_id( @@ -112,7 +112,7 @@ def test_saved_chart_is_admin(mocker: MockFixture, app_context: AppContext) -> N mocker.patch(can_access_datasource, return_value=True) mocker.patch(is_user_admin, return_value=True) mocker.patch(chart_find_by_id, return_value=Slice()) - assert check_access(dataset_id=1, chart_id=1, actor=User()) is True + check_access(dataset_id=1, chart_id=1, actor=User()) def test_saved_chart_is_owner(mocker: MockFixture, app_context: AppContext) -> None: @@ -125,7 +125,7 @@ def test_saved_chart_is_owner(mocker: MockFixture, app_context: AppContext) -> N mocker.patch(is_user_admin, return_value=False) mocker.patch(is_owner, return_value=True) mocker.patch(chart_find_by_id, return_value=Slice()) - assert check_access(dataset_id=1, chart_id=1, actor=User()) == True + check_access(dataset_id=1, chart_id=1, actor=User()) def test_saved_chart_has_access(mocker: MockFixture, app_context: AppContext) -> None: @@ -139,7 +139,7 @@ def test_saved_chart_has_access(mocker: MockFixture, app_context: AppContext) -> mocker.patch(is_owner, return_value=False) mocker.patch(can_access, return_value=True) mocker.patch(chart_find_by_id, return_value=Slice()) - assert check_access(dataset_id=1, chart_id=1, actor=User()) == True + check_access(dataset_id=1, chart_id=1, actor=User()) def test_saved_chart_no_access(mocker: MockFixture, app_context: AppContext) -> None: