Jobs tests (#1286)

* first tests * feat: reset dataset before every api test (#1131) * reset dataset before every api test * Clear collections before each api test * Fixes after jobs testing: 1. Use CreateJobAuth enum instead of AuthOp to define jobDatasetAuthorization 2. Shift a block if groupOwner is present down after the admin check was passed (admin create a job for anonym) 3. Check if current authorization is in jobDatasetAuthorization in instanceAuthorizationJobCreate (types mismatching) 4. Add a check if unauthorized user creates a job for someone else * Fixes after jobs testing: if anonymous user creates a job, initialize a jwt instance for him/her * Fixes after testing: string array includes string enum * dataset(s)Validation redundancy after a merged PR * test Jobs authorization wip * fix filtering syntax * fixes after Job testing: 1. comment out jobConfig matchin, as now in interceptor 2. fix filtering object hierarchy in #dataset kind of jobConfig's 3. not defining jobInstance ownerUser/Group as empty string 4. add jobInstance.jobParams initialization 5. add a check if datasets passed in the ID list exist * minor changes * fix findOne filtering syntax * add Jobs tests * remove redundancy * fix numbering * extend UPDATE_JOB_GROUP user's right to match docs * remove unneccessary arguments * make function return value of findOne an instance of JobClass * add statusUpdate tests * add jobParams to the job instance * Patch tests * Fixes after tests: * Anonymous user can patch status update to a job in #all config. Added instance authorization * Implement JobDelete authorization * add deleteJobGroups from env to configuration * add username for anonymous user when creating/updating status of the job * statusUpdate instead of update * Fixes after testing: * similarly to Datasets, mongoose returns an Object and not JobClass instances. To pass this into CASL we first need to make those class instances. generateJobInstanceForPermissions implements a copy of object as correct class instance only with properties required in casl. * functionality of instanceAuthorizationJobStatusUpdate is now implemented in the patch endpoint function. This is done to avoid redefining a found job as a instance of JobClass with all properties * For both GET endpoints only the endpoint authorisation was implemented for user in the functions of the controller. To add the instance authorization too, first create JobClass instance of the found job Objects and pass them to respective casl expressions. * adds implementation of delete endpoints. The rules are defined in the CheckPolicies decorator. * Finalized tests for authorization * add delete_job_groups definition * fix unit tests after test jobconfig.json changes * empty collections before testing * changes requested in PR * eremove version duplicate * change getJobMatchingConfiguration function to explicitly use job type, st it can be used for all cases where we need to extract the configuration, without having to pass the full dto * run full testing workflow on pushing to release branches * add env variable for jobs config path for github workflow * Fix lint errors - Run lint:fix - Ignore no-explicit-any rule for jobParams (which are not type checked) - remove 'read' jobOperation (not implemented) * Fix lint following merge * Update test/config/pretest.js to use "dotenv" Co-authored-by: Jay <[email protected]> * Fix pretest.js after merging suggestions * Fix pretest.js * Fix jobconfig no-explicit-any issues - Revert previous changes to ignore no-explicit-any - Use `unknown` for jobParam inputs plus stronger runtime checks - Fix compilation error in elastic-search service relating to deleting non-optional variables. - Remove rabbitmqaction validation. RabbitMQ should ignore the DTO body, and validate the config itself in the constructor. * Fix jobconfig no-explicit-any issue (missed logaction) * changes in tests: 0060 now checks if a job was passed without a required parameter JobParams 0065 (new) checks if a job was passed with an empty object for jobParams 1950 now tries to delete a job that doesn't exist and fails 1960 was called 1950 before counts on GET are changed (because 0060 before was passing) * address comments in the PR: * now makes one request to the db to find datasets and extracts their pids. * loggs the names of all dataset pids that are not in the db in case of an error * job endpoint implements the check that the jobParams were passed and are not empty, otherwise it will throw a bad request with respective message * job endpoint loggs the id of job to be deleted and implements a check if job exist in db, otherwise throws an error. --------- Co-authored-by: Jay Quan <[email protected]> Co-authored-by: Spencer Bliven <[email protected]> Co-authored-by: Despina Adamopoulou <[email protected]>
SciCatProject · Jul 29, 2024 · 4e67d3a · 4e67d3a
1 parent a1dfaf6
commit 4e67d3a
Show file tree

Hide file tree

Showing 39 changed files with 4,562 additions and 1,167 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -6,6 +6,7 @@ on:
   pull_request:
     branches:
       - master
+      - release-*
 
 jobs:
   install-and-cache:
@@ -152,6 +153,9 @@ jobs:
           CREATE_DATASET_WITH_PID_GROUPS: "group2"
           CREATE_DATASET_PRIVILEGED_GROUPS: "datasetIngestor,group3"
           ACCESS_GROUPS_STATIC_VALUES: "ess"
+          CREATE_JOB_GROUPS: group1,group2
+          UPDATE_JOB_GROUPS: group1
+          DELETE_JOB_GROUPS: "archivemanager"
           PROPOSAL_GROUPS: "proposalingestor"
           SAMPLE_PRIVILEGED_GROUPS: "sampleingestor"
           SAMPLE_GROUPS: "group1"
@@ -168,6 +172,7 @@ jobs:
           STACK_VERSION: 8.8.2
           CLUSTER_NAME: es-cluster
           MEM_LIMIT: 4G
+          JOB_CONFIGURATION_FILE: test/config/jobconfig.json
 
         # Start mongo container and app before running api tests
         run: |

diff --git a/src/casl/casl-ability.factory.ts b/src/casl/casl-ability.factory.ts
@@ -25,7 +25,10 @@ import { UserSettings } from "src/users/schemas/user-settings.schema";
 import { User } from "src/users/schemas/user.schema";
 import { AuthOp } from "./authop.enum";
 import configuration from "src/config/configuration";
-import { CreateJobAuth, StatusUpdateJobAuth } from "src/jobs/types/jobs-auth.enum";
+import {
+  CreateJobAuth,
+  StatusUpdateJobAuth,
+} from "src/jobs/types/jobs-auth.enum";
 
 type Subjects =
   | string
@@ -818,36 +821,53 @@ export class CaslAbilityFactory {
         ["configuration.create.auth" as string]: CreateJobAuth.DatasetPublic,
         datasetsValidation: true,
       });
+      can(AuthOp.JobStatusUpdateConfiguration, JobClass, {
+        ["configuration.statusUpdate.auth" as string]: StatusUpdateJobAuth.All,
+        ownerGroup: undefined,
+      });
     } else {
       /**
        * authenticated users
        */
-
       // check if this user is part of the admin group
       if (
         user.currentGroups.some((g) => configuration().adminGroups.includes(g))
       ) {
         /**
          * authenticated users belonging to any of the group listed in ADMIN_GROUPS
          */
-
         // -------------------------------------
         // endpoint authorization
         can(AuthOp.JobRead, JobClass);
         can(AuthOp.JobCreate, JobClass);
         can(AuthOp.JobStatusUpdate, JobClass);
+        cannot(AuthOp.JobDelete, JobClass);
 
         // -------------------------------------
         // data instance authorization
         can(AuthOp.JobReadAny, JobClass);
         can(AuthOp.JobCreateAny, JobClass);
         can(AuthOp.JobStatusUpdateAny, JobClass);
+      } else if (
+        user.currentGroups.some((g) =>
+          configuration().deleteJobGroups.includes(g),
+        )
+      ) {
+        /**
+         * authenticated users belonging to any of the group listed in DELETE_JOB_GROUPS
+         */
+        // -------------------------------------
+        // endpoint authorization
+        can(AuthOp.JobDelete, JobClass);
+
+        // -------------------------------------
+        // data instance authorization
+        can(AuthOp.JobDeleteAny, JobClass);
       } else {
         const jobUserAuthorizationValues = [
           ...user.currentGroups.map((g) => "@" + g),
           user.username,
         ];
-
         if (
           user.currentGroups.some((g) =>
             configuration().createJobGroups.includes(g),
@@ -880,7 +900,7 @@ export class CaslAbilityFactory {
           ];
           const jobCreateInstanceAuthorizationValues = [
             ...Object.values(CreateJobAuth).filter(
-              (v) => ~String(v).includes("#dataset"),
+              (v) => !String(v).includes("#dataset"),
             ),
             ...jobUserAuthorizationValues,
           ];
@@ -889,13 +909,17 @@ export class CaslAbilityFactory {
               String(v).includes("#dataset"),
             ),
           ];
-
           // -------------------------------------
           // endpoint authorization
           can(AuthOp.JobRead, JobClass);
+
           if (
             configuration().jobConfiguration.some(
-              (j) => j.create.auth! in jobCreateEndPointAuthorizationValues,
+              (j) =>
+                j.create.auth &&
+                jobCreateEndPointAuthorizationValues.includes(
+                  j.create.auth as string,
+                ),
             )
           ) {
             can(AuthOp.JobCreate, JobClass);
@@ -907,6 +931,7 @@ export class CaslAbilityFactory {
             ownerGroup: { $in: user.currentGroups },
             ownerUser: user.username,
           });
+
           can(AuthOp.JobCreateConfiguration, JobClass, {
             ["configuration.create.auth" as string]: {
               $in: jobCreateInstanceAuthorizationValues,
@@ -919,6 +944,16 @@ export class CaslAbilityFactory {
             datasetsValidation: true,
           });
         }
+        const jobUpdateEndPointAuthorizationValues = [
+          ...Object.values(StatusUpdateJobAuth),
+          ...jobUserAuthorizationValues,
+        ];
+        const jobUpdateInstanceAuthorizationValues = [
+          ...Object.values(StatusUpdateJobAuth).filter(
+            (v) => !String(v).includes("#job"),
+          ),
+          ...jobUserAuthorizationValues,
+        ];
 
         if (
           user.currentGroups.some((g) =>
@@ -931,29 +966,27 @@ export class CaslAbilityFactory {
 
           // -------------------------------------
           // data instance authorization
+          can(AuthOp.JobStatusUpdateConfiguration, JobClass, {
+            ["configuration.statusUpdate.auth" as string]: {
+              $in: jobUpdateInstanceAuthorizationValues,
+            },
+          });
           can(AuthOp.JobStatusUpdateOwner, JobClass, {
             ownerUser: user.username,
           });
           can(AuthOp.JobStatusUpdateOwner, JobClass, {
             ownerGroup: { $in: user.currentGroups },
           });
         } else {
-          const jobUpdateEndPointAuthorizationValues = [
-            ...Object.values(StatusUpdateJobAuth),
-            ...jobUserAuthorizationValues,
-          ];
-          const jobUpdateInstanceAuthorizationValues = [
-            ...Object.values(StatusUpdateJobAuth).filter(
-              (v) => ~String(v).includes("#job"),
-            ),
-            ...jobUserAuthorizationValues,
-          ];
-
           // -------------------------------------
           // endpoint authorization
           if (
             configuration().jobConfiguration.some(
-              (j) => j.statusUpdate.auth! in jobUpdateEndPointAuthorizationValues,
+              (j) =>
+                j.statusUpdate.auth &&
+                jobUpdateEndPointAuthorizationValues.includes(
+                  j.statusUpdate.auth as string,
+                ),
             )
           ) {
             can(AuthOp.JobStatusUpdate, JobClass);
@@ -975,6 +1008,7 @@ export class CaslAbilityFactory {
             ownerGroup: { $in: user.currentGroups },
           });
         }
+        cannot(AuthOp.JobDelete, JobClass);
       }
     }
 

diff --git a/src/common/handlebars-helpers.ts b/src/common/handlebars-helpers.ts
@@ -40,6 +40,7 @@ export const formatCamelCase = (camelCase: string): string => {
   return words;
 };
 
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
 export const jsonify = (context: any): string => {
   return JSON.stringify(context, null, 3);
 };
diff --git a/src/config/configuration.ts b/src/config/configuration.ts
@@ -1,4 +1,3 @@
-import { Logger } from "@nestjs/common";
 import {
   loadJobConfig,
   registerCreateAction,
@@ -31,6 +30,7 @@ const configuration = () => {
 
   const createJobGroups = process.env.CREATE_JOB_GROUPS || ("" as string);
   const statusUpdateJobGroups = process.env.UPDATE_JOB_GROUPS || ("" as string);
+  const deleteJobGroups = process.env.DELETE_JOB_GROUPS || ("" as string);
 
   const proposalGroups = process.env.PROPOSAL_GROUPS || ("" as string);
   const sampleGroups = process.env.SAMPLE_GROUPS || ("#all" as string);
@@ -44,7 +44,8 @@ const configuration = () => {
     process.env.OIDC_USERINFO_MAPPING_FIELD_USERNAME || ("" as string);
 
   const jobConfigurationFile =
-    process.env.JOB_CONFIGURATION_FILE || ("src/jobs/config/jobConfig.example.json" as string);
+    process.env.JOB_CONFIGURATION_FILE ||
+    ("src/jobs/config/jobConfig.example.json" as string);
 
   const defaultLogger = {
     type: "DefaultLogger",
@@ -109,6 +110,7 @@ const configuration = () => {
     datasetCreationValidationRegex: datasetCreationValidationRegex,
     createJobGroups: createJobGroups,
     statusUpdateJobGroups: statusUpdateJobGroups,
+    deleteJobGroups: deleteJobGroups,
     logoutURL: process.env.LOGOUT_URL ?? "", // Example: http://localhost:3000/
     accessGroupsGraphQlConfig: {
       enabled: boolean(process.env?.ACCESS_GROUPS_GRAPHQL_ENABLED || false),

diff --git a/src/elastic-search/elastic-search.service.ts b/src/elastic-search/elastic-search.service.ts
@@ -320,7 +320,7 @@ export class ElasticSearchService implements OnModuleInit {
 
       const totalCount = body.hits.hits.length || 0;
 
-      const data = body.hits.hits.map((item) => item._id);
+      const data = body.hits.hits.map((item) => item._id || "");
       return {
         totalCount,
         data,
@@ -361,13 +361,14 @@ export class ElasticSearchService implements OnModuleInit {
   }
   async updateInsertDocument(data: DatasetDocument) {
     //NOTE: Replace all keys with lower case, also replace spaces and dot with underscore
-    delete data._id;
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const { _id: unused, ...restData } = data; // type-safe delete _id
     const transformedScientificMetadata = transformKeysInObject(
-      data.scientificMetadata as Record<string, unknown>,
+      restData.scientificMetadata as Record<string, unknown>,
     );
 
     const transformedData = {
-      ...data,
+      ...restData,
       scientificMetadata: transformedScientificMetadata,
     };
     try {

diff --git a/src/jobs/actions/emailaction.ts b/src/jobs/actions/emailaction.ts
@@ -9,6 +9,13 @@ import { JobClass } from "../schemas/job.schema";
 import { createTransport, Transporter } from "nodemailer";
 import { compile, TemplateDelegate } from "handlebars";
 
+type MailOptions = {
+  to: string;
+  from: string;
+  subject: string;
+  text?: string;
+};
+
 /**
  * Send an email following a job
  */
@@ -25,7 +32,7 @@ export class EmailJobAction<T> implements JobAction<T> {
     return EmailJobAction.actionType;
   }
 
-  constructor(data: Record<string, any>) {
+  constructor(data: Record<string, unknown>) {
     Logger.log(
       "Initializing EmailJobAction. Params: " + JSON.stringify(data),
       "EmailJobAction",
@@ -40,6 +47,9 @@ export class EmailJobAction<T> implements JobAction<T> {
     if (!data["from"]) {
       throw new NotFoundException("Param 'from' is undefined");
     }
+    if (typeof data["from"] !== "string") {
+      throw new TypeError("from should be a string");
+    }
     if (!data["subject"]) {
       throw new NotFoundException("Param 'subject' is undefined");
     }
@@ -69,7 +79,7 @@ export class EmailJobAction<T> implements JobAction<T> {
     );
 
     // Fill templates
-    const mail: any = {
+    const mail: MailOptions = {
       to: this.toTemplate(job),
       from: this.from,
       subject: this.subjectTemplate(job),

diff --git a/src/jobs/actions/logaction.ts b/src/jobs/actions/logaction.ts
@@ -22,7 +22,7 @@ export class LogJobAction<T> implements JobAction<T> {
     Logger.log("Performing job: " + JSON.stringify(job), "LogJobAction");
   }
 
-  constructor(data: Record<string, any>) {
+  constructor(data: Record<string, unknown>) {
     Logger.log(
       "Initializing LogJobAction. Params: " + JSON.stringify(data),
       "LogJobAction",