iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(alerts): restrict list view and gamma perms (#21765)

This commit is contained in:
Ville Brofeldt 2022-10-15 10:03:26 +03:00 committed by GitHub
parent 196c3671e2
commit 4c1777f20d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 221 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
UPDATING.md
View File

@ -32,6 +32,8 @@ assists people when migrating to a new version.
### Breaking Changes
- [21765](https://github.com/apache/superset/pull/21765): For deployments that have enabled the "ALERT_REPORTS" feature flag, Gamma users will no longer have read and write access to Alerts & Reports by default. To give Gamma users the ability to schedule reports from the Dashboard and Explore view like before, create an additional role with "can read on ReportSchedule" and "can write on ReportSchedule" permissions. To further give Gamma users access to the "Alerts & Reports" menu and CRUD view, add "menu access on Manage" and "menu access on Alerts & Report" permissions to the role.
### Potential Downtime
### Other

4
superset-frontend/src/views/CRUD/alert/AlertList.test.jsx
View File

@ -55,7 +55,7 @@ const mockalerts = [...new Array(3)].map((_, i) => ({
last_eval_dttm: Date.now(),
last_state: 'ok',
name: `alert ${i} `,
owners: [],
owners: [{ id: 1 }],
recipients: [
{
id: `${i}`,
@ -67,6 +67,8 @@ const mockalerts = [...new Array(3)].map((_, i) => ({
const mockUser = {
userId: 1,
firstName: 'user 1',
lastName: 'lastname',
};
fetchMock.get(alertsEndpoint, {

37
superset-frontend/src/views/CRUD/alert/AlertList.tsx
View File

@ -44,6 +44,8 @@ import {
useSingleViewResource,
} from 'src/views/CRUD/hooks';
import { createErrorHandler, createFetchRelated } from 'src/views/CRUD/utils';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import Owner from 'src/types/Owner';
import AlertReportModal from './AlertReportModal';
import { AlertObject, AlertState } from './types';
@ -316,14 +318,21 @@ function AlertList({
size: 'xl',
},
{
Cell: ({ row: { original } }: any) => (
<Switch
data-test="toggle-active"
checked={original.active}
onClick={(checked: boolean) => toggleActive(original, checked)}
size="small"
/>
),
Cell: ({ row: { original } }: any) => {
const allowEdit =
original.owners.map((o: Owner) => o.id).includes(user.userId) ||
isUserAdmin(user);
return (
<Switch
disabled={!allowEdit}
data-test="toggle-active"
checked={original.active}
onClick={(checked: boolean) => toggleActive(original, checked)}
size="small"
/>
);
},
Header: t('Active'),
accessor: 'active',
id: 'active',
@ -337,6 +346,10 @@ function AlertList({
const handleGotoExecutionLog = () =>
history.push(`/${original.type.toLowerCase()}/${original.id}/log`);
const allowEdit =
original.owners.map((o: Owner) => o.id).includes(user.userId) ||
isUserAdmin(user);
const actions = [
canEdit
? {
@ -349,14 +362,14 @@ function AlertList({
: null,
canEdit
? {
label: 'edit-action',
tooltip: t('Edit'),
label: allowEdit ? 'edit-action' : 'preview-action',
tooltip: allowEdit ? t('Edit') : t('View'),
placement: 'bottom',
icon: 'Edit',
icon: allowEdit ? 'Edit' : 'Binoculars',
onClick: handleEdit,
}
: null,
canDelete
allowEdit && canDelete
? {
label: 'delete-action',
tooltip: t('Delete'),

6
superset/reports/api.py
View File

@ -43,7 +43,7 @@ from superset.reports.commands.exceptions import (
ReportScheduleUpdateFailedError,
)
from superset.reports.commands.update import UpdateReportScheduleCommand
from superset.reports.filters import ReportScheduleAllTextFilter
from superset.reports.filters import ReportScheduleAllTextFilter, ReportScheduleFilter
from superset.reports.models import ReportSchedule
from superset.reports.schemas import (
get_delete_ids_schema,
@ -80,6 +80,10 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
resource_name = "report"
allow_browser_login = True
base_filters = [
["id", ReportScheduleFilter, lambda: []],
]
show_columns = [
"id",
"active",

37
superset/reports/commands/execute.py
View File

@ -68,7 +68,7 @@ from superset.reports.notifications import create_notification
from superset.reports.notifications.base import NotificationContent
from superset.reports.notifications.exceptions import NotificationError
from superset.utils.celery import session_scope
from superset.utils.core import HeaderDataType
from superset.utils.core import HeaderDataType, override_user
from superset.utils.csv import get_chart_csv_data, get_chart_dataframe
from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
from superset.utils.urls import get_url_path
@ -77,6 +77,13 @@ from superset.utils.webdriver import DashboardStandaloneMode
logger = logging.getLogger(__name__)
def _get_user() -> User:
user = security_manager.find_user(username=app.config["THUMBNAIL_SELENIUM_USER"])
if not user:
raise ReportScheduleSelleniumUserNotFoundError()
return user
class BaseReportState:
current_states: List[ReportState] = []
initial: bool = False
@ -193,22 +200,13 @@ class BaseReportState:
**kwargs,
)
@staticmethod
def _get_user() -> User:
user = security_manager.find_user(
username=app.config["THUMBNAIL_SELENIUM_USER"]
)
if not user:
raise ReportScheduleSelleniumUserNotFoundError()
return user
def _get_screenshots(self) -> List[bytes]:
"""
Get chart or dashboard screenshots
:raises: ReportScheduleScreenshotFailedError
"""
url = self._get_url()
user = self._get_user()
user = _get_user()
if self._report_schedule.chart:
screenshot: Union[ChartScreenshot, DashboardScreenshot] = ChartScreenshot(
url,
@ -239,7 +237,7 @@ class BaseReportState:
def _get_csv_data(self) -> bytes:
url = self._get_url(result_format=ChartDataResultFormat.CSV)
auth_cookies = machine_auth_provider_factory.instance.get_auth_cookies(
self._get_user()
_get_user()
)
if self._report_schedule.chart.query_context is None:
@ -265,7 +263,7 @@ class BaseReportState:
"""
url = self._get_url(result_format=ChartDataResultFormat.JSON)
auth_cookies = machine_auth_provider_factory.instance.get_auth_cookies(
self._get_user()
_get_user()
)
if self._report_schedule.chart.query_context is None:
@ -679,12 +677,13 @@ class AsyncExecuteReportScheduleCommand(BaseCommand):
def run(self) -> None:
with session_scope(nullpool=True) as session:
try:
self.validate(session=session)
if not self._model:
raise ReportScheduleExecuteUnexpectedError()
ReportScheduleStateMachine(
session, self._execution_id, self._model, self._scheduled_dttm
).run()
with override_user(_get_user()):
self.validate(session=session)
if not self._model:
raise ReportScheduleExecuteUnexpectedError()
ReportScheduleStateMachine(
session, self._execution_id, self._model, self._scheduled_dttm
).run()
except CommandException as ex:
raise ex
except Exception as ex:

2
superset/reports/dao.py
View File

@ -26,6 +26,7 @@ from sqlalchemy.orm import Session
from superset.dao.base import BaseDAO
from superset.dao.exceptions import DAOCreateFailedError, DAODeleteFailedError
from superset.extensions import db
from superset.reports.filters import ReportScheduleFilter
from superset.reports.models import (
ReportExecutionLog,
ReportRecipients,
@ -43,6 +44,7 @@ REPORT_SCHEDULE_ERROR_NOTIFICATION_MARKER = "Notification sent with error"
class ReportScheduleDAO(BaseDAO):
model_cls = ReportSchedule
base_filter = ReportScheduleFilter
@staticmethod
def find_by_chart_id(chart_id: int) -> List[ReportSchedule]:

16
superset/reports/filters.py
View File

@ -20,10 +20,26 @@ from flask_babel import lazy_gettext as _
from sqlalchemy import or_
from sqlalchemy.orm.query import Query
from superset import db, security_manager
from superset.reports.models import ReportSchedule
from superset.views.base import BaseFilter
class ReportScheduleFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.can_access_all_datasources():
return query
owner_ids_query = (
db.session.query(ReportSchedule.id)
.join(ReportSchedule.owners)
.filter(
security_manager.user_model.id
== security_manager.user_model.get_user_id()
)
)
return query.filter(ReportSchedule.id.in_(owner_ids_query))
class ReportScheduleAllTextFilter(BaseFilter): # pylint: disable=too-few-public-methods
name = _("All Text")
arg_name = "report_all_text"

2
superset/security/manager.py
View File

@ -184,6 +184,8 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
"Queries",
"Import dashboards",
"Upload a CSV",
"ReportSchedule",
"Alerts & Report",
}
ADMIN_ONLY_PERMISSIONS = {

154
tests/integration_tests/reports/api_tests.py
View File

@ -23,9 +23,10 @@ import pytz
import pytest
import prison
from parameterized import parameterized
from sqlalchemy.sql import func
from superset import db
from superset import db, security_manager
from superset.models.core import Database
from superset.models.slice import Slice
from superset.models.dashboard import Dashboard
@ -48,11 +49,95 @@ from tests.integration_tests.fixtures.birth_names_dashboard import (
from tests.integration_tests.reports.utils import insert_report_schedule
REPORTS_COUNT = 10
REPORTS_ROLE_NAME = "reports_role"
REPORTS_GAMMA_USER = "reports_gamma"
class TestReportSchedulesApi(SupersetTestCase):
@pytest.fixture()
def create_working_report_schedule(self):
def gamma_user_with_alerts_role(self):
with self.create_app().app_context():
user = self.create_user(
REPORTS_GAMMA_USER,
"general",
"Gamma",
email=f"{REPORTS_GAMMA_USER}@superset.org",
)
security_manager.add_role(REPORTS_ROLE_NAME)
read_perm = security_manager.find_permission_view_menu(
"can_read",
"ReportSchedule",
)
write_perm = security_manager.find_permission_view_menu(
"can_write",
"ReportSchedule",
)
reports_role = security_manager.find_role(REPORTS_ROLE_NAME)
security_manager.add_permission_role(reports_role, read_perm)
security_manager.add_permission_role(reports_role, write_perm)
user.roles.append(reports_role)
yield user
# rollback changes (assuming cascade delete)
db.session.delete(reports_role)
db.session.delete(user)
db.session.commit()
@pytest.fixture()
def create_working_admin_report_schedule(self):
with self.create_app().app_context():
admin_user = self.get_user("admin")
chart = db.session.query(Slice).first()
example_db = get_example_database()
report_schedule = insert_report_schedule(
type=ReportScheduleType.ALERT,
name="name_admin_working",
crontab="* * * * *",
sql="SELECT value from table",
description="Report working",
chart=chart,
database=example_db,
owners=[admin_user],
last_state=ReportState.WORKING,
)
yield
db.session.delete(report_schedule)
db.session.commit()
@pytest.mark.usefixtures("gamma_user_with_alerts_role")
@pytest.fixture()
def create_working_gamma_report_schedule(self, gamma_user_with_alerts_role):
with self.create_app().app_context():
chart = db.session.query(Slice).first()
example_db = get_example_database()
report_schedule = insert_report_schedule(
type=ReportScheduleType.ALERT,
name="name_gamma_working",
crontab="* * * * *",
sql="SELECT value from table",
description="Report working",
chart=chart,
database=example_db,
owners=[gamma_user_with_alerts_role],
last_state=ReportState.WORKING,
)
yield
db.session.delete(report_schedule)
db.session.commit()
@pytest.mark.usefixtures("gamma_user_with_alerts_role")
@pytest.fixture()
def create_working_shared_report_schedule(self, gamma_user_with_alerts_role):
with self.create_app().app_context():
admin_user = self.get_user("admin")
@ -62,13 +147,13 @@ class TestReportSchedulesApi(SupersetTestCase):
report_schedule = insert_report_schedule(
type=ReportScheduleType.ALERT,
name="name_working",
name="name_shared_working",
crontab="* * * * *",
sql="SELECT value from table",
description="Report working",
chart=chart,
database=example_db,
owners=[admin_user, alpha_user],
owners=[admin_user, alpha_user, gamma_user_with_alerts_role],
last_state=ReportState.WORKING,
)
@ -305,6 +390,61 @@ class TestReportSchedulesApi(SupersetTestCase):
data_keys = sorted(list(data["result"][1]["recipients"][0].keys()))
assert expected_recipients_fields == data_keys
@parameterized.expand(
[
(
"admin",
{
"name_admin_working",
"name_gamma_working",
"name_shared_working",
},
),
(
"alpha",
{
"name_admin_working",
"name_gamma_working",
"name_shared_working",
},
),
(
REPORTS_GAMMA_USER,
{
"name_gamma_working",
"name_shared_working",
},
),
],
)
@pytest.mark.usefixtures(
"create_working_admin_report_schedule",
"create_working_gamma_report_schedule",
"create_working_shared_report_schedule",
"gamma_user_with_alerts_role",
)
def test_get_list_report_schedule_perms(self, username, report_names):
"""
ReportSchedule Api: Test get list report schedules for different roles
"""
self.login(username=username)
uri = f"api/v1/report/"
rv = self.get_assert_metric(uri, "get_list")
assert rv.status_code == 200
data = json.loads(rv.data.decode("utf-8"))
assert {report["name"] for report in data["result"]} == report_names
def test_get_list_report_schedule_gamma(self):
"""
ReportSchedule Api: Test get list report schedules for regular gamma user
"""
self.login(username="gamma")
uri = f"api/v1/report/"
rv = self.client.get(uri)
assert rv.status_code == 403
@pytest.mark.usefixtures("create_report_schedules")
def test_get_list_report_schedule_sorting(self):
"""
@ -1159,14 +1299,14 @@ class TestReportSchedulesApi(SupersetTestCase):
assert updated_model.chart_id == report_schedule_data["chart"]
assert updated_model.database_id == report_schedule_data["database"]
@pytest.mark.usefixtures("create_working_report_schedule")
@pytest.mark.usefixtures("create_working_shared_report_schedule")
def test_update_report_schedule_state_working(self):
"""
ReportSchedule Api: Test update state in a working report
"""
report_schedule = (
db.session.query(ReportSchedule)
.filter(ReportSchedule.name == "name_working")
.filter(ReportSchedule.name == "name_shared_working")
.one_or_none()
)
@ -1177,7 +1317,7 @@ class TestReportSchedulesApi(SupersetTestCase):
assert rv.status_code == 200
report_schedule = (
db.session.query(ReportSchedule)
.filter(ReportSchedule.name == "name_working")
.filter(ReportSchedule.name == "name_shared_working")
.one_or_none()
)
assert report_schedule.last_state == ReportState.NOOP

2
tests/integration_tests/reports/commands_tests.py
View File

@ -17,7 +17,7 @@
import json
from contextlib import contextmanager
from datetime import datetime, timedelta, timezone
from typing import Any, Dict, List, Optional
from typing import List, Optional
from unittest.mock import Mock, patch
from uuid import uuid4