fix(reports): enforce server-side recipient on chart/dashboard report subscriptions (#38847)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Shaitan
2026-05-20 18:36:42 +01:00
committed by GitHub
parent fbffae0444
commit 69adecd6a3
11 changed files with 514 additions and 35 deletions

View File

@@ -103,10 +103,12 @@ test('does not allow user to create a report without a name', () => {
});
test('creates a new email report via modal Add button', async () => {
// The modal now calls POST /api/v1/report/subscribe; creation_method, owners, and
// recipients are derived server-side — the client payload intentionally omits them.
fetchMock.post(
REPORT_ENDPOINT,
'glob:*/api/v1/report/subscribe',
{ id: 1, result: {} },
{ name: 'post-report' },
{ name: 'post-subscribe' },
);
render(<ReportModal {...defaultProps} />, { useRedux: true });
@@ -114,22 +116,22 @@ test('creates a new email report via modal Add button', async () => {
const addButton = screen.getByRole('button', { name: /add/i });
await waitFor(() => userEvent.click(addButton));
// Verify exactly one POST from the modal submit path
// Verify exactly one POST to the subscribe endpoint
await waitFor(() => {
const postCalls = fetchMock.callHistory.calls('post-report');
const postCalls = fetchMock.callHistory.calls('post-subscribe');
expect(postCalls).toHaveLength(1);
});
const postCalls = fetchMock.callHistory.calls('post-report');
const postCalls = fetchMock.callHistory.calls('post-subscribe');
const body = JSON.parse(postCalls[0].options.body as string);
expect(body.name).toBe('Weekly Report');
expect(body.type).toBe('Report');
expect(body.creation_method).toBe('dashboards');
expect(body.crontab).toBeDefined();
expect(body.recipients).toBeDefined();
expect(body.recipients[0].type).toBe('Email');
// creation_method, owners, and recipients are set server-side; not in the client payload
expect(body.creation_method).toBeUndefined();
expect(body.recipients).toBeUndefined();
fetchMock.removeRoute('post-report');
fetchMock.removeRoute('post-subscribe');
});
test('text-based chart hides screenshot width and shows message content', () => {

View File

@@ -174,6 +174,28 @@ export const addReport =
throw err;
});
export const SUBSCRIBE_REPORT = 'SUBSCRIBE_REPORT' as const;
export interface SubscribeReportAction {
type: typeof SUBSCRIBE_REPORT;
json: ReportApiJsonResponse;
}
export const subscribeReport =
(report: Partial<ReportObject>) => (dispatch: Dispatch<AnyAction>) =>
SupersetClient.post({
endpoint: `/api/v1/report/subscribe`,
jsonPayload: report,
})
.then(({ json }) => {
dispatch({ type: SUBSCRIBE_REPORT, json } as SubscribeReportAction);
dispatch(addSuccessToast(t('The report has been created')));
})
.catch(err => {
dispatch(addDangerToast(t('Failed to create report')));
throw err;
});
export const EDIT_REPORT = 'EDIT_REPORT' as const;
export interface EditReportAction {
@@ -255,5 +277,6 @@ export function deleteActiveReport(report: DeletableReport) {
export type ReportAction =
| SetReportAction
| AddReportAction
| SubscribeReportAction
| EditReportAction
| DeleteReportAction;

View File

@@ -31,8 +31,8 @@ import { Alert } from '@apache-superset/core/components';
import { SupersetTheme } from '@apache-superset/core/theme';
import { useDispatch, useSelector } from 'react-redux';
import {
addReport,
editReport,
subscribeReport,
} from 'src/features/reports/ReportModal/actions';
import {
Input,
@@ -179,26 +179,13 @@ function ReportModal({
}, [isEditMode, report]);
const onSave = async () => {
// Create new Report
const newReportValues: Partial<ReportObject> = {
const commonFields: Partial<ReportObject> = {
type: 'Report',
active: true,
force_screenshot: false,
custom_width: currentReport.custom_width,
creation_method: creationMethod,
dashboard: dashboardId,
chart: chart?.id,
owners: [userId],
recipients: [
{
recipient_config_json: {
target: userEmail,
ccTarget: ccEmail,
bccTarget: bccEmail,
},
type: 'Email',
},
],
name: currentReport.name,
description: currentReport.description,
crontab: currentReport.crontab,
@@ -209,12 +196,27 @@ function ReportModal({
setCurrentReport({ isSubmitting: true, error: undefined });
try {
if (isEditMode && currentReport.id) {
// Edit path: include all fields, PUT endpoint accepts recipients/owners directly
await dispatch(
editReport(currentReport.id, newReportValues as ReportObject),
editReport(currentReport.id, {
...commonFields,
creation_method: creationMethod,
owners: [userId],
recipients: [
{
recipient_config_json: {
target: userEmail,
ccTarget: ccEmail,
bccTarget: bccEmail,
},
type: 'Email',
},
],
} as ReportObject),
);
} else {
// Create new report (either not in edit mode, or edit mode without valid ID)
await dispatch(addReport(newReportValues as ReportObject));
// Subscribe path: creation_method, owners, and recipients are set server-side.
await dispatch(subscribeReport(commonFields as ReportObject));
}
onHide();
} catch (e) {

View File

@@ -21,11 +21,13 @@ import { omit } from 'lodash';
import {
SET_REPORT,
ADD_REPORT,
SUBSCRIBE_REPORT,
EDIT_REPORT,
DELETE_REPORT,
ReportAction,
SetReportAction,
AddReportAction,
SubscribeReportAction,
EditReportAction,
DeleteReportAction,
} from './actions';
@@ -105,6 +107,25 @@ export default function reportsReducer(
};
},
[SUBSCRIBE_REPORT]() {
const { result, id } = (action as SubscribeReportAction).json;
const report: ReportObject = { ...result, id } as ReportObject;
const creationMethod = report.creation_method as ReportCreationMethod;
const key = report.dashboard ?? report.chart;
if (key === undefined) {
return state;
}
return {
...state,
[creationMethod]: {
...state[creationMethod],
[key]: report,
},
};
},
[EDIT_REPORT]() {
const actionTyped = action as EditReportAction;
const report: ReportObject = {

View File

@@ -34,7 +34,10 @@ from superset.commands.report.exceptions import (
)
from superset.daos.chart import ChartDAO
from superset.daos.dashboard import DashboardDAO
from superset.reports.models import ReportCreationMethod, ReportScheduleType
from superset.reports.models import (
ReportCreationMethod,
ReportScheduleType,
)
from superset.reports.types import ReportScheduleExtra
from superset.utils import json

View File

@@ -33,12 +33,20 @@ from superset.commands.report.exceptions import (
ReportScheduleNameUniquenessValidationError,
ReportScheduleNotFoundError,
ReportScheduleUpdateFailedError,
ReportScheduleUserEmailNotFoundError,
)
from superset.daos.database import DatabaseDAO
from superset.daos.report import ReportScheduleDAO
from superset.exceptions import SupersetSecurityException
from superset.reports.models import ReportSchedule, ReportScheduleType, ReportState
from superset.reports.models import (
ReportCreationMethod,
ReportRecipientType,
ReportSchedule,
ReportScheduleType,
ReportState,
)
from superset.utils import json
from superset.utils.core import get_user_email
from superset.utils.decorators import on_error, transaction
logger = logging.getLogger(__name__)
@@ -89,6 +97,26 @@ class UpdateReportScheduleCommand(UpdateMixin, BaseReportScheduleCommand):
):
self._properties["last_state"] = ReportState.NOOP
# For reports created from charts or dashboards the recipient must always
# be the requesting user's own email address.
if (
self._model.creation_method
in (
ReportCreationMethod.CHARTS,
ReportCreationMethod.DASHBOARDS,
)
and "recipients" in self._properties
):
if user_email := get_user_email():
self._properties["recipients"] = [
{
"type": ReportRecipientType.EMAIL,
"recipient_config_json": {"target": user_email},
}
]
else:
exceptions.append(ReportScheduleUserEmailNotFoundError())
# Validate name/type uniqueness if either is changing
if name != self._model.name or report_type != self._model.type:
if not ReportScheduleDAO.validate_update_uniqueness(

View File

@@ -49,13 +49,14 @@ from superset.databases.filters import DatabaseFilter
from superset.exceptions import SupersetException
from superset.extensions import event_logger
from superset.reports.filters import ReportScheduleAllTextFilter, ReportScheduleFilter
from superset.reports.models import ReportSchedule
from superset.reports.models import ReportCreationMethod, ReportSchedule
from superset.reports.schemas import (
get_delete_ids_schema,
get_slack_channels_schema,
openapi_spec_methods_override,
ReportSchedulePostSchema,
ReportSchedulePutSchema,
ReportScheduleSubscribeSchema,
)
from superset.utils.slack import get_channels_with_search
from superset.views.base_api import (
@@ -82,6 +83,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
RouteMethod.RELATED,
"bulk_delete",
"slack_channels", # not using RouteMethod since locally defined
"subscribe",
}
class_permission_name = "ReportSchedule"
method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
@@ -198,6 +200,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
edit_columns = add_columns
add_model_schema = ReportSchedulePostSchema()
edit_model_schema = ReportSchedulePutSchema()
subscribe_schema = ReportScheduleSubscribeSchema()
order_columns = [
"active",
@@ -318,6 +321,80 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
)
return self.response_422(message=str(ex))
@expose("/subscribe", methods=("POST",))
@protect()
@statsd_metrics
@permission_name("subscribe")
@requires_json
def subscribe(self) -> Response:
"""Subscribe the current user to a chart or dashboard report.
---
post:
summary: Subscribe to a chart or dashboard report
description: >-
Creates a report schedule locked to the authenticated user's email.
``creation_method`` is derived server-side from the payload
(chart → charts, dashboard → dashboards). ``recipients`` are not
accepted and are always set to the requesting user's email address.
requestBody:
description: Report schedule subscription schema
required: true
content:
application/json:
schema:
$ref: '#/components/schemas/{{self.__class__.__name__}}.post'
responses:
201:
description: Report schedule subscription created
content:
application/json:
schema:
type: object
properties:
id:
type: number
result:
$ref: '#/components/schemas/{{self.__class__.__name__}}.post'
400:
$ref: '#/components/responses/400'
401:
$ref: '#/components/responses/401'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
try:
item = self.subscribe_schema.load(request.json)
except ValidationError as error:
return self.response_400(message=error.messages)
# Derive creation_method server-side from the payload
if item.get("dashboard") is not None:
item["creation_method"] = ReportCreationMethod.DASHBOARDS
elif item.get("chart") is not None:
item["creation_method"] = ReportCreationMethod.CHARTS
else:
return self.response_400(
message={"_schema": ["Either chart or dashboard is required"]}
)
try:
new_model = CreateReportScheduleCommand(item).run()
return self.response(201, id=new_model.id, result=item)
except ReportScheduleNotFoundError as ex:
return self.response_400(message=str(ex))
except ReportScheduleInvalidError as ex:
return self.response_422(message=ex.normalized_messages())
except ReportScheduleCreateFailedError as ex:
logger.error(
"Error creating report schedule %s: %s",
self.__class__.__name__,
str(ex),
exc_info=True,
)
return self.response_422(message=str(ex))
@expose("/", methods=("POST",))
@protect()
@statsd_metrics

View File

@@ -305,6 +305,30 @@ class ReportSchedulePostSchema(Schema):
)
class ReportScheduleSubscribeSchema(ReportSchedulePostSchema):
"""Schema for creating a chart/dashboard subscription.
``recipients`` and ``creation_method`` are excluded — both are set
server-side: recipients are locked to the authenticated user's email,
and creation_method is derived from the presence of ``chart`` or
``dashboard`` in the payload.
``type`` is restricted to ``Report`` — alert schedules cannot be
created through the subscribe endpoint.
"""
type = fields.String(
metadata={"description": type_description},
allow_none=False,
required=True,
validate=validate.OneOf(choices=[ReportScheduleType.REPORT.value]),
)
class Meta:
exclude = ("recipients", "creation_method", "owners")
unknown = EXCLUDE
class ReportSchedulePutSchema(Schema):
type = fields.String(
metadata={"description": type_description},

View File

@@ -337,7 +337,8 @@ class TestReportSchedulesApi(SupersetTestCase):
assert rv.status_code == 200
assert "can_read" in data["permissions"]
assert "can_write" in data["permissions"]
assert len(data["permissions"]) == 2
assert "can_subscribe" in data["permissions"]
assert len(data["permissions"]) == 3
@pytest.mark.usefixtures("create_report_schedules")
def test_get_report_schedule_not_found(self):

View File

@@ -14,11 +14,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""Unit tests for UpdateReportScheduleCommand.validate() database invariants."""
"""Unit tests for UpdateReportScheduleCommand.validate()."""
from __future__ import annotations
from unittest.mock import Mock
from unittest.mock import Mock, patch
import pytest
from pytest_mock import MockerFixture
@@ -30,7 +30,11 @@ from superset.commands.report.exceptions import (
from superset.commands.report.update import UpdateReportScheduleCommand
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetSecurityException
from superset.reports.models import ReportScheduleType, ReportState
from superset.reports.models import (
ReportCreationMethod,
ReportScheduleType,
ReportState,
)
def _make_model(
@@ -38,10 +42,12 @@ def _make_model(
*,
model_type: ReportScheduleType | str,
database_id: int | None,
creation_method: ReportCreationMethod = ReportCreationMethod.ALERTS_REPORTS,
) -> Mock:
model = mocker.Mock()
model.type = model_type
model.database_id = database_id
model.creation_method = creation_method
model.name = "test_schedule"
model.crontab = "0 9 * * *"
model.last_state = "noop"
@@ -257,6 +263,142 @@ def test_report_to_alert_with_db_accepted(mocker: MockerFixture) -> None:
cmd.validate() # should not raise
# --- Recipient enforcement for chart/dashboard reports ---
_PATCH_GET_USER_EMAIL = "superset.commands.report.update.get_user_email"
def test_chart_report_update_recipient_overridden_with_owner_email(
mocker: MockerFixture,
) -> None:
"""Updating recipients on a chart report always locks them to the owner's email."""
model = _make_model(
mocker,
model_type=ReportScheduleType.REPORT,
database_id=None,
creation_method=ReportCreationMethod.CHARTS,
)
_setup_mocks(mocker, model)
data = {
"recipients": [
{
"type": "Email",
"recipient_config_json": {"target": "other@example.com"},
}
]
}
cmd = UpdateReportScheduleCommand(model_id=1, data=data)
with patch(_PATCH_GET_USER_EMAIL, return_value="owner@example.com"):
cmd.validate()
recipients = cmd._properties["recipients"]
assert len(recipients) == 1
assert recipients[0]["recipient_config_json"]["target"] == "owner@example.com"
def test_dashboard_report_update_recipient_overridden_with_owner_email(
mocker: MockerFixture,
) -> None:
"""Updating recipients on a dashboard report locks them to the owner's email."""
model = _make_model(
mocker,
model_type=ReportScheduleType.REPORT,
database_id=None,
creation_method=ReportCreationMethod.DASHBOARDS,
)
_setup_mocks(mocker, model)
data = {
"recipients": [
{
"type": "Email",
"recipient_config_json": {"target": "other@example.com"},
}
]
}
cmd = UpdateReportScheduleCommand(model_id=1, data=data)
with patch(_PATCH_GET_USER_EMAIL, return_value="owner@example.com"):
cmd.validate()
recipients = cmd._properties["recipients"]
assert len(recipients) == 1
assert recipients[0]["recipient_config_json"]["target"] == "owner@example.com"
def test_alerts_reports_update_recipient_not_overridden(
mocker: MockerFixture,
) -> None:
"""Recipients on admin-created alerts/reports are not modified on update."""
model = _make_model(
mocker,
model_type=ReportScheduleType.REPORT,
database_id=None,
creation_method=ReportCreationMethod.ALERTS_REPORTS,
)
_setup_mocks(mocker, model)
original_recipient = {
"type": "Email",
"recipient_config_json": {"target": "team@example.com"},
}
cmd = UpdateReportScheduleCommand(
model_id=1, data={"recipients": [original_recipient]}
)
with patch(_PATCH_GET_USER_EMAIL, return_value="owner@example.com"):
cmd.validate()
assert (
cmd._properties["recipients"][0]["recipient_config_json"]["target"]
== "team@example.com"
)
def test_chart_report_update_no_recipients_in_payload_unchanged(
mocker: MockerFixture,
) -> None:
"""If recipients are not in the update payload, nothing is changed."""
model = _make_model(
mocker,
model_type=ReportScheduleType.REPORT,
database_id=None,
creation_method=ReportCreationMethod.CHARTS,
)
_setup_mocks(mocker, model)
cmd = UpdateReportScheduleCommand(model_id=1, data={"name": "new name"})
with patch(_PATCH_GET_USER_EMAIL, return_value="owner@example.com"):
cmd.validate()
assert "recipients" not in cmd._properties
def test_chart_report_update_no_user_email_raises(mocker: MockerFixture) -> None:
"""Update fails with a validation error when the user has no email address."""
model = _make_model(
mocker,
model_type=ReportScheduleType.REPORT,
database_id=None,
creation_method=ReportCreationMethod.CHARTS,
)
_setup_mocks(mocker, model)
cmd = UpdateReportScheduleCommand(
model_id=1,
data={
"recipients": [
{"type": "Email", "recipient_config_json": {"target": "x@y.com"}}
]
},
)
with patch(_PATCH_GET_USER_EMAIL, return_value=None):
with pytest.raises(ReportScheduleInvalidError) as exc_info:
cmd.validate()
messages = _get_validation_messages(exc_info)
assert "recipients" in messages
# --- Deactivation state reset ---

View File

@@ -19,7 +19,12 @@ import pytest
from marshmallow import ValidationError
from pytest_mock import MockerFixture
from superset.reports.schemas import ReportSchedulePostSchema, ReportSchedulePutSchema
from superset.reports.schemas import (
ReportRecipientSchema,
ReportSchedulePostSchema,
ReportSchedulePutSchema,
ReportScheduleSubscribeSchema,
)
def test_report_post_schema_custom_width_validation(mocker: MockerFixture) -> None:
@@ -77,6 +82,157 @@ def test_report_post_schema_custom_width_validation(mocker: MockerFixture) -> No
}
def test_report_recipient_schema_email_valid() -> None:
"""Valid email target is accepted by the recipient schema."""
schema = ReportRecipientSchema()
result = schema.load(
{
"type": "Email",
"recipient_config_json": {"target": "user@example.com"},
}
)
assert result["recipient_config_json"]["target"] == "user@example.com"
def test_report_recipient_schema_email_invalid_target() -> None:
"""Invalid email address in target field raises a validation error."""
schema = ReportRecipientSchema()
with pytest.raises(ValidationError) as excinfo:
schema.load(
{
"type": "Email",
"recipient_config_json": {"target": "not-an-email"},
}
)
assert "target" in excinfo.value.messages
def test_report_recipient_schema_email_invalid_cc() -> None:
"""Invalid address in ccTarget field raises a validation error."""
schema = ReportRecipientSchema()
with pytest.raises(ValidationError) as excinfo:
schema.load(
{
"type": "Email",
"recipient_config_json": {
"target": "user@example.com",
"ccTarget": "bad-email",
},
}
)
assert "ccTarget" in excinfo.value.messages
def test_report_recipient_schema_email_invalid_bcc() -> None:
"""Invalid address in bccTarget field raises a validation error."""
schema = ReportRecipientSchema()
with pytest.raises(ValidationError) as excinfo:
schema.load(
{
"type": "Email",
"recipient_config_json": {
"target": "user@example.com",
"bccTarget": "not-valid",
},
}
)
assert "bccTarget" in excinfo.value.messages
def test_report_recipient_schema_email_empty_bcc_allowed() -> None:
"""Empty string in bccTarget is accepted (optional field)."""
schema = ReportRecipientSchema()
result = schema.load(
{
"type": "Email",
"recipient_config_json": {
"target": "user@example.com",
"bccTarget": "",
},
}
)
assert result["recipient_config_json"]["target"] == "user@example.com"
def test_report_recipient_schema_email_empty_cc_allowed() -> None:
"""Empty string in ccTarget is accepted (optional field)."""
schema = ReportRecipientSchema()
result = schema.load(
{
"type": "Email",
"recipient_config_json": {
"target": "user@example.com",
"ccTarget": "",
},
}
)
assert result["recipient_config_json"]["target"] == "user@example.com"
def test_report_recipient_schema_slack_skips_email_validation() -> None:
"""Slack recipients are not validated as email addresses."""
schema = ReportRecipientSchema()
result = schema.load(
{
"type": "Slack",
"recipient_config_json": {"target": "#general"},
}
)
assert result["recipient_config_json"]["target"] == "#general"
def test_subscribe_schema_ignores_excluded_fields(mocker: MockerFixture) -> None:
"""Excluded fields sent by the client are silently dropped, not rejected."""
mocker.patch(
"flask.current_app.config",
{
"ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH": 100,
"ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH": 2000,
},
)
schema = ReportScheduleSubscribeSchema()
result = schema.load(
{
"type": "Report",
"name": "My subscription",
"crontab": "0 9 * * *",
"timezone": "UTC",
"chart": 1,
# These are excluded server-side — should be silently dropped
"recipients": [
{"type": "Email", "recipient_config_json": {"target": "x@y.com"}}
],
"creation_method": "alerts_reports",
}
)
assert "recipients" not in result
assert "creation_method" not in result
assert "owners" not in result
def test_subscribe_schema_rejects_alert_type(mocker: MockerFixture) -> None:
"""Subscribe endpoint must not allow Alert type — prevents privilege escalation."""
mocker.patch(
"flask.current_app.config",
{
"ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH": 100,
"ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH": 2000,
},
)
schema = ReportScheduleSubscribeSchema()
with pytest.raises(ValidationError) as exc_info:
schema.load(
{
"type": "Alert",
"name": "My alert",
"crontab": "0 9 * * *",
"timezone": "UTC",
"chart": 1,
}
)
assert "type" in exc_info.value.messages
MINIMAL_POST_PAYLOAD = {
"type": "Report",
"name": "A report",