From 2ba6e52f40ecc2b8b12bc1cd4505b903be1b2074 Mon Sep 17 00:00:00 2001
From: Frank Kong <frkong@redhat.com>
Date: Tue, 7 May 2024 12:02:17 -0400
Subject: [PATCH] chore: remove action read scaffolder permission

Signed-off-by: Frank Kong <frkong@redhat.com>
---
 .../scaffolder-backend/src/service/router.ts  | 34 ++---------------
 plugins/scaffolder-common/src/permissions.ts  | 37 ++++++-------------
 2 files changed, 15 insertions(+), 56 deletions(-)

diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts
index 759c5ca8d7..384b9f5340 100644
--- a/plugins/scaffolder-backend/src/service/router.ts
+++ b/plugins/scaffolder-backend/src/service/router.ts
@@ -30,12 +30,7 @@ import {
   UserEntity,
 } from '@backstage/catalog-model';
 import { Config, readDurationFromConfig } from '@backstage/config';
-import {
-  InputError,
-  NotAllowedError,
-  NotFoundError,
-  stringifyError,
-} from '@backstage/errors';
+import { InputError, NotFoundError, stringifyError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
 import { HumanDuration, JsonObject, JsonValue } from '@backstage/types';
 import {
@@ -56,7 +51,6 @@ import {
   templateParameterReadPermission,
   templateStepReadPermission,
   scaffolderTaskPermissions,
-  actionReadPermission,
 } from '@backstage/plugin-scaffolder-common/alpha';
 import express from 'express';
 import Router from 'express-promise-router';
@@ -78,10 +72,7 @@ import {
 import { createDryRunner } from '../scaffolder/dryrun';
 import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker';
 import { findTemplate, getEntityBaseUrl, getWorkingDirectory } from './helpers';
-import {
-  AuthorizeResult,
-  PermissionRuleParams,
-} from '@backstage/plugin-permission-common';
+import { PermissionRuleParams } from '@backstage/plugin-permission-common';
 import {
   createConditionAuthorizer,
   createPermissionIntegrationRouter,
@@ -420,12 +411,8 @@ export async function createRouter(
         permissions: scaffolderActionPermissions,
         rules: actionRules,
       },
-      {
-        resourceType: 'basic',
-        permissions: scaffolderTaskPermissions,
-        rules: [],
-      },
     ],
+    permissions: scaffolderTaskPermissions,
   });
 
   router.use(permissionIntegrationRouter);
@@ -464,20 +451,7 @@ export async function createRouter(
         });
       },
     )
-    .get('/v2/actions', async (req, res) => {
-      const credentials = await httpAuth.credentials(req);
-      if (permissions) {
-        const authorizationResponse = (
-          await permissions.authorizeConditional(
-            [{ permission: actionReadPermission }],
-            { credentials: credentials },
-          )
-        )[0];
-        if (authorizationResponse.result === AuthorizeResult.DENY) {
-          throw new NotAllowedError();
-        }
-      }
-
+    .get('/v2/actions', async (_req, res) => {
       const actionsList = actionRegistry.list().map(action => {
         return {
           id: action.id,
diff --git a/plugins/scaffolder-common/src/permissions.ts b/plugins/scaffolder-common/src/permissions.ts
index 6bd7e130ec..c441b48d5d 100644
--- a/plugins/scaffolder-common/src/permissions.ts
+++ b/plugins/scaffolder-common/src/permissions.ts
@@ -42,19 +42,6 @@ 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
- *
- * @alpha
- */
-export const actionReadPermission = createPermission({
-  name: 'scaffolder.action.read',
-  attributes: {
-    action: 'read',
-  },
-  resourceType: RESOURCE_TYPE_SCAFFOLDER_ACTION,
-});
 /**
  * This permission is used to authorize actions that involve reading
  * one or more parameters from a template.
@@ -126,15 +113,6 @@ export const taskCancelPermission = createPermission({
   attributes: {},
 });
 
-/**
- * List of all the scaffolder permissions
- * @alpha
- */
-export const scaffolderPermissions = [
-  templateParameterReadPermission,
-  templateStepReadPermission,
-];
-
 /**
  * List of the scaffolder permissions that are associated with template steps and parameters.
  * @alpha
@@ -148,10 +126,7 @@ export const scaffolderTemplatePermissions = [
  * List of the scaffolder permissions that are associated with scaffolder actions.
  * @alpha
  */
-export const scaffolderActionPermissions = [
-  actionExecutePermission,
-  actionReadPermission,
-];
+export const scaffolderActionPermissions = [actionExecutePermission];
 
 /**
  * List of the scaffolder permissions that are associated with scaffolder tasks.
@@ -162,3 +137,13 @@ export const scaffolderTaskPermissions = [
   taskCreatePermission,
   taskReadPermission,
 ];
+
+/**
+ * List of all the scaffolder permissions
+ * @alpha
+ */
+export const scaffolderPermissions = [
+  ...scaffolderTemplatePermissions,
+  ...scaffolderActionPermissions,
+  ...scaffolderTaskPermissions,
+];