Skip to content

Commit

Permalink
Merge pull request #229 from chingu-x/fix-post-new-tech
Browse files Browse the repository at this point in the history
Fix POST voyages/teams/{teamId}/techs bug
  • Loading branch information
JoshuaHinman authored Jan 7, 2025
2 parents 80a9747 + 7675c88 commit 17ed731
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 30 deletions.
2 changes: 0 additions & 2 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"
. "$(dirname "$0")/common.sh"

yarn lint-staged
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@
- Updated nestjs packages to latest version ([#233])(https://github.com/chingu-x/chingu-dashboard-be/pull/233)
- Refactoring of email service + unit tests ([#232](https://github.com/chingu-x/chingu-dashboard-be/pull/232))
- Added version release link to Swagger docs ([#218](https://github.com/chingu-x/chingu-dashboard-be/pull/231))
- Fixed POST voyages/teams/{teamId}/techs bug , verify that categoryId is owned by correct team ([#229](https://github.com/chingu-x/chingu-dashboard-be/pull/229))
### Added

### Changed

### Fixed


- fixed POST voyages/teams/{teamId}/techs bug , verify that categoryId is owned by correct team ([#229](https://github.com/chingu-x/chingu-dashboard-be/pull/229))
### Removed

## [v1.1.0-alpha]
Expand Down
38 changes: 37 additions & 1 deletion src/ability/conditions/voyage-teams.ability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { UserReq } from "../../global/types/CustomRequest";
import { Action } from "../ability.factory/ability.factory";
import { ForbiddenError } from "@casl/ability";
import { VoyageTeam } from "@prisma/client";
import { ForbiddenException } from "@nestjs/common";
import { ForbiddenException, NotFoundException } from "@nestjs/common";
import { abilityFactory } from "./shared.ability";
import prisma from "../../prisma/client";

export const manageOwnVoyageTeamWithIdParam = (
user: UserReq,
Expand All @@ -17,6 +18,41 @@ export const manageOwnVoyageTeamWithIdParam = (
}
};

export const userCanChangeCategory = async (
categoryId: number,
user: UserReq,
teamId?: number,
) => {
if (user.roles?.includes("admin")) return;

let match;
try {
match = await prisma.techStackCategory.findUnique({
where: {
id: categoryId,
},
});
} catch {
throw new NotFoundException(`Category ${categoryId} not found`);
}

if (teamId && teamId !== match?.voyageTeamId) {
throw new ForbiddenException(
`Team ${teamId} cannot change category ${categoryId}`,
);
}

let permission = false;
for (const team of user.voyageTeams) {
if (team.teamId === match?.voyageTeamId) permission = true;
}

if (!permission) {
throw new ForbiddenException(
`This user cannot change category ${categoryId}`,
);
}
};
/***
For future reference, this works, but it has to be of voyageTeamType *** without a special select statement ***
So it should be used for cases when a team Id is not supplied (this would be an extra database query in most cases)
Expand Down
2 changes: 2 additions & 0 deletions src/teams/teams.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { CustomRequest } from "@/global/types/CustomRequest";
import { VoyageTeamMemberUpdateResponse } from "./teams.response";
import { UpdateTeamMemberDto } from "./dto/update-team-member.dto";
import { toBeArray } from "jest-extended";

expect.extend({ toBeArray });

describe("TeamsController", () => {
let controller: TeamsController;

Expand Down
30 changes: 6 additions & 24 deletions src/techs/techs.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import { CreateTechStackCategoryDto } from "./dto/create-techstack-category.dto"
import { UpdateTechStackCategoryDto } from "./dto/update-techstack-category.dto";
import { UpdateTechSelectionDto } from "./dto/update-tech-selections.dto";
import { UpdateTeamTechDto } from "./dto/update-tech.dto";
import { CustomRequest, UserReq } from "../global/types/CustomRequest";
import { CustomRequest } from "../global/types/CustomRequest";
import { manageOwnVoyageTeamWithIdParam } from "@/ability/conditions/voyage-teams.ability";
import { userCanChangeCategory } from "@/ability/conditions/voyage-teams.ability";

const MAX_SELECTION_COUNT = 3;

Expand Down Expand Up @@ -129,9 +130,10 @@ export class TechsService {
//check for valid teamId
await this.validateTeamId(teamId);

await this.userCanChangeCategory(
await userCanChangeCategory(
createTeamTechDto.techCategoryId,
req.user,
teamId,
);

try {
Expand Down Expand Up @@ -361,7 +363,7 @@ export class TechsService {
techStackCategoryId: number,
updateTechStackCategoryDto: UpdateTechStackCategoryDto,
) {
await this.userCanChangeCategory(techStackCategoryId, req.user);
await userCanChangeCategory(techStackCategoryId, req.user);

try {
const newTechStackCategory =
Expand Down Expand Up @@ -389,7 +391,7 @@ export class TechsService {
req: CustomRequest,
techStackCategoryId: number,
) {
await this.userCanChangeCategory(techStackCategoryId, req.user);
await userCanChangeCategory(techStackCategoryId, req.user);

try {
await this.prisma.techStackCategory.delete({
Expand Down Expand Up @@ -516,24 +518,4 @@ export class TechsService {
throw e;
}
}

private async userCanChangeCategory(categoryId: number, user: UserReq) {
if (user.roles?.includes("admin")) return;

const userVoyageTeams = user.voyageTeams;
const match = await this.prisma.techStackCategory.findUnique({
where: {
id: categoryId,
},
});

const permission = userVoyageTeams.some(
(team) => team.teamId === match?.voyageTeamId,
);
if (!permission) {
throw new ForbiddenException(
`This user cannot change category ${categoryId}`,
);
}
}
}
3 changes: 2 additions & 1 deletion test/teams.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import { loginAndGetTokens } from "./utils";
import * as cookieParser from "cookie-parser";
import { CASLForbiddenExceptionFilter } from "@/exception-filters/casl-forbidden-exception.filter";

expect.extend({ toBeArray });

//Logged in user is Jessica Williamson for admin routes /teams and /teams/voyages/:voyageid
//Logged in user is Dan ko for team member routes /teams/:teamid and /teams/:teamid/members
//Dan Ko is part of the team with team id 4
expect.extend({ toBeArray });
describe("Teams Controller (e2e)", () => {
let app: INestApplication;
let prisma: PrismaService;
Expand Down
22 changes: 22 additions & 0 deletions test/techs.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ describe("Techs Controller (e2e)", () => {
);
});
});

it("should return 403 if a user of other team tries to add a tech stack item", async () => {
const { access_token } = await loginAndGetTokens(
"[email protected]",
Expand All @@ -233,6 +234,27 @@ describe("Techs Controller (e2e)", () => {
.expect(403);
});

it("should return 403 if a user tries to add a tech stack item to a category for another team", async () => {
const { access_token } = await loginAndGetTokens(
"[email protected]",
"password",
app,
);
const teamId: number = 1;
const teamMemberId: number = 1;
const categoryId: number = 51;

return request(app.getHttpServer())
.post(`/voyages/teams/${teamId}/techs`)
.set("Cookie", access_token)
.send({
techName: newTechName + "678",
techCategoryId: categoryId,
voyageTeamMemberId: teamMemberId,
})
.expect(403);
});

it("should return 404 if invalid teamId provided", async () => {
const teamId: number = 9999999;
const teamMemberId: number = 8;
Expand Down
2 changes: 2 additions & 0 deletions test/unverifiedUser.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import { loginAndGetTokens } from "./utils";
import * as cookieParser from "cookie-parser";
import { CASLForbiddenExceptionFilter } from "@/exception-filters/casl-forbidden-exception.filter";
import { toBeObject } from "jest-extended";

expect.extend({ toBeObject });

//Logged in user is Yoshi Amano
//Tests are for routes that are accessible to unverified users
// - route /auth/resend-email is not tested, since this case is already covered in existing e2e test
Expand Down
5 changes: 3 additions & 2 deletions test/users.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import { seed } from "@Prisma/seed/seed";
import { getUseridFromEmail, loginAndGetTokens } from "./utils";
import * as cookieParser from "cookie-parser";
import { CASLForbiddenExceptionFilter } from "@/exception-filters/casl-forbidden-exception.filter";
import { toBeArray, toBeObject } from "jest-extended";
import { toBeObject, toBeArray } from "jest-extended";

expect.extend({ toBeObject, toBeArray });

expect.extend({ toBeArray, toBeObject });
//Logged in user is Jessica Williamson for admin routes
//Logged in user is Dan ko for non admin routes

Expand Down

0 comments on commit 17ed731

Please sign in to comment.