mirror of
https://github.com/apache/superset.git
synced 2026-04-19 08:04:53 +00:00
fix(reports): Use authenticated user as recipient for chart/dashboard reports (#36981)
This commit is contained in:
committed by
GitHub
parent
e9b6791ffb
commit
4f5789abfe
@@ -18,6 +18,7 @@ import logging
|
||||
from functools import partial
|
||||
from typing import Any, Optional
|
||||
|
||||
from flask import g
|
||||
from flask_babel import gettext as _
|
||||
from marshmallow import ValidationError
|
||||
|
||||
@@ -30,11 +31,13 @@ from superset.commands.report.exceptions import (
|
||||
ReportScheduleCreationMethodUniquenessValidationError,
|
||||
ReportScheduleInvalidError,
|
||||
ReportScheduleNameUniquenessValidationError,
|
||||
ReportScheduleUserEmailNotFoundError,
|
||||
)
|
||||
from superset.daos.database import DatabaseDAO
|
||||
from superset.daos.report import ReportScheduleDAO
|
||||
from superset.reports.models import (
|
||||
ReportCreationMethod,
|
||||
ReportRecipientType,
|
||||
ReportSchedule,
|
||||
ReportScheduleType,
|
||||
)
|
||||
@@ -54,6 +57,35 @@ class CreateReportScheduleCommand(CreateMixin, BaseReportScheduleCommand):
|
||||
self.validate()
|
||||
return ReportScheduleDAO.create(attributes=self._properties)
|
||||
|
||||
def _populate_recipients(self, exceptions: list[ValidationError]) -> None:
|
||||
"""
|
||||
Populate recipients based on creation method and current user.
|
||||
|
||||
For reports initiated from charts or dashboards, always use
|
||||
the current user's email as the recipient, ignoring any
|
||||
client-provided recipient values. Raises validation error if
|
||||
user has no email address.
|
||||
"""
|
||||
creation_method = self._properties.get("creation_method")
|
||||
|
||||
# For reports from charts/dashboards, always use current user
|
||||
if creation_method in (
|
||||
ReportCreationMethod.CHARTS,
|
||||
ReportCreationMethod.DASHBOARDS,
|
||||
):
|
||||
if hasattr(g, "user") and g.user and g.user.email:
|
||||
# Override any provided recipients with current user's email
|
||||
self._properties["recipients"] = [
|
||||
{
|
||||
"type": ReportRecipientType.EMAIL,
|
||||
"recipient_config_json": {"target": g.user.email},
|
||||
}
|
||||
]
|
||||
else:
|
||||
# User doesn't have an email address - can't create report
|
||||
exceptions.append(ReportScheduleUserEmailNotFoundError())
|
||||
# For creation from alerts_reports view, keep the recipients as provided
|
||||
|
||||
def validate(self) -> None:
|
||||
"""
|
||||
Validates the properties of a report schedule configuration, including uniqueness
|
||||
@@ -75,6 +107,9 @@ class CreateReportScheduleCommand(CreateMixin, BaseReportScheduleCommand):
|
||||
|
||||
exceptions: list[ValidationError] = []
|
||||
|
||||
# Populate recipients if needed (may add validation errors)
|
||||
self._populate_recipients(exceptions)
|
||||
|
||||
# Validate name type uniqueness
|
||||
if not ReportScheduleDAO.validate_update_uniqueness(name, report_type):
|
||||
exceptions.append(
|
||||
|
||||
@@ -309,3 +309,18 @@ class ReportScheduleForbiddenError(ForbiddenError):
|
||||
|
||||
class ReportSchedulePruneLogError(CommandException):
|
||||
message = _("An error occurred while pruning logs ")
|
||||
|
||||
|
||||
class ReportScheduleUserEmailNotFoundError(ValidationError):
|
||||
"""
|
||||
Validation error when user email is required but not found
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
super().__init__(
|
||||
_(
|
||||
"Unable to create report: User email address is required but not "
|
||||
"found. Please ensure your user profile has a valid email address."
|
||||
),
|
||||
field_name="recipients",
|
||||
)
|
||||
|
||||
@@ -224,7 +224,7 @@ class ReportSchedulePostSchema(Schema):
|
||||
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||
)
|
||||
|
||||
recipients = fields.List(fields.Nested(ReportRecipientSchema))
|
||||
recipients = fields.List(fields.Nested(ReportRecipientSchema), required=False)
|
||||
report_format = fields.String(
|
||||
dump_default=ReportDataFormat.PNG,
|
||||
validate=validate.OneOf(choices=tuple(key.value for key in ReportDataFormat)),
|
||||
|
||||
179
tests/unit_tests/commands/report/test_create_recipients.py
Normal file
179
tests/unit_tests/commands/report/test_create_recipients.py
Normal file
@@ -0,0 +1,179 @@
|
||||
# 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 unittest.mock import MagicMock, patch
|
||||
|
||||
from superset.commands.report.create import CreateReportScheduleCommand
|
||||
from superset.commands.report.exceptions import ReportScheduleUserEmailNotFoundError
|
||||
from superset.reports.models import (
|
||||
ReportCreationMethod,
|
||||
ReportRecipientType,
|
||||
)
|
||||
|
||||
|
||||
def test_populate_recipients_chart_creation_with_user_email() -> None:
|
||||
"""Test that chart/dashboard creation methods use current user's email."""
|
||||
with patch("superset.commands.report.create.g") as mock_g:
|
||||
# Setup user with email
|
||||
mock_user = MagicMock()
|
||||
mock_user.email = "user@example.com"
|
||||
mock_g.user = mock_user
|
||||
|
||||
# Create command instance
|
||||
command = CreateReportScheduleCommand({})
|
||||
command._properties = {
|
||||
"creation_method": ReportCreationMethod.CHARTS,
|
||||
"recipients": [
|
||||
{
|
||||
"type": ReportRecipientType.EMAIL,
|
||||
"recipient_config_json": {"target": "ignored@example.com"},
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
exceptions: list[Exception] = []
|
||||
command._populate_recipients(exceptions)
|
||||
|
||||
# Check that recipients were overridden
|
||||
assert len(command._properties["recipients"]) == 1
|
||||
assert command._properties["recipients"][0]["type"] == ReportRecipientType.EMAIL
|
||||
assert (
|
||||
command._properties["recipients"][0]["recipient_config_json"]["target"]
|
||||
== "user@example.com"
|
||||
)
|
||||
assert len(exceptions) == 0
|
||||
|
||||
|
||||
def test_populate_recipients_dashboard_creation_with_user_email() -> None:
|
||||
"""Test that dashboard creation uses current user's email."""
|
||||
with patch("superset.commands.report.create.g") as mock_g:
|
||||
# Setup user with email
|
||||
mock_user = MagicMock()
|
||||
mock_user.email = "dashboard_user@example.com"
|
||||
mock_g.user = mock_user
|
||||
|
||||
# Create command instance
|
||||
command = CreateReportScheduleCommand({})
|
||||
command._properties = {
|
||||
"creation_method": ReportCreationMethod.DASHBOARDS,
|
||||
# No recipients provided initially
|
||||
}
|
||||
|
||||
exceptions: list[Exception] = []
|
||||
command._populate_recipients(exceptions)
|
||||
|
||||
# Check that recipients were set
|
||||
assert len(command._properties["recipients"]) == 1
|
||||
assert command._properties["recipients"][0]["type"] == ReportRecipientType.EMAIL
|
||||
assert (
|
||||
command._properties["recipients"][0]["recipient_config_json"]["target"]
|
||||
== "dashboard_user@example.com"
|
||||
)
|
||||
assert len(exceptions) == 0
|
||||
|
||||
|
||||
def test_populate_recipients_alerts_reports_keeps_original() -> None:
|
||||
"""Test that alerts_reports creation method preserves provided recipients."""
|
||||
command = CreateReportScheduleCommand({})
|
||||
original_recipients = [
|
||||
{
|
||||
"type": ReportRecipientType.EMAIL,
|
||||
"recipient_config_json": {"target": "admin@example.com"},
|
||||
},
|
||||
{
|
||||
"type": ReportRecipientType.SLACK,
|
||||
"recipient_config_json": {"target": "#alerts"},
|
||||
},
|
||||
]
|
||||
command._properties = {
|
||||
"creation_method": ReportCreationMethod.ALERTS_REPORTS,
|
||||
"recipients": original_recipients,
|
||||
}
|
||||
|
||||
exceptions: list[Exception] = []
|
||||
command._populate_recipients(exceptions)
|
||||
|
||||
# Check that recipients were NOT changed
|
||||
assert command._properties["recipients"] == original_recipients
|
||||
assert len(exceptions) == 0
|
||||
|
||||
|
||||
def test_populate_recipients_chart_creation_no_user_email() -> None:
|
||||
"""Test that chart creation fails when user has no email."""
|
||||
with patch("superset.commands.report.create.g") as mock_g:
|
||||
# Setup user without email
|
||||
mock_user = MagicMock()
|
||||
mock_user.email = None
|
||||
mock_g.user = mock_user
|
||||
|
||||
command = CreateReportScheduleCommand({})
|
||||
command._properties = {
|
||||
"creation_method": ReportCreationMethod.CHARTS,
|
||||
}
|
||||
|
||||
exceptions: list[Exception] = []
|
||||
command._populate_recipients(exceptions)
|
||||
|
||||
# Check that validation error was added
|
||||
assert len(exceptions) == 1
|
||||
assert isinstance(exceptions[0], ReportScheduleUserEmailNotFoundError)
|
||||
# Recipients should not be set
|
||||
assert (
|
||||
"recipients" not in command._properties
|
||||
or command._properties["recipients"] == []
|
||||
)
|
||||
|
||||
|
||||
def test_populate_recipients_dashboard_creation_no_user() -> None:
|
||||
"""Test that dashboard creation fails when there's no user."""
|
||||
with patch("superset.commands.report.create.g") as mock_g:
|
||||
# No user in context
|
||||
mock_g.user = None
|
||||
|
||||
command = CreateReportScheduleCommand({})
|
||||
command._properties = {
|
||||
"creation_method": ReportCreationMethod.DASHBOARDS,
|
||||
}
|
||||
|
||||
exceptions: list[Exception] = []
|
||||
command._populate_recipients(exceptions)
|
||||
|
||||
# Check that validation error was added
|
||||
assert len(exceptions) == 1
|
||||
assert isinstance(exceptions[0], ReportScheduleUserEmailNotFoundError)
|
||||
|
||||
|
||||
def test_populate_recipients_no_creation_method() -> None:
|
||||
"""Test that recipients are unchanged when no creation_method is specified."""
|
||||
command = CreateReportScheduleCommand({})
|
||||
original_recipients = [
|
||||
{
|
||||
"type": ReportRecipientType.EMAIL,
|
||||
"recipient_config_json": {"target": "user@example.com"},
|
||||
}
|
||||
]
|
||||
command._properties = {
|
||||
# No creation_method specified
|
||||
"recipients": original_recipients,
|
||||
}
|
||||
|
||||
exceptions: list[Exception] = []
|
||||
command._populate_recipients(exceptions)
|
||||
|
||||
# Check that recipients were NOT changed
|
||||
assert command._properties["recipients"] == original_recipients
|
||||
assert len(exceptions) == 0
|
||||
Reference in New Issue
Block a user