From 4c5fb9d534c5839e1a5eb6d9ed27b32c3c5e94a7 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Mon, 5 Sep 2022 13:32:48 +0100 Subject: [PATCH] fix: disallow users from viewing other user's profile on config (#21302) --- .../src/views/CRUD/chart/ChartList.tsx | 22 ++++++++++++------- .../views/CRUD/dashboard/DashboardList.tsx | 10 ++++++++- superset/config.py | 1 + superset/views/base.py | 1 + superset/views/core.py | 9 ++++++-- tests/integration_tests/core_tests.py | 12 ++++++++++ 6 files changed, 44 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx index 637e9b6ea72c0..1e2298538673d 100644 --- a/superset-frontend/src/views/CRUD/chart/ChartList.tsx +++ b/superset-frontend/src/views/CRUD/chart/ChartList.tsx @@ -60,6 +60,8 @@ import { nativeFilterGate } from 'src/dashboard/components/nativeFilters/utils'; import setupPlugins from 'src/setup/setupPlugins'; import InfoTooltip from 'src/components/InfoTooltip'; import CertifiedBadge from 'src/components/CertifiedBadge'; +import { bootstrapData } from 'src/preamble'; +import Owner from 'src/types/Owner'; import ChartCard from './ChartCard'; const FlexRowContainer = styled.div` @@ -209,7 +211,8 @@ function ChartList(props: ChartListProps) { const canExport = hasPerm('can_export') && isFeatureEnabled(FeatureFlag.VERSIONED_EXPORT); const initialSort = [{ id: 'changed_on_delta_humanized', desc: true }]; - + const enableBroadUserAccess = + bootstrapData?.common?.conf?.ENABLE_BROAD_ACTIVITY_ACCESS; const handleBulkChartExport = (chartsToExport: Chart[]) => { const ids = chartsToExport.map(({ id }) => id); handleResourceExport('chart', ids, () => { @@ -217,6 +220,10 @@ function ChartList(props: ChartListProps) { }); setPreparingExport(true); }; + const changedByName = (lastSavedBy: Owner) => + lastSavedBy?.first_name + ? `${lastSavedBy?.first_name} ${lastSavedBy?.last_name}` + : null; function handleBulkChartDelete(chartsToDelete: Chart[]) { SupersetClient.delete({ @@ -321,13 +328,12 @@ function ChartList(props: ChartListProps) { changed_by_url: changedByUrl, }, }, - }: any) => ( - - {lastSavedBy?.first_name - ? `${lastSavedBy?.first_name} ${lastSavedBy?.last_name}` - : null} - - ), + }: any) => + enableBroadUserAccess ? ( + {changedByName(lastSavedBy)} + ) : ( + <>{changedByName(lastSavedBy)} + ), Header: t('Modified by'), accessor: 'last_saved_by.first_name', size: 'xl', diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx index 7dbb30159d91e..8569f840d3688 100644 --- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx +++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx @@ -49,6 +49,7 @@ import ImportModelsModal from 'src/components/ImportModal/index'; import Dashboard from 'src/dashboard/containers/Dashboard'; import CertifiedBadge from 'src/components/CertifiedBadge'; +import { bootstrapData } from 'src/preamble'; import DashboardCard from './DashboardCard'; import { DashboardStatus } from './types'; @@ -132,6 +133,8 @@ function DashboardList(props: DashboardListProps) { const [importingDashboard, showImportModal] = useState(false); const [passwordFields, setPasswordFields] = useState([]); const [preparingExport, setPreparingExport] = useState(false); + const enableBroadUserAccess = + bootstrapData?.common?.conf?.ENABLE_BROAD_ACTIVITY_ACCESS; const openDashboardImportModal = () => { showImportModal(true); @@ -290,7 +293,12 @@ function DashboardList(props: DashboardListProps) { changed_by_url: changedByUrl, }, }, - }: any) => {changedByName}, + }: any) => + enableBroadUserAccess ? ( + {changedByName} + ) : ( + <>{changedByName} + ), Header: t('Modified by'), accessor: 'changed_by.first_name', size: 'xl', diff --git a/superset/config.py b/superset/config.py index bae75fed6ee46..c5f59e365fb08 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1289,6 +1289,7 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument MENU_HIDE_USER_INFO = False # Set to False to only allow viewing own recent activity +# or to disallow users from viewing other users profile page ENABLE_BROAD_ACTIVITY_ACCESS = True # the advanced data type key should correspond to that set in the column metadata diff --git a/superset/views/base.py b/superset/views/base.py index 9460b8d1aee69..42ec46a70601b 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -86,6 +86,7 @@ "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/core.py b/superset/views/core.py index f65385fc305a5..fd7539e916d8a 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2693,8 +2693,13 @@ def profile(self, username: str) -> FlaskResponse: user = ( db.session.query(ab_models.User).filter_by(username=username).one_or_none() ) - if not user: - abort(404, description=f"User: {username} does not exist.") + # 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 on with ENABLE_BROAD_ACTIVITY_ACCESS + error_obj = self.get_user_activity_access_error(user_id) + if error_obj: + return error_obj payload = { "user": bootstrap_user_data(user, include_perms=True), diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 58943246c545b..f0d79253345c5 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -851,6 +851,18 @@ def test_user_profile(self, username="admin"): data = self.get_json_resp(endpoint) self.assertNotIn("message", data) + def test_user_profile_optional_access(self): + self.login(username="gamma") + resp = self.client.get(f"/superset/profile/admin/") + self.assertEqual(resp.status_code, 200) + + app.config["ENABLE_BROAD_ACTIVITY_ACCESS"] = False + resp = self.client.get(f"/superset/profile/admin/") + self.assertEqual(resp.status_code, 403) + + # Restore config + app.config["ENABLE_BROAD_ACTIVITY_ACCESS"] = True + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_user_activity_access(self, username="gamma"): self.login(username=username)