You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2022/09/20 20:50:39 UTC
[superset] 29/29: fix: disallow users from viewing other user's profile on config (#21302)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 2.0-test
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 79351a4d92834b2f5692e44d0b66b9c5246df0d0
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Mon Sep 5 13:32:48 2022 +0100
fix: disallow users from viewing other user's profile on config (#21302)
---
.../src/views/CRUD/chart/ChartList.tsx | 22 ++++++++++++++--------
.../src/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 637e9b6ea7..1e22985386 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) => (
- <a href={changedByUrl}>
- {lastSavedBy?.first_name
- ? `${lastSavedBy?.first_name} ${lastSavedBy?.last_name}`
- : null}
- </a>
- ),
+ }: any) =>
+ enableBroadUserAccess ? (
+ <a href={changedByUrl}>{changedByName(lastSavedBy)}</a>
+ ) : (
+ <>{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 7dbb30159d..8569f840d3 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<boolean>(false);
const [passwordFields, setPasswordFields] = useState<string[]>([]);
const [preparingExport, setPreparingExport] = useState<boolean>(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) => <a href={changedByUrl}>{changedByName}</a>,
+ }: any) =>
+ enableBroadUserAccess ? (
+ <a href={changedByUrl}>{changedByName}</a>
+ ) : (
+ <>{changedByName}</>
+ ),
Header: t('Modified by'),
accessor: 'changed_by.first_name',
size: 'xl',
diff --git a/superset/config.py b/superset/config.py
index bae75fed6e..c5f59e365f 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1289,6 +1289,7 @@ DATASET_HEALTH_CHECK: Optional[Callable[["SqlaTable"], str]] = None
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 9460b8d1ae..42ec46a706 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -86,6 +86,7 @@ FRONTEND_CONF_KEYS = (
"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 f65385fc30..fd7539e916 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -2693,8 +2693,13 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
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 58943246c5..f0d7925334 100644
--- a/tests/integration_tests/core_tests.py
+++ b/tests/integration_tests/core_tests.py
@@ -851,6 +851,18 @@ class TestCore(SupersetTestCase):
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)