Merge pull request #1327 from SciCatProject/SWAP-4091-scicat-be-creat…

…e-a-permission-check-endpoint-for

Feat: add endpoint to check user access for sample and proposal

.github/workflows/test.yml

@@ -130,7 +130,7 @@ jobs:
       - name: API tests
         env:
           ELASTICSEARCH_ENABLED: ${{ matrix.elasticsearch_enabled }}
-          MONGODB_URI: mongodb://localhost:27017/scicat
+          MONGODB_URI: mongodb://localhost:27017/scicat-test-db
           EXPRESS_SESSION_SECRET: a_scicat_secret
           JWT_SECRET: a_scicat_secret
           PORT: 3000

README.md

@@ -23,6 +23,7 @@ If you have any questions, please feel free to contact any member of the develop
 or the SciCat team at ESS.
 
 ## Contributing
+
 If you're planning to contribute to this project by adding new functionality, we encourage you to discuss it first in the Discussions tab. This helps ensure that your proposed changes align with the project's goals and prevents duplicate efforts. Here's how you can initiate a discussion:
 
 1. Go to the [Discussions tab](https://github.com/SciCatProject/scicat-backend-next/discussions).

src/casl/casl-ability.factory.ts

@@ -1,9 +1,10 @@
 import {
-  Ability,
   AbilityBuilder,
-  AbilityClass,
   ExtractSubjectType,
   InferSubjects,
+  MongoAbility,
+  MongoQuery,
+  createMongoAbility,
 } from "@casl/ability";
 import { Injectable } from "@nestjs/common";
 import { Attachment } from "src/attachments/schemas/attachment.schema";
@@ -46,15 +47,17 @@ type Subjects =
       | typeof ElasticSearchActions
     >
   | "all";
+type PossibleAbilities = [Action, Subjects];
+type Conditions = MongoQuery;
 
-export type AppAbility = Ability<[Action, Subjects]>;
+export type AppAbility = MongoAbility<PossibleAbilities, Conditions>;
 
 @Injectable()
 export class CaslAbilityFactory {
   createForUser(user: JWTUser) {
-    const { can, cannot, build } = new AbilityBuilder<
-      Ability<[Action, Subjects]>
-    >(Ability as AbilityClass<AppAbility>);
+    const { can, cannot, build } = new AbilityBuilder(
+      createMongoAbility<PossibleAbilities, Conditions>,
+    );
 
     // // admin groups
     // const stringAdminGroups = process.env.ADMIN_GROUPS || "";

src/proposals/proposals.controller.ts

@@ -33,7 +33,6 @@ import {
   ApiTags,
 } from "@nestjs/swagger";
 import { PoliciesGuard } from "src/casl/guards/policies.guard";
-import { AuthenticatedPoliciesGuard } from "../casl/guards/auth-check.guard";
 import { CheckPolicies } from "src/casl/decorators/check-policies.decorator";
 import { AppAbility, CaslAbilityFactory } from "src/casl/casl-ability.factory";
 import { Action } from "src/casl/action.enum";
@@ -181,7 +180,7 @@ export class ProposalsController {
       );
 
       if (!canDoAction) {
-        throw new ForbiddenException("Unauthorized access");
+        throw new ForbiddenException("Unauthorized to this proposal");
       }
     }
     return proposal;
@@ -516,8 +515,8 @@ export class ProposalsController {
     return this.proposalsService.fullfacet(parsedFilters);
   }
 
-  // GET /proposals/:id
-  @UseGuards(AuthenticatedPoliciesGuard)
+  // GET /proposals/:pid
+  @UseGuards(PoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.ProposalsRead, ProposalClass),
   )
@@ -532,7 +531,7 @@ export class ProposalsController {
     type: String,
   })
   @ApiResponse({
-    status: 200,
+    status: HttpStatus.OK,
     type: ProposalClass,
     isArray: false,
     description: "Return proposal with pid specified",
@@ -546,9 +545,49 @@ export class ProposalsController {
       proposalId,
       Action.ProposalsRead,
     );
+
     return proposal;
   }
 
+  // GET /proposals/:pid/authorization
+  @UseGuards(PoliciesGuard)
+  @CheckPolicies((ability: AppAbility) =>
+    ability.can(Action.ProposalsRead, ProposalClass),
+  )
+  @Get("/:pid/authorization")
+  @ApiOperation({
+    summary: "Check user access to a specific proposal.",
+    description:
+      "Returns a boolean indicating whether the user has access to the proposal with the specified ID.",
+  })
+  @ApiParam({
+    name: "pid",
+    description: "ID of the proposal to check access for",
+    type: String,
+  })
+  @ApiResponse({
+    status: HttpStatus.OK,
+    type: Boolean,
+    description:
+      "Returns true if the user has access to the specified proposal, otherwise false.",
+  })
+  async findByIdAccess(
+    @Req() request: Request,
+    @Param("pid") proposalId: string,
+  ): Promise<{ canAccess: boolean }> {
+    const proposal = await this.proposalsService.findOne({
+      proposalId,
+    });
+    if (!proposal) return { canAccess: false };
+
+    const canAccess = await this.permissionChecker(
+      Action.ProposalsRead,
+      proposal,
+      request,
+    );
+    return { canAccess };
+  }
+
   // PATCH /proposals/:pid
   @UseGuards(PoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>

src/samples/samples.controller.ts

@@ -63,6 +63,7 @@ import { Request } from "express";
 import { JWTUser } from "src/auth/interfaces/jwt-user.interface";
 import { IDatasetFields } from "src/datasets/interfaces/dataset-filters.interface";
 import { CreateSubAttachmentDto } from "src/attachments/dto/create-sub-attachment.dto";
+import { AuthenticatedPoliciesGuard } from "src/casl/guards/auth-check.guard";
 
 @ApiBearerAuth()
 @ApiTags("samples")
@@ -165,9 +166,8 @@ export class SamplesController {
 
     if (sample) {
       const canDoAction = await this.permissionChecker(group, sample, request);
-
       if (!canDoAction) {
-        throw new ForbiddenException("Unauthorized to update this sample");
+        throw new ForbiddenException("Unauthorized to this sample");
       }
     }
 
@@ -242,7 +242,7 @@ export class SamplesController {
     return mergedFilters;
   }
   // POST /samples
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleCreate, SampleClass),
   )
@@ -314,7 +314,7 @@ export class SamplesController {
   }
 
   // GET /samples/fullquery
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleRead, SampleClass),
   )
