Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Prod] Resolve issues found during pen test #2087

Merged
merged 27 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
cc7eb2f
v2.0.19
thewatermethod Apr 3, 2024
4a3deb2
Update readme
thewatermethod Apr 3, 2024
dc0209f
escape HTML function in common
thewatermethod Apr 3, 2024
f08a19b
v2.1.0
thewatermethod Apr 3, 2024
c7a37ab
Remove function to use lodash version instead
thewatermethod Apr 3, 2024
859d687
v2.1.2
thewatermethod Apr 3, 2024
9eafa0b
Catch XSS
thewatermethod Apr 3, 2024
c9987d7
Use safe parse
thewatermethod Apr 3, 2024
a9e02a2
try to de-flakify mocks
thewatermethod Apr 3, 2024
b2689a5
Fix some additional tests and improve mocks
thewatermethod Apr 3, 2024
efbeab4
Is this the source of OOM?
thewatermethod Apr 4, 2024
b5f5634
Try without do... while
thewatermethod Apr 4, 2024
d0aa29b
revert changes to createGrant util
thewatermethod Apr 4, 2024
a53aaed
Move hooks to own folder from subfolder, to free up jest memory
thewatermethod Apr 4, 2024
7eae158
Update mock paths
thewatermethod Apr 4, 2024
1c743d7
rearrange test
thewatermethod Apr 4, 2024
06894fc
Tweak test
thewatermethod Apr 4, 2024
8a977da
add some unit tests for coverage
thewatermethod Apr 5, 2024
a842351
Remove duplicative check that implicitly cannot be tested
thewatermethod Apr 5, 2024
ccefab2
Add another missing test
thewatermethod Apr 5, 2024
588173a
Add ARC test
thewatermethod Apr 5, 2024
37514f2
Add test for ARO hooks
thewatermethod Apr 5, 2024
4a4db2b
add tests for model directory
thewatermethod Apr 5, 2024
4b72a4c
Remove console.log
thewatermethod Apr 5, 2024
83089e2
Add more tests
thewatermethod Apr 5, 2024
b2f7a52
Merge remote-tracking branch 'origin/main' into mb/TTAHUB-2575/escape…
thewatermethod Apr 8, 2024
4218e12
Merge pull request #2076 from HHS/mb/TTAHUB-2575/escape-html
thewatermethod Apr 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/test-backend-ci
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ main(){
check_exit "$?"

# then list through the folders and run the tests
targets=("lib" "middleware" "models" "policies" "routes" "scopes" "services" "tools" "widgets" "goalServices")
targets=("lib" "middleware" "models" "policies" "routes" "scopes" "services" "tools" "widgets" "goalServices" "hooks")

for target in "${targets[@]}"; do
# jest command to
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"db:seed:prod": "node_modules/.bin/sequelize db:seed:all --options-path .production.sequelizerc",
"db:seed:undo": "node_modules/.bin/sequelize db:seed:undo:all",
"db:seed:undo:prod": "node_modules/.bin/sequelize db:seed:undo:all --options-path .production.sequelizerc",
"docker:deps": "docker-compose run --rm backend yarn install && docker-compose run --rm frontend yarn install",
"docker:deps": "docker-compose run --rm backend yarn install && docker-compose run --rm frontend yarn install && docker-compose run --rm testingonly yarn install",
"docker:reset": "./bin/reset-all",
"docker:start": "docker-compose up",
"docker:start:debug": "docker-compose --compatibility -f docker-compose.yml -f docker-compose.debug.yml up",
Expand Down
3 changes: 3 additions & 0 deletions packages/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ Note: On Windows you will need to use `yarn add @ttahub/[email protected]` to update

## Versions

## 2.0.19
Add "escapeHTML" function

## 1.4.0
Add SUPPORT_TYPE

Expand Down
2 changes: 1 addition & 1 deletion packages/common/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ttahub/common",
"version": "2.0.18",
"version": "2.1.2",
"description": "The purpose of this package is to reduce code duplication between the frontend and backend projects.",
"main": "src/index.js",
"author": "",
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,5 @@ function determineMergeGoalStatus(statuses) {
}

module.exports = {
determineMergeGoalStatus
determineMergeGoalStatus,
}
24 changes: 15 additions & 9 deletions src/models/hooks/activityReport.js → src/hooks/activityReport.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,35 @@
import escapeFields from '../models/helpers/escapeFields';

const { Op } = require('sequelize');
const { REPORT_STATUSES } = require('@ttahub/common');
const {
OBJECTIVE_STATUS,
AWS_ELASTIC_SEARCH_INDEXES,
GOAL_COLLABORATORS,
OBJECTIVE_COLLABORATORS,
} = require('../../constants');
const { auditLogger } = require('../../logger');
} = require('../constants');
const { auditLogger } = require('../logger');
const { findOrCreateGoalTemplate } = require('./goal');
const { GOAL_STATUS } = require('../../constants');
const { GOAL_STATUS } = require('../constants');
const { findOrCreateObjectiveTemplate } = require('./objective');
const {
scheduleUpdateIndexDocumentJob,
scheduleDeleteIndexDocumentJob,
} = require('../../lib/awsElasticSearch/queueManager');
const { collectModelData } = require('../../lib/awsElasticSearch/datacollector');
const { formatModelForAwsElasticsearch } = require('../../lib/awsElasticSearch/modelMapper');
const { addIndexDocument, deleteIndexDocument } = require('../../lib/awsElasticSearch/index');
} = require('../lib/awsElasticSearch/queueManager');
const { collectModelData } = require('../lib/awsElasticSearch/datacollector');
const { formatModelForAwsElasticsearch } = require('../lib/awsElasticSearch/modelMapper');
const { addIndexDocument, deleteIndexDocument } = require('../lib/awsElasticSearch/index');
const {
findOrCreateCollaborator,
removeCollaboratorsForType,
} = require('../helpers/genericCollaborator');
} = require('../models/helpers/genericCollaborator');
const { destroyLinkedSimilarityGroups } = require('./activityReportGoal');

