diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 9484cbb5..b5158c45 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,3 +1,6 @@ * @grafana/grafana-oncall-backend +# don't tag @grafana/grafana-oncall-backend on changes to CHANGELOG.md +CHANGELOG.md + /grafana-plugin @grafana/grafana-oncall-frontend diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7e427978..d86df0f6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pycqa/isort - rev: 5.9.3 + rev: 5.12.0 hooks: - id: isort files: ^engine diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b1790de..38ac1685 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## v1.1.20 (2023-01-30) + +### Added + +- Add involved users filter to alert groups listing page (+ mine shortcut) ### Changed @@ -14,11 +18,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix bugs related to creating contact point for Grafana Alerting integration +- Fix minor UI bug on OnCall users page where it would idefinitely show a "Loading..." message +- Only show OnCall user's table to users that are authorized +- Fixed NPE in ScheduleUserDetails component ([#1229](https://github.com/grafana/oncall/issues/1229)) ## v1.1.19 (2023-01-25) ### Added +- Add Server URL below QR code for OSS for debugging purposes - Add Slack slash command allowing to trigger a direct page via a manually created alert group - Remove resolved and acknowledged filters as we switched to status ([#1201](https://github.com/grafana/oncall/pull/1201)) - Add sync with grafana on /users and /teams api calls from terraform plugin diff --git a/docs/.markdownlint.json b/docs/.markdownlint.json index 864e4431..a51e7c7e 100644 --- a/docs/.markdownlint.json +++ b/docs/.markdownlint.json @@ -1,7 +1,7 @@ { "extends": "../.markdownlint.json", "MD013": { - "line_length": "140" + "line_length": "160" }, "MD025": false, "MD036": false diff --git a/docs/sources/integrations/chatops-integrations/configure-slack/index.md b/docs/sources/integrations/chatops-integrations/configure-slack/index.md index 75aff6d9..e9595ba6 100644 --- a/docs/sources/integrations/chatops-integrations/configure-slack/index.md +++ b/docs/sources/integrations/chatops-integrations/configure-slack/index.md @@ -81,3 +81,26 @@ teams of their on-call shifts. Admins can configure shift notification behavior 1. When an on-call shift notification is sent to a person or channel, click the gear icon to access **Notifications preferences**. 2. Configure on-call notifications for future shift notifications. + +## Slack commands and message shortcuts + +The Grafana OnCall Slack app includes helpful message shortcuts and slash commands. + +### Slack commands + +Use the `/oncall` Slack command to create a new alert group directly from Slack. + +1. Type `/oncall` in the message box of the desired Slack channel then click **Send**. +1. Fill out the **Start New Escalation** creation form then click **Submit**. +1. Once the Grafana OnCall app sends a Slack message with the newly created alert, the alert group is open and firing. + +### Message shortcuts + +Use message shortcuts to add resolution notes directly from Slack. Message shortcuts are available in the More actions menu from any message. + +>**Note:** In order to associate the resolution note to an alert group, this message shortcut can only be applied to messages in the thread of an alert group. + +1. From an alert group thread, navigate to the Slack message that you wish to add as a resolution note. +1. Hover over the message and select **More actions** from the menu options. +1. Select **Add as resolution note**. +1. The Grafana OnCall app will react to the message in Slack with the memo emoji and add the message to the alert group timeline. diff --git a/engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py b/engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py new file mode 100644 index 00000000..690f9e4b --- /dev/null +++ b/engine/apps/alerts/migrations/0009_alertreceivechannel_web_templates_modified_at.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.16 on 2023-01-27 07:41 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('alerts', '0008_alter_alertgrouplogrecord_type'), + ] + + operations = [ + migrations.AddField( + model_name='alertreceivechannel', + name='web_templates_modified_at', + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/engine/apps/alerts/models/alert_receive_channel.py b/engine/apps/alerts/models/alert_receive_channel.py index 076d9469..edafbf34 100644 --- a/engine/apps/alerts/models/alert_receive_channel.py +++ b/engine/apps/alerts/models/alert_receive_channel.py @@ -160,6 +160,7 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject): web_title_template = models.TextField(null=True, default=None) web_message_template = models.TextField(null=True, default=None) web_image_url_template = models.TextField(null=True, default=None) + web_templates_modified_at = models.DateTimeField(blank=True, null=True) # email related fields are deprecated in favour of messaging backend based templates # these templates are stored in the messaging_backends_templates field diff --git a/engine/apps/alerts/paging.py b/engine/apps/alerts/paging.py index 12cb0b71..907f1e86 100644 --- a/engine/apps/alerts/paging.py +++ b/engine/apps/alerts/paging.py @@ -1,4 +1,4 @@ -from typing import Any +from typing import Any, Optional from uuid import uuid4 from django.db import transaction @@ -142,7 +142,7 @@ def direct_paging( schedules: ScheduleNotifications = None, escalation_chain: EscalationChain = None, alert_group: AlertGroup = None, -) -> None: +) -> Optional[AlertGroup]: """Trigger escalation targeting given users/schedules. If an alert group is given, update escalation to include the specified users. @@ -185,6 +185,8 @@ def direct_paging( ) notify_user_task.apply_async((u.pk, alert_group.pk), {"important": important}) + return alert_group + def unpage_user(alert_group: AlertGroup, user: User, from_user: User) -> None: """Remove user from alert group escalation.""" diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index e73ccc88..2655d55e 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -279,6 +279,19 @@ def test_direct_paging_custom_chain( assert ag.escalation_chain_with_respect_to_escalation_snapshot == custom_chain +@pytest.mark.django_db +def test_direct_paging_returns_alert_group(make_organization, make_user_for_organization): + organization = make_organization() + user = make_user_for_organization(organization) + from_user = make_user_for_organization(organization) + + with patch("apps.alerts.paging.notify_user_task"): + alert_group = direct_paging(organization, None, from_user, title="Help!", message="Fire", users=[(user, False)]) + + # check alert group returned by direct paging is the same as the one created + assert alert_group == AlertGroup.all_objects.get() + + @pytest.mark.django_db def test_unpage_user_not_exists( make_organization, make_user_for_organization, make_alert_receive_channel, make_alert_group diff --git a/engine/apps/api/serializers/alert_group.py b/engine/apps/api/serializers/alert_group.py index f25cd39b..5f3c854e 100644 --- a/engine/apps/api/serializers/alert_group.py +++ b/engine/apps/api/serializers/alert_group.py @@ -1,5 +1,7 @@ import logging +from django.core.cache import cache +from django.utils import timezone from rest_framework import serializers from apps.alerts.incident_appearance.renderers.classic_markdown_renderer import AlertGroupClassicMarkdownRenderer @@ -13,6 +15,7 @@ from .alert_receive_channel import FastAlertReceiveChannelSerializer from .user import FastUserSerializer logger = logging.getLogger(__name__) +logger.setLevel(logging.DEBUG) class ShortAlertGroupSerializer(serializers.ModelSerializer): @@ -92,10 +95,33 @@ class AlertGroupListSerializer(EagerLoadingMixin, serializers.ModelSerializer): if not obj.last_alert: return {} - return AlertGroupWebRenderer(obj, obj.last_alert).render() + web_templates_modified_at = obj.channel.web_templates_modified_at + last_alert_created_at = obj.last_alert.created_at + + CACHE_KEY = f"render_for_web_alert_group_{obj.id}" + CACHE_LIFEIME = 60 * 60 * 24 + cached_render_for_web = cache.get(CACHE_KEY, None) + + # use cache only if cache exists + # and cache was created after the last alert created + # and either web templates never modified + # or cache was created after templates were modified + if ( + cached_render_for_web is not None + and cached_render_for_web.get("cache_created_at") > last_alert_created_at + and ( + web_templates_modified_at is None + or cached_render_for_web.get("cache_created_at") > web_templates_modified_at + ) + ): + render_for_web = cached_render_for_web.get("render_for_web") + else: + render_for_web = AlertGroupWebRenderer(obj, obj.last_alert).render() + cache.set(CACHE_KEY, {"cache_created_at": timezone.now(), "render_for_web": render_for_web}, CACHE_LIFEIME) + + return render_for_web def get_render_for_classic_markdown(self, obj): - # alert group has no alerts if not obj.last_alert: return {} diff --git a/engine/apps/api/serializers/alert_receive_channel.py b/engine/apps/api/serializers/alert_receive_channel.py index 0df3d11d..b0e52c50 100644 --- a/engine/apps/api/serializers/alert_receive_channel.py +++ b/engine/apps/api/serializers/alert_receive_channel.py @@ -7,6 +7,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ValidationError as DjangoValidationError from django.core.validators import URLValidator from django.template.loader import render_to_string +from django.utils import timezone from jinja2 import TemplateSyntaxError from rest_framework import serializers from rest_framework.exceptions import ValidationError @@ -379,6 +380,7 @@ class AlertReceiveChannelTemplatesSerializer(EagerLoadingMixin, serializers.Mode self.instance.web_title_template = value.strip() elif default_template is not None and default_template.strip() == value.strip(): self.instance.web_title_template = None + self.instance.web_templates_modified_at = timezone.now() def get_web_message_template(self, obj): default_template = AlertReceiveChannel.INTEGRATION_TO_DEFAULT_WEB_MESSAGE_TEMPLATE[obj.integration] @@ -390,6 +392,7 @@ class AlertReceiveChannelTemplatesSerializer(EagerLoadingMixin, serializers.Mode self.instance.web_message_template = value.strip() elif default_template is not None and default_template.strip() == value.strip(): self.instance.web_message_template = None + self.instance.web_templates_modified_at = timezone.now() def get_web_image_url_template(self, obj): default_template = AlertReceiveChannel.INTEGRATION_TO_DEFAULT_WEB_IMAGE_URL_TEMPLATE[obj.integration] @@ -401,6 +404,7 @@ class AlertReceiveChannelTemplatesSerializer(EagerLoadingMixin, serializers.Mode self.instance.web_image_url_template = value.strip() elif default_template is not None and default_template.strip() == value.strip(): self.instance.web_image_url_template = None + self.instance.web_templates_modified_at = timezone.now() def get_telegram_title_template(self, obj): default_template = AlertReceiveChannel.INTEGRATION_TO_DEFAULT_TELEGRAM_TITLE_TEMPLATE[obj.integration] diff --git a/engine/apps/api/tests/test_paging.py b/engine/apps/api/tests/test_paging.py index 33b66c09..f2c52e03 100644 --- a/engine/apps/api/tests/test_paging.py +++ b/engine/apps/api/tests/test_paging.py @@ -58,6 +58,7 @@ def test_direct_paging_new_alert_group( ) assert response.status_code == status.HTTP_200_OK + assert "alert_group_id" in response.json() @pytest.mark.django_db @@ -104,6 +105,7 @@ def test_direct_paging_existing_alert_group( ) assert response.status_code == status.HTTP_200_OK + assert response.json()["alert_group_id"] == alert_group.public_primary_key @pytest.mark.django_db diff --git a/engine/apps/api/views/alert_group.py b/engine/apps/api/views/alert_group.py index d2457a4f..f94ed479 100644 --- a/engine/apps/api/views/alert_group.py +++ b/engine/apps/api/views/alert_group.py @@ -85,7 +85,11 @@ class AlertGroupFilter(DateRangeFilterMixin, ModelFieldFilterMixin, filters.Filt invitees_are = filters.ModelMultipleChoiceFilter( queryset=get_user_queryset, to_field_name="public_primary_key", method="filter_invitees_are" ) + involved_users_are = filters.ModelMultipleChoiceFilter( + queryset=get_user_queryset, to_field_name="public_primary_key", method="filter_by_involved_users" + ) with_resolution_note = filters.BooleanFilter(method="filter_with_resolution_note") + mine = filters.BooleanFilter(method="filter_mine") class Meta: model = AlertGroup @@ -132,10 +136,27 @@ class AlertGroupFilter(DateRangeFilterMixin, ModelFieldFilterMixin, filters.Filt if not users: return queryset - queryset = queryset.filter(acknowledged=False, resolved=False, log_records__author__in=users).distinct() + queryset = queryset.filter(log_records__author__in=users).distinct() return queryset + def filter_by_involved_users(self, queryset, name, value): + users = value + + if not users: + return queryset + + queryset = queryset.filter( + Q(personal_log_records__author__in=users) | Q(log_records__author__in=users) + ).distinct() + + return queryset + + def filter_mine(self, queryset, name, value): + if value: + return self.filter_by_involved_users(queryset, "users", [self.request.user.pk]) + return queryset + def filter_with_resolution_note(self, queryset, name, value): if value is True: queryset = queryset.filter(Q(resolution_notes__isnull=False, resolution_notes__deleted_at=None)).distinct() @@ -522,6 +543,12 @@ class AlertGroupView( "type": "options", "href": api_root + "users/?filters=true&roles=0&roles=1&roles=2", }, + { + "name": "involved_users_are", + "type": "options", + "href": api_root + "users/?filters=true&roles=0&roles=1&roles=2", + "default": {"display_name": self.request.user.username, "value": self.request.user.public_primary_key}, + }, { "name": "status", "type": "options", @@ -548,6 +575,11 @@ class AlertGroupView( "type": "boolean", "default": "true", }, + { + "name": "mine", + "type": "boolean", + "default": "true", + }, ] if filter_name is not None: diff --git a/engine/apps/api/views/paging.py b/engine/apps/api/views/paging.py index b0fcfd1d..0d44dc95 100644 --- a/engine/apps/api/views/paging.py +++ b/engine/apps/api/views/paging.py @@ -30,7 +30,7 @@ class DirectPagingAPIView(APIView): (schedule["instance"], schedule["important"]) for schedule in serializer.validated_data["schedules"] ] - direct_paging( + alert_group = direct_paging( organization=organization, team=team, from_user=from_user, @@ -42,4 +42,4 @@ class DirectPagingAPIView(APIView): alert_group=serializer.validated_data["alert_group"], ) - return Response(status=status.HTTP_200_OK) + return Response(data={"alert_group_id": alert_group.public_primary_key}, status=status.HTTP_200_OK) diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index 6f25e859..15ec4030 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -559,20 +559,37 @@ def _get_users_select(organization, team, input_id_prefix): } for user in users ] + if not user_options: - user_select = {"type": "context", "elements": [{"type": "mrkdwn", "text": "No users available"}]} + return {"type": "context", "elements": [{"type": "mrkdwn", "text": "No users available"}]} + + user_select = { + "type": "section", + "text": {"type": "mrkdwn", "text": "Add responders"}, + "block_id": input_id_prefix + DIRECT_PAGING_USER_SELECT_ID, + "accessory": { + "type": "static_select", + "placeholder": {"type": "plain_text", "text": "Select a user", "emoji": True}, + "action_id": OnPagingUserChange.routing_uid(), + }, + } + + if len(user_options) > scenario_step.MAX_STATIC_SELECT_OPTIONS: + # paginate user options in groups + max_length = scenario_step.MAX_STATIC_SELECT_OPTIONS + chunks = [user_options[x : x + max_length] for x in range(0, len(user_options), max_length)] + option_groups = [ + { + "label": {"type": "plain_text", "text": f"({(i * max_length)+1}-{(i * max_length)+max_length})"}, + "options": group, + } + for i, group in enumerate(chunks) + ] + user_select["accessory"]["option_groups"] = option_groups + else: - user_select = { - "type": "section", - "text": {"type": "mrkdwn", "text": "Add responders"}, - "block_id": input_id_prefix + DIRECT_PAGING_USER_SELECT_ID, - "accessory": { - "type": "static_select", - "placeholder": {"type": "plain_text", "text": "Select a user", "emoji": True}, - "options": user_options, - "action_id": OnPagingUserChange.routing_uid(), - }, - } + user_select["accessory"]["options"] = user_options + return user_select diff --git a/grafana-plugin/package.json b/grafana-plugin/package.json index 63b9c244..da6da4de 100644 --- a/grafana-plugin/package.json +++ b/grafana-plugin/package.json @@ -10,6 +10,7 @@ "build": "grafana-toolkit plugin:build", "build:dev": "grafana-toolkit plugin:build --skipTest --skipLint", "test": "jest --verbose", + "test:silent": "jest --silent", "dev": "grafana-toolkit plugin:dev", "watch": "grafana-toolkit plugin:dev --watch", "sign": "grafana-toolkit plugin:sign", diff --git a/grafana-plugin/src/components/PluginLink/PluginLink.tsx b/grafana-plugin/src/components/PluginLink/PluginLink.tsx index 2d380a43..4770ec7c 100644 --- a/grafana-plugin/src/components/PluginLink/PluginLink.tsx +++ b/grafana-plugin/src/components/PluginLink/PluginLink.tsx @@ -1,9 +1,8 @@ -import React, { FC, useMemo } from 'react'; +import React, { FC, useCallback, useMemo } from 'react'; import cn from 'classnames/bind'; import { Link } from 'react-router-dom'; -import Text from 'components/Text/Text'; import { getPathFromQueryParams } from 'utils/url'; import styles from './PluginLink.module.css'; @@ -23,12 +22,23 @@ const PluginLink: FC = (props) => { const newPath = useMemo(() => getPathFromQueryParams(query), [query]); - return disabled ? ( - - {children} - - ) : ( - + const handleClick = useCallback( + (event) => { + event.stopPropagation(); + + if (disabled) { + event.preventDefault(); + } + }, + [disabled] + ); + + return ( + {children} ); diff --git a/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx b/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx index ee0164d1..1f09dcc2 100644 --- a/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx +++ b/grafana-plugin/src/components/ScheduleUserDetails/ScheduleUserDetails.tsx @@ -30,8 +30,8 @@ const ScheduleUserDetails: FC = (props) => { const store = useStore(); const { teamStore } = store; - let slackWorkspaceNameOrigin = teamStore.currentTeam.slack_team_identity.cached_name; - const slackWorkspaceName = slackWorkspaceNameOrigin.replace(/[^0-9a-z]/gi, ''); + + const slackWorkspaceName = teamStore.currentTeam.slack_team_identity?.cached_name?.replace(/[^0-9a-z]/gi, '') || ''; return (
diff --git a/grafana-plugin/src/components/UserGroups/UserGroups.tsx b/grafana-plugin/src/components/UserGroups/UserGroups.tsx index 5a664060..7dfb9dcc 100644 --- a/grafana-plugin/src/components/UserGroups/UserGroups.tsx +++ b/grafana-plugin/src/components/UserGroups/UserGroups.tsx @@ -8,6 +8,7 @@ import { SortableContainer, SortableElement, SortableHandle } from 'react-sortab import Text from 'components/Text/Text'; import RemoteSelect from 'containers/RemoteSelect/RemoteSelect'; import { User } from 'models/user/user.types'; +import { UserActions } from 'utils/authorization'; import { fromPlainArray, toPlainArray } from './UserGroups.helpers'; import { Item } from './UserGroups.types'; @@ -114,6 +115,7 @@ const UserGroups = (props: UserGroupsProps) => { onChange={handleUserAdd} showError={showError} maxMenuHeight={150} + requiredUserAction={UserActions.UserSettingsWrite} /> { export const isInWorkingHours = (currentMoment: dayjs.Dayjs, workingHours, timezone) => { const timeFormat = 'HH:mm:ss'; const currentDayOfTheWeek = currentMoment.format('dddd').toLowerCase(); - const workingHourStart = workingHours[currentDayOfTheWeek][0].start; - const workingHourEnd = workingHours[currentDayOfTheWeek][0].end; + const workingHourStart = workingHours[currentDayOfTheWeek][0]?.start; + const workingHourEnd = workingHours[currentDayOfTheWeek][0]?.end; - const startTime = dayjs(workingHourStart, timeFormat).tz(timezone).format(timeFormat); - const endTime = dayjs(workingHourEnd, timeFormat).tz(timezone).format(timeFormat); - const currentTime = dayjs(currentMoment, timeFormat).format(timeFormat); + if (workingHourStart && workingHourEnd) { + const startTime = dayjs(workingHourStart, timeFormat).tz(timezone).format(timeFormat); + const endTime = dayjs(workingHourEnd, timeFormat).tz(timezone).format(timeFormat); + const currentTime = dayjs(currentMoment, timeFormat).format(timeFormat); - return currentTime < endTime && currentTime >= startTime; + return currentTime < endTime && currentTime >= startTime; + } + + return false; }; diff --git a/grafana-plugin/src/containers/ApiTokenSettings/ApiTokenSettings.tsx b/grafana-plugin/src/containers/ApiTokenSettings/ApiTokenSettings.tsx index c8f3cc2e..79970ec6 100644 --- a/grafana-plugin/src/containers/ApiTokenSettings/ApiTokenSettings.tsx +++ b/grafana-plugin/src/containers/ApiTokenSettings/ApiTokenSettings.tsx @@ -12,7 +12,7 @@ import { WithPermissionControl } from 'containers/WithPermissionControl/WithPerm import { ApiToken } from 'models/api_token/api_token.types'; import { WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; -import { isUserActionAllowed, UserActions } from 'utils/authorization'; +import { generateMissingPermissionMessage, isUserActionAllowed, UserActions } from 'utils/authorization'; import ApiTokenForm from './ApiTokenForm'; @@ -21,6 +21,7 @@ import styles from './ApiTokenSettings.module.css'; const cx = cn.bind(styles); const MAX_TOKENS_PER_USER = 5; +const REQUIRED_PERMISSION_TO_VIEW = UserActions.APIKeysWrite; interface ApiTokensProps extends WithStoreProps {} @@ -67,6 +68,15 @@ class ApiTokens extends React.Component { }, ]; + const authorizedToViewAPIKeys = isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW); + + let emptyText = 'Loading...'; + if (!authorizedToViewAPIKeys) { + emptyText = `${generateMissingPermissionMessage(REQUIRED_PERMISSION_TO_VIEW)} to be able to view API tokens.`; + } else if (apiTokens) { + emptyText = 'No tokens found'; + } + return ( <> { className="api-keys" showHeader={!isMobile} data={apiTokens} - emptyText={ - isUserActionAllowed(UserActions.APIKeysWrite) - ? apiTokens - ? 'No tokens found' - : 'Loading...' - : 'API tokens are available only for users with Admin permissions' - } + emptyText={emptyText} columns={columns} /> {showCreateTokenModal && ( diff --git a/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx b/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx index adb54919..55d1e7a1 100644 --- a/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx +++ b/grafana-plugin/src/containers/IncidentsFilters/IncidentsFilters.tsx @@ -71,6 +71,7 @@ class IncidentsFilters extends Component { ); } else if (QRCodeValue) { + const QRCodeDataParsed = getParsedQRCodeValue(); + content = ( @@ -163,6 +166,15 @@ const MobileAppConnection = observer(({ userPk }: Props) => { {isQRBlurry && }
+ {store.backendLicense === GRAFANA_LICENSE_OSS && QRCodeDataParsed && ( + + Server URL embedded in this QR: +
+ + {QRCodeDataParsed.oncall_api_url} + + + )} ); } @@ -178,6 +190,14 @@ const MobileAppConnection = observer(({ userPk }: Props) => { ); + function getParsedQRCodeValue() { + try { + return JSON.parse(QRCodeValue); + } catch (ex) { + return undefined; + } + } + function clearTimeouts(): void { clearTimeout(userTimeoutId); clearTimeout(refreshTimeoutId); diff --git a/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap b/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap index 6284efb3..56242d64 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap +++ b/grafana-plugin/src/containers/MobileAppConnection/__snapshots__/MobileAppConnection.test.tsx.snap @@ -58,20 +58,27 @@ exports[`MobileAppConnection if we disconnect the app, it disconnects and fetche
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2389,20 +2396,27 @@ exports[`MobileAppConnection it shows a QR code if the app isn't already connect
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2488,20 +2502,27 @@ exports[`MobileAppConnection it shows a loading message if it is currently disco
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2587,20 +2608,27 @@ exports[`MobileAppConnection it shows a loading message if it is currently fetch
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2686,20 +2714,27 @@ exports[`MobileAppConnection it shows a message when the mobile app is already c
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2885,20 +2920,27 @@ exports[`MobileAppConnection it shows an error message if there was an error dis
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ @@ -2975,20 +3017,27 @@ exports[`MobileAppConnection it shows an error message if there was an error fet
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ diff --git a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap index c355da1b..fe4b7560 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap +++ b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/__snapshots__/DownloadIcons.test.tsx.snap @@ -52,20 +52,27 @@ exports[`DownloadIcons it renders properly 1`] = `
-
- Play Store - - Android - -
+ Play Store + + Android + +
+ diff --git a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx index 2b140799..d3b68fc8 100644 --- a/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx +++ b/grafana-plugin/src/containers/MobileAppConnection/parts/DownloadIcons/index.tsx @@ -25,12 +25,19 @@ const DownloadIcons: FC = () => ( iOS - - Play Store - - Android - - + + + Play Store + + Android + + + ); diff --git a/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx b/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx index 753b3326..91b17d19 100644 --- a/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx +++ b/grafana-plugin/src/containers/RemoteSelect/RemoteSelect.tsx @@ -1,10 +1,11 @@ -import React, { useCallback, useEffect, useMemo, useReducer } from 'react'; +import React, { useCallback, useEffect, useMemo, useReducer, useState } from 'react'; import { SelectableValue } from '@grafana/data'; import { AsyncMultiSelect, AsyncSelect } from '@grafana/ui'; import { inject, observer } from 'mobx-react'; -import { makeRequest } from 'network'; +import { makeRequest, isNetworkError } from 'network'; +import { UserAction, generateMissingPermissionMessage } from 'utils/authorization'; interface RemoteSelectProps { autoFocus?: boolean; @@ -24,6 +25,7 @@ interface RemoteSelectProps { getOptionLabel?: (item: SelectableValue) => React.ReactNode; showError?: boolean; maxMenuHeight?: number; + requiredUserAction?: UserAction; } const RemoteSelect = inject('store')( @@ -45,9 +47,12 @@ const RemoteSelect = inject('store')( openMenuOnFocus = true, showError, maxMenuHeight, + requiredUserAction, } = props; - const getOptions = (data: any[]) => { + const [noOptionsMessage, setNoOptionsMessage] = useState('No options found'); + + const getOptions = (data: any[]): SelectableValue[] => { return data.map((option: any) => ({ value: option[valueField], label: option[fieldToShow], @@ -62,17 +67,23 @@ const RemoteSelect = inject('store')( const [options, setOptions] = useReducer(mergeOptions, []); - useEffect(() => { - makeRequest(href, {}).then((data) => { - setOptions(getOptions(data.results || data)); - }); + const loadOptionsCallback = useCallback(async (query?: string): Promise => { + try { + const data = await makeRequest(href, { params: { search: query } }); + const options = getOptions(data.results || data); + setOptions(options); + + return options; + } catch (e) { + if (isNetworkError(e) && e.response.status === 403 && requiredUserAction) { + setNoOptionsMessage(generateMissingPermissionMessage(requiredUserAction)); + } + return []; + } }, []); - const loadOptionsCallback = useCallback((query: string) => { - return makeRequest(href, { params: { search: query } }).then((data) => { - setOptions(getOptions(data.results || data)); - return getOptions(data.results || data); - }); + useEffect(() => { + loadOptionsCallback(); }, []); const onChangeCallback = useCallback( @@ -119,6 +130,7 @@ const RemoteSelect = inject('store')( defaultOptions={options} loadOptions={loadOptionsCallback} getOptionLabel={getOptionLabel} + noOptionsMessage={noOptionsMessage} invalid={showError} /> ); diff --git a/grafana-plugin/src/containers/UserSettings/parts/index.tsx b/grafana-plugin/src/containers/UserSettings/parts/index.tsx index 77692b62..cf0eb994 100644 --- a/grafana-plugin/src/containers/UserSettings/parts/index.tsx +++ b/grafana-plugin/src/containers/UserSettings/parts/index.tsx @@ -133,7 +133,6 @@ export const TabsContent = observer(({ id, activeTab, onTabChange, isDesktopOrLa ) : ( ))} - {/* TODO: we should probably hide this tab when a user (ie. Admin) is viewing the user settings for another user. Would it make sense for an Admin to be able to link their mobile app to another user's profile */} {activeTab === UserSettingsTab.MobileAppConnection && } {activeTab === UserSettingsTab.SlackInfo && } {activeTab === UserSettingsTab.TelegramInfo && } diff --git a/grafana-plugin/src/network/index.ts b/grafana-plugin/src/network/index.ts index e9c1930e..53b1f579 100644 --- a/grafana-plugin/src/network/index.ts +++ b/grafana-plugin/src/network/index.ts @@ -34,6 +34,8 @@ interface RequestConfig { validateStatus?: (status: number) => boolean; } +export const isNetworkError = axios.isAxiosError; + export const makeRequest = async (path: string, config: RequestConfig) => { const { method = 'GET', params, data, validateStatus } = config; diff --git a/grafana-plugin/src/pages/index.tsx b/grafana-plugin/src/pages/index.tsx index 8bcb10e0..6de5109a 100644 --- a/grafana-plugin/src/pages/index.tsx +++ b/grafana-plugin/src/pages/index.tsx @@ -127,7 +127,6 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'table', id: 'live-settings', text: 'Env Variables', - role: 'Admin', hideFromTabsFn: (store: RootBaseStore) => { const hasLiveSettings = store.hasFeature(AppFeature.LiveSettings); return isTopNavbar() || !hasLiveSettings; @@ -139,7 +138,6 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'cloud', id: 'cloud', text: 'Cloud', - role: 'Admin', hideFromTabsFn: (store: RootBaseStore) => { const hasCloudFeature = store.hasFeature(AppFeature.CloudConnection); return isTopNavbar() || !hasCloudFeature; diff --git a/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx b/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx index 38777de6..6eb3a7cf 100644 --- a/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx +++ b/grafana-plugin/src/pages/settings/tabs/Cloud/CloudPage.tsx @@ -16,6 +16,7 @@ import { WithStoreProps } from 'state/types'; import { useStore } from 'state/useStore'; import { withMobXProviderContext } from 'state/withStore'; import { openErrorNotification } from 'utils'; +import { determineRequiredAuthString, UserActions } from 'utils/authorization'; import { PLUGIN_ROOT } from 'utils/consts'; import styles from './CloudPage.module.css'; @@ -261,10 +262,9 @@ const CloudPage = observer((props: CloudPageProps) => {
- {/* TODO: should probably update this message? */} - { - 'Ask your users to sign up in Grafana Cloud, verify phone number and feel free to set up SMS & phone call notifications in personal settings! Only users with Admin or Editor role will be synced.' - } + {`Ask your users to sign up in Grafana Cloud, verify phone number and feel free to set up SMS & phone call notifications in personal settings! Users must have ${determineRequiredAuthString( + UserActions.NotificationsRead + )} in order to be synced.`} { {matched_users_count === 1 ? '' : 's'} {` matched between OSS and Cloud OnCall`} - {syncingUsers ? ( - - ) : ( - - )} +
)} diff --git a/grafana-plugin/src/pages/users/Users.tsx b/grafana-plugin/src/pages/users/Users.tsx index bbaff4da..19fa6853 100644 --- a/grafana-plugin/src/pages/users/Users.tsx +++ b/grafana-plugin/src/pages/users/Users.tsx @@ -23,7 +23,7 @@ import { User as UserType } from 'models/user/user.types'; import { PageProps, WithStoreProps } from 'state/types'; import { withMobXProviderContext } from 'state/withStore'; import LocationHelper from 'utils/LocationHelper'; -import { isUserActionAllowed, UserActions } from 'utils/authorization'; +import { generateMissingPermissionMessage, isUserActionAllowed, UserActions } from 'utils/authorization'; import { PLUGIN_ROOT } from 'utils/consts'; import { getUserRowClassNameFn } from './Users.helpers'; @@ -35,6 +35,7 @@ const cx = cn.bind(styles); interface UsersProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> {} const ITEMS_PER_PAGE = 100; +const REQUIRED_PERMISSION_TO_VIEW_USERS = UserActions.UserSettingsWrite; interface UsersState extends PageBaseState { page: number; @@ -43,6 +44,7 @@ interface UsersState extends PageBaseState { usersFilters?: { searchTerm: string; }; + initialUsersLoaded: boolean; } @observer @@ -56,10 +58,9 @@ class Users extends React.Component { }, errorData: initErrorDataState(), + initialUsersLoaded: false, }; - initialUsersLoaded = false; - async componentDidMount() { const { query: { p }, @@ -74,18 +75,19 @@ class Users extends React.Component { const { usersFilters, page } = this.state; const { userStore } = store; - if (!isUserActionAllowed(UserActions.UserSettingsWrite)) { + if (!isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW_USERS)) { return; } LocationHelper.update({ p: page }, 'partial'); - return await userStore.updateItems(usersFilters, page); + await userStore.updateItems(usersFilters, page); + + this.setState({ initialUsersLoaded: true }); }; componentDidUpdate(prevProps: UsersProps) { - if (!this.initialUsersLoaded && isUserActionAllowed(UserActions.UserSettingsWrite)) { + if (!this.state.initialUsersLoaded) { this.updateUsers(); - this.initialUsersLoaded = true; } if (prevProps.match.params.id !== this.props.match.params.id) { @@ -117,7 +119,7 @@ class Users extends React.Component { }; render() { - const { usersFilters, userPkToEdit, page, errorData } = this.state; + const { usersFilters, userPkToEdit, page, errorData, initialUsersLoaded } = this.state; const { store, match: { @@ -165,6 +167,8 @@ class Users extends React.Component { const { count, results } = userStore.getSearchResult(); + const authorizedToViewUsers = isUserActionAllowed(REQUIRED_PERMISSION_TO_VIEW_USERS); + return ( { Users - - To manage permissions or add users, please visit{' '} - Grafana user management - + {authorizedToViewUsers && ( + + To manage permissions or add users, please visit{' '} + Grafana user management + + )} @@ -194,7 +200,7 @@ class Users extends React.Component { - {isUserActionAllowed(UserActions.UserSettingsRead) ? ( + {authorizedToViewUsers ? ( <>
{
{ /* @ts-ignore */ title={ <> - You don't have enough permissions to view other users because you are not Admin.{' '} - Click here to open your profile + {generateMissingPermissionMessage(REQUIRED_PERMISSION_TO_VIEW_USERS)} to be able to view OnCall + users. Click here to open your + profile } severity="info" diff --git a/grafana-plugin/src/plugin.json b/grafana-plugin/src/plugin.json index 49790bce..efde7c9c 100644 --- a/grafana-plugin/src/plugin.json +++ b/grafana-plugin/src/plugin.json @@ -535,7 +535,7 @@ { "role": { "name": "User Settings Reader", - "description": "Read-only access to OnCall User Settings", + "description": "Read-only access to own OnCall User Settings", "permissions": [ { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" } @@ -546,7 +546,7 @@ { "role": { "name": "User Settings Editor", - "description": "Read/write access to own OnCall User Settings", + "description": "Read/write access to own OnCall User Settings + ability to view basic information about other OnCall users", "permissions": [ { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" }, diff --git a/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap b/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap index 334e4c70..1bb513e3 100644 --- a/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap +++ b/grafana-plugin/src/state/plugin/__snapshots__/plugin.test.ts.snap @@ -34,19 +34,19 @@ exports[`PluginState.generateUnknownErrorMsg it returns the proper error message Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 AxiosError properly - has custom error message: false 1`] = ` +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 network error properly - has custom error message: false 1`] = ` "An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 AxiosError properly - has custom error message: true 1`] = `"ohhhh nooo an error"`; +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a 400 network error properly - has custom error message: true 1`] = `"ohhhh nooo an error"`; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 AxiosError properly - status code: 409 1`] = ` +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 network error properly - status code: 409 1`] = ` "An unknown error occured when trying to install the plugin. Are you sure that your OnCall API URL, http://hello.com, is correct (NOTE: your OnCall API URL is currently being taken from process.env of your UI)? Refresh your page and try again, or try removing your plugin configuration and reconfiguring." `; -exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 AxiosError properly - status code: 502 1`] = ` +exports[`PluginState.getHumanReadableErrorFromOnCallError it handles a non-400 network error properly - status code: 502 1`] = ` "Could not communicate with your OnCall API at http://hello.com (NOTE: your OnCall API URL is currently being taken from process.env of your UI). Validate that the URL is correct, your OnCall API is running, and that it is accessible from your Grafana instance." `; diff --git a/grafana-plugin/src/state/plugin/index.ts b/grafana-plugin/src/state/plugin/index.ts index 454fee84..f0617bc9 100644 --- a/grafana-plugin/src/state/plugin/index.ts +++ b/grafana-plugin/src/state/plugin/index.ts @@ -1,8 +1,7 @@ import { getBackendSrv } from '@grafana/runtime'; -import axios from 'axios'; import { OnCallAppPluginMeta, OnCallPluginMetaJSONData, OnCallPluginMetaSecureJSONData } from 'types'; -import { makeRequest } from 'network'; +import { makeRequest, isNetworkError } from 'network'; import FaroHelper from 'utils/faro'; export type UpdateGrafanaPluginSettingsProps = { @@ -76,7 +75,7 @@ class PluginState { ); const consoleMsg = `occured while trying to ${installationVerb} the plugin w/ the OnCall backend`; - if (axios.isAxiosError(e)) { + if (isNetworkError(e)) { const { status: statusCode } = e.response; console.warn(`An HTTP related error ${consoleMsg}`, e.response); @@ -100,7 +99,7 @@ class PluginState { errorMsg = unknownErrorMsg; } } else { - // a non-axios related error occured.. this scenario shouldn't occur... + // a non-network related error occured.. this scenario shouldn't occur... console.warn(`An unknown error ${consoleMsg}`, e); errorMsg = unknownErrorMsg; } @@ -115,12 +114,12 @@ class PluginState { ): string => { let errorMsg: string; - if (axios.isAxiosError(e)) { + if (isNetworkError(e)) { // The user likely put in a bogus URL for the OnCall API URL console.warn('An HTTP related error occured while trying to provision the plugin w/ Grafana', e.response); errorMsg = this.generateInvalidOnCallApiURLErrorMsg(onCallApiUrl, onCallApiUrlIsConfiguredThroughEnvVar); } else { - // a non-axios related error occured.. this scenario shouldn't occur... + // a non-network related error occured.. this scenario shouldn't occur... console.warn('An unknown error occured while trying to provision the plugin w/ Grafana', e); errorMsg = this.generateUnknownErrorMsg(onCallApiUrl, installationVerb, onCallApiUrlIsConfiguredThroughEnvVar); } diff --git a/grafana-plugin/src/state/plugin/plugin.test.ts b/grafana-plugin/src/state/plugin/plugin.test.ts index 405d9913..c18ab7b8 100644 --- a/grafana-plugin/src/state/plugin/plugin.test.ts +++ b/grafana-plugin/src/state/plugin/plugin.test.ts @@ -1,8 +1,9 @@ -import { makeRequest as makeRequestOriginal } from 'network'; +import { makeRequest as makeRequestOriginal, isNetworkError as isNetworkErrorOriginal } from 'network'; import PluginState, { InstallationVerb, PluginSyncStatusResponse, UpdateGrafanaPluginSettingsProps } from './'; const makeRequest = makeRequestOriginal as jest.Mock>; +const isNetworkError = isNetworkErrorOriginal as unknown as jest.Mock>; jest.mock('network'); @@ -13,7 +14,7 @@ afterEach(() => { const ONCALL_BASE_URL = '/plugin'; const GRAFANA_PLUGIN_SETTINGS_URL = '/api/plugins/grafana-oncall-app/settings'; -const generateMockAxiosError = (status: number, data = {}) => ({ isAxiosError: true, response: { status, ...data } }); +const generateMockNetworkError = (status: number, data = {}) => ({ response: { status, ...data } }); describe('PluginState.generateOnCallApiUrlConfiguredThroughEnvVarMsg', () => { test.each([true, false])( @@ -54,10 +55,12 @@ describe('PluginState.getHumanReadableErrorFromOnCallError', () => { console.warn = () => {}; }); - test.each([502, 409])('it handles a non-400 AxiosError properly - status code: %s', (status) => { + test.each([502, 409])('it handles a non-400 network error properly - status code: %s', (status) => { + isNetworkError.mockReturnValueOnce(true); + expect( PluginState.getHumanReadableErrorFromOnCallError( - generateMockAxiosError(status), + generateMockNetworkError(status), 'http://hello.com', 'install', true @@ -66,19 +69,23 @@ describe('PluginState.getHumanReadableErrorFromOnCallError', () => { }); test.each([true, false])( - 'it handles a 400 AxiosError properly - has custom error message: %s', + 'it handles a 400 network error properly - has custom error message: %s', (hasCustomErrorMessage) => { - const axiosError = generateMockAxiosError(400) as any; + isNetworkError.mockReturnValueOnce(true); + + const networkError = generateMockNetworkError(400) as any; if (hasCustomErrorMessage) { - axiosError.response.data = { error: 'ohhhh nooo an error' }; + networkError.response.data = { error: 'ohhhh nooo an error' }; } expect( - PluginState.getHumanReadableErrorFromOnCallError(axiosError, 'http://hello.com', 'install', true) + PluginState.getHumanReadableErrorFromOnCallError(networkError, 'http://hello.com', 'install', true) ).toMatchSnapshot(); } ); test('it handles an unknown error properly', () => { + isNetworkError.mockReturnValueOnce(false); + expect( PluginState.getHumanReadableErrorFromOnCallError(new Error('asdfasdf'), 'http://hello.com', 'install', true) ).toMatchSnapshot(); @@ -86,23 +93,27 @@ describe('PluginState.getHumanReadableErrorFromOnCallError', () => { }); describe('PluginState.getHumanReadableErrorFromGrafanaProvisioningError', () => { - test.each([true, false])('it handles an error properly', (isAxiosError) => { + beforeEach(() => { + console.warn = () => {}; + }); + + test.each([true, false])('it handles an error properly - network error: %s', (networkError) => { const onCallApiUrl = 'http://hello.com'; const installationVerb = 'install'; const onCallApiUrlIsConfiguredThroughEnvVar = true; - const axiosError = generateMockAxiosError(400); - const nonAxiosError = new Error('oh noooo'); - const error = isAxiosError ? axiosError : nonAxiosError; + const error = networkError ? generateMockNetworkError(400) : new Error('oh noooo'); const mockGenerateInvalidOnCallApiURLErrorMsgResult = 'asdadslkjfkjlsd'; const mockGenerateUnknownErrorMsgResult = 'asdadslkjfkjlsd'; + isNetworkError.mockReturnValueOnce(networkError); + PluginState.generateInvalidOnCallApiURLErrorMsg = jest .fn() .mockReturnValueOnce(mockGenerateInvalidOnCallApiURLErrorMsgResult); PluginState.generateUnknownErrorMsg = jest.fn().mockReturnValueOnce(mockGenerateUnknownErrorMsgResult); - const expectedErrorMsg = isAxiosError + const expectedErrorMsg = networkError ? mockGenerateInvalidOnCallApiURLErrorMsgResult : mockGenerateUnknownErrorMsgResult; @@ -115,7 +126,7 @@ describe('PluginState.getHumanReadableErrorFromGrafanaProvisioningError', () => ) ).toEqual(expectedErrorMsg); - if (isAxiosError) { + if (networkError) { expect(PluginState.generateInvalidOnCallApiURLErrorMsg).toHaveBeenCalledTimes(1); expect(PluginState.generateInvalidOnCallApiURLErrorMsg).toHaveBeenCalledWith( onCallApiUrl, diff --git a/grafana-plugin/src/utils/authorization/authorization.test.ts b/grafana-plugin/src/utils/authorization/authorization.test.ts index b0c72d78..dc2dae38 100644 --- a/grafana-plugin/src/utils/authorization/authorization.test.ts +++ b/grafana-plugin/src/utils/authorization/authorization.test.ts @@ -63,6 +63,32 @@ describe('isUserActionAllowed', () => { }); }); +describe('determineRequiredAuthString', () => { + const testPerm = auth.UserActions.UserSettingsRead; + + test.each([ + [true, `${testPerm.permission} permission`], + [false, `${testPerm.fallbackMinimumRoleRequired} role`], + ])('RBAC enabled: %s', (rbacEnabled, expected) => { + config.featureToggles.accessControlOnCall = rbacEnabled; + + expect(auth.determineRequiredAuthString(testPerm)).toBe(expected); + }); +}); + +describe('generateMissingPermissionMessage', () => { + const testPerm = auth.UserActions.UserSettingsRead; + + test.each([ + [true, `You are missing the ${testPerm.permission} permission`], + [false, `You are missing the ${testPerm.fallbackMinimumRoleRequired} role`], + ])('RBAC enabled: %s', (rbacEnabled, expected) => { + config.featureToggles.accessControlOnCall = rbacEnabled; + + expect(auth.generateMissingPermissionMessage(testPerm)).toBe(expected); + }); +}); + describe('generatePermissionString', () => { test('it properly builds permission strings with prefixes', () => { expect(auth.generatePermissionString(auth.Resource.API_KEYS, auth.Action.READ, true)).toEqual( diff --git a/grafana-plugin/src/utils/authorization/index.ts b/grafana-plugin/src/utils/authorization/index.ts index 215f58b2..10574c47 100644 --- a/grafana-plugin/src/utils/authorization/index.ts +++ b/grafana-plugin/src/utils/authorization/index.ts @@ -90,12 +90,24 @@ export const userHasMinimumRequiredRole = (minimumRoleRequired: OrgRole): boolea * * As a fallback (second argument), for cases where RBAC is not enabled for a grafana instance, rely on basic roles */ -export const isUserActionAllowed = ({ permission, fallbackMinimumRoleRequired }: UserAction): boolean => { - if (config.featureToggles.accessControlOnCall) { - return !!contextSrv.user.permissions?.[permission]; - } - return userHasMinimumRequiredRole(fallbackMinimumRoleRequired); -}; +export const isUserActionAllowed = ({ permission, fallbackMinimumRoleRequired }: UserAction): boolean => + config.featureToggles.accessControlOnCall + ? !!contextSrv.user.permissions?.[permission] + : userHasMinimumRequiredRole(fallbackMinimumRoleRequired); + +/** + * Given a `UserAction`, returns the permission or fallback-role, prefixed with "permission" or "role" respectively + * depending on whether or not RBAC is enabled/disabled + */ +export const determineRequiredAuthString = ({ permission, fallbackMinimumRoleRequired }: UserAction): string => + config.featureToggles.accessControlOnCall ? `${permission} permission` : `${fallbackMinimumRoleRequired} role`; + +/** + * Can be used to generate a user-friendly message about which permission is missing. Method is RBAC-aware + * and shows user the missing permission/basic-role depending on whether or not RBAC is enabled/disabled + */ +export const generateMissingPermissionMessage = (permission: UserAction): string => + `You are missing the ${determineRequiredAuthString(permission)}`; export const generatePermissionString = (resource: Resource, action: Action, includePrefix: boolean): string => `${includePrefix ? `${ONCALL_PERMISSION_PREFIX}.` : ''}${resource}:${action}`; diff --git a/grafana-plugin/yarn.lock b/grafana-plugin/yarn.lock index d3cfc6d8..f06894fb 100644 --- a/grafana-plugin/yarn.lock +++ b/grafana-plugin/yarn.lock @@ -12411,9 +12411,9 @@ signal-exit@^3.0.2, signal-exit@^3.0.3: integrity sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ== simple-git@^3.6.0: - version "3.15.0" - resolved "https://registry.yarnpkg.com/simple-git/-/simple-git-3.15.0.tgz#301a95a943c4f9b0a21d051eb6e6d0ffe4c9754f" - integrity sha512-FiWoMPlcYHQ+ApRihUsGjC/ZmIlWj62S6MBCwOunczvXcLQt+9ZdrysDrR6QVepkRQfEAaBXrN2QtJKrN6zbtg== + version "3.16.0" + resolved "https://registry.yarnpkg.com/simple-git/-/simple-git-3.16.0.tgz#421773e24680f5716999cc4a1d60127b4b6a9dec" + integrity sha512-zuWYsOLEhbJRWVxpjdiXl6eyAyGo/KzVW+KFhhw9MqEEJttcq+32jTWSGyxTdf9e/YCohxRE+9xpWFj9FdiJNw== dependencies: "@kwsites/file-exists" "^1.1.1" "@kwsites/promise-deferred" "^1.1.1"