diff --git a/public/app/features/alerting/unified/Analytics.ts b/public/app/features/alerting/unified/Analytics.ts index 93101ec6250..8e744da63ad 100644 --- a/public/app/features/alerting/unified/Analytics.ts +++ b/public/app/features/alerting/unified/Analytics.ts @@ -29,9 +29,11 @@ export const LogMessages = { loadedCentralAlertStateHistory: 'loaded central alert state history', }; -const { logInfo, logError, logMeasurement } = createMonitoringLogger('features.alerting', { module: 'Alerting' }); +const { logInfo, logError, logMeasurement, logWarning } = createMonitoringLogger('features.alerting', { + module: 'Alerting', +}); -export { logError, logInfo, logMeasurement }; +export { logError, logInfo, logMeasurement, logWarning }; // eslint-disable-next-line @typescript-eslint/no-explicit-any export function withPerformanceLogging Promise>( diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx index 8805b40cf1e..e734019a164 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx @@ -33,6 +33,7 @@ import { import { shouldUsePrometheusRulesPrimary } from '../../../featureToggles'; import { useDeleteRuleFromGroup } from '../../../hooks/ruleGroup/useDeleteRuleFromGroup'; import { useAddRuleToRuleGroup, useUpdateRuleInRuleGroup } from '../../../hooks/ruleGroup/useUpsertRuleFromRuleGroup'; +import { useReturnTo } from '../../../hooks/useReturnTo'; import { useURLSearchParams } from '../../../hooks/useURLSearchParams'; import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; import { @@ -79,6 +80,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { const [addRuleToRuleGroup] = useAddRuleToRuleGroup(); const [updateRuleInRuleGroup] = useUpdateRuleInRuleGroup(); + const { returnTo } = useReturnTo(); const routeParams = useParams<{ type: string; id: string }>(); const ruleType = translateRouteParamToRuleType(routeParams.type); @@ -170,9 +172,9 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { const { dataSourceName, namespaceName, groupName } = ruleGroupIdentifier; if (exitOnSave) { - const returnTo = queryParams.get('returnTo') || getReturnToUrl(ruleGroupIdentifier, ruleDefinition); + const returnToUrl = returnTo || getReturnToUrl(ruleGroupIdentifier, ruleDefinition); - locationService.push(returnTo); + locationService.push(returnToUrl); return; } @@ -190,8 +192,8 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { existing, grafanaTypeRule, notifyApp, - queryParams, updateRuleInRuleGroup, + returnTo, ] ); diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx index ee890b033bc..409d67e5474 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx @@ -6,7 +6,6 @@ import { config } from '@grafana/runtime'; import { Button, CustomScrollbar, LinkButton, LoadingPlaceholder, Stack } from '@grafana/ui'; import { usePageToolbar } from 'app/core/components/Page/Page'; import { useAppNotification } from 'app/core/copy/appNotification'; -import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { AppChromeUpdate } from '../../../../../../core/components/AppChrome/AppChromeUpdate'; import { @@ -17,6 +16,7 @@ import { import { alertRuleApi } from '../../../api/alertRuleApi'; import { fetchRulerRulesGroup } from '../../../api/ruler'; import { useDataSourceFeatures } from '../../../hooks/useCombinedRule'; +import { useReturnTo } from '../../../hooks/useReturnTo'; import { RuleFormValues } from '../../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource'; import { DEFAULT_GROUP_EVALUATION_INTERVAL, formValuesToRulerGrafanaRuleDTO } from '../../../utils/rule-form'; @@ -41,11 +41,10 @@ export function ModifyExportRuleForm({ ruleForm, alertUid }: ModifyExportRuleFor defaultValues: ruleForm, shouldFocusError: true, }); - const [queryParams] = useQueryParams(); const existing = Boolean(ruleForm); // always should be true const notifyApp = useAppNotification(); - const returnTo = !queryParams.returnTo ? '/alerting/list' : String(queryParams.returnTo); + const { returnTo } = useReturnTo('/alerting/list'); const [exportData, setExportData] = useState(undefined); diff --git a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx index 4c2efd64e58..5070f7f58f4 100644 --- a/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx +++ b/public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx @@ -16,6 +16,7 @@ import { PromAlertingRuleState, PromRuleType } from 'app/types/unified-alerting- import { defaultPageNav } from '../../RuleViewer'; import { shouldUsePrometheusRulesPrimary } from '../../featureToggles'; import { usePrometheusCreationConsistencyCheck } from '../../hooks/usePrometheusConsistencyCheck'; +import { useReturnTo } from '../../hooks/useReturnTo'; import { PluginOriginBadge } from '../../plugins/PluginOriginBadge'; import { Annotation } from '../../utils/constants'; import { makeDashboardLink, makePanelLink, stringifyErrorLike } from '../../utils/misc'; @@ -244,9 +245,9 @@ interface TitleProps { export const Title = ({ name, paused = false, state, health, ruleType, ruleOrigin }: TitleProps) => { const styles = useStyles2(getStyles); - const [queryParams] = useQueryParams(); const isRecordingRule = ruleType === PromRuleType.Recording; - const returnTo = queryParams.returnTo ? String(queryParams.returnTo) : '/alerting/list'; + + const { returnTo } = useReturnTo('/alerting/list'); return (
diff --git a/public/app/features/alerting/unified/hooks/useReturnTo.test.tsx b/public/app/features/alerting/unified/hooks/useReturnTo.test.tsx new file mode 100644 index 00000000000..80adb1661cc --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useReturnTo.test.tsx @@ -0,0 +1,48 @@ +import { MemoryRouter } from 'react-router-dom-v5-compat'; +import { renderHook } from 'test/test-utils'; + +import { useReturnTo } from './useReturnTo'; + +describe('useReturnTo', () => { + beforeAll(() => { + // @ts-expect-error + delete window.location; + window.location = { origin: 'https://play.grafana.net' } as Location; + }); + + it('should return the fallback value when `returnTo` is not present in the query string', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { wrapper: MemoryRouter }); + + expect(result.current.returnTo).toBe('/fallback'); + }); + + it('should return the sanitized `returnTo` value when it is present in the query string and is a valid URL within the Grafana app', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(result.current.returnTo).toBe('/dashboard/db/my-dashboard'); + }); + + it('should return the fallback value when `returnTo` is present in the query string but is not a valid URL within the Grafana app', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(result.current.returnTo).toBe('/fallback'); + }); + + it('should return the fallback value when `returnTo` is present in the query string but is a malicious JavaScript URL', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(result.current.returnTo).toBe('/fallback'); + }); +}); diff --git a/public/app/features/alerting/unified/hooks/useReturnTo.ts b/public/app/features/alerting/unified/hooks/useReturnTo.ts new file mode 100644 index 00000000000..b522bf2c8f6 --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useReturnTo.ts @@ -0,0 +1,49 @@ +import { textUtil } from '@grafana/data'; +import { config } from '@grafana/runtime'; + +import { logWarning } from '../Analytics'; + +import { useURLSearchParams } from './useURLSearchParams'; + +/** + * This hook provides a safe way to obtain the `returnTo` URL from the query string parameter + * It validates the origin and protocol to ensure the URL is withing the Grafana app + */ +export function useReturnTo(fallback?: string): { returnTo: string | undefined } { + const emptyResult = { returnTo: fallback }; + + const [searchParams] = useURLSearchParams(); + const returnTo = searchParams.get('returnTo'); + + if (!returnTo) { + return emptyResult; + } + + const sanitizedReturnTo = textUtil.sanitizeUrl(returnTo); + const baseUrl = `${window.location.origin}/${config.appSubUrl}`; + + const sanitizedUrl = tryParseURL(sanitizedReturnTo, baseUrl); + + if (!sanitizedUrl) { + logWarning('Malformed returnTo parameter', { returnTo }); + return emptyResult; + } + + const { protocol, origin, pathname, search } = sanitizedUrl; + if (['http:', 'https:'].includes(protocol) === false || origin !== window.location.origin) { + logWarning('Malformed returnTo parameter', { returnTo }); + return emptyResult; + } + + return { returnTo: `${pathname}${search}` }; +} + +// Tries to mimic URL.parse method https://developer.mozilla.org/en-US/docs/Web/API/URL/parse_static +function tryParseURL(sanitizedReturnTo: string, baseUrl: string) { + try { + const url = new URL(sanitizedReturnTo, baseUrl); + return url; + } catch (error) { + return null; + } +}