diff --git a/docs/admin_docs/configuration/alerts-reports.mdx b/docs/admin_docs/configuration/alerts-reports.mdx index 8364b8e50ae..b66f641e290 100644 --- a/docs/admin_docs/configuration/alerts-reports.mdx +++ b/docs/admin_docs/configuration/alerts-reports.mdx @@ -233,6 +233,20 @@ def alert_dynamic_minimal_interval(**kwargs) -> int: ALERT_MINIMUM_INTERVAL = alert_dynamic_minimal_interval ``` +## External Link Redirection + +For security, Superset rewrites external links in alert/report email HTML so +they go through a warning page before the user is navigated to the external +site. Internal links (matching your configured base URL) are not affected. + +```python +# Disable external link redirection entirely (default: True) +ALERT_REPORTS_ENABLE_LINK_REDIRECT = False +``` + +The feature uses `WEBDRIVER_BASEURL_USER_FRIENDLY` (or `WEBDRIVER_BASEURL`) +to determine which hosts are internal. + ## Troubleshooting There are many reasons that reports might not be working. Try these steps to check for specific issues. diff --git a/superset-frontend/src/pages/RedirectWarning/index.tsx b/superset-frontend/src/pages/RedirectWarning/index.tsx new file mode 100644 index 00000000000..0f36da33ca8 --- /dev/null +++ b/superset-frontend/src/pages/RedirectWarning/index.tsx @@ -0,0 +1,175 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useState, useMemo, useCallback, useEffect } from 'react'; +import { t } from '@apache-superset/core/translation'; +import { css, styled, useTheme } from '@apache-superset/core/theme'; +import { + Button, + Card, + Checkbox, + Flex, + Typography, +} from '@superset-ui/core/components'; +import { Icons } from '@superset-ui/core/components/Icons'; +import { getTargetUrl, isUrlTrusted, trustUrl, isAllowedScheme } from './utils'; + +const PageContainer = styled(Flex)` + ${({ theme }) => css` + height: calc(100vh - 64px); + background-color: ${theme.colorBgLayout}; + padding: ${theme.padding}px; + `} +`; + +const WarningCard = styled(Card)` + ${({ theme }) => css` + max-width: 520px; + width: 100%; + box-shadow: ${theme.boxShadowSecondary}; + `} +`; + +const WarningHeader = styled(Flex)` + ${({ theme }) => css` + padding: ${theme.paddingLG}px ${theme.paddingXL}px; + border-bottom: 1px solid ${theme.colorBorderSecondary}; + `} +`; + +const WarningBody = styled.div` + ${({ theme }) => css` + padding: ${theme.paddingXL}px; + `} +`; + +const UrlDisplay = styled(Flex)` + ${({ theme }) => css` + background-color: ${theme.colorFillQuaternary}; + border-radius: ${theme.borderRadiusSM}px; + padding: ${theme.paddingSM}px ${theme.padding}px; + margin-bottom: ${theme.margin}px; + `} +`; + +const UrlText = styled(Typography.Text)` + ${({ theme }) => css` + font-family: ${theme.fontFamilyCode}; + font-size: ${theme.fontSize}px; + word-break: break-all; + `} +`; + +const WarningFooter = styled(Flex)` + ${({ theme }) => css` + padding: ${theme.padding}px ${theme.paddingXL}px; + background-color: ${theme.colorFillAlter}; + border-top: 1px solid ${theme.colorBorderSecondary}; + `} +`; + +const WarningTitle = styled(Typography.Title)` + && { + margin: 0; + } +`; + +export default function RedirectWarning() { + const theme = useTheme(); + const [trustChecked, setTrustChecked] = useState(false); + + const targetUrl = useMemo(() => getTargetUrl(), []); + + // Redirect immediately if the URL is already trusted + useEffect(() => { + if (targetUrl && isAllowedScheme(targetUrl) && isUrlTrusted(targetUrl)) { + window.location.href = targetUrl; + } + }, [targetUrl]); + + const handleContinue = useCallback(() => { + if (!targetUrl || !isAllowedScheme(targetUrl)) return; + if (trustChecked) { + trustUrl(targetUrl); + } + window.location.href = targetUrl; + }, [trustChecked, targetUrl]); + + const handleReturn = useCallback(() => { + window.location.href = '/'; + }, []); + + if (!targetUrl) { + return ( + + + + + {t('Missing URL parameter')} + + + + + ); + } + + return ( + + + + + {t('External link warning')} + + + + + {t( + 'This link will take you to an external website. We cannot guarantee the safety of external destinations.', + )} + + + + + {targetUrl} + + + + setTrustChecked(e.target.checked)} + > + {t("Trust this URL and don't ask again")} + + + + + {t('Only proceed if you trust the destination or its source.')} + + + + + + + + + + ); +} diff --git a/superset-frontend/src/pages/RedirectWarning/utils.test.ts b/superset-frontend/src/pages/RedirectWarning/utils.test.ts new file mode 100644 index 00000000000..c9d2d7fbdc0 --- /dev/null +++ b/superset-frontend/src/pages/RedirectWarning/utils.test.ts @@ -0,0 +1,124 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { isAllowedScheme, getTargetUrl, isUrlTrusted, trustUrl } from './utils'; + +const TRUSTED_URLS_KEY = 'superset_trusted_urls'; + +beforeEach(() => { + localStorage.clear(); +}); + +test('isAllowedScheme accepts http URLs', () => { + expect(isAllowedScheme('http://example.com')).toBe(true); +}); + +test('isAllowedScheme accepts https URLs', () => { + expect(isAllowedScheme('https://example.com/page?q=1')).toBe(true); +}); + +test('isAllowedScheme blocks javascript: URLs', () => { + // oxlint-disable-next-line no-script-url -- testing that dangerous schemes are blocked + expect(isAllowedScheme('javascript:alert(1)')).toBe(false); +}); + +test('isAllowedScheme blocks data: URLs', () => { + expect(isAllowedScheme('data:text/html,')).toBe( + false, + ); +}); + +test('isAllowedScheme blocks vbscript: URLs', () => { + expect(isAllowedScheme('vbscript:MsgBox("XSS")')).toBe(false); +}); + +test('isAllowedScheme blocks file: URLs', () => { + expect(isAllowedScheme('file:///etc/passwd')).toBe(false); +}); + +test('isAllowedScheme allows relative URLs (unparseable as absolute)', () => { + expect(isAllowedScheme('/dashboard/1')).toBe(true); +}); + +test('getTargetUrl reads the url query parameter', () => { + Object.defineProperty(window, 'location', { + value: { search: '?url=https%3A%2F%2Fexample.com%2Fpage' }, + writable: true, + }); + expect(getTargetUrl()).toBe('https://example.com/page'); +}); + +test('getTargetUrl returns empty string when url param is missing', () => { + Object.defineProperty(window, 'location', { + value: { search: '' }, + writable: true, + }); + expect(getTargetUrl()).toBe(''); +}); + +test('getTargetUrl does not double-decode percent-encoded values', () => { + // %253A is the double-encoding of ":" — after one decode it should remain %3A + Object.defineProperty(window, 'location', { + value: { search: '?url=javascript%253Aalert(1)' }, + writable: true, + }); + expect(getTargetUrl()).toBe('javascript%3Aalert(1)'); +}); + +test('trustUrl stores and isUrlTrusted retrieves a URL', () => { + const url = 'https://example.com/page'; + expect(isUrlTrusted(url)).toBe(false); + trustUrl(url); + expect(isUrlTrusted(url)).toBe(true); +}); + +test('isUrlTrusted normalizes URLs for comparison', () => { + trustUrl('https://example.com/page/'); + expect(isUrlTrusted('https://example.com/page')).toBe(true); +}); + +test('trustUrl does not add duplicates', () => { + trustUrl('https://example.com'); + trustUrl('https://example.com'); + const stored = JSON.parse( + localStorage.getItem(TRUSTED_URLS_KEY) ?? '[]', + ) as string[]; + expect(stored).toHaveLength(1); +}); + +test('isUrlTrusted returns false when localStorage contains invalid data', () => { + localStorage.setItem(TRUSTED_URLS_KEY, '"not-an-array"'); + expect(isUrlTrusted('https://example.com')).toBe(false); +}); + +test('isUrlTrusted returns false when localStorage contains a non-array object', () => { + localStorage.setItem(TRUSTED_URLS_KEY, '{"foo":"bar"}'); + expect(isUrlTrusted('https://example.com')).toBe(false); +}); + +test('trustUrl caps storage at 100 entries', () => { + const urls = Array.from({ length: 105 }, (_, i) => `https://example${i}.com`); + urls.forEach(url => trustUrl(url)); + const stored = JSON.parse( + localStorage.getItem(TRUSTED_URLS_KEY) ?? '[]', + ) as string[]; + expect(stored.length).toBeLessThanOrEqual(100); + // The most recent entries should be kept + expect(stored).toContain('https://example104.com'); +}); diff --git a/superset-frontend/src/pages/RedirectWarning/utils.ts b/superset-frontend/src/pages/RedirectWarning/utils.ts new file mode 100644 index 00000000000..43aa834789a --- /dev/null +++ b/superset-frontend/src/pages/RedirectWarning/utils.ts @@ -0,0 +1,96 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +const TRUSTED_URLS_KEY = 'superset_trusted_urls'; +const MAX_TRUSTED_URLS = 100; +const ALLOWED_SCHEMES = ['http:', 'https:']; + +/** + * Normalize a URL for comparison (origin + path without trailing slash + search). + */ +function normalizeUrl(url: string): string { + try { + const parsed = new URL(url); + return parsed.origin + parsed.pathname.replace(/\/$/, '') + parsed.search; + } catch { + return url; + } +} + +/** + * Return true if the URL scheme is safe for navigation. + * Blocks javascript:, data:, vbscript:, file:, etc. + */ +export function isAllowedScheme(url: string): boolean { + try { + const parsed = new URL(url); + return ALLOWED_SCHEMES.includes(parsed.protocol); + } catch { + // relative URLs or unparseable — allow (they'll resolve against current origin) + return true; + } +} + +/** + * Read the target URL from the current page's query string. + * + * URLSearchParams.get() already percent-decodes the value, so we must NOT + * call decodeURIComponent again (doing so would allow double-encoded + * payloads like `javascript%253Aalert(1)` to bypass scheme checks). + */ +export function getTargetUrl(): string { + const params = new URLSearchParams(window.location.search); + const url = params.get('url') ?? ''; + return url.trim(); +} + +function getTrustedUrls(): string[] { + try { + const stored = localStorage.getItem(TRUSTED_URLS_KEY); + if (!stored) return []; + const parsed: unknown = JSON.parse(stored); + return Array.isArray(parsed) ? (parsed as string[]) : []; + } catch { + return []; + } +} + +function saveTrustedUrls(urls: string[]): void { + const limited = + urls.length > MAX_TRUSTED_URLS ? urls.slice(-MAX_TRUSTED_URLS) : urls; + try { + localStorage.setItem(TRUSTED_URLS_KEY, JSON.stringify(limited)); + } catch { + // Ignore storage errors (private browsing, quota exceeded, etc.) + } +} + +export function isUrlTrusted(url: string): boolean { + const normalized = normalizeUrl(url); + return getTrustedUrls().some(t => normalizeUrl(t) === normalized); +} + +export function trustUrl(url: string): void { + const normalized = normalizeUrl(url); + const trusted = getTrustedUrls(); + if (!trusted.some(t => normalizeUrl(t) === normalized)) { + trusted.push(url); + saveTrustedUrls(trusted); + } +} diff --git a/superset-frontend/src/views/routes.tsx b/superset-frontend/src/views/routes.tsx index 674fbc5846c..4f066e3ec2c 100644 --- a/superset-frontend/src/views/routes.tsx +++ b/superset-frontend/src/views/routes.tsx @@ -180,6 +180,13 @@ const FileHandler = lazy( () => import(/* webpackChunkName: "FileHandler" */ 'src/pages/FileHandler'), ); +const RedirectWarning = lazy( + () => + import( + /* webpackChunkName: "RedirectWarning" */ 'src/pages/RedirectWarning' + ), +); + type Routes = { path: string; Component: ComponentType; @@ -188,6 +195,10 @@ type Routes = { }[]; export const routes: Routes = [ + { + path: '/redirect/', + Component: RedirectWarning, + }, { path: '/login/', Component: Login, diff --git a/superset/config.py b/superset/config.py index 30f0f801227..7477f7f4f9e 100644 --- a/superset/config.py +++ b/superset/config.py @@ -1964,6 +1964,8 @@ ALERT_REPORTS_QUERY_EXECUTION_MAX_TRIES = 1 # Custom width for screenshots ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH = 600 ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH = 2400 +# Rewrite external links in alert/report emails to go through a warning page +ALERT_REPORTS_ENABLE_LINK_REDIRECT = True # Set a minimum interval threshold between executions (for each Alert/Report) # Value should be an integer i.e. int(timedelta(minutes=5).total_seconds()) # You can also assign a function to the config that returns the expected integer diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index abd2943f1f5..878ed2f52d5 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -207,6 +207,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods from superset.views.groups import GroupsListView from superset.views.log.api import LogRestApi from superset.views.logs import ActionLogView + from superset.views.redirect import RedirectView from superset.views.roles import RolesListView from superset.views.sql_lab.views import ( SavedQueryView, @@ -446,6 +447,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods appbuilder.add_view_no_menu(TaggedObjectsModelView) appbuilder.add_view_no_menu(TagView) appbuilder.add_view_no_menu(ReportView) + appbuilder.add_view_no_menu(RedirectView) appbuilder.add_view_no_menu(RoleRestAPI) appbuilder.add_view_no_menu(UserInfoView) diff --git a/superset/reports/notifications/email.py b/superset/reports/notifications/email.py index 4f3e24d9dcc..d0f51c9c4fb 100644 --- a/superset/reports/notifications/email.py +++ b/superset/reports/notifications/email.py @@ -34,6 +34,7 @@ from superset.reports.notifications.exceptions import NotificationError from superset.utils import json from superset.utils.core import HeaderDataType, send_email_smtp from superset.utils.decorators import statsd_gauge +from superset.utils.link_redirect import process_html_links logger = logging.getLogger(__name__) @@ -133,6 +134,9 @@ class EmailNotification(BaseNotification): # pylint: disable=too-few-public-met attributes=ALLOWED_ATTRIBUTES, ) + # Rewrite external links to go through the redirect warning page + description = process_html_links(description) + # Strip malicious HTML from embedded data, allowing only table elements if self._content.embedded_data is not None: df = self._content.embedded_data @@ -144,6 +148,7 @@ class EmailNotification(BaseNotification): # pylint: disable=too-few-public-met tags=TABLE_TAGS, attributes=ALLOWED_TABLE_ATTRIBUTES, ) + html_table = process_html_links(html_table) else: html_table = "" diff --git a/superset/utils/link_redirect.py b/superset/utils/link_redirect.py new file mode 100644 index 00000000000..b95f262f339 --- /dev/null +++ b/superset/utils/link_redirect.py @@ -0,0 +1,149 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Utilities for processing links in alert/report emails. + +External links are rewritten to go through a redirect warning page so that +recipients see a confirmation before navigating to an external site. +""" + +import logging +import re +from urllib.parse import quote, urlparse + +from flask import current_app + +logger = logging.getLogger(__name__) + +# Matches href="..." in anchor tags (both single and double quotes) +_HREF_RE = re.compile( + r"""(]*?href\s*=\s*)(["'])(.*?)\2""", + re.IGNORECASE | re.DOTALL, +) + + +def _get_base_hosts() -> set[str]: + """Return the set of hosts that are considered internal (lower-cased).""" + hosts: set[str] = set() + for key in ("WEBDRIVER_BASEURL_USER_FRIENDLY", "WEBDRIVER_BASEURL"): + url = current_app.config.get(key, "") + if url: + parsed = urlparse(url) + if parsed.scheme and parsed.netloc: + hosts.add(parsed.netloc.lower()) + return hosts + + +def _get_redirect_base() -> str: + """Return the base URL used to build redirect links.""" + for key in ("WEBDRIVER_BASEURL_USER_FRIENDLY", "WEBDRIVER_BASEURL"): + url = current_app.config.get(key, "") + if url: + return url.rstrip("/") + return "" + + +def _is_external(href: str, base_hosts: set[str]) -> bool: + """Return True if *href* points to an external host.""" + parsed = urlparse(href) + # Only rewrite http(s) links with a host that differs from ours + if parsed.scheme not in ("http", "https"): + return False + return bool(parsed.netloc) and parsed.netloc.lower() not in base_hosts + + +def _replace_href( + match: re.Match[str], + base_hosts: set[str], + redirect_base: str, +) -> str: + """Regex replacer: rewrite external hrefs to go through the redirect page.""" + prefix, quote_char, href = match.group(1), match.group(2), match.group(3) + href = href.strip() + + # Don't double-redirect + if "/redirect/" in href: + return match.group(0) + + if not _is_external(href, base_hosts): + return match.group(0) + + redirect_url = f"{redirect_base}/redirect/?url={quote(href, safe='')}" + return f"{prefix}{quote_char}{redirect_url}{quote_char}" + + +def process_html_links(html_content: str) -> str: + """ + Rewrite external links in *html_content* to go through the redirect page. + + Internal links (matching the configured base URL hosts) are left untouched. + """ + if not html_content or not html_content.strip(): + return html_content + + if not current_app.config.get("ALERT_REPORTS_ENABLE_LINK_REDIRECT", True): + return html_content + + base_hosts = _get_base_hosts() + if not base_hosts: + logger.warning("No base URL configured, skipping link redirect processing") + return html_content + + redirect_base = _get_redirect_base() + if not redirect_base: + return html_content + + try: + return _HREF_RE.sub( + lambda m: _replace_href(m, base_hosts, redirect_base), + html_content, + ) + except Exception: + logger.warning("Failed to process HTML links", exc_info=True) + return html_content + + +def is_safe_redirect_url(url: str) -> bool: + """ + Return True if *url* is an internal Superset URL (safe to redirect to + without showing a warning). + """ + if not url or not url.strip(): + return False + + stripped = url.strip() + + # Block protocol-relative URLs + if stripped.startswith("//") or stripped.startswith("\\\\"): + return False + + parsed = urlparse(stripped) + + # Relative paths are safe + if not parsed.scheme and not parsed.netloc: + return True + + # Only allow http(s) + if parsed.scheme not in ("http", "https"): + return False + + base_hosts = _get_base_hosts() + if not base_hosts: + return False + + return parsed.netloc.lower() in base_hosts diff --git a/superset/views/redirect.py b/superset/views/redirect.py new file mode 100644 index 00000000000..90d0452e62c --- /dev/null +++ b/superset/views/redirect.py @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +View that handles external-link redirects with a warning page. + +Links in alert/report emails are rewritten to point here. Internal links +redirect immediately; external links are shown to the user for confirmation +via the React ``RedirectWarning`` page. +""" + +import logging +from urllib.parse import urlparse + +from flask import abort, redirect, request +from flask_appbuilder import expose + +from superset import is_feature_enabled +from superset.superset_typing import FlaskResponse +from superset.utils.link_redirect import is_safe_redirect_url +from superset.views.base import BaseSupersetView + +logger = logging.getLogger(__name__) + +DANGEROUS_SCHEMES: frozenset[str] = frozenset( + ("javascript", "data", "vbscript", "file") +) + + +class RedirectView(BaseSupersetView): + """ + Warning page for external links found in alert/report emails. + + This endpoint is publicly accessible (no authentication required) + because email recipients may not have an active Superset session. + """ + + route_base = "/redirect" + + @expose("/") + def redirect_warning(self) -> FlaskResponse: + """Validate the target URL and either redirect or show the warning page.""" + if not is_feature_enabled("ALERT_REPORTS"): + abort(404) + + target_url = request.args.get("url", "").strip() + + if not target_url: + abort(400, description="Missing URL parameter") + + # Block dangerous schemes using urlparse for robust detection + parsed = urlparse(target_url) + if parsed.scheme.lower() in DANGEROUS_SCHEMES: + logger.warning("Blocked dangerous URL scheme: %s", target_url[:80]) + abort(400, description="Invalid URL scheme") + + # Internal URLs redirect immediately + if is_safe_redirect_url(target_url): + return redirect(target_url) + + # External URLs: render the React warning page + return super().render_app_template() diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index c7bc6a8e862..2307d522a16 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1662,6 +1662,7 @@ class TestRolePermission(SupersetTestCase): ["SupersetAuthView", "logout"], ["SupersetRegisterUserView", "register"], ["SupersetRegisterUserView", "activation"], + ["RedirectView", "redirect_warning"], ] unsecured_views = [] for view_class in appbuilder.baseviews: diff --git a/tests/integration_tests/views/test_redirect_view.py b/tests/integration_tests/views/test_redirect_view.py new file mode 100644 index 00000000000..b72d3f9b1ef --- /dev/null +++ b/tests/integration_tests/views/test_redirect_view.py @@ -0,0 +1,66 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from tests.conftest import with_config +from tests.integration_tests.base_tests import SupersetTestCase +from tests.integration_tests.conftest import with_feature_flags + +REDIRECT_CONFIG = { + "WEBDRIVER_BASEURL": "http://localhost:8088", + "WEBDRIVER_BASEURL_USER_FRIENDLY": "http://localhost:8088", +} + + +class TestRedirectView(SupersetTestCase): + """Integration tests for the /redirect/ endpoint.""" + + @with_feature_flags(ALERT_REPORTS=True) + @with_config(REDIRECT_CONFIG) + def test_missing_url_returns_400(self): + resp = self.client.get("/redirect/") + assert resp.status_code == 400 + + @with_feature_flags(ALERT_REPORTS=True) + @with_config(REDIRECT_CONFIG) + def test_dangerous_scheme_returns_400(self): + resp = self.client.get("/redirect/?url=javascript:alert(1)") + assert resp.status_code == 400 + + @with_feature_flags(ALERT_REPORTS=True) + @with_config(REDIRECT_CONFIG) + def test_internal_url_redirects(self): + resp = self.client.get( + "/redirect/?url=http://localhost:8088/dashboard/1", + follow_redirects=False, + ) + assert resp.status_code == 302 + assert resp.headers["Location"] == "http://localhost:8088/dashboard/1" + + @with_feature_flags(ALERT_REPORTS=True) + @with_config(REDIRECT_CONFIG) + def test_external_url_renders_page(self): + resp = self.client.get( + "/redirect/?url=https://external.com/page", + ) + assert resp.status_code == 200 + + @with_feature_flags(ALERT_REPORTS=False) + def test_feature_flag_disabled_returns_404(self): + resp = self.client.get( + "/redirect/?url=https://external.com", + ) + assert resp.status_code == 404 diff --git a/tests/unit_tests/utils/test_link_redirect.py b/tests/unit_tests/utils/test_link_redirect.py new file mode 100644 index 00000000000..c02eaa04a9b --- /dev/null +++ b/tests/unit_tests/utils/test_link_redirect.py @@ -0,0 +1,143 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import pytest +from flask import Flask + +from superset.utils.link_redirect import is_safe_redirect_url, process_html_links + + +@pytest.fixture +def app(): + """Minimal Flask app with the config keys used by link_redirect.""" + application = Flask(__name__) + application.config["WEBDRIVER_BASEURL"] = "http://superset.example.com" + application.config["WEBDRIVER_BASEURL_USER_FRIENDLY"] = ( + "https://superset.example.com" + ) + application.config["ALERT_REPORTS_ENABLE_LINK_REDIRECT"] = True + with application.app_context(): + yield application + + +# --------------------------------------------------------------------------- # +# process_html_links +# --------------------------------------------------------------------------- # + + +def test_external_link_is_rewritten(app: Flask) -> None: + html = 'Click' + result = process_html_links(html) + assert "superset.example.com/redirect/?url=https%3A%2F%2Fevil.com%2Fpage" in result + assert "evil.com/page" not in result.split("url=")[0] + + +def test_internal_link_is_not_rewritten(app: Flask) -> None: + html = 'Dashboard' + result = process_html_links(html) + assert result == html + + +def test_relative_link_is_not_rewritten(app: Flask) -> None: + html = 'Dashboard' + result = process_html_links(html) + assert result == html + + +def test_no_double_redirect(app: Flask) -> None: + html = ( + 'Already redirected' + ) + result = process_html_links(html) + assert result.count("/redirect/") == 1 + + +def test_multiple_links(app: Flask) -> None: + html = ( + 'Bad' + 'Good' + 'Other' + ) + result = process_html_links(html) + assert result.count("/redirect/?url=") == 2 + assert "superset.example.com/x" in result + + +def test_disabled_via_config(app: Flask) -> None: + app.config["ALERT_REPORTS_ENABLE_LINK_REDIRECT"] = False + html = 'Click' + assert process_html_links(html) == html + + +def test_empty_html(app: Flask) -> None: + assert process_html_links("") == "" + assert process_html_links(" ") == " " + + +def test_no_base_url_configured(app: Flask) -> None: + app.config["WEBDRIVER_BASEURL"] = "" + app.config["WEBDRIVER_BASEURL_USER_FRIENDLY"] = "" + html = 'Click' + assert process_html_links(html) == html + + +def test_single_quoted_href(app: Flask) -> None: + html = "Click" + result = process_html_links(html) + assert "/redirect/?url=" in result + + +def test_html_without_links(app: Flask) -> None: + html = "

No links here

" + assert process_html_links(html) == html + + +# --------------------------------------------------------------------------- # +# is_safe_redirect_url +# --------------------------------------------------------------------------- # + + +def test_safe_internal_url(app: Flask) -> None: + assert is_safe_redirect_url("https://superset.example.com/dashboard/1") + + +def test_safe_relative_url(app: Flask) -> None: + assert is_safe_redirect_url("/dashboard/1") + + +def test_unsafe_external_url(app: Flask) -> None: + assert not is_safe_redirect_url("https://evil.com/phish") + + +def test_unsafe_javascript_scheme(app: Flask) -> None: + assert not is_safe_redirect_url("javascript:alert(1)") + + +def test_unsafe_protocol_relative(app: Flask) -> None: + assert not is_safe_redirect_url("//evil.com/x") + + +def test_unsafe_empty(app: Flask) -> None: + assert not is_safe_redirect_url("") + assert not is_safe_redirect_url(" ") + + +def test_unsafe_no_config(app: Flask) -> None: + app.config["WEBDRIVER_BASEURL"] = "" + app.config["WEBDRIVER_BASEURL_USER_FRIENDLY"] = "" + assert not is_safe_redirect_url("https://anything.com")