From 63e91f896b1118de56783dba1223f1940682bd5b Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 30 Jan 2023 11:23:02 +0100 Subject: [PATCH] [RBAC] - minor UI bug fixes (#1185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # What this PR does - remove hardcoded references to "Admin" and "Editor". Instead, use the `determineRequiredAuthString` and `generateMissingPermissionMessage` methods from `utils/authorization` which conditionally show the permission or role depending on whether RBAC is enabled or not - fix bug on list users page that always showed "Loading...". ![Screenshot 2023-01-21 at 09 29 35](https://user-images.githubusercontent.com/9406895/213859785-e9852838-5671-4275-aaed-4df75446ab6a.png) - only show users the user's table if they are allowed, otherwise show them this - RBAC enabled ![Screenshot 2023-01-26 at 09 09 57](https://user-images.githubusercontent.com/9406895/214786723-3389ce9c-7353-4216-9176-6547f2076660.png) - RBAC disabled ![Screenshot 2023-01-26 at 09 05 30](https://user-images.githubusercontent.com/9406895/214786739-eb9ee108-e79c-4105-912a-8bb5bf03cb32.png) - `Schedules Editor` role minor issue - Viewers are not allowed to list users by default, so granting schedule edit permission to them won’t allow them to see who rotations are assigned to or assign a rotation to a user. To make this a more straightforward user experience, instead of saying "No options found", we will tell the user that they are missing a particular role/permission to view these users: Before: ![Screenshot 2023-01-23 at 09 52 28](https://user-images.githubusercontent.com/9406895/213999896-d889540e-2835-4133-965a-306923a3e33b.png) After: ![Screenshot 2023-01-26 at 12 39 30](https://user-images.githubusercontent.com/9406895/214827179-d127002f-793e-4ab7-ad75-dfab653b7d7d.png) ## Which issue(s) this PR fixes - closes #971 ## Checklist - [x] Tests updated - [ ] Documentation added (N/A) - [x] `CHANGELOG.md` updated --- CHANGELOG.md | 2 + .../src/components/UserGroups/UserGroups.tsx | 2 + .../ApiTokenSettings/ApiTokenSettings.tsx | 20 ++++++---- .../containers/RemoteSelect/RemoteSelect.tsx | 36 +++++++++++------ .../containers/UserSettings/parts/index.tsx | 1 - grafana-plugin/src/network/index.ts | 2 + grafana-plugin/src/pages/index.tsx | 2 - .../pages/settings/tabs/Cloud/CloudPage.tsx | 20 ++++------ grafana-plugin/src/pages/users/Users.tsx | 39 +++++++++++-------- grafana-plugin/src/plugin.json | 4 +- .../plugin/__snapshots__/plugin.test.ts.snap | 8 ++-- grafana-plugin/src/state/plugin/index.ts | 11 +++--- .../src/state/plugin/plugin.test.ts | 39 ++++++++++++------- .../utils/authorization/authorization.test.ts | 26 +++++++++++++ .../src/utils/authorization/index.ts | 24 +++++++++--- 15 files changed, 152 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42865e21..72b86e13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ 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) 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} /> { }, ]; + 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/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}`;