Plugins: Fix catalog permissions for org and server admins (#37504)

* simplify toggle + add link to server admin

* feat(catalog): org admins can configure plugin apps, cannot install/uninstall plugins

* fix(catalog): dont show buttons if user doesn't have install permissions

* feat(catalog): cater for accessing catalog via /plugins and /admin/plugins

* feat(catalog): use location for list links and match.url to define breadcrumb links

* test(catalog): mock isGrafanaAdmin for PluginDetails tests

* test(catalog): preserve default bootdata in PluginDetails mock

* refactor(catalog): move orgAdmin check out of state and make easier to reason with

Co-authored-by: Will Browne <will.browne@grafana.com>

pkg/api/api.go

@@ -287,12 +287,10 @@ func (hs *HTTPServer) registerRoutes() {
 		apiRoute.Any("/plugins/:pluginId/resources/*", hs.CallResource)
 		apiRoute.Get("/plugins/errors", routing.Wrap(hs.GetPluginErrorsList))
 
-		if hs.Cfg.PluginAdminEnabled {
 		apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) {
 			pluginRoute.Post("/:pluginId/install", bind(dtos.InstallPluginCommand{}), routing.Wrap(hs.InstallPlugin))
 			pluginRoute.Post("/:pluginId/uninstall", routing.Wrap(hs.UninstallPlugin))
 		}, reqGrafanaAdmin)
-		}
 
 		apiRoute.Group("/plugins", func(pluginRoute routing.RouteRegister) {
 			pluginRoute.Get("/:pluginId/dashboards/", routing.Wrap(hs.GetPluginDashboards))

pkg/api/frontendsettings.go

@@ -247,7 +247,7 @@ func (hs *HTTPServer) getFrontendSettingsMap(c *models.ReqContext) (map[string]i
 		"http2Enabled":                     hs.Cfg.Protocol == setting.HTTP2Scheme,
 		"sentry":                           hs.Cfg.Sentry,
 		"pluginCatalogURL":                 hs.Cfg.PluginCatalogURL,
-		"pluginAdminEnabled":               (c.IsGrafanaAdmin || hs.Cfg.PluginAdminExternalManageEnabled) && hs.Cfg.PluginAdminEnabled,
+		"pluginAdminEnabled":               hs.Cfg.PluginAdminEnabled,
 		"pluginAdminExternalManageEnabled": hs.Cfg.PluginAdminEnabled && hs.Cfg.PluginAdminExternalManageEnabled,
 		"expressionsEnabled":               hs.Cfg.ExpressionsEnabled,
 		"awsAllowedAuthProviders":          hs.Cfg.AWSAllowedAuthProviders,

pkg/api/index.go

@@ -388,6 +388,12 @@ func (hs *HTTPServer) buildAdminNavLinks(c *models.ReqContext) []*dtos.NavLink {
 		})
 	}
 
+	if hs.Cfg.PluginAdminEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.ActionPluginsManage) {
+		adminNavLinks = append(adminNavLinks, &dtos.NavLink{
+			Text: "Plugins", Id: "admin-plugins", Url: hs.Cfg.AppSubURL + "/admin/plugins", Icon: "plug",
+		})
+	}
+
 	return adminNavLinks
 }
 

pkg/services/accesscontrol/models.go

