Merge pull request #752 from topcoder-platform/develop

PLAT-2614 Ampersand symbol getting converted

Author: ThomasKranitsas

.circleci/config.yml

@@ -154,7 +154,7 @@ workflows:
             - UnitTests
           filters:
             branches:
-              only: ['develop', 'connect-performance-testing', 'feature/new-milestone-concept']
+              only: ['develop', 'connect-performance-testing', 'feature/new-milestone-concept','feature/PLAT-2614']
       - deployProd:
           context : org-global
           requires:

config/default.json

@@ -14,6 +14,8 @@
   "directProjectServiceEndpoint": "",
   "directProjectServiceTimeout": 5000,
   "attachmentsS3Bucket": "topcoder-prod-media",
+  "attachmentsDMZS3Bucket": "topcoder-prod-media-dmz",
+  "attachmentsQuarantineS3Bucket": "topcoder-prod-media-quarantine",
   "projectAttachmentPathPrefix": "projects",
   "projectAttachmentPathSuffix": "attachments",
   "elasticsearchConfig": {
@@ -87,4 +89,4 @@
   },
   "STRIPE_SECRET_KEY": "",
   "sfdcBillingAccountNameField": "Billing_Account_Name__c"
-}
+}

config/development.json

@@ -2,6 +2,8 @@
   "pubsubQueueName": "dev.project.service",
   "pubsubExchangeName": "dev.projects",
   "attachmentsS3Bucket": "topcoder-dev-media",
+  "attachmentsDMZS3Bucket": "topcoder-dev-media-dmz",
+  "attachmentsQuarantineS3Bucket": "topcoder-dev-media-quarantine",
   "connectProjectsUrl": "https://connect.topcoder-dev.com/projects/",
   "fileServiceEndpoint": "https://api.topcoder-dev.com/v3/files/",
   "connectProjectsUrl": "https://connect.topcoder-dev.com/projects/",

package.json

@@ -57,7 +57,6 @@
     "express": "^4.13.4",
     "express-list-routes": "^0.1.4",
     "express-request-id": "^1.1.0",
-    "express-sanitizer": "^1.0.2",
     "express-validation": "^1.0.3",
     "handlebars": "^4.5.3",
     "http-aws-es": "^4.0.0",
@@ -70,13 +69,14 @@
     "moment": "^2.22.2",
     "no-kafka": "^3.4.3",
     "pg": "^7.11.0",
-    "pg-native": "^3.0.0",
+    "pg-native": "^3.0.1",
     "sequelize": "^5.8.7",
     "stripe": "^8.195.0",
     "swagger-ui-express": "^4.0.6",
     "tc-core-library-js": "github:appirio-tech/tc-core-library-js#v2.6.6",
     "traverse": "^0.6.6",
     "urlencode": "^1.1.0",
+    "xss": "^1.0.14",
     "yamljs": "^0.3.0"
   },
   "devDependencies": {
@@ -102,5 +102,8 @@
     "sinon": "^1.17.4",
     "sinon-chai": "^2.8.0",
     "supertest": "^4.0.2"
+  },
+  "volta": {
+    "node": "12.22.12"
   }
 }

src/app.js

@@ -2,7 +2,6 @@ import express from 'express';
 import methodOverride from 'method-override';
 import _ from 'lodash';
 import bodyParser from 'body-parser';
-import expressSanitizer from 'express-sanitizer';
 import config from 'config';
 import cors from 'cors';
 import coreLib from 'tc-core-library-js';
@@ -36,7 +35,6 @@ app.use(bodyParser.urlencoded({
   extended: false,
 }));
 app.use(bodyParser.json());
-app.use(expressSanitizer());
 
 // add request Id
 const addRequestId = expressRequestId();

src/constants.js

