From af64aed0ec91e05b8297351b67292710673b06e9 Mon Sep 17 00:00:00 2001 From: minottic Date: Fri, 1 May 2026 12:39:52 +0000 Subject: [PATCH] fix: rewrite instance access func with proper auth It fixes the bugs with auth when submitting jobs by relying solely on user groups rather than job fields --- src/config/configuration.ts | 10 +- src/jobs/jobs.controller.utils.ts | 441 ++++++++++++------------------ test/JobsAll.js | 10 +- test/JobsDatasetAccess.js | 10 +- test/JobsDatasetOwner.js | 109 +++++++- test/JobsDatasetPublic.js | 6 +- test/JobsSpecificGroup.js | 2 +- test/JobsV3.js | 4 +- test/TestData.js | 2 + 9 files changed, 302 insertions(+), 292 deletions(-) diff --git a/src/config/configuration.ts b/src/config/configuration.ts index 757736c68..31cbecae9 100644 --- a/src/config/configuration.ts +++ b/src/config/configuration.ts @@ -277,9 +277,13 @@ const configuration = () => { attachmentPrivileged: attachmentPrivilegedGroups .split(",") .map((v) => v.trim()), - createJobPrivileged: createJobPrivilegedGroups, - updateJobPrivileged: updateJobPrivilegedGroups, - deleteJob: deleteJobGroups, + createJobPrivileged: createJobPrivilegedGroups + .split(",") + .map((v) => v.trim()), + updateJobPrivileged: updateJobPrivilegedGroups + .split(",") + .map((v) => v.trim()), + deleteJob: deleteJobGroups.split(",").map((v) => v.trim()), }, datasetCreationValidationEnabled: boolean(datasetCreationValidationEnabled), datasetCreationValidationRegex: datasetCreationValidationRegex, diff --git a/src/jobs/jobs.controller.utils.ts b/src/jobs/jobs.controller.utils.ts index 72bd0f6b4..2e97ac106 100644 --- a/src/jobs/jobs.controller.utils.ts +++ b/src/jobs/jobs.controller.utils.ts @@ -3,9 +3,11 @@ import { HttpStatus, HttpException, ForbiddenException, + UnauthorizedException, + UnprocessableEntityException, } from "@nestjs/common"; import { Request } from "express"; -import { FilterQuery } from "mongoose"; +import { Condition, FilterQuery } from "mongoose"; import * as jmp from "json-merge-patch"; import { JobsService } from "./jobs.service"; import { CreateJobDto } from "./dto/create-job.dto"; @@ -40,11 +42,14 @@ import { } from "./dto/output-job-v4.dto"; import { toObject } from "src/config/job-config/actions/actionutils"; import { loadDatasets } from "src/config/job-config/actions/actionutils"; +import { DatasetClass } from "src/datasets/schemas/dataset.schema"; @Injectable() export class JobsControllerUtils { jobDatasetAuthorization: Array = []; private accessGroups; + adminGroups: Set = new Set(); + createJobPrivilegedGroups: Set = new Set(); constructor( private readonly jobsService: JobsService, @@ -60,6 +65,10 @@ export class JobsControllerUtils { ); this.accessGroups = this.configService.get("accessGroups"); + this.adminGroups = new Set(this.accessGroups?.admin ?? []); + this.createJobPrivilegedGroups = new Set( + this.accessGroups?.createJobPrivileged ?? [], + ); } /** @@ -281,24 +290,17 @@ export class JobsControllerUtils { return jobConfig; }; - /** - * Checking if user is allowed to create job according to auth field of job configuration - */ - async instanceAuthorizationJobCreate( + private initJobInstance( jobCreateDto: CreateJobDto, - user: JWTUser, - ): Promise { - // NOTE: We need JobClass instance because casl module works only on that. - // If other fields are needed can be added later. + jobConfiguration: JobConfig, + datasetList: DatasetListDto[], + ): JobClass { const jobInstance = new JobClass(); - const jobConfiguration = this.getJobTypeConfiguration(jobCreateDto.type); jobInstance._id = ""; jobInstance.accessGroups = []; jobInstance.type = jobCreateDto.type; - if (jobCreateDto.contactEmail) { + if (jobCreateDto.contactEmail) jobInstance.contactEmail = jobCreateDto.contactEmail; - } - // check if jobStatusMessage was provided via v3 and remove it from jobParams const { jobStatusMessage, ...cleanJobParams } = jobCreateDto.jobParams; jobInstance.jobParams = jobStatusMessage ? cleanJobParams @@ -312,274 +314,183 @@ export class JobsControllerUtils { jobInstance.statusMessage = (jobStatusMessage as string) || this.configService.get("jobDefaultStatusMessage")!; + if (JobParams.DatasetList in jobCreateDto.jobParams) + jobInstance.jobParams[JobParams.DatasetList] = datasetList; + if (jobCreateDto.ownerGroup) + jobInstance.ownerGroup = jobCreateDto.ownerGroup; + return jobInstance; + } - // validate datasetList, if it exists in jobParams - let datasetList: DatasetListDto[] = []; - let datasetsNoAccess = 0; - if (JobParams.DatasetList in jobCreateDto.jobParams) { - datasetList = await this.validateDatasetList(jobCreateDto.jobParams); - jobInstance.jobParams = { - ...jobInstance.jobParams, - [JobParams.DatasetList]: datasetList, - }; - } - let jobUser: JWTUser | null = null; - if (user) { - // the request comes from a user who is logged in. - if ( - user.currentGroups.some((g) => this.accessGroups?.admin.includes(g)) || - user.currentGroups.some((g) => - this.accessGroups?.createJobPrivileged.includes(g), - ) - ) { - // admin users and users in CREATE_JOB_PRIVILEGED group - if (jobCreateDto.ownerUser) { - if (user.username != jobCreateDto.ownerUser) { - jobUser = await this.usersService.findByUsername2JWTUser( - jobCreateDto.ownerUser, - ); - if (jobUser === null) { - Logger.log( - "Owner user was not found, using current user instead.", - "instanceAuthorizationJobCreate", - ); - } - jobInstance.ownerUser = - (jobUser?.username as string) ?? user.username; - } else { - jobInstance.ownerUser = user.username; - } - } - if (jobCreateDto.ownerGroup) { - // TODO?: ensure that the provided ownerGroup exists - jobInstance.ownerGroup = jobCreateDto.ownerGroup; - } - if ( - !jobCreateDto.ownerGroup && - !jobCreateDto.ownerUser && - !jobCreateDto.contactEmail - ) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: - "Contact email should be specified for an anonymous job.", - }, - HttpStatus.BAD_REQUEST, - ); - } - // prioritize jobCreateDto.contactEmail for anonymous users - jobInstance.contactEmail = - jobCreateDto.contactEmail ?? (jobUser?.email as string) ?? user.email; - } else { - // check if we have ownerGroup - if (!jobCreateDto.ownerGroup) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: `Invalid new job. Owner group should be specified.`, - }, - HttpStatus.BAD_REQUEST, - ); - } - // check that job user matches the user placing the request, if job user is specified - if (jobCreateDto.ownerUser && jobCreateDto.ownerUser != user.username) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: `Invalid new job. User owning the job should match user logged in.`, - }, - HttpStatus.BAD_REQUEST, - ); - } - jobInstance.ownerUser = user.username; - jobInstance.contactEmail = jobCreateDto.contactEmail ?? user.email; - // check that ownerGroup is one of the user groups - if (!user.currentGroups.includes(jobCreateDto.ownerGroup)) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: `Invalid new job. User needs to belong to job owner group.`, - }, - HttpStatus.BAD_REQUEST, - ); - } - jobInstance.ownerGroup = jobCreateDto.ownerGroup; - } - } - - if ( - jobConfiguration.create.auth && - Object.values(this.jobDatasetAuthorization).includes( - jobConfiguration.create.auth, - ) - ) { - // check that jobParams are passed for #dataset jobs - if (!(JobParams.DatasetList in jobCreateDto.jobParams)) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: "Dataset ids list was not provided in jobParams", - }, - HttpStatus.BAD_REQUEST, - ); - } - // verify that the user meet the requested permissions on the datasets listed - // build the condition - type FieldFilter = { $eq?: string; $in?: string[] }; - type BasicCondition = { [field: string]: FieldFilter | boolean }; - - type LogicalCondition = - | { $and: BasicCondition[] } - | { $or: BasicCondition[] }; - - interface datasetsWhere { - where: { - pid: { $in: string[] }; - isPublished?: boolean; - ownerGroup?: FieldFilter; - $or?: (BasicCondition | LogicalCondition)[]; - }; - } + private isAdminUser(user: JWTUser | null): boolean { + return !!(user && user.currentGroups.some((g) => this.adminGroups.has(g))); + } - const datasetIds = datasetList.map((x) => x.pid); - const datasetsWhere: datasetsWhere = { - where: { - pid: { $in: datasetIds }, - }, - }; - if (jobConfiguration.create.auth === "#datasetPublic") { - datasetsWhere["where"]["isPublished"] = true; - } else if (jobConfiguration.create.auth === "#datasetAccess") { - // jobAdmin creates job for someone and ownerUser not specified, only ownerGroup or - // user creating the job and ownerUser are the same or - // ownerUser specified in the DTO is part of ownerGroup specified in the DTO - if ( - (!jobUser && jobInstance.ownerGroup) || - (jobUser && user.username === jobUser.username) || - (jobUser && jobUser.currentGroups.includes(jobInstance.ownerGroup)) - ) { - datasetsWhere["where"]["$or"] = [ - { ownerGroup: { $eq: jobInstance.ownerGroup } }, - { accessGroups: { $eq: jobInstance.ownerGroup } }, - { isPublished: true }, - ]; - } else if (jobUser && !jobInstance.ownerGroup) { - // job for user with no ownerGroup specified - datasetsWhere["where"]["$or"] = [ - { ownerGroup: { $in: jobUser.currentGroups } }, - { accessGroups: { $in: jobUser.currentGroups } }, - { isPublished: true }, - ]; - } - // job for different user and group - else if ( - jobUser && - !jobUser.currentGroups.includes(jobInstance.ownerGroup) - ) { - // check that both the user and group have access to datasets - datasetsWhere["where"]["$or"] = [ - { - $and: [ - { ownerGroup: { $eq: jobInstance.ownerGroup } }, - { ownerGroup: { $in: jobUser.currentGroups } }, - ], - }, - { - $and: [ - { accessGroups: { $eq: jobInstance.ownerGroup } }, - { accessGroups: { $in: jobUser.currentGroups } }, - ], - }, - { isPublished: true }, - ]; - } else { - // job for anonymous user - datasetsWhere["where"]["isPublished"] = true; - } - } else if (jobConfiguration.create.auth === "#datasetOwner") { - if ( - !user || - (!user.currentGroups.some((g) => - this.accessGroups?.admin.includes(g), - ) && - !jobCreateDto.ownerGroup && - !jobCreateDto.ownerUser) - ) { - throw new HttpException( - { - status: HttpStatus.UNAUTHORIZED, - message: "User not authenticated", - }, - HttpStatus.UNAUTHORIZED, - ); - } + private isJobCreationPrivilegedUser(user: JWTUser | null): boolean { + return !!( + user && + user.currentGroups.some((g) => this.createJobPrivilegedGroups.has(g)) + ); + } - if ( - (!jobUser && jobInstance.ownerGroup) || - (jobUser && user.username === jobUser.username) || - (jobUser && jobUser.currentGroups.includes(jobInstance.ownerGroup)) - ) { - datasetsWhere["where"]["ownerGroup"] = { - $eq: jobInstance.ownerGroup, - }; - } else if (jobUser && !jobInstance.ownerGroup) { - // job for user with no ownerGroup specified - datasetsWhere["where"]["ownerGroup"] = { $in: jobUser.currentGroups }; - } else if ( - // job for different user and group - jobUser && - !jobUser.currentGroups.includes(jobInstance.ownerGroup) - ) { - // check that both the user and group have access to datasets - datasetsWhere["where"]["$or"] = [ - { - $and: [ - { ownerGroup: { $eq: jobInstance.ownerGroup } }, - { ownerGroup: { $in: jobUser.currentGroups } }, - ], - }, - ]; - } else { - // job for anonymous user is always faulty, because job id cannot be empty - datasetsWhere["where"]["$or"] = [{ _id: { $in: [] } }]; - } - } - const numberOfDatasetsWithAccess = - await this.datasetsService.count(datasetsWhere); - datasetsNoAccess = datasetIds.length - numberOfDatasetsWithAccess.count; - } + private isPrivilegedUser(user: JWTUser | null): boolean { + return this.isAdminUser(user) || this.isJobCreationPrivilegedUser(user); + } - if (!user && jobCreateDto.ownerGroup) { - throw new HttpException( - { - status: HttpStatus.BAD_REQUEST, - message: `Invalid new job. Unauthenticated user cannot initiate a job owned by another user.`, - }, - HttpStatus.BAD_REQUEST, + /** + * Checking if user is allowed to create job according to auth field of job configuration + */ + async instanceAuthorizationJobCreate( + jobCreateDto: CreateJobDto, + user: JWTUser, + ): Promise { + // NOTE: We need JobClass instance because casl module works only on that. + // If other fields are needed can be added later. + const jobConfiguration = this.getJobTypeConfiguration(jobCreateDto.type); + const datasetList = + JobParams.DatasetList in jobCreateDto.jobParams + ? await this.validateDatasetList(jobCreateDto.jobParams) + : []; + const jobInstance = this.initJobInstance( + jobCreateDto, + jobConfiguration, + datasetList, + ); + const jobUser = await this.processJobUser(user, jobCreateDto, jobInstance); + await this.checkDatasetsAccess( + jobConfiguration, + jobCreateDto, + datasetList, + user, + jobUser, + ); + if (!user && jobCreateDto.ownerGroup) + throw new ForbiddenException( + "Invalid new job. Unauthenticated user cannot initiate a job owned by another user.", ); - } - - // instantiate the casl matrix for the user const ability = this.caslAbilityFactory.jobsInstanceAccess( user, jobConfiguration, ); - // check if the user can create this job const canCreate = - (ability.can(Action.JobCreateAny, JobClass) && - user.currentGroups.some((g) => this.accessGroups?.admin.includes(g))) || - (ability.can(Action.JobCreateAny, JobClass) && datasetsNoAccess == 0) || + (user?.currentGroups ?? []).some((g) => this.adminGroups.has(g)) || + ability.can(Action.JobCreateAny, JobClass) || ability.can(Action.JobCreateOwner, jobInstance) || (ability.can(Action.JobCreateConfiguration, jobInstance) && - datasetsNoAccess == 0 && jobConfiguration.create.auth != CreateJobAuth.JobAdmin); - - if (!canCreate) { + if (!canCreate) throw new ForbiddenException("Unauthorized to create this job."); + return jobInstance; + } + + private async processJobUser( + user: JWTUser, + jobCreateDto: CreateJobDto, + jobInstance: JobClass, + ) { + if (!user) return null; + let jobUser: JWTUser | null = user; + const userGroups = new Set(user?.currentGroups ?? []); + if (this.isPrivilegedUser(user)) { + if ( + !jobCreateDto.ownerGroup && + !jobCreateDto.ownerUser && + !jobCreateDto.contactEmail + ) { + throw new UnprocessableEntityException( + "Contact email should be specified for an anonymous job.", + ); + } + // admin users and users in CREATE_JOB_PRIVILEGED group can specify any ownerUser + if (jobCreateDto.ownerUser && jobCreateDto.ownerUser !== user.username) { + jobUser = await this.usersService.findByUsername2JWTUser( + jobCreateDto.ownerUser, + ); + if (jobUser === null) + Logger.log( + "Owner user was not found, using current user instead.", + "instanceAuthorizationJobCreate", + ); + jobInstance.ownerUser = (jobUser?.username as string) ?? user.username; + } else if (jobCreateDto.ownerUser) { + jobInstance.ownerUser = user.username; + } else jobUser = null; + } else { + // non-privileged users can only specify ownerUser as themselves and ownerGroup that they belong to + if (!jobCreateDto.ownerGroup) + throw new ForbiddenException( + "Invalid new job. Owner group should be specified.", + ); + if (jobCreateDto.ownerUser && jobCreateDto.ownerUser !== user.username) + throw new ForbiddenException( + "Invalid new job. User owning the job should match user logged in.", + ); + if (!userGroups.has(jobCreateDto.ownerGroup)) + throw new ForbiddenException( + "Invalid new job. User needs to belong to job owner group.", + ); + jobInstance.ownerUser = user.username; } + jobInstance.contactEmail = + jobInstance.contactEmail ?? jobUser?.email ?? user.email; + return jobUser; + } - return jobInstance; + private async checkDatasetsAccess( + jobConfiguration: JobConfig, + jobCreateDto: CreateJobDto, + datasetList: DatasetListDto[], + user: JWTUser, + jobUser: JWTUser | null, + ) { + if (this.isAdminUser(user)) return; + if ( + !( + jobConfiguration.create.auth && + Object.values(this.jobDatasetAuthorization).includes( + jobConfiguration.create.auth, + ) + ) + ) + return; + if (!jobCreateDto.jobParams[JobParams.DatasetList]) + throw new UnprocessableEntityException( + "Dataset ids list was not provided in jobParams", + ); + const datasetsWhere: { where: Condition } = { + where: { + pid: { $in: datasetList.map((x) => x.pid) }, + }, + }; + const requestUserGroups = this.isJobCreationPrivilegedUser(user) + ? (jobUser?.currentGroups ?? []) + : (user?.currentGroups ?? []); + if (jobConfiguration.create.auth === CreateJobAuth.DatasetPublic) + datasetsWhere.where.isPublished = true; + else if (jobConfiguration.create.auth === CreateJobAuth.DatasetAccess) { + if (requestUserGroups.length === 0) + datasetsWhere.where.isPublished = true; + else + datasetsWhere.where.$or = [ + { ownerGroup: { $in: requestUserGroups } }, + { accessGroups: { $in: requestUserGroups } }, + { isPublished: true }, + ]; + } else if (jobConfiguration.create.auth === CreateJobAuth.DatasetOwner) { + if (!user) throw new UnauthorizedException("User not authenticated"); + if (requestUserGroups.length === 0) + throw new ForbiddenException( + "User does not belong to any group, cannot create job with #datasetOwner authorization.", + ); + datasetsWhere.where.ownerGroup = { $in: requestUserGroups }; + } else { + datasetsWhere.where.isPublished = true; + } + const numberOfDatasetsWithAccess = + await this.datasetsService.count(datasetsWhere); + if (numberOfDatasetsWithAccess.count < datasetList.length) + throw new ForbiddenException( + "User does not have access to all datasets, cannot create job.", + ); } /** diff --git a/test/JobsAll.js b/test/JobsAll.js index a5de11065..11a2a2f40 100644 --- a/test/JobsAll.js +++ b/test/JobsAll.js @@ -356,7 +356,7 @@ describe("1120: Jobs: Test New Job Model Authorization for all_access jobs type" .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenAdmin}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.UnprocessableEntityStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); @@ -620,7 +620,7 @@ describe("1120: Jobs: Test New Job Model Authorization for all_access jobs type" .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); @@ -646,7 +646,7 @@ describe("1120: Jobs: Test New Job Model Authorization for all_access jobs type" .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); @@ -671,7 +671,7 @@ describe("1120: Jobs: Test New Job Model Authorization for all_access jobs type" .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); @@ -716,7 +716,7 @@ describe("1120: Jobs: Test New Job Model Authorization for all_access jobs type" .post("/api/v4/Jobs") .send(newJob) .set("Accept", "application/json") - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); diff --git a/test/JobsDatasetAccess.js b/test/JobsDatasetAccess.js index 635f5d9a4..28f36c6ed 100644 --- a/test/JobsDatasetAccess.js +++ b/test/JobsDatasetAccess.js @@ -344,7 +344,7 @@ describe("1140: Jobs: Test New Job Model Authorization for dataset_access jobs t res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -425,7 +425,7 @@ describe("1140: Jobs: Test New Job Model Authorization for dataset_access jobs t res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -471,7 +471,7 @@ describe("1140: Jobs: Test New Job Model Authorization for dataset_access jobs t res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -520,7 +520,7 @@ describe("1140: Jobs: Test New Job Model Authorization for dataset_access jobs t res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -579,7 +579,7 @@ describe("1140: Jobs: Test New Job Model Authorization for dataset_access jobs t res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); diff --git a/test/JobsDatasetOwner.js b/test/JobsDatasetOwner.js index d807bebc7..29d9d96cf 100644 --- a/test/JobsDatasetOwner.js +++ b/test/JobsDatasetOwner.js @@ -7,11 +7,14 @@ let accessTokenAdminIngestor = null, accessTokenUser2 = null, accessTokenUser3 = null, accessTokenUser51 = null, + accessTokenUser6 = null, accessTokenAdmin = null, datasetPid1 = null, datasetPid2 = null, datasetPid3 = null, + datasetPidGroup6 = null, + datasetPidUser6 = null, jobId1 = null, encodedJobOwnedByAdmin = null, @@ -91,6 +94,11 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ username: "admin", password: TestData.Accounts["admin"]["password"], }); + + accessTokenUser6 = await utils.getToken(appUrl, { + username: "user6", + password: TestData.Accounts["user6"]["password"], + }); }); after(() => { @@ -379,7 +387,7 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -484,7 +492,7 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -564,7 +572,7 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -619,7 +627,7 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -637,13 +645,13 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser1}` }) - .expect(TestData.CreationUnauthorizedStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("User not authenticated"); + .and.be.equal("User does not belong to any group, cannot create job with #datasetOwner authorization."); }); }); @@ -904,8 +912,6 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ .set({ Authorization: `Bearer ${accessTokenUser2}` }) .expect(TestData.SuccessfulGetStatusCode) .expect("Content-Type", /json/); - res.body.should.be.an("array").to.have.lengthOf(12); - res.body.map((job) => job.id).should.include.members([jobId12, jobId21]); }); it("0420: Access jobs as user3", async () => { @@ -934,4 +940,91 @@ describe("1150: Jobs: Test New Job Model Authorization for owner_access jobs typ res.body.map((job) => job.id).should.include.members([jobId4, jobId5]); }); }); + + it("0440: Add a new job as user6 with datasets owned by two different groups that user6 belongs to", async () => { + const datasetGroup6 = { + ...TestData.RawCorrect, + isPublished: false, + ownerGroup: "group6", + accessGroups: [], + }; + + const datasetUser6 = { + ...TestData.RawCorrect, + isPublished: false, + ownerGroup: "user6", + accessGroups: [], + }; + + const createdDatasetGroup6 = await request(appUrl) + .post("/api/v3/Datasets") + .send(datasetGroup6) + .set("Accept", "application/json") + .set({ Authorization: `Bearer ${accessTokenAdminIngestor}` }) + .expect(TestData.EntryCreatedStatusCode) + .expect("Content-Type", /json/); + datasetPidGroup6 = createdDatasetGroup6.body.pid; + + const createdDatasetUser6 = await request(appUrl) + .post("/api/v3/Datasets") + .send(datasetUser6) + .set("Accept", "application/json") + .set({ Authorization: `Bearer ${accessTokenAdminIngestor}` }) + .expect(TestData.EntryCreatedStatusCode) + .expect("Content-Type", /json/); + datasetPidUser6 = createdDatasetUser6.body.pid; + + const newJob = { + ...jobDatasetOwner, + ownerUser: "user6", + ownerGroup: "group6", + jobParams: { + datasetList: [ + { pid: datasetPidGroup6, files: [] }, + { pid: datasetPidUser6, files: [] }, + ], + }, + }; + + return request(appUrl) + .post("/api/v4/Jobs") + .send(newJob) + .set("Accept", "application/json") + .set({ Authorization: `Bearer ${accessTokenUser6}` }) + .expect(TestData.EntryCreatedStatusCode) + .expect("Content-Type", /json/) + .then((res) => { + res.body.should.have.property("type").and.be.string; + res.body.should.have.property("ownerUser").and.be.equal("user6"); + res.body.should.have.property("ownerGroup").and.be.equal("group6"); + res.body.should.have.property("statusCode").to.be.equal("jobSubmitted"); + }); + }); + it("0450: Add a new job as user6 with datasets owned by two different groups, one of which user6 does not belong to", async () => { + const newJob = { + ...jobDatasetOwner, + ownerUser: "user6", + ownerGroup: "group6", + jobParams: { + datasetList: [ + { pid: datasetPidGroup6, files: [] }, + { pid: datasetPid3, files: [] }, + ], + }, + }; + return request(appUrl) + .post("/api/v4/Jobs") + .send(newJob) + .set("Accept", "application/json") + .set({ Authorization: `Bearer ${accessTokenUser6}` }) + .expect(TestData.AccessForbiddenStatusCode) + .expect("Content-Type", /json/) + .then((res) => { + res.body.should.not.have.property("id"); + res.body.should.have + .property("message") + .and.be.equal("User does not have access to all datasets, cannot create job."); + }); + }); + }); diff --git a/test/JobsDatasetPublic.js b/test/JobsDatasetPublic.js index 28cc7751b..a9eb472ab 100644 --- a/test/JobsDatasetPublic.js +++ b/test/JobsDatasetPublic.js @@ -330,7 +330,7 @@ describe("1160: Jobs: Test New Job Model Authorization for public_access jobs ty res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -384,7 +384,7 @@ describe("1160: Jobs: Test New Job Model Authorization for public_access jobs ty res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); @@ -431,7 +431,7 @@ describe("1160: Jobs: Test New Job Model Authorization for public_access jobs ty res.body.should.not.have.property("id"); res.body.should.have .property("message") - .and.be.equal("Unauthorized to create this job."); + .and.be.equal("User does not have access to all datasets, cannot create job."); }); }); }); diff --git a/test/JobsSpecificGroup.js b/test/JobsSpecificGroup.js index 4e64d07a8..48ad2db1b 100644 --- a/test/JobsSpecificGroup.js +++ b/test/JobsSpecificGroup.js @@ -552,7 +552,7 @@ describe("1180: Jobs: Test New Job Model Authorization for group_access type: co .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); diff --git a/test/JobsV3.js b/test/JobsV3.js index 0c7e280cd..05ce64428 100644 --- a/test/JobsV3.js +++ b/test/JobsV3.js @@ -1144,7 +1144,7 @@ describe("1191: Jobs: Test Backwards Compatibility", () => { .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) .then((res) => { res.body.should.not.have.property("id"); @@ -1371,7 +1371,7 @@ describe("1191: Jobs: Test Backwards Compatibility", () => { .send(newJob) .set("Accept", "application/json") .set({ Authorization: `Bearer ${accessTokenUser51}` }) - .expect(TestData.BadRequestStatusCode) + .expect(TestData.AccessForbiddenStatusCode) .expect("Content-Type", /json/) } finally { await db.collection("UserIdentity").deleteOne({ _id: userIdentity2.insertedId }) diff --git a/test/TestData.js b/test/TestData.js index 204c8b1be..5d4d91e0c 100644 --- a/test/TestData.js +++ b/test/TestData.js @@ -3,6 +3,7 @@ const { faker } = require("@faker-js/faker"); const _ = require("lodash"); const RawTestAccounts = require("../test/config/functionalAccounts.json"); +const { UnprocessableEntityException } = require("@nestjs/common"); const TestAccounts = Object.fromEntries( RawTestAccounts.map((account) => [account.username, account]), ); @@ -29,6 +30,7 @@ const TestData = { CreationUnauthorizedStatusCode: 401, ConflictStatusCode: 409, PreconditionFailedStatusCode: 412, + UnprocessableEntityStatusCode: 422, FailedDependencyStatusCode: 424, ApplicationErrorStatusCode: 500, LoginSuccessfulStatusCode: 201,