@@ -391,7 +391,7 @@ export class SamplesController {
   }
 
   // GET /samples/metadataKeys
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleRead, SampleClass),
   )
@@ -463,7 +463,7 @@ export class SamplesController {
   }
 
   // GET /samples/findOne
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleRead, SampleClass),
   )
@@ -546,12 +546,54 @@ export class SamplesController {
     @Req() request: Request,
     @Param("id") id: string,
   ): Promise<SampleClass | null> {
-    await this.checkPermissionsForSample(request, id, Action.SampleRead);
-    return this.samplesService.findOne({ sampleId: id });
+    const sample = await this.checkPermissionsForSample(
+      request,
+      id,
+      Action.SampleRead,
+    );
+    return sample;
   }
 
-  // PATCH /samples/:id
+  // GET /samples/:id/authorization
   @UseGuards(PoliciesGuard)
+  @CheckPolicies((ability: AppAbility) =>
+    ability.can(Action.SampleRead, SampleClass),
+  )
+  @Get("/:id/authorization")
+  @ApiOperation({
+    summary: "Check user access to a specific sample.",
+    description:
+      "Returns a boolean indicating whether the user has access to the sample with the specified ID.",
+  })
+  @ApiParam({
+    name: "pid",
+    description: "ID of the sample to check access for",
+    type: String,
+  })
+  @ApiResponse({
+    status: HttpStatus.OK,
+    type: Boolean,
+    description:
+      "Returns true if the user has access to the specified sample, otherwise false.",
+  })
+  async findByIdAccess(
+    @Req() request: Request,
+    @Param("id") id: string,
+  ): Promise<{ canAccess: boolean }> {
+    const sample = await this.samplesService.findOne({
+      sampleId: id,
+    });
+    if (!sample) return { canAccess: false };
+    const canAccess = await this.permissionChecker(
+      Action.SampleRead,
+      sample,
+      request,
+    );
+    return { canAccess };
+  }
+
+  // PATCH /samples/:id
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleUpdate, SampleClass),
   )
@@ -618,7 +660,7 @@ export class SamplesController {
   }
 
   // POST /samples/:id/attachments
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleAttachmentDelete, SampleClass),
   )
@@ -704,7 +746,7 @@ export class SamplesController {
   }
 
   // GET /samples/:id/attachments/:fk
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleAttachmentRead, SampleClass),
   )
@@ -747,7 +789,7 @@ export class SamplesController {
   }
 
   // DELETE /samples/:id/attachments/:fk
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleAttachmentDelete, SampleClass),
   )
@@ -789,7 +831,7 @@ export class SamplesController {
   }
 
   // POST /samples/:id/datasets
-  /*   @UseGuards(PoliciesGuard)
+  /*   @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) => ability.can(Action.Create, Dataset))
   @Post("/:id/datasets")
   async createDataset(
@@ -802,7 +844,7 @@ export class SamplesController {
  */
 
   // GET /samples/:id/datasets
-  @UseGuards(PoliciesGuard)
+  @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) =>
     ability.can(Action.SampleDatasetRead, SampleClass),
   )
@@ -865,7 +907,7 @@ export class SamplesController {
   }
 
   // PATCH /samples/:id/datasets/:fk
-  /* @UseGuards(PoliciesGuard)
+  /* @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) => ability.can(Action.Update, Dataset))
   @Patch("/:id/datasets/:fk")
   async findOneDatasetAndUpdate(
@@ -880,7 +922,7 @@ export class SamplesController {
   } */
 
   // DELETE /samples/:id/datasets/:fk
-  /*   @UseGuards(PoliciesGuard)
+  /*   @UseGuards(AuthenticatedPoliciesGuard)
   @CheckPolicies((ability: AppAbility) => ability.can(Action.Delete, Dataset))
   @Delete("/:id/datasets/:fk")
   async findOneDatasetAndRemove(