feat(jobs): fix ownerUser, contactEmail logic and datasetList validation #1758
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
despadam
changed the title
despadam
changed the title
sofyalaski
approved these changes
Mar 14, 2025
despadam
added a commit
that referenced
this pull request
Mar 21, 2025
* wip of the day implementing jobs * work in progress from scicat marathon in CPH 2023/10/24-25 * Initial suggestions #622 CreateJobDTO and UpdateJobDTO with API decorators etc.. Besides basic functionality already required in the controller some additional suggestions for properties. * fix: removed unnecessary imports * fix eslint complaints * fix typo in job id decription * fix: updated mathJS package version && improved number check * fix eslint complaints * fix typo in job id decription * replace update-job.dto with update-jobstatus.dto Removed update-job.dto as it will not be implemented. Added job-statusupdate.dto. * reviewed version of branch issue621_rel4_4 (by myself, NEEDS REVIEW) * Update job.schema.ts commented schema example * remove optional id from datasets * resolved conflicts * job schema discussion and definition * finish off, leave things out for later * merged latest changes from release-4-4, fixed import in job schema, added job auth enumeration * Initial refactor for job creation and status updates Updated service functions for job creation, status updates and querying. * Change sendJobStartEmail Suggestion for the change to the email at job start. Old version was very specific to archive/retrieve workflows, which should now go to separate microservices instead. * Minor refactor job.schema * updated jobs module * work from the meeting on 2023/12/06 * Remove custom email code from jobs.service * Fix return types from jobs.service Return types from jobs.service actions now match equivalent operations for datasets.service * Initial job config implementation (squashed) - config/jobconfig.ts provides data types and code for reading the configuration json - config/actions implement basic log and email actions - Fix some compilation errors (more remain) - Includes a unit test for config parsing - Implement job actions in the jobs.controller.ts 8 compilation errors remain. In particular, it's currently inconsistent whether `ActionJob.actionType` should be static. * Fix the static actionType compilation errors - JobActionClass specifies static properties required to register the action (eg `static actionType` property) - JobAction specifies instance properties. It adds `getActionType()` with the contract that it should return the same string. - Move oneOrMore to util.ts * Add 'JobOperation' layer in JobConfig data model JobOperation represents create, read, updated, etc. It primarily consists of a list of actions, but adds the ability to configure authentication at this level rather than as a JobAction. * Run lint:fix * Revert job-example change (but other things in this file still need to be updated) * WIP create job * Minor changes documenting job authorization Add jobConfig.json.example, which is intended to be customized and renamed to jobConfig.json as the main job configuration file. * Rename Action enum to AuthOp * schema validation * fixes * fixes + email action param validation * Load job configuration lazily as a singleton * Update comments in authop.enum.ts: change individual to data instance authorization * Job creation: fix missing _id * Update published-data.service.ts backported change from master * Job statusUpdate basic implementation * Validate 'type' instead of 'schema' for job dto * Initial urlaction implementation The test configuration simply calls /health and logs the result. * Add 'jsonify' Handlebars helper * Accept template for URLAction config This is a restrictive implementation; for more on security, see #1113 * Jobs: statusHistory, instance auth based on config, Get functions (#1133) * fixes based on PR 1110 comments * store configuration version * complete instance auth * Adding '@Req() request: Request' * restore jobParams in schema * fix: check authenticated user & job creation message * next draft of job authorization * new version of status update dto * applied improvements from comments * fixed linting * Improved types & implemented job-create interceptor * refactor: type improvement for jobs-aurhotization PR * remove statusHistory from job schema * Remove configVersion and rename statusUpdate variables (#1273) * remove configVersion from job schema, fix statusUpdate vars, update GET/jobs * remove additional filter from findAll, rename to StatusUpdateJobAuth and statusUpdateJobGroups * move configVersion to the top of the config file * Remove unused fields from job schema (#1307) * Jobs should only use the SciCat instance configuration (#1320) * use only global instance configuration * remove print * Job Status Update: Implement RabbitMQ Action (#1284) * add RabbitMQJobAction, fix how job config is stored * rabbitmq queue fixes * fix comment * fixes based on PR comments, register action for job create, send json * finalize rammitmq * 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]> * Improve URLAction templating (#1336) * Add templating to URLAction The jobs configured with a `url` action can now take templates for the `url`, `body` and `headers` fields in the job configuration. * Handlebar helpers: `urlencode` `base64enc` and `job_v3` - Use `{{{urlencode var}}}` for URL components - `base64enc` can be useful for passing data to services - `job_v3` converts the current v4 job schema to a backwards-compatible form. For instance, use `body: "{{{jsonify (job_v3 this)}}}"` to pass a more backwards-compatible json body. * fix linting --------- Co-authored-by: Despina Adamopoulou <[email protected]> * Validate actions during their creation (#1354) * remove validation from actions, store only configVersion inside each job * remove action validation from testing * fix testing * restore configuration in the schema, fix tests * fix linting * fix error message * Store configVersion in the job schema (#1364) * update job schema * store configVersion during job creation * fixes in controller * rename variable * remove job configuration in jobInstance to enable casl authorization (#1395) * wip cherry-picking commits * wip: casl for job instances before adding argument with jobConfig * merged cherry-picked commit that splits casl, jobs authorization requires two arguments, the new being the jobConfig property. Casl for jobs instances is restructred to use it in the if statements instead of configuration being part of jobInstance * fix linting * proposals pass tests --------- Co-authored-by: Sofya Laskina <[email protected]> * Fix error after merging * fix beforeEach in tests after merging * Add jobResultObject field (#1410) * add jobResultObject field * fix linting * fix optional values in statusUpdate dto * Jobs: templating for email action (#1326) * email action wip * fix email templating * remove unnecessary file * add optional auth field * rename field to bodyTemplateFile * fix lint errors * Add JobAction.validate method. This partially reverts commit b218952. - Tightened bounds on DtoType for JobAction - Validate DTOs * Remove job-create interceptor DTO validation is in the controller, so the interceptor isn't needed. * Add jsonpath-plus dependency * Add ValidateAction Also tests and example * Add fullfacet and fullquery for Jobs (#1422) * add fullfacet and fullquery for jobs * fix order of Get endpoints, fullfacet now returns empty array * fix statusCode, statusMessage in tests * add fullquery tests(wip) and endpoint examples, fix timestamps in job schema * finish fullquery,fullfacet tests * fix linting * Fix type error calling validate * Refactor job validate code - Revert the previous commit, which was incorrect and incomplete - Make valiateDTO and performActions generic so they can be shared between endpoints - Separate out checkConfigVersion, which is specific to statusUpdate - Move `getJobTypeConfiguration` calls up so this is only called once * fix issue 1411, empty jobParams allowed (#1433) * Jobs: implement fullfacet (#1436) * implement fullfacet * tests * Add mocha tests for ValidateAction. - Add new test for validate jobs to check for required parameters in both POST and PATCH operations - Remove some unused TestData which didn't match the current Job DTOs - Add 'validate' job to the test jobconfig.json * Add type checking to ValidateAction - rename top-level array to 'request'. This leaves open the possibility to validate other things beside the DTO - now configuration takes the format of `"JSONPath": JSON-Schema`. The schema part is validated with Avj. Note that JSON Schemas can be very concise for simple datatypes, so this doesn't require deep knowledge of how to write JSONSchema. - Update unit & mocha tests * Lint fixes - remove 'any' via typeguard - `npm run lint:fix` - remove duplicate (and outdated) docstring * Lint fixes * Merge branch 'release-jobs' into validateaction * Renumber tests within the 'Validate Job Action' section. * Fix jobconfig.example.json to match ValidateAction * Complete Jobs controller with datasetList checks (#1440) * update controller * update tests * fix jobConfig test * checkDatasetFiles should apply to all job types * Change example to use datasetList * Whitespace changes * Fix checkDatasetFiles after merging with master (#1459) * fix checkDatasetFiles * Fix test value (incorrect following merge) * Fix failing test * dep: Update jsonpath-plus This had a critical vulnerability * dep: add js-yaml dependency * Parse jobconfig as yaml. Example files are deliberately not converted in this commit to test json parsing with js-yaml * Convert jobConfig files to YAML JSON is still accepted as well, but documentation will be updated to use YAML. Move jobConfig.example.yaml to the top-level for better visibility. * fix lint errors * refactor: Implement JobConfig as a NestJS module (#1539) * Complete refactor of job configuration as NestJS modules - Developing new actions is documented in README.md for developers - Move everything related to job config to src/config/job-config - Major refactor of parsing from jobConfig.yaml to JobConfig objects - Create module, interface, and creator for each JobAction - Add handlebar-utils to help consistently applying templates to jobs and Dtos. - In test contexts the handlebars helpers may not be registered, so avoid using custom helpers or register them in the test - Add options to LogJobAction. This is also good for templating tests - Change MailService.sendMail to match nest's MailerService.sendMail - Accept empty jobConfigurationFile as no jobs - CaslModule now depends indirectly on the MailerModule. Thus all controllers need to either mock CaslAbilityFactory or add a mock MailerModule before importing CaslModule. - Most tests disable jobConfig except for those that directly test it. These use test/config/jobconfig.yaml * Support dataset validation in ValidateAction During a `create` operation, the `validate` action can take a `datasets` option listing path/schema pairs that will be applied to any datasets in jobParams.datasetList. Datasets are fetched from the database during the DTO validation step if needed. Validation of archive/retrieve/public lifecycle properties is no longer done automatically, but must be configured in the jobConfig file. Details: - Separate ValidateCreateJobAction (create) and ValidateJobAction (update) - Fix 'Internal Server Error' from job controller if a session timed out. If the session times out then only public jobs will be visible. - Update documentation for validate action - Remove checkDatasetState from the job controller. This must now be configured with a validate action - Add unit test - Remove hard-coded job types and dataset states * Jobs RabbitMQ Action connection info should come from ConfigService (#1575) * extract rabbitmq connection info from configService * update .env and jobConfig examples * rewrite as a common module * Fix circular dependency * fix: delete_job_groups instance authorization (#1534) ## Description When in .env delete-job-group was set to admin, flowing the authorization logic, admin should be able to delete a job. Instead, this user would get denied access. ## Motivation fix this ## Fixes user can delete a job based on delete-job-group ## Changes: * In casl, the endpoint authorization lines for JobDelete are separated into their own if/else clauses * added admin to delete_job_group in tests workflow * 1920 test shour return a successful status code * test for getting are adapted to minus one deleted test. ## Tests included - [x] Included for each change/fix? - [x] Passing? ## Documentation - [ ] swagger documentation updated (required for API changes) - [ ] official documentation updated * Minor fix and better naming for variables (#1612) <!-- Follow semantic-release guidelines for the PR title, which is used in the changelog. Title should follow the format `<type>(<scope>): <subject>`, where - Type is one of: build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test|BREAKING CHANGE - Scope (optional) describes the place of the change (eg a particular milestone) and is usually omitted - subject should be a non-capitalized one-line description in present imperative tense and not ending with a period See https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines for more details. --> ## Description Minor changes in variable names to improve documentation ## Motivation While creating documentation some variables created confusion. ## Fixes <!-- Please provide a list of the issues fixed by this PR --> * Bug fixed (#X) ## Changes: * tests 1750-1860 response for successful calls changed to SuccessfulGetStatusCode instead of Patch * lines 20-93 renamed variables to better represent their meaning. Based on job type and ownership info. * newDataset renamed to newJob in each test ## Tests included - [ ] Included for each change/fix? - [ ] Passing? <!-- Merge will not be approved unless tests pass --> ## Documentation - [ ] swagger documentation updated (required for API changes) - [ ] official documentation updated ### official documentation info <!-- If you have updated the official documentation, please provide PR # and URL of the updated pages --> * Use HTML for emailaction body * Refactor tests based on the "create" field in job config file (#1624) <!-- Follow semantic-release guidelines for the PR title, which is used in the changelog. Title should follow the format `<type>(<scope>): <subject>`, where - Type is one of: build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test|BREAKING CHANGE - Scope (optional) describes the place of the change (eg a particular milestone) and is usually omitted - subject should be a non-capitalized one-line description in present imperative tense and not ending with a period See https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines for more details. --> ## Description `Jobs.js` was extremely long, this PR separates it into smaller files ## Tests included - [ ] Included for each change/fix? - [ ] Passing? <!-- Merge will not be approved unless tests pass --> ## Documentation - [ ] swagger documentation updated (required for API changes) - [x] official documentation updated ### official documentation info <!-- If you have updated the official documentation, please provide PR # and URL of the updated pages --> * RabbitMQ service: fix logging and error handling (#1581) * fix logging and error handling * optional rabbitMQservice initialization * Resolve DatasetsService at runtime. DatasetsService is request scoped, so it can't be added on module init. This also avoids the circular dependency from import datasetService during module initialization. * Fix bug with dataset validation Mongoose documents do some Proxy shenanigans that confuses JSONPath. They should first be converted to simple objects. * Fix mocha tests for jobs following the validateaction changes - Update error messages for archivable, retrievable, and public jobs. These used to be bespoke errors, but now follow the standard 'Invalid request' error message. - Change dataset property validation to throw 409 errors, to be consistent with the previous behavior - Add datasetList to many job DTOs that now validate it - Set archivable/retrievable properly in test datasets * Add tests for job actions (#1644) * url and rabbitmq tests * email unit test * fix failing tests * remove unused vars * fix rabbitmq testing * fix lint * fix and test using default sender value for emails * Fix lint issues * wip: fix after merge, commit c729599 * fix after merge: add accessGroup in jobs controller * fix configuration * wip:broken dependency * fixed after merge * restore direct dependency on ConfigModule * revert unnecessary casting and fix linting * fix unit tests by providing a casl mock * Add comments to validateaction Also removes the type from ValidateFunction. The schema is applied to an arbitrary element of the JobDto or DatasetClass, so the type returned should be `unknown` (the default). * Lint fix * remove datasetsValidation fromJobClass (#1728) * removes the datasetsValidation from jobInstance * fixed linting * remove comment --------- Co-authored-by: Despina Adamopoulou <[email protected]> * Jobs: rename `statusUpdate` to `update` (#1460) * rename statusUpdate to update * linting * fixes after merge * fix config error * Fix bug in validateaction As reported by @Console, adding a job that validated a dataset property resulted in an error 'Invalid options for ValidateJobAction.' * refactor: remove RabbitMQ integration and related logic from App Module (#1754) refactor(app): remove RabbitMQ integration and related logic from AppModule * feat(jobs): fix ownerUser, contactEmail logic and datasetList validation (#1758) * cherry-pick from commit 61299cd * fix failing tests * strengthen datasetList validation * lint * use enum in controller * fix ownerUser * fix ownerUser logic * fix tests * Small fixes responding to review comments - Log the response if a url action returns an error - Remove trailing whitespace from js files * feat(jobs): register rabbitMQ as a conditional module (#1771) register rabbitMQ as a conditional module in app.module * feat(jobs): replace ApiProperty with openapi cli-plugin (#1770) * replace ApiProperty with openapi cli-plugin * make jobResultObject required * fix: permission for ownerUser to read jobs (#1774) * fix permission for ownerUser to read jobs; extract email of admin in testing * fix linting * requested changes * feat(jobs): Ensure backwards compatibility (#1742) * make jobs backwards compatible * - re-introduce v3 DTOs - update create/update interceptors and enforce types - ensure contactEmail does not get overwritten - add tests for api/v3/jobs * fix linting * fix linting * fix failing tests * fix v3 return types * refactor interceptors, fix job update, map executionTime correctly * fix linting * optional emailJobInitiator, fix jobResultObject * update dtos and tests * fix tests * strengthen datasetList validation * update JobClassV3 * lint * use enum in controller * finish tests * improve test descriptions * fix anonymous tests * fix testing * finish tests * fix utils and service for job update * add tests for anonymous jobs from normal user * fix job.v3 schema * job by anonymous user * rename JobClassV3 to OutputJobV3Dto and fix ownerGroup compatibility logic * add descriptions in dtos * fix tests after merging * fix test number * split v3,v4 for fullfacet,delete * fix details in 3 tests * revert change in failing test --------- Co-authored-by: Max Novelli <[email protected]> Co-authored-by: HayenNico <[email protected]> Co-authored-by: Jay Quan <[email protected]> Co-authored-by: Regina Hinzmann <[email protected]> Co-authored-by: Despina Adamopoulou <[email protected]> Co-authored-by: Despina Adamopoulou <[email protected]> Co-authored-by: Sofya Laskina <[email protected]> Co-authored-by: sofyalaski <[email protected]>
Junjiequan
added a commit
that referenced
this pull request
Mar 21, 2025
* wip of the day implementing jobs * work in progress from scicat marathon in CPH 2023/10/24-25 * Initial suggestions #622 CreateJobDTO and UpdateJobDTO with API decorators etc.. Besides basic functionality already required in the controller some additional suggestions for properties. * fix: removed unnecessary imports * fix eslint complaints * fix typo in job id decription * fix: updated mathJS package version && improved number check * fix eslint complaints * fix typo in job id decription * replace update-job.dto with update-jobstatus.dto Removed update-job.dto as it will not be implemented. Added job-statusupdate.dto. * reviewed version of branch issue621_rel4_4 (by myself, NEEDS REVIEW) * Update job.schema.ts commented schema example * remove optional id from datasets * resolved conflicts * job schema discussion and definition * finish off, leave things out for later * merged latest changes from release-4-4, fixed import in job schema, added job auth enumeration * Initial refactor for job creation and status updates Updated service functions for job creation, status updates and querying. * Change sendJobStartEmail Suggestion for the change to the email at job start. Old version was very specific to archive/retrieve workflows, which should now go to separate microservices instead. * Minor refactor job.schema * updated jobs module * work from the meeting on 2023/12/06 * Remove custom email code from jobs.service * Fix return types from jobs.service Return types from jobs.service actions now match equivalent operations for datasets.service * Initial job config implementation (squashed) - config/jobconfig.ts provides data types and code for reading the configuration json - config/actions implement basic log and email actions - Fix some compilation errors (more remain) - Includes a unit test for config parsing - Implement job actions in the jobs.controller.ts 8 compilation errors remain. In particular, it's currently inconsistent whether `ActionJob.actionType` should be static. * Fix the static actionType compilation errors - JobActionClass specifies static properties required to register the action (eg `static actionType` property) - JobAction specifies instance properties. It adds `getActionType()` with the contract that it should return the same string. - Move oneOrMore to util.ts * Add 'JobOperation' layer in JobConfig data model JobOperation represents create, read, updated, etc. It primarily consists of a list of actions, but adds the ability to configure authentication at this level rather than as a JobAction. * Run lint:fix * Revert job-example change (but other things in this file still need to be updated) * WIP create job * Minor changes documenting job authorization Add jobConfig.json.example, which is intended to be customized and renamed to jobConfig.json as the main job configuration file. * Rename Action enum to AuthOp * schema validation * fixes * fixes + email action param validation * Load job configuration lazily as a singleton * Update comments in authop.enum.ts: change individual to data instance authorization * Job creation: fix missing _id * Update published-data.service.ts backported change from master * Job statusUpdate basic implementation * Validate 'type' instead of 'schema' for job dto * Initial urlaction implementation The test configuration simply calls /health and logs the result. * Add 'jsonify' Handlebars helper * Accept template for URLAction config This is a restrictive implementation; for more on security, see #1113 * Jobs: statusHistory, instance auth based on config, Get functions (#1133) * fixes based on PR 1110 comments * store configuration version * complete instance auth * Adding '@Req() request: Request' * restore jobParams in schema * fix: check authenticated user & job creation message * next draft of job authorization * new version of status update dto * applied improvements from comments * fixed linting * Improved types & implemented job-create interceptor * refactor: type improvement for jobs-aurhotization PR * remove statusHistory from job schema * Remove configVersion and rename statusUpdate variables (#1273) * remove configVersion from job schema, fix statusUpdate vars, update GET/jobs * remove additional filter from findAll, rename to StatusUpdateJobAuth and statusUpdateJobGroups * move configVersion to the top of the config file * Remove unused fields from job schema (#1307) * Jobs should only use the SciCat instance configuration (#1320) * use only global instance configuration * remove print * Job Status Update: Implement RabbitMQ Action (#1284) * add RabbitMQJobAction, fix how job config is stored * rabbitmq queue fixes * fix comment * fixes based on PR comments, register action for job create, send json * finalize rammitmq * 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]> * Improve URLAction templating (#1336) * Add templating to URLAction The jobs configured with a `url` action can now take templates for the `url`, `body` and `headers` fields in the job configuration. * Handlebar helpers: `urlencode` `base64enc` and `job_v3` - Use `{{{urlencode var}}}` for URL components - `base64enc` can be useful for passing data to services - `job_v3` converts the current v4 job schema to a backwards-compatible form. For instance, use `body: "{{{jsonify (job_v3 this)}}}"` to pass a more backwards-compatible json body. * fix linting --------- Co-authored-by: Despina Adamopoulou <[email protected]> * Validate actions during their creation (#1354) * remove validation from actions, store only configVersion inside each job * remove action validation from testing * fix testing * restore configuration in the schema, fix tests * fix linting * fix error message * Store configVersion in the job schema (#1364) * update job schema * store configVersion during job creation * fixes in controller * rename variable * remove job configuration in jobInstance to enable casl authorization (#1395) * wip cherry-picking commits * wip: casl for job instances before adding argument with jobConfig * merged cherry-picked commit that splits casl, jobs authorization requires two arguments, the new being the jobConfig property. Casl for jobs instances is restructred to use it in the if statements instead of configuration being part of jobInstance * fix linting * proposals pass tests --------- Co-authored-by: Sofya Laskina <[email protected]> * Fix error after merging * fix beforeEach in tests after merging * Add jobResultObject field (#1410) * add jobResultObject field * fix linting * fix optional values in statusUpdate dto * Jobs: templating for email action (#1326) * email action wip * fix email templating * remove unnecessary file * add optional auth field * rename field to bodyTemplateFile * fix lint errors * Add JobAction.validate method. This partially reverts commit b218952. - Tightened bounds on DtoType for JobAction - Validate DTOs * Remove job-create interceptor DTO validation is in the controller, so the interceptor isn't needed. * Add jsonpath-plus dependency * Add ValidateAction Also tests and example * Add fullfacet and fullquery for Jobs (#1422) * add fullfacet and fullquery for jobs * fix order of Get endpoints, fullfacet now returns empty array * fix statusCode, statusMessage in tests * add fullquery tests(wip) and endpoint examples, fix timestamps in job schema * finish fullquery,fullfacet tests * fix linting * Fix type error calling validate * Refactor job validate code - Revert the previous commit, which was incorrect and incomplete - Make valiateDTO and performActions generic so they can be shared between endpoints - Separate out checkConfigVersion, which is specific to statusUpdate - Move `getJobTypeConfiguration` calls up so this is only called once * fix issue 1411, empty jobParams allowed (#1433) * Jobs: implement fullfacet (#1436) * implement fullfacet * tests * Add mocha tests for ValidateAction. - Add new test for validate jobs to check for required parameters in both POST and PATCH operations - Remove some unused TestData which didn't match the current Job DTOs - Add 'validate' job to the test jobconfig.json * Add type checking to ValidateAction - rename top-level array to 'request'. This leaves open the possibility to validate other things beside the DTO - now configuration takes the format of `"JSONPath": JSON-Schema`. The schema part is validated with Avj. Note that JSON Schemas can be very concise for simple datatypes, so this doesn't require deep knowledge of how to write JSONSchema. - Update unit & mocha tests * Lint fixes - remove 'any' via typeguard - `npm run lint:fix` - remove duplicate (and outdated) docstring * Lint fixes * Merge branch 'release-jobs' into validateaction * Renumber tests within the 'Validate Job Action' section. * Fix jobconfig.example.json to match ValidateAction * Complete Jobs controller with datasetList checks (#1440) * update controller * update tests * fix jobConfig test * checkDatasetFiles should apply to all job types * Change example to use datasetList * Whitespace changes * Fix checkDatasetFiles after merging with master (#1459) * fix checkDatasetFiles * Fix test value (incorrect following merge) * Fix failing test * dep: Update jsonpath-plus This had a critical vulnerability * dep: add js-yaml dependency * Parse jobconfig as yaml. Example files are deliberately not converted in this commit to test json parsing with js-yaml * Convert jobConfig files to YAML JSON is still accepted as well, but documentation will be updated to use YAML. Move jobConfig.example.yaml to the top-level for better visibility. * fix lint errors * refactor: Implement JobConfig as a NestJS module (#1539) * Complete refactor of job configuration as NestJS modules - Developing new actions is documented in README.md for developers - Move everything related to job config to src/config/job-config - Major refactor of parsing from jobConfig.yaml to JobConfig objects - Create module, interface, and creator for each JobAction - Add handlebar-utils to help consistently applying templates to jobs and Dtos. - In test contexts the handlebars helpers may not be registered, so avoid using custom helpers or register them in the test - Add options to LogJobAction. This is also good for templating tests - Change MailService.sendMail to match nest's MailerService.sendMail - Accept empty jobConfigurationFile as no jobs - CaslModule now depends indirectly on the MailerModule. Thus all controllers need to either mock CaslAbilityFactory or add a mock MailerModule before importing CaslModule. - Most tests disable jobConfig except for those that directly test it. These use test/config/jobconfig.yaml * Support dataset validation in ValidateAction During a `create` operation, the `validate` action can take a `datasets` option listing path/schema pairs that will be applied to any datasets in jobParams.datasetList. Datasets are fetched from the database during the DTO validation step if needed. Validation of archive/retrieve/public lifecycle properties is no longer done automatically, but must be configured in the jobConfig file. Details: - Separate ValidateCreateJobAction (create) and ValidateJobAction (update) - Fix 'Internal Server Error' from job controller if a session timed out. If the session times out then only public jobs will be visible. - Update documentation for validate action - Remove checkDatasetState from the job controller. This must now be configured with a validate action - Add unit test - Remove hard-coded job types and dataset states * Jobs RabbitMQ Action connection info should come from ConfigService (#1575) * extract rabbitmq connection info from configService * update .env and jobConfig examples * rewrite as a common module * Fix circular dependency * fix: delete_job_groups instance authorization (#1534) ## Description When in .env delete-job-group was set to admin, flowing the authorization logic, admin should be able to delete a job. Instead, this user would get denied access. ## Motivation fix this ## Fixes user can delete a job based on delete-job-group ## Changes: * In casl, the endpoint authorization lines for JobDelete are separated into their own if/else clauses * added admin to delete_job_group in tests workflow * 1920 test shour return a successful status code * test for getting are adapted to minus one deleted test. ## Tests included - [x] Included for each change/fix? - [x] Passing? ## Documentation - [ ] swagger documentation updated (required for API changes) - [ ] official documentation updated * Minor fix and better naming for variables (#1612) <!-- Follow semantic-release guidelines for the PR title, which is used in the changelog. Title should follow the format `<type>(<scope>): <subject>`, where - Type is one of: build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test|BREAKING CHANGE - Scope (optional) describes the place of the change (eg a particular milestone) and is usually omitted - subject should be a non-capitalized one-line description in present imperative tense and not ending with a period See https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines for more details. --> ## Description Minor changes in variable names to improve documentation ## Motivation While creating documentation some variables created confusion. ## Fixes <!-- Please provide a list of the issues fixed by this PR --> * Bug fixed (#X) ## Changes: * tests 1750-1860 response for successful calls changed to SuccessfulGetStatusCode instead of Patch * lines 20-93 renamed variables to better represent their meaning. Based on job type and ownership info. * newDataset renamed to newJob in each test ## Tests included - [ ] Included for each change/fix? - [ ] Passing? <!-- Merge will not be approved unless tests pass --> ## Documentation - [ ] swagger documentation updated (required for API changes) - [ ] official documentation updated ### official documentation info <!-- If you have updated the official documentation, please provide PR # and URL of the updated pages --> * Use HTML for emailaction body * Refactor tests based on the "create" field in job config file (#1624) <!-- Follow semantic-release guidelines for the PR title, which is used in the changelog. Title should follow the format `<type>(<scope>): <subject>`, where - Type is one of: build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test|BREAKING CHANGE - Scope (optional) describes the place of the change (eg a particular milestone) and is usually omitted - subject should be a non-capitalized one-line description in present imperative tense and not ending with a period See https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#-git-commit-guidelines for more details. --> ## Description `Jobs.js` was extremely long, this PR separates it into smaller files ## Tests included - [ ] Included for each change/fix? - [ ] Passing? <!-- Merge will not be approved unless tests pass --> ## Documentation - [ ] swagger documentation updated (required for API changes) - [x] official documentation updated ### official documentation info <!-- If you have updated the official documentation, please provide PR # and URL of the updated pages --> * RabbitMQ service: fix logging and error handling (#1581) * fix logging and error handling * optional rabbitMQservice initialization * Resolve DatasetsService at runtime. DatasetsService is request scoped, so it can't be added on module init. This also avoids the circular dependency from import datasetService during module initialization. * Fix bug with dataset validation Mongoose documents do some Proxy shenanigans that confuses JSONPath. They should first be converted to simple objects. * Fix mocha tests for jobs following the validateaction changes - Update error messages for archivable, retrievable, and public jobs. These used to be bespoke errors, but now follow the standard 'Invalid request' error message. - Change dataset property validation to throw 409 errors, to be consistent with the previous behavior - Add datasetList to many job DTOs that now validate it - Set archivable/retrievable properly in test datasets * Add tests for job actions (#1644) * url and rabbitmq tests * email unit test * fix failing tests * remove unused vars * fix rabbitmq testing * fix lint * fix and test using default sender value for emails * Fix lint issues * wip: fix after merge, commit c729599 * fix after merge: add accessGroup in jobs controller * fix configuration * wip:broken dependency * fixed after merge * restore direct dependency on ConfigModule * revert unnecessary casting and fix linting * fix unit tests by providing a casl mock * Add comments to validateaction Also removes the type from ValidateFunction. The schema is applied to an arbitrary element of the JobDto or DatasetClass, so the type returned should be `unknown` (the default). * Lint fix * remove datasetsValidation fromJobClass (#1728) * removes the datasetsValidation from jobInstance * fixed linting * remove comment --------- Co-authored-by: Despina Adamopoulou <[email protected]> * Jobs: rename `statusUpdate` to `update` (#1460) * rename statusUpdate to update * linting * fixes after merge * fix config error * Fix bug in validateaction As reported by @Console, adding a job that validated a dataset property resulted in an error 'Invalid options for ValidateJobAction.' * refactor: remove RabbitMQ integration and related logic from App Module (#1754) refactor(app): remove RabbitMQ integration and related logic from AppModule * feat(jobs): fix ownerUser, contactEmail logic and datasetList validation (#1758) * cherry-pick from commit 61299cd * fix failing tests * strengthen datasetList validation * lint * use enum in controller * fix ownerUser * fix ownerUser logic * fix tests * Small fixes responding to review comments - Log the response if a url action returns an error - Remove trailing whitespace from js files * feat(jobs): register rabbitMQ as a conditional module (#1771) register rabbitMQ as a conditional module in app.module * feat(jobs): replace ApiProperty with openapi cli-plugin (#1770) * replace ApiProperty with openapi cli-plugin * make jobResultObject required * fix: permission for ownerUser to read jobs (#1774) * fix permission for ownerUser to read jobs; extract email of admin in testing * fix linting * requested changes * feat(jobs): Ensure backwards compatibility (#1742) * make jobs backwards compatible * - re-introduce v3 DTOs - update create/update interceptors and enforce types - ensure contactEmail does not get overwritten - add tests for api/v3/jobs * fix linting * fix linting * fix failing tests * fix v3 return types * refactor interceptors, fix job update, map executionTime correctly * fix linting * optional emailJobInitiator, fix jobResultObject * update dtos and tests * fix tests * strengthen datasetList validation * update JobClassV3 * lint * use enum in controller * finish tests * improve test descriptions * fix anonymous tests * fix testing * finish tests * fix utils and service for job update * add tests for anonymous jobs from normal user * fix job.v3 schema * job by anonymous user * rename JobClassV3 to OutputJobV3Dto and fix ownerGroup compatibility logic * add descriptions in dtos * fix tests after merging * fix test number * split v3,v4 for fullfacet,delete * fix details in 3 tests * revert change in failing test --------- Co-authored-by: Max Novelli <[email protected]> Co-authored-by: HayenNico <[email protected]> Co-authored-by: Jay Quan <[email protected]> Co-authored-by: Regina Hinzmann <[email protected]> Co-authored-by: Despina Adamopoulou <[email protected]> Co-authored-by: Despina Adamopoulou <[email protected]> Co-authored-by: Sofya Laskina <[email protected]> Co-authored-by: sofyalaski <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
ownerUser,ownerGroup,contactEmailinCreateJobDtowere not typescript optional fields even thought they had the@IsOptional()annotationinstanceAuthorizationJobCreatelogic not covering allcontactEmailcasesdatasetListwas not deeply validatedMotivation
Topics that came up while working on #1742
Fixes
ownerUser,ownerGroup,contactEmailinCreateJobDtoare now also typescript optional fieldsdatasetListvalidation to ensure thatpidis a string andfilesis an arrayChanges:
instanceAuthorizationJobCreatelogicadminmode:ownerUseris provided and is the same as admin then the owner is the admin, otherwise if a different user is provided then we search for this user and assign the retrieved username as owner, unless it cannot be found in which case we set the admin as ownerownerUserandownerGroupare not provided, thencontactEmailis required to be provided via the dtocontactEmailwhen that is provided via the dto. If it is not provided then we fall back to theownerUseremail and ifownerUserdoes not exist then thecontactEmailis set to adminusermode:contactEmailis prioritized when providedAll tests where updated accordingly.
Tests included
Documentation
official documentation info
sbliven/scicat-documentation#2