Merge pull request #518 from SciCatProject/SWAP-3250-post-dataaset-wi…

…th-explicit-pid

Swap 3250 post dataset with explicit pid

.env.example

@@ -34,4 +34,8 @@ SITE=<SITE>
 SMTP_HOST=<SMTP_HOST>
 SMTP_MESSAGE_FROM=<SMTP_MESSAGE_FROM>
 SMTP_PORT=<SMTP_PORT>
-SMTP_SECURE=<SMTP_SECURE>
+SMTP_SECURE=<SMTP_SECURE>
+CREATE_DATASET_GROUPS="all"
+CREATE_DATASET_WITH_PID_GROUPS=""
+DATASET_CREATION_VALIDATION_ENABLED=true
+DATASET_CREATION_VALIDATION_REGEX="^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$"

.github/workflows/test.yml

@@ -102,6 +102,10 @@ jobs:
           ADMIN_GROUPS: admin,ingestor,archivemanager
           CREATE_DATASET_GROUPS: group1,group2,group3
           DELETE_GROUPS: archivemanager
+          ACCESS_GROUPS_STATIC_VALUES: "ess"
+          CREATE_DATASET_WITH_PID_GROUPS: "group2,group3"
+          DATASET_CREATION_VALIDATION_ENABLED: true
+          DATASET_CREATION_VALIDATION_REGEX: "^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$"
 
         # Start mongo container and app before running api tests
         run: |

CI/E2E/.env.backend-next

@@ -30,3 +30,6 @@ SITE="ESS"
 ADMIN_GROUPS="admin, ingestor, archivemanager"
 CREATE_DATASET_GROUPS="group1, group2, group3"
 DELETE_GROUPS="archivemanager"
+CREATE_DATASET_WITH_PID_GROUPS=""
+DATASET_CREATION_VALIDATION_ENABLED=true
+DATASET_CREATION_VALIDATION_REGEX="^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$"

CI/E2E/backend.env

@@ -30,4 +30,7 @@ ADMIN_GROUPS="admin, ingestor, archivemanager"
 CREATE_DATASET_GROUPS="group1, group2, group3"
 DELETE_GROUPS="admin, archivemanager"
 OAI_PROVIDER_ROUTER=
+CREATE_DATASET_WITH_PID_GROUPS=""
+DATASET_CREATION_VALIDATION_ENABLED=true
+DATASET_CREATION_VALIDATION_REGEX="^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$"
 

src/casl/casl-ability.factory.ts