@@ -88,6 +88,9 @@ const (
 	// Datasources actions
 	ActionDatasourcesExplore = "datasources:explore"
 
+	// Plugin actions
+	ActionPluginsManage = "plugins:manage"
+
 	// Global Scopes
 	ScopeGlobalUsersAll = "global:users:*"
 

public/app/features/plugins/admin/components/InstallControls.tsx

@@ -94,6 +94,11 @@ export const InstallControls = ({ plugin, isInflight, hasUpdate, isInstalled, ha
     );
   }
 
+  if (!hasPermission) {
+    const message = `You need server admin privileges to ${isInstalled ? 'uninstall' : 'install'} this plugin.`;
+    return <div className={styles.message}>{message}</div>;
+  }
+
   if (isInstalled) {
     return (
       <HorizontalGroup height="auto">
@@ -122,7 +127,6 @@ export const InstallControls = ({ plugin, isInflight, hasUpdate, isInstalled, ha
                 Please refresh your browser window before using this plugin.
               </div>
             )}
-            {!hasPermission && <div className={styles.message}>You need admin privileges to manage this plugin.</div>}
           </>
         )}
       </HorizontalGroup>
@@ -149,7 +153,6 @@ export const InstallControls = ({ plugin, isInflight, hasUpdate, isInstalled, ha
           <Button disabled={isInflight || !hasPermission} onClick={onInstall}>
             {isInflight ? 'Installing' : 'Install'}
           </Button>
-          {!hasPermission && <div className={styles.message}>You need admin privileges to install this plugin.</div>}
         </>
       )}
     </HorizontalGroup>

public/app/features/plugins/admin/components/PluginList.tsx

@@ -1,6 +1,7 @@
 import React from 'react';
 import { useStyles2 } from '@grafana/ui';
 import { css } from '@emotion/css';
+import { useLocation } from 'react-router-dom';
 import { Card } from '../components/Card';
 import { Grid } from '../components/Grid';
 
@@ -14,6 +15,7 @@ interface Props {
 
 export const PluginList = ({ plugins }: Props) => {
   const styles = useStyles2(getStyles);
+  const location = useLocation();
 
   return (
     <Grid>
@@ -23,7 +25,7 @@ export const PluginList = ({ plugins }: Props) => {
         return (
           <Card
             key={`${id}`}
-            href={`/plugins/${id}`}
+            href={`${location.pathname}/${id}`}
             image={
               <PluginLogo
                 src={plugin.info.logos.small}

public/app/features/plugins/admin/helpers.ts

@@ -1,11 +1,16 @@
 import { config } from '@grafana/runtime';
 import { gt } from 'semver';
+import { contextSrv } from 'app/core/services/context_srv';
 import { CatalogPlugin, CatalogPluginDetails, LocalPlugin, Plugin, Version, PluginFilter } from './types';
 
 export function isGrafanaAdmin(): boolean {
   return config.bootData.user.isGrafanaAdmin;
 }
 
+export function isOrgAdmin() {
+  return contextSrv.hasRole('Admin');
+}
+
 export function mapRemoteToCatalog(plugin: Plugin): CatalogPlugin {
   const {
     name,

public/app/features/plugins/admin/hooks/usePluginDetails.tsx

@@ -2,7 +2,7 @@ import { useReducer, useEffect } from 'react';
 import { PluginType, PluginIncludeType } from '@grafana/data';
 import { api } from '../api';
 import { loadPlugin } from '../../PluginPage';
-import { getCatalogPluginDetails, isGrafanaAdmin } from '../helpers';
+import { getCatalogPluginDetails, isOrgAdmin } from '../helpers';
 import { ActionTypes, PluginDetailsActions, PluginDetailsState } from '../types';
 
 const defaultTabs = [{ label: 'Overview' }, { label: 'Version history' }];
@@ -10,7 +10,6 @@ const defaultTabs = [{ label: 'Overview' }, { label: 'Version history' }];
 const initialState = {
   hasInstalledPanel: false,
   hasUpdate: false,
-  isAdmin: isGrafanaAdmin(),
   isInstalled: false,
   isInflight: false,
   loading: false,
@@ -87,6 +86,7 @@ const reducer = (state: PluginDetailsState, action: PluginDetailsActions) => {
 
 export const usePluginDetails = (id: string) => {
   const [state, dispatch] = useReducer(reducer, initialState);
+  const userCanConfigurePlugins = isOrgAdmin();
 
   useEffect(() => {
     const fetchPlugin = async () => {
@@ -125,7 +125,7 @@ export const usePluginDetails = (id: string) => {
     const pluginConfig = state.pluginConfig;
     const tabs: Array<{ label: string }> = [...defaultTabs];
 
-    if (pluginConfig && state.isAdmin) {
+    if (pluginConfig && userCanConfigurePlugins) {
       if (pluginConfig.meta.type === PluginType.app) {
         if (pluginConfig.angularConfigCtrl) {
           tabs.push({
@@ -149,7 +149,7 @@ export const usePluginDetails = (id: string) => {
       }
     }
     dispatch({ type: ActionTypes.UPDATE_TABS, payload: tabs });
-  }, [state.isAdmin, state.pluginConfig, id]);
+  }, [userCanConfigurePlugins, state.pluginConfig, id]);
 
   return { state, dispatch };
 };

public/app/features/plugins/admin/pages/Browse.test.tsx

@@ -1,11 +1,12 @@
 import React from 'react';
 import { Router } from 'react-router-dom';
 import { render, RenderResult, waitFor } from '@testing-library/react';
-import BrowsePage from './Browse';
-import { locationService } from '@grafana/runtime';
-import { configureStore } from 'app/store/configureStore';
 import { Provider } from 'react-redux';
-import { LocalPlugin, Plugin } from '../types';
+import { locationService } from '@grafana/runtime';
+import BrowsePage from './Browse';
+import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps';
+import { configureStore } from 'app/store/configureStore';
+import { LocalPlugin, Plugin, PluginAdminRoutes } from '../types';
 import { API_ROOT, GRAFANA_API_ROOT } from '../constants';
 
 jest.mock('@grafana/runtime', () => ({
@@ -27,11 +28,14 @@ jest.mock('@grafana/runtime', () => ({
 function setup(path = '/plugins'): RenderResult {
   const store = configureStore();
   locationService.push(path);
+  const props = getRouteComponentProps({
+    route: { routeName: PluginAdminRoutes.Home } as any,
+  });
 
   return render(
     <Provider store={store}>
       <Router history={locationService.getHistory()}>
-        <BrowsePage />
+        <BrowsePage {...props} />
       </Router>
     </Provider>
   );

public/app/features/plugins/admin/pages/Browse.tsx

@@ -4,11 +4,11 @@ import { SelectableValue, dateTimeParse, GrafanaTheme2 } from '@grafana/data';
 import { LoadingPlaceholder, Select, RadioButtonGroup, useStyles2 } from '@grafana/ui';
 import { useLocation } from 'react-router-dom';
 import { locationSearchToObject } from '@grafana/runtime';
-
+import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
 import { PluginList } from '../components/PluginList';
 import { SearchField } from '../components/SearchField';
 import { useHistory } from '../hooks/useHistory';
-import { CatalogPlugin } from '../types';
+import { CatalogPlugin, PluginAdminRoutes } from '../types';
 import { Page as PluginPage } from '../components/Page';
 import { HorizontalGroup } from '../components/HorizontalGroup';
 import { Page } from 'app/core/components/Page/Page';
@@ -17,10 +17,11 @@ import { useSelector } from 'react-redux';
 import { StoreState } from 'app/types/store';
 import { getNavModel } from 'app/core/selectors/navModel';
 
-export default function Browse(): ReactElement | null {
+export default function Browse({ route }: GrafanaRouteComponentProps): ReactElement | null {
   const location = useLocation();
   const query = locationSearchToObject(location.search);
-  const navModel = useSelector((state: StoreState) => getNavModel(state.navIndex, 'plugins'));
+  const navModelId = getNavModelId(route.routeName);
+  const navModel = useSelector((state: StoreState) => getNavModel(state.navIndex, navModelId));
   const styles = useStyles2(getStyles);
 
   const q = (query.q as string) ?? '';
@@ -127,6 +128,16 @@ const getStyles = (theme: GrafanaTheme2) => ({
   `,
 });
 
+// Because the component is used under multiple paths (/plugins and /admin/plugins) we need to get
+// the correct navModel from the store
+const getNavModelId = (routeName?: string) => {
+  if (routeName === PluginAdminRoutes.HomeAdmin || routeName === PluginAdminRoutes.BrowseAdmin) {
+    return 'admin-plugins';
+  }
+
+  return 'plugins';
+};
+
 const sorters: { [name: string]: (a: CatalogPlugin, b: CatalogPlugin) => number } = {
   name: (a: CatalogPlugin, b: CatalogPlugin) => a.name.localeCompare(b.name),
   updated: (a: CatalogPlugin, b: CatalogPlugin) =>

public/app/features/plugins/admin/pages/PluginDetails.test.tsx

@@ -31,6 +31,13 @@ jest.mock('@grafana/runtime', () => {
     }),
     config: {
       ...original.config,
+      bootData: {
+        ...original.config.bootData,
+        user: {
+          ...original.config.bootData.user,
+          isGrafanaAdmin: true,
+        },
+      },
       buildInfo: {
         ...original.config.buildInfo,
         version: 'v7.5.0',

public/app/features/plugins/admin/pages/PluginDetails.tsx

@@ -1,6 +1,5 @@
 import React from 'react';
 import { css } from '@emotion/css';
-
 import { GrafanaTheme2 } from '@grafana/data';
 import { useStyles2, TabsBar, TabContent, Tab, Icon, Alert } from '@grafana/ui';
 
@@ -34,6 +33,7 @@ export default function PluginDetails({ match }: PluginDetailsProps): JSX.Elemen
   } = state;
   const tab = tabs[activeTab];
   const styles = useStyles2(getStyles);
+  const breadcrumbHref = match.url.substring(0, match.url.lastIndexOf('/'));
 
   if (loading) {
     return (
@@ -66,13 +66,13 @@ export default function PluginDetails({ match }: PluginDetailsProps): JSX.Elemen
                       className={css`
                         text-decoration: underline;
                       `}
-                      href={'/plugins'}
+                      href={breadcrumbHref}
                     >
                       Plugins
                     </a>
                   </li>
                   <li>
-                    <a href={`/plugins/${pluginId}`} aria-current="page">
+                    <a href={`${match.url}`} aria-current="page">
                       {plugin.name}
                     </a>
                   </li>

public/app/features/plugins/admin/types.ts

@@ -1,5 +1,15 @@
 import { GrafanaPlugin, PluginMeta } from '@grafana/data';
 export type PluginTypeCode = 'app' | 'panel' | 'datasource';
+
+export enum PluginAdminRoutes {
+  Home = 'plugins-home',
+  Browse = 'plugins-browse',
+  Details = 'plugins-details',
+  HomeAdmin = 'plugins-home-admin',
+  BrowseAdmin = 'plugins-browse-admin',
+  DetailsAdmin = 'plugins-details-admin',
+}
+
 export interface CatalogPlugin {
   description: string;
   downloads: number;
@@ -137,7 +147,6 @@ export interface Org {
 export interface PluginDetailsState {
   hasInstalledPanel: boolean;
   hasUpdate: boolean;
-  isAdmin: boolean;
   isInstalled: boolean;
   isInflight: boolean;
   loading: boolean;

public/app/features/plugins/routes.ts

@@ -1,6 +1,26 @@
 import { SafeDynamicImport } from 'app/core/components/DynamicImports/SafeDynamicImport';
 import { config } from 'app/core/config';
 import { RouteDescriptor } from 'app/core/navigation/types';
+import { isGrafanaAdmin } from './admin/helpers';
+import { PluginAdminRoutes } from './admin/types';
+
+const pluginAdminRoutes = [
+  {
+    path: '/plugins',
+    routeName: PluginAdminRoutes.Home,
+    component: SafeDynamicImport(() => import(/* webpackChunkName: "PluginListPage" */ './admin/pages/Browse')),
+  },
+  {
+    path: '/plugins/browse',
+    routeName: PluginAdminRoutes.Browse,
+    component: SafeDynamicImport(() => import(/* webpackChunkName: "PluginListPage" */ './admin/pages/Browse')),
+  },
+  {
+    path: '/plugins/:pluginId/',
+    routeName: PluginAdminRoutes.Details,
+    component: SafeDynamicImport(() => import(/* webpackChunkName: "PluginPage" */ './admin/pages/PluginDetails')),
+  },
+];
 
 export function getPluginsAdminRoutes(cfg = config): RouteDescriptor[] {
   if (!cfg.pluginAdminEnabled) {
@@ -22,18 +42,26 @@ export function getPluginsAdminRoutes(cfg = config): RouteDescriptor[] {
     ];
   }
 
+  if (isGrafanaAdmin()) {
     return [
+      ...pluginAdminRoutes,
       {
-      path: '/plugins',
+        path: '/admin/plugins',
+        routeName: PluginAdminRoutes.HomeAdmin,
         component: SafeDynamicImport(() => import(/* webpackChunkName: "PluginListPage" */ './admin/pages/Browse')),
       },
       {
-      path: '/plugins/browse',
+        path: '/admin/plugins/browse',
+        routeName: PluginAdminRoutes.BrowseAdmin,
         component: SafeDynamicImport(() => import(/* webpackChunkName: "PluginListPage" */ './admin/pages/Browse')),
       },
       {
-      path: '/plugins/:pluginId/',
+        path: '/admin/plugins/:pluginId/',
+        routeName: PluginAdminRoutes.DetailsAdmin,
         component: SafeDynamicImport(() => import(/* webpackChunkName: "PluginPage" */ './admin/pages/PluginDetails')),
       },
     ];
+  }
+
+  return pluginAdminRoutes;
 }