From c5d1b295ec3e2b25a01c176dd366300846ac7e96 Mon Sep 17 00:00:00 2001 From: Jo Date: Fri, 16 Feb 2024 09:36:52 +0100 Subject: [PATCH] Plugins: Allow plugin page access granting via permissions (#82508) * AccessControl: Check permissions on AppRootPage * add frontend tests for app root permission checks * add accesscontrol oncall ft to tests --- .betterer.results.json | 38 ++--- packages/grafana-data/src/types/plugin.ts | 4 + .../plugins/components/AppRootPage.test.tsx | 151 +++++++++++++++++- .../plugins/components/AppRootPage.tsx | 12 +- 4 files changed, 177 insertions(+), 28 deletions(-) diff --git a/.betterer.results.json b/.betterer.results.json index 6733b2392d7..ca07d39f380 100644 --- a/.betterer.results.json +++ b/.betterer.results.json @@ -1028,6 +1028,12 @@ "count": 5 } ], + "/packages/grafana-ui/src/components/Splitter/Splitter.tsx": [ + { + "message": "Do not use any type assertions.", + "count": 1 + } + ], "/packages/grafana-ui/src/components/StatsPicker/StatsPicker.story.tsx": [ { "message": "Unexpected any. Specify a different type.", @@ -2962,12 +2968,12 @@ ], "/public/app/features/dashboard-scene/scene/PanelMenuBehavior.tsx": [ { - "message": "Do not use any type assertions.", + "message": "Unexpected any. Specify a different type.", "count": 2 }, { - "message": "Unexpected any. Specify a different type.", - "count": 2 + "message": "Do not use any type assertions.", + "count": 1 } ], "/public/app/features/dashboard-scene/scene/setDashboardPanelContext.test.ts": [ @@ -3018,6 +3024,12 @@ "count": 1 } ], + "/public/app/features/dashboard-scene/utils/PanelModelCompatibilityWrapper.ts": [ + { + "message": "Do not use any type assertions.", + "count": 1 + } + ], "/public/app/features/dashboard-scene/utils/test-utils.ts": [ { "message": "Do not use any type assertions.", @@ -4060,12 +4072,6 @@ "count": 1 } ], - "/public/app/features/explore/TraceView/components/common/Divider.tsx": [ - { - "message": "Styles should be written using objects.", - "count": 3 - } - ], "/public/app/features/explore/TraceView/components/common/LabeledList.tsx": [ { "message": "Styles should be written using objects.", @@ -6597,11 +6603,11 @@ "/public/app/plugins/datasource/tempo/datasource.ts": [ { "message": "Do not use any type assertions.", - "count": 7 + "count": 2 }, { "message": "Unexpected any. Specify a different type.", - "count": 5 + "count": 4 } ], "/public/app/plugins/datasource/tempo/language_provider.ts": [ @@ -7213,10 +7219,6 @@ } ], "/public/app/plugins/panel/timeseries/plugins/annotations/AnnotationEditor.tsx": [ - { - "message": "Styles should be written using objects.", - "count": 5 - }, { "message": "Do not use any type assertions.", "count": 1 @@ -7228,12 +7230,6 @@ "count": 7 } ], - "/public/app/plugins/panel/timeseries/plugins/annotations/AnnotationMarker.tsx": [ - { - "message": "Styles should be written using objects.", - "count": 3 - } - ], "/public/app/plugins/panel/timeseries/plugins/annotations/AnnotationTooltip.tsx": [ { "message": "Styles should be written using objects.", diff --git a/packages/grafana-data/src/types/plugin.ts b/packages/grafana-data/src/types/plugin.ts index 7d07e91f018..5c934d3b299 100644 --- a/packages/grafana-data/src/types/plugin.ts +++ b/packages/grafana-data/src/types/plugin.ts @@ -121,6 +121,10 @@ export interface PluginInclude { // "Admin", "Editor" or "Viewer". If set then the include will only show up in the navigation if the user has the required roles. role?: string; + // if action is set then the include will only show up in the navigation if the user has the required permission. + // The action will take precedence over the role. + action?: string; + // Adds the "page" or "dashboard" type includes to the navigation if set to `true`. addToNav?: boolean; diff --git a/public/app/features/plugins/components/AppRootPage.test.tsx b/public/app/features/plugins/components/AppRootPage.test.tsx index 4adc4b1e80c..e9009ff59d0 100644 --- a/public/app/features/plugins/components/AppRootPage.test.tsx +++ b/public/app/features/plugins/components/AppRootPage.test.tsx @@ -26,6 +26,25 @@ jest.mock('../plugin_loader', () => ({ importAppPlugin: jest.fn(), })); +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + config: { + featureToggles: { + accessControlOnCall: true, + }, + theme2: { + breakpoints: { + values: { + sm: 576, + md: 768, + lg: 992, + xl: 1200, + }, + }, + }, + }, +})); + const importAppPluginMock = importAppPlugin as jest.Mock< ReturnType, Parameters @@ -175,6 +194,19 @@ describe('AppRootPage', () => { name: 'Awesome page 2', path: '/a/my-awesome-plugin/page-without-role', }, + { + type: PluginIncludeType.page, + name: 'Awesome page 3', + path: '/a/my-awesome-plugin/page-with-action-no-role', + action: 'grafana-awesomeapp.user-settings:read', + }, + { + type: PluginIncludeType.page, + name: 'Awesome page 4', + path: '/a/my-awesome-plugin/page-with-action-and-role', + role: 'Viewer', + action: 'grafana-awesomeapp.user-settings:read', + }, ], }); @@ -194,13 +226,58 @@ describe('AppRootPage', () => { expect(await screen.findByText('Access denied')).toBeVisible(); }); + it('a None role user should only have access to pages with actions defined or undefined', async () => { + contextSrv.user.orgRole = OrgRole.None; + + // has access to a plugin entry page by default + await renderUnderRouter(''); + expect(await screen.findByText('my great component')).toBeVisible(); + + // does not have access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // does not have access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // has access to a page without roles + await renderUnderRouter('page-without-role'); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to Viewer page + await renderUnderRouter('viewer-page'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + contextSrv.user.permissions = { + 'grafana-awesomeapp.user-settings:read': true, + }; + + // has access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('my great component')).toBeVisible(); + }); + it('a Viewer should only have access to pages with "Viewer" roles', async () => { contextSrv.user.orgRole = OrgRole.Viewer; + contextSrv.user.permissions = {}; // Viewer has access to a plugin entry page by default await renderUnderRouter(''); expect(await screen.findByText('my great component')).toBeVisible(); + // Viewer does not have access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // Viewer does not have access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + // Viewer has access to a page without roles await renderUnderRouter('page-without-role'); expect(await screen.findByText('my great component')).toBeVisible(); @@ -221,35 +298,97 @@ describe('AppRootPage', () => { it('an Editor should have access to pages with both "Viewer" and "Editor" roles', async () => { contextSrv.user.orgRole = OrgRole.Editor; contextSrv.isEditor = true; + contextSrv.user.permissions = {}; - // Viewer has access to a plugin entry page by default + // does not have access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // does not have access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // has access to a plugin entry page by default await renderUnderRouter(''); expect(await screen.findByText('my great component')).toBeVisible(); - // Editor has access to a page without roles + // has access to a page without roles await renderUnderRouter('page-without-role'); expect(await screen.findByText('my great component')).toBeVisible(); - // Editor has access to Viewer page + // has access to Viewer page await renderUnderRouter('viewer-page'); expect(await screen.findByText('my great component')).toBeVisible(); - // Editor has access to Editor page + // has access to Editor page await renderUnderRouter('editor-page'); expect(await screen.findByText('my great component')).toBeVisible(); - // Editor does not have access to a Admin page + // does not have access to a Admin page await renderUnderRouter('admin-page'); expect(await screen.findByText('Access denied')).toBeVisible(); + + contextSrv.user.permissions = { + 'grafana-awesomeapp.user-settings:read': true, + }; + + // has access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('my great component')).toBeVisible(); }); - it('a Grafana Admin should be able to see any page', async () => { + it('an Admin should have access to pages with both "Viewer" and "Editor" roles', async () => { + contextSrv.user.orgRole = OrgRole.Admin; + contextSrv.user.permissions = {}; + + // does not have access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // does not have access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // has access to a plugin entry page by default + await renderUnderRouter(''); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to a page without roles + await renderUnderRouter('page-without-role'); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to Viewer page + await renderUnderRouter('viewer-page'); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to Editor page + await renderUnderRouter('editor-page'); + expect(await screen.findByText('my great component')).toBeVisible(); + + // has access to a Admin page + await renderUnderRouter('admin-page'); + expect(await screen.findByText('my great component')).toBeVisible(); + }); + + it('a Grafana Admin should be able to see any page without action specifier', async () => { contextSrv.isGrafanaAdmin = true; // Viewer has access to a plugin entry page await renderUnderRouter(''); expect(await screen.findByText('my great component')).toBeVisible(); + // Viewer does not have access to a page with an action but no role + await renderUnderRouter('page-with-action-no-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + + // Viewer does not have access to a page with an action and role + await renderUnderRouter('page-with-action-and-role'); + expect(await screen.findByText('Access denied')).toBeVisible(); + // Admin has access to a page without roles await renderUnderRouter('page-without-role'); expect(await screen.findByText('my great component')).toBeVisible(); diff --git a/public/app/features/plugins/components/AppRootPage.tsx b/public/app/features/plugins/components/AppRootPage.tsx index 95a9cc1f33b..33b6f905f55 100644 --- a/public/app/features/plugins/components/AppRootPage.tsx +++ b/public/app/features/plugins/components/AppRootPage.tsx @@ -101,7 +101,7 @@ export function AppRootPage({ pluginId, pluginNavSection }: Props) { // if the user has permissions to see the plugin page. const userHasPermissionsToPluginPage = () => { // Check if plugin does not have any configurations or the user is Grafana Admin - if (!plugin.meta?.includes || contextSrv.isGrafanaAdmin || contextSrv.user.orgRole === OrgRole.Admin) { + if (!plugin.meta?.includes) { return true; } @@ -110,6 +110,16 @@ export function AppRootPage({ pluginId, pluginNavSection }: Props) { if (!pluginInclude) { return true; } + + // Check if action exists and give access if user has the required permission. + if (pluginInclude?.action && config.featureToggles.accessControlOnCall) { + return contextSrv.hasPermission(pluginInclude.action); + } + + if (contextSrv.isGrafanaAdmin || contextSrv.user.orgRole === OrgRole.Admin) { + return true; + } + const pathRole: string = pluginInclude?.role || ''; // Check if role exists and give access to Editor to be able to see Viewer pages if (!pathRole || (contextSrv.isEditor && pathRole === OrgRole.Viewer)) {