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:48 UTC
[superset] branch 1.5 updated (7ad097371e -> 37bdf434bf)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 1.5
in repository https://gitbox.apache.org/repos/asf/superset.git
from 7ad097371e fix: Adds yeoman-environment to dev dependencies
new 9861c2b9e7 fix(celery cache warmup): add auth and use warm_up_cache endpoint (#21076)
new 85fb2433ab feat: adds TLS certificate validation option for SMTP (#21272)
new 8e7fb96f06 fix: disallow users from viewing other user's profile on config (#21302)
new 37bdf434bf fix: Connect database from top right corner button doesn't work
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
docker/pythonpath_dev/superset_config.py | 10 ++
docs/docs/installation/alerts-reports.mdx | 1 +
.../src/views/CRUD/chart/ChartList.tsx | 23 ++--
.../src/views/CRUD/dashboard/DashboardList.tsx | 10 +-
.../src/views/components/MenuRight.tsx | 2 +-
superset/config.py | 5 +-
superset/tasks/cache.py | 98 +++++++-------
superset/utils/core.py | 34 +++--
superset/views/base.py | 1 +
superset/views/core.py | 9 +-
tests/integration_tests/core_tests.py | 12 ++
tests/integration_tests/email_tests.py | 29 ++++-
tests/integration_tests/strategy_tests.py | 141 ++-------------------
tests/integration_tests/superset_test_config.py | 2 +
14 files changed, 168 insertions(+), 209 deletions(-)
[superset] 01/04: fix(celery cache warmup): add auth and use warm_up_cache endpoint (#21076)
Posted by mi...@apache.org.
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 9861c2b9e7fc5bf571c07511b4e7aa702808d7d4
Author: ʈᵃᵢ <td...@gmail.com>
AuthorDate: Tue Aug 30 09:24:24 2022 -0700
fix(celery cache warmup): add auth and use warm_up_cache endpoint (#21076)
(cherry picked from commit 04dd8d414db6a3cddcd073ad74acb2a4b7a53b0b)
---
docker/pythonpath_dev/superset_config.py | 10 ++
superset/tasks/cache.py | 98 ++++++++--------
tests/integration_tests/strategy_tests.py | 141 +++---------------------
tests/integration_tests/superset_test_config.py | 2 +
4 files changed, 70 insertions(+), 181 deletions(-)
diff --git a/docker/pythonpath_dev/superset_config.py b/docker/pythonpath_dev/superset_config.py
index 6c58bec79c..1c78baf59f 100644
--- a/docker/pythonpath_dev/superset_config.py
+++ b/docker/pythonpath_dev/superset_config.py
@@ -69,6 +69,16 @@ REDIS_RESULTS_DB = get_env_variable("REDIS_RESULTS_DB", "1")
RESULTS_BACKEND = FileSystemCache("/app/superset_home/sqllab")
+CACHE_CONFIG = {
+ "CACHE_TYPE": "redis",
+ "CACHE_DEFAULT_TIMEOUT": 300,
+ "CACHE_KEY_PREFIX": "superset_",
+ "CACHE_REDIS_HOST": REDIS_HOST,
+ "CACHE_REDIS_PORT": REDIS_PORT,
+ "CACHE_REDIS_DB": REDIS_RESULTS_DB,
+}
+DATA_CACHE_CONFIG = CACHE_CONFIG
+
class CeleryConfig(object):
BROKER_URL = f"redis://{REDIS_HOST}:{REDIS_PORT}/{REDIS_CELERY_DB}"
diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py
index ee73df5fde..137ec068e8 100644
--- a/superset/tasks/cache.py
+++ b/superset/tasks/cache.py
@@ -14,73 +14,36 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-import json
import logging
from typing import Any, Dict, List, Optional, Union
from urllib import request
from urllib.error import URLError
+from celery.beat import SchedulingError
from celery.utils.log import get_task_logger
from sqlalchemy import and_, func
-from superset import app, db
+from superset import app, db, security_manager
from superset.extensions import celery_app
from superset.models.core import Log
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.tags import Tag, TaggedObject
from superset.utils.date_parser import parse_human_datetime
-from superset.views.utils import build_extra_filters
+from superset.utils.machine_auth import MachineAuthProvider
logger = get_task_logger(__name__)
logger.setLevel(logging.INFO)
-def get_form_data(
- chart_id: int, dashboard: Optional[Dashboard] = None
-) -> Dict[str, Any]:
- """
- Build `form_data` for chart GET request from dashboard's `default_filters`.
-
- When a dashboard has `default_filters` they need to be added as extra
- filters in the GET request for charts.
-
- """
- form_data: Dict[str, Any] = {"slice_id": chart_id}
-
- if dashboard is None or not dashboard.json_metadata:
- return form_data
-
- json_metadata = json.loads(dashboard.json_metadata)
- default_filters = json.loads(json_metadata.get("default_filters", "null"))
- if not default_filters:
- return form_data
-
- filter_scopes = json_metadata.get("filter_scopes", {})
- layout = json.loads(dashboard.position_json or "{}")
- if (
- isinstance(layout, dict)
- and isinstance(filter_scopes, dict)
- and isinstance(default_filters, dict)
- ):
- extra_filters = build_extra_filters(
- layout, filter_scopes, default_filters, chart_id
- )
- if extra_filters:
- form_data["extra_filters"] = extra_filters
-
- return form_data
-
-
-def get_url(chart: Slice, extra_filters: Optional[Dict[str, Any]] = None) -> str:
+def get_url(chart: Slice, dashboard: Optional[Dashboard] = None) -> str:
"""Return external URL for warming up a given chart/table cache."""
with app.test_request_context():
- baseurl = (
- "{SUPERSET_WEBSERVER_PROTOCOL}://"
- "{SUPERSET_WEBSERVER_ADDRESS}:"
- "{SUPERSET_WEBSERVER_PORT}".format(**app.config)
- )
- return f"{baseurl}{chart.get_explore_url(overrides=extra_filters)}"
+ baseurl = "{WEBDRIVER_BASEURL}".format(**app.config)
+ url = f"{baseurl}superset/warm_up_cache/?slice_id={chart.id}"
+ if dashboard:
+ url += f"&dashboard_id={dashboard.id}"
+ return url
class Strategy: # pylint: disable=too-few-public-methods
@@ -179,8 +142,7 @@ class TopNDashboardsStrategy(Strategy): # pylint: disable=too-few-public-method
dashboards = session.query(Dashboard).filter(Dashboard.id.in_(dash_ids)).all()
for dashboard in dashboards:
for chart in dashboard.slices:
- form_data_with_filters = get_form_data(chart.id, dashboard)
- urls.append(get_url(chart, form_data_with_filters))
+ urls.append(get_url(chart, dashboard))
return urls
@@ -253,6 +215,30 @@ class DashboardTagsStrategy(Strategy): # pylint: disable=too-few-public-methods
strategies = [DummyStrategy, TopNDashboardsStrategy, DashboardTagsStrategy]
+@celery_app.task(name="fetch_url")
+def fetch_url(url: str, headers: Dict[str, str]) -> Dict[str, str]:
+ """
+ Celery job to fetch url
+ """
+ result = {}
+ try:
+ logger.info("Fetching %s", url)
+ req = request.Request(url, headers=headers)
+ response = request.urlopen( # pylint: disable=consider-using-with
+ req, timeout=600
+ )
+ logger.info("Fetched %s, status code: %s", url, response.code)
+ if response.code == 200:
+ result = {"success": url, "response": response.read().decode("utf-8")}
+ else:
+ result = {"error": url, "status_code": response.code}
+ logger.error("Error fetching %s, status code: %s", url, response.code)
+ except URLError as err:
+ logger.exception("Error warming up cache!")
+ result = {"error": url, "exception": str(err)}
+ return result
+
+
@celery_app.task(name="cache-warmup")
def cache_warmup(
strategy_name: str, *args: Any, **kwargs: Any
@@ -282,14 +268,18 @@ def cache_warmup(
logger.exception(message)
return message
- results: Dict[str, List[str]] = {"success": [], "errors": []}
+ user = security_manager.get_user_by_username(app.config["THUMBNAIL_SELENIUM_USER"])
+ cookies = MachineAuthProvider.get_auth_cookies(user)
+ headers = {"Cookie": f"session={cookies.get('session', '')}"}
+
+ results: Dict[str, List[str]] = {"scheduled": [], "errors": []}
for url in strategy.get_urls():
try:
- logger.info("Fetching %s", url)
- request.urlopen(url) # pylint: disable=consider-using-with
- results["success"].append(url)
- except URLError:
- logger.exception("Error warming up cache!")
+ logger.info("Scheduling %s", url)
+ fetch_url.delay(url, headers)
+ results["scheduled"].append(url)
+ except SchedulingError:
+ logger.exception("Error scheduling fetch_url: %s", url)
results["errors"].append(url)
return results
diff --git a/tests/integration_tests/strategy_tests.py b/tests/integration_tests/strategy_tests.py
index aec73b1efe..f31489bb04 100644
--- a/tests/integration_tests/strategy_tests.py
+++ b/tests/integration_tests/strategy_tests.py
@@ -38,9 +38,9 @@ from superset.models.core import Log
from superset.models.tags import get_tag, ObjectTypes, TaggedObject, TagTypes
from superset.tasks.cache import (
DashboardTagsStrategy,
- get_form_data,
TopNDashboardsStrategy,
)
+from superset.utils.urls import get_url_host
from .base_tests import SupersetTestCase
from .dashboard_utils import create_dashboard, create_slice, create_table_metadata
@@ -49,7 +49,6 @@ from .fixtures.unicode_dashboard import (
load_unicode_data,
)
-URL_PREFIX = "http://0.0.0.0:8081"
mock_positions = {
"DASHBOARD_VERSION_KEY": "v2",
@@ -69,128 +68,6 @@ mock_positions = {
class TestCacheWarmUp(SupersetTestCase):
- def test_get_form_data_chart_only(self):
- chart_id = 1
- result = get_form_data(chart_id, None)
- expected = {"slice_id": chart_id}
- self.assertEqual(result, expected)
-
- def test_get_form_data_no_dashboard_metadata(self):
- chart_id = 1
- dashboard = MagicMock()
- dashboard.json_metadata = None
- dashboard.position_json = json.dumps(mock_positions)
- result = get_form_data(chart_id, dashboard)
- expected = {"slice_id": chart_id}
- self.assertEqual(result, expected)
-
- def test_get_form_data_immune_slice(self):
- chart_id = 1
- filter_box_id = 2
- dashboard = MagicMock()
- dashboard.position_json = json.dumps(mock_positions)
- dashboard.json_metadata = json.dumps(
- {
- "filter_scopes": {
- str(filter_box_id): {
- "name": {"scope": ["ROOT_ID"], "immune": [chart_id]}
- }
- },
- "default_filters": json.dumps(
- {str(filter_box_id): {"name": ["Alice", "Bob"]}}
- ),
- }
- )
- result = get_form_data(chart_id, dashboard)
- expected = {"slice_id": chart_id}
- self.assertEqual(result, expected)
-
- def test_get_form_data_no_default_filters(self):
- chart_id = 1
- dashboard = MagicMock()
- dashboard.json_metadata = json.dumps({})
- dashboard.position_json = json.dumps(mock_positions)
- result = get_form_data(chart_id, dashboard)
- expected = {"slice_id": chart_id}
- self.assertEqual(result, expected)
-
- def test_get_form_data_immune_fields(self):
- chart_id = 1
- filter_box_id = 2
- dashboard = MagicMock()
- dashboard.position_json = json.dumps(mock_positions)
- dashboard.json_metadata = json.dumps(
- {
- "default_filters": json.dumps(
- {
- str(filter_box_id): {
- "name": ["Alice", "Bob"],
- "__time_range": "100 years ago : today",
- }
- }
- ),
- "filter_scopes": {
- str(filter_box_id): {
- "__time_range": {"scope": ["ROOT_ID"], "immune": [chart_id]}
- }
- },
- }
- )
- result = get_form_data(chart_id, dashboard)
- expected = {
- "slice_id": chart_id,
- "extra_filters": [{"col": "name", "op": "in", "val": ["Alice", "Bob"]}],
- }
- self.assertEqual(result, expected)
-
- def test_get_form_data_no_extra_filters(self):
- chart_id = 1
- filter_box_id = 2
- dashboard = MagicMock()
- dashboard.position_json = json.dumps(mock_positions)
- dashboard.json_metadata = json.dumps(
- {
- "default_filters": json.dumps(
- {str(filter_box_id): {"__time_range": "100 years ago : today"}}
- ),
- "filter_scopes": {
- str(filter_box_id): {
- "__time_range": {"scope": ["ROOT_ID"], "immune": [chart_id]}
- }
- },
- }
- )
- result = get_form_data(chart_id, dashboard)
- expected = {"slice_id": chart_id}
- self.assertEqual(result, expected)
-
- def test_get_form_data(self):
- chart_id = 1
- filter_box_id = 2
- dashboard = MagicMock()
- dashboard.position_json = json.dumps(mock_positions)
- dashboard.json_metadata = json.dumps(
- {
- "default_filters": json.dumps(
- {
- str(filter_box_id): {
- "name": ["Alice", "Bob"],
- "__time_range": "100 years ago : today",
- }
- }
- )
- }
- )
- result = get_form_data(chart_id, dashboard)
- expected = {
- "slice_id": chart_id,
- "extra_filters": [
- {"col": "name", "op": "in", "val": ["Alice", "Bob"]},
- {"col": "__time_range", "op": "==", "val": "100 years ago : today"},
- ],
- }
- self.assertEqual(result, expected)
-
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_top_n_dashboards_strategy(self):
# create a top visited dashboard
@@ -202,7 +79,12 @@ class TestCacheWarmUp(SupersetTestCase):
strategy = TopNDashboardsStrategy(1)
result = sorted(strategy.get_urls())
- expected = sorted([f"{URL_PREFIX}{slc.url}" for slc in dash.slices])
+ expected = sorted(
+ [
+ f"{get_url_host()}superset/warm_up_cache/?slice_id={slc.id}&dashboard_id={dash.id}"
+ for slc in dash.slices
+ ]
+ )
self.assertEqual(result, expected)
def reset_tag(self, tag):
@@ -228,7 +110,12 @@ class TestCacheWarmUp(SupersetTestCase):
# tag dashboard 'births' with `tag1`
tag1 = get_tag("tag1", db.session, TagTypes.custom)
dash = self.get_dash_by_slug("births")
- tag1_urls = sorted([f"{URL_PREFIX}{slc.url}" for slc in dash.slices])
+ tag1_urls = sorted(
+ [
+ f"{get_url_host()}superset/warm_up_cache/?slice_id={slc.id}"
+ for slc in dash.slices
+ ]
+ )
tagged_object = TaggedObject(
tag_id=tag1.id, object_id=dash.id, object_type=ObjectTypes.dashboard
)
@@ -248,7 +135,7 @@ class TestCacheWarmUp(SupersetTestCase):
# tag first slice
dash = self.get_dash_by_slug("unicode-test")
slc = dash.slices[0]
- tag2_urls = [f"{URL_PREFIX}{slc.url}"]
+ tag2_urls = [f"{get_url_host()}superset/warm_up_cache/?slice_id={slc.id}"]
object_id = slc.id
tagged_object = TaggedObject(
tag_id=tag2.id, object_id=object_id, object_type=ObjectTypes.chart
diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py
index 7c86232829..c3e80cb07a 100644
--- a/tests/integration_tests/superset_test_config.py
+++ b/tests/integration_tests/superset_test_config.py
@@ -66,6 +66,8 @@ FEATURE_FLAGS = {
"DASHBOARD_NATIVE_FILTERS": True,
}
+WEBDRIVER_BASEURL = "http://0.0.0.0:8081/"
+
def GET_FEATURE_FLAGS_FUNC(ff):
ff_copy = copy(ff)
[superset] 02/04: feat: adds TLS certificate validation option for SMTP (#21272)
Posted by mi...@apache.org.
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 85fb2433ab2ad9bffffa421ec3744005d96572dd
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Thu Sep 1 10:51:34 2022 +0100
feat: adds TLS certificate validation option for SMTP (#21272)
(cherry picked from commit 9fd752057eb261b0e5db87636836fd30579ffce6)
---
docs/docs/installation/alerts-reports.mdx | 1 +
superset/config.py | 4 +++-
superset/utils/core.py | 34 ++++++++++++++++++-------------
tests/integration_tests/email_tests.py | 29 +++++++++++++++++++++++++-
4 files changed, 52 insertions(+), 16 deletions(-)
diff --git a/docs/docs/installation/alerts-reports.mdx b/docs/docs/installation/alerts-reports.mdx
index 3ddb35caa1..bebeb91112 100644
--- a/docs/docs/installation/alerts-reports.mdx
+++ b/docs/docs/installation/alerts-reports.mdx
@@ -126,6 +126,7 @@ SLACK_API_TOKEN = "xoxb-"
# Email configuration
SMTP_HOST = "smtp.sendgrid.net" #change to your host
SMTP_STARTTLS = True
+SMTP_SSL_SERVER_AUTH = True # If your using an SMTP server with a valid certificate
SMTP_SSL = False
SMTP_USER = "your_user"
SMTP_PORT = 2525 # your port eg. 587
diff --git a/superset/config.py b/superset/config.py
index 765e5521ce..ccfb83c17b 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -969,7 +969,9 @@ SMTP_USER = "superset"
SMTP_PORT = 25
SMTP_PASSWORD = "superset"
SMTP_MAIL_FROM = "superset@superset.com"
-
+# If True creates a default SSL context with ssl.Purpose.CLIENT_AUTH using the
+# default system root CA certificates.
+SMTP_SSL_SERVER_AUTH = False
ENABLE_CHUNK_ENCODING = False
# Whether to bump the logging level to ERROR on the flask_appbuilder package
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 74b033fe5d..6d74cf459b 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -27,6 +27,7 @@ import platform
import re
import signal
import smtplib
+import ssl
import tempfile
import threading
import traceback
@@ -974,23 +975,28 @@ def send_mime_email(
smtp_password = config["SMTP_PASSWORD"]
smtp_starttls = config["SMTP_STARTTLS"]
smtp_ssl = config["SMTP_SSL"]
+ smpt_ssl_server_auth = config["SMTP_SSL_SERVER_AUTH"]
- if not dryrun:
- smtp = (
- smtplib.SMTP_SSL(smtp_host, smtp_port)
- if smtp_ssl
- else smtplib.SMTP(smtp_host, smtp_port)
- )
- if smtp_starttls:
- smtp.starttls()
- if smtp_user and smtp_password:
- smtp.login(smtp_user, smtp_password)
- logger.debug("Sent an email to %s", str(e_to))
- smtp.sendmail(e_from, e_to, mime_msg.as_string())
- smtp.quit()
- else:
+ if dryrun:
logger.info("Dryrun enabled, email notification content is below:")
logger.info(mime_msg.as_string())
+ return
+
+ # Default ssl context is SERVER_AUTH using the default system
+ # root CA certificates
+ ssl_context = ssl.create_default_context() if smpt_ssl_server_auth else None
+ smtp = (
+ smtplib.SMTP_SSL(smtp_host, smtp_port, context=ssl_context)
+ if smtp_ssl
+ else smtplib.SMTP(smtp_host, smtp_port)
+ )
+ if smtp_starttls:
+ smtp.starttls(context=ssl_context)
+ if smtp_user and smtp_password:
+ smtp.login(smtp_user, smtp_password)
+ logger.debug("Sent an email to %s", str(e_to))
+ smtp.sendmail(e_from, e_to, mime_msg.as_string())
+ smtp.quit()
def get_email_address_list(address_string: str) -> List[str]:
diff --git a/tests/integration_tests/email_tests.py b/tests/integration_tests/email_tests.py
index d6c46a08d9..68e4aaf71e 100644
--- a/tests/integration_tests/email_tests.py
+++ b/tests/integration_tests/email_tests.py
@@ -17,6 +17,7 @@
# under the License.
"""Unit tests for email service in Superset"""
import logging
+import ssl
import tempfile
import unittest
from email.mime.application import MIMEApplication
@@ -144,9 +145,35 @@ class TestEmailSmtp(SupersetTestCase):
utils.send_mime_email("from", "to", MIMEMultipart(), app.config, dryrun=False)
assert not mock_smtp.called
mock_smtp_ssl.assert_called_with(
- app.config["SMTP_HOST"], app.config["SMTP_PORT"]
+ app.config["SMTP_HOST"], app.config["SMTP_PORT"], context=None
)
+ @mock.patch("smtplib.SMTP_SSL")
+ @mock.patch("smtplib.SMTP")
+ def test_send_mime_ssl_server_auth(self, mock_smtp, mock_smtp_ssl):
+ app.config["SMTP_SSL"] = True
+ app.config["SMTP_SSL_SERVER_AUTH"] = True
+ mock_smtp.return_value = mock.Mock()
+ mock_smtp_ssl.return_value = mock.Mock()
+ utils.send_mime_email("from", "to", MIMEMultipart(), app.config, dryrun=False)
+ assert not mock_smtp.called
+ mock_smtp_ssl.assert_called_with(
+ app.config["SMTP_HOST"], app.config["SMTP_PORT"], context=mock.ANY
+ )
+ called_context = mock_smtp_ssl.call_args.kwargs["context"]
+ self.assertEqual(called_context.verify_mode, ssl.CERT_REQUIRED)
+
+ @mock.patch("smtplib.SMTP")
+ def test_send_mime_tls_server_auth(self, mock_smtp):
+ app.config["SMTP_STARTTLS"] = True
+ app.config["SMTP_SSL_SERVER_AUTH"] = True
+ mock_smtp.return_value = mock.Mock()
+ mock_smtp.return_value.starttls.return_value = mock.Mock()
+ utils.send_mime_email("from", "to", MIMEMultipart(), app.config, dryrun=False)
+ mock_smtp.return_value.starttls.assert_called_with(context=mock.ANY)
+ called_context = mock_smtp.return_value.starttls.call_args.kwargs["context"]
+ self.assertEqual(called_context.verify_mode, ssl.CERT_REQUIRED)
+
@mock.patch("smtplib.SMTP_SSL")
@mock.patch("smtplib.SMTP")
def test_send_mime_noauth(self, mock_smtp, mock_smtp_ssl):
[superset] 04/04: fix: Connect database from top right corner button doesn't work
Posted by mi...@apache.org.
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 37bdf434bffc81c04dead571038b72a39b1d948d
Author: Michael S. Molina <mi...@gmail.com>
AuthorDate: Tue Sep 6 14:13:32 2022 -0300
fix: Connect database from top right corner button doesn't work
---
superset-frontend/src/views/components/MenuRight.tsx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/superset-frontend/src/views/components/MenuRight.tsx b/superset-frontend/src/views/components/MenuRight.tsx
index 6495b62912..4628a47e24 100644
--- a/superset-frontend/src/views/components/MenuRight.tsx
+++ b/superset-frontend/src/views/components/MenuRight.tsx
@@ -203,7 +203,7 @@ const RightMenu = ({
typeof item !== 'string' && item.name && item.perm ? (
<Fragment key={item.name}>
{idx === 2 && <Menu.Divider />}
- <Menu.Item>
+ <Menu.Item key={item.name}>
{item.url ? (
<a href={item.url}> {item.label} </a>
) : (
[superset] 03/04: fix: disallow users from viewing other user's profile on config (#21302)
Posted by mi...@apache.org.
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)