const AR_FIELDS_TO_ESCAPE = ['additionalNotes', 'context'];

const processForEmbeddedResources = async (sequelize, instance, options) => {
// eslint-disable-next-line global-require
const { calculateIsAutoDetectedForActivityReport, processActivityReportForResourcesById } = require('../../services/resource');
const { calculateIsAutoDetectedForActivityReport, processActivityReportForResourcesById } = require('../services/resource');
const changed = instance.changed() || Object.keys(instance);
if (calculateIsAutoDetectedForActivityReport(changed)) {
await processActivityReportForResourcesById(instance.id);
Expand Down Expand Up @@ -816,6 +820,7 @@ const automaticGoalObjectiveStatusCachingOnApproval = async (sequelize, instance

const beforeCreate = async (instance) => {
copyStatus(instance);
escapeFields(instance, AR_FIELDS_TO_ESCAPE);
};

const getActivityReportDocument = async (sequelize, instance) => {
Expand Down Expand Up @@ -1033,6 +1038,7 @@ const beforeValidate = async (sequelize, instance, options) => {
};

const beforeUpdate = async (sequelize, instance, options) => {
escapeFields(instance, AR_FIELDS_TO_ESCAPE);
copyStatus(instance);
setSubmittedDate(sequelize, instance, options);
clearAdditionalNotes(sequelize, instance, options);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ import db, {
Recipient,
Grant,
User,
} from '..';
import { unlockReport } from '../../routes/activityReports/handlers';
import ActivityReportPolicy from '../../policies/activityReport';
} from '../models';
import { unlockReport } from '../routes/activityReports/handlers';
import ActivityReportPolicy from '../policies/activityReport';
import {
moveDraftGoalsToNotStartedOnSubmission,
propagateSubmissionStatus,
} from './activityReport';
import { auditLogger } from '../../logger';
import { auditLogger } from '../logger';

jest.mock('../../policies/activityReport');
jest.mock('../policies/activityReport');

describe('activity report model hooks', () => {
describe('automatic goal status changes', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { APPROVER_STATUSES, REPORT_STATUSES } = require('@ttahub/common');
const { default: escapeFields } = require('../models/helpers/escapeFields');

const FIELDS_TO_ESCAPE = ['note'];

/**
* Helper function called by model hooks.
Expand Down Expand Up @@ -124,9 +127,19 @@ const afterUpdate = async (sequelize, instance) => {
await updateReportStatus(sequelize, instance);
};

const beforeUpdate = async (_sequelize, instance) => {
escapeFields(instance, FIELDS_TO_ESCAPE);
};

const beforeCreate = async (_sequelize, instance) => {
escapeFields(instance, FIELDS_TO_ESCAPE);
};

export {
calculateReportStatusFromApprovals,
calculateReportStatus,
beforeCreate,
beforeUpdate,
afterCreate,
afterDestroy,
afterRestore,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
User,
ActivityReport,
ActivityReportApprover,
} from '..';
} from '../models';
import {
calculateReportStatusFromApprovals,
afterDestroy,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { REPORT_STATUSES } from '@ttahub/common';
import { propagateDestroyToFile } from './genericFile';

const { cleanupOrphanFiles } = require('../helpers/orphanCleanupHelper');
const { cleanupOrphanFiles } = require('../models/helpers/orphanCleanupHelper');

const checkForUseOnApprovedReport = async (sequelize, instance, options) => {
const activityReport = await sequelize.models.ActivityReport.findOne({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ import {
sequelize,
User,
ActivityReport,
} from '..';
} from '../models';
import { propagateDestroyToFile } from './genericFile';
import { cleanupOrphanFiles } from '../helpers/orphanCleanupHelper';
import { cleanupOrphanFiles } from '../models/helpers/orphanCleanupHelper';
import { draftObject, mockApprovers, approverUserIds } from './testHelpers';

jest.mock('./genericFile', () => ({
propagateDestroyToFile: jest.fn(),
}));

jest.mock('../helpers/orphanCleanupHelper', () => ({
jest.mock('../models/helpers/orphanCleanupHelper', () => ({
cleanupOrphanFiles: jest.fn(),
}));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
const { REPORT_STATUSES } = require('@ttahub/common');
const { GOAL_COLLABORATORS } = require('../../constants');
const { GOAL_COLLABORATORS } = require('../constants');
const {
currentUserPopulateCollaboratorForType,
removeCollaboratorsForType,
} = require('../helpers/genericCollaborator');
const { onlyAllowTrGoalSourceForGoalsCreatedViaTr } = require('../helpers/goalSource');
} = require('../models/helpers/genericCollaborator');
const { onlyAllowTrGoalSourceForGoalsCreatedViaTr } = require('../models/helpers/goalSource');

const processForEmbeddedResources = async (sequelize, instance, options) => {
// eslint-disable-next-line global-require
const { calculateIsAutoDetectedForActivityReportGoal, processActivityReportGoalForResourcesById } = require('../../services/resource');
const { calculateIsAutoDetectedForActivityReportGoal, processActivityReportGoalForResourcesById } = require('../services/resource');
const changed = instance.changed() || Object.keys(instance);
if (calculateIsAutoDetectedForActivityReportGoal(changed)) {
await processActivityReportGoalForResourcesById(instance.id);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { getSingularOrPluralData } = require('../helpers/hookMetadata');
const { cleanupOrphanResources } = require('../helpers/orphanCleanupHelper');
const { getSingularOrPluralData } = require('../models/helpers/hookMetadata');
const { cleanupOrphanResources } = require('../models/helpers/orphanCleanupHelper');

const propagateOnAR = async (sequelize, instance, options) => sequelize.models.GoalResource
.update(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
ActivityReportGoalResource,
User,
Resource,
} from '..';
} from '../models';
import { recalculateOnAR } from './activityReportGoalResource';

const draftObject = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Op } from 'sequelize';
import { validateChangedOrSetEnums } from '../helpers/enum';
import { OBJECTIVE_COLLABORATORS, OBJECTIVE_STATUS } from '../../constants';
import {
const { Op } = require('sequelize');
const { validateChangedOrSetEnums } = require('../models/helpers/enum');
const { OBJECTIVE_COLLABORATORS, OBJECTIVE_STATUS } = require('../constants');
const {
currentUserPopulateCollaboratorForType,
removeCollaboratorsForType,
} from '../helpers/genericCollaborator';
} = require('../models/helpers/genericCollaborator');

const propagateDestroyToMetadata = async (sequelize, instance, options) => Promise.all(
[
Expand Down Expand Up @@ -97,7 +97,7 @@ const afterCreate = async (sequelize, instance, options) => {
await propagateSupportTypeToObjective(sequelize, instance, options);
};

const beforeValidate = async (sequelize, instance, options) => {
const beforeValidate = async (sequelize, instance) => {
validateChangedOrSetEnums(sequelize, instance);
};

Expand All @@ -110,7 +110,7 @@ const afterDestroy = async (sequelize, instance, options) => {
await recalculateOnAR(sequelize, instance, options);
};

export {
module.exports = {
propagateDestroyToMetadata,
recalculateOnAR,
afterCreate,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { SUPPORT_TYPES } from '@ttahub/common';
import {
sequelize,
ActivityReportObjective,
Expand All @@ -9,17 +10,20 @@ import {
File,
ActivityReport,
Topic,
} from '..';
} from '../models';

import { draftObject } from './testHelpers';
import { FILE_STATUSES, OBJECTIVE_STATUS } from '../../constants';
import { FILE_STATUSES, OBJECTIVE_STATUS } from '../constants';
import { beforeDestroy } from './activityReportObjective';
import { processObjectiveForResourcesById, processActivityReportObjectiveForResourcesById } from '../../services/resource';
import { processObjectiveForResourcesById, processActivityReportObjectiveForResourcesById } from '../services/resource';

describe('activityReportObjective hooks', () => {
let ar;
let topic;
let objective;
let secondObjective;
let thirdObjective;

let aro;
let file;

Expand All @@ -31,10 +35,32 @@ describe('activityReportObjective hooks', () => {
status: OBJECTIVE_STATUS.IN_PROGRESS,
});

secondObjective = await Objective.create({
title: 'second test objective',
status: OBJECTIVE_STATUS.IN_PROGRESS,
});

thirdObjective = await Objective.create({
title: 'third test objective',
status: OBJECTIVE_STATUS.IN_PROGRESS,
supportType: SUPPORT_TYPES[0],
});

aro = await ActivityReportObjective.create({
activityReportId: ar.id,
objectiveId: objective.id,
});
supportType: SUPPORT_TYPES[0],
}, { individualHooks: true });

await ActivityReportObjective.create({
activityReportId: ar.id,
objectiveId: secondObjective.id,
}, { individualHooks: true });

await ActivityReportObjective.create({
activityReportId: ar.id,
objectiveId: thirdObjective.id,
}, { individualHooks: true });

topic = await Topic.create({
name: 'Javascript Mastery',
Expand Down Expand Up @@ -87,11 +113,23 @@ describe('activityReportObjective hooks', () => {
});

await ActivityReportObjective.destroy({
where: { id: aro.id },
where: {
objectiveId: [
objective.id,
secondObjective.id,
thirdObjective.id,
],
},
});

await Objective.destroy({
where: { id: objective.id },
where: {
id: [
objective.id,
secondObjective.id,
thirdObjective.id,
],
},
force: true,
});

Expand All @@ -102,6 +140,18 @@ describe('activityReportObjective hooks', () => {
await sequelize.close();
});

describe('supportType', () => {
it('should propogate supportType to objective', async () => {
const objective1 = await Objective.findByPk(objective.id);
const objective2 = await Objective.findByPk(secondObjective.id);
const objective3 = await Objective.findByPk(thirdObjective.id);

expect(objective1.supportType).toBe(SUPPORT_TYPES[0]);
expect(objective2.supportType).toBe(null);
expect(objective3.supportType).toBe(SUPPORT_TYPES[0]);
});
});

describe('beforeDestroy', () => {
it('should propagate destroy to metadata', async () => {
const transaction = await sequelize.transaction();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { getSingularOrPluralData } = require('../helpers/hookMetadata');
const { cleanupOrphanFiles } = require('../helpers/orphanCleanupHelper');
const { getSingularOrPluralData } = require('../models/helpers/hookMetadata');
const { cleanupOrphanFiles } = require('../models/helpers/orphanCleanupHelper');

const recalculateOnAR = async (sequelize, instance, options) => {
// check to see if objectiveId or objectiveIds is validly defined
Expand All @@ -9,8 +9,7 @@ const recalculateOnAR = async (sequelize, instance, options) => {
let fileOnReport;
// by using the passed in objectives we can use a more performant version of the query
if (objectiveIds !== undefined
&& Array.isArray(objectiveIds)
&& objectiveIds.map((i) => typeof i).every((i) => i === 'number')) {
&& Array.isArray(objectiveIds)) {
fileOnReport = `
SELECT
f."id",
Expand Down
Loading