Skip to content

Commit

Permalink
fix(role.utility.ts): updated role-utility.ts
Browse files Browse the repository at this point in the history 
Implemented two API security role check functions and replaced declarations of deprecated functions
throughout the codebase
  • Loading branch information
@alejandrosaenz117
alejandrosaenz117 committed May 10, 2021
1 parent 2cdc01a commit f0fc0c5
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 104 deletions.
11 changes: 11 additions & 0 deletions src/routes/assessment.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ describe('Assessment Controller', () => {
params: {
id: savedAsset.id,
},
userAssets: [1],
userTeams: [savedTeam],
});
await assessmentController.getAssessmentsByAssetId(request, response);
Expand All @@ -231,5 +232,15 @@ describe('Assessment Controller', () => {
});
await assessmentController.getAssessmentsByAssetId(request3, response3);
expect(response3.statusCode).toBe(400);
const response4 = new MockExpressResponse();
const request4 = new MockExpressRequest({
params: {
id: savedAsset.id,
},
userAssets: [2],
userTeams: [savedTeam],
});
await assessmentController.getAssessmentsByAssetId(request4, response4);
expect(response4.statusCode).toBe(404);
});
});
63 changes: 25 additions & 38 deletions src/routes/assessment.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ import { Organization } from '../entity/Organization';
import { Report } from '../classes/Report';
import { Config } from '../entity/Config';
import {
hasAssessmentAccess,
hasAssetAccess,
hasTesterAssetAccess,
hasAssetReadAccess,
hasAssetWriteAccess,
} from '../utilities/role.utility';
const userController = require('../routes/user.controller');