@@ -54,18 +54,18 @@ export class CaslAbilityFactory {
     >(Ability as AbilityClass<AppAbility>);
 
     // admin groups
-    const stringAdminGroups = process.env.ADMIN_GROUPS || ("" as string);
+    const stringAdminGroups = process.env.ADMIN_GROUPS || "";
     const adminGroups: string[] = stringAdminGroups
       ? stringAdminGroups.split(",").map((v) => v.trim())
       : [];
     // delete groups
-    const stringDeleteGroups = process.env.DELETE_GROUPS || ("" as string);
+    const stringDeleteGroups = process.env.DELETE_GROUPS || "";
     const deleteGroups: string[] = stringDeleteGroups
       ? stringDeleteGroups.split(",").map((v) => v.trim())
       : [];
     // create dataset groups
     const stringCreateDatasetGroups =
-      process.env.CREATE_DATASET_GROUPS || ("all" as string);
+      process.env.CREATE_DATASET_GROUPS || "all";
     const createDatasetGroups: string[] = stringCreateDatasetGroups
       .split(",")
       .map((v) => v.trim());

src/datasets/datasets.controller.ts

@@ -18,7 +18,11 @@ import {
   NotFoundException,
   Req,
   ForbiddenException,
+  InternalServerErrorException,
+  ConflictException,
+  BadRequestException,
 } from "@nestjs/common";
+import { MongoError } from "mongodb";
 import {
   ApiBearerAuth,
   ApiBody,
@@ -200,6 +204,32 @@ export class DatasetsController {
     return dataset;
   }
 
+  getUserPermissionsFromGroups(user: JWTUser) {
+    const createDatasetWithPidGroupsString =
+      process.env.CREATE_DATASET_WITH_PID_GROUPS || "";
+    const createDatasetWithPidGroups: string[] =
+      createDatasetWithPidGroupsString
+        ? createDatasetWithPidGroupsString.split(",").map((v) => v.trim())
+        : [];
+
+    const stringAdminGroups = process.env.ADMIN_GROUPS || "";
+    const adminGroups: string[] = stringAdminGroups
+      ? stringAdminGroups.split(",").map((v) => v.trim())
+      : [];
+
+    const isPartOfAdminGroups = user.currentGroups.some((g) =>
+      adminGroups.includes(g),
+    );
+    const userCanCreateDatasetWithPid = createDatasetWithPidGroups.some(
+      (value) => user.currentGroups.includes(value),
+    );
+
+    return {
+      isPartOfAdminGroups,
+      userCanCreateDatasetWithPid,
+    };
+  }
+
   async checkPermissionsForDatasetCreate(
     request: Request,
     dataset: CreateRawDatasetDto | CreateDerivedDatasetDto,
@@ -210,18 +240,57 @@ export class DatasetsController {
       // NOTE: We need DatasetClass instance because casl module can not recognize the type from dataset mongo database model. If other fields are needed can be added later.
       const datasetInstance = new DatasetClass();
       datasetInstance._id = "";
-      datasetInstance.pid = "";
+      datasetInstance.pid = dataset.pid || "";
       datasetInstance.accessGroups = dataset.accessGroups || [];
       datasetInstance.ownerGroup = dataset.ownerGroup;
       datasetInstance.sharedWith = dataset.sharedWith;
       datasetInstance.isPublished = dataset.isPublished || false;
       datasetInstance.owner = dataset.owner;
       datasetInstance.ownerEmail = dataset.ownerEmail;
       if (user) {
-        const ability = this.caslAbilityFactory.createForUser(user);
-        const canCreate = ability.can(Action.Create, datasetInstance);
-        if (!canCreate) {
-          throw new ForbiddenException("Unauthorized to create this dataset");
+        const { isPartOfAdminGroups, userCanCreateDatasetWithPid } =
+          this.getUserPermissionsFromGroups(user);
+        if (
+          datasetInstance.pid &&
+          !userCanCreateDatasetWithPid &&
+          !isPartOfAdminGroups
+        ) {
+          throw new ForbiddenException(
+            "Unauthorized to create datasets with explicit PID",
+          );
+        } else {
+          // NOTE: If it can create dataset but not part of ADMIN_GROUPS and not part of CREATE_DATASET_WITH_PID_GROUPS,
+          // then we make sure that pid is not provided by user but it is generated by the system.
+          if (!isPartOfAdminGroups && !userCanCreateDatasetWithPid) {
+            delete dataset.pid;
+          }
+
+          const ability = this.caslAbilityFactory.createForUser(user);
+          const canCreate = ability.can(Action.Create, datasetInstance);
+
+          if (!canCreate) {
+            throw new ForbiddenException("Unauthorized to create this dataset");
+          }
+
+          const datasetCreationValidationEnabled =
+            process.env.DATASET_CREATION_VALIDATION_ENABLED;
+
+          const datasetCreationValidationRegex =
+            process.env.DATASET_CREATION_VALIDATION_REGEX;
+
+          if (
+            datasetCreationValidationEnabled &&
+            datasetCreationValidationRegex &&
+            dataset.pid
+          ) {
+            const re = new RegExp(datasetCreationValidationRegex);
+
+            if (!re.test(dataset.pid)) {
+              throw new BadRequestException(
+                "PID is not following required standards",
+              );
+            }
+          }
         }
       } else {
         throw new ForbiddenException("Unauthorized to create datasets");
@@ -276,9 +345,24 @@ export class DatasetsController {
         : CreateDerivedDatasetDto,
     );
 
-    await this.checkPermissionsForDatasetCreate(request, createDatasetDto);
+    const datasetDTO = await this.checkPermissionsForDatasetCreate(
+      request,
+      createDatasetDto,
+    );
+
+    try {
+      const createdDataset = await this.datasetsService.create(datasetDTO);
 
-    return this.datasetsService.create(createDatasetDto);
+      return createdDataset;
+    } catch (error) {
+      if ((error as MongoError).code === 11000) {
+        throw new ConflictException(
+          "A dataset with this this unique key already exists!",
+        );
+      } else {
+        throw new InternalServerErrorException();
+      }
+    }
   }
 
   async validateDataset(

src/datasets/dto/create-dataset.dto.ts

@@ -24,6 +24,15 @@ import { LifecycleClass } from "../schemas/lifecycle.schema";
 
 @ApiTags("datasets")
 export class CreateDatasetDto extends OwnableDto {
+  @ApiProperty({
+    type: String,
+    required: false,
+    description: "Persistent identifier of the dataset.",
+  })
+  @IsOptional()
+  @IsString()
+  pid?: string;
+
   @ApiProperty({
     type: String,
     required: true,

src/users/users.controller.ts

@@ -274,7 +274,8 @@ export class UsersController {
   @UseGuards(JwtAuthGuard)
   @Post("logout")
   async logout(@Req() req: Request, @Res() res: Response) {
-    return await this.authService.logout(req, res);
+    await this.authService.logout(req, res);
+    return res.send({ logout: "successful" });
   }
 
   @UseGuards(PoliciesGuard)

test/DerivedDataset.js

@@ -6,7 +6,10 @@ const { TestData } = require("./TestData");
 
 var accessToken = null;
 var accessTokenArchiveManager = null;
+var accessTokenUser1 = null;
+var accessTokenUser2 = null;
 var pid = null;
+var explicitPid = "B69A6239-5FD1-4244-8363-58C2DA1B6915";
 var minPid = null;
 
 describe("DerivedDataset: Derived Datasets", () => {
@@ -22,12 +25,32 @@ describe("DerivedDataset: Derived Datasets", () => {
         utils.getToken(
           appUrl,
           {
-            username: "archiveManager",
-            password: "aman",
+            username: "user1",
+            password: "a609316768619f154ef58db4d847b75e",
           },
           (tokenVal) => {
-            accessTokenArchiveManager = tokenVal;
-            done();
+            accessTokenUser1 = tokenVal;
+            utils.getToken(
+              appUrl,
+              {
+                username: "user2",
+                password: "f522d1d715970073a6413474ca0e0f63",
+              },
+              (tokenVal) => {
+                accessTokenUser2 = tokenVal;
+                utils.getToken(
+                  appUrl,
+                  {
+                    username: "archiveManager",
+                    password: "aman",
+                  },
+                  (tokenVal) => {
+                    accessTokenArchiveManager = tokenVal;
+                    done();
+                  },
+                );
+              },
+            );
           },
         );
       },
@@ -79,6 +102,57 @@ describe("DerivedDataset: Derived Datasets", () => {
       });
   });
 
+  it("should not be able to add new derived dataset with incorrect explicit pid", async () => {
+    const derivedDatasetWithExplicitPID = {
+      ...TestData.DerivedCorrect,
+      pid: "test-pid-1",
+    };
+    return request(appUrl)
+      .post("/api/v3/Datasets")
+      .send(derivedDatasetWithExplicitPID)
+      .set("Accept", "application/json")
+      .set({ Authorization: `Bearer ${accessToken}` })
+      .expect(400)
+      .expect("Content-Type", /json/);
+  });
+
+  it("should not be able to add new derived dataset with group that is not part of allowed groups", async () => {
+    const derivedDatasetWithExplicitPID = {
+      ...TestData.DerivedCorrect,
+      pid: "test-pid-1",
+    };
+    return request(appUrl)
+      .post("/api/v3/Datasets")
+      .send(derivedDatasetWithExplicitPID)
+      .set("Accept", "application/json")
+      .set({ Authorization: `Bearer ${accessTokenUser1}` })
+      .expect(403)
+      .expect("Content-Type", /json/);
+  });
+
+  it("should be able to add new derived dataset with group that is part of allowed groups and correct explicit PID", async () => {
+    const derivedDatasetWithExplicitPID = {
+      ...TestData.DerivedCorrect,
+      ownerGroup: "group2",
+      pid: explicitPid,
+    };
+    return request(appUrl)
+      .post("/api/v3/Datasets")
+      .send(derivedDatasetWithExplicitPID)
+      .set("Accept", "application/json")
+      .set({ Authorization: `Bearer ${accessTokenUser2}` })
+      .expect(200)
+      .expect("Content-Type", /json/)
+      .then((res) => {
+        res.body.should.have.property("owner").and.be.string;
+        res.body.should.have.property("type").and.equal("derived");
+        res.body.should.have
+          .property("pid")
+          .and.equal(derivedDatasetWithExplicitPID.pid);
+        explicitPid = res.body["pid"];
+      });
+  });
+
   // check if dataset is valid
   it("check if invalid derived dataset is valid", async () => {
     return request(appUrl)
@@ -204,4 +278,12 @@ describe("DerivedDataset: Derived Datasets", () => {
       .expect(200)
       .expect("Content-Type", /json/);
   });
+  it("should delete a derived dataset with explicit PID", async () => {
+    return request(appUrl)
+      .delete("/api/v3/Datasets/" + explicitPid)
+      .set("Accept", "application/json")
+      .set({ Authorization: `Bearer ${accessTokenArchiveManager}` })
+      .expect(200)
+      .expect("Content-Type", /json/);
+  });
 });

test/RandomizedDatasetPermissions.js

@@ -24,10 +24,10 @@ function generateRandomDataset() {
     principalInvestigator: faker.internet.email(),
     endTime: faker.date.past().toISOString(),
     creationLocation: faker.system.directoryPath(),
-    dataFormat: faker.random.words(3),
+    dataFormat: faker.lorem.words(3),
     scientificMetadata: {
       approx_file_size_mb: {
-        value: faker.random.numeric(5),
+        value: faker.string.numeric(5),
         unit: "bytes",
       },
       beamlineParameters: {
@@ -90,8 +90,8 @@ function generateRandomDataset() {
     numberOfFiles: 0,
     numberOfFilesArchived: 0,
     creationTime: faker.date.past().toISOString(),
-    description: faker.random.words(10),
-    datasetName: faker.random.words(2),
+    description: faker.lorem.words(10),
+    datasetName: faker.lorem.words(2),
     classification: "AV=medium,CO=low",
     license: "CC BY-SA 4.0",
     isPublished: false,
@@ -104,7 +104,7 @@ function generateRandomDataset() {
     accessGroups: [],
     proposalId: process.env.PID_PREFIX
       ? process.env.PID_PREFIX
-      : "" + faker.random.numeric(6),
+      : "" + faker.string.numeric(6),
     type: "raw",
     keywords: ["sls", "protein"],
   };