Skip to content

Commit

Permalink
feat: security patch (#3946)
Browse files Browse the repository at this point in the history
* feat: security patch

* fix: update per eric
  • Loading branch information
YazeedLoonat authored Mar 13, 2024
1 parent ef71350 commit dfd580c
Show file tree
Hide file tree
Showing 15 changed files with 323 additions and 74 deletions.
8 changes: 8 additions & 0 deletions api/prisma/seed-helpers/application-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const applicationFactory = async (optionalParams?: {
householdMember?: Prisma.HouseholdMemberCreateWithoutApplicationsInput[];
demographics?: Prisma.DemographicsCreateWithoutApplicationsInput;
multiselectQuestions?: Partial<MultiselectQuestions>[];
userId?: string;
}): Promise<Prisma.ApplicationsCreateInput> => {
let preferredUnitTypes: Prisma.UnitTypesCreateNestedManyWithoutApplicationsInput;
if (optionalParams?.unitTypeId) {
Expand Down Expand Up @@ -88,6 +89,13 @@ export const applicationFactory = async (optionalParams?: {
demographics: {
create: demographics,
},
userAccounts: optionalParams?.userId
? {
connect: {
id: optionalParams.userId,
},
}
: undefined,
};
};

Expand Down
10 changes: 7 additions & 3 deletions api/src/controllers/application.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ export class ApplicationController {
})
@ApiOkResponse({ type: Application })
async mostRecentlyCreated(
@Request() req: ExpressRequest,
@Query() queryParams: MostRecentApplicationQueryParams,
): Promise<Application> {
return await this.applicationService.mostRecentlyCreated(queryParams);
return await this.applicationService.mostRecentlyCreated(queryParams, req);
}

@Get(`csv`)
Expand Down Expand Up @@ -119,8 +120,11 @@ export class ApplicationController {
operationId: 'retrieve',
})
@ApiOkResponse({ type: Application })
async retrieve(@Param('applicationId') applicationId: string) {
return this.applicationService.findOne(applicationId);
async retrieve(
@Request() req: ExpressRequest,
@Param('applicationId') applicationId: string,
) {
return this.applicationService.findOne(applicationId, req);
}

@Post()
Expand Down
15 changes: 7 additions & 8 deletions api/src/permission-configs/permission_policy.csv
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ p, partner, asset, true, .*
p, admin, multiselectQuestion, true, .*
p, jurisdictionAdmin, multiselectQuestion, true, .*
p, partner, multiselectQuestion, true, .*
p, anonymous, multiselectQuestion, true, read

p, admin, applicationMethod, true, .*
p, jurisdictionAdmin, applicationMethod, true, .*
Expand All @@ -40,7 +39,7 @@ p, partner, propertyGroup, true, read

p, admin, amiChart, true, .*
p, jurisdictionAdmin, amiChart, true, .*
p, anonymous, amiChart, true, read
p, partner, amiChart, true, read

p, admin, applicationFlaggedSet, true, .*
p, jurisdictionAdmin, applicationFlaggedSet, true, .*
Expand All @@ -57,27 +56,27 @@ p, anonymous, listing, true, read

p, admin, reservedCommunityType, true, .*
p, jurisdictionAdmin, reservedCommunityType, true, read
p, anonymous, reservedCommunityType, true, read
p, partner, reservedCommunityType, true, read

p, admin, unitType, true, .*
p, admin, jurisdictionAdmin, true, read
p, anonymous, unitType, true, read
p, partner, unitType, true, read

p, admin, unitRentType, true, .*
p, jurisdictionAdmin, jurisdictionAdmin, true, read
p, anonymous, unitRentType, true, read
p, partner, unitRentType, true, read

p, admin, unitAccessibilityPriorityType, true, .*
p, jurisdictionAdmin, jurisdictionAdmin, true, .*
p, anonymous, unitAccessibilityPriorityType, true, read
p, partner, unitAccessibilityPriorityType, true, read

p, admin, applicationMethod, true, .*
p, jurisdictionAdmin, applicationMethod, true, .*
p, anonymous, applicationMethod, true, read
p, partner, applicationMethod, true, read

p, admin, paperApplication, true, .*
p, jurisdictionAdmin, paperApplication, true, .*
p, anonymous, paperApplication, true, read
p, partner, paperApplication, true, read

p, admin, mapLayers, true, .*
p, jurisdictionAdmin, mapLayers, true, .*
Expand Down
43 changes: 35 additions & 8 deletions api/src/services/application.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ export class ApplicationService {
where: whereClause,
});

await Promise.all(
rawApplications.map(async (application) => {
await this.authorizeAction(
user,
application.listings?.id,
permissionActions.read,
application.userId,
);
}),
);

const applications = mapTo(Application, rawApplications);

const promiseArray = applications.map((application) =>
Expand Down Expand Up @@ -135,6 +146,7 @@ export class ApplicationService {
*/
async mostRecentlyCreated(
params: MostRecentApplicationQueryParams,
req: ExpressRequest,
): Promise<Application> {
const rawApplication = await this.prisma.applications.findFirst({
select: {
Expand All @@ -150,7 +162,7 @@ export class ApplicationService {
return null;
}

return await this.findOne(rawApplication.id);
return await this.findOne(rawApplication.id, req);
}

/*
Expand Down Expand Up @@ -262,13 +274,30 @@ export class ApplicationService {
/*
this will return 1 application or error
*/
async findOne(applicationId: string): Promise<Application> {
async findOne(
applicationId: string,
req: ExpressRequest,
): Promise<Application> {
const user = mapTo(User, req['user']);
if (!user) {
throw new ForbiddenException();
}

const rawApplication = await this.findOrThrow(
applicationId,
ApplicationViews.details,
);

return mapTo(Application, rawApplication);
const application = mapTo(Application, rawApplication);

await this.authorizeAction(
user,
application.listings?.id,
permissionActions.read,
rawApplication.userId,
);

return application;
}

/*
Expand All @@ -282,7 +311,6 @@ export class ApplicationService {
if (!forPublic) {
await this.authorizeAction(
requestingUser,
dto as Application,
dto.listings.id,
permissionActions.create,
);
Expand Down Expand Up @@ -465,7 +493,6 @@ export class ApplicationService {

await this.authorizeAction(
requestingUser,
mapTo(Application, rawApplication),
rawApplication.listingId,
permissionActions.update,
);
Expand Down Expand Up @@ -616,7 +643,6 @@ export class ApplicationService {

await this.authorizeAction(
requestingUser,
mapTo(Application, application),
application.listingId,
permissionActions.delete,
);
Expand Down Expand Up @@ -674,9 +700,9 @@ export class ApplicationService {

async authorizeAction(
user: User,
application: Application,
listingId: string,
action: permissionActions,
applicantUserId?: string,
): Promise<void> {
const listingJurisdiction = await this.prisma.jurisdictions.findFirst({
where: {
Expand All @@ -689,7 +715,8 @@ export class ApplicationService {
});
await this.permissionService.canOrThrow(user, 'application', action, {
listingId,
jurisdictionId: listingJurisdiction.id,
jurisdictionId: listingJurisdiction?.id,
userId: applicantUserId,
});
}

Expand Down
19 changes: 18 additions & 1 deletion api/src/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { EmailService } from './email.service';
import { PermissionService } from './permission.service';
import { permissionActions } from '../enums/permissions/permission-actions-enum';
import { buildWhereClause } from '../utilities/build-user-where';
import { UserRole } from '../dtos/users/user-role.dto';

/*
this is the service for users
Expand Down Expand Up @@ -197,7 +198,7 @@ export class UserService {
// only update userRoles if something has changed
if (dto.userRoles && storedUser.userRoles) {
if (
requestingUser.userRoles.isAdmin &&
this.isUserRoleChangeAllowed(requestingUser, dto.userRoles) &&
!(
dto.userRoles.isAdmin === storedUser.userRoles.isAdmin &&
dto.userRoles.isJurisdictionalAdmin ===
Expand Down Expand Up @@ -858,4 +859,20 @@ export class UserService {
containsInvalidCharacters(value: string): boolean {
return value.includes('.') || value.includes('http');
}

isUserRoleChangeAllowed(
requestingUser: User,
userRoleChange: UserRole,
): boolean {
if (requestingUser?.userRoles?.isAdmin) {
return true;
} else if (requestingUser?.userRoles?.isJurisdictionalAdmin) {
if (userRoleChange?.isAdmin) {
return false;
}
return true;
}

return false;
}
}
2 changes: 2 additions & 0 deletions api/test/integration/application.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ describe('Application Controller Tests', () => {

const res = await request(app.getHttpServer())
.get(`/applications/${applicationA.id}`)
.set('Cookie', cookies)
.expect(200);

expect(res.body.applicant.firstName).toEqual(
Expand All @@ -257,6 +258,7 @@ describe('Application Controller Tests', () => {

const res = await request(app.getHttpServer())
.get(`/applications/${id}`)
.set('Cookie', cookies)
.expect(404);

expect(res.body.message).toEqual(
Expand Down
4 changes: 4 additions & 0 deletions api/test/integration/multiselect-question.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ describe('MultiselectQuestion Controller Tests', () => {

const res = await request(app.getHttpServer())
.get(`/multiselectQuestions?`)
.set('Cookie', cookies)
.expect(200);

expect(res.body.length).toBeGreaterThanOrEqual(2);
Expand Down Expand Up @@ -102,6 +103,7 @@ describe('MultiselectQuestion Controller Tests', () => {

const res = await request(app.getHttpServer())
.get(`/multiselectQuestions?${query}`)
.set('Cookie', cookies)
.expect(200);

expect(res.body.length).toBeGreaterThanOrEqual(2);
Expand All @@ -114,6 +116,7 @@ describe('MultiselectQuestion Controller Tests', () => {
const id = randomUUID();
const res = await request(app.getHttpServer())
.get(`/multiselectQuestions/${id}`)
.set('Cookie', cookies)
.expect(404);
expect(res.body.message).toEqual(
`multiselectQuestionId ${id} was requested but not found`,
Expand All @@ -127,6 +130,7 @@ describe('MultiselectQuestion Controller Tests', () => {

const res = await request(app.getHttpServer())
.get(`/multiselectQuestions/${multiselectQuestionA.id}`)
.set('Cookie', cookies)
.expect(200);

expect(res.body.text).toEqual(multiselectQuestionA.text);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,13 @@ describe('Testing Permissioning of endpoints as Jurisdictional Admin in the corr
});

it('should succeed for list endpoint', async () => {
const listing1 = await listingFactory(jurisId, prisma);
const listing1Created = await prisma.listings.create({
data: listing1,
});

await request(app.getHttpServer())
.get(`/applications?`)
.get(`/applications?listingId=${listing1Created.id}`)
.set('Cookie', cookies)
.expect(200);
});
Expand All @@ -217,8 +222,16 @@ describe('Testing Permissioning of endpoints as Jurisdictional Admin in the corr
UnitTypeEnum.oneBdrm,
);

const listing1 = await listingFactory(jurisId, prisma);
const listing1Created = await prisma.listings.create({
data: listing1,
});

const applicationA = await prisma.applications.create({
data: await applicationFactory({ unitTypeId: unitTypeA.id }),
data: await applicationFactory({
unitTypeId: unitTypeA.id,
listingId: listing1Created.id,
}),
include: {
applicant: true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,20 +208,33 @@ describe('Testing Permissioning of endpoints as Jurisdictional Admin in the wron
});

it('should succeed for list endpoint', async () => {
const listing1 = await listingFactory(jurisId, prisma);
const listing1Created = await prisma.listings.create({
data: listing1,
});

await request(app.getHttpServer())
.get(`/applications?`)
.get(`/applications?listingId=${listing1Created.id}`)
.set('Cookie', cookies)
.expect(200);
});

it('should succeed for retrieve endpoint', async () => {
it('should error as forbidden for retrieve endpoint', async () => {
const unitTypeA = await unitTypeFactorySingle(
prisma,
UnitTypeEnum.oneBdrm,
);

const listing1 = await listingFactory(jurisId, prisma);
const listing1Created = await prisma.listings.create({
data: listing1,
});

const applicationA = await prisma.applications.create({
data: await applicationFactory({ unitTypeId: unitTypeA.id }),
data: await applicationFactory({
unitTypeId: unitTypeA.id,
listingId: listing1Created.id,
}),
include: {
applicant: true,
},
Expand All @@ -230,7 +243,7 @@ describe('Testing Permissioning of endpoints as Jurisdictional Admin in the wron
await request(app.getHttpServer())
.get(`/applications/${applicationA.id}`)
.set('Cookie', cookies)
.expect(200);
.expect(403);
});

it('should error as forbidden for delete endpoint', async () => {
Expand Down
Loading

0 comments on commit dfd580c

Please sign in to comment.