@@ -139,6 +139,8 @@ export const EVENT = {
 };
 
 export const BUS_API_EVENT = {
+  AV_SCAN_REQUEST: 'avscan.action.scan',
+
   PROJECT_CREATED: 'project.action.create',
   PROJECT_UPDATED: 'project.action.update',
   PROJECT_DELETED: 'project.action.delete',
@@ -151,6 +153,7 @@ export const BUS_API_EVENT = {
   PROJECT_ATTACHMENT_ADDED: 'project.action.create',
   PROJECT_ATTACHMENT_REMOVED: 'project.action.delete',
   PROJECT_ATTACHMENT_UPDATED: 'project.action.update',
+  PROJECT_ATTACHMENT_SCAN_RESULT: 'avscan.projects.assets.result',
 
   // When phase is added/updated/deleted from the project,
   // When product is added/deleted from a phase

src/events/attachments/index.js

@@ -0,0 +1,88 @@
+/**
+ * Event handlers for attachment create, and av scan result
+ */
+import Joi from 'joi';
+import config from 'config';
+import util from '../../util';
+import { BUS_API_EVENT } from '../../constants';
+import { createEvent } from '../../services/busApi';
+
+/**
+ * Payload for new unified BUS events like `avscan.projects.assets.result` with `resource=attachment`
+ */
+const attachmentScanResultPayloadSchema = Joi.object().keys({
+  url: Joi.string().required(),
+  fileName: Joi.string().required(),
+  isInfected: Joi.boolean().required(),
+}).unknown(true).required();
+
+/**
+ * Updates project activity fields. throws exceptions in case of error
+ * @param   {Object}  app       Application object used to interact with RMQ service
+ * @param   {String}  topic     Kafka topic
+ * @param   {Object}  payload   Message payload
+ * @return  {Promise} Promise
+ */
+async function attachmentScanResultKafkaHandler(app, topic, payload) {
+  // Validate payload
+  const result = Joi.validate(payload, attachmentScanResultPayloadSchema);
+  if (result.error) {
+    throw new Error(result.error);
+  }
+  const sourceBucket = config.get('attachmentsDMZS3Bucket');
+  // if the attachment is infected, move it to the quarantine s3 bucket, if not move it to the clean s3 bucket
+  if (payload.isInfected) {
+    // move to quarantine
+    const destBucket = config.get('attachmentsQuarantineS3Bucket');
+    util.s3FileTransfer({ log: app.logger }, sourceBucket, payload.path, destBucket, payload.path);
+    app.logger.debug(`Attachment ${payload.fileName} is infected, moving to quarantine`);
+  } else {
+    // move to clean
+    const destBucket = config.get('attachmentsS3Bucket');
+    util.s3FileTransfer({ log: app.logger }, sourceBucket, payload.path, destBucket, payload.path);
+  }
+}
+
+/**
+ * Payload for new unified BUS events like `avscan.action.scan` with `resource=attachment`
+ */
+const attachmentPayloadSchema = Joi.object().keys({
+  path: Joi.string().required(),
+}).unknown(true).required();
+
+/**
+ * Attachment Created BUS API event handler.
+ * - requests av scan by posting a kafka message to `avscan.action.scan` topic
+ * - throws exceptions in case of error
+ *
+ * @param   {Object}  app       Application object
+ * @param   {String}  topic     Kafka topic
+ * @param   {Object}  payload   Message payload
+ * @return  {Promise} Promise
+ */
+async function attachmentCreatedKafkaHandler(app, topic, payload) {
+  // Validate payload
+  const result = Joi.validate(payload, attachmentPayloadSchema);
+  if (result.error) {
+    throw new Error(result.error);
+  }
+  // Construct s3 url
+  const avScanPayload = {
+    url: `https://${config.get('attachmentsDMZS3Bucket')}.s3.amazonaws.com/${encodeURIComponent(payload.path)}`,
+    path: payload.path,
+    fileName: payload.path.split('/').pop(),
+    moveFile: false,
+    callbackOption: 'kafka',
+    callbackKafkaTopic: BUS_API_EVENT.PROJECT_ATTACHMENT_SCAN_RESULT,
+  };
+  await createEvent(
+    BUS_API_EVENT.AV_SCAN_REQUEST,
+    avScanPayload,
+    app.logger,
+  );
+}
+
+module.exports = {
+  attachmentScanResultKafkaHandler,
+  attachmentCreatedKafkaHandler,
+};

src/events/kafkaHandlers.js

@@ -17,6 +17,10 @@ import {
 } from './projectPhases';
 import { timelineAdjustedKafkaHandler } from './timelines';
 import { milestoneUpdatedKafkaHandler } from './milestones';
+import {
+  attachmentScanResultKafkaHandler,
+  attachmentCreatedKafkaHandler,
+} from './attachments';
 
 const kafkaHandlers = {
   /**
@@ -37,6 +41,9 @@ const kafkaHandlers = {
   // Events coming from timeline/milestones (considering it as a separate module/service in future)
   [CONNECT_NOTIFICATION_EVENT.MILESTONE_TRANSITION_COMPLETED]: milestoneUpdatedKafkaHandler,
   [CONNECT_NOTIFICATION_EVENT.TIMELINE_ADJUSTED]: timelineAdjustedKafkaHandler,
+
+  // Events coming from attachments
+  [BUS_API_EVENT.PROJECT_ATTACHMENT_SCAN_RESULT]: attachmentScanResultKafkaHandler,
 };
 
 /**
@@ -95,6 +102,10 @@ registerKafkaHandler(
   RESOURCES.PHASE,
   projectPhaseRemovedKafkaHandler,
 );
-
+registerKafkaHandler(
+  BUS_API_EVENT.PROJECT_ATTACHMENT_ADDED,
+  RESOURCES.ATTACHMENT,
+  attachmentCreatedKafkaHandler,
+);
 
 export default kafkaHandlers;

src/routes/attachments/create.js

@@ -70,7 +70,7 @@ module.exports = [
 
     const sourceBucket = data.s3Bucket;
     const sourceKey = data.path;
-    const destBucket = config.get('attachmentsS3Bucket');
+    const destBucket = config.get('attachmentsDMZS3Bucket');
     const destKey = path;
 
     if (data.type === ATTACHMENT_TYPES.LINK) {

src/routes/projects/create.js

@@ -14,6 +14,7 @@ import util from '../../util';
 import { PERMISSION } from '../../permissions/constants';
 
 const traverse = require('traverse');
+const xss = require('xss');
 
 /**
  * API to handle creating a new project.
@@ -418,7 +419,11 @@ module.exports = [
       // keep the raw '&&' string in conditions string in estimation
       const isEstimationCondition =
         (this.path.length === 3) && (this.path[0] === 'estimation') && (this.key === 'conditions');
-      if (this.isLeaf && typeof x === 'string' && (!isEstimationCondition)) this.update(req.sanitize(x));
+      // if (this.isLeaf && typeof x === 'string' && (!isEstimationCondition)) this.update(req.sanitize(x));
+      if (this.isLeaf && typeof x === 'string' && !isEstimationCondition) {
+        const sanitizedData = xss(x);
+        this.update(sanitizedData);
+      }
     });
     // override values
     _.assign(project, {

src/routes/projects/update.js

@@ -15,6 +15,7 @@ import util from '../../util';
 import { PERMISSION } from '../../permissions/constants';
 
 const traverse = require('traverse');
+const xss = require('xss');
 
 /**
  * API to handle updating a project.
@@ -190,7 +191,11 @@ module.exports = [
     // prune any fields that cannot be updated directly
     updatedProps = _.omit(updatedProps, ['createdBy', 'createdAt', 'updatedBy', 'updatedAt', 'id']);
     traverse(updatedProps).forEach(function (x) { // eslint-disable-line func-names
-      if (x && this.isLeaf && typeof x === 'string') this.update(req.sanitize(x));
+      // if (x && this.isLeaf && typeof x === 'string') this.update(req.sanitize(x));
+      if (x && this.isLeaf && typeof x === 'string') {
+        const sanitizedData = xss(x);
+        this.update(sanitizedData);
+      }
     });
     let previousValue;
     models.sequelize.transaction(() => models.Project.findOne({