feat: Add challenge validation middleware

api-server/server/boot/challenge.js

@@ -7,8 +7,10 @@
 import { Observable } from 'rx';
 import { isEmpty, pick, omit, find, uniqBy, last } from 'lodash';
 import debug from 'debug';
-import accepts from 'accepts';
 import dedent from 'dedent';
+import { ObjectID } from 'mongodb';
+import isNumeric from 'validator/lib/isNumeric';
+import isURL from 'validator/lib/isURL';
 
 import { homeLocation } from '../../../config/env';
 
@@ -31,28 +33,21 @@ export default async function bootChallenge(app, done) {
   api.post(
     '/modern-challenge-completed',
     send200toNonUser,
+    isValidChallengeCompletion,
     modernChallengeCompleted
   );
 
-  // deprecate endpoint
-  // remove once new endpoint is live
-  api.post('/completed-challenge', send200toNonUser, completedChallenge);
-
-  api.post('/challenge-completed', send200toNonUser, completedChallenge);
-
-  // deprecate endpoint
-  // remove once new endpoint is live
   api.post(
-    '/completed-zipline-or-basejump',
+    '/project-completed',
     send200toNonUser,
+    isValidChallengeCompletion,
     projectCompleted
   );
 
-  api.post('/project-completed', send200toNonUser, projectCompleted);
-
   api.post(
     '/backend-challenge-completed',
     send200toNonUser,
+    isValidChallengeCompletion,
     backendChallengeCompleted
   );
 
@@ -65,7 +60,6 @@ export default async function bootChallenge(app, done) {
   router.get('/map', redirectToLearn);
 
   app.use(api);
-  app.use('/external', api);
   app.use('/internal', api);
   app.use(router);
   done();
@@ -191,20 +185,27 @@ export async function createChallengeUrlResolver(
   };
 }
 
-function modernChallengeCompleted(req, res, next) {
+export function isValidChallengeCompletion(req, res, next) {
-  const type = accepts(req).type('html', 'json', 'text');
+  const {
-  req.checkBody('id', 'id must be an ObjectId').isMongoId();
+    body: { id, challengeType, solution }
+  } = req;
 
-  const errors = req.validationErrors(true);
+  if (!ObjectID.isValid(id)) {
-  if (errors) {
+    log('isObjectId', id, ObjectID.isValid(id));
-    if (type === 'json') {
-      return res.status(403).send({ errors });
-    }
-
-    log('errors', errors);
     return res.sendStatus(403);
   }
+  if ('challengeType' in req.body && !isNumeric(challengeType)) {
+    log('challengeType', challengeType, isNumeric(challengeType));
+    return res.sendStatus(403);
+  }
+  if ('solution' in req.body && !isURL(solution)) {
+    log('isObjectId', id, ObjectID.isValid(id));
+    return res.sendStatus(403);
+  }
+  return next();
+}
 
+export function modernChallengeCompleted(req, res, next) {
   const user = req.user;
   return user
     .getCompletedChallenges$()
@@ -228,88 +229,17 @@ function modernChallengeCompleted(req, res, next) {
         })
       );
       return Observable.fromPromise(updatePromise).map(() => {
-        if (type === 'json') {
         return res.json({
           points,
           alreadyCompleted,
           completedDate
         });
-        }
-        return res.sendStatus(200);
-      });
-    })
-    .subscribe(() => {}, next);
-}
-
-function completedChallenge(req, res, next) {
-  req.checkBody('id', 'id must be an ObjectId').isMongoId();
-  const type = accepts(req).type('html', 'json', 'text');
-  const errors = req.validationErrors(true);
-
-  const { user } = req;
-
-  if (errors) {
-    if (type === 'json') {
-      return res.status(403).send({ errors });
-    }
-
-    log('errors', errors);
-    return res.sendStatus(403);
-  }
-
-  return user
-    .getCompletedChallenges$()
-    .flatMap(() => {
-      const completedDate = Date.now();
-      const { id, solution, timezone, files } = req.body;
-
-      const { alreadyCompleted, updateData } = buildUserUpdate(
-        user,
-        id,
-        { id, solution, completedDate, files },
-        timezone
-      );
-
-      const points = alreadyCompleted ? user.points : user.points + 1;
-
-      const updatePromise = new Promise((resolve, reject) =>
-        user.updateAttributes(updateData, err => {
-          if (err) {
-            return reject(err);
-          }
-          return resolve();
-        })
-      );
-      return Observable.fromPromise(updatePromise).map(() => {
-        if (type === 'json') {
-          return res.json({
-            points,
-            alreadyCompleted,
-            completedDate
-          });
-        }
-        return res.sendStatus(200);
       });
     })
     .subscribe(() => {}, next);
 }
 
 function projectCompleted(req, res, next) {
-  const type = accepts(req).type('html', 'json', 'text');
-  req.checkBody('id', 'id must be an ObjectId').isMongoId();
-  req.checkBody('challengeType', 'must be a number').isNumber();
-  req.checkBody('solution', 'solution must be a URL').isURL();
-
-  const errors = req.validationErrors(true);
-
-  if (errors) {
-    if (type === 'json') {
-      return res.status(403).send({ errors });
-    }
-    log('errors', errors);
-    return res.sendStatus(403);
-  }
-
   const { user, body = {} } = req;
 
   const completedChallenge = pick(body, [
@@ -351,34 +281,17 @@ function projectCompleted(req, res, next) {
         })
       );
       return Observable.fromPromise(updatePromise).doOnNext(() => {
-        if (type === 'json') {
         return res.send({
           alreadyCompleted,
           points: alreadyCompleted ? user.points : user.points + 1,
           completedDate: completedChallenge.completedDate
         });
-        }
-        return res.status(200).send(true);
       });
     })
     .subscribe(() => {}, next);
 }
 
 function backendChallengeCompleted(req, res, next) {
-  const type = accepts(req).type('html', 'json', 'text');
-  req.checkBody('id', 'id must be an ObjectId').isMongoId();
-  req.checkBody('solution', 'solution must be a URL').isURL();
-
-  const errors = req.validationErrors(true);
-
-  if (errors) {
-    if (type === 'json') {
-      return res.status(403).send({ errors });
-    }
-    log('errors', errors);
-    return res.sendStatus(403);
-  }
-
   const { user, body = {} } = req;
 
   const completedChallenge = pick(body, ['id', 'solution']);
@@ -402,14 +315,11 @@ function backendChallengeCompleted(req, res, next) {
         })
       );
       return Observable.fromPromise(updatePromise).doOnNext(() => {
-        if (type === 'json') {
         return res.send({
           alreadyCompleted,
           points: alreadyCompleted ? user.points : user.points + 1,
           completedDate: completedChallenge.completedDate
         });
-        }
-        return res.status(200).send(true);
       });
     })
     .subscribe(() => {}, next);

