Add extra validations for endpoints that do not use principal to get the resources (#2112)

Fixes OPS-3895.

---------

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

Author: MarceloRGonc

packages/server/api/src/app/flows/common/flow-validations.ts

@@ -0,0 +1,41 @@
+import {
+  ApplicationError,
+  ErrorCode,
+  Flow,
+  FlowVersion,
+  PopulatedFlow,
+} from '@openops/shared';
+import { flowService } from '../flow/flow.service';
+
+export async function assertFlowVersionBelongsToProject(
+  flowVersion: FlowVersion,
+  projectId: string,
+): Promise<void> {
+  const flow = await flowService.getOne({
+    id: flowVersion.flowId,
+    projectId,
+  });
+
+  if (flow == null) {
+    throw new ApplicationError({
+      code: ErrorCode.AUTHORIZATION,
+      params: {
+        message: 'The flow and version are not associated with the project',
+      },
+    });
+  }
+}
+
+export function assertFlowBelongsToProject(
+  flow: PopulatedFlow | Flow,
+  projectId: string,
+): void {
+  if (flow.projectId !== projectId) {
+    throw new ApplicationError({
+      code: ErrorCode.AUTHORIZATION,
+      params: {
+        message: 'The flow is not associated with the project',
+      },
+    });
+  }
+}

packages/server/api/src/app/flows/common/flow-version-validation.ts

@@ -83,6 +83,7 @@ export const flowRunController: FastifyPluginCallbackTypebox = (
     const flowRun = await flowRunService.retry({
       flowRunId: req.params.id,
       strategy: req.body.strategy,
+      projectId: req.principal.projectId,
     });
 
     if (isNil(flowRun)) {

packages/server/api/src/app/flows/flow-run/flow-run-controller.ts

@@ -260,7 +260,11 @@ export const flowRunService = {
 
     return query.getCount();
   },
-  async retry({ flowRunId, strategy }: RetryParams): Promise<FlowRun | null> {
+  async retry({
+    flowRunId,
+    strategy,
+    projectId,
+  }: RetryParams): Promise<FlowRun | null> {
     if (strategy !== FlowRetryStrategy.ON_LATEST_VERSION) {
       logger.warn(`The provided strategy (${strategy}) is not valid.`, {
         flowRunId,
@@ -271,7 +275,7 @@ export const flowRunService = {
 
     const flowRun = await flowRunService.getOnePopulatedOrThrow({
       id: flowRunId,
-      projectId: undefined,
+      projectId,
     });
 
     const flowVersion = await flowVersionService.getLatestLockedVersionOrThrow(
@@ -684,6 +688,7 @@ type PauseParams = {
 };
 
 type RetryParams = {
+  projectId: ProjectId;
   flowRunId: FlowRunId;
   strategy: FlowRetryStrategy;
 };

packages/server/api/src/app/flows/flow-run/flow-run-service.ts

@@ -4,19 +4,17 @@ import {
 } from '@fastify/type-provider-typebox';
 import {
   ApplicationError,
-  ErrorCode,
   FlowVersionState,
   MinimalFlow,
   OpenOpsId,
   Permission,
   PrincipalType,
   SERVICE_KEY_SECURITY_OPENAPI,
-  StepOutputWithData,
   UpdateFlowVersionRequest,
 } from '@openops/shared';
 import { StatusCodes } from 'http-status-codes';
 import { getProjectScopedRoutePolicy } from '../../core/security/route-policies/route-security-policy-factory';
-import { validateFlowVersionBelongsToProject } from '../common/flow-version-validation';
+import { assertFlowVersionBelongsToProject } from '../common/flow-validations';
 import { flowVersionService } from '../flow-version/flow-version.service';
 import { flowStepTestOutputService } from '../step-test-output/flow-step-test-output.service';
 
@@ -62,16 +60,11 @@ export const flowVersionController: FastifyPluginAsyncTypebox = async (
           });
         }
 
-        const isValid = await validateFlowVersionBelongsToProject(
+        await assertFlowVersionBelongsToProject(
           flowVersion,
           request.principal.projectId,
-          reply,
         );
 
-        if (!isValid) {
-          return;
-        }
-
         if (flowVersion.state === FlowVersionState.LOCKED) {
           await reply.status(StatusCodes.BAD_REQUEST).send({
             success: false,
@@ -103,6 +96,10 @@ export const flowVersionController: FastifyPluginAsyncTypebox = async (
           message: newFlowVersion.updated,
         });
       } catch (error) {
+        if (error instanceof ApplicationError) {
+          throw error;
+        }
+
         await reply.status(StatusCodes.INTERNAL_SERVER_ERROR).send({
           success: false,
           message: (error as Error).message,
@@ -147,16 +144,23 @@ export const flowVersionController: FastifyPluginAsyncTypebox = async (
         }),
       },
     },
-    async (request): Promise<Record<OpenOpsId, StepOutputWithData>> => {
+    async (request) => {
       const { stepIds } = request.query;
       const { flowVersionId } = request.params;
 
+      const flowVersion = await flowVersionService.getOneOrThrow(flowVersionId);
+      await assertFlowVersionBelongsToProject(
+        flowVersion,
+        request.principal.projectId,
+      );
+
       const flowStepTestOutputs = await flowStepTestOutputService.listDecrypted(
         {
           stepIds,
           flowVersionId,
         },
       );
+
       return Object.fromEntries(
         flowStepTestOutputs.map((flowStepTestOutput) => [
           flowStepTestOutput.stepId as OpenOpsId,
@@ -203,8 +207,11 @@ export const flowVersionController: FastifyPluginAsyncTypebox = async (
             message: Type.String(),
           }),
           [StatusCodes.NOT_FOUND]: Type.Object({
-            success: Type.Boolean(),
-            message: Type.String(),
+            code: Type.String(),
+            params: Type.Object({
+              entityId: Type.String(),
+              entityType: Type.String(),
+            }),
           }),
         },
       },
@@ -216,40 +223,33 @@ export const flowVersionController: FastifyPluginAsyncTypebox = async (
         const flowVersion = await flowVersionService.getOneOrThrow(
           flowVersionId,
         );
-        const isValid = await validateFlowVersionBelongsToProject(
+
+        await assertFlowVersionBelongsToProject(
           flowVersion,
           request.principal.projectId,
-          reply,
         );
-        if (!isValid) {
-          return;
-        }
+
         const saved = await flowStepTestOutputService.save({
           stepId,
           flowVersionId,
           input,
           output,
           success,
         });
+
         await reply.status(StatusCodes.OK).send({
           success: true,
           id: saved.id,
         });
       } catch (error) {
-        if (
-          error instanceof ApplicationError &&
-          error.error.code === ErrorCode.ENTITY_NOT_FOUND
-        ) {
-          await reply.status(StatusCodes.NOT_FOUND).send({
-            success: false,
-            message: 'The defined flow version was not found',
-          });
-        } else {
-          await reply.status(StatusCodes.BAD_REQUEST).send({
-            success: false,
-            message: (error as Error).message,
-          });
+        if (error instanceof ApplicationError) {
+          throw error;
         }
+
+        await reply.status(StatusCodes.BAD_REQUEST).send({
+          success: false,
+          message: (error as Error).message,
+        });
       }
     },
   );

packages/server/api/src/app/flows/flow/flow-version.controller.ts

@@ -418,7 +418,7 @@ const DeleteFlowRequestOptions = {
     allowedPrincipals: [PrincipalType.USER, PrincipalType.SERVICE],
     security: getProjectScopedRoutePolicy({
       allowedPrincipals: [PrincipalType.USER, PrincipalType.SERVICE],
-      permission: Permission.WRITE_FLOW,
+      permission: Permission.DELETE_FLOW,
     }),
   },
   schema: {

packages/server/api/src/app/flows/flow/flow.controller.ts

@@ -15,6 +15,7 @@ export const formController: FastifyPluginAsyncTypebox = async (app) => {
   app.get('/:flowId', GetFormRequest, async (request) => {
     return formService.getFormByFlowIdOrThrow(
       request.params.flowId,
+      request.principal.projectId,
       request.query.useDraft ?? false,
     );
   });

packages/server/api/src/app/flows/flow/form/form.controller.ts

@@ -8,6 +8,7 @@ import {
   isNil,
   PopulatedFlow,
 } from '@openops/shared';
+import { assertFlowBelongsToProject } from '../../common/flow-validations';
 import { flowVersionService } from '../../flow-version/flow-version.service';
 import { flowRepo } from '../flow.repo';
 
@@ -30,6 +31,7 @@ const FORMS_TRIGGER_NAMES = [FORM_TRIIGGER, FILE_TRIGGER];
 export const formService = {
   getFormByFlowIdOrThrow: async (
     flowId: string,
+    projectId: string,
     useDraft: boolean,
   ): Promise<FormResponse> => {
     const flow = await getPopulatedFlowById(flowId, useDraft);
@@ -48,6 +50,8 @@ export const formService = {
         },
       });
     }
+
+    assertFlowBelongsToProject(flow, projectId);
     logger.info(flow.version.trigger.settings);
     const triggerName = flow.version.trigger.settings.triggerName;
     return {

packages/server/api/src/app/flows/flow/form/form.service.ts

@@ -11,7 +11,7 @@ import {
 } from '@openops/shared';
 import { StatusCodes } from 'http-status-codes';
 import { getProjectScopedRoutePolicy } from '../../core/security/route-policies/route-security-policy-factory';
-import { validateFlowVersionBelongsToProject } from '../common/flow-version-validation';
+import { assertFlowVersionBelongsToProject } from '../common/flow-validations';
 import { flowRunService } from '../flow-run/flow-run-service';
 import { flowVersionService } from '../flow-version/flow-version.service';
 import { stepRunService } from '../step-run/step-run-service';
@@ -24,15 +24,7 @@ export const testController: FastifyPluginAsyncTypebox = async (fastify) => {
 
     const flowVersion = await flowVersionService.getOneOrThrow(flowVersionId);
 
-    const isValid = await validateFlowVersionBelongsToProject(
-      flowVersion,
-      projectId,
-      reply,
-    );
-
-    if (!isValid) {
-      return;
-    }
+    await assertFlowVersionBelongsToProject(flowVersion, projectId);
 
     const step = flowHelper
       .getAllSteps(flowVersion.trigger)
@@ -76,15 +68,8 @@ export const testController: FastifyPluginAsyncTypebox = async (fastify) => {
     try {
       const flowVersion = await flowVersionService.getOneOrThrow(flowVersionId);
 
-      const isValid = await validateFlowVersionBelongsToProject(
-        flowVersion,
-        projectId,
-        reply,
-      );
+      await assertFlowVersionBelongsToProject(flowVersion, projectId);
 
-      if (!isValid) {
-        return;
-      }
       const flowRun = await flowRunService.test({
         projectId,
         flowVersionId: flowVersion.id,

packages/server/api/src/app/flows/test/test.controller.ts

@@ -150,7 +150,7 @@ describe('Flow Version API', () => {
 
       expect(response?.statusCode).toBe(StatusCodes.FORBIDDEN);
       const responseBody = JSON.parse(response?.body || '{}');
-      expect(responseBody?.message).toBe(
+      expect(responseBody?.params.message).toBe(
         'The flow and version are not associated with the project',
       );
     });
@@ -368,10 +368,7 @@ describe('POST to update test-output', () => {
     });
 
     expect(response?.statusCode).toBe(StatusCodes.NOT_FOUND);
-    const responseBody = response?.json();
-    expect(responseBody).toMatchObject({
-      success: false,
-      message: 'The defined flow version was not found',
-    });
+    const responseBody = JSON.parse(response?.body || '{}');
+    expect(responseBody?.params.entityType).toBe('FlowVersion');
   });
 });