permission-node: rename and adjust policy return type to reduce nesting

Signed-off-by: MT Lewis <mtlewis@users.noreply.github.com>

.changeset/hungry-impalas-wave.md

@@ -0,0 +1,6 @@
+---
+'@backstage/plugin-permission-node': minor
+'@backstage/plugin-permission-backend': patch
+---
+
+Rename and adjust permission policy return type to reduce nesting

plugins/permission-backend/src/service/router.test.ts

@@ -161,12 +161,10 @@ describe('createRouter', () => {
       beforeEach(() => {
         policy.handle.mockReturnValueOnce({
           result: AuthorizeResult.CONDITIONAL,
+          pluginId: 'test-plugin',
+          resourceType: 'test-resource-1',
           conditions: {
-            pluginId: 'test-plugin',
-            resourceType: 'test-resource-1',
-            conditions: {
-              anyOf: [{ rule: 'test-rule', params: ['abc'] }],
-            },
+            anyOf: [{ rule: 'test-rule', params: ['abc'] }],
           },
         });
       });
@@ -265,11 +263,9 @@ describe('createRouter', () => {
     it('returns a 500 error if the policy returns a different resourceType', async () => {
       policy.handle.mockReturnValueOnce({
         result: AuthorizeResult.CONDITIONAL,
-        conditions: {
-          pluginId: 'test-plugin',
-          resourceType: 'test-resource-2',
-          conditions: {},
-        },
+        pluginId: 'test-plugin',
+        resourceType: 'test-resource-2',
+        conditions: {},
       });
 
       const response = await request(app)

plugins/permission-backend/src/service/router.ts

@@ -80,7 +80,7 @@ const handleRequest = async (
 
   if (response.result === AuthorizeResult.CONDITIONAL) {
     // Sanity check that any resource provided matches the one expected by the permission
-    if (request.permission.resourceType !== response.conditions.resourceType) {
+    if (request.permission.resourceType !== response.resourceType) {
       throw new Error(
         `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`,
       );
@@ -92,7 +92,9 @@ const handleRequest = async (
         ...(await permissionIntegrationClient.applyConditions(
           {
             resourceRef,
-            ...response.conditions,
+            pluginId: response.pluginId,
+            resourceType: response.resourceType,
+            conditions: response.conditions,
           },
           authHeader,
         )),
@@ -102,10 +104,7 @@ const handleRequest = async (
     return {
       id,
       result: AuthorizeResult.CONDITIONAL,
-      // TODO(mtlewis): this .conditions.conditions situation is a bit awkward. I think it's
-      // worth exploring a bit of reorganization of the ConditionalPolicyResult type so that
-      // the naming of property chains like this makes a bit more sense.
-      conditions: response.conditions.conditions,
+      conditions: response.conditions,
     };
   }
 

plugins/permission-node/api-report.md

@@ -32,13 +32,11 @@ export type Condition<TRule> = TRule extends PermissionRule<
   : never;
 
 // @public
-export type ConditionalPolicyResult = {
+export type ConditionalPolicyDecision = {
   result: AuthorizeResult.CONDITIONAL;
-  conditions: {
-    pluginId: string;
-    resourceType: string;
-    conditions: PermissionCriteria<PermissionCondition>;
-  };
+  pluginId: string;
+  resourceType: string;
+  conditions: PermissionCriteria<PermissionCondition>;
 };
 
 // @public
@@ -63,11 +61,9 @@ export const createConditionExports: <
   rules: TRules;
 }) => {
   conditions: Conditions<TRules>;
-  createConditions: (conditions: PermissionCriteria<PermissionCondition>) => {
-    pluginId: string;
-    resourceType: string;
-    conditions: PermissionCriteria<PermissionCondition>;
-  };
+  createPolicyDecision: (
+    conditions: PermissionCriteria<PermissionCondition>,
+  ) => ConditionalPolicyDecision;
 };
 
 // @public
@@ -103,7 +99,7 @@ export interface PermissionPolicy {
   handle(
     request: PolicyAuthorizeRequest,
     user?: BackstageIdentity,
-  ): Promise<PolicyResult>;
+  ): Promise<PolicyDecision>;
 }
 
 // @public
@@ -122,9 +118,9 @@ export type PermissionRule<
 export type PolicyAuthorizeRequest = Omit<AuthorizeRequest, 'resourceRef'>;
 
 // @public
-export type PolicyResult =
+export type PolicyDecision =
   | {
       result: AuthorizeResult.ALLOW | AuthorizeResult.DENY;
     }
-  | ConditionalPolicyResult;
+  | ConditionalPolicyDecision;
 ```

plugins/permission-node/src/integration/createConditionExports.test.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { AuthorizeResult } from '@backstage/plugin-permission-common';
 import { createConditionExports } from './createConditionExports';
 
 const testIntegration = () =>
@@ -63,11 +64,14 @@ describe('createConditionExports', () => {
 
   describe('createConditions', () => {
     it('wraps conditions in an object with resourceType and pluginId', () => {
-      const { createConditions } = testIntegration();
+      const { createPolicyDecision } = testIntegration();
 
       expect(
-        createConditions({ allOf: [{ rule: 'testRule1', params: ['a', 1] }] }),
+        createPolicyDecision({
+          allOf: [{ rule: 'testRule1', params: ['a', 1] }],
+        }),
       ).toEqual({
+        result: AuthorizeResult.CONDITIONAL,
         pluginId: 'test-plugin',
         resourceType: 'test-resource',
         conditions: {

plugins/permission-node/src/integration/createConditionExports.ts

@@ -15,9 +15,11 @@
  */
 
 import {
+  AuthorizeResult,
   PermissionCondition,
   PermissionCriteria,
 } from '@backstage/plugin-permission-common';
+import { ConditionalPolicyDecision } from '../policy';
 import { PermissionRule } from '../types';
 import { createConditionFactory } from './createConditionFactory';
 
@@ -73,11 +75,9 @@ export const createConditionExports = <
   rules: TRules;
 }): {
   conditions: Conditions<TRules>;
-  createConditions: (conditions: PermissionCriteria<PermissionCondition>) => {
-    pluginId: string;
-    resourceType: string;
-    conditions: PermissionCriteria<PermissionCondition>;
-  };
+  createPolicyDecision: (
+    conditions: PermissionCriteria<PermissionCondition>,
+  ) => ConditionalPolicyDecision;
 } => {
   const { pluginId, resourceType, rules } = options;
 
@@ -89,9 +89,10 @@ export const createConditionExports = <
       }),
       {} as Conditions<TRules>,
     ),
-    createConditions: (
+    createPolicyDecision: (
       conditions: PermissionCriteria<PermissionCondition>,
     ) => ({
+      result: AuthorizeResult.CONDITIONAL,
       pluginId,
       resourceType,
       conditions,

plugins/permission-node/src/policy/index.ts

@@ -15,8 +15,8 @@
  */
 
 export type {
-  ConditionalPolicyResult,
+  ConditionalPolicyDecision,
   PermissionPolicy,
   PolicyAuthorizeRequest,
-  PolicyResult,
+  PolicyDecision,
 } from './types';

plugins/permission-node/src/policy/types.ts

@@ -48,13 +48,11 @@ export type PolicyAuthorizeRequest = Omit<AuthorizeRequest, 'resourceRef'>;
  * identifiers needed to evaluate the returned conditions.
  * @public
  */
-export type ConditionalPolicyResult = {
+export type ConditionalPolicyDecision = {
   result: AuthorizeResult.CONDITIONAL;
-  conditions: {
-    pluginId: string;
-    resourceType: string;
-    conditions: PermissionCriteria<PermissionCondition>;
-  };
+  pluginId: string;
+  resourceType: string;
+  conditions: PermissionCriteria<PermissionCondition>;
 };
 
 /**
@@ -62,9 +60,9 @@ export type ConditionalPolicyResult = {
  *
  * @public
  */
-export type PolicyResult =
+export type PolicyDecision =
   | { result: AuthorizeResult.ALLOW | AuthorizeResult.DENY }
-  | ConditionalPolicyResult;
+  | ConditionalPolicyDecision;
 
 /**
  * A policy to evaluate authorization requests for any permissioned action performed in Backstage.
@@ -86,5 +84,5 @@ export interface PermissionPolicy {
   handle(
     request: PolicyAuthorizeRequest,
     user?: BackstageIdentity,
-  ): Promise<PolicyResult>;
+  ): Promise<PolicyDecision>;
 }