Skip to content

Commit

Permalink
#2020 - Remediate high risk vulnerabilities from WAVA report (API Mas…
Browse files Browse the repository at this point in the history
…s Assignment) (#2045)

### Context

Enabled the Nestjs configuration `forbidNonWhitelisted` to force the API
to throw an error and return a "bad request" if some property not mapped
to the DTO is received (more info on
[forbidNonWhitelisted](https://docs.nestjs.com/techniques/validation)).

Currently, the API already has the configuration `whitelist` which will
remove any DTO-non-mapped-property from the payload received. Enabling
the `forbidNonWhitelisted` will allow the API to return a better
response and also restrict unknown query string parameters.

#### PR Goal

The PR intention is primarily to enable the API configuration and adjust
the HTTP call from the web application to ensure that non-extra
properties will be sent. The web application sends extra values when the
payload is not "manually created" which happens basically when the
form.io submissions objects are sent directly to the API. These payloads
contain the properties needed by the API and enforced by the DTO but
they also contain some extra non-expected values that were part of the
form.io form but do not need to be part of the HTTP request.

To enforce the DTO properties, the class-transformer package was used
(the same used by Nestjs) to convert a "plain object", like the form.io
submission, to a DTO class, also removing any extra property along the
process.

Not every form.io definition had its output converted to a class because
not every form.io form is consumed in the same way.
Please see below the ones affected.

Typed to a class to exclude extraneous values.

- educationprogram
- educationprogramoffering
- institutionlocation
- institutionprofile
- institutionprofilecreation
- institutionuserprofile
- programinformationrequest
- uploadstudentdocuments
- studentprofile
- studentexceptions

No changes are needed because the payload is controlled.

- confirmsstudentenrollment
- designationagreementdetails
- reportscholasticstandingchange
- uploadstudentdocumentsaest
- exportfinancialreports
- staffapprovalappeal
- studentrequestchange

To be removed in an upcoming PR.

- approvedeclineoffering
- approvedenydesignation
- approveeducationprogram
- createnote
- declineeducationprogram
- studentapplicationdetails
- trackstudentapplication
  • Loading branch information
andrewsignori-aot authored Jun 27, 2023
1 parent d521713 commit 0ec81a8
Show file tree
Hide file tree
Showing 53 changed files with 635 additions and 226 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,12 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-createOffering", (
const studyBreak = {
breakStartDate: "2023-12-01",
breakEndDate: "2024-01-01",
breakDays: 32,
eligibleBreakDays: 21,
ineligibleBreakDays: 11,
};
const studyPeriodBreakdown = {
totalDays: 304,
totalFundedWeeks: 42,
fundedStudyPeriodDays: 293,
unfundedStudyPeriodDays: 11,
};
const payload = {
offeringName: "Offering 1",
Expand All @@ -81,13 +84,12 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-createOffering", (
studyStartDate: "2023-09-01",
studyEndDate: "2024-06-30",
lacksStudyBreaks: false,
studyBreaks: [studyBreak],
studyPeriodBreakdown: {
totalDays: 304,
totalFundedWeeks: 42,
fundedStudyPeriodDays: 293,
unfundedStudyPeriodDays: 11,
},
studyBreaks: [
{
breakStartDate: studyBreak.breakStartDate,
breakEndDate: studyBreak.breakEndDate,
},
],
offeringType: OfferingTypes.Public,
offeringDeclaration: true,
actualTuitionCosts: 1234,
Expand Down Expand Up @@ -129,13 +131,19 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-createOffering", (
hasOfferingWILComponent: payload.hasOfferingWILComponent,
offeringWILType: null,
studyBreaks: {
totalDays: payload.studyPeriodBreakdown.totalDays,
studyBreaks: [studyBreak],
totalFundedWeeks: payload.studyPeriodBreakdown.totalFundedWeeks,
fundedStudyPeriodDays:
payload.studyPeriodBreakdown.fundedStudyPeriodDays,
unfundedStudyPeriodDays:
payload.studyPeriodBreakdown.unfundedStudyPeriodDays,
totalDays: studyPeriodBreakdown.totalDays,
studyBreaks: [
{
breakStartDate: studyBreak.breakStartDate,
breakEndDate: studyBreak.breakEndDate,
breakDays: 32,
eligibleBreakDays: 21,
ineligibleBreakDays: 11,
},
],
totalFundedWeeks: studyPeriodBreakdown.totalFundedWeeks,
fundedStudyPeriodDays: studyPeriodBreakdown.fundedStudyPeriodDays,
unfundedStudyPeriodDays: studyPeriodBreakdown.unfundedStudyPeriodDays,
},
offeringDeclaration: payload.offeringDeclaration,
assessedDate: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,12 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-updateProgramOffer
const studyBreak = {
breakStartDate: "2023-12-01",
breakEndDate: "2024-01-01",
breakDays: 32,
eligibleBreakDays: 21,
ineligibleBreakDays: 11,
};
const studyPeriodBreakdown = {
totalDays: 304,
totalFundedWeeks: 42,
fundedStudyPeriodDays: 293,
unfundedStudyPeriodDays: 11,
};
const payload = {
offeringName: "Updated offering name",
Expand All @@ -89,13 +92,12 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-updateProgramOffer
studyStartDate: "2023-09-01",
studyEndDate: "2024-06-30",
lacksStudyBreaks: false,
studyBreaks: [studyBreak],
studyPeriodBreakdown: {
totalDays: 304,
totalFundedWeeks: 42,
fundedStudyPeriodDays: 293,
unfundedStudyPeriodDays: 11,
},
studyBreaks: [
{
breakStartDate: studyBreak.breakStartDate,
breakEndDate: studyBreak.breakEndDate,
},
],
offeringType: OfferingTypes.Public,
offeringDeclaration: true,
actualTuitionCosts: 1234,
Expand Down Expand Up @@ -133,13 +135,19 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-updateProgramOffer
hasOfferingWILComponent: payload.hasOfferingWILComponent,
offeringWILType: null,
studyBreaks: {
totalDays: payload.studyPeriodBreakdown.totalDays,
studyBreaks: [studyBreak],
totalFundedWeeks: payload.studyPeriodBreakdown.totalFundedWeeks,
fundedStudyPeriodDays:
payload.studyPeriodBreakdown.fundedStudyPeriodDays,
unfundedStudyPeriodDays:
payload.studyPeriodBreakdown.unfundedStudyPeriodDays,
totalDays: studyPeriodBreakdown.totalDays,
studyBreaks: [
{
breakStartDate: studyBreak.breakStartDate,
breakEndDate: studyBreak.breakEndDate,
breakDays: 32,
eligibleBreakDays: 21,
ineligibleBreakDays: 11,
},
],
totalFundedWeeks: studyPeriodBreakdown.totalFundedWeeks,
fundedStudyPeriodDays: studyPeriodBreakdown.fundedStudyPeriodDays,
unfundedStudyPeriodDays: studyPeriodBreakdown.unfundedStudyPeriodDays,
},
offeringDeclaration: payload.offeringDeclaration,
assessedDate: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,8 @@ describe("EducationProgramOfferingInstitutionsController(e2e)-validateOffering",
{
breakStartDate: "2023-12-01",
breakEndDate: "2023-12-18",
breakDays: 18,
eligibleBreakDays: 18,
ineligibleBreakDays: 0,
},
],
studyPeriodBreakdown: {
totalDays: 304,
totalFundedWeeks: 42,
fundedStudyPeriodDays: 293,
unfundedStudyPeriodDays: 11,
},
offeringType: OfferingTypes.Public,
offeringDeclaration: true,
actualTuitionCosts: 1234,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ describe("EducationProgramInstitutionsController(e2e)-createEducationProgram", (

it("Should create an education program when valid data is passed.", async () => {
// Arrange
const programStatus = ProgramStatus.Approved;
const payload = {
name: "Education Program test",
description: "Education Program description...",
Expand Down Expand Up @@ -83,17 +84,16 @@ describe("EducationProgramInstitutionsController(e2e)-createEducationProgram", (
hasTravel: "no",
hasIntlExchange: "no",
programDeclaration: true,
programStatus: ProgramStatus.Approved,
hasOfferings: false,
};
const formService = await getProviderInstanceForModule(
testingModule,
AppInstitutionsModule,
FormService,
);
formService.dryRunSubmission = jest
.fn()
.mockResolvedValue({ valid: true, data: { data: payload } });
formService.dryRunSubmission = jest.fn().mockResolvedValue({
valid: true,
data: { data: { ...payload, programStatus } },
});
const institutionUserToken = await getInstitutionToken(
InstitutionTokenTypes.CollegeFUser,
);
Expand Down Expand Up @@ -134,7 +134,7 @@ describe("EducationProgramInstitutionsController(e2e)-createEducationProgram", (
eslEligibility: payload.eslEligibility,
hasJointInstitution: payload.hasJointInstitution,
hasJointDesignatedInstitution: null,
programStatus: payload.programStatus,
programStatus,
programIntensity: payload.programIntensity,
institutionProgramCode: payload.institutionProgramCode,
minHoursWeek: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ import {
BadRequestException,
Body,
Controller,
DefaultValuePipe,
Get,
NotFoundException,
Param,
ParseBoolPipe,
ParseIntPipe,
Patch,
Post,
Expand Down Expand Up @@ -38,6 +36,7 @@ import { getISODateOnlyString, CustomNamedError } from "@sims/utilities";
import {
ActiveApplicationDataAPIOutDTO,
ActiveApplicationSummaryAPIOutDTO,
ActiveApplicationSummaryQueryStringAPIInDTO,
transformToActiveApplicationDataAPIOutDTO,
} from "./models/application.dto";
import BaseController from "../BaseController";
Expand All @@ -50,10 +49,7 @@ import {
} from "./models/institution-location.dto";
import { FormNames } from "../../services/form/constants";
import { transformAddressDetailsForAddressBlockForm } from "../utils/address-utils";
import {
ApplicationStatusPaginationOptionsAPIInDTO,
PaginatedResultsAPIOutDTO,
} from "../models/pagination.dto";
import { PaginatedResultsAPIOutDTO } from "../models/pagination.dto";
import { DUPLICATE_INSTITUTION_LOCATION_CODE } from "../../constants";
import { InstitutionLocationModel } from "../../services/institution-location/institution-location.models";

Expand Down Expand Up @@ -147,22 +143,21 @@ export class InstitutionLocationInstitutionsController extends BaseController {
* Get all active application of a location in an institution
* with application_status is completed.
* @param locationId location id.
* @param pagination options to execute the pagination.
* @param queryStringDTO represents all query strings expected to be received by
* this endpoint (pagination options and archive filter).
* @param archived archive value of applications requested by user.
* @returns Student active application list of an institution location.
*/
@HasLocationAccess("locationId")
@Get(":locationId/active-applications")
async getActiveApplications(
@Param("locationId", ParseIntPipe) locationId: number,
@Query() pagination: ApplicationStatusPaginationOptionsAPIInDTO,
@Query("archived", new DefaultValuePipe(false), ParseBoolPipe)
archived: boolean,
@Query() queryStringDTO: ActiveApplicationSummaryQueryStringAPIInDTO,
): Promise<PaginatedResultsAPIOutDTO<ActiveApplicationSummaryAPIOutDTO>> {
const applications = await this.applicationService.getActiveApplications(
locationId,
pagination,
archived,
queryStringDTO,
queryStringDTO.archived,
);

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import {
} from "@sims/sims-db";
import { StudyBreak } from "../../confirmation-of-enrollment/models/confirmation-of-enrollment.dto";
import { ApplicationScholasticStandingStatus } from "../../../services/application/application.models";
import { ApplicationStatusPaginationOptionsAPIInDTO } from "../../models/pagination.dto";
import { ToBoolean } from "../../../utilities/class-transform";
import { IsBoolean } from "class-validator";

export class ActiveApplicationDataAPIOutDTO {
applicationProgramName: string;
Expand Down Expand Up @@ -44,6 +47,15 @@ export class ActiveApplicationSummaryAPIOutDTO {
applicationScholasticStandingStatus: ApplicationScholasticStandingStatus;
}

/**
* Query string options available for active applications summary.
*/
export class ActiveApplicationSummaryQueryStringAPIInDTO extends ApplicationStatusPaginationOptionsAPIInDTO {
@IsBoolean()
@ToBoolean()
archived: boolean;
}

/**
* Transformation util for Active application.
* @param application application object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ describe("InstitutionAESTController(e2e)-createInstitution", () => {
provinceState: "BC",
canadaPostalCode: "V1V1V1",
},
clientType: "aest",
};
const endpoint = "/aest/institution";
const token = await getAESTToken(AESTGroups.BusinessAdministrators);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export function setGlobalPipes(app: INestApplication) {
app.useGlobalPipes(
new ValidationPipe({
whitelist: true,
forbidNonWhitelisted: true,
transform: true,
transformOptions: { enableImplicitConversion: true },
disableErrorMessages: false,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./to-boolean";
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { Transform } from "class-transformer";

/**
* Converts a plain property to boolean.
* @returns function that will return the converted boolean value.
*/
export const ToBoolean = (): ((target: unknown, key: string) => void) => {
return (target: unknown, key: string) => {
return Transform(
({ obj }) => {
return valueToBoolean(obj[key]);
},
{
toClassOnly: true,
},
)(target, key);
};
};

/**
* Converts an object to boolean value, if not a boolean already.
* @param value expected value to be converted, if needed.
* @returns boolean value if object is defined.
*/
const valueToBoolean = (value: unknown): boolean | undefined => {
if (value === null || value === undefined) {
return undefined;
}
if (typeof value === "boolean") {
return value;
}
switch ((value as string).toLowerCase()) {
case "true":
return true;
case "false":
return false;
default:
return undefined;
}
};
24 changes: 24 additions & 0 deletions sources/packages/web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 0ec81a8

Please sign in to comment.