From e04ac79718d934e143e24ca683349865e3199ea5 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Tue, 5 Mar 2024 13:18:06 +1100 Subject: [PATCH 1/7] [WIP] - add decorators --- src/features/features.controller.ts | 12 +++++++++--- src/forms/forms.controller.ts | 6 +++++- src/teams/teams.controller.ts | 9 +++++---- src/users/users.controller.ts | 7 ++++--- 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/features/features.controller.ts b/src/features/features.controller.ts index 1841890d..a576abe3 100644 --- a/src/features/features.controller.ts +++ b/src/features/features.controller.ts @@ -34,6 +34,8 @@ import { ExtendedFeaturesResponse, FeatureResponse, } from "./features.response"; +import { AppPermissions } from "../auth/auth.permissions"; +import { Permissions } from "../global/decorators/permissions.decorator"; @Controller() @ApiTags("Voyage - Features") @@ -42,7 +44,7 @@ export class FeaturesController { @ApiOperation({ summary: - "Adds a new feature for a team given a teamId (int) and that the user is logged in.", + "[Permission: own_team] Adds a new feature for a team given a teamId (int) and that the user is logged in.", }) @ApiResponse({ status: HttpStatus.NOT_FOUND, @@ -60,6 +62,7 @@ export class FeaturesController { description: "Successfully created a new feature.", type: FeatureResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Post("/:teamId/features") @ApiCreatedResponse({ type: Feature }) async createFeature( @@ -75,7 +78,8 @@ export class FeaturesController { } @ApiOperation({ - summary: "Gets all feature category options.", + summary: + "Gets all feature category options. e.g. Must have, should have, nice to have", }) @ApiResponse({ status: HttpStatus.OK, @@ -113,7 +117,8 @@ export class FeaturesController { } @ApiOperation({ - summary: "Gets all features for a team given a teamId (int).", + summary: + "[Permission: own_team] Gets all features for a team given a teamId (int).", }) @ApiResponse({ status: HttpStatus.OK, @@ -133,6 +138,7 @@ export class FeaturesController { "Could not find features for project. Team with given ID does not exist.", type: NotFoundErrorResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Get("/:teamId/features") findAllFeatures( @Request() req, diff --git a/src/forms/forms.controller.ts b/src/forms/forms.controller.ts index 9ef8cbd1..ee3383ba 100644 --- a/src/forms/forms.controller.ts +++ b/src/forms/forms.controller.ts @@ -10,6 +10,9 @@ import { ApiOperation, ApiParam, ApiResponse, ApiTags } from "@nestjs/swagger"; import { FormResponse } from "./forms.response"; import { NotFoundErrorResponse } from "../global/responses/errors"; +import { AppRoles } from "../auth/auth.roles"; +import { Roles } from "../global/decorators/roles.decorator"; + @Controller("forms") @ApiTags("Forms") export class FormsController { @@ -17,7 +20,7 @@ export class FormsController { @Get() @ApiOperation({ - summary: "gets all forms from the database", + summary: "[Roles: admin] gets all forms from the database", description: "Returns all forms details with questions.
" + "This is currently for development purpose, or admin in future", @@ -28,6 +31,7 @@ export class FormsController { type: FormResponse, isArray: true, }) + @Roles(AppRoles.Admin) getAllForms() { return this.formsService.getAllForms(); } diff --git a/src/teams/teams.controller.ts b/src/teams/teams.controller.ts index e4a8a8bb..10fcdb2d 100644 --- a/src/teams/teams.controller.ts +++ b/src/teams/teams.controller.ts @@ -31,7 +31,7 @@ export class TeamsController { constructor(private readonly teamsService: TeamsService) {} @ApiOperation({ - summary: "Gets all voyage teams.", + summary: "[Roles: Admin] Gets all voyage teams.", description: "For development/admin purpose", }) @ApiResponse({ @@ -47,7 +47,8 @@ export class TeamsController { } @ApiOperation({ - summary: "Gets all teams for a voyage given a voyageId (int).", + summary: + "[Roles: Admin] Gets all teams for a voyage given a voyageId (int).", }) // Will need to be fixed to be RESTful @ApiResponse({ @@ -75,7 +76,7 @@ export class TeamsController { } @ApiOperation({ - summary: "Gets one team given a teamId (int).", + summary: "[Permission: own_team] Gets one team given a teamId (int).", }) @ApiResponse({ status: HttpStatus.OK, @@ -102,7 +103,7 @@ export class TeamsController { @ApiOperation({ summary: - "Updates team member hours per a sprint given a teamId (int) and userId (int).", + "[Permission: own_team] Updates team member hours per a sprint given a teamId (int) and userId (int).", }) @ApiResponse({ status: HttpStatus.OK, diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 8a730c07..2054c607 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -25,7 +25,7 @@ export class UsersController { constructor(private readonly usersService: UsersService) {} @ApiOperation({ - summary: "Gets all users.", + summary: "[Roles: Admin] Gets all users.", description: "This endpoint is for development/admin purpose.", }) @ApiResponse({ @@ -64,7 +64,8 @@ export class UsersController { } @ApiOperation({ - summary: "Gets a user with full details given a userId (uuid).", + summary: + "[Roles: Admin] Gets a user with full details given a userId (uuid).", description: "This is currently only for development/admin", }) @ApiResponse({ @@ -98,7 +99,7 @@ export class UsersController { } @ApiOperation({ - summary: "Gets a user with full details given an email.", + summary: "[Roles: Admin] Gets a user with full details given an email.", description: "This is currently only for development/admin", }) @ApiResponse({ From f0fe1a1777efb97119c2399c994bc66416bb0aa8 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Wed, 6 Mar 2024 21:05:21 +1100 Subject: [PATCH 2/7] add voyage team ID to req.user and update permission guard --- src/auth/guards/permissions.guard.ts | 8 +++++--- src/auth/strategies/at.strategy.ts | 7 ++++++- src/features/features.service.ts | 1 + src/users/users.controller.ts | 3 ++- src/users/users.service.ts | 1 + 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/auth/guards/permissions.guard.ts b/src/auth/guards/permissions.guard.ts index 16cd4934..8447d46c 100644 --- a/src/auth/guards/permissions.guard.ts +++ b/src/auth/guards/permissions.guard.ts @@ -27,15 +27,17 @@ export class PermissionsGuard implements CanActivate { const { user, params } = context.switchToHttp().getRequest(); if (requiredPermissions.includes(AppPermissions.OWN_TEAM)) { + // Admin can bypass this if (user.roles.includes(AppRoles.Admin)) return true; if (!params.teamId) { throw new InternalServerErrorException( "This permission guard requires :teamId param", ); } - const canAccess = user.voyageTeams?.includes( - parseInt(params?.teamId), - ); + + const canAccess = user.voyageTeams + ?.map((t) => t.teamId) + ?.includes(parseInt(params?.teamId)); if (!canAccess) { throw new ForbiddenException( diff --git a/src/auth/strategies/at.strategy.ts b/src/auth/strategies/at.strategy.ts index 477ce2fb..878ad704 100644 --- a/src/auth/strategies/at.strategy.ts +++ b/src/auth/strategies/at.strategy.ts @@ -34,7 +34,12 @@ export class AtStrategy extends PassportStrategy(Strategy, "jwt-at") { userId: payload.sub, email: payload.email, roles: userInDb.roles, - voyageTeams: userInDb.voyageTeamMembers.map((t) => t.voyageTeamId), + voyageTeams: userInDb.voyageTeamMembers.map((t) => { + return { + teamId: t.voyageTeamId, + memberId: t.id, + }; + }), }; } } diff --git a/src/features/features.service.ts b/src/features/features.service.ts index 9ebf7c7c..8415ea56 100644 --- a/src/features/features.service.ts +++ b/src/features/features.service.ts @@ -23,6 +23,7 @@ export class FeaturesService { ) { const { featureCategoryId, description } = createFeatureDto; + console.log("req", req.user); const teamMember = await this.globalService.validateLoggedInAndTeamMember( teamId, diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 2054c607..fb705c5c 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -41,7 +41,8 @@ export class UsersController { } @ApiOperation({ - summary: "Gets a logged in users detail via userId:uuid in jwt token.", + summary: + "Gets a logged in users own detail via userId:uuid in jwt token.", }) @ApiResponse({ status: HttpStatus.OK, diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 191a462e..554dd0e8 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -50,6 +50,7 @@ export class UsersService { }, voyageTeamMembers: { select: { + id: true, voyageTeamId: true, }, }, From f7afa0d09a99bb1e071f7d9934d25d98728a7b1c Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Thu, 7 Mar 2024 09:59:50 +1100 Subject: [PATCH 3/7] update features endpoints --- src/auth/guards/permissions.guard.ts | 3 +- src/auth/strategies/at.strategy.ts | 1 + src/features/features.controller.ts | 46 +++++++++++++------------- src/features/features.service.ts | 49 ++++++++++++---------------- src/global/global.service.ts | 18 +++++++++- src/global/types/CustomRequest.ts | 13 ++++++++ 6 files changed, 76 insertions(+), 54 deletions(-) create mode 100644 src/global/types/CustomRequest.ts diff --git a/src/auth/guards/permissions.guard.ts b/src/auth/guards/permissions.guard.ts index 8447d46c..7e1e90c8 100644 --- a/src/auth/guards/permissions.guard.ts +++ b/src/auth/guards/permissions.guard.ts @@ -9,7 +9,6 @@ import { Reflector } from "@nestjs/core"; import { Observable } from "rxjs"; import { AppPermissions } from "../auth.permissions"; import { PERM_KEY } from "../../global/decorators/permissions.decorator"; -import { AppRoles } from "../auth.roles"; @Injectable() export class PermissionsGuard implements CanActivate { @@ -28,7 +27,7 @@ export class PermissionsGuard implements CanActivate { if (requiredPermissions.includes(AppPermissions.OWN_TEAM)) { // Admin can bypass this - if (user.roles.includes(AppRoles.Admin)) return true; + // if (user.roles.includes(AppRoles.Admin)) return true; if (!params.teamId) { throw new InternalServerErrorException( "This permission guard requires :teamId param", diff --git a/src/auth/strategies/at.strategy.ts b/src/auth/strategies/at.strategy.ts index 878ad704..ea3d78bc 100644 --- a/src/auth/strategies/at.strategy.ts +++ b/src/auth/strategies/at.strategy.ts @@ -30,6 +30,7 @@ export class AtStrategy extends PassportStrategy(Strategy, "jwt-at") { async validate(payload: any) { const userInDb = await this.usersService.getUserRolesById(payload.sub); + // Note: Update global/types/CustomRequest when updating this return { userId: payload.sub, email: payload.email, diff --git a/src/features/features.controller.ts b/src/features/features.controller.ts index a576abe3..a92ba23a 100644 --- a/src/features/features.controller.ts +++ b/src/features/features.controller.ts @@ -36,6 +36,7 @@ import { } from "./features.response"; import { AppPermissions } from "../auth/auth.permissions"; import { Permissions } from "../global/decorators/permissions.decorator"; +import { CustomRequest } from "../global/types/CustomRequest"; @Controller() @ApiTags("Voyage - Features") @@ -53,10 +54,14 @@ export class FeaturesController { }) @ApiResponse({ status: HttpStatus.UNAUTHORIZED, - description: - "Invalid uuid or teamID. User is not authorized to perform this action.", + description: "User is not authorized to perform this action.", type: UnauthorizedErrorResponse, }) + @ApiResponse({ + status: HttpStatus.BAD_REQUEST, + description: "Invalid teamId", + type: BadRequestErrorResponse, + }) @ApiResponse({ status: HttpStatus.CREATED, description: "Successfully created a new feature.", @@ -66,7 +71,7 @@ export class FeaturesController { @Post("/:teamId/features") @ApiCreatedResponse({ type: Feature }) async createFeature( - @Request() req, + @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @Body() createFeatureDto: CreateFeatureDto, ) { @@ -93,11 +98,13 @@ export class FeaturesController { } @ApiOperation({ - summary: "Gets one feature given a featureId (int).", + summary: + "[Permission: own_team] Gets all features for a team given a teamId (int).", }) @ApiResponse({ status: HttpStatus.OK, - description: "Successfully found feature.", + description: "Successfully got all features for project.", + isArray: true, type: ExtendedFeaturesResponse, }) @ApiResponse({ @@ -108,22 +115,22 @@ export class FeaturesController { }) @ApiResponse({ status: HttpStatus.NOT_FOUND, - description: "Feature with given ID does not exist.", + description: + "Could not find features for project. Team with given ID does not exist.", type: NotFoundErrorResponse, }) - @Get("/features/:featureId") - findOneFeature(@Param("featureId", ParseIntPipe) featureId: number) { - return this.featuresService.findOneFeature(featureId); + @Permissions(AppPermissions.OWN_TEAM) + @Get("/:teamId/features") + findAllFeatures(@Param("teamId", ParseIntPipe) teamId: number) { + return this.featuresService.findAllFeatures(teamId); } @ApiOperation({ - summary: - "[Permission: own_team] Gets all features for a team given a teamId (int).", + summary: "Gets one feature given a featureId (int).", }) @ApiResponse({ status: HttpStatus.OK, - description: "Successfully got all features for project.", - isArray: true, + description: "Successfully found feature.", type: ExtendedFeaturesResponse, }) @ApiResponse({ @@ -134,17 +141,12 @@ export class FeaturesController { }) @ApiResponse({ status: HttpStatus.NOT_FOUND, - description: - "Could not find features for project. Team with given ID does not exist.", + description: "Feature with given ID does not exist.", type: NotFoundErrorResponse, }) - @Permissions(AppPermissions.OWN_TEAM) - @Get("/:teamId/features") - findAllFeatures( - @Request() req, - @Param("teamId", ParseIntPipe) teamId: number, - ) { - return this.featuresService.findAllFeatures(req, teamId); + @Get("/features/:featureId") + findOneFeature(@Param("featureId", ParseIntPipe) featureId: number) { + return this.featuresService.findOneFeature(featureId); } @ApiOperation({ diff --git a/src/features/features.service.ts b/src/features/features.service.ts index 8415ea56..1a0deaa0 100644 --- a/src/features/features.service.ts +++ b/src/features/features.service.ts @@ -23,13 +23,6 @@ export class FeaturesService { ) { const { featureCategoryId, description } = createFeatureDto; - console.log("req", req.user); - const teamMember = - await this.globalService.validateLoggedInAndTeamMember( - teamId, - req.user.userId, - ); - const validCategory = await this.prisma.featureCategory.findFirst({ where: { id: featureCategoryId, @@ -59,7 +52,10 @@ export class FeaturesService { const newFeature = await this.prisma.projectFeature.create({ data: { - teamMemberId: teamMember.id, + teamMemberId: this.globalService.getVoyageTeamMemberId( + req, + teamId, + ), featureCategoryId, description, order: newOrder, @@ -81,11 +77,13 @@ export class FeaturesService { } } - async findOneFeature(featureId: number) { + async findAllFeatures(teamId: number) { try { - const projectFeature = await this.prisma.projectFeature.findFirst({ + const allTeamFeatures = await this.prisma.projectFeature.findMany({ where: { - id: featureId, + addedBy: { + voyageTeamId: teamId, + }, }, select: { id: true, @@ -113,32 +111,26 @@ export class FeaturesService { }, }, }, + orderBy: [{ category: { id: "asc" } }, { order: "asc" }], }); - if (!projectFeature) { + if (!allTeamFeatures) { throw new NotFoundException( - `FeatureId (id: ${featureId}) does not exist.`, + `TeamId (id: ${teamId}) does not exist.`, ); } - return projectFeature; + return allTeamFeatures; } catch (e) { throw e; } } - async findAllFeatures(req, teamId: number) { - await this.globalService.validateLoggedInAndTeamMember( - teamId, - req.user.userId, - ); - + async findOneFeature(featureId: number) { try { - const allTeamFeatures = await this.prisma.projectFeature.findMany({ + const projectFeature = await this.prisma.projectFeature.findFirst({ where: { - addedBy: { - voyageTeamId: teamId, - }, + id: featureId, }, select: { id: true, @@ -166,16 +158,15 @@ export class FeaturesService { }, }, }, - orderBy: [{ category: { id: "asc" } }, { order: "asc" }], }); - if (!allTeamFeatures) { + if (!projectFeature) { throw new NotFoundException( - `TeamId (id: ${teamId}) does not exist.`, + `FeatureId (id: ${featureId}) does not exist.`, ); } - return allTeamFeatures; + return projectFeature; } catch (e) { throw e; } @@ -336,7 +327,7 @@ export class FeaturesService { }, }); } - const newCategoryFeaturesList = await this.findAllFeatures(req, teamId); + const newCategoryFeaturesList = await this.findAllFeatures(teamId); return newCategoryFeaturesList; } diff --git a/src/global/global.service.ts b/src/global/global.service.ts index ce3dc8df..4bba1abe 100644 --- a/src/global/global.service.ts +++ b/src/global/global.service.ts @@ -1,11 +1,17 @@ -import { Injectable, UnauthorizedException } from "@nestjs/common"; +import { + BadRequestException, + Injectable, + UnauthorizedException, +} from "@nestjs/common"; import { PrismaService } from "../prisma/prisma.service"; +import { CustomRequest } from "./types/CustomRequest"; @Injectable() export class GlobalService { constructor(private prisma: PrismaService) {} //verifies user is logged in by using uuid from cookie and teamId to pull a teamMember. + // TODO: remove as it's replaced by permission guard public async validateLoggedInAndTeamMember(teamId: number, uuid: any) { const teamMember = await this.prisma.voyageTeamMember.findFirst({ where: { @@ -26,4 +32,14 @@ export class GlobalService { } return teamMember; } + + public getVoyageTeamMemberId(req: CustomRequest, teamId: number) { + const teamMemberId = req.user.voyageTeams.find( + (t) => t.teamId == teamId, + )?.memberId; + if (!teamMemberId) { + throw new BadRequestException(`Invalid Team Id (id: ${teamId}).`); + } + return teamMemberId; + } } diff --git a/src/global/types/CustomRequest.ts b/src/global/types/CustomRequest.ts new file mode 100644 index 00000000..d4609430 --- /dev/null +++ b/src/global/types/CustomRequest.ts @@ -0,0 +1,13 @@ +type VoyageTeam = { + teamId: number; + memberId: number; +}; + +export interface CustomRequest extends Request { + user: { + userId: string; + email: string; + roles: string[]; + voyageTeams: VoyageTeam[]; + }; +} From 94d8cebd58976f762841c74a8e4e90ad7994d140 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Thu, 7 Mar 2024 13:34:24 +1100 Subject: [PATCH 4/7] add decorators for ideations endpoints --- src/ideations/ideations.controller.ts | 36 +++++++------ src/ideations/ideations.service.ts | 77 ++++++++++++--------------- 2 files changed, 54 insertions(+), 59 deletions(-) diff --git a/src/ideations/ideations.controller.ts b/src/ideations/ideations.controller.ts index af6187ce..8a302405 100644 --- a/src/ideations/ideations.controller.ts +++ b/src/ideations/ideations.controller.ts @@ -1,14 +1,14 @@ import { + Body, Controller, + Delete, Get, + HttpStatus, Param, - Post, - Body, - Patch, - Delete, ParseIntPipe, + Patch, + Post, Request, - HttpStatus, } from "@nestjs/common"; import { IdeationsService } from "./ideations.service"; import { CreateIdeationDto } from "./dto/create-ideation.dto"; @@ -21,10 +21,13 @@ import { UnauthorizedErrorResponse, } from "../global/responses/errors"; import { - IdeationVoteResponse, IdeationResponse, + IdeationVoteResponse, TeamIdeationsResponse, } from "./ideations.response"; +import { AppPermissions } from "../auth/auth.permissions"; +import { Permissions } from "../global/decorators/permissions.decorator"; +import { CustomRequest } from "../global/types/CustomRequest"; @Controller() @ApiTags("Voyage - Ideations") @@ -52,9 +55,10 @@ export class IdeationsController { description: "Successfully created a new ideation and vote added.", type: IdeationResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Post() createIdeation( - @Request() req, + @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @Body() createIdeationDto: CreateIdeationDto, ) { @@ -90,9 +94,10 @@ export class IdeationsController { description: "Successfully created a new ideation vote.", type: IdeationVoteResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Post("/:ideationId/ideation-votes") createIdeationVote( - @Request() req, + @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { @@ -117,6 +122,7 @@ export class IdeationsController { isArray: true, type: TeamIdeationsResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Get() getIdeationsByVoyageTeam(@Param("teamId", ParseIntPipe) teamId: number) { return this.ideationsService.getIdeationsByVoyageTeam(teamId); @@ -137,19 +143,15 @@ export class IdeationsController { description: "Ideation with given ID does not exist.", type: NotFoundErrorResponse, }) - @ApiResponse({ - status: HttpStatus.CONFLICT, - description: "Uuid does not match team member ID on Ideation.", - type: ConflictErrorResponse, - }) @ApiResponse({ status: HttpStatus.OK, description: "Successfully updated ideation.", type: IdeationResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Patch("/:ideationId") updateIdeation( - @Request() req, + @Request() req: CustomRequest, @Param("ideationId", ParseIntPipe) ideationId: number, @Param("teamId", ParseIntPipe) teamId: number, @Body() updateIdeationDto: UpdateIdeationDto, @@ -192,9 +194,10 @@ export class IdeationsController { description: "Ideation cannot be deleted when any votes exist.", type: ConflictErrorResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Delete("/:ideationId") deleteIdeation( - @Request() req, + @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { @@ -221,9 +224,10 @@ export class IdeationsController { description: "Successfully deleted ideation vote.", type: IdeationVoteResponse, }) + @Permissions(AppPermissions.OWN_TEAM) @Delete("/:ideationId/ideation-votes") deleteIdeationVote( - @Request() req, + @Request() req: CustomRequest, @Param("teamId", ParseIntPipe) teamId: number, @Param("ideationId", ParseIntPipe) ideationId: number, ) { diff --git a/src/ideations/ideations.service.ts b/src/ideations/ideations.service.ts index 2f1af98b..8207c868 100644 --- a/src/ideations/ideations.service.ts +++ b/src/ideations/ideations.service.ts @@ -3,11 +3,13 @@ import { ConflictException, Injectable, NotFoundException, + UnauthorizedException, } from "@nestjs/common"; import { PrismaService } from "../prisma/prisma.service"; import { CreateIdeationDto } from "./dto/create-ideation.dto"; import { UpdateIdeationDto } from "./dto/update-ideation.dto"; import { GlobalService } from "../global/global.service"; +import { CustomRequest } from "../global/types/CustomRequest"; @Injectable() export class IdeationsService { @@ -21,17 +23,13 @@ export class IdeationsService { teamId: number, createIdeationDto: CreateIdeationDto, ) { - const uuid = req.user.userId; const { title, description, vision } = createIdeationDto; - const voyageTeamMember = - await this.globalService.validateLoggedInAndTeamMember( - teamId, - uuid, - ); + try { const createdIdeation = await this.prisma.projectIdea.create({ data: { - voyageTeamMemberId: voyageTeamMember.id, + voyageTeamMemberId: + this.globalService.getVoyageTeamMemberId(req, teamId), title, description, vision, @@ -46,12 +44,6 @@ export class IdeationsService { } async createIdeationVote(req, teamId: number, ideationId: number) { - const uuid = req.user.userId; - const voyageTeamMember = - await this.globalService.validateLoggedInAndTeamMember( - teamId, - uuid, - ); const ideationExistsCheck = await this.prisma.projectIdea.findUnique({ where: { id: ideationId, @@ -65,14 +57,18 @@ export class IdeationsService { try { const userHasVoted = await this.hasIdeationVote( - voyageTeamMember.id, + this.globalService.getVoyageTeamMemberId(req, teamId), ideationId, ); //if user has not voted then a vote can be created if (!userHasVoted) { const createVote = await this.prisma.projectIdeaVote.create({ data: { - voyageTeamMemberId: voyageTeamMember.id, + voyageTeamMemberId: + this.globalService.getVoyageTeamMemberId( + req, + teamId, + ), projectIdeaId: ideationId, }, }); @@ -154,18 +150,17 @@ export class IdeationsService { } async updateIdeation( - req, + req: CustomRequest, ideationId: number, teamId: number, updateIdeationDto: UpdateIdeationDto, ) { - const uuid = req.user.userId; const { title, description, vision } = updateIdeationDto; - const voyageTeamMember = - await this.globalService.validateLoggedInAndTeamMember( - teamId, - uuid, - ); + + const voyageTeamMemberId = this.globalService.getVoyageTeamMemberId( + req, + teamId, + ); const ideationExistsCheck = await this.prisma.projectIdea.findUnique({ where: { @@ -183,9 +178,7 @@ export class IdeationsService { try { //only allow the user that created the idea to edit it - if ( - voyageTeamMember.id === ideationExistsCheck.voyageTeamMemberId - ) { + if (voyageTeamMemberId === ideationExistsCheck.voyageTeamMemberId) { const updatedIdeation = await this.prisma.projectIdea.update({ where: { id: ideationId, @@ -198,8 +191,8 @@ export class IdeationsService { }); return updatedIdeation; } else { - throw new ConflictException( - `voyageTeamMember.userId: ${voyageTeamMember.userId} on ideation does not match userId: ${uuid} input.`, + throw new UnauthorizedException( + "You can only update your own project ideas.", ); } } catch (e) { @@ -208,13 +201,8 @@ export class IdeationsService { } async deleteIdeation(req, teamId, ideationId: number) { - const uuid = req.user.userId; - let voteCount; - const voyageTeamMember = - await this.globalService.validateLoggedInAndTeamMember( - teamId, - req.user.userId, - ); + let voteCount: number; + const checkVotes = await this.getIdeationVoteCount(ideationId); if (checkVotes > 1) { throw new ConflictException( @@ -225,7 +213,7 @@ export class IdeationsService { await this.deleteIdeationVote(req, teamId, ideationId); voteCount = await this.getIdeationVoteCount(ideationId); //only allow the user that created the idea to delete it and only if it has no votes - if (voteCount === 0 && voyageTeamMember.userId === uuid) { + if (voteCount === 0) { const deleteIdeation = await this.prisma.projectIdea.delete({ where: { id: ideationId, @@ -238,16 +226,18 @@ export class IdeationsService { } } - async deleteIdeationVote(req, teamId: number, ideationId: number) { - const uuid = req.user.userId; - const voyageTeamMember = - await this.globalService.validateLoggedInAndTeamMember( - teamId, - uuid, - ); + async deleteIdeationVote( + req: CustomRequest, + teamId: number, + ideationId: number, + ) { + const voyageTeamMemberId = this.globalService.getVoyageTeamMemberId( + req, + teamId, + ); const ideationVote = await this.getIdeationVote( ideationId, - voyageTeamMember.id, + voyageTeamMemberId, ); try { const deleteIdeationVote = await this.prisma.projectIdeaVote.delete( @@ -268,6 +258,7 @@ export class IdeationsService { } } + // TODO: this function seems to be unused but might be useful for making new permission guard private async getTeamMemberIdByIdeation(ideationId: number) { const voyageTeamMemberId = await this.prisma.projectIdea.findFirst({ where: { From d7989543a39d15a9f0eea6852ca9bda86f50d978 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Thu, 7 Mar 2024 22:18:03 +1100 Subject: [PATCH 5/7] update changelog --- CHANGELOG.md | 2 ++ src/auth/guards/permissions.guard.ts | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbea3efc..2e03d156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ Another example [here](https://co-pilot.dev/changelog) - Restructure seed/index.ts to work with e2e tests, and add --runInBand to e2e scripts[#101](https://github.com/chingu-x/chingu-dashboard-be/pull/101) - Update changelog ([#104](https://github.com/chingu-x/chingu-dashboard-be/pull/104)) - Update test.yml to run e2e tests on pull requests to the main branch [#105](https://github.com/chingu-x/chingu-dashboard-be/pull/105) +- Add role and permission guard to some existing routes + [#112](https://github.com/chingu-x/chingu-dashboard-be/pull/112) ### Fixed - Fix failed tests in app and ideation due to the change from jwt token response to http cookies ([#98](https://github.com/chingu-x/chingu-dashboard-be/pull/98)) diff --git a/src/auth/guards/permissions.guard.ts b/src/auth/guards/permissions.guard.ts index 7e1e90c8..8447d46c 100644 --- a/src/auth/guards/permissions.guard.ts +++ b/src/auth/guards/permissions.guard.ts @@ -9,6 +9,7 @@ import { Reflector } from "@nestjs/core"; import { Observable } from "rxjs"; import { AppPermissions } from "../auth.permissions"; import { PERM_KEY } from "../../global/decorators/permissions.decorator"; +import { AppRoles } from "../auth.roles"; @Injectable() export class PermissionsGuard implements CanActivate { @@ -27,7 +28,7 @@ export class PermissionsGuard implements CanActivate { if (requiredPermissions.includes(AppPermissions.OWN_TEAM)) { // Admin can bypass this - // if (user.roles.includes(AppRoles.Admin)) return true; + if (user.roles.includes(AppRoles.Admin)) return true; if (!params.teamId) { throw new InternalServerErrorException( "This permission guard requires :teamId param", From 0604e8ee821ca462865714048356514d864151c3 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Tue, 12 Mar 2024 01:08:55 +1100 Subject: [PATCH 6/7] add role guard test to form --- src/teams/teams.service.ts | 6 +---- test/auth.e2e-spec.ts | 1 + test/forms.e2e-spec.ts | 52 ++++++++++++++------------------------ test/utils.ts | 45 +++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 38 deletions(-) diff --git a/src/teams/teams.service.ts b/src/teams/teams.service.ts index 7fdb7e01..dd2c3296 100644 --- a/src/teams/teams.service.ts +++ b/src/teams/teams.service.ts @@ -1,15 +1,11 @@ import { Injectable, NotFoundException } from "@nestjs/common"; import { UpdateTeamMemberDto } from "./dto/update-team-member.dto"; import { PrismaService } from "../prisma/prisma.service"; -import { GlobalService } from "../global/global.service"; import { publicVoyageTeamUserSelect } from "../global/selects/teams.select"; @Injectable() export class TeamsService { - constructor( - private prisma: PrismaService, - private readonly globalService: GlobalService, - ) {} + constructor(private prisma: PrismaService) {} findAll() { return this.prisma.voyageTeam.findMany({}); diff --git a/test/auth.e2e-spec.ts b/test/auth.e2e-spec.ts index 6370f922..90a79dd7 100644 --- a/test/auth.e2e-spec.ts +++ b/test/auth.e2e-spec.ts @@ -91,6 +91,7 @@ describe("AuthController e2e Tests", () => { afterAll(async () => { await app.close(); + await prisma.$disconnect(); }); describe("Creating new users POST /auth/signup", () => { diff --git a/test/forms.e2e-spec.ts b/test/forms.e2e-spec.ts index ff0fc54c..ff823e5e 100644 --- a/test/forms.e2e-spec.ts +++ b/test/forms.e2e-spec.ts @@ -4,35 +4,12 @@ import { AppModule } from "../src/app.module"; import { seed } from "../prisma/seed/seed"; import * as request from "supertest"; import * as cookieParser from "cookie-parser"; -import { extractCookieByKey } from "./utils"; -import { PrismaClient } from "@prisma/client"; - -const loginUrl = "/auth/login"; - -const loginAndGetTokens = async ( - email: string, - password: string, - app: INestApplication, -) => { - const r = await request(app.getHttpServer()).post(loginUrl).send({ - email, - password, - }); - - const access_token = extractCookieByKey( - r.headers["set-cookie"], - "access_token", - ); - const refresh_token = extractCookieByKey( - r.headers["set-cookie"], - "refresh_token", - ); - - return { access_token, refresh_token }; -}; +import { getNonAdminUser, loginAndGetTokens } from "./utils"; +import { PrismaService } from "../src/prisma/prisma.service"; describe("FormController e2e Tests", () => { let app: INestApplication; + let prisma: PrismaService; beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ @@ -42,6 +19,7 @@ describe("FormController e2e Tests", () => { await seed(); app = moduleFixture.createNestApplication(); + prisma = moduleFixture.get(PrismaService); app.useGlobalPipes(new ValidationPipe()); app.use(cookieParser()); await app.init(); @@ -49,6 +27,7 @@ describe("FormController e2e Tests", () => { afterAll(async () => { await app.close(); + await prisma.$disconnect(); }); describe("GET ALL /forms", () => { @@ -63,11 +42,21 @@ describe("FormController e2e Tests", () => { .set("Cookie", [access_token, refresh_token]) .expect(200); - const prisma = new PrismaClient(); const forms = await prisma.form.findMany(); - const formsLength = forms.length; - expect(response.body.length).toEqual(formsLength); - await prisma.$disconnect(); + expect(response.body.length).toEqual(forms.length); + }); + + it("should return 403 when accessed by a user without the admin role", async () => { + const nonAdminUser = await getNonAdminUser(); + const { access_token } = await loginAndGetTokens( + nonAdminUser.email, + "password", + app, + ); + await request(app.getHttpServer()) + .get("/forms") + .set("Cookie", [access_token]) + .expect(403); }); }); describe("GET /forms/:formId", () => { @@ -78,7 +67,6 @@ describe("FormController e2e Tests", () => { app, ); const formId = 1; - const prisma = new PrismaClient(); const expectedForm = await prisma.form.findUnique({ where: { id: formId, @@ -90,8 +78,6 @@ describe("FormController e2e Tests", () => { .expect(200); expect(response.body.id).toEqual(expectedForm.id); - - await prisma.$disconnect(); }); it("should return a 404 error for a non-existent form ID", async () => { const { access_token, refresh_token } = await loginAndGetTokens( diff --git a/test/utils.ts b/test/utils.ts index 2aa05975..aaf97d99 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -1,3 +1,9 @@ +import { PrismaClient } from "@prisma/client"; +import { INestApplication } from "@nestjs/common"; +import * as request from "supertest"; + +const prisma = new PrismaClient(); + // returns just the token export const extractResCookieValueByKey = (cookies, key) => { return cookies @@ -11,3 +17,42 @@ export const extractResCookieValueByKey = (cookies, key) => { export const extractCookieByKey = (cookie, key) => { return cookie.filter((cookie) => cookie.includes(key))[0]; }; + +export const loginAndGetTokens = async ( + email: string, + password: string, + app: INestApplication, +) => { + const r = await request(app.getHttpServer()).post("/auth/login").send({ + email, + password, + }); + + const access_token = extractCookieByKey( + r.headers["set-cookie"], + "access_token", + ); + const refresh_token = extractCookieByKey( + r.headers["set-cookie"], + "refresh_token", + ); + + return { access_token, refresh_token }; +}; + +export const getNonAdminUser = async () => { + const adminRole = await prisma.role.findUnique({ + where: { + name: "admin", + }, + }); + return prisma.user.findFirst({ + where: { + roles: { + none: { + roleId: adminRole.id, + }, + }, + }, + }); +}; From 8e6e3870a24f6f742707df11468175f862e9e8a0 Mon Sep 17 00:00:00 2001 From: Cheryl M Date: Tue, 12 Mar 2024 22:25:59 +1100 Subject: [PATCH 7/7] add more swagger api responses to form controllers --- src/forms/forms.controller.ts | 21 ++++++++++++++++++++- test/forms.e2e-spec.ts | 13 +++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/forms/forms.controller.ts b/src/forms/forms.controller.ts index ee3383ba..85e3f6eb 100644 --- a/src/forms/forms.controller.ts +++ b/src/forms/forms.controller.ts @@ -8,7 +8,11 @@ import { import { FormsService } from "./forms.service"; import { ApiOperation, ApiParam, ApiResponse, ApiTags } from "@nestjs/swagger"; import { FormResponse } from "./forms.response"; -import { NotFoundErrorResponse } from "../global/responses/errors"; +import { + ForbiddenErrorResponse, + NotFoundErrorResponse, + UnauthorizedErrorResponse, +} from "../global/responses/errors"; import { AppRoles } from "../auth/auth.roles"; import { Roles } from "../global/decorators/roles.decorator"; @@ -31,6 +35,16 @@ export class FormsController { type: FormResponse, isArray: true, }) + @ApiResponse({ + status: HttpStatus.UNAUTHORIZED, + description: "unauthorized access - not logged in", + type: UnauthorizedErrorResponse, + }) + @ApiResponse({ + status: HttpStatus.FORBIDDEN, + description: "forbidden - user does not have the required permission", + type: ForbiddenErrorResponse, + }) @Roles(AppRoles.Admin) getAllForms() { return this.formsService.getAllForms(); @@ -49,6 +63,11 @@ export class FormsController { "Successfully gets the form (with a given formId) from the database", type: FormResponse, }) + @ApiResponse({ + status: HttpStatus.UNAUTHORIZED, + description: "unauthorized access - not logged in", + type: UnauthorizedErrorResponse, + }) @ApiResponse({ status: HttpStatus.NOT_FOUND, description: "Invalid Form ID (FormId does not exist).", diff --git a/test/forms.e2e-spec.ts b/test/forms.e2e-spec.ts index ff823e5e..b99f8cf0 100644 --- a/test/forms.e2e-spec.ts +++ b/test/forms.e2e-spec.ts @@ -31,7 +31,7 @@ describe("FormController e2e Tests", () => { }); describe("GET ALL /forms", () => { - it("should successfully retrieve all forms", async () => { + it("should return 200 when successfully retrieve all forms", async () => { const { access_token, refresh_token } = await loginAndGetTokens( "jessica.williamson@gmail.com", "password", @@ -46,6 +46,10 @@ describe("FormController e2e Tests", () => { expect(response.body.length).toEqual(forms.length); }); + it("should return 401 when user is not logged in", async () => { + await request(app.getHttpServer()).get("/forms").expect(401); + }); + it("should return 403 when accessed by a user without the admin role", async () => { const nonAdminUser = await getNonAdminUser(); const { access_token } = await loginAndGetTokens( @@ -60,7 +64,7 @@ describe("FormController e2e Tests", () => { }); }); describe("GET /forms/:formId", () => { - it("should successfully retrieve a specific form by ID", async () => { + it("should return 200 when successfully retrieve a specific form by ID", async () => { const { access_token, refresh_token } = await loginAndGetTokens( "jessica.williamson@gmail.com", "password", @@ -79,6 +83,11 @@ describe("FormController e2e Tests", () => { expect(response.body.id).toEqual(expectedForm.id); }); + + it("should return 401 when user is not logged in", async () => { + await request(app.getHttpServer()).get("/forms/1").expect(401); + }); + it("should return a 404 error for a non-existent form ID", async () => { const { access_token, refresh_token } = await loginAndGetTokens( "jessica.williamson@gmail.com",