Plugin Extensions: Update extension-point ID validation (#107959)
* feat(extensions): don't allow core grafana extension point ids in plugins * feat(extensions): log more specific errors if extension point id validation fails * chore: move the ExtensionSidebar ext. point id to grafana-data * review: remove type assertion
This commit is contained in:
@@ -195,6 +195,7 @@ export enum PluginExtensionPoints {
|
||||
TraceViewResourceAttributes = 'grafana/traceview/resource-attributes',
|
||||
LogsViewResourceAttributes = 'grafana/logsview/resource-attributes',
|
||||
AppChrome = 'grafana/app/chrome/v1',
|
||||
ExtensionSidebar = 'grafana/extension-sidebar/v0-alpha',
|
||||
}
|
||||
|
||||
export type PluginExtensionPanelContext = {
|
||||
|
||||
@@ -1,14 +1,10 @@
|
||||
import { css } from '@emotion/css';
|
||||
|
||||
import { GrafanaTheme2 } from '@grafana/data';
|
||||
import { GrafanaTheme2, PluginExtensionPoints } from '@grafana/data';
|
||||
import { usePluginComponents } from '@grafana/runtime';
|
||||
import { useTheme2 } from '@grafana/ui';
|
||||
|
||||
import {
|
||||
EXTENSION_SIDEBAR_EXTENSION_POINT_ID,
|
||||
getComponentMetaFromComponentId,
|
||||
useExtensionSidebarContext,
|
||||
} from './ExtensionSidebarProvider';
|
||||
import { getComponentMetaFromComponentId, useExtensionSidebarContext } from './ExtensionSidebarProvider';
|
||||
|
||||
export const DEFAULT_EXTENSION_SIDEBAR_WIDTH = 300;
|
||||
export const MIN_EXTENSION_SIDEBAR_WIDTH = 100;
|
||||
@@ -22,7 +18,7 @@ export function ExtensionSidebar() {
|
||||
const styles = getStyles(useTheme2());
|
||||
const { dockedComponentId, isEnabled, props = {} } = useExtensionSidebarContext();
|
||||
const { components, isLoading } = usePluginComponents<ExtensionSidebarComponentProps>({
|
||||
extensionPointId: EXTENSION_SIDEBAR_EXTENSION_POINT_ID,
|
||||
extensionPointId: PluginExtensionPoints.ExtensionSidebar,
|
||||
});
|
||||
|
||||
if (isLoading || !dockedComponentId || !isEnabled) {
|
||||
|
||||
@@ -1,14 +1,13 @@
|
||||
import { createContext, ReactNode, useCallback, useContext, useEffect, useState, useMemo } from 'react';
|
||||
import { useLocalStorage } from 'react-use';
|
||||
|
||||
import { store, type ExtensionInfo } from '@grafana/data';
|
||||
import { PluginExtensionPoints, store, type ExtensionInfo } from '@grafana/data';
|
||||
import { config, getAppEvents, reportInteraction, usePluginLinks, locationService } from '@grafana/runtime';
|
||||
import { ExtensionPointPluginMeta, getExtensionPointPluginMeta } from 'app/features/plugins/extensions/utils';
|
||||
import { OpenExtensionSidebarEvent } from 'app/types/events';
|
||||
|
||||
import { DEFAULT_EXTENSION_SIDEBAR_WIDTH } from './ExtensionSidebar';
|
||||
|
||||
export const EXTENSION_SIDEBAR_EXTENSION_POINT_ID = 'grafana/extension-sidebar/v0-alpha';
|
||||
export const EXTENSION_SIDEBAR_DOCKED_LOCAL_STORAGE_KEY = 'grafana.navigation.extensionSidebarDocked';
|
||||
export const EXTENSION_SIDEBAR_WIDTH_LOCAL_STORAGE_KEY = 'grafana.navigation.extensionSidebarWidth';
|
||||
const PERMITTED_EXTENSION_SIDEBAR_PLUGINS = [
|
||||
@@ -94,7 +93,7 @@ export const ExtensionSidebarContextProvider = ({ children }: ExtensionSidebarCo
|
||||
// `grafana/extension-sidebar/v0-alpha` and the link's `configure` method would control
|
||||
// whether the component is rendered or not
|
||||
const { links, isLoading } = usePluginLinks({
|
||||
extensionPointId: EXTENSION_SIDEBAR_EXTENSION_POINT_ID,
|
||||
extensionPointId: PluginExtensionPoints.ExtensionSidebar,
|
||||
context: {
|
||||
path: currentPath,
|
||||
},
|
||||
@@ -107,7 +106,7 @@ export const ExtensionSidebarContextProvider = ({ children }: ExtensionSidebarCo
|
||||
() =>
|
||||
isEnabled
|
||||
? new Map(
|
||||
Array.from(getExtensionPointPluginMeta(EXTENSION_SIDEBAR_EXTENSION_POINT_ID).entries()).filter(
|
||||
Array.from(getExtensionPointPluginMeta(PluginExtensionPoints.ExtensionSidebar).entries()).filter(
|
||||
([pluginId, pluginMeta]) =>
|
||||
PERMITTED_EXTENSION_SIDEBAR_PLUGINS.includes(pluginId) &&
|
||||
links.some(
|
||||
|
||||
@@ -1,5 +1,13 @@
|
||||
export const INVALID_EXTENSION_POINT_ID =
|
||||
'Invalid usage of extension point. Reason: Extension point id should be prefixed with your plugin id, e.g "myorg-foo-app/toolbar/v1".';
|
||||
'Invalid usage of extension point. Reason: Extension point id should be prefixed with your plugin id, e.g "myorg-foo-app/toolbar/v1". Returning an empty array of extensions.';
|
||||
|
||||
export const INVALID_EXTENSION_POINT_ID_PLUGIN = (pluginId: string, extensionPointId: string) =>
|
||||
`Invalid usage of extension point. Reason: Extension point id should be prefixed with your plugin id, e.g "${pluginId}/${extensionPointId}".`;
|
||||
|
||||
export const INVALID_EXTENSION_POINT_ID_GRAFANA_PREFIX = (extensionPointId: string) =>
|
||||
`Invalid usage of extension point. Reason: Core Grafana extension point id should be prefixed with "grafana/", e.g "grafana/${extensionPointId}".`;
|
||||
|
||||
export const INVALID_EXTENSION_POINT_ID_GRAFANA_EXPOSED = `Invalid usage of extension point. Reason: Core Grafana extension point id should be exposed to plugins via the "PluginExtensionPoints" enum in the "grafana-data" package (/packages/grafana-data/src/types/pluginExtensions.ts).`;
|
||||
|
||||
export const EXTENSION_POINT_META_INFO_MISSING =
|
||||
'Invalid usage of extension point. Reason: The extension point is not recorded in the "plugin.json" file. Extension points must be listed in the section "extensions.extensionPoints[]". Returning an empty array of extensions.';
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { act, render, renderHook, screen } from '@testing-library/react';
|
||||
import React from 'react';
|
||||
|
||||
import { PluginContextProvider, PluginMeta, PluginType } from '@grafana/data';
|
||||
import { PluginContextProvider, PluginExtensionPoints, PluginMeta, PluginType } from '@grafana/data';
|
||||
import { config } from '@grafana/runtime';
|
||||
|
||||
import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext';
|
||||
@@ -498,7 +498,7 @@ describe('usePluginComponents()', () => {
|
||||
pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context
|
||||
configs: [
|
||||
{
|
||||
targets: 'grafana/extension-point/v1',
|
||||
targets: PluginExtensionPoints.DashboardPanelMenu,
|
||||
title: '1',
|
||||
description: '1',
|
||||
component: () => <div>Component</div>,
|
||||
@@ -506,14 +506,17 @@ describe('usePluginComponents()', () => {
|
||||
],
|
||||
});
|
||||
|
||||
let { result } = renderHook(() => usePluginComponents({ extensionPointId: 'grafana/extension-point/v1' }), {
|
||||
wrapper,
|
||||
});
|
||||
let { result } = renderHook(
|
||||
() => usePluginComponents({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }),
|
||||
{
|
||||
wrapper,
|
||||
}
|
||||
);
|
||||
expect(result.current.components.length).toBe(1);
|
||||
expect(log.error).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not validate the extension point id if used in Grafana core (no plugin context)', () => {
|
||||
it('should not allow to create an extension point in core Grafana that is not exposed to plugins', () => {
|
||||
// Imitate running in dev mode
|
||||
jest.mocked(isGrafanaDevMode).mockReturnValue(true);
|
||||
|
||||
@@ -522,11 +525,26 @@ describe('usePluginComponents()', () => {
|
||||
<ExtensionRegistriesProvider registries={registries}>{children}</ExtensionRegistriesProvider>
|
||||
);
|
||||
|
||||
let { result } = renderHook(() => usePluginComponents({ extensionPointId: 'invalid-extension-point-id' }), {
|
||||
const extensionPointId = 'grafana/not-exposed-extension-point/v1';
|
||||
|
||||
// Adding an extension to the extension point
|
||||
registries.addedComponentsRegistry.register({
|
||||
pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context
|
||||
configs: [
|
||||
{
|
||||
targets: extensionPointId,
|
||||
title: '1',
|
||||
description: '1',
|
||||
component: () => <div>Component</div>,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
let { result } = renderHook(() => usePluginComponents({ extensionPointId }), {
|
||||
wrapper,
|
||||
});
|
||||
expect(result.current.components.length).toBe(0);
|
||||
expect(log.error).not.toHaveBeenCalled();
|
||||
expect(log.error).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should validate if the extension point meta-info is correct if in dev-mode and used by a plugin', () => {
|
||||
|
||||
@@ -28,8 +28,7 @@ export function usePluginComponents<Props extends object = {}>({
|
||||
const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId));
|
||||
|
||||
return useMemo(() => {
|
||||
// For backwards compatibility we don't enable restrictions in production or when the hook is used in core Grafana.
|
||||
const enableRestrictions = isGrafanaDevMode() && pluginContext;
|
||||
const isInsidePlugin = Boolean(pluginContext);
|
||||
const components: Array<ComponentTypeWithExtensionMeta<Props>> = [];
|
||||
const extensionsByPlugin: Record<string, number> = {};
|
||||
const pluginId = pluginContext?.meta.id ?? '';
|
||||
@@ -38,13 +37,16 @@ export function usePluginComponents<Props extends object = {}>({
|
||||
extensionPointId,
|
||||
});
|
||||
|
||||
// Only log error for an invalid `extensionPointId` in DEV mode
|
||||
if (enableRestrictions && !isExtensionPointIdValid({ extensionPointId, pluginId })) {
|
||||
pointLog.error(errors.INVALID_EXTENSION_POINT_ID);
|
||||
// Don't show extensions if the extension-point id is invalid in DEV mode
|
||||
if (isGrafanaDevMode() && !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, log: pointLog })) {
|
||||
return {
|
||||
isLoading: false,
|
||||
components: [],
|
||||
};
|
||||
}
|
||||
|
||||
// Don't show extensions if the extension-point misses meta info (plugin.json) in DEV mode
|
||||
if (enableRestrictions && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) {
|
||||
if (isGrafanaDevMode() && pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) {
|
||||
pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING);
|
||||
return {
|
||||
isLoading: false,
|
||||
|
||||
@@ -23,8 +23,7 @@ export function usePluginFunctions<Signature>({
|
||||
const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(deps);
|
||||
|
||||
return useMemo(() => {
|
||||
// For backwards compatibility we don't enable restrictions in production or when the hook is used in core Grafana.
|
||||
const enableRestrictions = isGrafanaDevMode() && pluginContext;
|
||||
const isInsidePlugin = Boolean(pluginContext);
|
||||
const results: Array<PluginExtensionFunction<Signature>> = [];
|
||||
const extensionsByPlugin: Record<string, number> = {};
|
||||
const pluginId = pluginContext?.meta.id ?? '';
|
||||
@@ -32,11 +31,15 @@ export function usePluginFunctions<Signature>({
|
||||
pluginId,
|
||||
extensionPointId,
|
||||
});
|
||||
if (enableRestrictions && !isExtensionPointIdValid({ extensionPointId, pluginId })) {
|
||||
pointLog.error(errors.INVALID_EXTENSION_POINT_ID);
|
||||
|
||||
if (isGrafanaDevMode() && !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, log: pointLog })) {
|
||||
return {
|
||||
isLoading: false,
|
||||
functions: [],
|
||||
};
|
||||
}
|
||||
|
||||
if (enableRestrictions && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) {
|
||||
if (isGrafanaDevMode() && pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) {
|
||||
pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING);
|
||||
return {
|
||||
isLoading: false,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { act, renderHook } from '@testing-library/react';
|
||||
|
||||
import { PluginContextProvider, PluginMeta, PluginType } from '@grafana/data';
|
||||
import { PluginContextProvider, PluginExtensionPoints, PluginMeta, PluginType } from '@grafana/data';
|
||||
|
||||
import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext';
|
||||
import { log } from './logs/log';
|
||||
@@ -256,7 +256,7 @@ describe('usePluginLinks()', () => {
|
||||
pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context
|
||||
configs: [
|
||||
{
|
||||
targets: 'grafana/extension-point/v1',
|
||||
targets: PluginExtensionPoints.DashboardPanelMenu,
|
||||
title: '1',
|
||||
description: '1',
|
||||
path: `/a/grafana/${pluginId}/2`,
|
||||
@@ -264,11 +264,42 @@ describe('usePluginLinks()', () => {
|
||||
],
|
||||
});
|
||||
|
||||
let { result } = renderHook(() => usePluginLinks({ extensionPointId: 'grafana/extension-point/v1' }), { wrapper });
|
||||
let { result } = renderHook(() => usePluginLinks({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), {
|
||||
wrapper,
|
||||
});
|
||||
expect(result.current.links.length).toBe(1);
|
||||
expect(log.warning).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not allow to create an extension point in core Grafana that is not exposed to plugins', () => {
|
||||
// Imitate running in dev mode
|
||||
jest.mocked(isGrafanaDevMode).mockReturnValue(true);
|
||||
|
||||
// No plugin context -> used in Grafana core
|
||||
wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<ExtensionRegistriesProvider registries={registries}>{children}</ExtensionRegistriesProvider>
|
||||
);
|
||||
|
||||
const extensionPointId = 'grafana/not-exposed-extension-point/v1';
|
||||
|
||||
// Adding an extension to the extension point
|
||||
registries.addedLinksRegistry.register({
|
||||
pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context
|
||||
configs: [
|
||||
{
|
||||
targets: extensionPointId,
|
||||
title: '1',
|
||||
description: '1',
|
||||
path: `/a/grafana/${pluginId}/2`,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper });
|
||||
expect(result.current.links.length).toBe(0);
|
||||
expect(log.error).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not validate the extension point id if used in Grafana core (no plugin context)', () => {
|
||||
// Imitate running in dev mode
|
||||
jest.mocked(isGrafanaDevMode).mockReturnValue(true);
|
||||
|
||||
@@ -32,23 +32,21 @@ export function usePluginLinks({
|
||||
const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId));
|
||||
|
||||
return useMemo(() => {
|
||||
// For backwards compatibility we don't enable restrictions in production or when the hook is used in core Grafana.
|
||||
const enableRestrictions = isGrafanaDevMode() && pluginContext !== null;
|
||||
const isInsidePlugin = Boolean(pluginContext);
|
||||
const pluginId = pluginContext?.meta.id ?? '';
|
||||
const pointLog = log.child({
|
||||
pluginId,
|
||||
extensionPointId,
|
||||
});
|
||||
|
||||
if (enableRestrictions && !isExtensionPointIdValid({ extensionPointId, pluginId })) {
|
||||
pointLog.error(errors.INVALID_EXTENSION_POINT_ID);
|
||||
if (isGrafanaDevMode() && !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, log: pointLog })) {
|
||||
return {
|
||||
isLoading: false,
|
||||
links: [],
|
||||
};
|
||||
}
|
||||
|
||||
if (enableRestrictions && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) {
|
||||
if (isGrafanaDevMode() && pluginContext && isExtensionPointMetaInfoMissing(extensionPointId, pluginContext)) {
|
||||
pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING);
|
||||
return {
|
||||
isLoading: false,
|
||||
|
||||
@@ -198,9 +198,8 @@ describe('Plugin Extension Validators', () => {
|
||||
|
||||
describe('isExtensionPointIdValid()', () => {
|
||||
test.each([
|
||||
// We (for now allow core Grafana extension points to run without a version)
|
||||
['grafana/extension-point', ''],
|
||||
['grafana/extension-point', 'grafana'],
|
||||
[PluginExtensionPoints.DashboardPanelMenu, ''],
|
||||
[PluginExtensionPoints.DashboardPanelMenu, 'grafana'],
|
||||
['myorg-extensions-app/extension-point', 'myorg-extensions-app'],
|
||||
['myorg-extensions-app/extension-point/v1', 'myorg-extensions-app'],
|
||||
['plugins/myorg-extensions-app/extension-point/v1', 'myorg-extensions-app'],
|
||||
@@ -217,6 +216,8 @@ describe('Plugin Extension Validators', () => {
|
||||
isExtensionPointIdValid({
|
||||
extensionPointId,
|
||||
pluginId,
|
||||
isInsidePlugin: pluginId !== 'grafana' && pluginId !== '',
|
||||
log: createLogMock(),
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
@@ -232,11 +233,18 @@ describe('Plugin Extension Validators', () => {
|
||||
'extension-point/v1',
|
||||
'myorgs-extensions-app',
|
||||
],
|
||||
[
|
||||
// Not exposed to plugins
|
||||
'grafana/not-exposed-extension-point/v1',
|
||||
'grafana',
|
||||
],
|
||||
])('should return FALSE if the extension point id is invalid ("%s", "%s")', (extensionPointId, pluginId) => {
|
||||
expect(
|
||||
isExtensionPointIdValid({
|
||||
extensionPointId,
|
||||
pluginId,
|
||||
isInsidePlugin: pluginId !== 'grafana' && pluginId !== '',
|
||||
log: createLogMock(),
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
@@ -68,15 +68,33 @@ export function isLinkPathValid(pluginId: string, path: string) {
|
||||
export function isExtensionPointIdValid({
|
||||
extensionPointId,
|
||||
pluginId,
|
||||
isInsidePlugin,
|
||||
log,
|
||||
}: {
|
||||
extensionPointId: string;
|
||||
pluginId: string;
|
||||
isInsidePlugin: boolean;
|
||||
log: ExtensionsLog;
|
||||
}) {
|
||||
if (extensionPointId.startsWith('grafana/')) {
|
||||
return true;
|
||||
const startsWithPluginId =
|
||||
extensionPointId.startsWith(`${pluginId}/`) || extensionPointId.startsWith(`plugins/${pluginId}/`);
|
||||
|
||||
if (isInsidePlugin && !startsWithPluginId) {
|
||||
log.error(errors.INVALID_EXTENSION_POINT_ID_PLUGIN(pluginId, extensionPointId));
|
||||
return false;
|
||||
}
|
||||
|
||||
return Boolean(extensionPointId.startsWith(`plugins/${pluginId}/`) || extensionPointId.startsWith(`${pluginId}/`));
|
||||
if (!isInsidePlugin && !extensionPointId.startsWith('grafana/')) {
|
||||
log.error(errors.INVALID_EXTENSION_POINT_ID_GRAFANA_PREFIX(extensionPointId));
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!isInsidePlugin && !Object.values<string>(PluginExtensionPoints).includes(extensionPointId)) {
|
||||
log.error(errors.INVALID_EXTENSION_POINT_ID_GRAFANA_EXPOSED);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
export function extensionPointEndsWithVersion(extensionPointId: string) {
|
||||
|
||||
Reference in New Issue
Block a user