Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
15 changes: 9 additions & 6 deletions src/casl/casl-ability.factory.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 || "";
Expand Down
49 changes: 44 additions & 5 deletions src/proposals/proposals.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -181,7 +180,7 @@ export class ProposalsController {
);

if (!canDoAction) {
throw new ForbiddenException("Unauthorized access");
throw new ForbiddenException("Unauthorized to this proposal");
}
}
return proposal;
Expand Down Expand Up @@ -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),
)
Expand All @@ -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",
Expand All @@ -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) =>
Expand Down
74 changes: 58 additions & 16 deletions src/samples/samples.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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");
}
}

Expand Down Expand Up @@ -242,7 +242,7 @@ export class SamplesController {
return mergedFilters;
}
// POST /samples
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleCreate, SampleClass),
)
Expand Down Expand Up @@ -314,7 +314,7 @@ export class SamplesController {
}

// GET /samples/fullquery
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleRead, SampleClass),
)
Expand Down Expand Up @@ -391,7 +391,7 @@ export class SamplesController {
}

// GET /samples/metadataKeys
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleRead, SampleClass),
)
Expand Down Expand Up @@ -463,7 +463,7 @@ export class SamplesController {
}

// GET /samples/findOne
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleRead, SampleClass),
)
Expand Down Expand Up @@ -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),
)
Expand Down Expand Up @@ -618,7 +660,7 @@ export class SamplesController {
}

// POST /samples/:id/attachments
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleAttachmentDelete, SampleClass),
)
Expand Down Expand Up @@ -704,7 +746,7 @@ export class SamplesController {
}

// GET /samples/:id/attachments/:fk
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleAttachmentRead, SampleClass),
)
Expand Down Expand Up @@ -747,7 +789,7 @@ export class SamplesController {
}

// DELETE /samples/:id/attachments/:fk
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleAttachmentDelete, SampleClass),
)
Expand Down Expand Up @@ -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(
Expand All @@ -802,7 +844,7 @@ export class SamplesController {
*/

// GET /samples/:id/datasets
@UseGuards(PoliciesGuard)
@UseGuards(AuthenticatedPoliciesGuard)
@CheckPolicies((ability: AppAbility) =>
ability.can(Action.SampleDatasetRead, SampleClass),
)
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
Loading