api-server/server/boot_tests/challenge.test.js

@@ -7,7 +7,8 @@ import {
   buildChallengeUrl,
   createChallengeUrlResolver,
   createRedirectToCurrentChallenge,
-  getFirstChallenge
+  getFirstChallenge,
+  isValidChallengeCompletion
 } from '../boot/challenge';
 
 const firstChallengeUrl = '/learn/the/first/challenge';
@@ -44,6 +45,7 @@ const mockApp = {
 };
 const mockGetFirstChallenge = () => firstChallengeUrl;
 const firstChallengeQuery = {
+  // first challenge of the first block of the first superBlock
   where: { challengeOrder: 0, superOrder: 1, order: 0 }
 };
 
@@ -100,8 +102,6 @@ describe('boot/challenge', () => {
     });
   });
 
-  xdescribe('completedChallenge');
-
   describe('getFirstChallenge', () => {
     const createMockChallengeModel = success =>
       success
@@ -129,9 +129,92 @@ describe('boot/challenge', () => {
     });
   });
 
-  xdescribe('modernChallengeCompleted');
+  describe('isValidChallengeCompletion', () => {
+    const validObjectId = '5c716d1801013c3ce3aa23e6';
 
-  xdescribe('projectcompleted');
+    it('declares a 403 for an invalid id in the body', () => {
+      expect.assertions(3);
+      const req = mockReq({
+        body: { id: 'not-a-real-id' }
+      });
+      const res = mockRes();
+      const next = sinon.spy();
+
+      isValidChallengeCompletion(req, res, next);
+
+      expect(res.sendStatus.called).toBe(true);
+      expect(res.sendStatus.getCall(0).args[0]).toBe(403);
+      expect(next.called).toBe(false);
+    });
+
+    it('declares a 403 for an invalid challengeType in the body', () => {
+      expect.assertions(3);
+      const req = mockReq({
+        body: { id: validObjectId, challengeType: 'ponyfoo' }
+      });
+      const res = mockRes();
+      const next = sinon.spy();
+
+      isValidChallengeCompletion(req, res, next);
+
+      expect(res.sendStatus.called).toBe(true);
+      expect(res.sendStatus.getCall(0).args[0]).toBe(403);
+      expect(next.called).toBe(false);
+    });
+
+    it('declares a 403 for an invalid solution in the body', () => {
+      expect.assertions(3);
+      const req = mockReq({
+        body: {
+          id: validObjectId,
+          challengeType: '1',
+          solution: 'https://not-a-url'
+        }
+      });
+      const res = mockRes();
+      const next = sinon.spy();
+
+      isValidChallengeCompletion(req, res, next);
+
+      expect(res.sendStatus.called).toBe(true);
+      expect(res.sendStatus.getCall(0).args[0]).toBe(403);
+      expect(next.called).toBe(false);
+    });
+
+    it('calls next if the body is valid', () => {
+      const req = mockReq({
+        body: {
+          id: validObjectId,
+          challengeType: '1',
+          solution: 'https://www.freecodecamp.org'
+        }
+      });
+      const res = mockRes();
+      const next = sinon.spy();
+
+      isValidChallengeCompletion(req, res, next);
+
+      expect(next.called).toBe(true);
+    });
+
+    it('calls next if only the id is provided', () => {
+      const req = mockReq({
+        body: {
+          id: validObjectId
+        }
+      });
+      const res = mockRes();
+      const next = sinon.spy();
+
+      isValidChallengeCompletion(req, res, next);
+
+      expect(next.called).toBe(true);
+    });
+  });
+
+  xdescribe('modernChallengeCompleted', () => {});
+
+  xdescribe('projectCompleted');
 
   describe('redirectToCurrentChallenge', () => {
     const mockHomeLocation = 'https://www.example.com';