fix(security): scope screenshots, baselines and metrics to the caller's team

app/controllers/baseline.controller.js

@@ -1,6 +1,7 @@
 const { validationResult } = require('express-validator');
 const debug = require('debug');
 const Baseline = require('../models/baseline.js');
+const Build = require('../models/build.js');
 const Screenshot = require('../models/screenshot.js');
 const validationUtils = require('../utils/validation-utils.js');
 const baselineUtils = require('../utils/baseline-utils.js');
@@ -15,6 +16,22 @@ const {
 
 const log = debug('baseline:controller');
 
+// The team a screenshot belongs to, through its build. A screenshot whose build is gone
+// cannot be attributed to a team, so it cannot back a baseline.
+const teamOfScreenshot = async (screenshot) => {
+  const build = await Build.findById(screenshot.build).select('team').lean().exec();
+  if (!build) {
+    throw new NotFoundError(`No build found for screenshot with id ${screenshot._id}`);
+  }
+  return build.team;
+};
+
+// A baseline is readable and editable by its team. One written before baselines recorded
+// a team (and not yet backfilled) cannot be attributed, so only an admin may touch it.
+const hasBaselineAccess = (user, baseline) => (baseline.team
+  ? authMiddleware.hasTeamAccess(user, baseline.team)
+  : Boolean(user && user.role === 'admin'));
+
 // Create and save a new test execution
 exports.create = (req, res) => {
   // check the request is valid
@@ -24,18 +41,21 @@ exports.create = (req, res) => {
   }
 
   const { screenshotId, view: requestView, ignoreBoxes } = req.body;
-  // determine if type is defined, otherwise set it to IMAGE (and handle default).
-  const promises = [
-    Screenshot.findById(screenshotId).lean().exec(),
-    Baseline.find({ view: requestView }).lean().exec(),
-  ];
-  return Promise.all(promises)
-    .then((results) => {
-      const screenshot = results[0];
-      const baselinesFound = results[1];
+  let team;
+  return Screenshot.findById(screenshotId).lean().exec()
+    .then(async (screenshot) => {
       if (!screenshot) {
         throw new NotFoundError(`No screenshot found with id ${screenshotId}`);
       }
+      team = await teamOfScreenshot(screenshot);
+      if (!authMiddleware.hasTeamAccess(req.user, team)) {
+        throw new ForbiddenError('You do not have access to this screenshot');
+      }
+      // Only this team's baselines count: another team may use the same view name.
+      const baselinesFound = await Baseline.find({ team, view: requestView }).lean().exec();
+      return { screenshot, baselinesFound };
+    })
+    .then(({ screenshot, baselinesFound }) => {
       const { view: screenshotView, platform } = screenshot;
       if (screenshotView !== requestView) {
         throw new InvalidRequestError(`The screenshot with id ${screenshotId} is not for the same view. Expected [${screenshotView}], Actual [${requestView}]`);
@@ -61,7 +81,7 @@ exports.create = (req, res) => {
           throw new ConflictError(`Baseline for view [${requestView}], platform [${platformName}] and browser [${browserName}] with resolution [${width} x ${height}] already exists`);
         }
       }
-      const baseline = baselineUtils.createBaseline(requestView, screenshot, ignoreBoxes);
+      const baseline = baselineUtils.createBaseline(requestView, screenshot, ignoreBoxes, team);
       return baseline.save();
     })
     .then((savedBaseline) => {
@@ -85,11 +105,23 @@ exports.findAll = (req, res) => {
     browserName,
     screenHeight,
     screenWidth,
+    teamId,
   } = req.query;
   const baseLineQuery = {
     view,
     'platform.platformName': platformName,
   };
+  // Baselines are per team. A named team must be one the caller can read; otherwise the
+  // result is limited to the caller's own teams (admins, who can read every team, are not
+  // limited, so an admin should name the team when view names collide across teams).
+  if (teamId) {
+    if (!authMiddleware.hasTeamAccess(req.user, teamId)) {
+      return handleError(new ForbiddenError('You do not have access to this team'), res);
+    }
+    baseLineQuery.team = teamId;
+  } else if (!req.user || req.user.role !== 'admin') {
+    baseLineQuery.team = { $in: (req.user && req.user.teams) || [] };
+  }
   if (deviceName) baseLineQuery['platform.deviceName'] = deviceName;
   if (browserName) baseLineQuery['platform.browserName'] = browserName;
   if (screenHeight) baseLineQuery.screenHeight = screenHeight;
@@ -106,12 +138,15 @@ exports.findOne = (req, res) => {
   if (!errors.isEmpty()) {
     return res.status(422).json({ errors: errors.array() });
   }
-  const { baselineId } = req.query;
+  const { baselineId } = req.params;
   return Baseline.findById(baselineId).lean()
     .then((baseline) => {
       if (!baseline) {
         throw new NotFoundError(`Baseline not found with id ${baselineId}`);
       }
+      if (!hasBaselineAccess(req.user, baseline)) {
+        throw new ForbiddenError('You do not have access to this baseline');
+      }
       return res.status(200).send(baseline);
     }).catch((err) => handleError(err, res));
 };
@@ -133,7 +168,7 @@ exports.update = (req, res) => {
     Baseline.findById(baselineId).exec(),
   ];
   return Promise.all(promises)
-    .then((results) => {
+    .then(async (results) => {
       const screenshot = results[0];
       const baselineFound = results[1];
       if (screenshotId && !screenshot) {
@@ -142,6 +177,20 @@ exports.update = (req, res) => {
       if (!baselineFound) {
         throw new NotFoundError(`Baseline not found with id ${baselineId}`);
       }
+      if (!hasBaselineAccess(req.user, baselineFound)) {
+        throw new ForbiddenError('You do not have access to this baseline');
+      }
+      // The new image has to come from the baseline's own team, or a baseline could be
+      // pointed at (and so expose) another team's screenshot.
+      if (screenshotId) {
+        const screenshotTeam = await teamOfScreenshot(screenshot);
+        const baselineTeam = baselineFound.team || screenshotTeam;
+        if (screenshotTeam.toString() !== baselineTeam.toString()
+          || !authMiddleware.hasTeamAccess(req.user, screenshotTeam)) {
+          throw new ForbiddenError('The screenshot must belong to the same team as the baseline');
+        }
+        if (!baselineFound.team) baselineFound.team = screenshotTeam;
+      }
       if (screenshotId && screenshot.view !== baselineFound.view) {
         throw new InvalidRequestError(`The screenshot with id ${screenshotId} has a different view to the baseline and therefore can not be used for the requested baseline. Expected [${screenshot.view}], Actual [${baselineFound.view}].`);
       }
@@ -174,7 +223,10 @@ exports.delete = (req, res) => {
       if (!baselineFound) {
         throw new NotFoundError(`Baseline not found with id ${baselineId}`);
       }
-      if (!authMiddleware.hasTeamLeadAccess(req.user, baselineFound.screenshot.build.team)) {
+      const team = baselineFound.team
+        || (baselineFound.screenshot && baselineFound.screenshot.build
+          && baselineFound.screenshot.build.team);
+      if (!team || !authMiddleware.hasTeamLeadAccess(req.user, team)) {
         throw new ForbiddenError('You do not have permission to delete this baseline');
       }
       return Baseline.findByIdAndRemove(baselineId);

app/controllers/execution.controller.js

@@ -38,6 +38,9 @@ exports.create = (req, res) => {
       if (!buildFound) {
         throw new NotFoundError(`No build found with id ${buildId}`);
       }
+      if (!authMiddleware.hasTeamAccess(req.user, buildFound.team)) {
+        throw new ForbiddenError('You do not have access to this build');
+      }
       testExecution = buildMetricsUtils.createExecution(req, buildFound);
       return testExecution.save();
     })

app/controllers/metrics.controller.js

@@ -7,7 +7,8 @@ const { Team } = require('../models/team.js');
 // const Environment = require('../models/environment.js');
 // const Screenshot = require('../models/screenshot.js');
 const Execution = require('../models/execution.js');
-const { handleError, NotFoundError } = require('../exceptions/errors.js');
+const { handleError, NotFoundError, ForbiddenError } = require('../exceptions/errors.js');
+const authMiddleware = require('../utils/auth-middleware.js');
 // const Baseline = require('../models/baseline.js');
 // const Phase = require('../models/phase.js');
 
@@ -68,6 +69,9 @@ exports.retrieveMetricsPerPhase = (req, res) => {
       if (!teamFound) {
         throw new NotFoundError(`No team found with id ${teamId}`);
       }
+      if (!authMiddleware.hasTeamAccess(req.user, teamFound._id)) {
+        throw new ForbiddenError('You do not have access to this team');
+      }
       const buildQuery = { team: teamFound._id };
       // Applied to the build query rather than the execution aggregation: the executions
       // are already scoped to these builds, so narrowing here narrows both. Absent means