From b019d801d1ef91aac9abf860be6e6f09c31f78c1 Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Mon, 7 Jul 2025 09:41:59 +0200 Subject: [PATCH 1/3] Introduce package configuration permission checks Signed-off-by: Dominika Zemanovicova --- .../.changeset/olive-terms-learn.md | 6 + workspaces/marketplace/app-config.yaml | 4 + .../marketplace-backend/src/router.test.ts | 166 +++++++++++++++++- .../plugins/marketplace-backend/src/router.ts | 62 +++++-- .../plugins/marketplace-common/report.api.md | 21 +-- .../src/api/MarketplaceApi.ts | 5 + .../src/api/MarketplaceBackendClient.ts | 10 ++ .../src/api/MarketplaceCatalogClient.test.ts | 28 +++ .../src/api/MarketplaceCatalogClient.ts | 17 ++ .../marketplace-common/src/permissions.ts | 49 ------ 10 files changed, 286 insertions(+), 82 deletions(-) create mode 100644 workspaces/marketplace/.changeset/olive-terms-learn.md diff --git a/workspaces/marketplace/.changeset/olive-terms-learn.md b/workspaces/marketplace/.changeset/olive-terms-learn.md new file mode 100644 index 00000000000..ba14547fe48 --- /dev/null +++ b/workspaces/marketplace/.changeset/olive-terms-learn.md @@ -0,0 +1,6 @@ +--- +'@red-hat-developer-hub/backstage-plugin-marketplace-backend': minor +'@red-hat-developer-hub/backstage-plugin-marketplace-common': minor +--- + +Use plugin permissions in package configuration endpoints. `AuthorizeResult` for particular package is based upon if user has ALLOW permission for any of plugins that contain this package. Removes unused `extensions-package` permissions. diff --git a/workspaces/marketplace/app-config.yaml b/workspaces/marketplace/app-config.yaml index 77757540000..940c01296aa 100644 --- a/workspaces/marketplace/app-config.yaml +++ b/workspaces/marketplace/app-config.yaml @@ -124,6 +124,10 @@ kubernetes: permission: # setting this to `false` will disable permissions enabled: true + rbac: + pluginsWithPermission: + - catalog + - extensions extensions: ### Example for how to enable installation to a file. diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts index a51d8077666..070a31eebc5 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts @@ -23,6 +23,10 @@ import { ExtendedHttpServer } from '@backstage/backend-defaults/rootHttpRouter'; import { BackendFeature } from '@backstage/backend-plugin-api'; import { mockServices, startTestBackend } from '@backstage/backend-test-utils'; import type { JsonObject } from '@backstage/types'; +import { + AuthorizeResult, + type QueryPermissionResponse, +} from '@backstage/plugin-permission-common'; import { MarketplaceCollection, MarketplaceKind, @@ -75,14 +79,35 @@ const PLUGIN_SETUP = { }; const PACKAGE_SETUP = { - mockData: mockPackages, + mockData: [...mockPackages, ...mockPlugins], name: 'package11', kind: MarketplaceKind.Package, config: FILE_INSTALL_CONFIG, + relationName: 'plugin1', }; +const mockedAuthorizeConditional = async (): Promise< + QueryPermissionResponse[] +> => [ + { + result: AuthorizeResult.CONDITIONAL as const, + pluginId: 'extensions', + resourceType: 'extensions-plugin', + conditions: { + anyOf: [ + { + rule: 'HAS_NAME', + resourceType: 'extensions-plugin', + params: { pluginNames: ['other-plugin'] }, + }, + ], + }, + }, +]; + async function startBackendServer( config?: JsonObject, + authorizeResult?: AuthorizeResult, ): Promise { const features: (BackendFeature | Promise<{ default: BackendFeature }>)[] = [ marketplacePlugin, @@ -92,6 +117,24 @@ async function startBackendServer( }), ]; + if ( + authorizeResult === AuthorizeResult.ALLOW || + authorizeResult === AuthorizeResult.DENY + ) { + features.push( + mockServices.permissions.mock({ + authorizeConditional: async () => [{ result: authorizeResult }], + }).factory, + ); + } + if (authorizeResult === AuthorizeResult.CONDITIONAL) { + features.push( + mockServices.permissions.mock({ + authorizeConditional: mockedAuthorizeConditional, + }).factory, + ); + } + return (await startTestBackend({ features })).server; } @@ -117,6 +160,19 @@ const expectInputError = async ( }); }; +const expectPermissionError = async ( + response: request.Response, + action: string, + namespace: string, + name: string, +) => { + expect(response.status).toEqual(403); + expect(response.body.error).toEqual({ + message: `Not allowed to ${action} the configuration of ${namespace}:${name}`, + name: 'NotAllowedError', + }); +}; + const setupTest = () => { let server: SetupServer; @@ -163,16 +219,21 @@ describe('createRouter', () => { name, kind = MarketplaceKind.Plugin, config, + authorizeResult, }: { mockData: MockMarketplaceEntity[] | {}; name?: string; kind?: string; config?: JsonObject; + authorizeResult?: AuthorizeResult; }): Promise<{ backendServer: ExtendedHttpServer; }> => { const { server } = testSetup(); - const backendServer: ExtendedHttpServer = await startBackendServer(config); + const backendServer: ExtendedHttpServer = await startBackendServer( + config, + authorizeResult, + ); server.use( rest.get( `http://localhost:${backendServer.port()}/api/catalog/entities/by-query`, @@ -203,6 +264,20 @@ describe('createRouter', () => { ); }, ), + rest.get( + `http://localhost:${backendServer.port()}/api/catalog/entities`, + (_, res, ctx) => { + if (!Array.isArray(mockData)) { + throw new Error('Internal server error'); + } + const hasPackage = (r: { type: string; targetRef: string }) => + r.type === 'hasPart' && r.targetRef === `package:${name}`; + const foundEntities = mockData.filter( + e => e.kind === 'Plugin' && e.relations.some(hasPackage), + ); + return res(ctx.json(foundEntities)); + }, + ), ); return { backendServer }; @@ -560,7 +635,7 @@ describe('createRouter', () => { it('should get the package configuration', async () => { const { backendServer } = await setupTestWithMockCatalog({ - mockData: mockPackages, + mockData: [...mockPackages, ...mockPlugins], name: 'package11', kind: MarketplaceKind.Package, config: FILE_INSTALL_CONFIG, @@ -685,4 +760,89 @@ describe('createRouter', () => { expect(response.body).toEqual({ status: 'OK' }); }); }); + + describe('Denial when missing permissions', () => { + const permissionTestCases = [ + { + description: 'GET /package/:namespace/:name/configuration', + reqBuilder: (req: request.SuperTest) => + req.get('/api/extensions/package/default/package11/configuration'), + body: undefined, + }, + { + description: 'POST /package/:namespace/:name/configuration', + reqBuilder: (req: request.SuperTest) => + req.post('/api/extensions/package/default/package11/configuration'), + body: { configYaml: stringify(mockDynamicPackage11) }, + }, + { + description: 'POST /package/:namespace/:name/configuration/disable', + reqBuilder: (req: request.SuperTest) => + req.post( + '/api/extensions/package/default/package11/configuration/disable', + ), + body: { disabled: true }, + }, + { + description: 'GET /plugin/:namespace/:name/configuration', + reqBuilder: (req: request.SuperTest) => + req.get('/api/extensions/plugin/default/plugin1/configuration'), + body: undefined, + }, + { + description: 'POST /plugin/:namespace/:name/configuration', + reqBuilder: (req: request.SuperTest) => + req.post('/api/extensions/package/default/plugin1/configuration'), + body: { configYaml: stringify(mockDynamicPlugin1) }, + }, + { + description: 'PATCH /plugin/:namespace/:name/configuration/disable', + reqBuilder: (req: request.SuperTest) => + req.patch( + '/api/extensions/plugin/default/plugin1/configuration/disable', + ), + body: { disabled: true }, + }, + ]; + + const authorizeResults = [ + { result: AuthorizeResult.DENY, denyAction: 'outright denied' }, + { + result: AuthorizeResult.CONDITIONAL, + denyAction: 'conditionally denied', + }, + ]; + + const allTestCases = authorizeResults.flatMap(({ result, denyAction }) => + permissionTestCases.map(testCase => ({ + ...testCase, + denyAction, + result, + })), + ); + + it.each(allTestCases)( + '$description: returns 403 when $denyAction by permission framework', + async ({ description, reqBuilder, body, result }) => { + const isPackage = description.includes('/package'); + const name = isPackage ? 'package11' : 'plugin1'; + const { backendServer } = await setupTestWithMockCatalog({ + mockData: [...mockPackages, ...mockPlugins], + name, + kind: isPackage ? MarketplaceKind.Package : MarketplaceKind.Plugin, + config: FILE_INSTALL_CONFIG, + authorizeResult: result, + }); + + const requestBuilder = reqBuilder(request(backendServer)); + const response = body + ? await requestBuilder.send(body) + : await requestBuilder; + + expect(response.status).toEqual(403); + const action = description.includes('GET') ? 'read' : 'write'; + expectPermissionError(response, action, 'default', name); + }, + ); + }); }); diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.ts index 2ff549f3883..82676583ebd 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.ts @@ -90,9 +90,7 @@ export async function createRouter( const authorizeConditional = async ( request: Request, - permission: - | ResourcePermission<'extensions-plugin' | 'extensions-package'> - | BasicPermission, + permission: ResourcePermission<'extensions-plugin'> | BasicPermission, ) => { const credentials = await httpAuth.credentials(request); let decision: PolicyDecision; @@ -120,9 +118,7 @@ export async function createRouter( const getAuthorizedPlugin = async ( request: Request, - permission: - | ResourcePermission<'extensions-plugin' | 'extensions-package'> - | BasicPermission, + permission: ResourcePermission<'extensions-plugin'> | BasicPermission, ) => { const decision = await authorizeConditional(request, permission); const action = @@ -154,6 +150,42 @@ export async function createRouter( return plugin; }; + const getAuthorizedPackage = async ( + request: Request, + permission: ResourcePermission<'extensions-plugin'> | BasicPermission, + ) => { + const decision = await authorizeConditional(request, permission); + const action = + permission.attributes.action === 'create' + ? 'write' + : permission.attributes.action; + + if (decision.result === AuthorizeResult.DENY) { + throw new NotAllowedError( + `Not allowed to ${action} the configuration of ${request.params.namespace}:${request.params.name}`, + ); + } + + const packagePlugins = await marketplaceApi.getPackagePlugins( + request.params.namespace, + request.params.name, + ); + const hasAccess = + decision.result === AuthorizeResult.ALLOW || + (decision.result === AuthorizeResult.CONDITIONAL && + packagePlugins.some(plugin => matches(plugin, decision.conditions))); + if (!hasAccess) { + throw new NotAllowedError( + `Not allowed to ${action} the configuration of ${request.params.namespace}:${request.params.name}`, + ); + } + + return await marketplaceApi.getPackageByName( + request.params.namespace, + request.params.name, + ); + }; + router.get('/collections', async (req, res) => { const request = decodeGetEntitiesRequest(createSearchParams(req)); const collections = await marketplaceApi.getCollections(request); @@ -209,9 +241,9 @@ export async function createRouter( '/package/:namespace/:name/configuration', requireInitializedInstallationDataService, async (req, res) => { - const marketplacePackage = await marketplaceApi.getPackageByName( - req.params.namespace, - req.params.name, + const marketplacePackage = await getAuthorizedPackage( + req, + extensionsPluginReadPermission, ); if (!marketplacePackage.spec?.dynamicArtifact) { @@ -230,9 +262,9 @@ export async function createRouter( '/package/:namespace/:name/configuration', requireInitializedInstallationDataService, async (req, res) => { - const marketplacePackage = await marketplaceApi.getPackageByName( - req.params.namespace, - req.params.name, + const marketplacePackage = await getAuthorizedPackage( + req, + extensionsPluginWritePermission, ); if (!marketplacePackage.spec?.dynamicArtifact) { throw new Error( @@ -269,9 +301,9 @@ export async function createRouter( '/package/:namespace/:name/configuration/disable', requireInitializedInstallationDataService, async (req, res) => { - const marketplacePackage = await marketplaceApi.getPackageByName( - req.params.namespace, - req.params.name, + const marketplacePackage = await getAuthorizedPackage( + req, + extensionsPluginWritePermission, ); if (!marketplacePackage.spec?.dynamicArtifact) { diff --git a/workspaces/marketplace/plugins/marketplace-common/report.api.md b/workspaces/marketplace/plugins/marketplace-common/report.api.md index 18328e95182..b6e6fe095dc 100644 --- a/workspaces/marketplace/plugins/marketplace-common/report.api.md +++ b/workspaces/marketplace/plugins/marketplace-common/report.api.md @@ -83,18 +83,6 @@ export const encodeGetEntityFacetsRequest: (request: GetEntityFacetsRequest) => // @public (undocumented) export const EXTENSIONS_API_VERSION = "extensions.backstage.io/v1alpha1"; -// @public -export const extensionsPackageDeletePermission: ResourcePermission<"extensions-package">; - -// @public -export type ExtensionsPackagePermission = ResourcePermission; - -// @public -export const extensionsPackageReadPermission: ResourcePermission<"extensions-package">; - -// @public -export const extensionsPackageWritePermission: ResourcePermission<"extensions-package">; - // @public (undocumented) export const extensionsPermissions: ResourcePermission<"extensions-plugin">[]; @@ -190,6 +178,8 @@ export interface MarketplaceApi { // (undocumented) getPackageConfigByName?(namespace: string, name: string): Promise; // (undocumented) + getPackagePlugins(namespace: string, name: string): Promise; + // (undocumented) getPackages(request: GetEntitiesRequest): Promise>; // (undocumented) getPackagesFacets(request: GetEntityFacetsRequest): Promise; @@ -256,6 +246,8 @@ export class MarketplaceBackendClient implements MarketplaceApi { // (undocumented) getPackageConfigByName(namespace: string, name: string): Promise; // (undocumented) + getPackagePlugins(namespace: string, name: string): Promise; + // (undocumented) getPackages(request: GetEntitiesRequest): Promise>; // (undocumented) getPackagesFacets(request: GetEntityFacetsRequest): Promise; @@ -306,6 +298,8 @@ export class MarketplaceCatalogClient implements MarketplaceApi { // (undocumented) getPackageByName(namespace: string, name: string): Promise; // (undocumented) + getPackagePlugins(namespace: string, name: string): Promise; + // (undocumented) getPackages(request: GetEntitiesRequest): Promise>; // (undocumented) getPackagesFacets(request: GetEntityFacetsRequest): Promise; @@ -464,9 +458,6 @@ export interface MarketplacePluginSpec extends JsonObject { // @public (undocumented) export type NodeEnvironmentType = 'production' | 'development' | 'test'; -// @public (undocumented) -export const RESOURCE_TYPE_EXTENSIONS_PACKAGE = "extensions-package"; - // @public (undocumented) export const RESOURCE_TYPE_EXTENSIONS_PLUGIN = "extensions-plugin"; diff --git a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceApi.ts b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceApi.ts index 6345c0f0c3d..bb344d80b74 100644 --- a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceApi.ts +++ b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceApi.ts @@ -137,4 +137,9 @@ export interface MarketplaceApi { namespace: string, name: string, ): Promise; + + getPackagePlugins( + namespace: string, + name: string, + ): Promise; } diff --git a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceBackendClient.ts b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceBackendClient.ts index 325a808321e..a9c1318ef70 100644 --- a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceBackendClient.ts +++ b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceBackendClient.ts @@ -303,4 +303,14 @@ export class MarketplaceBackendClient implements MarketplaceApi { 'GET', ); } + + getPackagePlugins( + namespace: string, + name: string, + ): Promise { + return this.request( + `/package/${encodeURIComponent(namespace)}/${encodeURIComponent(name)}/plugins`, + 'GET', + ); + } } diff --git a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.test.ts b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.test.ts index 4a4a4430c1a..18655de990b 100644 --- a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.test.ts +++ b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.test.ts @@ -82,12 +82,14 @@ const mockQueryEntities = jest.fn(); const mockEntityFacets = jest.fn(); const mockGetEntityByRef = jest.fn(); const mockQueryEntitiesByRefs = jest.fn(); +const mockGetEntities = jest.fn(); const mockCatalogClient = { queryEntities: mockQueryEntities, getEntityFacets: mockEntityFacets, getEntityByRef: mockGetEntityByRef, getEntitiesByRefs: mockQueryEntitiesByRefs, + getEntities: mockGetEntities, } as unknown as CatalogClient; beforeEach(() => { @@ -563,4 +565,30 @@ describe('MarketplaceCatalogClient', () => { ).rejects.toThrow('Plugin default/not-found not found'); }); }); + + describe('getPackagePlugins', () => { + beforeEach(() => { + mockGetEntities.mockResolvedValue({ items: [mockPlugins[0]] }); + }); + + it('should call catalog with right kind filter', async () => { + const api = new MarketplaceCatalogClient(options); + const plugins = await api.getPackagePlugins('default', 'package1'); + + expect(mockGetEntities).toHaveBeenCalledTimes(1); + expect(mockGetEntities).toHaveBeenCalledWith( + { + filter: { + kind: 'Plugin', + 'relations.hasPart': 'package:default/package1', + }, + }, + { + token: 'mockedToken', + }, + ); + + expect(plugins).toEqual([mockPlugins[0]]); + }); + }); }); diff --git a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.ts b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.ts index 487711d9494..db0e9891685 100644 --- a/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.ts +++ b/workspaces/marketplace/plugins/marketplace-common/src/api/MarketplaceCatalogClient.ts @@ -245,4 +245,21 @@ export class MarketplaceCatalogClient implements MarketplaceApi { // Double check that we return (only) the right Packages return result.items.filter(isMarketplacePackage); } + + async getPackagePlugins( + namespace: string, + name: string, + ): Promise { + const entityRef = stringifyEntityRef({ + kind: MarketplaceKind.Package, + namespace, + name, + }); + const token = await this.getServiceToken(); + const result = await this.catalog.getEntities( + { filter: { kind: 'Plugin', 'relations.hasPart': entityRef } }, + token, + ); + return result.items; + } } diff --git a/workspaces/marketplace/plugins/marketplace-common/src/permissions.ts b/workspaces/marketplace/plugins/marketplace-common/src/permissions.ts index 3dca690cab0..61918a51301 100644 --- a/workspaces/marketplace/plugins/marketplace-common/src/permissions.ts +++ b/workspaces/marketplace/plugins/marketplace-common/src/permissions.ts @@ -24,11 +24,6 @@ import { */ export const RESOURCE_TYPE_EXTENSIONS_PLUGIN = 'extensions-plugin'; -/** - * @public - */ -export const RESOURCE_TYPE_EXTENSIONS_PACKAGE = 'extensions-package'; - /** * @public * Convenience type for extensions plugin @@ -37,14 +32,6 @@ export type ExtensionsPluginPermission = ResourcePermission< typeof RESOURCE_TYPE_EXTENSIONS_PLUGIN >; -/** - * @public - * Convenience type for extensions package - */ -export type ExtensionsPackagePermission = ResourcePermission< - typeof RESOURCE_TYPE_EXTENSIONS_PACKAGE ->; - /** This permission grants access to the endpoint that reads the configuration of the extensions plugin * @public */ @@ -78,39 +65,6 @@ export const extensionsPluginDeletePermission = createPermission({ resourceType: RESOURCE_TYPE_EXTENSIONS_PLUGIN, }); -/** This permission grants access to the endpoint that reads the configuration of the extensions package - * @public - */ -export const extensionsPackageReadPermission = createPermission({ - name: 'extensions.package.configuration.read', - attributes: { - action: 'read', - }, - resourceType: RESOURCE_TYPE_EXTENSIONS_PACKAGE, -}); - -/** This permission grants access to the endpoint that installs or updates the extensions package - * @public - */ -export const extensionsPackageWritePermission = createPermission({ - name: 'extensions.package.configuration.write', - attributes: { - action: 'create', - }, - resourceType: RESOURCE_TYPE_EXTENSIONS_PACKAGE, -}); - -/** This permission grants access to the endpoint that disables the extensions package - * @public - */ -export const extensionsPackageDeletePermission = createPermission({ - name: 'extensions.package.configuration.delete', - attributes: { - action: 'delete', - }, - resourceType: RESOURCE_TYPE_EXTENSIONS_PACKAGE, -}); - /** * @public */ @@ -118,7 +72,4 @@ export const extensionsPermissions = [ extensionsPluginWritePermission, extensionsPluginReadPermission, // extensionsPluginDeletePermission, - // extensionsPackageReadPermission, - // extensionsPackageDeletePermission, - // extensionsPackageWritePermission, ]; From 56e34ca82350a6e84dc5d1d35e840c48c67b9923 Mon Sep 17 00:00:00 2001 From: Oleksandr Andriienko Date: Wed, 9 Jul 2025 17:16:18 +0300 Subject: [PATCH 2/3] feat(extensions): simplify router.test.ts (#1) Signed-off-by: Oleksandr Andriienko --- .../marketplace-backend/src/router.test.ts | 87 +++++++++---------- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts index 070a31eebc5..4946426f8b8 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts @@ -25,7 +25,7 @@ import { mockServices, startTestBackend } from '@backstage/backend-test-utils'; import type { JsonObject } from '@backstage/types'; import { AuthorizeResult, - type QueryPermissionResponse, + PolicyDecision, } from '@backstage/plugin-permission-common'; import { MarketplaceCollection, @@ -86,28 +86,9 @@ const PACKAGE_SETUP = { relationName: 'plugin1', }; -const mockedAuthorizeConditional = async (): Promise< - QueryPermissionResponse[] -> => [ - { - result: AuthorizeResult.CONDITIONAL as const, - pluginId: 'extensions', - resourceType: 'extensions-plugin', - conditions: { - anyOf: [ - { - rule: 'HAS_NAME', - resourceType: 'extensions-plugin', - params: { pluginNames: ['other-plugin'] }, - }, - ], - }, - }, -]; - async function startBackendServer( config?: JsonObject, - authorizeResult?: AuthorizeResult, + authorizeResult?: PolicyDecision, ): Promise { const features: (BackendFeature | Promise<{ default: BackendFeature }>)[] = [ marketplacePlugin, @@ -117,20 +98,10 @@ async function startBackendServer( }), ]; - if ( - authorizeResult === AuthorizeResult.ALLOW || - authorizeResult === AuthorizeResult.DENY - ) { + if (authorizeResult) { features.push( mockServices.permissions.mock({ - authorizeConditional: async () => [{ result: authorizeResult }], - }).factory, - ); - } - if (authorizeResult === AuthorizeResult.CONDITIONAL) { - features.push( - mockServices.permissions.mock({ - authorizeConditional: mockedAuthorizeConditional, + authorizeConditional: async () => [authorizeResult], }).factory, ); } @@ -219,20 +190,20 @@ describe('createRouter', () => { name, kind = MarketplaceKind.Plugin, config, - authorizeResult, + policyDecision, }: { mockData: MockMarketplaceEntity[] | {}; name?: string; kind?: string; config?: JsonObject; - authorizeResult?: AuthorizeResult; + policyDecision?: PolicyDecision; }): Promise<{ backendServer: ExtendedHttpServer; }> => { const { server } = testSetup(); const backendServer: ExtendedHttpServer = await startBackendServer( config, - authorizeResult, + policyDecision, ); server.use( rest.get( @@ -805,25 +776,47 @@ describe('createRouter', () => { }, ]; - const authorizeResults = [ - { result: AuthorizeResult.DENY, denyAction: 'outright denied' }, + const policyDecisions: { + policyDecision: PolicyDecision; + denyAction: string; + }[] = [ { - result: AuthorizeResult.CONDITIONAL, + policyDecision: { + result: AuthorizeResult.DENY, + }, + denyAction: 'outright denied', + }, + { + policyDecision: { + result: AuthorizeResult.CONDITIONAL, + pluginId: 'extensions', + resourceType: 'extensions-plugin', + conditions: { + anyOf: [ + { + rule: 'HAS_NAME', + resourceType: 'extensions-plugin', + params: { pluginNames: ['other-plugin'] }, + }, + ], + }, + }, denyAction: 'conditionally denied', }, ]; - const allTestCases = authorizeResults.flatMap(({ result, denyAction }) => - permissionTestCases.map(testCase => ({ - ...testCase, - denyAction, - result, - })), + const allTestCases = policyDecisions.flatMap( + ({ policyDecision, denyAction }) => + permissionTestCases.map(testCase => ({ + ...testCase, + denyAction, + policyDecision, + })), ); it.each(allTestCases)( '$description: returns 403 when $denyAction by permission framework', - async ({ description, reqBuilder, body, result }) => { + async ({ description, reqBuilder, body, policyDecision }) => { const isPackage = description.includes('/package'); const name = isPackage ? 'package11' : 'plugin1'; const { backendServer } = await setupTestWithMockCatalog({ @@ -831,7 +824,7 @@ describe('createRouter', () => { name, kind: isPackage ? MarketplaceKind.Package : MarketplaceKind.Plugin, config: FILE_INSTALL_CONFIG, - authorizeResult: result, + policyDecision, }); const requestBuilder = reqBuilder(request(backendServer)); From 944cd2288c7fa31c1a12160fd3b6406795b9e0b9 Mon Sep 17 00:00:00 2001 From: Dominika Zemanovicova Date: Wed, 9 Jul 2025 16:18:20 +0200 Subject: [PATCH 3/3] Fix action name Co-authored-by: Oleksandr Andriienko Signed-off-by: Dominika Zemanovicova --- .../marketplace-backend/src/router.test.ts | 2 +- .../plugins/marketplace-backend/src/router.ts | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts index 4946426f8b8..5a686cb49b5 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.test.ts @@ -833,7 +833,7 @@ describe('createRouter', () => { : await requestBuilder; expect(response.status).toEqual(403); - const action = description.includes('GET') ? 'read' : 'write'; + const action = description.includes('GET') ? 'read' : 'create'; expectPermissionError(response, action, 'default', name); }, ); diff --git a/workspaces/marketplace/plugins/marketplace-backend/src/router.ts b/workspaces/marketplace/plugins/marketplace-backend/src/router.ts index 82676583ebd..d5118de128e 100644 --- a/workspaces/marketplace/plugins/marketplace-backend/src/router.ts +++ b/workspaces/marketplace/plugins/marketplace-backend/src/router.ts @@ -121,14 +121,10 @@ export async function createRouter( permission: ResourcePermission<'extensions-plugin'> | BasicPermission, ) => { const decision = await authorizeConditional(request, permission); - const action = - permission.attributes.action === 'create' - ? 'write' - : permission.attributes.action; if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError( - `Not allowed to ${action} the configuration of ${request.params.namespace}:${request.params.name}`, + `Not allowed to ${permission.attributes.action} the configuration of ${request.params.namespace}:${request.params.name}`, ); } @@ -143,7 +139,7 @@ export async function createRouter( matches(plugin, decision.conditions)); if (!hasAccess) { throw new NotAllowedError( - `Not allowed to ${action} the configuration of ${request.params.namespace}:${request.params.name}`, + `Not allowed to ${permission.attributes.action} the configuration of ${request.params.namespace}:${request.params.name}`, ); } @@ -155,14 +151,10 @@ export async function createRouter( permission: ResourcePermission<'extensions-plugin'> | BasicPermission, ) => { const decision = await authorizeConditional(request, permission); - const action = - permission.attributes.action === 'create' - ? 'write' - : permission.attributes.action; if (decision.result === AuthorizeResult.DENY) { throw new NotAllowedError( - `Not allowed to ${action} the configuration of ${request.params.namespace}:${request.params.name}`, + `Not allowed to ${permission.attributes.action} the configuration of ${request.params.namespace}:${request.params.name}`, ); } @@ -176,7 +168,7 @@ export async function createRouter( packagePlugins.some(plugin => matches(plugin, decision.conditions))); if (!hasAccess) { throw new NotAllowedError( - `Not allowed to ${action} the configuration of ${request.params.namespace}:${request.params.name}`, + `Not allowed to ${permission.attributes.action} the configuration of ${request.params.namespace}:${request.params.name}`, ); }