From ef6164ef41350399cd36dec4d97c1523534ad463 Mon Sep 17 00:00:00 2001 From: Haris Rozajac <58232930+harisrozajac@users.noreply.github.com> Date: Tue, 5 Aug 2025 10:46:53 -0600 Subject: [PATCH] Kubernetes Dashboards: Allow access when user has access to dashboard but not to parent folder (#109160) * allow access when user has access to dashboard but not folder * disable folder picker when user doesn't have access to folder --- .../NestedFolderPicker/NestedFolderPicker.tsx | 6 +++++ public/app/features/dashboard/api/v1.test.ts | 24 ++++++++++++++++++- public/app/features/dashboard/api/v1.ts | 8 ++++++- public/app/features/dashboard/api/v2.test.ts | 16 ++++++++++++- public/app/features/dashboard/api/v2.ts | 5 +++- 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx index b1ceef4d2e7..b84eff3cb67 100644 --- a/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx +++ b/public/app/core/components/NestedFolderPicker/NestedFolderPicker.tsx @@ -9,6 +9,7 @@ import { GrafanaTheme2 } from '@grafana/data'; import { t } from '@grafana/i18n'; import { config } from '@grafana/runtime'; import { Alert, Icon, Input, LoadingBar, Stack, Text, useStyles2 } from '@grafana/ui'; +import { getStatusFromError } from 'app/core/utils/errors'; import { useGetFolderQuery } from 'app/features/browse-dashboards/api/browseDashboardsAPI'; import { DashboardViewItemWithUIItems, DashboardsTreeItem } from 'app/features/browse-dashboards/types'; import { getGrafanaSearcher } from 'app/features/search/service/searcher'; @@ -72,6 +73,10 @@ export function NestedFolderPicker({ }: NestedFolderPickerProps) { const styles = useStyles2(getStyles); const selectedFolder = useGetFolderQuery(value || skipToken); + // user might not have access to the folder, but they have access to the dashboard + // in this case we disable the folder picker - this is an edge case when user has edit access to a dashboard + // but doesn't have access to the folder + const isForbidden = getStatusFromError(selectedFolder.error) === 403; const nestedFoldersEnabled = Boolean(config.featureToggles.nestedFolders); @@ -296,6 +301,7 @@ export function NestedFolderPicker({ : undefined } {...getReferenceProps()} + disabled={isForbidden} /> ); } diff --git a/public/app/features/dashboard/api/v1.test.ts b/public/app/features/dashboard/api/v1.test.ts index 20c6699d13b..c92d083c2fc 100644 --- a/public/app/features/dashboard/api/v1.test.ts +++ b/public/app/features/dashboard/api/v1.test.ts @@ -163,7 +163,7 @@ describe('v1 dashboard API', () => { expect(result.dashboard.version).toBe(1); }); - it('throws an error if folder is not found', async () => { + it('throws an error if folder service returns an error other than 403', async () => { mockGet.mockResolvedValueOnce({ ...mockDashboardDto, metadata: { @@ -179,6 +179,28 @@ describe('v1 dashboard API', () => { await expect(api.getDashboardDTO('test')).rejects.toThrow('Failed to load folder'); }); + it('should not throw an error if folder is not found and user has access to dashboard but not to folder', async () => { + mockGet.mockResolvedValueOnce({ + ...mockDashboardDto, + metadata: { ...mockDashboardDto.metadata, annotations: { [AnnoKeyFolder]: 'new-folder' } }, + }); + jest.spyOn(backendSrv, 'getFolderByUid').mockRejectedValueOnce({ message: 'folder not found', status: 403 }); + + const api = new K8sDashboardAPI(); + const dashboardDTO = await api.getDashboardDTO('test'); + expect(dashboardDTO.dashboard).toMatchObject({ + schemaVersion: 0, + title: 'test', + uid: 'dash-uid', + version: 1, + }); + // we still want to save the folder uid so that we can properly handle disabling the folder picker in Settings -> General + expect(dashboardDTO.meta.folderUid).toBe('new-folder'); + expect(dashboardDTO.meta.folderTitle).toBeUndefined(); + expect(dashboardDTO.meta.folderUrl).toBeUndefined(); + expect(dashboardDTO.meta.folderId).toBeUndefined(); + }); + describe('saveDashboard', () => { beforeEach(() => { locationUtil.initialize({ diff --git a/public/app/features/dashboard/api/v1.ts b/public/app/features/dashboard/api/v1.ts index d58fa1d78a7..d0d61ed3188 100644 --- a/public/app/features/dashboard/api/v1.ts +++ b/public/app/features/dashboard/api/v1.ts @@ -161,7 +161,13 @@ export class K8sDashboardAPI implements DashboardAPI { result.meta.folderUid = folder.uid; result.meta.folderId = folder.id; } catch (e) { - throw new Error('Failed to load folder'); + // If user has access to dashboard but not to folder, continue without folder info + if (getStatusFromError(e) !== 403) { + throw new Error('Failed to load folder'); + } + // we still want to save the folder uid so that we can properly handle disabling the folder picker in Settings -> General + // this is an edge case when user has edit access to a dashboard but doesn't have access to the folder + result.meta.folderUid = dash.metadata.annotations?.[AnnoKeyFolder]; } } diff --git a/public/app/features/dashboard/api/v2.test.ts b/public/app/features/dashboard/api/v2.test.ts index b06387f6770..98efae8dc2c 100644 --- a/public/app/features/dashboard/api/v2.test.ts +++ b/public/app/features/dashboard/api/v2.test.ts @@ -127,7 +127,7 @@ describe('v2 dashboard API', () => { expect(result.metadata.annotations![AnnoKeyFolder]).toBe('new-folder'); }); - it('throws an error if folder is not found', async () => { + it('throws an error if folder service returns an error other than 403', async () => { mockGet.mockResolvedValueOnce({ ...mockDashboardDto, metadata: { @@ -142,6 +142,20 @@ describe('v2 dashboard API', () => { const api = new K8sDashboardV2API(); await expect(api.getDashboardDTO('test')).rejects.toThrow('Failed to load folder'); }); + + it('should not throw an error if folder is not found and user has access to dashboard but not to folder', async () => { + mockGet.mockResolvedValueOnce({ + ...mockDashboardDto, + metadata: { ...mockDashboardDto.metadata, annotations: { [AnnoKeyFolder]: 'new-folder' } }, + }); + jest.spyOn(backendSrv, 'getFolderByUid').mockRejectedValueOnce({ message: 'folder not found', status: 403 }); + + const api = new K8sDashboardV2API(); + const dashboardDTO = await api.getDashboardDTO('test'); + expect(dashboardDTO.spec).toMatchObject({ + title: '', + }); + }); describe('v2 dashboard API - Save', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/public/app/features/dashboard/api/v2.ts b/public/app/features/dashboard/api/v2.ts index 3e0768992dd..3bb640776d5 100644 --- a/public/app/features/dashboard/api/v2.ts +++ b/public/app/features/dashboard/api/v2.ts @@ -64,7 +64,10 @@ export class K8sDashboardV2API dashboard.metadata.annotations[AnnoKeyFolderTitle] = folder.title; dashboard.metadata.annotations[AnnoKeyFolderUrl] = folder.url; } catch (e) { - throw new Error('Failed to load folder'); + // If user has access to dashboard but not to folder, continue without folder info + if (getStatusFromError(e) !== 403) { + throw new Error('Failed to load folder'); + } } } else if (dashboard.metadata.annotations && !dashboard.metadata.annotations[AnnoKeyFolder]) { // Set AnnoKeyFolder to empty string for top-level dashboards