From dfee0fc77dea92d1244334967cbb985a41d4cd81 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 14 Jun 2023 15:53:58 +0100 Subject: [PATCH 1/8] chore: remove deprecated apis on superset --- RESOURCES/STANDARD_ROLES.md | 9 - UPDATING.md | 1 + superset/views/base.py | 2 +- superset/views/core.py | 291 +--------------------- tests/integration_tests/core_tests.py | 221 +++++++++------- tests/integration_tests/security_tests.py | 4 - 6 files changed, 131 insertions(+), 397 deletions(-) diff --git a/RESOURCES/STANDARD_ROLES.md b/RESOURCES/STANDARD_ROLES.md index cbef53abbec39..5a413b14e9c56 100644 --- a/RESOURCES/STANDARD_ROLES.md +++ b/RESOURCES/STANDARD_ROLES.md @@ -61,30 +61,21 @@ |can my queries on SqlLab|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| |can log on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can schemas access for csv upload on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can user slices on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can favstar on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can import dashboards on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can schemas on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can sqllab history on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| |can publish on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can csv on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| -|can fave dashboards by username on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can slice on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can sync druid source on Superset|:heavy_check_mark:|O|O|O| |can explore on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can fave slices on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can slice json on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can approve on Superset|:heavy_check_mark:|O|O|O| |can explore json on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can fetch datasource metadata on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can override role permissions on Superset|:heavy_check_mark:|O|O|O| -|can created dashboards on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can csrf token on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can created slices on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can annotation json on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can fave dashboards on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can sqllab on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| -|can recent activity on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can select star on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can warm up cache on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can sqllab table viz on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| diff --git a/UPDATING.md b/UPDATING.md index 3db0ab6a4b09a..2da1efaad5bd2 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,6 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [---](https://github.com/apache/superset/pull/---): Removed deprecated APIs `/superset/recent_activity/...`, `/superset/fave_dashboards_by_username/...`, `/superset/fave_dashboards/...`, `/superset/created_dashboards/...`, `/superset/user_slices/`, `/superset/created_slices/...`, `/superset/fave_slices/...`, `/superset/favstar/...`, - [24375](https://github.com/apache/superset/pull/24375): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz` - [24360](https://github.com/apache/superset/pull/24360): Removed deprecated APIs `/superset/stop_query/...`, `/superset/queries/...`, `/superset/search_queries` - [24353](https://github.com/apache/superset/pull/24353): Removed deprecated APIs `/copy_dash/int:dashboard_id/`, `/save_dash/int:dashboard_id/`, `/add_slices/int:dashboard_id/`. diff --git a/superset/views/base.py b/superset/views/base.py index 2d49511882104..54a843d5f3c84 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -391,7 +391,7 @@ def menu_data(user: User) -> dict[str, Any]: "user_login_url": appbuilder.get_url_for_login, "user_profile_url": None if user.is_anonymous or is_feature_enabled("MENU_HIDE_USER_INFO") - else f"/superset/profile/{user.username}", + else f"/superset/profile/", "locale": session.get("locale", "en"), }, } diff --git a/superset/views/core.py b/superset/views/core.py index 782fad4978b8f..7c1de9eabce95 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -840,226 +840,6 @@ def get_user_activity_access_error(user_id: int) -> FlaskResponse | None: ) return None - @api - @has_access_api - @event_logger.log_this - @expose("/recent_activity//", methods=("GET",)) - @deprecated(new_target="/api/v1/log/recent_activity//") - def recent_activity(self, user_id: int) -> FlaskResponse: - """Recent activity (actions) for a given user""" - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - - limit = request.args.get("limit") - limit = int(limit) if limit and limit.isdigit() else 100 - actions = request.args.get("actions", "explore,dashboard").split(",") - # whether to get distinct subjects - distinct = request.args.get("distinct") != "false" - - payload = LogDAO.get_recent_activity(user_id, actions, distinct, 0, limit) - - return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - - @api - @has_access_api - @event_logger.log_this - @expose("/fave_dashboards_by_username//", methods=("GET",)) - @deprecated(new_target="api/v1/dashboard/favorite_status/") - def fave_dashboards_by_username(self, username: str) -> FlaskResponse: - """This lets us use a user's username to pull favourite dashboards""" - user = security_manager.find_user(username=username) - return self.fave_dashboards(user.id) - - @api - @has_access_api - @event_logger.log_this - @expose("/fave_dashboards//", methods=("GET",)) - @deprecated(new_target="api/v1/dashboard/favorite_status/") - def fave_dashboards(self, user_id: int) -> FlaskResponse: - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - qry = ( - db.session.query(Dashboard, FavStar.dttm) - .join( - FavStar, - and_( - FavStar.user_id == int(user_id), - FavStar.class_name == "Dashboard", - Dashboard.id == FavStar.obj_id, - ), - ) - .order_by(FavStar.dttm.desc()) - ) - payload = [] - for o in qry.all(): - dash = { - "id": o.Dashboard.id, - "dashboard": o.Dashboard.dashboard_link(), - "title": o.Dashboard.dashboard_title, - "url": o.Dashboard.url, - "dttm": o.dttm, - } - if o.Dashboard.created_by: - user = o.Dashboard.created_by - dash["creator"] = str(user) - dash["creator_url"] = f"/superset/profile/{user.username}/" - payload.append(dash) - return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - - @api - @has_access_api - @event_logger.log_this - @expose("/created_dashboards//", methods=("GET",)) - @deprecated(new_target="api/v1/dashboard/") - def created_dashboards(self, user_id: int) -> FlaskResponse: - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - qry = ( - db.session.query(Dashboard) - .filter( # pylint: disable=comparison-with-callable - or_( - Dashboard.created_by_fk == user_id, - Dashboard.changed_by_fk == user_id, - ) - ) - .order_by(Dashboard.changed_on.desc()) - ) - payload = [ - { - "id": o.id, - "dashboard": o.dashboard_link(), - "title": o.dashboard_title, - "url": o.url, - "dttm": o.changed_on, - } - for o in qry.all() - ] - return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - - @api - @has_access_api - @event_logger.log_this - @expose("/user_slices", methods=("GET",)) - @expose("/user_slices//", methods=("GET",)) - @deprecated(new_target="/api/v1/chart/") - def user_slices(self, user_id: int | None = None) -> FlaskResponse: - """List of slices a user owns, created, modified or faved""" - if not user_id: - user_id = cast(int, get_user_id()) - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - - owner_ids_query = ( - db.session.query(Slice.id) - .join(Slice.owners) - .filter(security_manager.user_model.id == user_id) - ) - - qry = ( - db.session.query(Slice, FavStar.dttm) - .join( - FavStar, - and_( - FavStar.user_id == user_id, - FavStar.class_name == "slice", - Slice.id == FavStar.obj_id, - ), - isouter=True, - ) - .filter( # pylint: disable=comparison-with-callable - or_( - Slice.id.in_(owner_ids_query), - Slice.created_by_fk == user_id, - Slice.changed_by_fk == user_id, - FavStar.user_id == user_id, - ) - ) - .order_by(Slice.slice_name.asc()) - ) - payload = [ - { - "id": o.Slice.id, - "title": o.Slice.slice_name, - "url": o.Slice.slice_url, - "data": o.Slice.form_data, - "dttm": o.dttm if o.dttm else o.Slice.changed_on, - "viz_type": o.Slice.viz_type, - } - for o in qry.all() - ] - return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - - @api - @has_access_api - @event_logger.log_this - @expose("/created_slices", methods=("GET",)) - @expose("/created_slices//", methods=("GET",)) - @deprecated(new_target="api/v1/chart/") - def created_slices(self, user_id: int | None = None) -> FlaskResponse: - """List of slices created by this user""" - if not user_id: - user_id = cast(int, get_user_id()) - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - qry = ( - db.session.query(Slice) - .filter( # pylint: disable=comparison-with-callable - or_(Slice.created_by_fk == user_id, Slice.changed_by_fk == user_id) - ) - .order_by(Slice.changed_on.desc()) - ) - payload = [ - { - "id": o.id, - "title": o.slice_name, - "url": o.slice_url, - "dttm": o.changed_on, - "viz_type": o.viz_type, - } - for o in qry.all() - ] - return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - - @api - @has_access_api - @event_logger.log_this - @expose("/fave_slices", methods=("GET",)) - @expose("/fave_slices//", methods=("GET",)) - @deprecated(new_target="api/v1/chart/") - def fave_slices(self, user_id: int | None = None) -> FlaskResponse: - """Favorite slices for a user""" - if user_id is None: - user_id = cast(int, get_user_id()) - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - qry = ( - db.session.query(Slice, FavStar.dttm) - .join( - FavStar, - and_( - FavStar.user_id == user_id, - FavStar.class_name == "slice", - Slice.id == FavStar.obj_id, - ), - ) - .order_by(FavStar.dttm.desc()) - ) - payload = [] - for o in qry.all(): - dash = { - "id": o.Slice.id, - "title": o.Slice.slice_name, - "url": o.Slice.slice_url, - "dttm": o.dttm, - "viz_type": o.Slice.viz_type, - } - if o.Slice.created_by: - user = o.Slice.created_by - dash["creator"] = str(user) - dash["creator_url"] = f"/superset/profile/{user.username}/" - payload.append(dash) - return json_success(json.dumps(payload, default=utils.json_int_dttm_ser)) - @event_logger.log_this @api @has_access_api @@ -1158,42 +938,6 @@ def warm_up_cache( # pylint: disable=too-many-locals,no-self-use return json_success(json.dumps(result)) - @has_access_api - @event_logger.log_this - @expose("/favstar////") - @deprecated(new_target="api/v1/dashboard|chart//favorites/") - def favstar( # pylint: disable=no-self-use - self, class_name: str, obj_id: int, action: str - ) -> FlaskResponse: - """Toggle favorite stars on Slices and Dashboard""" - if not get_user_id(): - return json_error_response("ERROR: Favstar toggling denied", status=403) - session = db.session() - count = 0 - favs = ( - session.query(FavStar) - .filter_by(class_name=class_name, obj_id=obj_id, user_id=get_user_id()) - .all() - ) - if action == "select": - if not favs: - session.add( - FavStar( - class_name=class_name, - obj_id=obj_id, - user_id=get_user_id(), - dttm=datetime.now(), - ) - ) - count = 1 - elif action == "unselect": - for fav in favs: - session.delete(fav) - else: - count = len(favs) - session.commit() - return json_success(json.dumps({"count": count})) - @has_access @expose("/dashboard//") @event_logger.log_this_with_extra_payload @@ -1279,23 +1023,6 @@ def dashboard_permalink( # pylint: disable=no-self-use def log(self) -> FlaskResponse: # pylint: disable=no-self-use return Response(status=200) - @has_access - @expose("/extra_table_metadata////") - @event_logger.log_this - @deprecated( - new_target="api/v1/database//table_extra///" - ) - def extra_table_metadata( # pylint: disable=no-self-use - self, database_id: int, table_name: str, schema: str - ) -> FlaskResponse: - parsed_schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) - table_name = utils.parse_js_uri_path_item(table_name) # type: ignore - mydb = db.session.query(Database).filter_by(id=database_id).one() - payload = mydb.db_engine_spec.extra_table_metadata( - mydb, table_name, parsed_schema - ) - return json_success(json.dumps(payload)) - @expose("/theme/") def theme(self) -> FlaskResponse: return self.render_template("superset/theme.html") @@ -1363,27 +1090,17 @@ def welcome(self) -> FlaskResponse: @has_access @event_logger.log_this - @expose("/profile//") - def profile(self, username: str) -> FlaskResponse: + @expose("/profile/") + def profile(self) -> FlaskResponse: """User profile page""" - user = ( - db.session.query(ab_models.User).filter_by(username=username).one_or_none() - ) - # Prevent returning 404 when user is not found to prevent username scanning - user_id = -1 if not user else user.id - # Prevent unauthorized access to other user's profiles, - # unless configured to do so with ENABLE_BROAD_ACTIVITY_ACCESS - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj - payload = { - "user": bootstrap_user_data(user, include_perms=True), + "user": bootstrap_user_data(g.user, include_perms=True), "common": common_bootstrap_payload(g.user), } return self.render_template( "superset/basic.html", - title=_("%(user)s's profile", user=username).__str__(), + title=_("%(user)s's profile", user=g.user.username).__str__(), entry="profile", bootstrap_data=json.dumps( payload, default=utils.pessimistic_json_iso_dttm_ser diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 86f4a7e3f499a..4e1a7d9214e1b 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -23,6 +23,7 @@ import logging from urllib.parse import quote +import prison import superset.utils.database from superset.utils.core import backend from tests.integration_tests.fixtures.birth_names_dashboard import ( @@ -47,6 +48,7 @@ load_energy_table_with_slice, load_energy_table_data, ) +from tests.integration_tests.insert_chart_mixin import InsertChartMixin from tests.integration_tests.test_app import app import superset.views.utils from superset import ( @@ -89,7 +91,7 @@ def cleanup(): yield -class TestCore(SupersetTestCase): +class TestCore(SupersetTestCase, InsertChartMixin): def setUp(self): self.table_ids = { tbl.table_name: tbl.id for tbl in (db.session.query(SqlaTable).all()) @@ -100,6 +102,50 @@ def tearDown(self): db.session.query(Query).delete() app.config["PREVENT_UNSAFE_DB_CONNECTIONS"] = self.original_unsafe_db_setting + def insert_dashboard_created_by(self, username: str) -> Dashboard: + user = self.get_user(username) + dashboard = self.insert_dashboard( + f"create_title_test", + f"create_slug_test", + [user.id], + created_by=user, + ) + return dashboard + + def insert_chart_created_by(self, username: str) -> Slice: + user = self.get_user(username) + dataset = db.session.query(SqlaTable).first() + chart = self.insert_chart( + f"create_title_test", + [user.id], + dataset.id, + created_by=user, + ) + return chart + + @pytest.fixture() + def insert_dashboard_created_by_admin(self): + with self.create_app().app_context(): + dashboard = self.insert_dashboard_created_by("admin") + yield dashboard + db.session.delete(dashboard) + db.session.commit() + + @pytest.fixture() + def insert_dashboard_created_by_gamma(self): + dashboard = self.insert_dashboard_created_by("gamma") + yield dashboard + db.session.delete(dashboard) + db.session.commit() + + @pytest.fixture() + def insert_chart_created_by_admin(self): + with self.create_app().app_context(): + chart = self.insert_chart_created_by("admin") + yield chart + db.session.delete(chart) + db.session.commit() + def test_login(self): resp = self.get_resp("/login/", data=dict(username="admin", password="general")) self.assertNotIn("User confirmation needed", resp) @@ -262,43 +308,6 @@ def test_add_slice(self): resp = self.client.get(url) self.assertEqual(resp.status_code, 200) - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_get_user_slices_for_owners(self): - self.login(username="alpha") - user = security_manager.find_user("alpha") - slice_name = "Girls" - - # ensure user is not owner of any slices - url = f"/superset/user_slices/{user.id}/" - resp = self.client.get(url) - data = json.loads(resp.data) - self.assertEqual(data, []) - - # make user owner of slice and verify that endpoint returns said slice - slc = self.get_slice( - slice_name=slice_name, session=db.session, expunge_from_session=False - ) - slc.owners = [user] - db.session.merge(slc) - db.session.commit() - url = f"/superset/user_slices/{user.id}/" - resp = self.client.get(url) - data = json.loads(resp.data) - self.assertEqual(len(data), 1) - self.assertEqual(data[0]["title"], slice_name) - - # remove ownership and ensure user no longer gets slice - slc = self.get_slice( - slice_name=slice_name, session=db.session, expunge_from_session=False - ) - slc.owners = [] - db.session.merge(slc) - db.session.commit() - url = f"/superset/user_slices/{user.id}/" - resp = self.client.get(url) - data = json.loads(resp.data) - self.assertEqual(data, []) - def test_get_user_slices(self): self.login(username="admin") userid = security_manager.find_user("admin").id @@ -483,71 +492,91 @@ def test_fetch_datasource_metadata(self): for k in keys: self.assertIn(k, resp.keys()) - @staticmethod - def _get_user_activity_endpoints(user: str): - userid = security_manager.find_user(user).id - return ( - f"/superset/recent_activity/{userid}/", - f"/superset/created_slices/{userid}/", - f"/superset/created_dashboards/{userid}/", - f"/superset/fave_slices/{userid}/", - f"/superset/fave_dashboards/{userid}/", - f"/superset/user_slices/{userid}/", - f"/superset/fave_dashboards_by_username/{user}/", - ) - + @pytest.mark.usefixtures("insert_dashboard_created_by_admin") + @pytest.mark.usefixtures("insert_chart_created_by_admin") @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_user_profile(self, username="admin"): self.login(username=username) slc = self.get_slice("Girls", db.session) + dashboard = db.session.query(Dashboard).filter_by(slug="births").first() + # Set a favorite dashboard + self.client.post(f"/api/v1/dashboard/{dashboard.id}/favorites/", json={}) + # Set a favorite chart + self.client.post(f"/api/v1/chart/{slc.id}/favorites/", json={}) + + # Get favorite dashboards: + request_query = { + "columns": ["created_on_delta_humanized", "dashboard_title", "url"], + "filters": [{"col": "id", "opr": "dashboard_is_favorite", "value": True}], + "keys": ["none"], + "order_column": "changed_on", + "order_direction": "desc", + "page": 0, + "page_size": 100, + } + url = f"/api/v1/dashboard/?q={prison.dumps(request_query)}" + raw_data = self.client.get(url) + assert raw_data.json["count"] == 1 + assert raw_data.json["result"][0]["dashboard_title"] == "USA Births Names" + + # Get Favorite Charts + request_query = { + "filters": [{"col": "id", "opr": "chart_is_favorite", "value": True}], + "order_column": "slice_name", + "order_direction": "asc", + "page": 0, + "page_size": 25, + } + url = f"api/v1/chart/?q={prison.dumps(request_query)}" + raw_data = self.client.get(url) + assert raw_data.json["count"] == 1 + assert raw_data.json["result"][0]["id"] == slc.id + + # Get recent activity + url = "/api/v1/log/recent_activity/1/?q=(page_size:50)" + raw_data = self.client.get(url) + assert raw_data.json["result"] == [] + + # Get dashboards created by the user + request_query = { + "columns": ["created_on_delta_humanized", "dashboard_title", "url"], + "filters": [ + {"col": "created_by", "opr": "dashboard_created_by_me", "value": "me"} + ], + "keys": ["none"], + "order_column": "changed_on", + "order_direction": "desc", + "page": 0, + "page_size": 100, + } + url = f"/api/v1/dashboard/?q={prison.dumps(request_query)}" + raw_data = self.client.get(url) + assert raw_data.json["result"][0]["dashboard_title"] == "create_title_test" + + # Get charts created by the user + request_query = { + "columns": ["created_on_delta_humanized", "slice_name", "url"], + "filters": [ + {"col": "created_by", "opr": "chart_created_by_me", "value": "me"} + ], + "keys": ["none"], + "order_column": "changed_on_delta_humanized", + "order_direction": "desc", + "page": 0, + "page_size": 100, + } + url = f"/api/v1/chart/?q={prison.dumps(request_query)}" + raw_data = self.client.get(url) + assert raw_data.json["count"] == 1 + assert raw_data.json["result"][0]["slice_name"] == "create_title_test" - # Setting some faves - url = f"/superset/favstar/Slice/{slc.id}/select/" - resp = self.get_json_resp(url) - self.assertEqual(resp["count"], 1) - - dash = db.session.query(Dashboard).filter_by(slug="births").first() - url = f"/superset/favstar/Dashboard/{dash.id}/select/" - resp = self.get_json_resp(url) - self.assertEqual(resp["count"], 1) - - resp = self.get_resp(f"/superset/profile/{username}/") + resp = self.get_resp(f"/superset/profile/") self.assertIn('"app"', resp) - for endpoint in self._get_user_activity_endpoints(username): - data = self.get_json_resp(endpoint) - self.assertNotIn("message", data) - - def test_user_profile_default_access(self): - self.login(username="gamma") - resp = self.client.get(f"/superset/profile/admin/") - self.assertEqual(resp.status_code, 403) - - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=True) - def test_user_profile_broad_access(self): + def test_user_profile_gamma(self): self.login(username="gamma") - resp = self.client.get(f"/superset/profile/admin/") - self.assertEqual(resp.status_code, 200) - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_user_activity_default_access(self, username="gamma"): - self.login(username=username) - - for user in ("admin", "gamma"): - for endpoint in self._get_user_activity_endpoints(user): - resp = self.client.get(endpoint) - expected_status_code = 200 if user == username else 403 - assert resp.status_code == expected_status_code - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=True) - def test_user_activity_broad_access(self, username="gamma"): - self.login(username=username) - - for user in ("admin", "gamma"): - for endpoint in self._get_user_activity_endpoints(user): - resp = self.client.get(endpoint) - assert resp.status_code == 200 + resp = self.get_resp(f"/superset/profile/") + self.assertIn('"app"', resp) @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_slice_id_is_always_logged_correctly_on_web_request(self): @@ -1025,7 +1054,7 @@ def test_feature_flag_serialization(self): "/superset/sqllab", "/superset/welcome", f"/superset/dashboard/{dash_id}/", - "/superset/profile/admin/", + "/superset/profile/", f"/explore/?datasource_type=table&datasource_id={tbl_id}", ] for url in urls: diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index e7b85498a5a0c..cb96ceba95a3a 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1350,7 +1350,6 @@ def assert_can_gamma(self, perm_set): # make sure that user can create slices and dashboards self.assert_can_all("Dashboard", perm_set) self.assert_can_all("Chart", perm_set) - self.assertIn(("can_created_dashboards", "Superset"), perm_set) self.assertIn(("can_created_slices", "Superset"), perm_set) self.assertIn(("can_csv", "Superset"), perm_set) self.assertIn(("can_dashboard", "Superset"), perm_set) @@ -1358,7 +1357,6 @@ def assert_can_gamma(self, perm_set): self.assertIn(("can_share_chart", "Superset"), perm_set) self.assertIn(("can_share_dashboard", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) - self.assertIn(("can_fave_dashboards", "Superset"), perm_set) self.assertIn(("can_fave_slices", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), perm_set) @@ -1525,7 +1523,6 @@ def test_gamma_permissions(self): self.assert_cannot_write("UserDBModelView", gamma_perm_set) self.assert_cannot_write("RoleModelView", gamma_perm_set) - self.assertIn(("can_created_dashboards", "Superset"), gamma_perm_set) self.assertIn(("can_created_slices", "Superset"), gamma_perm_set) self.assertIn(("can_csv", "Superset"), gamma_perm_set) self.assertIn(("can_dashboard", "Superset"), gamma_perm_set) @@ -1533,7 +1530,6 @@ def test_gamma_permissions(self): self.assertIn(("can_share_chart", "Superset"), gamma_perm_set) self.assertIn(("can_share_dashboard", "Superset"), gamma_perm_set) self.assertIn(("can_explore_json", "Superset"), gamma_perm_set) - self.assertIn(("can_fave_dashboards", "Superset"), gamma_perm_set) self.assertIn(("can_fave_slices", "Superset"), gamma_perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), gamma_perm_set) From 4bcfb35c73df7eb54030d19a45c667506b54ff55 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 14 Jun 2023 17:03:48 +0100 Subject: [PATCH 2/8] fix lint and tests --- superset/views/base.py | 2 +- superset/views/core.py | 5 +---- tests/integration_tests/security_tests.py | 4 ---- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/superset/views/base.py b/superset/views/base.py index 54a843d5f3c84..db7c8cbac9d76 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -391,7 +391,7 @@ def menu_data(user: User) -> dict[str, Any]: "user_login_url": appbuilder.get_url_for_login, "user_profile_url": None if user.is_anonymous or is_feature_enabled("MENU_HIDE_USER_INFO") - else f"/superset/profile/", + else "/superset/profile/", "locale": session.get("locale", "en"), }, } diff --git a/superset/views/core.py b/superset/views/core.py index 7c1de9eabce95..c000954a07a60 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -30,9 +30,7 @@ has_access_api, permission_name, ) -from flask_appbuilder.security.sqla import models as ab_models from flask_babel import gettext as __, lazy_gettext as _ -from sqlalchemy import and_, or_ from sqlalchemy.exc import SQLAlchemyError from superset import ( @@ -69,7 +67,7 @@ from superset.explore.permalink.commands.get import GetExplorePermalinkCommand from superset.explore.permalink.exceptions import ExplorePermalinkGetFailedError from superset.extensions import async_query_manager, cache_manager -from superset.models.core import Database, FavStar +from superset.models.core import Database from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import Query, TabState @@ -93,7 +91,6 @@ json_error_response, json_success, ) -from superset.views.log.dao import LogDAO from superset.views.utils import ( bootstrap_user_data, check_datasource_perms, diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index cb96ceba95a3a..17a6a78548124 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1350,14 +1350,12 @@ def assert_can_gamma(self, perm_set): # make sure that user can create slices and dashboards self.assert_can_all("Dashboard", perm_set) self.assert_can_all("Chart", perm_set) - self.assertIn(("can_created_slices", "Superset"), perm_set) self.assertIn(("can_csv", "Superset"), perm_set) self.assertIn(("can_dashboard", "Superset"), perm_set) self.assertIn(("can_explore", "Superset"), perm_set) self.assertIn(("can_share_chart", "Superset"), perm_set) self.assertIn(("can_share_dashboard", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) - self.assertIn(("can_fave_slices", "Superset"), perm_set) self.assertIn(("can_explore_json", "Superset"), perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), perm_set) self.assert_can_menu("Databases", perm_set) @@ -1523,14 +1521,12 @@ def test_gamma_permissions(self): self.assert_cannot_write("UserDBModelView", gamma_perm_set) self.assert_cannot_write("RoleModelView", gamma_perm_set) - self.assertIn(("can_created_slices", "Superset"), gamma_perm_set) self.assertIn(("can_csv", "Superset"), gamma_perm_set) self.assertIn(("can_dashboard", "Superset"), gamma_perm_set) self.assertIn(("can_explore", "Superset"), gamma_perm_set) self.assertIn(("can_share_chart", "Superset"), gamma_perm_set) self.assertIn(("can_share_dashboard", "Superset"), gamma_perm_set) self.assertIn(("can_explore_json", "Superset"), gamma_perm_set) - self.assertIn(("can_fave_slices", "Superset"), gamma_perm_set) self.assertIn(("can_userinfo", "UserDBModelView"), gamma_perm_set) def test_views_are_secured(self): From 548e416ce3949088657b3c5f36069a3a9e76a871 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 14 Jun 2023 22:06:23 +0100 Subject: [PATCH 3/8] fix test --- UPDATING.md | 2 +- tests/integration_tests/core_tests.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 2da1efaad5bd2..9dbb44e167f94 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,7 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes -- [---](https://github.com/apache/superset/pull/---): Removed deprecated APIs `/superset/recent_activity/...`, `/superset/fave_dashboards_by_username/...`, `/superset/fave_dashboards/...`, `/superset/created_dashboards/...`, `/superset/user_slices/`, `/superset/created_slices/...`, `/superset/fave_slices/...`, `/superset/favstar/...`, +- [24400](https://github.com/apache/superset/pull/24400): Removed deprecated APIs `/superset/recent_activity/...`, `/superset/fave_dashboards_by_username/...`, `/superset/fave_dashboards/...`, `/superset/created_dashboards/...`, `/superset/user_slices/`, `/superset/created_slices/...`, `/superset/fave_slices/...`, `/superset/favstar/...`, - [24375](https://github.com/apache/superset/pull/24375): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz` - [24360](https://github.com/apache/superset/pull/24360): Removed deprecated APIs `/superset/stop_query/...`, `/superset/queries/...`, `/superset/search_queries` - [24353](https://github.com/apache/superset/pull/24353): Removed deprecated APIs `/copy_dash/int:dashboard_id/`, `/save_dash/int:dashboard_id/`, `/add_slices/int:dashboard_id/`. diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 4e1a7d9214e1b..bcfc2a79dd885 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -535,7 +535,9 @@ def test_user_profile(self, username="admin"): # Get recent activity url = "/api/v1/log/recent_activity/1/?q=(page_size:50)" raw_data = self.client.get(url) - assert raw_data.json["result"] == [] + # TODO data for recent activity varies for sqlite, we should be able to assert + # the returned data + assert raw_data.status == 200 # Get dashboards created by the user request_query = { From 3e21fed7750840b42296969725c7eff63499f760 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Wed, 14 Jun 2023 23:37:00 +0100 Subject: [PATCH 4/8] remove ENABLE_BROAD_ACTIVITY_ACCESS --- .../src/utils/featureFlags.ts | 1 - superset-frontend/src/SqlLab/fixtures.ts | 1 - .../PropertiesModal/PropertiesModal.test.tsx | 1 - .../src/pages/ChartList/index.tsx | 15 ++-------- .../DashboardList/DashboardList.test.jsx | 1 - .../src/pages/DashboardList/index.tsx | 18 ++---------- .../pages/DatasetList/DatasetList.test.tsx | 1 - .../src/pages/DatasetList/index.tsx | 1 - superset-frontend/src/types/Dataset.ts | 1 - superset-frontend/src/views/CRUD/types.ts | 1 - superset/charts/api.py | 2 -- superset/config.py | 1 - superset/connectors/sqla/models.py | 8 ------ superset/dashboards/api.py | 1 - superset/dashboards/schemas.py | 1 - superset/datasets/api.py | 1 - superset/models/dashboard.py | 8 ------ superset/models/filter_set.py | 8 ------ superset/models/slice.py | 8 ------ superset/security/manager.py | 17 ----------- superset/views/base.py | 1 - superset/views/log/api.py | 13 ++++----- superset/views/log/dao.py | 8 ++++-- tests/integration_tests/charts/api_tests.py | 2 -- tests/integration_tests/core_tests.py | 28 +++++++++---------- .../integration_tests/dashboards/api_tests.py | 3 -- tests/integration_tests/datasets/api_tests.py | 3 -- tests/integration_tests/log_api_tests.py | 23 ++++----------- 28 files changed, 35 insertions(+), 142 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 138f21258bbd9..48e54d18416a8 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -40,7 +40,6 @@ export enum FeatureFlag { EMBEDDABLE_CHARTS = 'EMBEDDABLE_CHARTS', EMBEDDED_SUPERSET = 'EMBEDDED_SUPERSET', ENABLE_ADVANCED_DATA_TYPES = 'ENABLE_ADVANCED_DATA_TYPES', - ENABLE_BROAD_ACTIVITY_ACCESS = 'ENABLE_BROAD_ACTIVITY_ACCESS', ENABLE_EXPLORE_DRAG_AND_DROP = 'ENABLE_EXPLORE_DRAG_AND_DROP', ENABLE_JAVASCRIPT_CONTROLS = 'ENABLE_JAVASCRIPT_CONTROLS', ENABLE_TEMPLATE_PROCESSING = 'ENABLE_TEMPLATE_PROCESSING', diff --git a/superset-frontend/src/SqlLab/fixtures.ts b/superset-frontend/src/SqlLab/fixtures.ts index e295b02366b1a..18faaebea25d7 100644 --- a/superset-frontend/src/SqlLab/fixtures.ts +++ b/superset-frontend/src/SqlLab/fixtures.ts @@ -710,7 +710,6 @@ export const testQuery: ISaveableDatasource = { export const mockdatasets = [...new Array(3)].map((_, i) => ({ changed_by_name: 'user', kind: i === 0 ? 'virtual' : 'physical', // ensure there is 1 virtual - changed_by_url: 'changed_by_url', changed_by: 'user', changed_on: new Date().toISOString(), database_name: `db ${i}`, diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx index a453d943907c1..386467e8de281 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx @@ -99,7 +99,6 @@ const dashboardInfo = { certification_details: 'Sample certification', changed_by: null, changed_by_name: '', - changed_by_url: '', changed_on: '2021-03-30T19:30:14.020942', charts: [ 'Vaccine Candidates per Country & Stage', diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx index dbdfcebff4ae8..c853c40d8f972 100644 --- a/superset-frontend/src/pages/ChartList/index.tsx +++ b/superset-frontend/src/pages/ChartList/index.tsx @@ -231,9 +231,6 @@ function ChartList(props: ChartListProps) { const canExport = hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT); const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; - const enableBroadUserAccess = isFeatureEnabled( - FeatureFlag.ENABLE_BROAD_ACTIVITY_ACCESS, - ); const handleBulkChartExport = (chartsToExport: Chart[]) => { const ids = chartsToExport.map(({ id }) => id); handleResourceExport('chart', ids, () => { @@ -415,17 +412,9 @@ function ChartList(props: ChartListProps) { { Cell: ({ row: { - original: { - last_saved_by: lastSavedBy, - changed_by_url: changedByUrl, - }, + original: { last_saved_by: lastSavedBy }, }, - }: any) => - enableBroadUserAccess ? ( - {changedByName(lastSavedBy)} - ) : ( - <>{changedByName(lastSavedBy)} - ), + }: any) => <>{changedByName(lastSavedBy)}, Header: t('Modified by'), accessor: 'last_saved_by.first_name', size: 'xl', diff --git a/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx b/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx index be04c323318ec..bd91faf614111 100644 --- a/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx +++ b/superset-frontend/src/pages/DashboardList/DashboardList.test.jsx @@ -58,7 +58,6 @@ const mockDashboards = [...new Array(3)].map((_, i) => ({ url: 'url', dashboard_title: `title ${i}`, changed_by_name: 'user', - changed_by_url: 'changed_by_url', changed_by_fk: 1, published: true, changed_on_utc: new Date().toISOString(), diff --git a/superset-frontend/src/pages/DashboardList/index.tsx b/superset-frontend/src/pages/DashboardList/index.tsx index 1808573bea03d..3db775e66ec82 100644 --- a/superset-frontend/src/pages/DashboardList/index.tsx +++ b/superset-frontend/src/pages/DashboardList/index.tsx @@ -83,7 +83,6 @@ interface DashboardListProps { interface Dashboard { changed_by_name: string; - changed_by_url: string; changed_on_delta_humanized: string; changed_by: string; dashboard_title: string; @@ -140,9 +139,6 @@ function DashboardList(props: DashboardListProps) { const [importingDashboard, showImportModal] = useState(false); const [passwordFields, setPasswordFields] = useState([]); const [preparingExport, setPreparingExport] = useState(false); - const enableBroadUserAccess = isFeatureEnabled( - FeatureFlag.ENABLE_BROAD_ACTIVITY_ACCESS, - ); const [sshTunnelPasswordFields, setSSHTunnelPasswordFields] = useState< string[] >([]); @@ -193,7 +189,6 @@ function DashboardList(props: DashboardListProps) { if (dashboard.id === json?.result?.id) { const { changed_by_name, - changed_by_url, changed_by, dashboard_title = '', slug = '', @@ -208,7 +203,6 @@ function DashboardList(props: DashboardListProps) { return { ...dashboard, changed_by_name, - changed_by_url, changed_by, dashboard_title, slug, @@ -310,17 +304,9 @@ function DashboardList(props: DashboardListProps) { { Cell: ({ row: { - original: { - changed_by_name: changedByName, - changed_by_url: changedByUrl, - }, + original: { changed_by_name: changedByName }, }, - }: any) => - enableBroadUserAccess ? ( - {changedByName} - ) : ( - <>{changedByName} - ), + }: any) => <>{changedByName}, Header: t('Modified by'), accessor: 'changed_by.first_name', size: 'xl', diff --git a/superset-frontend/src/pages/DatasetList/DatasetList.test.tsx b/superset-frontend/src/pages/DatasetList/DatasetList.test.tsx index 861624716e806..358a5fcfcca34 100644 --- a/superset-frontend/src/pages/DatasetList/DatasetList.test.tsx +++ b/superset-frontend/src/pages/DatasetList/DatasetList.test.tsx @@ -50,7 +50,6 @@ const datasetsEndpoint = 'glob:*/api/v1/dataset/?*'; const mockdatasets = [...new Array(3)].map((_, i) => ({ changed_by_name: 'user', kind: i === 0 ? 'virtual' : 'physical', // ensure there is 1 virtual - changed_by_url: 'changed_by_url', changed_by: 'user', changed_on: new Date().toISOString(), database_name: `db ${i}`, diff --git a/superset-frontend/src/pages/DatasetList/index.tsx b/superset-frontend/src/pages/DatasetList/index.tsx index fa006ad42502f..43913a828029c 100644 --- a/superset-frontend/src/pages/DatasetList/index.tsx +++ b/superset-frontend/src/pages/DatasetList/index.tsx @@ -109,7 +109,6 @@ const Actions = styled.div` type Dataset = { changed_by_name: string; - changed_by_url: string; changed_by: string; changed_on_delta_humanized: string; database: { diff --git a/superset-frontend/src/types/Dataset.ts b/superset-frontend/src/types/Dataset.ts index 7d69932f6f0dd..1bb14207d6d30 100644 --- a/superset-frontend/src/types/Dataset.ts +++ b/superset-frontend/src/types/Dataset.ts @@ -20,7 +20,6 @@ import Owner from './Owner'; export default interface Dataset { changed_by_name: string; - changed_by_url: string; changed_by: string; changed_on_delta_humanized: string; database: { diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts index 87748d1539711..20800b71ce474 100644 --- a/superset-frontend/src/views/CRUD/types.ts +++ b/superset-frontend/src/views/CRUD/types.ts @@ -55,7 +55,6 @@ export interface Dashboard { certified_by?: string; certification_details?: string; changed_by_name: string; - changed_by_url: string; changed_on_delta_humanized?: string; changed_on_utc?: string; changed_by: string; diff --git a/superset/charts/api.py b/superset/charts/api.py index 9c1756b2a537c..b52ccf84bd07f 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -157,7 +157,6 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "changed_by.first_name", "changed_by.last_name", "changed_by_name", - "changed_by_url", "changed_on_delta_humanized", "changed_on_dttm", "changed_on_utc", @@ -165,7 +164,6 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "created_by.id", "created_by.last_name", "created_by_name", - "created_by_url", "created_on_delta_humanized", "datasource_id", "datasource_name_text", diff --git a/superset/config.py b/superset/config.py index 53d399110d4b2..d62003991a8fb 100644 --- a/superset/config.py +++ b/superset/config.py @@ -476,7 +476,6 @@ class D3Format(TypedDict, total=False): "AVOID_COLORS_COLLISION": True, # Set to False to only allow viewing own recent activity # or to disallow users from viewing other users profile page - "ENABLE_BROAD_ACTIVITY_ACCESS": False, # Do not show user info or profile in the menu "MENU_HIDE_USER_INFO": False, } diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index edde2232056c6..fd3e9bb71acde 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -599,14 +599,6 @@ def changed_by_name(self) -> str: return "" return str(self.changed_by) - @property - def changed_by_url(self) -> str: - if not self.changed_by or not is_feature_enabled( - "ENABLE_BROAD_ACTIVITY_ACCESS" - ): - return "" - return f"/superset/profile/{self.changed_by.username}" - @property def connection(self) -> str: return str(self.database) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index be248f7877f5f..2c03c81fb662c 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -174,7 +174,6 @@ def ensure_thumbnails_enabled(self) -> Optional[Response]: "changed_by.last_name", "changed_by.id", "changed_by_name", - "changed_by_url", "changed_on_utc", "changed_on_delta_humanized", "created_on_delta_humanized", diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 846ed39e825cf..8d47d39cd3baa 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -194,7 +194,6 @@ class DashboardGetResponseSchema(Schema): metadata={"description": certification_details_description} ) changed_by_name = fields.String() - changed_by_url = fields.String() changed_by = fields.Nested(UserSchema(exclude=(["username"]))) changed_on = fields.DateTime() charts = fields.List(fields.String(metadata={"description": charts_description})) diff --git a/superset/datasets/api.py b/superset/datasets/api.py index b44bdf0121895..c043efc682174 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -101,7 +101,6 @@ class DatasetRestApi(BaseSupersetModelRestApi): "database.id", "database.database_name", "changed_by_name", - "changed_by_url", "changed_by.first_name", "changed_by.last_name", "changed_on_utc", diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 8d690d7914804..3adffd122dbc1 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -268,14 +268,6 @@ def changed_by_name(self) -> str: return "" return str(self.changed_by) - @property - def changed_by_url(self) -> str: - if not self.changed_by or not is_feature_enabled( - "ENABLE_BROAD_ACTIVITY_ACCESS" - ): - return "" - return f"/superset/profile/{self.changed_by.username}" - @property def data(self) -> dict[str, Any]: positions = self.position_json diff --git a/superset/models/filter_set.py b/superset/models/filter_set.py index 096935f99baee..4efc850794800 100644 --- a/superset/models/filter_set.py +++ b/superset/models/filter_set.py @@ -65,14 +65,6 @@ def changed_by_name(self) -> str: return "" return str(self.changed_by) - @property - def changed_by_url(self) -> str: - if not self.changed_by or not is_feature_enabled( - "ENABLE_BROAD_ACTIVITY_ACCESS" - ): - return "" - return f"/superset/profile/{self.changed_by.username}" - def to_dict(self) -> dict[str, Any]: return { "id": self.id, diff --git a/superset/models/slice.py b/superset/models/slice.py index 7cf8fec3f7fc3..ff7231b4376be 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -337,14 +337,6 @@ def created_by_url(self) -> str: return "" return f"/superset/profile/{self.created_by.username}" - @property - def changed_by_url(self) -> str: - if not self.changed_by or not is_feature_enabled( - "ENABLE_BROAD_ACTIVITY_ACCESS" - ): - return "" - return f"/superset/profile/{self.changed_by.username}" - @property def icons(self) -> str: return f""" diff --git a/superset/security/manager.py b/superset/security/manager.py index 1e01bc74177d2..99c15726f5a36 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1991,23 +1991,6 @@ def get_rls_cache_key(self, datasource: "BaseDatasource") -> list[str]: guest_rls = self.get_guest_rls_filters_str(datasource) return guest_rls + rls_str - @staticmethod - def raise_for_user_activity_access(user_id: int) -> None: - # pylint: disable=import-outside-toplevel - from superset.extensions import feature_flag_manager - - if not get_user_id() or ( - not feature_flag_manager.is_feature_enabled("ENABLE_BROAD_ACTIVITY_ACCESS") - and user_id != get_user_id() - ): - raise SupersetSecurityException( - SupersetError( - error_type=SupersetErrorType.USER_ACTIVITY_SECURITY_ACCESS_ERROR, - message="Access to user's activity data is restricted", - level=ErrorLevel.ERROR, - ) - ) - def raise_for_dashboard_access(self, dashboard: "Dashboard") -> None: """ Raise an exception if the user cannot access the dashboard. diff --git a/superset/views/base.py b/superset/views/base.py index db7c8cbac9d76..e66fea0a48c21 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -90,7 +90,6 @@ "SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE", "DISABLE_DATASET_SOURCE_EDIT", "ENABLE_JAVASCRIPT_CONTROLS", - "ENABLE_BROAD_ACTIVITY_ACCESS", "DEFAULT_SQLLAB_LIMIT", "DEFAULT_VIZ_TYPE", "SQL_MAX_ROW", diff --git a/superset/views/log/api.py b/superset/views/log/api.py index e218792c25970..cfea864b50f0b 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -23,9 +23,12 @@ import superset.models.core as models from superset import event_logger, security_manager +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP from superset.exceptions import SupersetSecurityException from superset.superset_typing import FlaskResponse +from superset.utils.core import get_user_id from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics +from superset.views.log import LogMixin from superset.views.log.dao import LogDAO from superset.views.log.schemas import ( get_recent_activity_schema, @@ -33,9 +36,6 @@ RecentActivitySchema, ) -from ...constants import MODEL_API_RW_METHOD_PERMISSION_MAP -from . import LogMixin - class LogRestApi(LogMixin, BaseSupersetModelRestApi): datamodel = SQLAInterface(models.Log) @@ -82,7 +82,7 @@ def get_user_activity_access_error(self, user_id: int) -> Optional[FlaskResponse return self.response(403, message=ex.message) return None - @expose("/recent_activity//", methods=("GET",)) + @expose("/recent_activity/", methods=("GET",)) @protect() @safe @statsd_metrics @@ -92,7 +92,7 @@ def get_user_activity_access_error(self, user_id: int) -> Optional[FlaskResponse f".recent_activity", log_to_statsd=False, ) - def recent_activity(self, user_id: int, **kwargs: Any) -> FlaskResponse: + def recent_activity(self, **kwargs: Any) -> FlaskResponse: """Get recent activity data for a user --- get: @@ -125,8 +125,7 @@ def recent_activity(self, user_id: int, **kwargs: Any) -> FlaskResponse: 500: $ref: '#/components/responses/500' """ - if error_obj := self.get_user_activity_access_error(user_id): - return error_obj + user_id = get_user_id() args = kwargs["rison"] page, page_size = self._sanitize_page_args(*self._handle_page_args(args)) diff --git a/superset/views/log/dao.py b/superset/views/log/dao.py index 87bc0817daf98..668176d17744f 100644 --- a/superset/views/log/dao.py +++ b/superset/views/log/dao.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. from datetime import datetime, timedelta -from typing import Any +from typing import Any, Optional import humanize from sqlalchemy import and_, or_ @@ -34,7 +34,11 @@ class LogDAO(BaseDAO): @staticmethod def get_recent_activity( - user_id: int, actions: list[str], distinct: bool, page: int, page_size: int + user_id: Optional[int], + actions: list[str], + distinct: bool, + page: int, + page_size: int, ) -> list[dict[str, Any]]: has_subject_title = or_( and_( diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 202c7987d6761..26065ac8e5d76 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -624,7 +624,6 @@ def test_chart_activity_access_disabled(self): model = db.session.query(Slice).get(chart_id) self.assertEqual(model.slice_name, new_name) - self.assertEqual(model.changed_by_url, "") db.session.delete(model) db.session.commit() @@ -648,7 +647,6 @@ def test_chart_activity_access_enabled(self): model = db.session.query(Slice).get(chart_id) self.assertEqual(model.slice_name, new_name) - self.assertEqual(model.changed_by_url, "/superset/profile/admin") db.session.delete(model) db.session.commit() diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index bcfc2a79dd885..06d5aa1f6ae7f 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -515,9 +515,9 @@ def test_user_profile(self, username="admin"): "page_size": 100, } url = f"/api/v1/dashboard/?q={prison.dumps(request_query)}" - raw_data = self.client.get(url) - assert raw_data.json["count"] == 1 - assert raw_data.json["result"][0]["dashboard_title"] == "USA Births Names" + resp = self.client.get(url) + assert resp.json["count"] == 1 + assert resp.json["result"][0]["dashboard_title"] == "USA Births Names" # Get Favorite Charts request_query = { @@ -528,16 +528,16 @@ def test_user_profile(self, username="admin"): "page_size": 25, } url = f"api/v1/chart/?q={prison.dumps(request_query)}" - raw_data = self.client.get(url) - assert raw_data.json["count"] == 1 - assert raw_data.json["result"][0]["id"] == slc.id + resp = self.client.get(url) + assert resp.json["count"] == 1 + assert resp.json["result"][0]["id"] == slc.id # Get recent activity - url = "/api/v1/log/recent_activity/1/?q=(page_size:50)" - raw_data = self.client.get(url) + url = "/api/v1/log/recent_activity/?q=(page_size:50)" + resp = self.client.get(url) # TODO data for recent activity varies for sqlite, we should be able to assert # the returned data - assert raw_data.status == 200 + assert resp.status_code == 200 # Get dashboards created by the user request_query = { @@ -552,8 +552,8 @@ def test_user_profile(self, username="admin"): "page_size": 100, } url = f"/api/v1/dashboard/?q={prison.dumps(request_query)}" - raw_data = self.client.get(url) - assert raw_data.json["result"][0]["dashboard_title"] == "create_title_test" + resp = self.client.get(url) + assert resp.json["result"][0]["dashboard_title"] == "create_title_test" # Get charts created by the user request_query = { @@ -568,9 +568,9 @@ def test_user_profile(self, username="admin"): "page_size": 100, } url = f"/api/v1/chart/?q={prison.dumps(request_query)}" - raw_data = self.client.get(url) - assert raw_data.json["count"] == 1 - assert raw_data.json["result"][0]["slice_name"] == "create_title_test" + resp = self.client.get(url) + assert resp.json["count"] == 1 + assert resp.json["result"][0]["slice_name"] == "create_title_test" resp = self.get_resp(f"/superset/profile/") self.assertIn('"app"', resp) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 420c66badfafa..0bcd2610d8c7f 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -368,7 +368,6 @@ def test_get_dashboard(self): "certification_details": None, "changed_by": None, "changed_by_name": "", - "changed_by_url": "", "charts": [], "created_by": { "id": 1, @@ -1344,7 +1343,6 @@ def test_dashboard_activity_access_disabled(self): model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model.dashboard_title, "title2") - self.assertEqual(model.changed_by_url, "") db.session.delete(model) db.session.commit() @@ -1367,7 +1365,6 @@ def test_dashboard_activity_access_enabled(self): model = db.session.query(Dashboard).get(dashboard_id) self.assertEqual(model.dashboard_title, "title2") - self.assertEqual(model.changed_by_url, "/superset/profile/admin") db.session.delete(model) db.session.commit() diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index 384f95458ac77..e955e17240f90 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -207,7 +207,6 @@ def test_get_dataset_list(self): expected_columns = [ "changed_by", "changed_by_name", - "changed_by_url", "changed_on_delta_humanized", "changed_on_utc", "database", @@ -1378,7 +1377,6 @@ def test_dataset_activity_access_enabled(self): current_dataset = [d for d in res if d["id"] == dataset.id][0] self.assertEqual(current_dataset["description"], "changed_description") - self.assertEqual(current_dataset["changed_by_url"], "/superset/profile/admin") db.session.delete(dataset) db.session.commit() @@ -1403,7 +1401,6 @@ def test_dataset_activity_access_disabled(self): current_dataset = [d for d in res if d["id"] == dataset.id][0] self.assertEqual(current_dataset["description"], "changed_description") - self.assertEqual(current_dataset["changed_by_url"], "") db.session.delete(dataset) db.session.commit() diff --git a/tests/integration_tests/log_api_tests.py b/tests/integration_tests/log_api_tests.py index 2555354d5f437..6a18ea926ffd3 100644 --- a/tests/integration_tests/log_api_tests.py +++ b/tests/integration_tests/log_api_tests.py @@ -159,19 +159,6 @@ def test_update_log(self): db.session.delete(log) db.session.commit() - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=False) - def test_get_recent_activity_no_broad_access(self): - """ - Log API: Test recent activity not visible for other users without - ENABLE_BROAD_ACTIVITY_ACCESS flag on - """ - admin_user = self.get_user("admin") - self.login(username="admin") - - uri = f"api/v1/log/recent_activity/{admin_user.id + 1}/" - rv = self.client.get(uri) - self.assertEqual(rv.status_code, 403) - def test_get_recent_activity(self): """ Log API: Test recent activity endpoint @@ -182,7 +169,7 @@ def test_get_recent_activity(self): log1 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) - uri = f"api/v1/log/recent_activity/{admin_user.id}/" + uri = f"api/v1/log/recent_activity/" rv = self.client.get(uri) self.assertEqual(rv.status_code, 200) response = json.loads(rv.data.decode("utf-8")) @@ -219,7 +206,7 @@ def test_get_recent_activity_actions_filter(self): log2 = self.insert_log("explore", admin_user, dashboard_id=dash.id) arguments = {"actions": ["dashboard"]} - uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" rv = self.client.get(uri) db.session.delete(log) @@ -244,7 +231,7 @@ def test_get_recent_activity_distinct_false(self): log2 = self.insert_log("dashboard", admin_user, dashboard_id=dash.id) arguments = {"distinct": False} - uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" rv = self.client.get(uri) db.session.delete(log) @@ -274,7 +261,7 @@ def test_get_recent_activity_pagination(self): log.dttm = now - timedelta(days=2) arguments = {"page": 0, "page_size": 2} - uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" rv = self.client.get(uri) self.assertEqual(rv.status_code, 200) @@ -304,7 +291,7 @@ def test_get_recent_activity_pagination(self): ) arguments = {"page": 1, "page_size": 2} - uri = f"api/v1/log/recent_activity/{admin_user.id}/?q={prison.dumps(arguments)}" + uri = f"api/v1/log/recent_activity/?q={prison.dumps(arguments)}" rv = self.client.get(uri) db.session.delete(log) From e50b6db3eef25f43c5c6039919435ea8528d1e1f Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 15 Jun 2023 00:25:47 +0100 Subject: [PATCH 5/8] more removal --- superset/models/filter_set.py | 2 +- tests/integration_tests/charts/api_tests.py | 46 ------------------ .../integration_tests/dashboards/api_tests.py | 44 ----------------- tests/integration_tests/datasets/api_tests.py | 48 ------------------- 4 files changed, 1 insertion(+), 139 deletions(-) diff --git a/superset/models/filter_set.py b/superset/models/filter_set.py index 4efc850794800..e2b19f32a04cb 100644 --- a/superset/models/filter_set.py +++ b/superset/models/filter_set.py @@ -25,7 +25,7 @@ from sqlalchemy.orm import relationship from sqlalchemy_utils import generic_relationship -from superset import app, db, is_feature_enabled +from superset import app, db from superset.models.helpers import AuditMixinNullable metadata = Model.metadata # pylint: disable=no-member diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 26065ac8e5d76..60633c8894471 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -605,52 +605,6 @@ def test_update_chart(self): db.session.delete(model) db.session.commit() - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=False) - def test_chart_activity_access_disabled(self): - """ - Chart API: Test ENABLE_BROAD_ACTIVITY_ACCESS = False - """ - admin = self.get_user("admin") - birth_names_table_id = SupersetTestCase.get_table(name="birth_names").id - chart_id = self.insert_chart("title", [admin.id], birth_names_table_id).id - chart_data = { - "slice_name": (new_name := "title1_changed"), - } - self.login(username="admin") - uri = f"api/v1/chart/{chart_id}" - rv = self.put_assert_metric(uri, chart_data, "put") - self.assertEqual(rv.status_code, 200) - model = db.session.query(Slice).get(chart_id) - - self.assertEqual(model.slice_name, new_name) - - db.session.delete(model) - db.session.commit() - - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=True) - def test_chart_activity_access_enabled(self): - """ - Chart API: Test ENABLE_BROAD_ACTIVITY_ACCESS = True - """ - admin = self.get_user("admin") - birth_names_table_id = SupersetTestCase.get_table(name="birth_names").id - chart_id = self.insert_chart("title", [admin.id], birth_names_table_id).id - chart_data = { - "slice_name": (new_name := "title1_changed"), - } - self.login(username="admin") - uri = f"api/v1/chart/{chart_id}" - rv = self.put_assert_metric(uri, chart_data, "put") - self.assertEqual(rv.status_code, 200) - model = db.session.query(Slice).get(chart_id) - - self.assertEqual(model.slice_name, new_name) - - db.session.delete(model) - db.session.commit() - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_chart_get_list_no_username(self): """ diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 0bcd2610d8c7f..f676e873b700f 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -1325,50 +1325,6 @@ def test_update_dashboard(self): db.session.delete(model) db.session.commit() - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=False) - def test_dashboard_activity_access_disabled(self): - """ - Dashboard API: Test ENABLE_BROAD_ACTIVITY_ACCESS = False - """ - admin = self.get_user("admin") - admin_role = self.get_role("Admin") - dashboard_id = self.insert_dashboard( - "title1", "slug1", [admin.id], roles=[admin_role.id] - ).id - self.login(username="admin") - uri = f"api/v1/dashboard/{dashboard_id}" - dashboard_data = {"dashboard_title": "title2"} - rv = self.client.put(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 200) - model = db.session.query(Dashboard).get(dashboard_id) - - self.assertEqual(model.dashboard_title, "title2") - - db.session.delete(model) - db.session.commit() - - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=True) - def test_dashboard_activity_access_enabled(self): - """ - Dashboard API: Test ENABLE_BROAD_ACTIVITY_ACCESS = True - """ - admin = self.get_user("admin") - admin_role = self.get_role("Admin") - dashboard_id = self.insert_dashboard( - "title1", "slug1", [admin.id], roles=[admin_role.id] - ).id - self.login(username="admin") - uri = f"api/v1/dashboard/{dashboard_id}" - dashboard_data = {"dashboard_title": "title2"} - rv = self.client.put(uri, json=dashboard_data) - self.assertEqual(rv.status_code, 200) - model = db.session.query(Dashboard).get(dashboard_id) - - self.assertEqual(model.dashboard_title, "title2") - - db.session.delete(model) - db.session.commit() - def test_dashboard_get_list_no_username(self): """ Dashboard API: Tests that no username is returned diff --git a/tests/integration_tests/datasets/api_tests.py b/tests/integration_tests/datasets/api_tests.py index e955e17240f90..02ef23f0b8d28 100644 --- a/tests/integration_tests/datasets/api_tests.py +++ b/tests/integration_tests/datasets/api_tests.py @@ -1357,54 +1357,6 @@ def test_dataset_get_no_username(self): db.session.delete(dataset) db.session.commit() - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=True) - def test_dataset_activity_access_enabled(self): - """ - Dataset API: Test ENABLE_BROAD_ACTIVITY_ACCESS = True - """ - if backend() == "sqlite": - return - - dataset = self.insert_default_dataset() - self.login(username="admin") - table_data = {"description": "changed_description"} - uri = f"api/v1/dataset/{dataset.id}" - rv = self.client.put(uri, json=table_data) - self.assertEqual(rv.status_code, 200) - - response = self.get_assert_metric("api/v1/dataset/", "get_list") - res = json.loads(response.data.decode("utf-8"))["result"] - - current_dataset = [d for d in res if d["id"] == dataset.id][0] - self.assertEqual(current_dataset["description"], "changed_description") - - db.session.delete(dataset) - db.session.commit() - - @with_feature_flags(ENABLE_BROAD_ACTIVITY_ACCESS=False) - def test_dataset_activity_access_disabled(self): - """ - Dataset API: Test ENABLE_BROAD_ACTIVITY_ACCESS = Fase - """ - if backend() == "sqlite": - return - - dataset = self.insert_default_dataset() - self.login(username="admin") - table_data = {"description": "changed_description"} - uri = f"api/v1/dataset/{dataset.id}" - rv = self.put_assert_metric(uri, table_data, "put") - self.assertEqual(rv.status_code, 200) - - response = self.get_assert_metric("api/v1/dataset/", "get_list") - res = json.loads(response.data.decode("utf-8"))["result"] - - current_dataset = [d for d in res if d["id"] == dataset.id][0] - self.assertEqual(current_dataset["description"], "changed_description") - - db.session.delete(dataset) - db.session.commit() - def test_update_dataset_item_not_owned(self): """ Dataset API: Test update dataset item not owned From fbd8ddba406545f32bf3bf11dee16fe8c9e1b11c Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 15 Jun 2023 02:17:41 +0100 Subject: [PATCH 6/8] address comments, add anonymous and guest cases on profile --- superset/views/core.py | 27 ++++++++++++--------------- superset/views/log/api.py | 6 +----- superset/views/log/dao.py | 5 +++-- tests/integration_tests/core_tests.py | 7 +++++++ 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index c000954a07a60..ed7cd25c7d258 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -77,7 +77,12 @@ from superset.utils import core as utils from superset.utils.async_query_manager import AsyncQueryTokenException from superset.utils.cache import etag_cache -from superset.utils.core import DatasourceType, get_user_id, ReservedUrlParameters +from superset.utils.core import ( + DatasourceType, + get_user_id, + get_username, + ReservedUrlParameters, +) from superset.views.base import ( api, BaseSupersetView, @@ -826,17 +831,6 @@ def save_or_overwrite_slice( return json_success(json.dumps(response)) - @staticmethod - def get_user_activity_access_error(user_id: int) -> FlaskResponse | None: - try: - security_manager.raise_for_user_activity_access(user_id) - except SupersetSecurityException as ex: - return json_error_response( - ex.message, - status=403, - ) - return None - @event_logger.log_this @api @has_access_api @@ -1090,14 +1084,17 @@ def welcome(self) -> FlaskResponse: @expose("/profile/") def profile(self) -> FlaskResponse: """User profile page""" + user = g.user if hasattr(g, "user") and g.user else None + if not user or security_manager.is_guest_user(user) or user.is_anonymous: + abort(404) payload = { - "user": bootstrap_user_data(g.user, include_perms=True), - "common": common_bootstrap_payload(g.user), + "user": bootstrap_user_data(user, include_perms=True), + "common": common_bootstrap_payload(user), } return self.render_template( "superset/basic.html", - title=_("%(user)s's profile", user=g.user.username).__str__(), + title=_("%(user)s's profile", user=get_username()).__str__(), entry="profile", bootstrap_data=json.dumps( payload, default=utils.pessimistic_json_iso_dttm_ser diff --git a/superset/views/log/api.py b/superset/views/log/api.py index cfea864b50f0b..c94156fd83ea9 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -125,15 +125,11 @@ def recent_activity(self, **kwargs: Any) -> FlaskResponse: 500: $ref: '#/components/responses/500' """ - user_id = get_user_id() - args = kwargs["rison"] page, page_size = self._sanitize_page_args(*self._handle_page_args(args)) actions = args.get("actions", ["explore", "dashboard"]) distinct = args.get("distinct", True) - payload = LogDAO.get_recent_activity( - user_id, actions, distinct, page, page_size - ) + payload = LogDAO.get_recent_activity(actions, distinct, page, page_size) return self.response(200, result=payload) diff --git a/superset/views/log/dao.py b/superset/views/log/dao.py index 668176d17744f..ab2961ade694a 100644 --- a/superset/views/log/dao.py +++ b/superset/views/log/dao.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. from datetime import datetime, timedelta -from typing import Any, Optional +from typing import Any import humanize from sqlalchemy import and_, or_ @@ -26,6 +26,7 @@ from superset.models.core import Log from superset.models.dashboard import Dashboard from superset.models.slice import Slice +from superset.utils.core import get_user_id from superset.utils.dates import datetime_to_epoch @@ -34,12 +35,12 @@ class LogDAO(BaseDAO): @staticmethod def get_recent_activity( - user_id: Optional[int], actions: list[str], distinct: bool, page: int, page_size: int, ) -> list[dict[str, Any]]: + user_id = get_user_id() has_subject_title = or_( and_( Dashboard.dashboard_title is not None, diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 06d5aa1f6ae7f..c96555503b598 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -26,6 +26,7 @@ import prison import superset.utils.database from superset.utils.core import backend +from tests.integration_tests.fixtures.public_role import public_role_like_gamma from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, load_birth_names_data, @@ -580,6 +581,12 @@ def test_user_profile_gamma(self): resp = self.get_resp(f"/superset/profile/") self.assertIn('"app"', resp) + @pytest.mark.usefixtures("public_role_like_gamma") + def test_user_profile_anonymous(self): + self.logout() + resp = self.client.get("/superset/profile/") + assert resp.status_code == 404 + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_slice_id_is_always_logged_correctly_on_web_request(self): # explore case From 7c824aad5578f14e10b0ddb32021e1783487c4a3 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 15 Jun 2023 02:27:25 +0100 Subject: [PATCH 7/8] fix lint --- superset/views/core.py | 7 +------ superset/views/log/api.py | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index ed7cd25c7d258..6af3d500ccce9 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -55,12 +55,7 @@ from superset.databases.dao import DatabaseDAO from superset.datasets.commands.exceptions import DatasetNotFoundError from superset.datasource.dao import DatasourceDAO -from superset.exceptions import ( - CacheLoadError, - DatabaseNotFound, - SupersetException, - SupersetSecurityException, -) +from superset.exceptions import CacheLoadError, DatabaseNotFound, SupersetException from superset.explore.form_data.commands.create import CreateFormDataCommand from superset.explore.form_data.commands.get import GetFormDataCommand from superset.explore.form_data.commands.parameters import CommandParameters diff --git a/superset/views/log/api.py b/superset/views/log/api.py index c94156fd83ea9..eefa45b5fdfba 100644 --- a/superset/views/log/api.py +++ b/superset/views/log/api.py @@ -26,7 +26,6 @@ from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP from superset.exceptions import SupersetSecurityException from superset.superset_typing import FlaskResponse -from superset.utils.core import get_user_id from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics from superset.views.log import LogMixin from superset.views.log.dao import LogDAO From 63ea2bdd07daa482b4b47298e4ae9f0b31afa0ee Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Thu, 15 Jun 2023 11:18:20 +0100 Subject: [PATCH 8/8] remove created_by_url --- superset-frontend/src/profile/components/Favorites.tsx | 2 +- superset-frontend/src/profile/types.ts | 1 - superset/models/slice.py | 6 ------ 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/superset-frontend/src/profile/components/Favorites.tsx b/superset-frontend/src/profile/components/Favorites.tsx index 1a52c8b047837..834f933071676 100644 --- a/superset-frontend/src/profile/components/Favorites.tsx +++ b/superset-frontend/src/profile/components/Favorites.tsx @@ -33,7 +33,7 @@ export default class Favorites extends React.PureComponent { const mutator = (payload: { result: Chart[] }) => payload.result.map(slice => ({ slice: {slice.slice_name}, - creator: {slice.created_by_name}, + creator: slice.created_by_name, favorited: moment.utc(slice.changed_on_dttm).fromNow(), _favorited: slice.changed_on_dttm, })); diff --git a/superset-frontend/src/profile/types.ts b/superset-frontend/src/profile/types.ts index 1a4dc784a04af..370434dfe817d 100644 --- a/superset-frontend/src/profile/types.ts +++ b/superset-frontend/src/profile/types.ts @@ -31,7 +31,6 @@ export type Chart = { slice_name: string; slice_url: string; created_by_name?: string; - created_by_url?: string; changed_on_dttm: number; }; diff --git a/superset/models/slice.py b/superset/models/slice.py index ff7231b4376be..eb48cc148a5a5 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -331,12 +331,6 @@ def slice_link(self) -> Markup: name = escape(self.chart) return Markup(f'{name}') - @property - def created_by_url(self) -> str: - if not self.created_by: - return "" - return f"/superset/profile/{self.created_by.username}" - @property def icons(self) -> str: return f"""