From aede3bb5bab55cd89bb91e3188865d355cf60f11 Mon Sep 17 00:00:00 2001
From: Sam Firke <sfirke@users.noreply.github.com>
Date: Thu, 16 Oct 2025 16:33:37 -0400
Subject: [PATCH] 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 +++++++++++++++++--
 .../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 9bb10dfd1d0..67552a316de 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 140c0775aa3..b7610f6a2f1 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 56cd356a125..236291d96ec 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 cbe5ef0dcfd..86f72ef376b 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 78218744b36..31dd1e25db4 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 c6ea8116367..843a1102b09 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 c4ea0deaeb1..b569bc72d68 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))