Merge pull request #17077 from backstage/permission-router-options-array

createPermissionIntegrationRouter: accept an array of options
2023-04-25 10:37:18 +02:00
parent 1d8230e65c 7996d8900b
commit 16685cf8a2
4 changed files with 605 additions and 71 deletions
@@ -113,20 +113,31 @@ export const createConditionTransformer: <

 // @public
 export function createPermissionIntegrationRouter<
-  TResourceType extends string,
-  TResource,
+  TResourceType1 extends string,
+  TResource1,
+  TResourceType2 extends string,
+  TResource2,
+  TResourceType3 extends string,
+  TResource3,
 >(
-  options: CreatePermissionIntegrationRouterResourceOptions<
-    TResourceType,
-    TResource
-  >,
+  options:
+    | {
+        permissions: Array<Permission>;
+      }
+    | CreatePermissionIntegrationRouterResourceOptions<
+        TResourceType1,
+        TResource1
+      >
+    | PermissionIntegrationRouterOptions<
+        TResourceType1,
+        TResource1,
+        TResourceType2,
+        TResource2,
+        TResourceType3,
+        TResource3
+      >,
 ): express.Router;

-// @public
-export function createPermissionIntegrationRouter(options: {
-  permissions: Array<Permission>;
-}): express.Router;
-
 // @public
 export type CreatePermissionIntegrationRouterResourceOptions<
  TResourceType extends string,
@@ -188,6 +199,49 @@ export type MetadataResponseSerializedRule = {
  paramsSchema?: ReturnType<typeof zodToJsonSchema>;
 };

