This is an automated email from the ASF dual-hosted git repository.
sfirke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new aede3bb5ba fix(auth): redirect anonymous attempts to view dashboard
with next (#35345)
aede3bb5ba is described below
commit aede3bb5bab55cd89bb91e3188865d355cf60f11
Author: Sam Firke <[email protected]>
AuthorDate: Thu Oct 16 16:33:37 2025 -0400
fix(auth): redirect anonymous attempts to view dashboard with next (#35345)
---
superset-frontend/src/pages/Login/index.tsx | 28 ++++++--
superset/views/core.py | 17 +++--
superset/views/error_handling.py | 17 +++--
superset/views/utils.py | 34 +++++++++-
tests/integration_tests/dashboard_tests.py | 74 ++++++++++++++++++++--
.../dashboards/security/security_rbac_tests.py | 8 ++-
.../dashboards/superset_factory_util.py | 2 +
7 files changed, 155 insertions(+), 25 deletions(-)
diff --git a/superset-frontend/src/pages/Login/index.tsx
b/superset-frontend/src/pages/Login/index.tsx
index 9bb10dfd1d..67552a316d 100644
--- a/superset-frontend/src/pages/Login/index.tsx
+++ b/superset-frontend/src/pages/Login/index.tsx
@@ -27,7 +27,7 @@ import {
Typography,
Icons,
} from '@superset-ui/core/components';
-import { useState, useEffect } from 'react';
+import { useState, useEffect, useMemo } from 'react';
import { capitalize } from 'lodash/fp';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import { useDispatch } from 'react-redux';
@@ -82,6 +82,26 @@ export default function Login() {
const dispatch = useDispatch();
const bootstrapData = getBootstrapData();
+ const nextUrl = useMemo(() => {
+ try {
+ const params = new URLSearchParams(window.location.search);
+ return params.get('next') || '';
+ } catch (_error) {
+ return '';
+ }
+ }, []);
+
+ const loginEndpoint = useMemo(
+ () => (nextUrl ? `/login/?next=${encodeURIComponent(nextUrl)}` :
'/login/'),
+ [nextUrl],
+ );
+
+ const buildProviderLoginUrl = (providerName: string) => {
+ const base = `/login/${providerName}`;
+ return nextUrl
+ ? `${base}${base.includes('?') ? '&' :
'?'}next=${encodeURIComponent(nextUrl)}`
+ : base;
+ };
const authType: AuthType = bootstrapData.common.conf.AUTH_TYPE;
const providers: Provider[] = bootstrapData.common.conf.AUTH_PROVIDERS;
@@ -109,7 +129,7 @@ export default function Login() {
sessionStorage.setItem('login_attempted', 'true');
// Use standard form submission for Flask-AppBuilder compatibility
- SupersetClient.postForm('/login/', values, '');
+ SupersetClient.postForm(loginEndpoint, values, '');
};
const getAuthIconElement = (
@@ -146,7 +166,7 @@ export default function Login() {
{providers.map((provider: OIDProvider) => (
<Form.Item<LoginForm>>
<Button
- href={`/login/${provider.name}`}
+ href={buildProviderLoginUrl(provider.name)}
block
iconPosition="start"
icon={getAuthIconElement(provider.name)}
@@ -164,7 +184,7 @@ export default function Login() {
{providers.map((provider: OAuthProvider) => (
<Form.Item<LoginForm>>
<Button
- href={`/login/${provider.name}`}
+ href={buildProviderLoginUrl(provider.name)}
block
iconPosition="start"
icon={getAuthIconElement(provider.name)}
diff --git a/superset/views/core.py b/superset/views/core.py
index 140c0775aa..b7610f6a2f 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -46,7 +46,6 @@ from sqlalchemy.exc import SQLAlchemyError
from werkzeug.utils import safe_join
from superset import (
- appbuilder,
db,
event_logger,
is_feature_enabled,
@@ -80,6 +79,7 @@ from superset.models.slice import Slice
from superset.models.sql_lab import Query
from superset.models.user_attributes import UserAttribute
from superset.superset_typing import FlaskResponse
+from superset.tasks.utils import get_current_user
from superset.utils import core as utils, json
from superset.utils.cache import etag_cache
from superset.utils.core import (
@@ -108,6 +108,7 @@ from superset.views.utils import (
get_form_data,
get_viz,
loads_request_json,
+ redirect_to_login,
sanitize_datasource_data,
)
from superset.viz import BaseViz
@@ -765,13 +766,21 @@ class Superset(BaseSupersetView):
dashboard = Dashboard.get(dashboard_id_or_slug)
if not dashboard:
+ if not get_current_user():
+ return redirect_to_login()
abort(404)
+ # Redirect anonymous users to login for unpublished dashboards,
+ # in the edge case where a dataset has been shared with public
+ if not get_current_user() and not dashboard.published:
+ return redirect_to_login()
+
try:
dashboard.raise_for_access()
except SupersetSecurityException:
- # Return 404 to avoid revealing dashboard existence
- return Response(status=404)
+ if not get_current_user():
+ return redirect_to_login()
+ abort(404)
add_extra_log_payload(
dashboard_id=dashboard.id,
dashboard_version="v2",
@@ -882,7 +891,7 @@ class Superset(BaseSupersetView):
def welcome(self) -> FlaskResponse:
"""Personalized welcome page"""
if not g.user or not get_user_id():
- return redirect(appbuilder.get_url_for_login)
+ return redirect_to_login()
if welcome_dashboard_id := (
db.session.query(UserAttribute.welcome_dashboard_id)
diff --git a/superset/views/error_handling.py b/superset/views/error_handling.py
index 56cd356a12..236291d96e 100644
--- a/superset/views/error_handling.py
+++ b/superset/views/error_handling.py
@@ -25,7 +25,6 @@ from typing import Any, Callable, cast
from flask import (
Flask,
- redirect,
request,
Response,
send_file,
@@ -34,7 +33,6 @@ from flask_wtf.csrf import CSRFError
from sqlalchemy import exc
from werkzeug.exceptions import HTTPException
-from superset import appbuilder
from superset.commands.exceptions import CommandException, CommandInvalidError
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import (
@@ -46,6 +44,7 @@ from superset.exceptions import (
from superset.superset_typing import FlaskResponse
from superset.utils import core as utils, json
from superset.utils.log import get_logger_from_status
+from superset.views.utils import redirect_to_login
if typing.TYPE_CHECKING:
from superset.views.base import BaseSupersetView
@@ -153,7 +152,7 @@ def set_app_error_handlers(app: Flask) -> None: # noqa:
C901
if request.is_json:
return show_http_exception(ex)
- return redirect(appbuilder.get_url_for_login)
+ return redirect_to_login()
@app.errorhandler(HTTPException)
def show_http_exception(ex: HTTPException) -> FlaskResponse:
@@ -165,7 +164,11 @@ def set_app_error_handlers(app: Flask) -> None: # noqa:
C901
and ex.code in {404, 500}
):
path = files("superset") / f"static/assets/{ex.code}.html"
- return send_file(path, max_age=0), ex.code
+ # Try to serve HTML file; fall back to JSON if not built
+ try:
+ return send_file(path, max_age=0), ex.code
+ except FileNotFoundError:
+ pass
return json_error_response(
[
@@ -189,7 +192,11 @@ def set_app_error_handlers(app: Flask) -> None: # noqa:
C901
if "text/html" in request.accept_mimetypes and not app.config["DEBUG"]:
path = files("superset") / "static/assets/500.html"
- return send_file(path, max_age=0), 500
+ # Try to serve HTML file; fall back to JSON if not built
+ try:
+ return send_file(path, max_age=0), 500
+ except FileNotFoundError:
+ pass
extra = ex.normalized_messages() if isinstance(ex,
CommandInvalidError) else {}
return json_error_response(
diff --git a/superset/views/utils.py b/superset/views/utils.py
index cbe5ef0dcf..86f72ef376 100644
--- a/superset/views/utils.py
+++ b/superset/views/utils.py
@@ -19,16 +19,17 @@ import logging
from collections import defaultdict
from functools import wraps
from typing import Any, Callable, DefaultDict, Optional, Union
+from urllib import parse
import msgpack
import pyarrow as pa
-from flask import current_app as app, g, has_request_context, request
+from flask import current_app as app, g, has_request_context, redirect, request
from flask_appbuilder.security.sqla import models as ab_models
from flask_appbuilder.security.sqla.models import User
from flask_babel import _
from sqlalchemy.exc import NoResultFound
-from superset import dataframe, db, result_set, viz
+from superset import appbuilder, dataframe, db, result_set, viz
from superset.common.db_query_status import QueryStatus
from superset.daos.datasource import DatasourceDAO
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
@@ -44,7 +45,7 @@ from superset.models.core import Database
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import Query
-from superset.superset_typing import FormData
+from superset.superset_typing import FlaskResponse, FormData
from superset.utils import json
from superset.utils.core import DatasourceType
from superset.utils.decorators import stats_timing
@@ -58,6 +59,33 @@ if not
feature_flag_manager.is_feature_enabled("ENABLE_JAVASCRIPT_CONTROLS"):
REJECTED_FORM_DATA_KEYS = ["js_tooltip", "js_onclick_href",
"js_data_mutator"]
+def redirect_to_login(next_target: str | None = None) -> FlaskResponse:
+ """Return a redirect response to the login view, preserving target URL.
+
+ When ``next_target`` is ``None`` the current request path (including query
+ string) is used, provided a request context is available. The resulting URL
+ always remains relative, mirroring Flask-AppBuilder expectations.
+ """
+
+ login_url = appbuilder.get_url_for_login
+ parsed = parse.urlparse(login_url)
+ query = parse.parse_qs(parsed.query, keep_blank_values=True)
+
+ target = next_target
+ if target is None and has_request_context():
+ if request.query_string:
+ target = request.full_path.rstrip("?")
+ else:
+ target = request.path
+
+ if target:
+ query["next"] = [target]
+
+ encoded_query = parse.urlencode(query, doseq=True)
+ redirect_url = parse.urlunparse(parsed._replace(query=encoded_query))
+ return redirect(redirect_url)
+
+
def sanitize_datasource_data(datasource_data: dict[str, Any]) -> dict[str,
Any]:
if datasource_data:
datasource_database = datasource_data.get("database")
diff --git a/tests/integration_tests/dashboard_tests.py
b/tests/integration_tests/dashboard_tests.py
index 78218744b3..31dd1e25db 100644
--- a/tests/integration_tests/dashboard_tests.py
+++ b/tests/integration_tests/dashboard_tests.py
@@ -18,8 +18,8 @@
"""Unit tests for Superset"""
import re
-import unittest
from random import random
+from urllib.parse import parse_qs, urlparse
import pytest
from flask import Response, escape, url_for
@@ -52,7 +52,7 @@ from tests.integration_tests.fixtures.world_bank_dashboard
import (
load_world_bank_data, # noqa: F401
)
-from .base_tests import SupersetTestCase
+from .base_tests import DEFAULT_PASSWORD, SupersetTestCase
class TestDashboard(SupersetTestCase):
@@ -186,6 +186,72 @@ class TestDashboard(SupersetTestCase):
# Cleanup
self.revoke_public_access_to_table(table)
+ @pytest.mark.usefixtures(
+ "load_energy_table_with_slice",
+ "load_dashboard",
+ )
+ def test_anonymous_user_redirects_to_login_with_next(self):
+ self.logout()
+ target_path = f"/superset/dashboard/{pytest.hidden_dash_slug}/"
+
+ response = self.client.get(target_path, follow_redirects=False)
+
+ assert response.status_code == 302
+
+ redirect_location = response.headers["Location"]
+ parsed = urlparse(redirect_location)
+ assert parsed.path.rstrip("/") == "/login"
+
+ next_values = parse_qs(parsed.query).get("next")
+ assert next_values is not None
+ assert next_values[0].endswith(target_path)
+
+ login_target = (
+ f"{parsed.path}{'?' + parsed.query if parsed.query else ''}"
+ if parsed.scheme or parsed.netloc
+ else redirect_location
+ )
+
+ login_response = self.client.post(
+ login_target,
+ data={"username": ADMIN_USERNAME, "password": DEFAULT_PASSWORD},
+ follow_redirects=False,
+ )
+ assert login_response.status_code == 302
+ assert login_response.headers["Location"].endswith(target_path)
+
+ target_response: Response = self.client.get(target_path,
follow_redirects=False)
+ assert target_response.status_code == 200
+
+ def test_anonymous_user_redirects_to_login_for_missing_dashboard(self):
+ self.logout()
+ target_path = "/superset/dashboard/nonexistent-dashboard/"
+
+ response = self.client.get(target_path, follow_redirects=False)
+
+ assert response.status_code == 302
+ parsed = urlparse(response.headers["Location"])
+ assert parsed.path.rstrip("/") == "/login"
+ next_values = parse_qs(parsed.query).get("next")
+ assert next_values is not None
+ assert next_values[0].endswith(target_path)
+
+ @pytest.mark.usefixtures(
+ "public_role_like_gamma",
+ "load_energy_table_with_slice",
+ "load_dashboard",
+ )
+ def test_authenticated_user_without_access_gets_404(self):
+ self.login(GAMMA_USERNAME)
+ target_path = f"/superset/dashboard/{pytest.hidden_dash_slug}/"
+
+ response = self.client.get(
+ target_path,
+ follow_redirects=False,
+ headers={"Accept": "text/html"},
+ )
+ assert response.status_code == 404
+
@pytest.mark.usefixtures(
"public_role_like_gamma",
"load_energy_table_with_slice",
@@ -248,7 +314,3 @@ class TestDashboard(SupersetTestCase):
db.session.commit()
assert f"/superset/dashboard/{slug}/" not in resp
-
-
-if __name__ == "__main__":
- unittest.main()
diff --git a/tests/integration_tests/dashboards/security/security_rbac_tests.py
b/tests/integration_tests/dashboards/security/security_rbac_tests.py
index c6ea811636..843a1102b0 100644
--- a/tests/integration_tests/dashboards/security/security_rbac_tests.py
+++ b/tests/integration_tests/dashboards/security/security_rbac_tests.py
@@ -108,7 +108,7 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
# act
response = self.get_dashboard_view_response(dashboard_to_access)
- assert response.status_code == 404
+ assert response.status_code == 404 # Authenticated users without
access get 404
request_payload = get_query_context("birth_names")
rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
@@ -221,7 +221,8 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
- assert response.status_code == 404
+ # Anonymous users are redirected to login instead of getting 404
+ assert response.status_code == 302
@pytest.mark.usefixtures("public_role_like_gamma")
def
test_get_dashboard_view__public_user_with_dashboard_permission_can_not_access_draft(
# noqa: E501
@@ -234,7 +235,8 @@ class
TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
response = self.get_dashboard_view_response(dashboard_to_access)
# assert
- assert response.status_code == 404
+ # Anonymous users are redirected to login for unpublished dashboards
+ assert response.status_code == 302
# post
revoke_access_to_dashboard(dashboard_to_access, "Public") # noqa: F405
diff --git a/tests/integration_tests/dashboards/superset_factory_util.py
b/tests/integration_tests/dashboards/superset_factory_util.py
index c4ea0deaeb..b569bc72d6 100644
--- a/tests/integration_tests/dashboards/superset_factory_util.py
+++ b/tests/integration_tests/dashboards/superset_factory_util.py
@@ -190,6 +190,8 @@ def delete_all_inserted_objects() -> None:
def delete_all_inserted_dashboards():
try:
+ # Expire all objects to ensure fresh state after potential rollbacks
+ db.session.expire_all()
dashboards_to_delete: list[Dashboard] = (
db.session.query(Dashboard)
.filter(Dashboard.id.in_(inserted_dashboards_ids))
