You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2022/09/06 17:14:51 UTC
[superset] 03/04: fix: disallow users from viewing other user's profile on config (#21302)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8e7fb96f06be236da7376dc21157d6a1cb462afe
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 | 23 ++++++++++++++--------
.../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, 45 insertions(+), 11 deletions(-)
diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx
index 2645aa41c7..bf032cd1f2 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`
@@ -206,7 +208,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, () => {
@@ -215,6 +218,11 @@ 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({
endpoint: `/api/v1/chart/?q=${rison.encode(
@@ -320,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 6b4bb04b1c..3ea92dabfd 100644
--- a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
+++ b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
@@ -50,6 +50,7 @@ import OmniContainer from 'src/components/OmniContainer';
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';
@@ -133,6 +134,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 ccfb83c17b..206e0d815d 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -1365,6 +1365,7 @@ SQLALCHEMY_DOCS_URL = "https://docs.sqlalchemy.org/en/13/core/engines.html"
SQLALCHEMY_DISPLAY_TEXT = "SQLAlchemy docs"
# 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
# -------------------------------------------------------------------
diff --git a/superset/views/base.py b/superset/views/base.py
index 173ba5eb19..2fa98cb69d 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -91,6 +91,7 @@ FRONTEND_CONF_KEYS = (
"DISABLE_DATASET_SOURCE_EDIT",
"DRUID_IS_ACTIVE",
"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 81e04c535f..a4acaaa8bf 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -2872,8 +2872,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 5c2b81b283..3cba31dafd 100644
--- a/tests/integration_tests/core_tests.py
+++ b/tests/integration_tests/core_tests.py
@@ -865,6 +865,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)