chore: apply suggestions

Signed-off-by: Frank Kong <frkong@redhat.com>

plugins/catalog/package.json

@@ -89,7 +89,7 @@
     "@testing-library/react": "^15.0.0",
     "@testing-library/user-event": "^14.0.0",
     "@types/pluralize": "^0.0.33",
-    "swr": "^2.0.0"
+    "swr": "^2.2.5"
   },
   "peerDependencies": {
     "react": "^16.13.1 || ^17.0.0 || ^18.0.0",

plugins/catalog/src/components/AboutCard/AboutCard.tsx

@@ -116,10 +116,9 @@ export function AboutCard(props: AboutCardProps) {
   const { allowed: canRefresh } = useEntityPermission(
     catalogEntityRefreshPermission,
   );
-  const { kind, name, namespace } = entity.metadata;
+
   const { allowed: canCreateTemplateTask } = usePermission({
     permission: taskCreatePermission,
-    resourceRef: `${kind}:${namespace}/${name}`,
   });
 
   const entitySourceLocation = getEntitySourceLocation(

plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts

@@ -31,6 +31,7 @@ import {
   SecureTemplateRenderer,
 } from '../../lib/templating/SecureTemplater';
 import {
+  TaskRecovery,
   TaskSpec,
   TaskSpecV1beta3,
   TaskStep,
@@ -52,7 +53,6 @@ import {
 } from '@backstage/plugin-permission-common';
 import { scaffolderActionRules } from '../../service/rules';
 import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha';
-import { TaskRecovery } from '@backstage/plugin-scaffolder-common';
 import { PermissionsService } from '@backstage/backend-plugin-api';
 import { loggerToWinstonLogger } from '@backstage/backend-common';
 import { BackstageLoggerTransport, WinstonLogger } from './logger';

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

@@ -30,7 +30,12 @@ import {
   UserEntity,
 } from '@backstage/catalog-model';
 import { Config, readDurationFromConfig } from '@backstage/config';
-import { InputError, NotFoundError, stringifyError } from '@backstage/errors';
+import {
+  InputError,
+  NotAllowedError,
+  NotFoundError,
+  stringifyError,
+} from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
 import { HumanDuration, JsonObject, JsonValue } from '@backstage/types';
 import {
@@ -43,7 +48,6 @@ import {
 import {
   RESOURCE_TYPE_SCAFFOLDER_ACTION,
   RESOURCE_TYPE_SCAFFOLDER_TEMPLATE,
-  RESOURCE_TYPE_SCAFFOLDER_TASK,
   scaffolderActionPermissions,
   scaffolderTemplatePermissions,
   taskCancelPermission,
@@ -74,7 +78,10 @@ import {
 import { createDryRunner } from '../scaffolder/dryrun';
 import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker';
 import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers';
-import { PermissionRuleParams } from '@backstage/plugin-permission-common';
+import {
+  AuthorizeResult,
+  PermissionRuleParams,
+} from '@backstage/plugin-permission-common';
 import {
   createConditionAuthorizer,
   createPermissionIntegrationRouter,
@@ -97,11 +104,7 @@ import {
 import { InternalTaskSecrets } from '../scaffolder/tasks/types';
 import { checkPermission } from '../util/checkPermissions';
 
-/**
- *
- * @public
- */
-export type ScaffolderPermissionRuleInput =
+type ScaffolderPermissionRuleInput =
   | TemplatePermissionRuleInput
   | ActionPermissionRuleInput
   | TaskPermissionRuleInput;
@@ -440,7 +443,7 @@ export async function createRouter(
         rules: actionRules,
       },
       {
-        resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK,
+        resourceType: 'basic',
         permissions: scaffolderTaskPermissions,
         rules: taskRules,
       },
@@ -485,11 +488,18 @@ export async function createRouter(
     )
     .get('/v2/actions', async (req, res) => {
       const credentials = await httpAuth.credentials(req);
-      await checkPermission({
-        credentials,
-        permissions: [actionReadPermission],
-        permissionService: permissions,
-      });
+      if (permissions) {
+        const authorizationResponse = (
+          await permissions.authorizeConditional(
+            [{ permission: actionReadPermission }],
+            { credentials: credentials },
+          )
+        )[0];
+        if (authorizationResponse.result === AuthorizeResult.DENY) {
+          throw new NotAllowedError();
+        }
+      }
+
       const actionsList = actionRegistry.list().map(action => {
         return {
           id: action.id,

plugins/scaffolder-backend/src/util/checkPermissions.ts

@@ -20,12 +20,12 @@ import {
 import { NotAllowedError } from '@backstage/errors';
 import {
   AuthorizeResult,
-  ResourcePermission,
+  BasicPermission,
 } from '@backstage/plugin-permission-common';
 
 export type checkPermissionOptions = {
   credentials: BackstageCredentials;
-  permissions: ResourcePermission[];
+  permissions: BasicPermission[];
   permissionService?: PermissionsService;
 };
 
@@ -39,7 +39,7 @@ export async function checkPermission(options: checkPermissionOptions) {
     const permissionRequest = permissions.map(resourcePermission => ({
       permission: resourcePermission,
     }));
-    const authorizationResponses = await permissionService.authorizeConditional(
+    const authorizationResponses = await permissionService.authorize(
       permissionRequest,
       { credentials: credentials },
     );

plugins/scaffolder-common/src/permissions.ts

@@ -30,13 +30,6 @@ export const RESOURCE_TYPE_SCAFFOLDER_TEMPLATE = 'scaffolder-template';
  */
 export const RESOURCE_TYPE_SCAFFOLDER_ACTION = 'scaffolder-action';
 
-/**
- * Permission resource type which corresponds to a scaffolder task.
- *
- * @alpha
- */
-export const RESOURCE_TYPE_SCAFFOLDER_TASK = 'scaffolder-task';
-
 /**
  * This permission is used to authorize actions that involve executing
  * an action from a template.
@@ -49,6 +42,7 @@ export const actionExecutePermission = createPermission({
   resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
 });
 
+// TODO: Figure out whether to convert this to a basic permission or remove it completely since the current rules aren't applicable to this permission
 /**
  * This permission is used to authorize actions that involve access the action registry
  *
@@ -101,8 +95,6 @@ export const templateStepReadPermission = createPermission({
  * This permission is used to authorize actions that involve reading one or more tasks in the scaffolder,
  * and reading logs of tasks
  *
- * Task cancellation would also require this permission.
- *
  * @alpha
  */
 export const taskReadPermission = createPermission({
@@ -110,7 +102,6 @@ export const taskReadPermission = createPermission({
   attributes: {
     action: 'read',
   },
-  resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK,
 });
 
 /**
@@ -123,20 +114,16 @@ export const taskCreatePermission = createPermission({
   attributes: {
     action: 'create',
   },
-  resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK,
 });
 
 /**
- * This permission us used to authorize actions that involve the cancellation of tasks in the scaffolder.
- *
- * This will require the `scaffolder.task.read` permission to be authorized.
+ * This permission is used to authorize actions that involve the cancellation of tasks in the scaffolder.
  *
  * @alpha
  */
 export const taskCancelPermission = createPermission({
   name: 'scaffolder.task.cancel',
   attributes: {},
-  resourceType: RESOURCE_TYPE_SCAFFOLDER_TASK,
 });
 
 /**
@@ -144,7 +131,6 @@ export const taskCancelPermission = createPermission({
  * @alpha
  */
 export const scaffolderPermissions = [
-  actionExecutePermission,
   templateParameterReadPermission,
   templateStepReadPermission,
 ];

plugins/scaffolder-react/src/next/components/TemplateCard/TemplateCard.tsx

@@ -112,7 +112,6 @@ export const TemplateCard = (props: TemplateCardProps) => {
 
   const { allowed: canCreateTask } = usePermission({
     permission: taskCreatePermission,
-    resourceRef: 'task',
   });
   const handleChoose = useCallback(() => {
     analytics.captureEvent('click', `Template has been opened`);

plugins/scaffolder/src/components/OngoingTask/ContextMenu.tsx

@@ -77,25 +77,18 @@ export const ContextMenu = (props: ContextMenuProps) => {
     }
   });
 
-  // Used dummy string value for `resourceRef` since `allowed` field will always return `false` if `resourceRef` is `undefined`
   const { allowed: canCancelTask } = usePermission({
     permission: taskCancelPermission,
-    resourceRef: 'task',
   });
 
   const { allowed: canReadTask } = usePermission({
     permission: taskReadPermission,
-    resourceRef: 'task',
   });
 
   const { allowed: canCreateTask } = usePermission({
     permission: taskCreatePermission,
-    resourceRef: 'task',
   });
 
-  // Cancel endpoint requires user to have both read and cancel permissions
-  const cancelNotAllowed = !(canReadTask && canCancelTask);
-
   // Start Over endpoint requires user to have both read (to grab parameters) and create (to create new task) permissions
   const canStartOver = canReadTask && canCreateTask;
 
@@ -150,7 +143,7 @@ export const ContextMenu = (props: ContextMenuProps) => {
             disabled={
               !cancelEnabled ||
               cancelStatus !== 'not-executed' ||
-              cancelNotAllowed
+              !canCancelTask
             }
             data-testid="cancel-task"
           >

plugins/scaffolder/src/components/OngoingTask/OngoingTask.tsx

@@ -91,22 +91,16 @@ export const OngoingTask = (props: {
   // Used dummy string value for `resourceRef` since `allowed` field will always return `false` if `resourceRef` is `undefined`
   const { allowed: canCancelTask } = usePermission({
     permission: taskCancelPermission,
-    resourceRef: 'task',
   });
 
   const { allowed: canReadTask } = usePermission({
     permission: taskReadPermission,
-    resourceRef: 'task',
   });
 
   const { allowed: canCreateTask } = usePermission({
     permission: taskCreatePermission,
-    resourceRef: 'task',
   });
 
-  // Cancel endpoint requires user to have both read and cancel permissions
-  const cancelNotAllowed = !(canReadTask && canCancelTask);
-
   // Start Over endpoint requires user to have both read (to grab parameters) and create (to create new task) permissions
   const canStartOver = canReadTask && canCreateTask;
 
@@ -228,7 +222,7 @@ export const OngoingTask = (props: {
                     disabled={
                       !cancelEnabled ||
                       cancelStatus !== 'not-executed' ||
-                      cancelNotAllowed
+                      !canCancelTask
                     }
                     onClick={triggerCancel}
                     data-testid="cancel-button"

yarn.lock

@@ -5684,7 +5684,7 @@ __metadata:
     lodash: ^4.17.21
     pluralize: ^8.0.0
     react-use: ^17.2.4
-    swr: ^2.0.0
+    swr: ^2.2.5
     zen-observable: ^0.10.0
   peerDependencies:
     react: ^16.13.1 || ^17.0.0 || ^18.0.0
@@ -39520,7 +39520,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"swr@npm:^2.0.0":
+"swr@npm:^2.0.0, swr@npm:^2.2.5":
   version: 2.2.5
   resolution: "swr@npm:2.2.5"
   dependencies: