Vitor-Avila commented on code in PR #44992: URL: https://github.com/apache/superset/pull/44992#discussion_r4203207742
########## superset/reports/utils.py: ########## @@ -0,0 +1,190 @@ +# 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. +""" +Helpers shared by the Alerts & Reports commands, DAO and execution engine. + +Recipient and interval helpers are shared by save-time and execution-time +validation. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable +from typing import Any, TYPE_CHECKING + +from croniter import croniter + +from superset import is_feature_enabled +from superset.commands.report.exceptions import ReportScheduleExecutorNotFoundError +from superset.reports.models import ReportRecipientType +from superset.tasks.types import ExecutorType +from superset.utils import json + +if TYPE_CHECKING: + from flask_appbuilder.security.sqla.models import User + + from superset.reports.models import ReportSchedule + +# Recipient fields inside ``recipient_config_json`` holding e-mail addresses. +EMAIL_RECIPIENT_FIELDS = ("target", "ccTarget", "bccTarget") +_EMAIL_SEPARATOR_RE = re.compile(r"[,;]") + +# Simple hostname validation for the allowed e-mail domains configuration. +EMAIL_DOMAIN_REGEX = re.compile( + r"^(?!\*\..*\.\*$)(?:\*\.)?(?=.{1,253}$)([a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?\.)+" + r"(?:[a-zA-Z]{2,63}|\*)$" +) + + +def get_dynamic_executor( + report_schedule: ReportSchedule, + *, + alert_query: bool = False, +) -> User | None: + """ + Return the user explicitly configured to execute ``report_schedule``. + + When ``alert_query`` is set, a typed query executor is preferred; an unset + query type inherits the content selection. An unset content type returns + ``None`` so callers use ``ALERT_REPORTS_EXECUTORS``. The type marker is + authoritative even if a stale user relationship is present. + + The returned user may be inactive; callers decide how to surface that. + """ + if not is_feature_enabled("ALERT_REPORT_DYNAMIC_EXECUTOR"): + return None + field = "run_as" + if alert_query and report_schedule.run_alert_query_as_type is not None: + field = "run_alert_query_as" + executor_type = getattr(report_schedule, f"{field}_type") + if executor_type is None: + return None + if executor_type != ExecutorType.FIXED_USER: + raise ReportScheduleExecutorNotFoundError(str(executor_type)) + user = getattr(report_schedule, field) + if user is None: + raise ReportScheduleExecutorNotFoundError("deleted user") + return user + + +def split_email_addresses(value: str | None) -> list[str]: Review Comment: Fixed ########## docs/admin_docs/configuration/alerts-reports.mdx: ########## @@ -418,6 +418,111 @@ ALERT_REPORTS_EXECUTORS = [FixedExecutor("admin")] Please refer to `ExecutorType` in the codebase for other executor types. +#### Per-schedule "Run As" executor + +`ALERT_REPORTS_EXECUTORS` is a global, server-side setting that is invisible to the people +configuring alerts and reports, which makes permission problems hard to diagnose and is +especially painful for databases authenticated with OAuth2, where the impersonated account +must hold a valid token. [SIP-209](https://github.com/apache/superset/issues/39642) +introduces an explicit, per-schedule alternative behind the `ALERT_REPORT_DYNAMIC_EXECUTOR` +[feature flag](/admin-docs/configuration/configuring-superset#feature-flags): + +```python +FEATURE_FLAGS = { + "ALERT_REPORTS": True, + "ALERT_REPORT_DYNAMIC_EXECUTOR": True, +} +``` + +When the flag is enabled: + +- Every alert/report gets a **Run as** field recording which user's RBAC permissions and + database credentials are used to render the content. Alerts get an additional + **Run alert query as** field for the alert condition SQL, so the audience of an alert + does not need direct database access. +- Admins can select **Application default** or **Specific user**: + * **Application default:** resolves through `ALERT_REPORTS_EXECUTORS` value. + * **Specific user:** User picker option. + Non-admins can only select themselves. They see no user search and the API rejects + selecting another user. Reports scheduled from a dashboard or chart always execute + as the subscribing user. Review Comment: good catch as well! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