+// @public
+export type PermissionIntegrationRouterOptions<
+  TResourceType1 extends string = string,
+  TResource1 = any,
+  TResourceType2 extends string = string,
+  TResource2 = any,
+  TResourceType3 extends string = string,
+  TResource3 = any,
+> = {
+  resources: Readonly<
+    | [
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >,
+      ]
+    | [
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType2,
+          TResource2
+        >,
+      ]
+    | [
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType2,
+          TResource2
+        >,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType3,
+          TResource3
+        >,
+      ]
+  >;
+};
+
 // @public
 export interface PermissionPolicy {
  // (undocumented)
@@ -26,6 +26,7 @@ import {
  createPermissionIntegrationRouter,
  CreatePermissionIntegrationRouterResourceOptions,
  createConditionAuthorizer,
+  PermissionIntegrationRouterOptions,
 } from './createPermissionIntegrationRouter';
 import { createPermissionRule } from './createPermissionRule';

@@ -34,6 +35,11 @@ const testPermission: Permission = createPermission({
  attributes: {},
 });

+const testPermission2: Permission = createPermission({
+  name: 'test.permission2',
+  attributes: {},
+});
+
 const mockTestRule1Apply = jest
  .fn()
  .mockImplementation((_resource: any, _params) => true);
@@ -60,17 +66,52 @@ const testRule2 = createPermissionRule({
  toQuery: () => ({}),
 });

-const defaultMockedGetResources: CreatePermissionIntegrationRouterResourceOptions<
+const mockTestRule3Apply = jest
+  .fn()
+  .mockImplementation((_resource: any) => false);
+const testRule3 = createPermissionRule({
+  // simulating a clash of name with test-rule-1 rule of test-resource
+  name: 'test-rule-1',
+  description: 'Test rule 3',
+  resourceType: 'test-resource-2',
+  apply: mockTestRule3Apply,
+  toQuery: () => ({}),
+});
+
+const defaultMockedGetResources1: CreatePermissionIntegrationRouterResourceOptions<
  string,
  { id: string }
 >['getResources'] = jest.fn(async resourceRefs =>
  resourceRefs.map(resourceRef => ({ id: resourceRef })),
 );

+const defaultMockedGetResources2: CreatePermissionIntegrationRouterResourceOptions<
+  string,
+  { id: string }
+>['getResources'] = jest.fn(async resourceRefs =>
+  resourceRefs.map(resourceRef => ({ id: resourceRef })),
+);
+
+const mockedOptionResources: PermissionIntegrationRouterOptions = {
+  resources: [
+    {
+      resourceType: 'test-resource',
+      permissions: [testPermission],
+      getResources: defaultMockedGetResources1,
+      rules: [testRule1, testRule2],
+    },
+    {
+      resourceType: 'test-resource-2',
+      permissions: [testPermission2],
+      getResources: defaultMockedGetResources2,
+      rules: [testRule3],
+    },
+  ],
+};
+
 const createApp = (
  mockedGetResources:
-    | typeof defaultMockedGetResources
-    | null = defaultMockedGetResources,
+    | typeof defaultMockedGetResources1 = defaultMockedGetResources1,
 ) => {
  const router = mockedGetResources
    ? createPermissionIntegrationRouter({
@@ -160,7 +201,7 @@ describe('createPermissionIntegrationRouter', () => {
        ],
      },
    ])('returns 200/ALLOW when criteria match (case %#)', async conditions => {
-      const response = await request(createApp())
+      let response = await request(createApp())
        .post('/.well-known/backstage/permissions/apply-conditions')
        .send({
          items: [
@@ -182,6 +223,41 @@ describe('createPermissionIntegrationRouter', () => {
          },
        ],
      });
+
+      expect(defaultMockedGetResources1).toHaveBeenCalled();
+      expect(mockTestRule3Apply).not.toHaveBeenCalled();
+
+      (defaultMockedGetResources1 as jest.Mock).mockClear();
+
+      const app = express().use(
+        createPermissionIntegrationRouter(mockedOptionResources),
+      );
+
+      response = await request(app)
+        .post('/.well-known/backstage/permissions/apply-conditions')
+        .send({
+          items: [
+            {
+              id: '123',
+              resourceRef: 'default:test/resource',
+              resourceType: 'test-resource',
+              conditions,
+            },
+          ],
+        });
+
+      expect(response.status).toEqual(200);
+      expect(response.body).toEqual({
+        items: [
+          {
+            id: '123',
+            result: AuthorizeResult.ALLOW,
+          },
+        ],
+      });
+      expect(defaultMockedGetResources1).toHaveBeenCalled();
+      expect(defaultMockedGetResources2).not.toHaveBeenCalled();
+      expect(mockTestRule3Apply).not.toHaveBeenCalled();
    });

    it.each([
@@ -363,7 +439,7 @@ describe('createPermissionIntegrationRouter', () => {
      });

      it('calls getResources for all required resources at once', () => {
-        expect(defaultMockedGetResources).toHaveBeenCalledWith([
+        expect(defaultMockedGetResources1).toHaveBeenCalledWith([
          'default:test/resource-1',
          'default:test/resource-2',
          'default:test/resource-3',
@@ -372,6 +448,116 @@ describe('createPermissionIntegrationRouter', () => {
      });
    });

+    describe('batched requests with different resource types', () => {
+      let response: Response;
+
+      beforeEach(async () => {
+        const app = express().use(
+          createPermissionIntegrationRouter(mockedOptionResources),
+        );
+
+        response = await request(app)
+          .post('/.well-known/backstage/permissions/apply-conditions')
+          .send({
+            items: [
+              {
+                id: '123',
+                resourceRef: 'default:test/resource-1',
+                resourceType: 'test-resource',
+                conditions: {
+                  rule: 'test-rule-1',
+                  resourceType: 'test-resource',
+                  params: {
+                    foo: 'a',
+                    bar: 1,
+                  },
+                },
+              },
+              {
+                id: '234',
+                resourceRef: 'default:test/resource-1',
+                resourceType: 'test-resource',
+                conditions: {
+                  rule: 'test-rule-2',
+                  resourceType: 'test-resource',
+                },
+              },
+              {
+                id: '345',
+                resourceRef: 'default:test/resource-2',
+                resourceType: 'test-resource-2',
+                conditions: {
+                  not: {
+                    rule: 'test-rule-1',
+                    resourceType: 'test-resource-2',
+                    params: {
+                      foo: 'a',
+                      bar: 1,
+                    },
+                  },
+                },
+              },
+              {
+                id: '456',
+                resourceRef: 'default:test/resource-3',
+                resourceType: 'test-resource',
+                conditions: {
+                  not: {
+                    rule: 'test-rule-2',
+                    resourceType: 'test-resource',
+                  },
+                },
+              },
+              {
+                id: '567',
+                resourceRef: 'default:test/resource-4',
+                resourceType: 'test-resource-2',
+                conditions: {
+                  anyOf: [
+                    {
+                      rule: 'test-rule-1',
+                      resourceType: 'test-resource-2',
+                      params: {
+                        foo: 'a',
+                        bar: 1,
+                      },
+                    },
+                    {
+                      rule: 'test-rule-1',
+                      resourceType: 'test-resource-2',
+                    },
+                  ],
+                },
+              },
+            ],
+          });
+      });
+
+      it('processes batched requests', () => {
+        expect(response.status).toEqual(200);
+        expect(response.body).toEqual({
+          items: [
+            { id: '123', result: AuthorizeResult.ALLOW },
+            { id: '234', result: AuthorizeResult.DENY },
+            { id: '345', result: AuthorizeResult.ALLOW },
+            { id: '456', result: AuthorizeResult.ALLOW },
+            { id: '567', result: AuthorizeResult.DENY },
+          ],
+        });
+      });
+
+      it('calls getResources for all required resources at once', () => {
+        expect(defaultMockedGetResources1).toHaveBeenCalledWith([
+          'default:test/resource-1',
+          'default:test/resource-3',
+        ]);
+        expect(defaultMockedGetResources2).toHaveBeenCalledWith([
+          'default:test/resource-2',
+          'default:test/resource-4',
+        ]);
+      });
+    });
+
    it('returns 400 when called with incorrect resource type', async () => {
      const response = await request(createApp())
        .post('/.well-known/backstage/permissions/apply-conditions')
@@ -573,21 +759,43 @@ describe('createPermissionIntegrationRouter', () => {
    });

    it('returns 501 with no getResources implementation', async () => {
-      const response = await request(createApp(null))
+      const response = await request(
+        express().use(
+          createPermissionIntegrationRouter({
+            resourceType: 'test-resource',
+            permissions: [testPermission],
+            rules: [testRule1, testRule2],
+          }),
+        ),
+      )
        .post('/.well-known/backstage/permissions/apply-conditions')
        .send({
-          items: [],
+          items: [
+            {
+              id: '345',
+              resourceRef: 'default:test/resource-2',
+              resourceType: 'test-resource',
+              conditions: {
+                rule: 'test-rule-1',
+                resourceType: 'test-resource',
+                params: {
+                  foo: 'a',
+                  bar: 1,
+                },
+              },
+            },
+          ],
        });

      expect(response.status).toEqual(501);
      expect(response.body.error.message).toEqual(
-        `This plugin does not expose any permission rule or can't evaluate conditional decisions`,
+        `This plugin does not expose any permission rule or can't evaluate the conditions request for test-resource`,
      );
    });
  });

  describe('GET /.well-known/backstage/permissions/metadata', () => {
-    it('returns a list of permissions and rules used by a given backend', async () => {
+    it('returns a list of permissions and rules of a single resource type', async () => {
      const response = await request(createApp()).get(
        '/.well-known/backstage/permissions/metadata',
      );
@@ -630,6 +838,139 @@ describe('createPermissionIntegrationRouter', () => {
        ],
      });
    });
+
+    it('returns a list of permissions and rules from multiple resource types', async () => {
+      const response = await request(
+        express().use(createPermissionIntegrationRouter(mockedOptionResources)),
+      ).get('/.well-known/backstage/permissions/metadata');
+
+      expect(response.status).toEqual(200);
+      expect(response.body).toEqual({
+        permissions: [testPermission, testPermission2],
+        rules: [
+          {
+            name: testRule1.name,
+            description: testRule1.description,
+            resourceType: testRule1.resourceType,
+            paramsSchema: {
+              $schema: 'http://json-schema.org/draft-07/schema#',
+              additionalProperties: false,
+              properties: {
+                foo: {
+                  type: 'string',
+                },
+                bar: {
+                  description: 'bar',
+                  type: 'number',
+                },
+              },
+              required: ['foo', 'bar'],
+              type: 'object',
+            },
+          },
+          {
+            name: testRule2.name,
+            description: testRule2.description,
+            resourceType: testRule2.resourceType,
+            paramsSchema: {
+              $schema: 'http://json-schema.org/draft-07/schema#',
+              additionalProperties: false,
+              properties: {},
+              type: 'object',
+            },
+          },
+          {
+            name: testRule3.name,
+            description: testRule3.description,
+            resourceType: testRule3.resourceType,
+            paramsSchema: {
+              $schema: 'http://json-schema.org/draft-07/schema#',
+              additionalProperties: false,
+              properties: {},
+              type: 'object',
+            },
+          },
+        ],
+      });
+    });
+  });
+
+  it('returns a list of basic permissions together with permissions and rules from multiple resource types', async () => {
+    const aPermission = createPermission({
+      name: 'a.permission',
+      attributes: {},
+    });
+
+    const response = await request(
+      express().use(
+        createPermissionIntegrationRouter({
+          permissions: [aPermission],
+          resources: [
+            {
+              resourceType: 'test-resource',
+              permissions: [testPermission],
+              getResources: defaultMockedGetResources1,
+              rules: [testRule1, testRule2],
+            },
+            {
+              resourceType: 'test-resource-2',
+              permissions: [testPermission2],
+              getResources: defaultMockedGetResources2,
+              rules: [testRule3],
+            },
+          ],
+        }),
+      ),
+    ).get('/.well-known/backstage/permissions/metadata');
+
+    expect(response.status).toEqual(200);
+    expect(response.body).toEqual({
+      permissions: [aPermission, testPermission, testPermission2],
+      rules: [
+        {
+          name: testRule1.name,
+          description: testRule1.description,
+          resourceType: testRule1.resourceType,
+          paramsSchema: {
+            $schema: 'http://json-schema.org/draft-07/schema#',
+            additionalProperties: false,
+            properties: {
+              foo: {
+                type: 'string',
+              },
+              bar: {
+                description: 'bar',
+                type: 'number',
+              },
+            },
+            required: ['foo', 'bar'],
+            type: 'object',
+          },
+        },
+        {
+          name: testRule2.name,
+          description: testRule2.description,
+          resourceType: testRule2.resourceType,
+          paramsSchema: {
+            $schema: 'http://json-schema.org/draft-07/schema#',
+            additionalProperties: false,
+            properties: {},
+            type: 'object',
+          },
+        },
+        {
+          name: testRule3.name,
+          description: testRule3.description,
+          resourceType: testRule3.resourceType,
+          paramsSchema: {
+            $schema: 'http://json-schema.org/draft-07/schema#',
+            additionalProperties: false,
+            properties: {},
+            type: 'object',
+          },
+        },
+      ],
+    });
  });
 });

@@ -207,6 +207,54 @@ export type CreatePermissionIntegrationRouterResourceOptions<
  ) => Promise<Array<TResource | undefined>>;
 };

+/**
+ * Options for creating a permission integration router exposing
+ * permissions and rules from multiple resource types.
+ *
+ * @public
+ */
+export type PermissionIntegrationRouterOptions<
+  TResourceType1 extends string = string,
+  TResource1 = any,
+  TResourceType2 extends string = string,
+  TResource2 = any,
+  TResourceType3 extends string = string,
+  TResource3 = any,
+> = {
+  resources: Readonly<
+    | [
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >,
+      ]
+    | [
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType2,
+          TResource2
+        >,
+      ]
+    | [
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType2,
+          TResource2
+        >,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType3,
+          TResource3
+        >,
+      ]
+  >;
+};
+
 /**
 * Create an express Router which provides an authorization route to allow
 * integration between the permission backend and other Backstage backend
@@ -217,6 +265,9 @@ export type CreatePermissionIntegrationRouterResourceOptions<
 * In case the `permissions` option is provided, the router also
 * provides a route that exposes permissions and routes of a plugin.
 *
+ * In case resources is provided, the routes can handle permissions
+ * for multiple resource types.
+ *
 * @remarks
 *
 * To make this concrete, we can use the Backstage software catalog as an
@@ -246,45 +297,75 @@ export type CreatePermissionIntegrationRouterResourceOptions<
 * @public
 */
 export function createPermissionIntegrationRouter<
-  TResourceType extends string,
-  TResource,
->(
-  options: CreatePermissionIntegrationRouterResourceOptions<
-    TResourceType,
-    TResource
-  >,
-): express.Router;
-
-/**
- *
- * Create an express Router which provides a route that exposes
- * permissions and routes of a plugin.
- * @public
- */
-export function createPermissionIntegrationRouter(options: {
-  permissions: Array<Permission>;
-}): express.Router;
-
-/**
- * @public
- */
-export function createPermissionIntegrationRouter<
-  TResourceType extends string,
-  TResource,
+  TResourceType1 extends string,
+  TResource1,
+  TResourceType2 extends string,
+  TResource2,
+  TResourceType3 extends string,
+  TResource3,
 >(
  options:
    | { permissions: Array<Permission> }
    | CreatePermissionIntegrationRouterResourceOptions<
-        TResourceType,
-        TResource
+        TResourceType1,
+        TResource1
+      >
+    | PermissionIntegrationRouterOptions<
+        TResourceType1,
+        TResource1,
+        TResourceType2,
+        TResource2,
+        TResourceType3,
+        TResource3
      >,
 ): express.Router {
+  const optionsWithResources = options as PermissionIntegrationRouterOptions;
+  const allOptions = [
+    optionsWithResources.resources ? optionsWithResources.resources : options,
+  ].flat();
+  const allRules = allOptions.flatMap(
+    option =>
+      (
+        option as CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >
+      ).rules || [],
+  );
+  const allPermissions = [
+    ...((options as { permissions: Permission[] }).permissions || []),
+    ...(optionsWithResources.resources?.flatMap(o => o.permissions || []) ||
+      []),
+  ];
+
+  const allResourceTypes = allOptions.reduce((acc, option) => {
+    if (
+      isCreatePermissionIntegrationRouterResourceOptions(
+        option as
+          | { permissions: Array<Permission> }
+          | CreatePermissionIntegrationRouterResourceOptions<
+              TResourceType1,
+              TResource1
+            >,
+      )
+    ) {
+      acc.push(
+        (
+          option as CreatePermissionIntegrationRouterResourceOptions<
+            TResourceType1,
+            TResource1
+          >
+        ).resourceType,
+      );
+    }
+    return acc;
+  }, [] as string[]);
+
  const router = Router();
  router.use(express.json());

-  const { permissions = [], rules = [] } = { rules: [], ...options };
  router.get('/.well-known/backstage/permissions/metadata', (_, res) => {
-    const serializedRules: MetadataResponseSerializedRule[] = rules.map(
+    const serializedRules: MetadataResponseSerializedRule[] = allRules.map(
      rule => ({
        name: rule.name,
        description: rule.description,
@@ -294,7 +375,7 @@ export function createPermissionIntegrationRouter<
    );

    const responseJson: MetadataResponse = {
-      permissions,
+      permissions: allPermissions,
      rules: serializedRules,
    };

@@ -304,23 +385,39 @@ export function createPermissionIntegrationRouter<
  router.post(
    '/.well-known/backstage/permissions/apply-conditions',
    async (req, res: Response<ApplyConditionsResponse | string>) => {
-      if (
-        !isCreatePermissionIntegrationRouterResourceOptions(options) ||
-        options.getResources === undefined
-      ) {
-        throw new NotImplementedError(
-          `This plugin does not expose any permission rule or can't evaluate conditional decisions`,
-        );
-      }
-      const { resourceType, getResources } = options;
+      const ruleMapByResourceType: Record<
+        string,
+        ReturnType<typeof createGetRule>
+      > = {};
+      const getResourcesByResourceType: Record<
+        string,
+        CreatePermissionIntegrationRouterResourceOptions<
+          TResourceType1,
+          TResource1
+        >['getResources']
+      > = {};

-      const getRule = createGetRule(rules);
+      for (let option of allOptions) {
+        option = option as
+          | { permissions: Array<Permission> }
+          | CreatePermissionIntegrationRouterResourceOptions<
+              TResourceType1,
+              TResource1
+            >;
+        if (isCreatePermissionIntegrationRouterResourceOptions(option)) {
+          ruleMapByResourceType[option.resourceType] = createGetRule(
+            option.rules,
+          );
+
+          getResourcesByResourceType[option.resourceType] = option.getResources;
+        }
+      }

      const assertValidResourceTypes = (
        requests: ApplyConditionsRequestEntry[],
      ) => {
        const invalidResourceTypes = requests
-          .filter(request => request.resourceType !== resourceType)
+          .filter(request => !allResourceTypes.includes(request.resourceType))
          .map(request => request.resourceType);

        if (invalidResourceTypes.length) {
@@ -340,23 +437,43 @@ export function createPermissionIntegrationRouter<

      assertValidResourceTypes(body.items);

-      const resourceRefs = Array.from(
-        new Set(body.items.map(({ resourceRef }) => resourceRef)),
-      );
-      const resourceArray = await getResources(resourceRefs);
-      const resources = resourceRefs.reduce((acc, resourceRef, index) => {
-        acc[resourceRef] = resourceArray[index];
-
+      const resourceRefsByResourceType = body.items.reduce<
+        Record<string, Set<string>>
+      >((acc, item) => {
+        if (!acc[item.resourceType]) {
+          acc[item.resourceType] = new Set();
+        }
+        acc[item.resourceType].add(item.resourceRef);
        return acc;
-      }, {} as Record<string, TResource | undefined>);
+      }, {});
+
+      const resourcesByResourceType: Record<string, Record<string, any>> = {};
+      for (const resourceType of Object.keys(resourceRefsByResourceType)) {
+        const getResources = getResourcesByResourceType[resourceType];
+        if (!getResources) {
+          throw new NotImplementedError(
+            `This plugin does not expose any permission rule or can't evaluate the conditions request for ${resourceType}`,
+          );
+        }
+        const resourceRefs = Array.from(
+          resourceRefsByResourceType[resourceType],
+        );
+        const resources = await getResources(resourceRefs);
+        resourceRefs.forEach((resourceRef, index) => {
+          if (!resourcesByResourceType[resourceType]) {
+            resourcesByResourceType[resourceType] = {};
+          }
+          resourcesByResourceType[resourceType][resourceRef] = resources[index];
+        });
+      }

      return res.json({
        items: body.items.map(request => ({
          id: request.id,
          result: applyConditions(
            request.conditions,
-            resources[request.resourceRef],
-            getRule,
+            resourcesByResourceType[request.resourceType][request.resourceRef],
+            ruleMapByResourceType[request.resourceType],
          )
            ? AuthorizeResult.ALLOW
            : AuthorizeResult.DENY,