Expand All @@ -31,20 +30,17 @@ export const getAssessmentsByAssetId = async (
if (isNaN(+req.params.id)) {
return res.status(400).json('Invalid Asset ID');
}
const assetAccess = await hasAssetAccess(req, +req.params.id);
if (!assetAccess) {
return res.status(404).json('Asset not found');
}
const asset = await getConnection()
.getRepository(Asset)
.findOne(req.params.id, { relations: ['organization'] });
if (!asset) {
return res.status(404).json('Asset does not exist');
}
const hasTesterAccess = await hasTesterAssetAccess(
req,
asset.organization.id
);
const assetAccess = await hasAssetReadAccess(req, asset.id);
if (!assetAccess) {
return res.status(404).json('Asset not found');
}
const hasTesterAccess = await hasAssetWriteAccess(req, asset.id);
const assessments = await getConnection()
.getRepository(Assessment)
.createQueryBuilder('assessment')
Expand Down Expand Up @@ -77,10 +73,6 @@ export const getAssessmentVulns = async (req: UserRequest, res: Response) => {
if (isNaN(+req.params.id)) {
return res.status(400).json('Invalid Assessment ID');
}
const hasReadAccess = await hasAssessmentAccess(req, +req.params.id);
if (!hasReadAccess) {
return res.status(404).json('Assessment not found');
}
const assessment = await getConnection()
.getRepository(Assessment)
.createQueryBuilder('assessment')
Expand All @@ -92,10 +84,11 @@ export const getAssessmentVulns = async (req: UserRequest, res: Response) => {
if (!assessment) {
return res.status(404).json('Assessment does not exist');
}
const hasAccess = await hasTesterAssetAccess(
req,
assessment.asset.organization.id
);
const hasReadAccess = await hasAssetReadAccess(req, assessment.asset.id);
if (!hasReadAccess) {
return res.status(404).json('Assessment not found');
}
const hasTesterAccess = await hasAssetWriteAccess(req, assessment.asset.id);
const vulnerabilities = await getConnection()
.getRepository(Vulnerability)
.find({
Expand All @@ -104,7 +97,7 @@ export const getAssessmentVulns = async (req: UserRequest, res: Response) => {
if (!vulnerabilities) {
return res.status(404).json('Vulnerabilities do not exist');
}
return res.status(200).json({ vulnerabilities, readOnly: !hasAccess });
return res.status(200).json({ vulnerabilities, readOnly: !hasTesterAccess });
};
/**
* @description Create assessment
Expand All @@ -122,7 +115,7 @@ export const createAssessment = async (req: UserRequest, res: Response) => {
if (!asset) {
return res.status(404).json('Asset does not exist');
}
const hasAccess = await hasTesterAssetAccess(req, asset.organization.id);
const hasAccess = await hasAssetWriteAccess(req, asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
Expand Down Expand Up @@ -185,14 +178,11 @@ export const getAssessmentById = async (req: UserRequest, res: Response) => {
if (!assessment) {
return res.status(404).json('Assessment does not exist');
}
const hasAccess = await hasAssessmentAccess(req, assessment.id);
if (!hasAccess) {
const hasReadAccess = await hasAssetReadAccess(req, assessment.asset.id);
if (!hasReadAccess) {
return res.status(404).json('Assessment not found');
}
const hasTesterAccess = await hasTesterAssetAccess(
req,
assessment.asset.organization.id
);
const hasTesterAccess = await hasAssetWriteAccess(req, assessment.asset.id);
return res.status(200).json({ assessment, readOnly: !hasTesterAccess });
};
/**
Expand All @@ -210,11 +200,11 @@ export const updateAssessmentById = async (req: UserRequest, res: Response) => {
}
let assessment = await getConnection()
.getRepository(Assessment)
.findOne(req.params.assessmentId, { relations: ['testers'] });
.findOne(req.params.assessmentId, { relations: ['testers', 'asset'] });
if (!assessment) {
return res.status(404).json('Assessment does not exist');
}
const hasAccess = await hasAssessmentAccess(req, assessment.id);
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
Expand Down Expand Up @@ -255,13 +245,6 @@ export const queryReportDataByAssessment = async (
if (isNaN(+req.params.assessmentId)) {
return res.status(400).json('Invalid Assessment ID');
}
const hasReadAccess = await hasAssessmentAccess(
req,
+req.params.assessmentId
);
if (!hasReadAccess) {
return res.status(404).json('Assessment not found');
}
const assessment = await getConnection()
.getRepository(Assessment)
.createQueryBuilder('assessment')
Expand All @@ -285,6 +268,10 @@ export const queryReportDataByAssessment = async (
.findOne(assessmentForId.asset.id, {
relations: ['organization'],
});
const hasReadAccess = await hasAssetReadAccess(req, asset.id);
if (!hasReadAccess) {
return res.status(404).json('Assessment not found');
}
const organization = await getConnection()
.getRepository(Organization)
.findOne(asset.organization.id);
Expand Down Expand Up @@ -337,11 +324,11 @@ export const deleteAssessmentById = async (req: UserRequest, res: Response) => {
}
const assessment = await getConnection()
.getRepository(Assessment)
.findOne(req.params.assessmentId);
.findOne(req.params.assessmentId, { relations: ['asset'] });
if (!assessment) {
return res.status(404).send('Assessment does not exist.');
} else {
const hasAccess = await hasAssessmentAccess(req, assessment.id);
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
Expand Down
36 changes: 36 additions & 0 deletions src/routes/asset.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,45 @@ describe('Asset Controller', () => {
params: {
assetId: 1,
},
userAssets: [1],
});
const response = new MockExpressResponse();
await assetController.getAssetById(request, response);
expect(response.statusCode).toBe(200);
});
test('Get asset by id failure no access', async () => {
const org: Organization = {
id: null,
name: 'testOrg',
asset: null,
status: 'A',
teams: null,
};
await getConnection().getRepository(Organization).insert(org);
const savedOrg = await getConnection()
.getRepository(Organization)
.findOne(1);
const assessments: Assessment[] = [];
const asset: Asset = {
id: null,
name: 'Test Asset',
status: 'A',
assessment: assessments,
organization: savedOrg,
jira: null,
teams: null,
};
await getConnection().getRepository(Asset).insert(asset);
const request = new MockExpressRequest({
params: {
assetId: 1,
},
userAssets: [2],
});
const response = new MockExpressResponse();
await assetController.getAssetById(request, response);
expect(response.statusCode).toBe(404);
});
test('get asset by id failure', async () => {
const request = new MockExpressRequest({
params: {
Expand Down Expand Up @@ -504,6 +538,7 @@ describe('Asset Controller', () => {
params: {
assetId: 2,
},
userAssets: [2],
});
const response = new MockExpressResponse();
await assetController.getAssetById(request, response);
Expand Down Expand Up @@ -545,6 +580,7 @@ describe('Asset Controller', () => {
params: {
assetId: 1,
},
userAssets: [1],
});
const response = new MockExpressResponse();
await assetController.getAssetById(request, response);
Expand Down
10 changes: 5 additions & 5 deletions src/routes/asset.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Organization } from '../entity/Organization';
import { status } from '../enums/status-enum';
import { encrypt } from '../utilities/crypto.utility';
import { Jira } from '../entity/Jira';
import { hasAssetAccess, hasOrgAccess } from '../utilities/role.utility';
import { hasAssetReadAccess, hasOrgAccess } from '../utilities/role.utility';
/**
* @description Get organization assets
* @param {UserRequest} req
Expand Down Expand Up @@ -216,16 +216,16 @@ export const getAssetById = async (req: UserRequest, res: Response) => {
if (isNaN(+req.params.assetId)) {
return res.status(400).json('Invalid Asset ID');
}
const hasAccess = await hasAssetAccess(req, +req.params.assetId);
if (!hasAccess) {
return res.status(404).json('Asset not found');
}
const asset = await getConnection()
.getRepository(Asset)
.findOne(req.params.assetId, { relations: ['jira'] });
if (!asset) {
return res.status(404).send('Asset does not exist');
}
const hasAccess = await hasAssetReadAccess(req, asset.id);
if (!hasAccess) {
return res.status(404).json('Asset not found');
}
if (asset.jira) {
delete asset.jira.apiKey;
}
Expand Down
30 changes: 15 additions & 15 deletions src/routes/vulnerability.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import { File } from '../entity/File';
import { ProblemLocation } from '../entity/ProblemLocation';
import { Resource } from '../entity/Resource';
import { exportToJiraIssue } from '../utilities/jira.utility';
import { JiraInit } from 'src/interfaces/jira/jira-init.interface';
import { JiraInit } from '../interfaces/jira/jira-init.interface';
import { Jira } from '../entity/Jira';
import {
hasAssessmentAccess,
hasTesterAssetAccess,
hasAssetReadAccess,
hasAssetWriteAccess,
} from '../utilities/role.utility';
const fileUploadController = require('../routes/file-upload.controller');

Expand Down Expand Up @@ -45,14 +45,11 @@ export const getVulnById = async (req: UserRequest, res: Response) => {
})
.select(['assessment', 'asset', 'organization'])
.getOne();
const hasReadAccess = await hasAssessmentAccess(req, vuln.assessment.id);
const hasReadAccess = await hasAssetReadAccess(req, assessment.asset.id);
if (!hasReadAccess) {
return res.status(404).json('Vulnerability not found');
}
const hasAccess = await hasTesterAssetAccess(
req,
assessment.asset.organization.id
);
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
const jira = await getConnection()
.getRepository(Jira)
.findOne({ where: { asset: assessment.asset } });
Expand Down Expand Up @@ -83,12 +80,15 @@ export const deleteVulnById = async (req: UserRequest, res: Response) => {
if (!vuln) {
return res.status(404).send('Vulnerability does not exist.');
} else {
const hasAccess = await hasAssessmentAccess(req, vuln.assessment.id);
const assessment = await getConnection()
.getRepository(Assessment)
.findOne(req.body.assessment, { relations: ['asset'] });
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
await getConnection().getRepository(Vulnerability).delete(vuln);
res
return res
.status(200)
.json(`Vulnerability #${vuln.id}: "${vuln.name}" successfully deleted`);
}
Expand All @@ -107,11 +107,11 @@ export const patchVulnById = async (req: UserRequest, res: Response) => {
}
const assessment = await getConnection()
.getRepository(Assessment)
.findOne(req.body.assessment);
.findOne(req.body.assessment, { relations: ['asset'] });
if (!assessment) {
return res.status(404).json('Assessment does not exist');
}
const hasAccess = await hasAssessmentAccess(req, assessment.id);
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
Expand Down Expand Up @@ -214,11 +214,11 @@ export const createVuln = async (req: UserRequest, res: Response) => {
}
const assessment = await getConnection()
.getRepository(Assessment)
.findOne(req.body.assessment);
.findOne(req.body.assessment, { relations: ['asset'] });
if (!assessment) {
return res.status(404).json('Assessment does not exist');
}
const hasAccess = await hasAssessmentAccess(req, assessment.id);
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
Expand Down Expand Up @@ -362,7 +362,7 @@ export const exportToJira = async (req: UserRequest, res: Response) => {
const assessment = await getConnection()
.getRepository(Assessment)
.findOne(vuln.assessment.id, { relations: ['asset'] });
const hasAccess = await hasAssessmentAccess(req, assessment.id);
const hasAccess = await hasAssetWriteAccess(req, assessment.asset.id);
if (!hasAccess) {
return res.status(403).json('Authorization is required');
}
Expand Down
8 changes: 5 additions & 3 deletions src/utilities/puppeteer.utility.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Response } from 'express';
import { insertReportAuditRecord } from '../routes/report-audit.controller';
import puppeteer = require('puppeteer');
import * as path from 'path';
import { hasAssessmentAccess } from './role.utility';
import { hasAssetReadAccess } from './role.utility';
const fs = require('fs');
/**
* @description API backend for report generation with Puppeteer
Expand All @@ -16,9 +16,11 @@ export const generateReport = async (req: UserRequest, res: Response) => {
if (!req.body.orgId || !req.body.assetId || !req.body.assessmentId) {
return res.status(400).send('Invalid report parameters');
}
const hasReadAccess = await hasAssessmentAccess(req, +req.body.assessmentId);
const hasReadAccess = await hasAssetReadAccess(req, +req.body.assetId);
if (!hasReadAccess) {
return res.status(404).json('Assessment not found');
return res
.status(404)
.json('Failed to generate report. Please contact an administrator.');
}
const url =
process.env.NODE_ENV === 'production'
Expand Down
Loading

0 comments on commit f0fc0c5

Please sign in to comment.