feat(extensions): add permission checks for package configuration endpoints

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.

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.

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,
+  PolicyDecision,
+} from '@backstage/plugin-permission-common';
 import {
   MarketplaceCollection,
   MarketplaceKind,
@@ -75,14 +79,16 @@ const PLUGIN_SETUP = {
 };
 
 const PACKAGE_SETUP = {
-  mockData: mockPackages,
+  mockData: [...mockPackages, ...mockPlugins],
   name: 'package11',
   kind: MarketplaceKind.Package,
   config: FILE_INSTALL_CONFIG,
+  relationName: 'plugin1',
 };
 
 async function startBackendServer(
   config?: JsonObject,
+  authorizeResult?: PolicyDecision,
 ): Promise<ExtendedHttpServer> {
   const features: (BackendFeature | Promise<{ default: BackendFeature }>)[] = [
     marketplacePlugin,
@@ -92,6 +98,14 @@ async function startBackendServer(
     }),
   ];
 
+  if (authorizeResult) {
+    features.push(
+      mockServices.permissions.mock({
+        authorizeConditional: async () => [authorizeResult],
+      }).factory,
+    );
+  }
+
   return (await startTestBackend({ features })).server;
 }
 
@@ -117,6 +131,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 +190,21 @@ describe('createRouter', () => {
     name,
     kind = MarketplaceKind.Plugin,
     config,
+    policyDecision,
   }: {
     mockData: MockMarketplaceEntity[] | {};
     name?: string;
     kind?: string;
     config?: JsonObject;
+    policyDecision?: PolicyDecision;
   }): Promise<{
     backendServer: ExtendedHttpServer;
   }> => {
     const { server } = testSetup();
-    const backendServer: ExtendedHttpServer = await startBackendServer(config);
+    const backendServer: ExtendedHttpServer = await startBackendServer(
+      config,
+      policyDecision,
+    );
     server.use(
       rest.get(
         `http://localhost:${backendServer.port()}/api/catalog/entities/by-query`,
@@ -203,6 +235,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 +606,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 +731,111 @@ 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<request.Test>) =>
+          req.get('/api/extensions/package/default/package11/configuration'),
+        body: undefined,
+      },
+      {
+        description: 'POST /package/:namespace/:name/configuration',
+        reqBuilder: (req: request.SuperTest<request.Test>) =>
+          req.post('/api/extensions/package/default/package11/configuration'),
+        body: { configYaml: stringify(mockDynamicPackage11) },
+      },
+      {
+        description: 'POST /package/:namespace/:name/configuration/disable',
+        reqBuilder: (req: request.SuperTest<request.Test>) =>
+          req.post(
+            '/api/extensions/package/default/package11/configuration/disable',
+          ),
+        body: { disabled: true },
+      },
+      {
+        description: 'GET /plugin/:namespace/:name/configuration',
+        reqBuilder: (req: request.SuperTest<request.Test>) =>
+          req.get('/api/extensions/plugin/default/plugin1/configuration'),
+        body: undefined,
+      },
+      {
+        description: 'POST /plugin/:namespace/:name/configuration',
+        reqBuilder: (req: request.SuperTest<request.Test>) =>
+          req.post('/api/extensions/package/default/plugin1/configuration'),
+        body: { configYaml: stringify(mockDynamicPlugin1) },
+      },
+      {
+        description: 'PATCH /plugin/:namespace/:name/configuration/disable',
+        reqBuilder: (req: request.SuperTest<request.Test>) =>
+          req.patch(
+            '/api/extensions/plugin/default/plugin1/configuration/disable',
+          ),
+        body: { disabled: true },
+      },
+    ];
+
+    const policyDecisions: {
+      policyDecision: PolicyDecision;
+      denyAction: string;
+    }[] = [
+      {
+        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 = 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, policyDecision }) => {
+        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,
+          policyDecision,
+        });
+
+        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' : 'create';
+        expectPermissionError(response, action, 'default', name);
+      },
+    );
+  });
 });

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,19 +118,13 @@ 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 =
-      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}`,
       );
     }
 
@@ -147,13 +139,45 @@ 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}`,
       );
     }
 
     return plugin;
   };
 
+  const getAuthorizedPackage = async (
+    request: Request,
+    permission: ResourcePermission<'extensions-plugin'> | BasicPermission,
+  ) => {
+    const decision = await authorizeConditional(request, permission);
+
+    if (decision.result === AuthorizeResult.DENY) {
+      throw new NotAllowedError(
+        `Not allowed to ${permission.attributes.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 ${permission.attributes.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 +233,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 +254,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 +293,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) {