Skip to content

Commit 6948252

Browse files
karthikjeeyarkarthikjeeyar
committed
fix(extensions): make configuration and authorization api calls only if plugin name is passed (#1916)
* make plugin configuration and authorization api calls only when a plugin name is passed * add types for pluginPermissions
1 parent 7c0fb0f commit 6948252

9 files changed

Lines changed: 363 additions & 17 deletions

File tree

‎workspaces/marketplace/.changeset/wild-poets-pump.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-marketplace': patch
3+
---
4+
5+
add checks to plugin authorization and configuration API calls

‎workspaces/marketplace/plugins/marketplace/src/components/MarketplacePluginContent.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import { useExtensionsConfiguration } from '../hooks/useExtensionsConfiguration'
6969
import { usePluginConfigurationPermissions } from '../hooks/usePluginConfigurationPermissions';
7070
import { useNodeEnvironment } from '../hooks/useNodeEnvironment';
7171
import { getPluginActionTooltipMessage } from '../utils';
72+
import { Permission } from '../types';
7273

7374
import { BadgeChip } from './Badges';
7475
import { PluginIcon } from './PluginIcon';
@@ -425,8 +426,8 @@ export const MarketplacePluginContent = ({
425426
title={getPluginActionTooltipMessage(
426427
isProductionEnvironment,
427428
{
428-
read: pluginConfigPerm.data?.read ?? 'DENY',
429-
write: pluginConfigPerm.data?.write ?? 'DENY',
429+
read: pluginConfigPerm.data?.read ?? Permission.DENY,
430+
write: pluginConfigPerm.data?.write ?? Permission.DENY,
430431
},
431432
t,
432433
)}

‎workspaces/marketplace/plugins/marketplace/src/components/MarketplacePluginInstallContent.tsx‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import {
6464
getPluginActionTooltipMessage,
6565
isPluginInstalled,
6666
} from '../utils';
67+
import { Permission } from '../types';
6768

6869
import { CodeEditorContextProvider, useCodeEditor } from './CodeEditor';
6970
import {
@@ -301,7 +302,7 @@ export const MarketplacePluginInstallContent = ({
301302
const isInstallDisabled =
302303
isProductionEnvironment ||
303304
installationError ||
304-
pluginConfigPermissions.data?.write !== 'ALLOW' ||
305+
pluginConfigPermissions.data?.write !== Permission.ALLOW ||
305306
(pluginConfig.data as any)?.error ||
306307
!extensionsConfig?.data?.enabled ||
307308
isSubmitting ||
@@ -310,8 +311,8 @@ export const MarketplacePluginInstallContent = ({
310311
const installTooltip = getPluginActionTooltipMessage(
311312
isProductionEnvironment,
312313
{
313-
read: pluginConfigPermissions.data?.read ?? 'DENY',
314-
write: pluginConfigPermissions.data?.write ?? 'DENY',
314+
read: pluginConfigPermissions.data?.read ?? Permission.DENY,
315+
write: pluginConfigPermissions.data?.write ?? Permission.DENY,
315316
},
316317
t,
317318
!extensionsConfig?.data?.enabled,
@@ -564,8 +565,8 @@ export const MarketplacePluginInstallContent = ({
564565
>
565566
{isInstallDisabled ? t('install.back') : t('install.cancel')}
566567
</Button>
567-
{(pluginConfigPermissions.data?.write === 'ALLOW' ||
568-
pluginConfigPermissions.data?.read === 'ALLOW') && (
568+
{(pluginConfigPermissions.data?.write === Permission.ALLOW ||
569+
pluginConfigPermissions.data?.read === Permission.ALLOW) && (
569570
<Button
570571
variant="text"
571572
color="primary"

‎workspaces/marketplace/plugins/marketplace/src/hooks/usePackageConfig.test.tsx‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*
2+
* Copyright The Backstage Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { renderHook, waitFor } from '@testing-library/react';
18+
import { useQuery } from '@tanstack/react-query';
19+
20+
import { usePackageConfig } from './usePackageConfig';
21+
22+
jest.mock('@backstage/core-plugin-api', () => ({
23+
...jest.requireActual('@backstage/core-plugin-api'),
24+
useApi: jest.fn(),
25+
}));
26+
27+
jest.mock('@tanstack/react-query', () => ({
28+
...jest.requireActual('@tanstack/react-query'),
29+
useQuery: jest.fn(),
30+
}));
31+
32+
describe('usePackageConfig', () => {
33+
beforeEach(() => {
34+
jest.clearAllMocks();
35+
});
36+
37+
it('returns package config when namespace and name are provided', async () => {
38+
const mockConfig = { configYaml: 'key: value\nother: config' };
39+
(useQuery as jest.Mock).mockReturnValue({
40+
data: mockConfig,
41+
isLoading: false,
42+
error: undefined,
43+
});
44+
45+
const { result } = renderHook(() =>
46+
usePackageConfig('default', 'my-package'),
47+
);
48+
49+
await waitFor(() => expect(result.current.isLoading).toBe(false));
50+
expect(result.current.data).toEqual(mockConfig);
51+
expect(result.current.error).toBeUndefined();
52+
});
53+
54+
it('returns null when namespace is empty', async () => {
55+
(useQuery as jest.Mock).mockReturnValue({
56+
data: null,
57+
isLoading: false,
58+
error: undefined,
59+
});
60+
61+
const { result } = renderHook(() => usePackageConfig('', 'my-package'));
62+
63+
await waitFor(() => expect(result.current.isLoading).toBe(false));
64+
expect(result.current.data).toBeNull();
65+
});
66+
67+
it('returns null when name is empty', async () => {
68+
(useQuery as jest.Mock).mockReturnValue({
69+
data: null,
70+
isLoading: false,
71+
error: undefined,
72+
});
73+
74+
const { result } = renderHook(() => usePackageConfig('default', ''));
75+
76+
await waitFor(() => expect(result.current.isLoading).toBe(false));
77+
expect(result.current.data).toBeNull();
78+
});
79+
80+
it('handles loading state', async () => {
81+
(useQuery as jest.Mock).mockReturnValue({
82+
data: undefined,
83+
isLoading: true,
84+
error: undefined,
85+
});
86+
87+
const { result } = renderHook(() =>
88+
usePackageConfig('default', 'my-package'),
89+
);
90+
91+
expect(result.current.isLoading).toBe(true);
92+
expect(result.current.data).toBeUndefined();
93+
});
94+
95+
it('handles error state', async () => {
96+
const error = new Error('Failed to fetch package config');
97+
(useQuery as jest.Mock).mockReturnValue({
98+
data: undefined,
99+
isLoading: false,
100+
error,
101+
});
102+
103+
const { result } = renderHook(() =>
104+
usePackageConfig('default', 'my-package'),
105+
);
106+
107+
await waitFor(() => expect(result.current.isLoading).toBe(false));
108+
expect(result.current.error).toBeInstanceOf(Error);
109+
expect(result.current.error?.message).toBe(
110+
'Failed to fetch package config',
111+
);
112+
});
113+
114+
it('uses correct query key with namespace and name', () => {
115+
(useQuery as jest.Mock).mockReturnValue({
116+
data: { configYaml: '' },
117+
isLoading: false,
118+
error: undefined,
119+
});
120+
121+
renderHook(() => usePackageConfig('my-namespace', 'my-package'));
122+
123+
expect(useQuery).toHaveBeenCalledWith(
124+
expect.objectContaining({
125+
queryKey: [
126+
'marketplaceApi',
127+
'getPackageConfigByName',
128+
'my-namespace',
129+
'my-package',
130+
],
131+
refetchOnWindowFocus: false,
132+
}),
133+
);
134+
});
135+
136+
it('disables refetch on window focus', () => {
137+
(useQuery as jest.Mock).mockReturnValue({
138+
data: { configYaml: '' },
139+
isLoading: false,
140+
error: undefined,
141+
});
142+
143+
renderHook(() => usePackageConfig('default', 'my-package'));
144+
145+
expect(useQuery).toHaveBeenCalledWith(
146+
expect.objectContaining({
147+
refetchOnWindowFocus: false,
148+
}),
149+
);
150+
});
151+
});

‎workspaces/marketplace/plugins/marketplace/src/hooks/usePackageConfig.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ export const usePackageConfig = (namespace: string, name: string) => {
2121
const marketplaceApi = useMarketplaceApi();
2222
return useQuery({
2323
queryKey: ['marketplaceApi', 'getPackageConfigByName', namespace, name],
24-
queryFn: () => marketplaceApi.getPackageConfigByName?.(namespace, name),
24+
queryFn: () => {
25+
if (namespace && name) {
26+
return marketplaceApi.getPackageConfigByName?.(namespace, name);
27+
}
28+
return null;
29+
},
2530
refetchOnWindowFocus: false,
2631
});
2732
};

‎workspaces/marketplace/plugins/marketplace/src/hooks/usePluginConfigurationPermissions.test.tsx‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
/*
2+
* Copyright The Backstage Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { renderHook, waitFor } from '@testing-library/react';
18+
import { useQuery } from '@tanstack/react-query';
19+
20+
import { usePluginConfigurationPermissions } from './usePluginConfigurationPermissions';
21+
22+
jest.mock('@backstage/core-plugin-api', () => ({
23+
...jest.requireActual('@backstage/core-plugin-api'),
24+
useApi: jest.fn(),
25+
}));
26+
27+
jest.mock('@tanstack/react-query', () => ({
28+
...jest.requireActual('@tanstack/react-query'),
29+
useQuery: jest.fn(),
30+
}));
31+
32+
describe('usePluginConfigurationPermissions', () => {
33+
beforeEach(() => {
34+
jest.clearAllMocks();
35+
});
36+
37+
it('returns read and write permissions when namespace and name are provided', async () => {
38+
(useQuery as jest.Mock).mockReturnValue({
39+
data: { read: 'ALLOW', write: 'ALLOW' },
40+
isLoading: false,
41+
error: undefined,
42+
});
43+
44+
const { result } = renderHook(() =>
45+
usePluginConfigurationPermissions('default', 'my-plugin'),
46+
);
47+
48+
await waitFor(() => expect(result.current.isLoading).toBe(false));
49+
expect(result.current.data).toEqual({ read: 'ALLOW', write: 'ALLOW' });
50+
expect(result.current.error).toBeUndefined();
51+
});
52+
53+
it('returns DENY permissions when namespace is empty', async () => {
54+
(useQuery as jest.Mock).mockReturnValue({
55+
data: { read: 'DENY', write: 'DENY' },
56+
isLoading: false,
57+
error: undefined,
58+
});
59+
60+
const { result } = renderHook(() =>
61+
usePluginConfigurationPermissions('', 'my-plugin'),
62+
);
63+
64+
await waitFor(() => expect(result.current.isLoading).toBe(false));
65+
expect(result.current.data).toEqual({ read: 'DENY', write: 'DENY' });
66+
});
67+
68+
it('returns DENY permissions when name is empty', async () => {
69+
(useQuery as jest.Mock).mockReturnValue({
70+
data: { read: 'DENY', write: 'DENY' },
71+
isLoading: false,
72+
error: undefined,
73+
});
74+
75+
const { result } = renderHook(() =>
76+
usePluginConfigurationPermissions('default', ''),
77+
);
78+
79+
await waitFor(() => expect(result.current.isLoading).toBe(false));
80+
expect(result.current.data).toEqual({ read: 'DENY', write: 'DENY' });
81+
});
82+
83+
it('returns read-only permissions', async () => {
84+
(useQuery as jest.Mock).mockReturnValue({
85+
data: { read: 'ALLOW', write: 'DENY' },
86+
isLoading: false,
87+
error: undefined,
88+
});
89+
90+
const { result } = renderHook(() =>
91+
usePluginConfigurationPermissions('default', 'my-plugin'),
92+
);
93+
94+
await waitFor(() => expect(result.current.isLoading).toBe(false));
95+
expect(result.current.data).toEqual({ read: 'ALLOW', write: 'DENY' });
96+
});
97+
98+
it('handles loading state', async () => {
99+
(useQuery as jest.Mock).mockReturnValue({
100+
data: undefined,
101+
isLoading: true,
102+
error: undefined,
103+
});
104+
105+
const { result } = renderHook(() =>
106+
usePluginConfigurationPermissions('default', 'my-plugin'),
107+
);
108+
109+
expect(result.current.isLoading).toBe(true);
110+
expect(result.current.data).toBeUndefined();
111+
});
112+
113+
it('handles error state', async () => {
114+
const error = new Error('Failed to fetch permissions');
115+
(useQuery as jest.Mock).mockReturnValue({
116+
data: undefined,
117+
isLoading: false,
118+
error,
119+
});
120+
121+
const { result } = renderHook(() =>
122+
usePluginConfigurationPermissions('default', 'my-plugin'),
123+
);
124+
125+
await waitFor(() => expect(result.current.isLoading).toBe(false));
126+
expect(result.current.error).toBeInstanceOf(Error);
127+
expect(result.current.error?.message).toBe('Failed to fetch permissions');
128+
});
129+
130+
it('uses correct query key with namespace and name', () => {
131+
(useQuery as jest.Mock).mockReturnValue({
132+
data: { read: 'ALLOW', write: 'ALLOW' },
133+
isLoading: false,
134+
error: undefined,
135+
});
136+
137+
renderHook(() =>
138+
usePluginConfigurationPermissions('my-namespace', 'my-plugin'),
139+
);
140+
141+
expect(useQuery).toHaveBeenCalledWith(
142+
expect.objectContaining({
143+
queryKey: [
144+
'marketplaceApi',
145+
'getPluginConfigAuthorization',
146+
'my-namespace',
147+
'my-plugin',
148+
],
149+
}),
150+
);
151+
});
152+
});

0 commit comments

Comments
 (0)